subscriber: add MakeWriter::make_writer_for (#1141)

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>

Author: hawkw

tracing-subscriber/src/fmt/fmt_subscriber.rs

@@ -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());
             }
 
@@ -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;
@@ -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()
+        );
+    }
 }

tracing-subscriber/src/fmt/writer.rs

@@ -7,6 +7,7 @@ use std::{
     io::{self, Write},
     sync::{Mutex, MutexGuard},
 };
+use tracing_core::Metadata;
 
 /// A type that can create [`io::Write`] instances.
 ///
@@ -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
@@ -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`].
     ///
@@ -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
@@ -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);
@@ -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 ===