subscriber: add `MakeWriter::make_writer_for` #1141

hawkw · 2020-12-15T18:54:20Z

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 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.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

max-heller · 2020-12-15T20:23:59Z

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.

This would just mean writing something like |_| io::stdout(), right?

max-heller · 2020-12-17T14:50:47Z

tracing-subscriber/src/fmt/writer.rs

+    /// might write some values to the writer before or after providing it to
+    /// the caller.
+    ///
+    /// For example, we ight want to write data from spans and events at the


Typo (ight instead of might)

max-heller · 2020-12-17T14:51:31Z

tracing-subscriber/src/fmt/writer.rs

+    ///     Stderr(StderrLock<'a>),
+    /// }
+    ///
+    /// impl io::Write for StdioLock {


StdioLock needs a lifetime parameter

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

hawkw · 2021-03-01T18:41:26Z

@davidbarsky et al. mind giving this a review? i have some ideas for subsequent things we can add based on this...

davidbarsky

I genuinely thought i reviewed this thing. Sorry! Looks good to me.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

raftario · 2021-05-13T00:47:23Z

Currently looking into hooking tracing into Android's logging system, which has dedicated interfaces for levels and source locations, and this would be amazing.

Somewhat related and useful in similar usecases would be a way to reliably tell that two written events are in fact two distinct events. Currently it's only possible by depending on internal implementation details and either

~~assuming MakeWriter::make_writer is called exactly once per event and flushing the event in the returned writer's drop impl, or~~
~~assuming the buffer passed to std::io::Write::write_all contains exactly one event~~

~~where 1 definitely seems like the more reasonable thing to depend on and potentially guarantee as part of the stable API.~~

Looks like I'm absolutely awful at reading docs and after double checking this is already a thing. My bad, sorry for the noise.

Depends on #1141. This branch adds a `MakeWriterExt` trait which adds a number of combinators for composing multiple types implementing `MakeWriter`. `MakeWriter`s can now be teed together, filtered with minimum and maximum levels, filtered with a `Metadata` predicate, and combined with a fallback for when a writer is _not_ made for a particular `Metadata`. I've also added a `MakeWriter` impl for `std::fs::File`, because `&File` implements `Write`. Ideally, we'd have a default impl of `MakeWriter` for `T where &T: Write`, but that's not possible due to the impl of `MakeWriter` for `F: Fn() -> T: Write`. We could add a generic `by_ref` adapter for any `T where &T: Write`, though... Signed-off-by: Eliza Weisman <eliza@buoyant.io>

This backports PR #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 #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>

This backports #1274 from `master`. Depends on #1141. This branch adds a `MakeWriterExt` trait which adds a number of combinators for composing multiple types implementing `MakeWriter`. `MakeWriter`s can now be teed together, filtered with minimum and maximum levels, filtered with a `Metadata` predicate, and combined with a fallback for when a writer is _not_ made for a particular `Metadata`. I've also added a `MakeWriter` impl for `Arc<W>` when `&W` implements `Write`. This enables `File`s to be used as `MakeWriter`s similarly to how we will be able to in 0.3, with the additional overhead of having to do ref-counting because we can't return a reference from `MakeWriter` until the next breaking change. Signed-off-by: Eliza Weisman <eliza@buoyant.io>

This backports PR #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 #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>

This backports #1274 from `master`. Depends on #1141. This branch adds a `MakeWriterExt` trait which adds a number of combinators for composing multiple types implementing `MakeWriter`. `MakeWriter`s can now be teed together, filtered with minimum and maximum levels, filtered with a `Metadata` predicate, and combined with a fallback for when a writer is _not_ made for a particular `Metadata`. I've also added a `MakeWriter` impl for `Arc<W>` when `&W` implements `Write`. This enables `File`s to be used as `MakeWriter`s similarly to how we will be able to in 0.3, with the additional overhead of having to do ref-counting because we can't return a reference from `MakeWriter` until the next breaking change. Signed-off-by: Eliza Weisman <eliza@buoyant.io>

@Folyd

# 0.2.19 (June 25, 2021) ### Deprecated - **registry**: `SpanRef::parents`, `SpanRef::from_root`, and `Context::scope` iterators, which are replaced by new `SpanRef::scope` and `Scope::from_root` iterators (#1413) ### Added - **registry**: `SpanRef::scope` method, which returns a leaf-to-root `Iterator` including the leaf span (#1413) - **registry**: `Scope::from_root` method, which reverses the `scope` iterator to iterate root-to-leaf (#1413) - **registry**: `Context::event_span` method, which looks up the parent span of an event (#1434) - **registry**: `Context::event_scope` method, returning a `Scope` iterator over the span scope of an event (#1434) - **fmt**: `MakeWriter::make_writer_for` method, which allows returning a different writer based on a span or event's metadata (#1141) - **fmt**: `MakeWriterExt` trait, with `with_max_level`, `with_min_level`, `with_filter`, `and`, and `or_else` combinators (#1274) - **fmt**: `MakeWriter` implementation for `Arc<W> where &W: io::Write` (#1274) Thanks to @teozkr and @Folyd for contributing to this release!

@Folyd

