Skip to content

Commit

Permalink
Based on tokio-rs#2095
Browse files Browse the repository at this point in the history
Issue tokio-rs#1388

Allow dynamic filters to override statis cones, thus
enabling the possibility to have env filters like
warn,pageserver=info,[{tenant=98d670ab7bee6f0051494306a1ab888f}]=error,[{tenant=19cbf2bf51f42a5a5a90aa8954fb3e42}]=debug
  • Loading branch information
SomeoneToIgnore committed Jan 13, 2023
1 parent b28c935 commit a55ed7e
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 9 deletions.
27 changes: 18 additions & 9 deletions tracing-subscriber/src/filter/env/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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();
}
}

Expand Down
27 changes: 27 additions & 0 deletions tracing-subscriber/tests/env_filter/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ mod per_layer;
use tracing::{self, subscriber::with_default, Level};
use tracing_subscriber::{
filter::{EnvFilter, LevelFilter},
layer::Layered,
prelude::*,
};

Expand Down Expand Up @@ -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 {
Expand Down
27 changes: 27 additions & 0 deletions tracing-subscriber/tests/env_filter/per_layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

0 comments on commit a55ed7e

Please sign in to comment.