Skip to content

Commit

Permalink
subscriber: replace dyn Write with a Writer type (tokio-rs#1661)
Browse files Browse the repository at this point in the history
## 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 tokio-rs#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>
  • Loading branch information
hawkw authored and kaffarell committed May 22, 2024
1 parent f153919 commit 8379fd5
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 30 deletions.
70 changes: 41 additions & 29 deletions tracing-subscriber/src/fmt/fmt_layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion tracing-subscriber/src/fmt/format/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1158,7 +1158,6 @@ where
}

// === impl FormatFields ===

impl<'writer, M> FormatFields<'writer> for M
where
M: MakeOutput<Writer<'writer>, fmt::Result>,
Expand Down

0 comments on commit 8379fd5

Please sign in to comment.