Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Filter::event_enabled #2245

Merged
merged 7 commits into from Jul 27, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 18 additions & 0 deletions tracing-subscriber/src/filter/subscriber_filters/combinator.rs
Expand Up @@ -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());
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -421,6 +431,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.
CAD97 marked this conversation as resolved.
Show resolved Hide resolved
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);
Expand Down
24 changes: 24 additions & 0 deletions tracing-subscriber/src/filter/subscriber_filters/mod.rs
Expand Up @@ -639,6 +639,22 @@ where
}
}

fn event_enabled(&self, event: &Event<'_>, cx: Context<'_, C>) -> 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.subscriber.event_enabled(event, cx)
} else {
// Otherwise, return `true`. See the comment in `enabled` for why this
// is necessary.
true
}
hawkw marked this conversation as resolved.
Show resolved Hide resolved
}

fn on_event(&self, event: &Event<'_>, cx: Context<'_, C>) {
self.did_enable(|| {
self.subscriber.on_event(event, cx.with_filter(self.id()));
Expand Down Expand Up @@ -1006,6 +1022,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
Expand Down
7 changes: 7 additions & 0 deletions tracing-subscriber/src/registry/sharded.rs
Expand Up @@ -275,6 +275,13 @@ impl Collect for Registry {

fn record_follows_from(&self, _span: &span::Id, _follows: &span::Id) {}

fn event_enabled(&self, _event: &Event<'_>) -> bool {
if self.has_per_subscriber_filters() {
return FilterState::event_enabled();
}
true
}

/// This is intentionally not implemented, as recording events
/// is the responsibility of subscribers atop of this registry.
fn event(&self, _: &Event<'_>) {}
Expand Down
12 changes: 12 additions & 0 deletions tracing-subscriber/src/subscribe/mod.rs
Expand Up @@ -1382,6 +1382,18 @@ pub trait Filter<S> {
None
}

/// Called before the filtered subscribers' [`on_event`], to determine if
/// `on_event` should be called.
///
/// This gives a chance to filter events based on their fields.
///
CAD97 marked this conversation as resolved.
Show resolved Hide resolved
/// [`on_event`]: crate::subscribe::Subscribe::on_event
#[inline] // collapse this to a constant please mrs optimizer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i like how the optimizer is, apparently, married :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:)

fn event_enabled(&self, event: &Event<'_>, cx: &Context<'_, S>) -> bool {
let _ = (event, cx);
true
}

/// Notifies this filter that a new span was constructed with the given
/// `Attributes` and `Id`.
///
Expand Down
1 change: 1 addition & 0 deletions tracing-subscriber/tests/subscriber_filters/main.rs
Expand Up @@ -3,6 +3,7 @@
mod support;
use self::support::*;
mod filter_scopes;
mod per_event;
mod targets;
mod trees;
mod vec;
Expand Down
61 changes: 61 additions & 0 deletions tracing-subscriber/tests/subscriber_filters/per_event.rs
@@ -0,0 +1,61 @@
use crate::support::*;
use tracing::Level;
use tracing_subscriber::{field::Visit, prelude::*, subscribe::Filter};

struct FilterEvent;

impl<S> Filter<S> for FilterEvent {
fn enabled(
&self,
_meta: &tracing::Metadata<'_>,
_cx: &tracing_subscriber::subscribe::Context<'_, S>,
) -> bool {
true
}

fn event_enabled(
&self,
event: &tracing::Event<'_>,
_cx: &tracing_subscriber::subscribe::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) = subscriber::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();
}