fmt: support customizing output stream #299

jarrodldavis · 2019-08-21T06:52:43Z

Motivation

Closes #225

When logging trace data, users may wish to write to a number of output streams, such as to stdout, to stderr, or to a file, depending on their use-case. Currently, however, the IO stream that tracing-fmt writes to is hard-coded to stdout

Solution

Add a new trait, NewWriter, to abstract over the action of creating a new object that implements std::io::Write
Add a new type parameter to FmtSubscriber and its Builder for that trait
Default that type parameter to std::io::stdout
Add documentation as needed
Add tests for the new functionality
Add example(s) to demonstrate the new functionality?

- Add a new trait, `NewWriter`, that represents a type that can create a writer (an object that implements `std::io::Writer`) - Add an implementation of that trait that wraps `std::io::stdout` - Add a new type parameter, `W`, to `FmtSubscriber` and it's `Builder`, that allows overriding the default `NewWriter` implementation, which is `NewStdout`

- add implementation of `NewWriter` for functions that return an object that implements `std::io::Writer` - update the default type and value for the `W` type parameter of `FmtSubscriber` and it's `Builder` to directly use `std::io::stdout`

tracing-fmt/src/lib.rs

jarrodldavis · 2019-08-21T07:01:23Z

tracing-fmt/src/lib.rs

@@ -206,6 +215,24 @@ where
    }
 }

