From 7c5bc38a9c610c49ff0c992b9e153befdb1f5423 Mon Sep 17 00:00:00 2001 From: Chris Connelly Date: Sat, 23 Jul 2022 18:18:49 +0100 Subject: [PATCH 1/4] subscriber: add a method to `Targets` to get the default level This makes it possible to fully "override" some base `Targets` filter with another (e.g. user-supplied) filter. Without some way to obtain the default, only explicit targets can be overridden (via `IntoIter` and friends). Ideally the method would be named `default`, corresponding to `with_default`, however this conflicts with `Default::default` and so would be a breaking change (and harm ergonomics). `default_level` seemed a better name than `get_default`, since "getters" of this style are generally considered unidiomatic[citation needed]. --- tracing-subscriber/src/filter/targets.rs | 40 ++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tracing-subscriber/src/filter/targets.rs b/tracing-subscriber/src/filter/targets.rs index 626fc5c39b..00b4bb731f 100644 --- a/tracing-subscriber/src/filter/targets.rs +++ b/tracing-subscriber/src/filter/targets.rs @@ -277,6 +277,31 @@ impl Targets { self } + /// Returns the explicitly set default level for this filter, if any. + /// + /// If a default level was set explicitly using [`with_default`](Self::with_default), that level + /// will be returned. Otherwise `None` will be returned. + /// + /// A return value of `None` is behaviourly equivalent to [`LevelFilter::OFF`], but + /// distinguishes between an explicitly set default level or the default one. If you only care + /// about the behaviour you can use `unwrap_or`: + /// + /// ``` + /// use tracing_subscriber::filter::{LevelFilter, Targets}; + /// + /// let filter = Targets::new(); + /// let default_level = filter.default_level().unwrap_or(LevelFilter::OFF); + /// + /// assert_eq!(default_level, LevelFilter::OFF); + /// ``` + pub fn default_level(&self) -> Option { + self.0 + .directives() + .into_iter() + .find(|d| d.target.is_none()) + .map(|d| d.level) + } + /// Returns an iterator over the [target]-[`LevelFilter`] pairs in this filter. /// /// The order of iteration is undefined. @@ -685,6 +710,21 @@ mod tests { ); } + #[test] + fn targets_default_level() { + let filter = expect_parse("crate1::mod1=error,crate1::mod2,crate2=debug,crate3=off"); + assert_eq!(filter.default_level(), None); + + let filter = expect_parse("crate1::mod1=error,crate1::mod2,crate2=debug,crate3=off") + .with_default(LevelFilter::OFF); + assert_eq!(filter.default_level(), Some(LevelFilter::OFF)); + + let filter = expect_parse("crate1::mod1=error,crate1::mod2,crate2=debug,crate3=off") + .with_default(LevelFilter::OFF) + .with_default(LevelFilter::INFO); + assert_eq!(filter.default_level(), Some(LevelFilter::INFO)); + } + #[test] // `println!` is only available with `libstd`. #[cfg(feature = "std")] From 3f127c0b701c5a153e29fe7ae1ab3d80d41a7749 Mon Sep 17 00:00:00 2001 From: Chris Connelly Date: Sat, 23 Jul 2022 23:03:50 +0100 Subject: [PATCH 2/4] use `find_map` instead of `find` and `map` Co-authored-by: Eliza Weisman --- tracing-subscriber/src/filter/targets.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tracing-subscriber/src/filter/targets.rs b/tracing-subscriber/src/filter/targets.rs index 00b4bb731f..a35f163790 100644 --- a/tracing-subscriber/src/filter/targets.rs +++ b/tracing-subscriber/src/filter/targets.rs @@ -295,11 +295,13 @@ impl Targets { /// assert_eq!(default_level, LevelFilter::OFF); /// ``` pub fn default_level(&self) -> Option { - self.0 - .directives() - .into_iter() - .find(|d| d.target.is_none()) - .map(|d| d.level) + self.0.directives().into_iter().find_map(|d| { + if d.target.is_none() { + Some(d.level) + } else { + None + } + }) } /// Returns an iterator over the [target]-[`LevelFilter`] pairs in this filter. From 83e4a47549cc534702fc916bba0084c26ef8d23e Mon Sep 17 00:00:00 2001 From: Chris Connelly Date: Sat, 23 Jul 2022 23:40:38 +0100 Subject: [PATCH 3/4] subscriber: update `Targets::default_level` docs The new documentation includes more complete examples showing defaults set by `with_default` or parsed, as well as examples of no defaults and a specifically set `LevelFilter::OFF` default. You may also notice that 2nd person language has been removed. --- tracing-subscriber/src/filter/targets.rs | 42 +++++++++++++++++++----- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/tracing-subscriber/src/filter/targets.rs b/tracing-subscriber/src/filter/targets.rs index a35f163790..3774e88420 100644 --- a/tracing-subscriber/src/filter/targets.rs +++ b/tracing-subscriber/src/filter/targets.rs @@ -277,22 +277,48 @@ impl Targets { self } - /// Returns the explicitly set default level for this filter, if any. + /// Returns the default level for this filter, if set. /// - /// If a default level was set explicitly using [`with_default`](Self::with_default), that level - /// will be returned. Otherwise `None` will be returned. + /// The default level can be set for a filter either by using + /// [`with_default`](Self::with_default) or when parsing from a filter string that includes a + /// level without a target (e.g. `"trace"`). /// - /// A return value of `None` is behaviourly equivalent to [`LevelFilter::OFF`], but - /// distinguishes between an explicitly set default level or the default one. If you only care - /// about the behaviour you can use `unwrap_or`: + /// # Examples /// /// ``` /// use tracing_subscriber::filter::{LevelFilter, Targets}; /// + /// let filter = Targets::new().with_default(LevelFilter::INFO); + /// assert_eq!(filter.default_level(), Some(LevelFilter::INFO)); + /// + /// let filter: Targets = "info".parse().unwrap(); + /// assert_eq!(filter.default_level(), Some(LevelFilter::INFO)); + /// ``` + /// + /// The default level is `None` if no default is set: + /// + /// ``` + /// use tracing_subscriber::filter::Targets; + /// /// let filter = Targets::new(); - /// let default_level = filter.default_level().unwrap_or(LevelFilter::OFF); + /// assert_eq!(filter.default_level(), None); + /// + /// let filter: Targets = "my_crate=info".parse().unwrap(); + /// assert_eq!(filter.default_level(), None); + /// ``` + /// + /// Note that an unset default level (`None`) behaves like `LevelFilter::OFF` when the filter is + /// used, but it could also be set explicitly which may be useful to distinguish (such as when + /// merging multiple `Targets`). + /// + /// ``` + /// use tracing_subscriber::filter::{LevelFilter, Targets}; + /// + /// let filter = Targets::new().with_default(LevelFilter::OFF); + /// assert_eq!(filter.default_level(), Some(LevelFilter::OFF)); /// - /// assert_eq!(default_level, LevelFilter::OFF); + /// let filter: Targets = "off".parse().unwrap(); + /// assert_eq!(filter.default_level(), Some(LevelFilter::OFF)); /// ``` pub fn default_level(&self) -> Option { self.0.directives().into_iter().find_map(|d| { From f630db72082b03a88ce2d60d7e989b2f1a16c783 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Mon, 25 Jul 2022 09:58:00 -0700 Subject: [PATCH 4/4] Apply docs suggestions from code review --- tracing-subscriber/src/filter/targets.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tracing-subscriber/src/filter/targets.rs b/tracing-subscriber/src/filter/targets.rs index 3774e88420..2dee1c4645 100644 --- a/tracing-subscriber/src/filter/targets.rs +++ b/tracing-subscriber/src/filter/targets.rs @@ -277,7 +277,10 @@ impl Targets { self } - /// Returns the default level for this filter, if set. + /// Returns the default level for this filter, if one is set. + /// + /// The default level is used to filter any spans or events with targets + /// that do not match any of the configured set of prefixes. /// /// The default level can be set for a filter either by using /// [`with_default`](Self::with_default) or when parsing from a filter string that includes a @@ -307,7 +310,7 @@ impl Targets { /// assert_eq!(filter.default_level(), None); /// ``` /// - /// Note that an unset default level (`None`) behaves like `LevelFilter::OFF` when the filter is + /// Note that an unset default level (`None`) behaves like [`LevelFilter::OFF`] when the filter is /// used, but it could also be set explicitly which may be useful to distinguish (such as when /// merging multiple `Targets`). ///