Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions tracing-subscriber/src/subscribe/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1485,7 +1485,10 @@ where
fn max_level_hint(&self) -> Option<LevelFilter> {
match self {
Some(ref inner) => inner.max_level_hint(),
None => None,
None => {
// There is no underlying `Subscribe` so our max level is OFF
Some(LevelFilter::OFF)
}
}
}

Expand Down Expand Up @@ -1690,7 +1693,8 @@ feature! {
}

fn max_level_hint(&self) -> Option<LevelFilter> {
let mut max_level = LevelFilter::ERROR;
// Default to `OFF` if there are no underlying `Subscribe`s
let mut max_level = LevelFilter::OFF;
for s in self {
// NOTE(eliza): this is slightly subtle: if *any* subscriber
// returns `None`, we have to return `None`, assuming there is
Expand Down
1 change: 1 addition & 0 deletions tracing-subscriber/tests/subscriber_filters/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
mod support;
use self::support::*;
mod filter_scopes;
mod option;
mod targets;
mod trees;
mod vec;
Expand Down
45 changes: 45 additions & 0 deletions tracing-subscriber/tests/subscriber_filters/option.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
use super::*;
use tracing::Collect;

// This test is just used to compare to the tests below
#[test]
fn just_layer() {
let info = subscriber::named("info")
.run()
.with_filter(LevelFilter::INFO)
.boxed();

let collector = tracing_subscriber::registry().with(info);
assert_eq!(collector.max_level_hint(), Some(LevelFilter::INFO));
}

#[test]
fn layer_and_option_layer() {
let info = subscriber::named("info")
.run()
.with_filter(LevelFilter::INFO)
.boxed();

let debug = subscriber::named("debug")
.run()
.with_filter(LevelFilter::DEBUG)
.boxed();

let mut collector = tracing_subscriber::registry().with(info).with(Some(debug));
assert_eq!(collector.max_level_hint(), Some(LevelFilter::DEBUG));

let error = subscriber::named("error")
.run()
.with_filter(LevelFilter::ERROR)
.boxed();
// None means the other layer takes control
collector = tracing_subscriber::registry().with(error).with(None);
assert_eq!(collector.max_level_hint(), Some(LevelFilter::ERROR));
}

#[test]
fn just_option_layer() {
// Just a None means everything is off
let collector = tracing_subscriber::registry().with(None::<ExpectSubscriber>);
assert_eq!(collector.max_level_hint(), Some(LevelFilter::OFF));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should probably have the same tests for the Subscribe implementation for Option, as well as the Filter impl.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There isn't a filter impl is there? I can move these tests out of this directory

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, i misspoke: there isn't a Filter impl for Option<impl Filter>, but some of these tests do also exercise per-layer filtering, because they use with_filter on the inner subscribers. thus, those tests do need to be in this test module because of feature flagging. however, we could have other tests in a different module that don't use with_filter, and instead use the Layer impl for LevelFilter...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooh, I like the Layer impl of LevelFilter, let me push that up!

7 changes: 7 additions & 0 deletions tracing-subscriber/tests/subscriber_filters/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,10 @@ fn all_filtered_max_level_hint() {

assert_eq!(collector.max_level_hint(), Some(LevelFilter::DEBUG));
}

#[test]
fn empty_vec() {
// Just a None means everything is off
let collector = tracing_subscriber::registry().with(Vec::<ExpectSubscriber>::new());
assert_eq!(collector.max_level_hint(), Some(LevelFilter::OFF));
}