+pub trait NewWriter {


I took this name from the proposal in #225, but let me know if a different name is desired.

We might want to consider renaming this to MakeWriter, for consistency with the MakeVisitor trait that will replace NewVisitor in #241 --- see tower-rs/tower#108 (comment) and #240 (comment) for a discussion of this naming.

jarrodldavis · 2019-08-23T06:51:42Z

I'm having issues trying to get a mock NewWriter implementation for a unit test (or even an example) because it's not possible to return a mutable reference from NewWriter::new_writer, but mutable references are needed to provide implementations of std::io::Write.

Unfortunately, I can't relax the immutable self restriction of NewWriter::new_writer since it's used in FmtSubscriber::event (which is the implementation for Subscriber::event), and Subscriber::event also has an immutable self restriction.

- Add `MockWriter` that proxies `io::Write` implementation to a mutex-protected byte vector - Add unit test that uses `MockWriter` with a shared byte buffer to test `FmtSubscriber`'s behavior when customizing the writer

jarrodldavis · 2019-08-24T18:40:22Z

I finally figured out how to create a mock writer with a Mutex and have added a unit test for Builder::with_writer.

- move `NewWriter` and related code (including the unit test) into a `writer` submodule - move the `extern crate` statement for `lazy_static` to the top of `lib.rs` - remove now-unnecessary TODO comment in `FmtSubscriber::event`

- consolidate common logic for both `Builder::with_writer` unit tests - replace wildcard `use` statement with specific types used from the crate root

- move trait bounds for type parameter `T` in `writer::test::test_writer` to a `where` clause

hawkw

This looks really close! I noticed some issues with the links in RustDoc that I think it would be good to fix. In addition, I commented on some code style nits, but those are not blockers.

I think this will be ready to merge once this round of feedback has been addressed! Thanks for adding this.

tracing-fmt/src/writer.rs

tracing-fmt/src/lib.rs

- rename `NewWriter` to `MakeWriter`, and its method `new_writer` to `make_writer`

- replace intra-rustdoc links with URL reference links

- add a `stderr` example for `Builder::make_writer`

- move the impl block for `Builder::make_writer` back to the create root with the other impl blocks

- update the `Builder::make_writer` unit tests to not be dependent on ANSI codes - use Edition 2018 idioms for importing the `lazy_static` macro

jarrodldavis · 2019-08-25T23:24:42Z

tracing-fmt/src/writer.rs

+/// return an instance of [`io::Write`], such as [`io::stdout`] and [`io::stderr`].
+///
+/// [`io::Write`]: https://doc.rust-lang.org/std/io/trait.Write.html
+/// [`FmtSubscriber`]: struct.FmtSubscriber.html


This link will work from the trait.MakeWriter.html page at the create root (e.g. https://docs.rs/tracing-fmt/latest/tracing_fmt/trait.MakeWriter.html) but not from the writer sub-module page (e.g. https://docs.rs/tracing-fmt/latest/tracing_fmt/writer/trait.MakeWriter.html). Would you like me to use an absolute URL instead?

I also noticed a similar issue with tracing::Subscriber; compare the links to Interest on these two pages:

https://docs.rs/tracing/0.1.6/tracing/trait.Subscriber.html

https://docs.rs/tracing/0.1.6/tracing/subscriber/trait.Subscriber.html

Hmm, yeah, we should probably fix that.

My concern with absolute URLs is that if we use an absolute docs.rs URL, then we'll link to docs.rs even in local builds or (eventually) master RustDoc built on CI, which could be potentially confusing. It's possible that there's no solution that avoids this while also fixing the problem you mentioned though.

Okay, updated links to FmtSubscriber to use absolute https://docs.rs URLs. I had to use a specific version instead of latest since latest points to the empty holding version 0.1.0.

- use absolute URLs for links to `FmtSubscriber`

hawkw · 2019-08-26T21:32:46Z

Great, this looks good to me! Thanks for all your work on this @jarrodldavis!

jarrodldavis added 4 commits August 20, 2019 23:57

fmt: generalize writer trait

6fcc85c

- add implementation of `NewWriter` for functions that return an object that implements `std::io::Writer` - update the default type and value for the `W` type parameter of `FmtSubscriber` and it's `Builder` to directly use `std::io::stdout`

fmt: rustfmt

1ee893f

fmt: add builder function for setting the writer

b8b9a21

jarrodldavis commented Aug 21, 2019

View reviewed changes

fmt: add test for customizing writer

d3cd76f

- Add `MockWriter` that proxies `io::Write` implementation to a mutex-protected byte vector - Add unit test that uses `MockWriter` with a shared byte buffer to test `FmtSubscriber`'s behavior when customizing the writer

jarrodldavis mentioned this pull request Aug 24, 2019

fmt: with_ansi(false) doesn't affect time output #300

Closed

jarrodldavis added 5 commits August 24, 2019 13:06

fmt: move writer into submodule

03acd6f

- move `NewWriter` and related code (including the unit test) into a `writer` submodule - move the `extern crate` statement for `lazy_static` to the top of `lib.rs` - remove now-unnecessary TODO comment in `FmtSubscriber::event`

fmt: add example for printing to stderr

225c824

fmt: add unit test for custom writer struct

84ded82

fmt: refactor writer unit tests

7149c6e

- consolidate common logic for both `Builder::with_writer` unit tests - replace wildcard `use` statement with specific types used from the crate root

fmt: finish writer documentation comments

956857b

jarrodldavis marked this pull request as ready for review August 24, 2019 22:44

fmt: reformat

7d79a3e

- move trait bounds for type parameter `T` in `writer::test::test_writer` to a `where` clause

hawkw requested changes Aug 25, 2019

View reviewed changes

jarrodldavis added 5 commits August 25, 2019 12:57

fmt: rename writer abstraction

8f70f8f

- rename `NewWriter` to `MakeWriter`, and its method `new_writer` to `make_writer`

fmt: fix links for rustdoc stable

c29482d

- replace intra-rustdoc links with URL reference links

fmt: add example for make_writer

a3f514a

- add a `stderr` example for `Builder::make_writer`

fmt: move make_writer back to crate root

0b9a737

- move the impl block for `Builder::make_writer` back to the create root with the other impl blocks

fmt: refactor writer tests

d19003a

- update the `Builder::make_writer` unit tests to not be dependent on ANSI codes - use Edition 2018 idioms for importing the `lazy_static` macro

jarrodldavis commented Aug 25, 2019

View reviewed changes

fmt: fix rustdoc links

c4c6332

- use absolute URLs for links to `FmtSubscriber`

hawkw approved these changes Aug 26, 2019

View reviewed changes

hawkw merged commit 83aa04c into tokio-rs:master Aug 26, 2019

jarrodldavis deleted the fmt-output-stream branch August 26, 2019 22:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fmt: support customizing output stream #299

fmt: support customizing output stream #299

jarrodldavis commented Aug 21, 2019 •

edited

Loading

jarrodldavis Aug 21, 2019

hawkw Aug 25, 2019

jarrodldavis commented Aug 23, 2019

jarrodldavis commented Aug 24, 2019

hawkw left a comment

jarrodldavis Aug 25, 2019

hawkw Aug 26, 2019

jarrodldavis Aug 26, 2019

hawkw commented Aug 26, 2019

fmt: support customizing output stream #299

fmt: support customizing output stream #299

Conversation

jarrodldavis commented Aug 21, 2019 • edited Loading

Motivation

Solution

jarrodldavis Aug 21, 2019

Choose a reason for hiding this comment

hawkw Aug 25, 2019

Choose a reason for hiding this comment

jarrodldavis commented Aug 23, 2019

jarrodldavis commented Aug 24, 2019

hawkw left a comment

Choose a reason for hiding this comment

jarrodldavis Aug 25, 2019

Choose a reason for hiding this comment

hawkw Aug 26, 2019

Choose a reason for hiding this comment

jarrodldavis Aug 26, 2019

Choose a reason for hiding this comment

hawkw commented Aug 26, 2019

jarrodldavis commented Aug 21, 2019 •

edited

Loading