Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

subscriber: replace dyn Write with a Writer type #1661

Merged
merged 5 commits into from
Oct 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions tracing-error/src/subscriber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,9 @@ where
if span.extensions().get::<FormattedFields<F>>().is_some() {
return;
}
let mut fields = String::new();
if self.format.format_fields(&mut fields, attrs).is_ok() {
span.extensions_mut()
.insert(FormattedFields::<F>::new(fields));
let mut fields = FormattedFields::<F>::new(String::new());
if self.format.format_fields(fields.as_writer(), attrs).is_ok() {
span.extensions_mut().insert(fields);
}
}

Expand Down
70 changes: 41 additions & 29 deletions tracing-subscriber/src/fmt/fmt_subscriber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -485,37 +485,46 @@ where
///
/// [extensions]: crate::registry::Extensions
#[derive(Default)]
pub struct FormattedFields<E> {
_format_event: PhantomData<fn(E)>,
pub struct FormattedFields<E: ?Sized> {
_format_fields: PhantomData<fn(E)>,
/// The formatted fields of a span.
pub fields: String,
}

impl<E> FormattedFields<E> {
impl<E: ?Sized> FormattedFields<E> {
/// Returns a new `FormattedFields`.
pub fn new(fields: String) -> Self {
Self {
fields,
_format_event: PhantomData,
_format_fields: PhantomData,
}
}

/// Returns a new [`format::Writer`] for writing to this `FormattedFields`.
///
/// The returned [`format::Writer`] can be used with the
/// [`FormatFields::format_fields`] method.
pub fn as_writer(&mut self) -> format::Writer<'_> {
format::Writer::new(&mut self.fields)
}
}

impl<E> fmt::Debug for FormattedFields<E> {
impl<E: ?Sized> fmt::Debug for FormattedFields<E> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("FormattedFields")
.field("fields", &self.fields)
.field("formatter", &format_args!("{}", std::any::type_name::<E>()))
.finish()
}
}

impl<E> fmt::Display for FormattedFields<E> {
impl<E: ?Sized> fmt::Display for FormattedFields<E> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.fields)
fmt::Display::fmt(&self.fields, f)
}
}

