subscriber: better default filtering

[hawkw]

Feature Request

Crates

• tracing-subscriber

Motivation

Currently, the tracing-subscriber FmtSubscriber::default
implementation defaults to no filtering. This means that all
instrumentation, including extremely verbose trace-level diagnostics
from crates like tokio are enabled by default.

This is because the subscriber itself does not implement filtering, in
order to allow it to be composed with filters implemented by Layers.
However, defaulting to no filtering at all is definitely a surprising
behavior. I didn't want to conditionally return a different type based
on whether or not filtering was enabled by the filter feature flag,
but this is probably not worth the confusion introduced by this
behavior. We should make this more intuitive.

Solution

This branch changes tracing-subscriber to default to enabling
the INFO level and above. If the filter feature flag is enabled,
users may opt-in to env_logger-style filtering. Additionally,
regardless of feature flags, a new with_max_level method is
added to the FmtSubscriber builder, which takes a Level or
LevelFilter. LevelFilter now implements Layer by enabling
any spans and events that are less than or equal to that Level.

Fixes: #331
Fixes: #332

[Ralith]

I'm a bit confused by the description and change here. The behavior I'd like to see is that, regardless of whether the filter feature is enabled or not, the FmtSubscriber would display all messages at info level or above. To respect RUST_LOG, a user would have to opt in to it by explicitly passing in Filter::from_default_env(). This ensures that neither types nor default behavior change solely because a feature flag is enabled, which should minimize unpleasant surprises.

In addition to the high potential surprise, it looks like this change preserves the default behavior of displaying everything, which is practically unusable if you're using tokio (or any hypothetical other library that contains many trace statements).

[Ralith]

To clarify, my suggestion was that the default FmtSubscriber not use Filter at all, but some other filtering implementation which would hopefully be simple enough not to require any form of feature gating.

[hawkw]

To clarify, my suggestion was that the default FmtSubscriber not use Filter at all, but some other filtering implementation which would hopefully be simple enough not to require any form of feature gating.

Ah, I misunderstood your suggestion. Thanks for clarifying that. This makes significantly more sense.

[hawkw]

@Ralith I've changed this up a bit so that it actually implements the behavior you suggested (which, IMO, makes significantly more sense). The default max level is now always INFO; env-logger filtering is never used by default. Users can set a different max level even without the filter feature enabled, or can change to an env-logger style filter.

Let me know what you think now?

[Ralith]

Looks awesome, thanks! I think this will make the stack a lot more approachable.

It might be clearer to rename the type (and feature) from Filter to EnvFilter or similar, but that can be handled independently of this change.

[hawkw]

@Ralith I also thought renaming to `EnvFilter` (and the corresponding feature flag) was probably a good idea, but I wanted to do that in a separate branch since there would be a lot of find-and-replace.

…

On Tue, Sep 10, 2019 at 11:40 AM Benjamin Saunders ***@***.***> wrote: ***@***.**** approved this pull request. Looks awesome, thanks! I think this will make the stack a lot more approachable. It might be clearer to rename the type (and feature) from Filter to EnvFilter or similar, but that can be handled independently of this change. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#336?email_source=notifications&email_token=AAVKXMWBX6PZ3UORZYI327DQI7SYTA5CNFSM4IVA4ME2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCEIXKSY#pullrequestreview-286356811>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAVKXMRNCCBGCCYT2ADARADQI7SYTANCNFSM4IVA4MEQ> .