subscriber: add lifetime parameter to MakeWriter

Motivation
----------

Currently, the `tracing-subscriber` crate has the `MakeWriter` trait for
customizing the io writer used by `fmt`. This trait is necessary (rather
than simply using a `Write` instance) because the default implementation
performs the IO on the thread where an event was recorded, meaning that
a separate writer needs to be acquired by each thread (either by calling
a function like `io::stdout`, by locking a shared `Write` instance,
etc).

Right now there is a blanket impl for `Fn() -> T where T: Write`. This
works fine with functions like `io::stdout`. However, the _other_ common
case for this trait is locking a shared writer.

Therefore, it makes sense to see an implementation like this:

``` rust
impl<'a, W: io::Write> MakeWriter for Mutex<W>
where
    W: io::Write,
{
    type Writer = MutexWriter<'a, W>;
    fn make_writer(&self) -> Self::Writer {
        MutexWriter(self.lock().unwrap())
    }
}

pub struct MutexWriter<'a, W>(MutexGuard<'a, W>);

impl<W: io::Write> io::Write for MutexWriter<'_, W> {
    // write to the shared writer in the `MutexGuard`...
}
```

Unfortunately, it's impossible to write this. Since `MakeWriter` always
takes an `&self` parameter and returns `Self::Writer`, the generic
parameter is unbounded:
```
    Checking tracing-subscriber v0.2.4 (/home/eliza/code/tracing/tracing-subscriber)
error[E0207]: the lifetime parameter `'a` is not constrained by the impl trait, self type, or predicates
  --> tracing-subscriber/src/fmt/writer.rs:61:6
   |
61 | impl<'a, W: io::Write> MakeWriter for Mutex<W>
   |      ^^ unconstrained lifetime parameter

error: aborting due to previous error
```

This essentially precludes any `MakeWriter` impl where the writer is
borrowed from the type implementing `MakeWriter`. This is a significant
blow to the usefulness of the trait. For example, it prevented the use
of `MakeWriter` in `tracing-flame` as suggested in
#631 (comment).

Proposal
--------

This PR changes `MakeWriter` to be generic over a lifetime `'a`:

```rust
pub trait MakeWriter<'a> {
    type Writer: io::Write;

    fn make_writer(&'a self) -> Self::Writer;
}
```
The `self` parameter is now borrowed for the `&'a` lifetime, so it is
okay to return a writer borrowed from `self`, such as in the `Mutex`
case.

I've also added an impl of `MakeWriter` for `Mutex<T> where T: Writer`.

Unfortunately, this is a breaking change and will need to wait until we
release `tracing-subscriber` 0.3.

Fixes #675.

Signed-off-by: Eliza Weisman <[email protected]>

examples/Cargo.toml

@@ -15,7 +15,7 @@ tracing-core = { path = "../tracing-core", version = "0.1"}
 tracing-error = { path = "../tracing-error" }
 tracing-flame = { path = "../tracing-flame" }
 tracing-tower = { version = "0.1.0", path = "../tracing-tower" }
-tracing-subscriber = { path = "../tracing-subscriber", version = "0.2.4", features = ["json", "chrono"] }
+tracing-subscriber = { path = "../tracing-subscriber", version = "0.3", features = ["json", "chrono"] }
 tracing-futures = { version = "0.2.1", path = "../tracing-futures", features = ["futures-01"] }
 tracing-attributes =  { path = "../tracing-attributes", version = "0.1.2"}
 tracing-log = { path = "../tracing-log", version = "0.1.1", features = ["env_logger"] }

tracing-appender/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "tracing-appender"
-version = "0.1.0"
+version = "0.2.0"
 authors = [
     "Zeki Sherif <[email protected]>",
     "Tokio Contributors <[email protected]>"
@@ -20,7 +20,7 @@ keywords = ["logging", "tracing", "file-appender", "non-blocking-writer"]
 edition = "2018"
 
 [dependencies]
-tracing-subscriber = {path = "../tracing-subscriber", version = "0.2.4"}
+tracing-subscriber = {path = "../tracing-subscriber", version = "0.3"}
 crossbeam-channel = "0.4.2"
 chrono = "0.4.11"
 

tracing-appender/src/non_blocking.rs

@@ -236,10 +236,10 @@ impl std::io::Write for NonBlocking {
     }
 }
 
