From 8379fd536f6016d407dab566c2c5fbc0f1d80844 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Wed, 20 Oct 2021 16:35:22 -0700 Subject: [PATCH] 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 --- tracing-subscriber/src/fmt/fmt_layer.rs | 70 ++++++++++++++---------- tracing-subscriber/src/fmt/format/mod.rs | 1 - 2 files changed, 41 insertions(+), 30 deletions(-) diff --git a/tracing-subscriber/src/fmt/fmt_layer.rs b/tracing-subscriber/src/fmt/fmt_layer.rs index 9052e30a08..90269a7732 100644 --- a/tracing-subscriber/src/fmt/fmt_layer.rs +++ b/tracing-subscriber/src/fmt/fmt_layer.rs @@ -533,37 +533,46 @@ where /// /// [extensions]: ../registry/struct.Extensions.html #[derive(Default)] -pub struct FormattedFields { - _format_event: PhantomData, +pub struct FormattedFields { + _format_fields: PhantomData, /// The formatted fields of a span. pub fields: String, } -impl FormattedFields { +impl FormattedFields { /// 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 fmt::Debug for FormattedFields { +impl fmt::Debug for FormattedFields { 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::())) .finish() } } -impl fmt::Display for FormattedFields { +impl fmt::Display for FormattedFields { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.fields) + fmt::Display::fmt(&self.fields, f) } } -impl Deref for FormattedFields { +impl Deref for FormattedFields { 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::>().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::, - }; - extensions.insert(fmt_fields); + let mut fields = FormattedFields::::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::>() - { + if let Some(fields) = extensions.get_mut::>() { 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::, - }; - extensions.insert(fmt_fields); - } + return; + } + + let mut fields = FormattedFields::::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( &self, - writer: &'writer mut dyn fmt::Write, + writer: format::Writer<'writer>, fields: R, ) -> fmt::Result { self.fmt_fields.format_fields(writer, fields) diff --git a/tracing-subscriber/src/fmt/format/mod.rs b/tracing-subscriber/src/fmt/format/mod.rs index 7a7700a6eb..952a39c23a 100644 --- a/tracing-subscriber/src/fmt/format/mod.rs +++ b/tracing-subscriber/src/fmt/format/mod.rs @@ -1158,7 +1158,6 @@ where } // === impl FormatFields === - impl<'writer, M> FormatFields<'writer> for M where M: MakeOutput, fmt::Result>,