subscriber: replace dyn Write with a Writer type (#1661)

## Motivation

Currently, the `FormatEvent` and `FormatFields` traits in
`tracing-subscriber` are passed a `&mut dyn fmt::Write` trait object to
write formatted representations of events and fields to. This is fine,
but it doesn't give us the ability to easily provide additional
configuration to the formatter, such as whether ANSI color codes are
supported.

Issue #1651 describes some approaches for adding a way to expose the
ANSI color code configuration to custom formatters. In particular, the
proposed solution involves wrapping the `fmt::Write` trait object in an
opaque struct, which can then implement additional methods for exposing
information like "are ANSI colors enabled" to the formatter. Since this
changes the signature of the `FormatEvent::format_event` and
`FormatFields::format_fields` methods, it's a breaking change.
Therefore, we need to make this change _now_ if we want to get the API
change in for `tracing-subscriber` 0.3.

## Solution

This branch adds a `Writer` struct that wraps the `&mut dyn fmt::Write`
trait object, and changes the various method signatures as appropriate.
It does **not** actually implement the change related to ANSI color
formatting. Once we change these methods' signatures to accept a
`Writer` struct, we can add as many methods to that struct as we like
without making additional breaking API changes. Therefore, this branch
_just_ makes the breaking change that's necessary to get in before v0.3
is released.

Propagating the ANSI color configuration can be implemented in a future
branch.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

Author: hawkw

tracing-error/src/layer.rs

@@ -45,10 +45,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);
         }
     }
 

tracing-subscriber/src/fmt/fmt_layer.rs

@@ -533,37 +533,46 @@ where
 ///
 /// [extensions]: ../registry/struct.Extensions.html
 #[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
@@ -600,13 +609,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);
             }
         }
 
@@ -629,19 +638,18 @@ where
     fn on_record(&self, id: &Id, values: &Record<'_>, ctx: Context<'_, S>) {
         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);
         }
     }
 
@@ -743,7 +751,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());
             }
@@ -786,7 +798,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)

tracing-subscriber/src/fmt/format/json.rs

@@ -1,4 +1,4 @@
-use super::{Format, FormatEvent, FormatFields, FormatTime};
+use super::{Format, FormatEvent, FormatFields, FormatTime, Writer};
 use crate::{
     field::{RecordFields, VisitOutput},
     fmt::{
@@ -188,14 +188,14 @@ where
     fn format_event(
         &self,
         ctx: &FmtContext<'_, S, N>,
-        writer: &mut dyn fmt::Write,
+        mut writer: Writer<'_>,
         event: &Event<'_>,
     ) -> fmt::Result
     where
         S: Subscriber + 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();
@@ -205,7 +205,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)?;
 
@@ -323,12 +323,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()
     }
@@ -338,38 +334,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(())
     }
 }
@@ -489,7 +491,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")
         }
     }