-impl MakeWriter for NonBlocking {
+impl<'a> MakeWriter<'a> for NonBlocking {
     type Writer = NonBlocking;
 
-    fn make_writer(&self) -> Self::Writer {
+    fn make_writer(&'a self) -> Self::Writer {
         self.clone()
     }
 }

tracing-error/Cargo.toml

@@ -38,7 +38,7 @@ default = ["traced-error"]
 traced-error = []
 
 [dependencies]
-tracing-subscriber = { path = "../tracing-subscriber", version = "0.2.4", default-features = false, features = ["registry", "fmt"] }
+tracing-subscriber = { path = "../tracing-subscriber", version = ">= 0.2.4, < 0.4", default-features = false, features = ["registry", "fmt"] }
 tracing = { path = "../tracing", version = "0.1.12"}
 
 [badges]

tracing-flame/Cargo.toml

@@ -25,7 +25,7 @@ default = ["smallvec"]
 smallvec = ["tracing-subscriber/smallvec"]
 
 [dependencies]
-tracing-subscriber = { path = "../tracing-subscriber", version = "0.2.3", default-features = false, features = ["registry", "fmt"] }
+tracing-subscriber = { path = "../tracing-subscriber", version = "0.3", default-features = false, features = ["registry", "fmt"] }
 tracing = { path = "../tracing", version = "0.1.12"}
 lazy_static = "1.3.0"
 

tracing-opentelemetry/Cargo.toml

@@ -26,7 +26,7 @@ opentelemetry = { version = "0.6", default-features = false, features = ["trace"
 rand = "0.7"
 tracing = { path = "../tracing", version = "0.1" }
 tracing-core = { path = "../tracing-core", version = "0.1" }
-tracing-subscriber = { path = "../tracing-subscriber", version = "0.2", default-features = false, features = ["registry"] }
+tracing-subscriber = { path = "../tracing-subscriber", version = ">= 0.2, < 0.4", default-features = false, features = ["registry"] }
 tracing-log = { path = "../tracing-log", version = "0.1", default-features = false, optional = true }
 
 [dev-dependencies]

tracing-subscriber/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "tracing-subscriber"
-version = "0.2.5"
+version = "0.3.0"
 authors = [
     "Eliza Weisman <[email protected]>",
     "David Barsky <[email protected]>",

tracing-subscriber/src/fmt/fmt_layer.rs

@@ -107,7 +107,7 @@ impl<S, N, E, W> Layer<S, N, E, W>
 where
     S: Subscriber + for<'a> LookupSpan<'a>,
     N: for<'writer> FormatFields<'writer> + 'static,
-    W: MakeWriter + 'static,
+    W: for<'writer> MakeWriter<'writer> + 'static,
 {
     /// Sets the [event formatter][`FormatEvent`] that the layer will use to
     /// format events.
@@ -146,7 +146,7 @@ where
 
 // This needs to be a seperate impl block because they place different bounds on the type paramaters.
 impl<S, N, E, W> Layer<S, N, E, W> {
-    /// Sets the [`MakeWriter`] that the [`Layer`] being built will use to write events.
+    /// Sets the [`for<'writer> MakeWriter<'writer>`] that the [`Layer`] being built will use to write events.
     ///
     /// # Examples
     ///
@@ -163,11 +163,11 @@ impl<S, N, E, W> Layer<S, N, E, W> {
     /// # let _ = layer.with_subscriber(tracing_subscriber::registry::Registry::default());
     /// ```
     ///
-    /// [`MakeWriter`]: ../fmt/trait.MakeWriter.html
+    /// [`for<'writer> MakeWriter<'writer>`]: ../fmt/trait.for<'writer> MakeWriter<'writer>.html
     /// [`Layer`]: ../layer/trait.Layer.html
     pub fn with_writer<W2>(self, make_writer: W2) -> Layer<S, N, E, W2>
     where
-        W2: MakeWriter + 'static,
+        W2: for<'writer> MakeWriter<'writer> + 'static,
     {
         Layer {
             fmt_fields: self.fmt_fields,
@@ -326,7 +326,7 @@ where
     S: Subscriber + for<'a> LookupSpan<'a>,
     N: for<'writer> FormatFields<'writer> + 'static,
     E: FormatEvent<S, N> + 'static,
-    W: MakeWriter + 'static,
+    W: for<'writer> MakeWriter<'writer> + 'static,
 {
     /// Builds a [`Layer`] with the provided configuration.
     ///
@@ -356,7 +356,7 @@ where
     S: Subscriber + for<'a> LookupSpan<'a>,
     N: for<'writer> FormatFields<'writer> + 'static,
     E: FormatEvent<S, N> + 'static,
-    W: MakeWriter + 'static,
+    W: for<'writer> MakeWriter<'writer> + 'static,
 {
     #[inline]
     fn make_ctx<'a>(&'a self, ctx: Context<'a, S>) -> FmtContext<'a, S, N> {
@@ -421,7 +421,7 @@ where
     S: Subscriber + for<'a> LookupSpan<'a>,
     N: for<'writer> FormatFields<'writer> + 'static,
     E: FormatEvent<S, N> + 'static,
-    W: MakeWriter + 'static,
+    W: for<'writer> MakeWriter<'writer> + 'static,
 {
     fn new_span(&self, attrs: &Attributes<'_>, id: &Id, ctx: Context<'_, S>) {
         let span = ctx.span(id).expect("Span not found, this is a bug");
@@ -490,7 +490,7 @@ where
 
     unsafe fn downcast_raw(&self, id: TypeId) -> Option<*const ()> {
         // This `downcast_raw` impl allows downcasting a `fmt` layer to any of
-        // its components (event formatter, field formatter, and `MakeWriter`)
+        // its components (event formatter, field formatter, and `for<'writer> MakeWriter<'writer>`)
         // as well as to the layer's type itself. The potential use-cases for
         // this *may* be somewhat niche, though...
         match () {

tracing-subscriber/src/fmt/format/json.rs

@@ -366,11 +366,10 @@ impl<'a> fmt::Debug for WriteAdaptor<'a> {
 
 #[cfg(test)]
 mod test {
-    use crate::fmt::{test::MockWriter, time::FormatTime};
-    use lazy_static::lazy_static;
+    use crate::fmt::{test::MockMakeWriter, time::FormatTime};
     use tracing::{self, subscriber::with_default};
 
-    use std::{fmt, sync::Mutex};
+    use std::fmt;
 
     struct MockTime;
     impl FormatTime for MockTime {
@@ -381,45 +380,33 @@ mod test {
 
     #[test]
     fn json() {
-        lazy_static! {
-            static ref BUF: Mutex<Vec<u8>> = Mutex::new(vec![]);
-        }
-
-        let make_writer = || MockWriter::new(&BUF);
-
         let expected =
         "{\"timestamp\":\"fake time\",\"level\":\"INFO\",\"span\":{\"answer\":42,\"name\":\"json_span\",\"number\":3},\"target\":\"tracing_subscriber::fmt::format::json::test\",\"fields\":{\"message\":\"some json test\"}}\n";
 
-        test_json(make_writer, expected, &BUF, false);
+        test_json(expected, false);
     }
 
     #[test]
     fn json_flattened_event() {
-        lazy_static! {
-            static ref BUF: Mutex<Vec<u8>> = Mutex::new(vec![]);
-        }
-
-        let make_writer = || MockWriter::new(&BUF);
-
         let expected =
         "{\"timestamp\":\"fake time\",\"level\":\"INFO\",\"span\":{\"answer\":42,\"name\":\"json_span\",\"number\":3},\"target\":\"tracing_subscriber::fmt::format::json::test\",\"message\":\"some json test\"}\n";
 
-        test_json(make_writer, expected, &BUF, true);
+        test_json(expected, true);
     }
 
     #[test]
     fn record_works() {
         // This test reproduces issue #707, where using `Span::record` causes
         // any events inside the span to be ignored.
-        lazy_static! {
-            static ref BUF: Mutex<Vec<u8>> = Mutex::new(vec![]);
-        }
 
-        let make_writer = || MockWriter::new(&BUF);
-        let subscriber = crate::fmt().json().with_writer(make_writer).finish();
+        let make_writer = MockMakeWriter::default();
+        let subscriber = crate::fmt()
+            .json()
+            .with_writer(make_writer.clone())
+            .finish();
 
         let parse_buf = || -> serde_json::Value {
-            let buf = String::from_utf8(BUF.try_lock().unwrap().to_vec()).unwrap();
+            let buf = String::from_utf8(make_writer.buf().to_vec()).unwrap();
             let json = buf
                 .lines()
                 .last()
@@ -453,14 +440,12 @@ mod test {
     }
 
     #[cfg(feature = "json")]
-    fn test_json<T>(make_writer: T, expected: &str, buf: &Mutex<Vec<u8>>, flatten_event: bool)
-    where
-        T: crate::fmt::MakeWriter + Send + Sync + 'static,
-    {
+    fn test_json(expected: &str, flatten_event: bool) {
+        let make_writer = MockMakeWriter::default();
         let subscriber = crate::fmt::Subscriber::builder()
             .json()
             .flatten_event(flatten_event)
-            .with_writer(make_writer)
+            .with_writer(make_writer.clone())
             .with_timer(MockTime)
             .finish();
 
@@ -470,7 +455,7 @@ mod test {
             tracing::info!("some json test");
         });
 
-        let actual = String::from_utf8(buf.try_lock().unwrap().to_vec()).unwrap();
+        let actual = String::from_utf8(make_writer.buf().to_vec()).unwrap();
         assert_eq!(expected, actual.as_str());
     }
 }

tracing-subscriber/src/fmt/format/mod.rs

@@ -825,12 +825,10 @@ impl<'a, F> fmt::Debug for FieldFnVisitor<'a, F> {
 
 #[cfg(test)]
 mod test {
+    use crate::fmt::{test::MockMakeWriter, time::FormatTime};
+    use tracing::dispatcher::{set_default, Dispatch};
 
-    use crate::fmt::{test::MockWriter, time::FormatTime};
-    use lazy_static::lazy_static;
-    use tracing::{self, subscriber::with_default};
-
-    use std::{fmt, sync::Mutex};
+    use std::fmt;
 
     struct MockTime;
     impl FormatTime for MockTime {
@@ -842,90 +840,55 @@ mod test {
     #[cfg(feature = "ansi")]
     #[test]
     fn with_ansi_true() {
-        lazy_static! {
-            static ref BUF: Mutex<Vec<u8>> = Mutex::new(vec![]);
-        }
-
-        let make_writer = || MockWriter::new(&BUF);
-        let expected = "\u{1b}[2mfake time\u{1b}[0m \u{1b}[32m INFO\u{1b}[0m tracing_subscriber::fmt::format::test: some ansi test\n";
-        test_ansi(make_writer, expected, true, &BUF);
+        let expected = "\u{1b}[2mfake time\u{1b}[0m \u{1b}[32m INFO\u{1b}[0m tracing_subscriber::fmt::format::test: hello\n";
+        test_ansi(true, expected);
     }
 
     #[cfg(feature = "ansi")]
     #[test]
     fn with_ansi_false() {
-        lazy_static! {
-            static ref BUF: Mutex<Vec<u8>> = Mutex::new(vec![]);
-        }
-
-        let make_writer = || MockWriter::new(&BUF);
-        let expected = "fake time  INFO tracing_subscriber::fmt::format::test: some ansi test\n";
-
-        test_ansi(make_writer, expected, false, &BUF);
+        let expected = "fake time  INFO tracing_subscriber::fmt::format::test: hello\n";
+        test_ansi(false, expected);
     }
 
     #[cfg(not(feature = "ansi"))]
     #[test]
     fn without_ansi() {
-        lazy_static! {
-            static ref BUF: Mutex<Vec<u8>> = Mutex::new(vec![]);
-        }
-
-        let make_writer = || MockWriter::new(&BUF);
-        let expected = "fake time  INFO tracing_subscriber::fmt::format::test: some ansi test\n";
+        let make_writer = MockMakeWriter::default();
+        let expected = "fake time  INFO tracing_subscriber::fmt::format::test: hello\n";
         let subscriber = crate::fmt::Subscriber::builder()
             .with_writer(make_writer)
-            .with_timer(MockTime)
-            .finish();
+            .with_timer(MockTime);
+        run_test(subscriber, make_writer, expected);
+    }
 
-        with_default(subscriber, || {
-            tracing::info!("some ansi test");
-        });
+    #[test]
+    fn without_level() {
+        let make_writer = MockMakeWriter::default();
+        let subscriber = crate::fmt::Subscriber::builder()
+            .with_writer(make_writer.clone())
+            .with_level(false)
+            .with_ansi(false)
+            .with_timer(MockTime);
+        let expected = "fake time tracing_subscriber::fmt::format::test: hello\n";
 
-        let actual = String::from_utf8(BUF.try_lock().unwrap().to_vec()).unwrap();
-        assert_eq!(expected, actual.as_str());
+        run_test(subscriber, make_writer, expected);
     }
 
     #[cfg(feature = "ansi")]
-    fn test_ansi<T>(make_writer: T, expected: &str, is_ansi: bool, buf: &Mutex<Vec<u8>>)
-    where
-        T: crate::fmt::MakeWriter + Send + Sync + 'static,
-    {
+    fn test_ansi(is_ansi: bool, expected: &str) {
+        let make_writer = MockMakeWriter::default();
         let subscriber = crate::fmt::Subscriber::builder()
-            .with_writer(make_writer)
+            .with_writer(make_writer.clone())
             .with_ansi(is_ansi)
-            .with_timer(MockTime)
-            .finish();
-
-        with_default(subscriber, || {
-            tracing::info!("some ansi test");
-        });
-
-        let actual = String::from_utf8(buf.try_lock().unwrap().to_vec()).unwrap();
-        assert_eq!(expected, actual.as_str());
+            .with_timer(MockTime);
+        run_test(subscriber, make_writer, expected)
     }
 
-    #[test]
-    fn without_level() {
-        lazy_static! {
-            static ref BUF: Mutex<Vec<u8>> = Mutex::new(vec![]);
-        }
-
-        let make_writer = || MockWriter::new(&BUF);
-        let subscriber = crate::fmt::Subscriber::builder()
-            .with_writer(make_writer)
-            .with_level(false)
-            .with_ansi(false)
-            .with_timer(MockTime)
-            .finish();
-
-        with_default(subscriber, || {
-            tracing::info!("hello");
-        });
-        let actual = String::from_utf8(BUF.try_lock().unwrap().to_vec()).unwrap();
-        assert_eq!(
-            "fake time tracing_subscriber::fmt::format::test: hello\n",
-            actual.as_str()
-        );
+    fn run_test(subscriber: impl Into<Dispatch>, buf: MockMakeWriter, expected: &str) {
+        let _default = set_default(&subscriber.into());
+        tracing::info!("hello");
+        let actual = String::from_utf8(buf.buf().to_vec()).unwrap();
+        assert_eq!(expected, actual.as_str())
     }
 }