impl<E> Deref for FormattedFields<E> {
impl<E: ?Sized> Deref for FormattedFields<E> {
type Target = String;
fn deref(&self) -> &Self::Target {
&self.fields
Expand Down Expand Up @@ -552,13 +561,13 @@ where
let mut extensions = span.extensions_mut();

if extensions.get_mut::<FormattedFields<N>>().is_none() {
let mut buf = String::new();
if self.fmt_fields.format_fields(&mut buf, attrs).is_ok() {
let fmt_fields = FormattedFields {
fields: buf,
_format_event: PhantomData::<fn(N)>,
};
extensions.insert(fmt_fields);
let mut fields = FormattedFields::<N>::new(String::new());
if self
.fmt_fields
.format_fields(fields.as_writer(), attrs)
.is_ok()
{
extensions.insert(fields);
}
}

Expand All @@ -581,19 +590,18 @@ where
fn on_record(&self, id: &Id, values: &Record<'_>, ctx: Context<'_, C>) {
let span = ctx.span(id).expect("Span not found, this is a bug");
let mut extensions = span.extensions_mut();
if let Some(FormattedFields { ref mut fields, .. }) =
extensions.get_mut::<FormattedFields<N>>()
{
if let Some(fields) = extensions.get_mut::<FormattedFields<N>>() {
let _ = self.fmt_fields.add_fields(fields, values);
} else {
let mut buf = String::new();
if self.fmt_fields.format_fields(&mut buf, values).is_ok() {
let fmt_fields = FormattedFields {
fields: buf,
_format_event: PhantomData::<fn(N)>,
};
extensions.insert(fmt_fields);
}
return;
}

let mut fields = FormattedFields::<N>::new(String::new());
if self
.fmt_fields
.format_fields(fields.as_writer(), values)
.is_ok()
{
extensions.insert(fields);
}
}

Expand Down Expand Up @@ -695,7 +703,11 @@ where
};

let ctx = self.make_ctx(ctx);
if self.fmt_event.format_event(&ctx, &mut buf, event).is_ok() {
if self
.fmt_event
.format_event(&ctx, format::Writer::new(&mut buf), event)
.is_ok()
{
let mut writer = self.make_writer.make_writer_for(event.metadata());
let _ = io::Write::write_all(&mut writer, buf.as_bytes());
}
Expand Down Expand Up @@ -738,7 +750,7 @@ where
{
fn format_fields<R: RecordFields>(
&self,
writer: &'writer mut dyn fmt::Write,
writer: format::Writer<'writer>,
fields: R,
) -> fmt::Result {
self.fmt_fields.format_fields(writer, fields)
Expand Down
76 changes: 39 additions & 37 deletions tracing-subscriber/src/fmt/format/json.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::{Format, FormatEvent, FormatFields, FormatTime};
use super::{Format, FormatEvent, FormatFields, FormatTime, Writer};
use crate::{
field::{RecordFields, VisitOutput},
fmt::{
Expand Down Expand Up @@ -185,14 +185,14 @@ where
fn format_event(
&self,
ctx: &FmtContext<'_, C, N>,
writer: &mut dyn fmt::Write,
mut writer: Writer<'_>,
event: &Event<'_>,
) -> fmt::Result
where
C: Collect + for<'a> LookupSpan<'a>,
{
let mut timestamp = String::new();
self.timer.format_time(&mut timestamp)?;
self.timer.format_time(&mut Writer::new(&mut timestamp))?;

#[cfg(feature = "tracing-log")]
let normalized_meta = event.normalized_metadata();
Expand All @@ -202,7 +202,7 @@ where
let meta = event.metadata();

let mut visit = || {
let mut serializer = Serializer::new(WriteAdaptor::new(writer));
let mut serializer = Serializer::new(WriteAdaptor::new(&mut writer));

let mut serializer = serializer.serialize_map(None)?;

Expand Down Expand Up @@ -318,12 +318,8 @@ impl Default for JsonFields {

impl<'a> FormatFields<'a> for JsonFields {
/// Format the provided `fields` to the provided `writer`, returning a result.
fn format_fields<R: RecordFields>(
&self,
writer: &'a mut dyn fmt::Write,
fields: R,
) -> fmt::Result {
let mut v = JsonVisitor::new(writer);
fn format_fields<R: RecordFields>(&self, mut writer: Writer<'_>, fields: R) -> fmt::Result {
let mut v = JsonVisitor::new(&mut writer);
fields.record(&mut v);
v.finish()
}
Expand All @@ -333,38 +329,44 @@ impl<'a> FormatFields<'a> for JsonFields {
/// By default, this appends a space to the current set of fields if it is
/// non-empty, and then calls `self.format_fields`. If different behavior is
/// required, the default implementation of this method can be overridden.
fn add_fields(&self, current: &'a mut String, fields: &Record<'_>) -> fmt::Result {
if !current.is_empty() {
// If fields were previously recorded on this span, we need to parse
// the current set of fields as JSON, add the new fields, and
// re-serialize them. Otherwise, if we just appended the new fields
// to a previously serialized JSON object, we would end up with
// malformed JSON.
//
// XXX(eliza): this is far from efficient, but unfortunately, it is
// necessary as long as the JSON formatter is implemented on top of
// an interface that stores all formatted fields as strings.
//
// We should consider reimplementing the JSON formatter as a
// separate layer, rather than a formatter for the `fmt` layer —
// then, we could store fields as JSON values, and add to them
// without having to parse and re-serialize.
let mut new = String::new();
let map: BTreeMap<&'_ str, serde_json::Value> =
serde_json::from_str(current).map_err(|_| fmt::Error)?;
let mut v = JsonVisitor::new(&mut new);
v.values = map;
fields.record(&mut v);
v.finish()?;
*current = new;
} else {
fn add_fields(
&self,
current: &'a mut FormattedFields<Self>,
fields: &Record<'_>,
) -> fmt::Result {
if current.is_empty() {
// If there are no previously recorded fields, we can just reuse the
// existing string.
let mut v = JsonVisitor::new(current);
let mut writer = current.as_writer();
let mut v = JsonVisitor::new(&mut writer);
fields.record(&mut v);
v.finish()?;
return Ok(());
}

// If fields were previously recorded on this span, we need to parse
// the current set of fields as JSON, add the new fields, and
// re-serialize them. Otherwise, if we just appended the new fields
// to a previously serialized JSON object, we would end up with
// malformed JSON.
//
// XXX(eliza): this is far from efficient, but unfortunately, it is
// necessary as long as the JSON formatter is implemented on top of
// an interface that stores all formatted fields as strings.
//
// We should consider reimplementing the JSON formatter as a
// separate layer, rather than a formatter for the `fmt` layer —
// then, we could store fields as JSON values, and add to them
// without having to parse and re-serialize.
let mut new = String::new();
let map: BTreeMap<&'_ str, serde_json::Value> =
serde_json::from_str(current).map_err(|_| fmt::Error)?;
let mut v = JsonVisitor::new(&mut new);
v.values = map;
fields.record(&mut v);
v.finish()?;
current.fields = new;

Ok(())
}
}
Expand Down Expand Up @@ -484,7 +486,7 @@ mod test {

struct MockTime;
impl FormatTime for MockTime {
fn format_time(&self, w: &mut dyn fmt::Write) -> fmt::Result {
fn format_time(&self, w: &mut Writer<'_>) -> fmt::Result {
write!(w, "fake time")
}
}
Expand Down
Loading