Skip to content

Commit

Permalink
subscriber: add MakeWriter::make_writer_for (#1141)
Browse files Browse the repository at this point in the history
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 #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]: #1137 (comment)

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
  • Loading branch information
hawkw committed May 14, 2021
1 parent b15b119 commit b3af7e1
Show file tree
Hide file tree
Showing 2 changed files with 189 additions and 6 deletions.
67 changes: 65 additions & 2 deletions tracing-subscriber/src/fmt/fmt_subscriber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,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 @@ -858,11 +858,12 @@ impl Timings {

#[cfg(test)]
mod test {
use super::*;
use crate::fmt::{
self,
format::{self, test::MockTime, Format},
subscribe::Subscribe as _,
test::MockMakeWriter,
test::{MockMakeWriter, MockWriter},
time,
};
use crate::Registry;
Expand Down Expand Up @@ -1035,4 +1036,66 @@ mod test {
actual.as_str()
);
}

#[test]
fn make_writer_based_on_meta() {
struct MakeByTarget {
make_writer1: MockMakeWriter,
make_writer2: MockMakeWriter,
}

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

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

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

let make_writer1 = MockMakeWriter::default();
let make_writer2 = MockMakeWriter::default();

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

let subscriber = crate::fmt::Collector::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()
);
}
}
128 changes: 124 additions & 4 deletions tracing-subscriber/src/fmt/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::{
io::{self, Write},
sync::{Mutex, MutexGuard},
};
use tracing_core::Metadata;

