From 27c71d45eb9915f8ad2d87b6fb0940c1533ff15e Mon Sep 17 00:00:00 2001 From: Chris Connelly Date: Sat, 18 Sep 2021 18:42:15 +0100 Subject: [PATCH] subscriber: add `Targets::targets` method to iterate directives (#1574) There's currently no way to get the registered directives from a `Targets` instance, which is a barrier to some use-cases such as combining user-supplied directives with application-supplied defaults. This commit adds a `Targets::targets` method, which returns an iterator of `(&str, LevelFilter)` pairs, one for each directive (excluding the default level, whose `target` is `None`). Items are yielded as per the underlying `DirectiveSet::directives`, which would yield itms in specifity order (as constructed by `DirectiveSet::add`). However, the order has been left explicitly undefined in the documentation for `Targets::targets` on the basis that this is an implementation detail that may change. ## Motivation As hinted in the commit message, my use-case is to have an application-supplied 'base' `Targets` filter, which can then be extended/overridden by a user-supplied filter parsed from `RUST_LOG` (e.g.). Sadly, there's currently no way of getting the directives out of a `Targets` instance, nor to re-serialize to the string representation. Thus, the only way to implement the above would be to manually parse the user-supplied filters in the application, which is shame since it duplicates the implementation here and would be prone to drift/subtle incompatibilities. ## Solution The proposed solution is to add methods to `Targets` to retrieve the underlying directives. ```rust // application-defined defaults, for a useful level of information let mut filter = Targets::new() .with_target("my_crate", LevelFilter::INFO) .with_target("important_dependency", LevelFilter::INFO); if let Ok(overrides) = std::env::var("RUST_LOG") { let overrides: Targets = overrides.parse().unwrap(); // merge user-supplied overrides, which can enable additional tracing // or disable default tracing filter.extend(overrides.iter()); // ^^^^^^^^^ this is new } ``` For this PR, I've gone for `fn iter(&self) -> Iter`, e.g. a method `targets` that returns an iterator over the pairs of `(&str, LevelFilter)` in the underlying directives. The iterator excludes any default level(s) where `target` would be `None`. This matches nicely with the `FromIterator` and `Extend` impls, which use `(impl Into, impl Into)`. In addition, I've added `IntoIterator` implementations for `&Targets` and for `Targets`. The by-value `IntoIterator` implementation returns an `IntoIter` type that moves the targets as `String`s, rather than as borrowed `&str`s. This makes `extend` more efficient when moving a second `Targets` by value. --- tracing-subscriber/src/filter/directive.rs | 13 ++ tracing-subscriber/src/filter/mod.rs | 2 +- tracing-subscriber/src/filter/targets.rs | 199 +++++++++++++++++- .../tests/subscriber_filters/targets.rs | 4 +- 4 files changed, 208 insertions(+), 10 deletions(-) diff --git a/tracing-subscriber/src/filter/directive.rs b/tracing-subscriber/src/filter/directive.rs index e95b999a97..5c6b38eb4b 100644 --- a/tracing-subscriber/src/filter/directive.rs +++ b/tracing-subscriber/src/filter/directive.rs @@ -112,6 +112,19 @@ impl Extend for DirectiveSet { } } +impl IntoIterator for DirectiveSet { + type Item = T; + + #[cfg(feature = "smallvec")] + type IntoIter = smallvec::IntoIter<[T; 8]>; + #[cfg(not(feature = "smallvec"))] + type IntoIter = std::vec::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.directives.into_iter() + } +} + // === impl Statics === impl DirectiveSet { diff --git a/tracing-subscriber/src/filter/mod.rs b/tracing-subscriber/src/filter/mod.rs index b48817323f..92959924bb 100644 --- a/tracing-subscriber/src/filter/mod.rs +++ b/tracing-subscriber/src/filter/mod.rs @@ -15,7 +15,7 @@ mod filter_fn; mod level; #[cfg(feature = "registry")] mod subscriber_filters; -mod targets; +pub mod targets; pub use self::directive::ParseError; pub use self::filter_fn::*; diff --git a/tracing-subscriber/src/filter/targets.rs b/tracing-subscriber/src/filter/targets.rs index 4d1bd6fd23..12209c54cf 100644 --- a/tracing-subscriber/src/filter/targets.rs +++ b/tracing-subscriber/src/filter/targets.rs @@ -1,3 +1,11 @@ +//! A [filter] that enables or disables spans and events based on their [target] and [level]. +//! +//! See [`Targets`] for details. +//! +//! [target]: tracing_core::Metadata::target +//! [level]: tracing_core::Level +//! [filter]: crate::layer#filtering-with-layers + use crate::{ filter::{ directive::{DirectiveSet, ParseError, StaticDirective}, @@ -6,7 +14,7 @@ use crate::{ subscribe, }; use std::{ - iter::{Extend, FromIterator}, + iter::{Extend, FilterMap, FromIterator}, str::FromStr, }; use tracing_core::{Collect, Interest, Metadata}; @@ -266,6 +274,34 @@ impl Targets { self } + /// Returns an iterator over the [target]-[`LevelFilter`] pairs in this filter. + /// + /// The order of iteration is undefined. + /// + /// # Examples + /// + /// ``` + /// use tracing_subscriber::filter::{Targets, LevelFilter}; + /// use tracing_core::Level; + /// + /// let filter = Targets::new() + /// .with_target("my_crate", Level::INFO) + /// .with_target("my_crate::interesting_module", Level::DEBUG); + /// + /// let mut targets: Vec<_> = filter.iter().collect(); + /// targets.sort(); + /// + /// assert_eq!(targets, vec![ + /// ("my_crate", LevelFilter::INFO), + /// ("my_crate::interesting_module", LevelFilter::DEBUG), + /// ]); + /// ``` + /// + /// [target]: tracing_core::Metadata::target + pub fn iter(&self) -> Iter<'_> { + self.into_iter() + } + #[inline] fn interested(&self, metadata: &'static Metadata<'static>) -> Interest { if self.0.enabled(metadata) { @@ -344,20 +380,129 @@ impl subscribe::Filter for Targets { } } +impl IntoIterator for Targets { + type Item = (String, LevelFilter); + + type IntoIter = IntoIter; + + fn into_iter(self) -> Self::IntoIter { + IntoIter::new(self) + } +} + +impl<'a> IntoIterator for &'a Targets { + type Item = (&'a str, LevelFilter); + + type IntoIter = Iter<'a>; + + fn into_iter(self) -> Self::IntoIter { + Iter::new(self) + } +} + +/// An owning iterator over the [target]-[level] pairs of a `Targets` filter. +/// +/// This struct is created by the `IntoIterator` trait implementation of [`Targets`]. +/// +/// # Examples +/// +/// Merge the targets from one `Targets` with another: +/// +/// ``` +/// use tracing_subscriber::filter::Targets; +/// use tracing_core::Level; +/// +/// let mut filter = Targets::new().with_target("my_crate", Level::INFO); +/// let overrides = Targets::new().with_target("my_crate::interesting_module", Level::DEBUG); +/// +/// filter.extend(overrides); +/// # drop(filter); +/// ``` +/// +/// [target]: tracing_core::Metadata::target +/// [level]: tracing_core::Level +#[derive(Debug)] +pub struct IntoIter( + #[allow(clippy::type_complexity)] // alias indirection would probably make this more confusing + FilterMap< + as IntoIterator>::IntoIter, + fn(StaticDirective) -> Option<(String, LevelFilter)>, + >, +); + +impl IntoIter { + fn new(targets: Targets) -> Self { + Self(targets.0.into_iter().filter_map(|directive| { + let level = directive.level; + directive.target.map(|target| (target, level)) + })) + } +} + +impl Iterator for IntoIter { + type Item = (String, LevelFilter); + + fn next(&mut self) -> Option { + self.0.next() + } + + fn size_hint(&self) -> (usize, Option) { + self.0.size_hint() + } +} + +/// A borrowing iterator over the [target]-[level] pairs of a `Targets` filter. +/// +/// This struct is created by [`iter`] method of [`Targets`], or from the `IntoIterator` +/// implementation for `&Targets`. +/// +/// [target]: tracing_core::Metadata::target +/// [level]: tracing_core::Level +/// [`iter`]: Targets::iter +#[derive(Debug)] +pub struct Iter<'a>( + FilterMap< + std::slice::Iter<'a, StaticDirective>, + fn(&'a StaticDirective) -> Option<(&'a str, LevelFilter)>, + >, +); + +impl<'a> Iter<'a> { + fn new(targets: &'a Targets) -> Self { + Self(targets.0.iter().filter_map(|directive| { + directive + .target + .as_deref() + .map(|target| (target, directive.level)) + })) + } +} + +impl<'a> Iterator for Iter<'a> { + type Item = (&'a str, LevelFilter); + + fn next(&mut self) -> Option { + self.0.next() + } + + fn size_hint(&self) -> (usize, Option) { + self.0.size_hint() + } +} + #[cfg(test)] mod tests { use super::*; - use crate::filter::directive::FilterVec; - fn expect_parse(s: &str) -> FilterVec { + fn expect_parse(s: &str) -> Targets { match dbg!(s).parse::() { Err(e) => panic!("string {:?} did not parse successfully: {}", s, e), - Ok(e) => e.0.into_vec(), + Ok(e) => e, } } fn expect_parse_ralith(s: &str) { - let dirs = expect_parse(s); + let dirs = expect_parse(s).0.into_vec(); assert_eq!(dirs.len(), 2, "\nparsed: {:#?}", dirs); assert_eq!(dirs[0].target, Some("server".to_string())); assert_eq!(dirs[0].level, LevelFilter::DEBUG); @@ -369,7 +514,7 @@ mod tests { } fn expect_parse_level_directives(s: &str) { - let dirs = expect_parse(s); + let dirs = expect_parse(s).0.into_vec(); assert_eq!(dirs.len(), 6, "\nparsed: {:#?}", dirs); assert_eq!(dirs[0].target, Some("crate3::mod2::mod1".to_string())); @@ -414,7 +559,9 @@ mod tests { #[test] fn expect_parse_valid() { - let dirs = expect_parse("crate1::mod1=error,crate1::mod2,crate2=debug,crate3=off"); + let dirs = expect_parse("crate1::mod1=error,crate1::mod2,crate2=debug,crate3=off") + .0 + .into_vec(); assert_eq!(dirs.len(), 4, "\nparsed: {:#?}", dirs); assert_eq!(dirs[0].target, Some("crate1::mod2".to_string())); assert_eq!(dirs[0].level, LevelFilter::TRACE); @@ -457,6 +604,44 @@ mod tests { ) } + #[test] + fn targets_iter() { + let filter = expect_parse("crate1::mod1=error,crate1::mod2,crate2=debug,crate3=off") + .with_default(LevelFilter::WARN); + + let mut targets: Vec<_> = filter.iter().collect(); + targets.sort(); + + assert_eq!( + targets, + vec![ + ("crate1::mod1", LevelFilter::ERROR), + ("crate1::mod2", LevelFilter::TRACE), + ("crate2", LevelFilter::DEBUG), + ("crate3", LevelFilter::OFF), + ] + ); + } + + #[test] + fn targets_into_iter() { + let filter = expect_parse("crate1::mod1=error,crate1::mod2,crate2=debug,crate3=off") + .with_default(LevelFilter::WARN); + + let mut targets: Vec<_> = filter.into_iter().collect(); + targets.sort(); + + assert_eq!( + targets, + vec![ + ("crate1::mod1".to_string(), LevelFilter::ERROR), + ("crate1::mod2".to_string(), LevelFilter::TRACE), + ("crate2".to_string(), LevelFilter::DEBUG), + ("crate3".to_string(), LevelFilter::OFF), + ] + ); + } + #[test] fn size_of_filters() { fn print_sz(s: &str) { diff --git a/tracing-subscriber/tests/subscriber_filters/targets.rs b/tracing-subscriber/tests/subscriber_filters/targets.rs index 74d8acae65..f96e9feb0d 100644 --- a/tracing-subscriber/tests/subscriber_filters/targets.rs +++ b/tracing-subscriber/tests/subscriber_filters/targets.rs @@ -33,9 +33,9 @@ fn log_events() { } #[test] -fn inner_layer_short_circuits() { +fn inner_subscriber_short_circuits() { // This test ensures that when a global filter short-circuits `Interest` - // evaluation, we aren't left with a "dirty" per-layer filter state. + // evaluation, we aren't left with a "dirty" per-subscriber filter state. let (subscriber, handle) = subscriber::mock() .event(event::msg("hello world"))