diff --git a/tracing-subscriber/src/filter/layer_filters/combinator.rs b/tracing-subscriber/src/filter/layer_filters/combinator.rs index e79de20870..669ddf63a7 100644 --- a/tracing-subscriber/src/filter/layer_filters/combinator.rs +++ b/tracing-subscriber/src/filter/layer_filters/combinator.rs @@ -137,6 +137,11 @@ where cmp::min(self.a.max_level_hint(), self.b.max_level_hint()) } + #[inline] + fn event_enabled(&self, event: &tracing_core::Event<'_>, cx: &Context<'_, S>) -> bool { + self.a.event_enabled(event, cx) && self.b.event_enabled(event, cx) + } + #[inline] fn on_new_span(&self, attrs: &Attributes<'_>, id: &Id, ctx: Context<'_, S>) { self.a.on_new_span(attrs, id, ctx.clone()); @@ -324,6 +329,11 @@ where Some(cmp::max(self.a.max_level_hint()?, self.b.max_level_hint()?)) } + #[inline] + fn event_enabled(&self, event: &tracing_core::Event<'_>, cx: &Context<'_, S>) -> bool { + self.a.event_enabled(event, cx) || self.b.event_enabled(event, cx) + } + #[inline] fn on_new_span(&self, attrs: &Attributes<'_>, id: &Id, ctx: Context<'_, S>) { self.a.on_new_span(attrs, id, ctx.clone()); @@ -393,7 +403,16 @@ where /// If the wrapped filter would enable a span or event, it will be disabled. If /// it would disable a span or event, that span or event will be enabled. /// + /// This inverts the values returned by the [`enabled`] and [`callsite_enabled`] + /// methods on the wrapped filter; it does *not* invert [`event_enabled`], as + /// implementing that method is optional, and filters which do not implement + /// filtering on event field values will return `true` even for events that their + /// [`enabled`] method would disable. + /// /// [`Filter`]: crate::layer::Filter + /// [`enabled`]: crate::layer::Filter::enabled + /// [`event_enabled`]: crate::layer::Filter::event_enabled + /// [`callsite_enabled`]: crate::layer::Filter::callsite_enabled pub(crate) fn new(a: A) -> Self { Self { a, _s: PhantomData } } @@ -421,6 +440,14 @@ where None } + #[inline] + fn event_enabled(&self, event: &tracing_core::Event<'_>, cx: &Context<'_, S>) -> bool { + // Never disable based on event_enabled; we "disabled" it in `enabled`, + // so the `not` has already been applied and filtered this not out. + let _ = (event, cx); + true + } + #[inline] fn on_new_span(&self, attrs: &Attributes<'_>, id: &Id, ctx: Context<'_, S>) { self.a.on_new_span(attrs, id, ctx); diff --git a/tracing-subscriber/src/filter/layer_filters/mod.rs b/tracing-subscriber/src/filter/layer_filters/mod.rs index 8949cfb5a8..fbfa55a443 100644 --- a/tracing-subscriber/src/filter/layer_filters/mod.rs +++ b/tracing-subscriber/src/filter/layer_filters/mod.rs @@ -298,6 +298,17 @@ pub trait FilterExt: layer::Filter { /// Inverts `self`, returning a filter that enables spans and events only if /// `self` would *not* enable them. + /// + /// This inverts the values returned by the [`enabled`] and [`callsite_enabled`] + /// methods on the wrapped filter; it does *not* invert [`event_enabled`], as + /// implementing that method is optional, and filters which do not implement + /// filtering on event field values will return `true` even for events that their + /// [`enabled`] method would disable. + /// + /// [`Filter`]: crate::subscribe::Filter + /// [`enabled`]: crate::subscribe::Filter::enabled + /// [`event_enabled`]: crate::subscribe::Filter::event_enabled + /// [`callsite_enabled`]: crate::subscribe::Filter::callsite_enabled fn not(self) -> combinator::Not where Self: Sized, @@ -643,6 +654,22 @@ where } } + fn event_enabled(&self, event: &Event<'_>, cx: Context<'_, S>) -> bool { + let cx = cx.with_filter(self.id()); + let enabled = FILTERING + .with(|filtering| filtering.and(self.id(), || self.filter.event_enabled(event, &cx))); + + if enabled { + // If the filter enabled this event, ask the wrapped subscriber if + // _it_ wants it --- it might have a global filter. + self.layer.event_enabled(event, cx) + } else { + // Otherwise, return `true`. See the comment in `enabled` for why this + // is necessary. + true + } + } + fn on_event(&self, event: &Event<'_>, cx: Context<'_, S>) { self.did_enable(|| { self.layer.on_event(event, cx.with_filter(self.id())); @@ -1014,6 +1041,14 @@ impl FilterState { } } + /// Run a second filtering pass, e.g. for Subscribe::event_enabled. + fn and(&self, filter: FilterId, f: impl FnOnce() -> bool) -> bool { + let map = self.enabled.get(); + let enabled = map.is_enabled(filter) && f(); + self.enabled.set(map.set(filter, enabled)); + enabled + } + /// Clears the current in-progress filter state. /// /// This resets the [`FilterMap`] and current [`Interest`] as well as diff --git a/tracing-subscriber/src/layer/mod.rs b/tracing-subscriber/src/layer/mod.rs index 5122559ce5..753b6cb249 100644 --- a/tracing-subscriber/src/layer/mod.rs +++ b/tracing-subscriber/src/layer/mod.rs @@ -1352,6 +1352,26 @@ feature! { Interest::sometimes() } + /// Called before the filtered [`Layer]'s [`on_event`], to determine if + /// `on_event` should be called. + /// + /// This gives a chance to filter events based on their fields. Note, + /// however, that this *does not* override [`enabled`], and is not even + /// called if [`enabled`] returns `false`. + /// + /// ## Default Implementation + /// + /// By default, this method returns `true`, indicating that no events are + /// filtered out based on their fields. + /// + /// [`enabled`]: crate::layer::Filter::enabled + /// [`on_event`]: crate::layer::Layer::on_event + #[inline] // collapse this to a constant please mrs optimizer + fn event_enabled(&self, event: &Event<'_>, cx: &Context<'_, S>) -> bool { + let _ = (event, cx); + true + } + /// Returns an optional hint of the highest [verbosity level][level] that /// this `Filter` will enable. /// diff --git a/tracing-subscriber/src/registry/sharded.rs b/tracing-subscriber/src/registry/sharded.rs index b81d5fef83..7978997678 100644 --- a/tracing-subscriber/src/registry/sharded.rs +++ b/tracing-subscriber/src/registry/sharded.rs @@ -275,6 +275,13 @@ impl Subscriber for Registry { fn record_follows_from(&self, _span: &span::Id, _follows: &span::Id) {} + fn event_enabled(&self, _event: &Event<'_>) -> bool { + if self.has_per_layer_filters() { + return FilterState::event_enabled(); + } + true + } + /// This is intentionally not implemented, as recording events /// is the responsibility of layers atop of this registry. fn event(&self, _: &Event<'_>) {} diff --git a/tracing-subscriber/tests/layer_filters/main.rs b/tracing-subscriber/tests/layer_filters/main.rs index 0233f063b9..10f06c24c6 100644 --- a/tracing-subscriber/tests/layer_filters/main.rs +++ b/tracing-subscriber/tests/layer_filters/main.rs @@ -5,6 +5,7 @@ use self::support::*; mod boxed; mod downcast_raw; mod filter_scopes; +mod per_event; mod targets; mod trees; mod vec; diff --git a/tracing-subscriber/tests/layer_filters/per_event.rs b/tracing-subscriber/tests/layer_filters/per_event.rs new file mode 100644 index 0000000000..9c785f9a23 --- /dev/null +++ b/tracing-subscriber/tests/layer_filters/per_event.rs @@ -0,0 +1,61 @@ +use crate::support::*; +use tracing::Level; +use tracing_subscriber::{field::Visit, layer::Filter, prelude::*}; + +struct FilterEvent; + +impl Filter for FilterEvent { + fn enabled( + &self, + _meta: &tracing::Metadata<'_>, + _cx: &tracing_subscriber::layer::Context<'_, S>, + ) -> bool { + true + } + + fn event_enabled( + &self, + event: &tracing::Event<'_>, + _cx: &tracing_subscriber::layer::Context<'_, S>, + ) -> bool { + struct ShouldEnable(bool); + impl Visit for ShouldEnable { + fn record_bool(&mut self, field: &tracing_core::Field, value: bool) { + if field.name() == "enable" { + self.0 = value; + } + } + + fn record_debug( + &mut self, + _field: &tracing_core::Field, + _value: &dyn core::fmt::Debug, + ) { + } + } + let mut should_enable = ShouldEnable(false); + event.record(&mut should_enable); + should_enable.0 + } +} + +#[test] +fn per_subscriber_event_field_filtering() { + let (expect, handle) = layer::mock() + .event(event::mock().at_level(Level::TRACE)) + .event(event::mock().at_level(Level::INFO)) + .done() + .run_with_handle(); + + let _subscriber = tracing_subscriber::registry() + .with(expect.with_filter(FilterEvent)) + .set_default(); + + tracing::trace!(enable = true, "hello trace"); + tracing::debug!("hello debug"); + tracing::info!(enable = true, "hello info"); + tracing::warn!(enable = false, "hello warn"); + tracing::error!("hello error"); + + handle.assert_finished(); +}