Skip to content

Commit

Permalink
subscriber: add Targets::targets method to iterate directives (#1574)
Browse files Browse the repository at this point in the history
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<String>, impl Into<LevelFilter>)`.

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.
  • Loading branch information
connec authored and hawkw committed Mar 24, 2022
1 parent 1dac314 commit e212166
Show file tree
Hide file tree
Showing 4 changed files with 208 additions and 10 deletions.
13 changes: 13 additions & 0 deletions tracing-subscriber/src/filter/directive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,19 @@ impl<T: Match + Ord> Extend<T> for DirectiveSet<T> {
}
}

impl<T> IntoIterator for DirectiveSet<T> {
type Item = T;

#[cfg(feature = "smallvec")]
type IntoIter = smallvec::IntoIter<[T; 8]>;
#[cfg(not(feature = "smallvec"))]
type IntoIter = std::vec::IntoIter<T>;

fn into_iter(self) -> Self::IntoIter {
self.directives.into_iter()
}
}

// === impl Statics ===

impl DirectiveSet<StaticDirective> {
Expand Down
2 changes: 1 addition & 1 deletion tracing-subscriber/src/filter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down
199 changes: 192 additions & 7 deletions tracing-subscriber/src/filter/targets.rs
Original file line number Diff line number Diff line change
@@ -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},
Expand All @@ -6,7 +14,7 @@ use crate::{
subscribe,
};
use std::{
iter::{Extend, FromIterator},
iter::{Extend, FilterMap, FromIterator},
str::FromStr,
};
use tracing_core::{Collect, Interest, Metadata};
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -344,20 +380,129 @@ impl<C> subscribe::Filter<C> 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<
<DirectiveSet<StaticDirective> 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::Item> {
self.0.next()
}

fn size_hint(&self) -> (usize, Option<usize>) {
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::Item> {
self.0.next()
}

fn size_hint(&self) -> (usize, Option<usize>) {
self.0.size_hint()
}
}

#[cfg(test)]
mod tests {
use super::*;
use crate::filter::directive::FilterVec;

fn expect_parse(s: &str) -> FilterVec<StaticDirective> {
fn expect_parse(s: &str) -> Targets {
match dbg!(s).parse::<Targets>() {
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);
Expand All @@ -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()));
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions tracing-subscriber/tests/subscriber_filters/targets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down

0 comments on commit e212166

Please sign in to comment.