diff --git a/tracing-core/src/callsite.rs b/tracing-core/src/callsite.rs index 9c6e6b31ad..f6904befda 100644 --- a/tracing-core/src/callsite.rs +++ b/tracing-core/src/callsite.rs @@ -28,13 +28,21 @@ impl Registry { fn rebuild_callsite_interest(&self, callsite: &'static dyn Callsite) { let meta = callsite.metadata(); - let mut interest = Interest::never(); - - for registrar in &self.dispatchers { - if let Some(sub_interest) = registrar.try_register(meta) { - interest = interest.and(sub_interest); - } - } + // Iterate over the subscribers in the registry, and — if they are + // active — register the callsite with them. + let mut interests = self + .dispatchers + .iter() + .filter_map(|registrar| registrar.try_register(meta)); + + // Use the first subscriber's `Interest` as the base value. + let interest = if let Some(interest) = interests.next() { + // Combine all remaining `Interest`s. + interests.fold(interest, Interest::and) + } else { + // If nobody was interested in this thing, just return `never`. + Interest::never() + }; callsite.set_interest(interest) } diff --git a/tracing-core/src/subscriber.rs b/tracing-core/src/subscriber.rs index 2c8911d90f..2b56f8d483 100644 --- a/tracing-core/src/subscriber.rs +++ b/tracing-core/src/subscriber.rs @@ -556,24 +556,15 @@ impl Interest { /// Returns the common interest between these two Interests. /// - /// The common interest is defined as the least restrictive, so if one - /// interest is `never` and the other is `always` the common interest is - /// `always`. + /// If both interests are the same, this propagates that interest. + /// Otherwise, if they differ, the result must always be + /// `Interest::sometimes` --- if the two subscribers differ in opinion, we + /// will have to ask the current subscriber what it thinks, no matter what. pub(crate) fn and(self, rhs: Interest) -> Self { - match rhs.0 { - // If the added interest is `never()`, don't change anything — - // either a different subscriber added a higher interest, which we - // want to preserve, or the interest is 0 anyway (as it's - // initialized to 0). - InterestKind::Never => self, - // If the interest is `sometimes()`, that overwrites a `never()` - // interest, but doesn't downgrade an `always()` interest. - InterestKind::Sometimes if self.0 == InterestKind::Never => rhs, - // If the interest is `always()`, we overwrite the current interest, - // as always() is the highest interest level and should take - // precedent. - InterestKind::Always => rhs, - _ => self, + if self.0 == rhs.0 { + self + } else { + Interest::sometimes() } } } diff --git a/tracing-subscriber/src/filter/env/mod.rs b/tracing-subscriber/src/filter/env/mod.rs index 2e7d5e821b..92a4665992 100644 --- a/tracing-subscriber/src/filter/env/mod.rs +++ b/tracing-subscriber/src/filter/env/mod.rs @@ -297,6 +297,19 @@ impl Layer for EnvFilter { // if not, we can avoid the thread local access + iterating over the // spans in the current scope. if self.has_dynamics && self.dynamics.max_level >= *level { + if metadata.is_span() { + // If the metadata is a span, see if we care about its callsite. + let enabled_by_cs = self + .by_cs + .read() + .ok() + .map(|by_cs| by_cs.contains_key(&metadata.callsite())) + .unwrap_or(false); + if enabled_by_cs { + return true; + } + } + let enabled_by_scope = SCOPE.with(|scope| { for filter in scope.borrow().iter() { if filter >= level { @@ -314,9 +327,6 @@ impl Layer for EnvFilter { if self.statics.max_level >= *level { // Otherwise, fall back to checking if the callsite is // statically enabled. - // TODO(eliza): we *might* want to check this only if the `log` - // feature is enabled, since if this is a `tracing` event with a - // real callsite, it would already have been statically enabled... return self.statics.enabled(metadata); } diff --git a/tracing/tests/filters_dont_leak.rs b/tracing/tests/filters_dont_leak.rs new file mode 100644 index 0000000000..5f8f559717 --- /dev/null +++ b/tracing/tests/filters_dont_leak.rs @@ -0,0 +1,80 @@ +#![cfg(feature = "std")] + +mod support; +use self::support::*; + +#[test] +fn spans_dont_leak() { + fn do_span() { + let span = tracing::debug_span!("alice"); + let _e = span.enter(); + } + + let (subscriber, handle) = subscriber::mock() + .named("spans/subscriber1") + .with_filter(|_| false) + .done() + .run_with_handle(); + + let _guard = tracing::subscriber::set_default(subscriber); + + do_span(); + + let alice = span::mock().named("alice"); + let (subscriber2, handle2) = subscriber::mock() + .named("spans/subscriber2") + .with_filter(|_| true) + .new_span(alice.clone()) + .enter(alice.clone()) + .exit(alice.clone()) + .drop_span(alice) + .done() + .run_with_handle(); + + tracing::subscriber::with_default(subscriber2, || { + println!("--- subscriber 2 is default ---"); + do_span() + }); + + println!("--- subscriber 1 is default ---"); + do_span(); + + handle.assert_finished(); + handle2.assert_finished(); +} + +#[test] +fn events_dont_leak() { + fn do_event() { + tracing::debug!("alice"); + } + + let (subscriber, handle) = subscriber::mock() + .named("events/subscriber1") + .with_filter(|_| false) + .done() + .run_with_handle(); + + let _guard = tracing::subscriber::set_default(subscriber); + + do_event(); + + let (subscriber2, handle2) = subscriber::mock() + .named("events/subscriber2") + .with_filter(|_| true) + .event(event::mock()) + .done() + .run_with_handle(); + + tracing::subscriber::with_default(subscriber2, || { + println!("--- subscriber 2 is default ---"); + do_event() + }); + + println!("--- subscriber 1 is default ---"); + + do_event(); + + handle.assert_finished(); + handle2.assert_finished(); +}