# 0.2.19 (June 25, 2021) ### Deprecated - **registry**: `SpanRef::parents`, `SpanRef::from_root`, and `Context::scope` iterators, which are replaced by new `SpanRef::scope` and `Scope::from_root` iterators (#1413) ### Added - **registry**: `SpanRef::scope` method, which returns a leaf-to-root `Iterator` including the leaf span (#1413) - **registry**: `Scope::from_root` method, which reverses the `scope` iterator to iterate root-to-leaf (#1413) - **registry**: `Context::event_span` method, which looks up the parent span of an event (#1434) - **registry**: `Context::event_scope` method, returning a `Scope` iterator over the span scope of an event (#1434) - **fmt**: `MakeWriter::make_writer_for` method, which allows returning a different writer based on a span or event's metadata (#1141) - **fmt**: `MakeWriterExt` trait, with `with_max_level`, `with_min_level`, `with_filter`, `and`, and `or_else` combinators (#1274) - **fmt**: `MakeWriter` implementation for `Arc<W> where &W: io::Write` (#1274) Thanks to @teozkr and @Folyd for contributing to this release!

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>

This backports tokio-rs#1274 from `master`. Depends on tokio-rs#1141. This branch adds a `MakeWriterExt` trait which adds a number of combinators for composing multiple types implementing `MakeWriter`. `MakeWriter`s can now be teed together, filtered with minimum and maximum levels, filtered with a `Metadata` predicate, and combined with a fallback for when a writer is _not_ made for a particular `Metadata`. I've also added a `MakeWriter` impl for `Arc<W>` when `&W` implements `Write`. This enables `File`s to be used as `MakeWriter`s similarly to how we will be able to in 0.3, with the additional overhead of having to do ref-counting because we can't return a reference from `MakeWriter` until the next breaking change. Signed-off-by: Eliza Weisman <eliza@buoyant.io>

@Folyd

# 0.2.19 (June 25, 2021) ### Deprecated - **registry**: `SpanRef::parents`, `SpanRef::from_root`, and `Context::scope` iterators, which are replaced by new `SpanRef::scope` and `Scope::from_root` iterators (tokio-rs#1413) ### Added - **registry**: `SpanRef::scope` method, which returns a leaf-to-root `Iterator` including the leaf span (tokio-rs#1413) - **registry**: `Scope::from_root` method, which reverses the `scope` iterator to iterate root-to-leaf (tokio-rs#1413) - **registry**: `Context::event_span` method, which looks up the parent span of an event (tokio-rs#1434) - **registry**: `Context::event_scope` method, returning a `Scope` iterator over the span scope of an event (tokio-rs#1434) - **fmt**: `MakeWriter::make_writer_for` method, which allows returning a different writer based on a span or event's metadata (tokio-rs#1141) - **fmt**: `MakeWriterExt` trait, with `with_max_level`, `with_min_level`, `with_filter`, `and`, and `or_else` combinators (tokio-rs#1274) - **fmt**: `MakeWriter` implementation for `Arc<W> where &W: io::Write` (tokio-rs#1274) Thanks to @teozkr and @Folyd for contributing to this release!

hawkw added 3 commits December 15, 2020 10:05

subscriber: add MakeWriter::make_writer_for

a4b727a

fmt: call make_writer_for when formatting events

5154e11

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

fmt: add test for writers based on meta

8f163bd

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

hawkw requested a review from davidbarsky December 15, 2020 18:54

hawkw requested a review from a team as a code owner December 15, 2020 18:54

hawkw self-assigned this Dec 15, 2020

hawkw mentioned this pull request Dec 15, 2020

tracing-syslog crate #1137

Closed

max-heller reviewed Dec 17, 2020

View reviewed changes

fixup docs

24a9022

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

davidbarsky approved these changes Mar 1, 2021

View reviewed changes

hawkw mentioned this pull request Mar 1, 2021

subscriber: add MakeWriter combinators #1274

Merged

hawkw added 2 commits March 12, 2021 10:33

Merge branch 'master' into eliza/make-for-metadata

43535cf

doctest fixy

ba74af9

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

davidbarsky added 2 commits May 14, 2021 17:30

Merge branch 'master' into eliza/make-for-metadata

22a1d45

Merge branch 'master' into eliza/make-for-metadata

aff0b83

davidbarsky merged commit b3af7e1 into master May 14, 2021

davidbarsky deleted the eliza/make-for-metadata branch May 14, 2021 22:55

hawkw mentioned this pull request Jun 25, 2021

backport recent changes from master #1447

Merged

hawkw mentioned this pull request Jun 26, 2021

subscriber: prepare to release 0.2.19 #1448

Merged

davidbarsky mentioned this pull request Sep 26, 2023

chore: backport roughly a year's worth of changes #2728

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

subscriber: add `MakeWriter::make_writer_for` #1141

subscriber: add `MakeWriter::make_writer_for` #1141

hawkw commented Dec 15, 2020 •

edited

Loading

max-heller commented Dec 15, 2020

max-heller Dec 17, 2020

max-heller Dec 17, 2020

hawkw commented Mar 1, 2021

davidbarsky left a comment

raftario commented May 13, 2021 •

edited

Loading

subscriber: add MakeWriter::make_writer_for #1141

subscriber: add MakeWriter::make_writer_for #1141

Conversation

hawkw commented Dec 15, 2020 • edited Loading

Motivation

Solution

max-heller commented Dec 15, 2020

max-heller Dec 17, 2020

Choose a reason for hiding this comment

max-heller Dec 17, 2020

Choose a reason for hiding this comment

hawkw commented Mar 1, 2021

davidbarsky left a comment

Choose a reason for hiding this comment

raftario commented May 13, 2021 • edited Loading

subscriber: add `MakeWriter::make_writer_for` #1141

subscriber: add `MakeWriter::make_writer_for` #1141

hawkw commented Dec 15, 2020 •

edited

Loading

raftario commented May 13, 2021 •

edited

Loading