/// A type that can create [`io::Write`] instances.
///
Expand All @@ -19,6 +20,18 @@ use std::{
/// [`std::sync::Mutex`][mutex] when the type inside the mutex implements
/// [`io::Write`].
///
/// The [`MakeWriter::make_writer_for`] method takes [`Metadata`] describing a
/// span or event and returns a writer. `MakeWriter`s can optionally provide
/// implementations of this method with behaviors that differ based on the span
/// or event being written. For example, events at different [levels] might be
/// written to different output streams, or data from different [targets] might
/// be written to separate log files. When the `MakeWriter` has no custom
/// behavior based on metadata, the default implementation of `make_writer_for`
/// simply calls `self.make_writer()`, ignoring the metadata. Therefore, when
/// metadata _is_ available, callers should prefer to call `make_writer_for`,
/// passing in that metadata, so that the `MakeWriter` implementation can choose
/// the appropriate behavior.
///
/// # Examples
///
/// The simplest usage is to pass in a named function that returns a writer. For
Expand Down Expand Up @@ -89,6 +102,10 @@ use std::{
/// [`io::stdout`]: std::io::stdout()
/// [`io::stderr`]: std::io::stderr()
/// [mutex]: std::sync::Mutex
/// [`MakeWriter::make_writer_for`]: MakeWriter::make_writer_for
/// [`Metadata`]: tracing_core::Metadata
/// [levels]: tracing_core::Level
/// [targets]: tracing_core::Metadata::target
pub trait MakeWriter<'a> {
/// The concrete [`io::Write`] implementation returned by [`make_writer`].
///
Expand All @@ -100,16 +117,110 @@ pub trait MakeWriter<'a> {
///
/// # Implementer notes
///
/// [`fmt::Subscriber`] or [`fmt::Collector`] will call this method each time an event is recorded. Ensure any state
/// that must be saved across writes is not lost when the [`Writer`] instance is dropped. If
/// creating a [`io::Write`] instance is expensive, be sure to cache it when implementing
/// [`MakeWriter`] to improve performance.
/// [`fmt::Subscriber`] or [`fmt::Collector`] will call this method each
/// time an event is recorded. Ensure any state that must be saved across
/// writes is not lost when the [`Writer`] instance is dropped. If creating
/// a [`io::Write`] instance is expensive, be sure to cache it when
/// implementing [`MakeWriter`] to improve performance.
///
/// [`Writer`]: MakeWriter::Writer
/// [`fmt::Subscriber`]: super::super::fmt::Subscriber
/// [`fmt::Collector`]: super::super::fmt::Collector
/// [`io::Write`]: std::io::Write
fn make_writer(&'a self) -> Self::Writer;

/// Returns a [`Writer`] for writing data from the span or event described
/// by the provided [`Metadata`].
///
/// By default, this calls [`self.make_writer()`][make_writer], ignoring
/// the provided metadata, but implementations can override this to provide
/// metadata-specific behaviors.
///
/// This method allows `MakeWriter` implementations to implement different
/// behaviors based on the span or event being written. The `MakeWriter`
/// type might return different writers based on the provided metadata, or
/// might write some values to the writer before or after providing it to
/// the caller.
///
/// For example, we might want to write data from spans and events at the
/// [`ERROR`] and [`WARN`] levels to `stderr`, and data from spans or events
/// at lower levels to stdout:
///
/// ```
/// use std::io::{self, Stdout, Stderr, StdoutLock, StderrLock};
/// use tracing_subscriber::fmt::writer::MakeWriter;
/// use tracing_core::{Metadata, Level};
///
/// pub struct MyMakeWriter {
/// stdout: Stdout,
/// stderr: Stderr,
/// }
///
/// /// A lock on either stdout or stderr, depending on the verbosity level
/// /// of the event being written.
/// pub enum StdioLock<'a> {
/// Stdout(StdoutLock<'a>),
/// Stderr(StderrLock<'a>),
/// }
///
/// impl<'a> io::Write for StdioLock<'a> {
/// fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
/// match self {
/// StdioLock::Stdout(lock) => lock.write(buf),
/// StdioLock::Stderr(lock) => lock.write(buf),
/// }
/// }
///
/// fn write_all(&mut self, buf: &[u8]) -> io::Result<()> {
/// // ...
/// # match self {
/// # StdioLock::Stdout(lock) => lock.write_all(buf),
/// # StdioLock::Stderr(lock) => lock.write_all(buf),
/// # }
/// }
///
/// fn flush(&mut self) -> io::Result<()> {
/// // ...
/// # match self {
/// # StdioLock::Stdout(lock) => lock.flush(),
/// # StdioLock::Stderr(lock) => lock.flush(),
/// # }
/// }
/// }
///
/// impl<'a> MakeWriter<'a> for MyMakeWriter {
/// type Writer = StdioLock<'a>;
///
/// fn make_writer(&'a self) -> Self::Writer {
/// // We must have an implementation of `make_writer` that makes
/// // a "default" writer without any configuring metadata. Let's
/// // just return stdout in that case.
/// StdioLock::Stdout(self.stdout.lock())
/// }
///
/// fn make_writer_for(&'a self, meta: &Metadata<'_>) -> Self::Writer {
/// // Here's where we can implement our special behavior. We'll
/// // check if the metadata's verbosity level is WARN or ERROR,
/// // and return stderr in that case.
/// if meta.level() <= &Level::WARN {
/// return StdioLock::Stderr(self.stderr.lock());
/// }
///
/// // Otherwise, we'll return stdout.
/// StdioLock::Stdout(self.stdout.lock())
/// }
/// }
/// ```
///
/// [`Writer`]: MakeWriter::Writer
/// [`Metadata`]: tracing_core::Metadata
/// [make_writer]: MakeWriter::make_writer
/// [`WARN`]: tracing_core::Level::WARN
/// [`ERROR`]: tracing_core::Level::ERROR
fn make_writer_for(&'a self, meta: &Metadata<'_>) -> Self::Writer {
let _ = meta;
self.make_writer()
}
}

/// A type implementing [`io::Write`] for a [`MutexGuard`] where the type
Expand Down Expand Up @@ -248,6 +359,10 @@ impl<'a> MakeWriter<'a> for BoxMakeWriter {
fn make_writer(&'a self) -> Self::Writer {
self.inner.make_writer()
}

fn make_writer_for(&'a self, meta: &Metadata<'_>) -> Self::Writer {
self.inner.make_writer_for(meta)
}
}

struct Boxed<M>(M);
Expand All @@ -262,6 +377,11 @@ where
let w = self.0.make_writer();
Box::new(w)
}

fn make_writer_for(&'a self, meta: &Metadata<'_>) -> Self::Writer {
let w = self.0.make_writer_for(meta);
Box::new(w)
}
}

// === impl Mutex/MutexGuardWriter ===
Expand Down

0 comments on commit b3af7e1

Please sign in to comment.