Skip to content

Commit

Permalink
subscriber: add MakeWriter::make_writer_for (tokio-rs#1141)
Browse files Browse the repository at this point in the history
This backports PR tokio-rs#1141 from `master`.

subscriber: add `MakeWriter::make_writer_for`

## Motivation

In some cases, it might be desirable to configure the writer used for
writing out trace data based on the metadata of the span or event being
written. For example, we might want to send different levels to
different outputs, write logs from different targets to separate files,
or wrap formatted output in ANSI color codes based on levels. Currently,
it's not possible for the `MakeWriter` trait to model this kind of
behavior --- it has one method, `make_writer`, which is completely
unaware of *where* the data being written came from.

In particular, this came up in PR tokio-rs#1137, when discussing a proposal that
writing to syslog could be implemented as a `MakeWriter` implementation
rather than as a `Subscribe` implementation, so that all the formatting
logic from `tracing_subscriber::fmt` could be reused. See [here][1] for
details.

## Solution

This branch adds a new `make_writer_for` method to `MakeWriter`, taking
a `Metadata`. Implementations can opt in to metadata-specific behavior
by implementing this method. The method has a default implementation
that just calls `self.make_writer()` and ignores the metadata, so it's
only necessary to implement this when per-metadata behavior is required.
This isn't a breaking change to existing implementations.

There are a couple downsides to this approach: it's possible for callers
to skip the metadata-specific behavior by calling `make_writer` rather
than `make_writer_for`, and the impls for closures can't easily provide
metadata-specific behavior.

Since the upcoming release is going to be a breaking change anyway, we
may want to just make the breaking change of having
`MakeWriter::make_writer` _always_ take a `Metadata`, which solves these
problems. However, that can't be backported to v0.1.x as easily. Additionally,
that would mean that functions like `io::stdout` no longer implement
`MakeWriter`; they would have to be wrapped in a wrapper type or closure
that ignores metadata.

[1]: tokio-rs#1137 (comment)

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
  • Loading branch information
hawkw authored and kaffarell committed May 22, 2024
1 parent 56ecdcc commit 0823223
Showing 1 changed file with 69 additions and 2 deletions.
71 changes: 69 additions & 2 deletions tracing-subscriber/src/fmt/fmt_layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -740,7 +740,7 @@ where

let ctx = self.make_ctx(ctx);
if self.fmt_event.format_event(&ctx, &mut buf, event).is_ok() {
let mut writer = self.make_writer.make_writer();
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 @@ -913,11 +913,12 @@ impl Timings {

#[cfg(test)]
mod test {
use super::*;
use crate::fmt::{
self,
format::{self, test::MockTime, Format},
layer::Layer as _,
test::MockWriter,
test::{MockMakeWriter, MockWriter},
time,
};
use crate::Registry;
Expand Down Expand Up @@ -1110,4 +1111,70 @@ mod test {
actual.as_str()
);
}

#[test]
fn make_writer_based_on_meta() {
lazy_static! {
static ref BUF1: Mutex<Vec<u8>> = Mutex::new(vec![]);
static ref BUF2: Mutex<Vec<u8>> = Mutex::new(vec![]);
}
struct MakeByTarget<'a> {
make_writer1: MockMakeWriter<'a>,
make_writer2: MockMakeWriter<'a>,
}

impl<'a> MakeWriter for MakeByTarget<'a> {
type Writer = MockWriter<'a>;

fn make_writer(&self) -> Self::Writer {
self.make_writer1.make_writer()
}

fn make_writer_for(&self, meta: &Metadata<'_>) -> Self::Writer {
if meta.target() == "writer2" {
return self.make_writer2.make_writer();
}
self.make_writer()
}
}

let make_writer1 = MockMakeWriter::new(&BUF1);
let make_writer2 = MockMakeWriter::new(&BUF2);

let make_writer = MakeByTarget {
make_writer1: make_writer1.clone(),
make_writer2: make_writer2.clone(),
};

let subscriber = crate::fmt::Subscriber::builder()
.with_writer(make_writer)
.with_level(false)
.with_target(false)
.with_ansi(false)
.with_timer(MockTime)
.with_span_events(FmtSpan::CLOSE)
.finish();

with_default(subscriber, || {
let span1 = tracing::info_span!("writer1_span", x = 42);
let _e = span1.enter();
tracing::info!(target: "writer2", "hello writer2!");
let span2 = tracing::info_span!(target: "writer2", "writer2_span");
let _e = span2.enter();
tracing::warn!(target: "writer1", "hello writer1!");
});

let actual = sanitize_timings(make_writer1.get_string());
assert_eq!(
"fake time writer1_span{x=42}:writer2_span: hello writer1!\n\
fake time writer1_span{x=42}: close timing timing\n",
actual.as_str()
);
let actual = sanitize_timings(make_writer2.get_string());
assert_eq!(
"fake time writer1_span{x=42}: hello writer2!\n\
fake time writer1_span{x=42}:writer2_span: close timing timing\n",
actual.as_str()
);
}
}

0 comments on commit 0823223

Please sign in to comment.