subscriber: adds From<Level> for Directive #918

leshow · 2020-08-12T16:02:51Z

Motivation

If you have a Level and want to set a directive for EnvFilter you need to do the mechanical transformation of: .add_directive(LevelFilter::from_level(log_lvl).into()). To me, it wasn't obvious initially that this was even possible, I had to look through the docs for two different tracing repos to find that the pathway would work.

Solution

If this implementation is added, the definition will be shown on the Directive docs page, and you can write .add_directive(Level::INFO.into()) and skip the intermediate transformation

There was some discussion on discord about this, if it's not desirable because of other reasons feel free to close

hawkw

This is great, thanks!

If you have a minute, it might be nice to add an example in the add_directive docs (here) showing usage with a Level rather than a LevelFilter? That should make this more obvious for other users.

hawkw

thanks for adding the docs! i had a couple little nits to pick, but if we get those taken care of, i think this will be good to merge!

hawkw · 2020-08-12T17:17:32Z

tracing-subscriber/src/filter/env/mod.rs

    /// enable all traces at or below a certain verbosity level, or
    /// parsed from a string specifying a directive.
    ///
    /// If a filter directive is inserted that matches exactly the same spans
    /// and events as a previous filter, but sets a different level for those
    /// spans and events, the previous directive is overwritten.
    ///
-    /// [`LevelFilter`]: struct.LevelFilter.html
+    /// [`LevelFilter`]: ../filter/struct.LevelFilter.html
+    /// [`Level`]: ../../tracing_core/struct.Level.html


does this link work on docs.rs? i think relative URLs like this will work locally & on netlify, since we are building all the other docs in the same target dir, but i don't think it will work on docs.rs. typically, i use the full docs.rs URL when linking across crates.

When I built with cargo doc --open it did. I actually copied it from the top of the same file, there's a bunch that use that path. If it doesn't work on docsrs we may want to change the links at the top too.

Yeah so I just tried https://docs.rs/tracing-subscriber/0.2.11/tracing_subscriber/filter/struct.EnvFilter.html and none of the links at the top work, but they do work locally.

So, what should I put here? literally https://docs.rs/tracing-core/0.1.14/tracing_core/metadata/struct.Level.html ?

So, what should I put here? literally https://docs.rs/tracing-core/0.1.14/tracing_core/metadata/struct.Level.html ?

Yup, that's right.

hawkw · 2020-08-12T17:18:11Z

tracing-subscriber/src/filter/env/mod.rs

+    /// From [`LevelFilter`]
    /// ```rust
    /// use tracing_subscriber::filter::{EnvFilter, LevelFilter};
    /// let mut filter = EnvFilter::from_default_env()
    ///     .add_directive(LevelFilter::INFO.into());
    /// ```
+    /// Or from [`Level`]


a little formatting nit:

Suggested change

/// From [`LevelFilter`]

/// ```rust

/// use tracing_subscriber::filter::{EnvFilter, LevelFilter};

/// let mut filter = EnvFilter::from_default_env()

/// .add_directive(LevelFilter::INFO.into());

/// ```

/// Or from [`Level`]

///

/// From [`LevelFilter`]:

///

/// ```rust

/// use tracing_subscriber::filter::{EnvFilter, LevelFilter};

/// let mut filter = EnvFilter::from_default_env()

/// .add_directive(LevelFilter::INFO.into());

/// ```

///

/// Or from [`Level`]:

///

hawkw · 2020-08-12T17:18:32Z

tracing-subscriber/src/filter/env/mod.rs

+    /// ```
+    /// Parsed from a string


nit:

Suggested change

/// ```

/// Parsed from a string

/// ```

///

/// Parsed from a string:

///

hawkw

once the CI build passes, i'll be happy to merge this!

hawkw · 2020-08-12T18:55:16Z

tracing-subscriber/src/filter/env/mod.rs

+    ///
+    /// ```rust
+    /// # use tracing_subscriber::filter::{EnvFilter, LevelFilter};
+    /// # use tracing::Level


this doctest doesn't compile, because it's missing a semicolon. i think adding that

Suggested change

/// # use tracing::Level

/// # use tracing::Level;

should fix CI.

Woops, fixed that.

tracing-subscriber/src/filter/env/mod.rs

hawkw

okay, this looks great! thanks again!

@Pothulapati

