diff --git a/tracing-subscriber/src/filter/env/mod.rs b/tracing-subscriber/src/filter/env/mod.rs index 81a9ae2bde..4ed9adca9e 100644 --- a/tracing-subscriber/src/filter/env/mod.rs +++ b/tracing-subscriber/src/filter/env/mod.rs @@ -481,7 +481,7 @@ impl EnvFilter { // is it possible for a dynamic filter directive to enable this event? // if not, we can avoid the thread loca'l access + iterating over the // spans in the current scope. - if self.has_dynamics && self.dynamics.max_level >= *level { + if self.has_dynamics { if metadata.is_span() { // If the metadata is a span, see if we care about its callsite. let enabled_by_cs = self @@ -502,6 +502,10 @@ impl EnvFilter { return true; } } + if !scope.is_empty() { + // at least one dynamic filter disabled this + return false; + } false }; if enabled_by_scope { @@ -619,14 +623,19 @@ impl EnvFilter { } fn register_callsite(&self, metadata: &'static Metadata<'static>) -> Interest { - if self.has_dynamics && metadata.is_span() { - // If this metadata describes a span, first, check if there is a - // dynamic filter that should be constructed for it. If so, it - // should always be enabled, since it influences filtering. - if let Some(matcher) = self.dynamics.matcher(metadata) { - let mut by_cs = try_lock!(self.by_cs.write(), else return self.base_interest()); - by_cs.insert(metadata.callsite(), matcher); - return Interest::always(); + if self.has_dynamics { + if metadata.is_span() { + // If this metadata describes a span, first, check if there is a + // dynamic filter that should be constructed for it. If so, it + // should always be enabled, since it influences filtering. + if let Some(matcher) = self.dynamics.matcher(metadata) { + let mut by_cs = try_lock!(self.by_cs.write(), else return self.base_interest()); + by_cs.insert(metadata.callsite(), matcher); + return Interest::always(); + } + } else if *metadata.level() <= self.dynamics.max_level { + // Any dynamic could turn this metadata off + return Interest::sometimes(); } } diff --git a/tracing-subscriber/tests/env_filter/main.rs b/tracing-subscriber/tests/env_filter/main.rs index 3c3d4868be..c82d0bd83a 100644 --- a/tracing-subscriber/tests/env_filter/main.rs +++ b/tracing-subscriber/tests/env_filter/main.rs @@ -9,6 +9,7 @@ mod per_layer; use tracing::{self, subscriber::with_default, Level}; use tracing_subscriber::{ filter::{EnvFilter, LevelFilter}, + layer::Layered, prelude::*, }; @@ -239,6 +240,32 @@ fn method_name_resolution() { filter.max_level_hint(); } +#[test] +fn more_specific_dynamic_directives_override_static_directives() { + let filter: EnvFilter = "info,my_target[my_span]=warn".parse().unwrap(); + let (subscriber, handle) = subscriber::mock() + .enter(span::mock().at_level(Level::INFO)) + .event( + event::mock() + .at_level(Level::WARN) + .in_scope(vec![span::named("my_span")]), + ) + .exit(span::mock().at_level(Level::INFO)) + .done() + .run_with_handle(); + + let subscriber: Layered<_, _> = subscriber.with(filter); + + with_default(subscriber, || { + // tracing::info!("should be logged"); + let _info = tracing::info_span!(target: "my_target", "my_span").entered(); + tracing::info!("should be ignored"); + tracing::warn!("should be logged"); + }); + + handle.assert_finished(); +} + // contains the same tests as the first half of this file // but using EnvFilter as a `Filter`, not as a `Layer` mod per_layer_filter { diff --git a/tracing-subscriber/tests/env_filter/per_layer.rs b/tracing-subscriber/tests/env_filter/per_layer.rs index 8bf5698a4d..c1758eabbf 100644 --- a/tracing-subscriber/tests/env_filter/per_layer.rs +++ b/tracing-subscriber/tests/env_filter/per_layer.rs @@ -303,3 +303,30 @@ fn multiple_dynamic_filters() { handle1.assert_finished(); handle2.assert_finished(); } + +#[test] +fn more_specific_dynamic_directives_override_static_directives() { + let filter: EnvFilter = "info,my_target[my_span]=warn".parse().unwrap(); + let (layer, handle) = layer::mock() + .enter(span::mock().at_level(Level::INFO)) + .event( + event::mock() + .at_level(Level::WARN) + .in_scope(vec![span::named("my_span")]), + ) + .exit(span::mock().at_level(Level::INFO)) + .done() + .run_with_handle(); + + let _subscriber = tracing_subscriber::registry() + .with(layer.with_filter(filter)) + .set_default(); + + { + let _info = tracing::info_span!(target: "my_target", "my_span").entered(); + tracing::info!(target: "my_target", "should be ignored"); + tracing::warn!(target: "my_target", "should be logged"); + } + + handle.assert_finished(); +}