Fixed - **env-filter**: Fixed a regression where `Option<Level>` lost its `Into<LevelFilter>` impl ([#966]) - **env-filter**: Fixed `EnvFilter` enabling spans that should not be enabled when multiple subscribers are in use ([#927]) Changed - **json**: `format::Json` now outputs fields in a more readable order ([#892]) - Updated `tracing-core` dependency to 0.1.16 Added - **fmt**: Add `BoxMakeWriter` for erasing the type of a `MakeWriter` implementation ([#958]) - **fmt**: Add `TestWriter` `MakeWriter` implementation to support libtest output capturing ([#938]) - **layer**: Add `Layer` impl for `Option<T> where T: Layer` ([#910]) - **env-filter**: Add `From<Level>` impl for `Directive` ([#918]) - Multiple documentation fixes and improvements Thanks to @Pothulapati, @samrg472, @bryanburgers, @keetonian, and @SriRamanujam for contributing to this release! [#927]: #927 [#966]: #966 [#958]: #958 [#892]: #892 [#938]: #938 [#910]: #910 [#918]: #918

@Pothulapati

# 0.2.12 (September 8, 2020) ### Fixed - **env-filter**: Fixed a regression where `Option<Level>` lost its `Into<LevelFilter>` impl ([#966]) - **env-filter**: Fixed `EnvFilter` enabling spans that should not be enabled when multiple subscribers are in use ([#927]) ### Changed - **json**: `format::Json` now outputs fields in a more readable order ([#892]) - Updated `tracing-core` dependency to 0.1.16 ### Added - **fmt**: Add `BoxMakeWriter` for erasing the type of a `MakeWriter` implementation ([#958]) - **fmt**: Add `TestWriter` `MakeWriter` implementation to support libtest output capturing ([#938]) - **layer**: Add `Layer` impl for `Option<T> where T: Layer` ([#910]) - **env-filter**: Add `From<Level>` impl for `Directive` ([#918]) - Multiple documentation fixes and improvements Thanks to @Pothulapati, @samrg472, @bryanburgers, @keetonian, and @SriRamanujam for contributing to this release! [#927]: #927 [#966]: #966 [#958]: #958 [#892]: #892 [#938]: #938 [#910]: #910 [#918]: #918

leshow requested review from hawkw and a team as code owners August 12, 2020 16:02

hawkw approved these changes Aug 12, 2020

View reviewed changes

hawkw reviewed Aug 12, 2020

View reviewed changes

subscriber: adds From<Level> for Directive

6b06a1e

hawkw approved these changes Aug 12, 2020

View reviewed changes

hawkw merged commit fb4443f into tokio-rs:master Aug 12, 2020

hawkw mentioned this pull request Sep 8, 2020

subscriber: prepare to release 0.2.12 #970

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

subscriber: adds From<Level> for Directive #918

subscriber: adds From<Level> for Directive #918

leshow commented Aug 12, 2020

hawkw left a comment

hawkw left a comment

hawkw Aug 12, 2020

leshow Aug 12, 2020

leshow Aug 12, 2020 •

edited

Loading

hawkw Aug 12, 2020

hawkw Aug 12, 2020

hawkw Aug 12, 2020

hawkw left a comment

hawkw Aug 12, 2020

leshow Aug 12, 2020

hawkw left a comment

subscriber: adds From<Level> for Directive #918

subscriber: adds From<Level> for Directive #918

Conversation

leshow commented Aug 12, 2020

Motivation

Solution

hawkw left a comment

Choose a reason for hiding this comment

hawkw left a comment

Choose a reason for hiding this comment

hawkw Aug 12, 2020

Choose a reason for hiding this comment

leshow Aug 12, 2020

Choose a reason for hiding this comment

leshow Aug 12, 2020 • edited Loading

Choose a reason for hiding this comment

hawkw Aug 12, 2020

Choose a reason for hiding this comment

hawkw Aug 12, 2020

Choose a reason for hiding this comment

hawkw Aug 12, 2020

Choose a reason for hiding this comment

hawkw left a comment

Choose a reason for hiding this comment

hawkw Aug 12, 2020

Choose a reason for hiding this comment

leshow Aug 12, 2020

Choose a reason for hiding this comment

hawkw left a comment

Choose a reason for hiding this comment

leshow Aug 12, 2020 •

edited

Loading