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.

Sign up for GitHub

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 new filter methods #1973

Merged
merged 14 commits into from
Mar 8, 2022
Merged

Add new filter methods #1973

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
e66104b
Add on_{new_span, enter, exit, close} methods to Filter
tfreiberg-fastly Mar 8, 2022
a12c96f
Use new Filter methods in filter combinators
tfreiberg-fastly Mar 8, 2022
0510795
Call new Filter methods in impl Layer for Filtered
tfreiberg-fastly Mar 8, 2022
7b104a1
Update tracing-subscriber/src/filter/layer_filters/combinator.rs
tfreiberg-fastly Mar 8, 2022
8f26227
Update tracing-subscriber/src/filter/layer_filters/combinator.rs
tfreiberg-fastly Mar 8, 2022
e45da8f
Update tracing-subscriber/src/layer/mod.rs
tfreiberg-fastly Mar 8, 2022
e8f10d9
Update tracing-subscriber/src/layer/mod.rs
tfreiberg-fastly Mar 8, 2022
c3da622
Update tracing-subscriber/src/layer/mod.rs
tfreiberg-fastly Mar 8, 2022
b1aad56
Update tracing-subscriber/src/layer/mod.rs
tfreiberg-fastly Mar 8, 2022
9242b5d
Update tracing-subscriber/src/filter/layer_filters/combinator.rs
tfreiberg-fastly Mar 8, 2022
ed10ba3
Merge branch 'v0.1.x' into 1955-new-filter-methods
hawkw Mar 8, 2022
6371aa0
Import instead of fully qualify
tfreiberg-fastly Mar 8, 2022
7d35fc5
Remove underscores from trait method arg names
tfreiberg-fastly Mar 8, 2022
a162473
Update tracing-subscriber/src/filter/layer_filters/combinator.rs
hawkw Mar 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 72 additions & 1 deletion tracing-subscriber/src/filter/layer_filters/combinator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
//! Filter combinators
use crate::layer::{Context, Filter};
use std::{cmp, fmt, marker::PhantomData};
use tracing_core::{subscriber::Interest, LevelFilter, Metadata};
use tracing_core::{
span::{Attributes, Id},
subscriber::Interest,
LevelFilter, Metadata,
};

/// Combines two [`Filter`]s so that spans and events are enabled if and only if
/// *both* filters return `true`.
Expand Down Expand Up @@ -132,6 +136,30 @@ where
// If either hint is `None`, return `None`. Otherwise, return the most restrictive.
cmp::min(self.a.max_level_hint(), self.b.max_level_hint())
}

#[inline]
fn on_new_span(&self, attrs: &Attributes<'_>, id: &Id, ctx: Context<'_, S>) {
self.a.on_new_span(attrs, id, ctx.clone());
tfreiberg-fastly marked this conversation as resolved.
Show resolved Hide resolved
self.b.on_new_span(attrs, id, ctx)
}

#[inline]
fn on_enter(&self, id: &Id, ctx: Context<'_, S>) {
self.a.on_enter(id, ctx.clone());
self.b.on_enter(id, ctx);
}

#[inline]
fn on_exit(&self, id: &Id, ctx: Context<'_, S>) {
self.a.on_exit(id, ctx.clone());
self.b.on_exit(id, ctx);
}

#[inline]
fn on_close(&self, id: Id, ctx: Context<'_, S>) {
self.a.on_close(id.clone(), ctx.clone());
self.b.on_close(id, ctx);
}
}

impl<A, B, S> Clone for And<A, B, S>
Expand Down Expand Up @@ -289,6 +317,29 @@ where
// If either hint is `None`, return `None`. Otherwise, return the less restrictive.
Some(cmp::max(self.a.max_level_hint()?, self.b.max_level_hint()?))
}
#[inline]
fn on_new_span(&self, attrs: &Attributes<'_>, id: &Id, ctx: Context<'_, S>) {
self.a.on_new_span(attrs, id, ctx.clone());
tfreiberg-fastly marked this conversation as resolved.
Show resolved Hide resolved
self.b.on_new_span(attrs, id, ctx)
}

#[inline]
fn on_enter(&self, id: &Id, ctx: Context<'_, S>) {
self.a.on_enter(id, ctx.clone());
self.b.on_enter(id, ctx);
}

#[inline]
fn on_exit(&self, id: &Id, ctx: Context<'_, S>) {
self.a.on_exit(id, ctx.clone());
self.b.on_exit(id, ctx);
}

#[inline]
fn on_close(&self, id: Id, ctx: Context<'_, S>) {
self.a.on_close(id.clone(), ctx.clone());
self.b.on_close(id, ctx);
}
}

impl<A, B, S> Clone for Or<A, B, S>
Expand Down Expand Up @@ -356,6 +407,26 @@ where
// TODO(eliza): figure this out???
None
}

#[inline]
fn on_new_span(&self, attrs: &Attributes<'_>, id: &Id, ctx: Context<'_, S>) {
self.a.on_new_span(attrs, id, ctx);
}

#[inline]
fn on_enter(&self, id: &Id, ctx: Context<'_, S>) {
self.a.on_enter(id, ctx);
}

#[inline]
fn on_exit(&self, id: &Id, ctx: Context<'_, S>) {
self.a.on_exit(id, ctx);
}

#[inline]
fn on_close(&self, id: Id, ctx: Context<'_, S>) {
self.a.on_close(id, ctx);
}
}

impl<A, S> Clone for Not<A, S>
Expand Down
13 changes: 9 additions & 4 deletions tracing-subscriber/src/filter/layer_filters/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,9 @@ where

fn on_new_span(&self, attrs: &span::Attributes<'_>, id: &span::Id, cx: Context<'_, S>) {
self.did_enable(|| {
self.layer.on_new_span(attrs, id, cx.with_filter(self.id()));
let cx = cx.with_filter(self.id());
self.filter.on_new_span(attrs, id, cx.clone());
Copy link
Contributor Author

@tfreiberg-fastly tfreiberg-fastly Mar 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I mostly guessed here: should the filter.on_new_span method be called with cx.with_filter or just with cx? I don't really understand these details here.

Also, I guessed that it might be more correct to first update the filters and then the layer, not sure if that can make a difference or of the filter should be called last.
The question about the order of filter and layer calls applies to all four changes in this file

Copy link
Member

Choose a reason for hiding this comment

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

I mostly guessed here: should the filter.on_new_span method be called with cx.with_filter or just with cx? I don't really understand these details here.

This is a good question. The difference is that calling cx.with_filter(...) means that the context will filter out any spans from the current span context that were previously disabled by that filter. I think it makes sense for the filter to not see any spans that it previously filtered out. So, passing the with_filter context to the filter is probably the right thing.

Also, I guessed that it might be more correct to first update the filters and then the layer, not sure if that can make a difference or of the filter should be called last. The question about the order of filter and layer calls applies to all four changes in this file

I don't think it actually makes a difference which order the methods are called in. The filter can only enable/disable things in its enabled method, not in on_$WHATEVER, so calling the on_$WHATEVER method is only going to effect the next enabled call. It won't have an effect on what the inner layer sees in its on_$WHATEVER method. So, we can actually call them in whatever order and it shouldn't make a difference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it makes sense for the filter to not see any spans that it previously filtered out.
that sounds to me like it might depend on what Interest that filter would return for that span? Very low confidence though 😅

Good to know about the order though. Thanks for the explanations!

self.layer.on_new_span(attrs, id, cx);
})
}

Expand Down Expand Up @@ -610,19 +612,22 @@ where

fn on_enter(&self, id: &span::Id, cx: Context<'_, S>) {
if let Some(cx) = cx.if_enabled_for(id, self.id()) {
self.layer.on_enter(id, cx)
self.filter.on_enter(id, cx.clone());
self.layer.on_enter(id, cx);
}
}

fn on_exit(&self, id: &span::Id, cx: Context<'_, S>) {
if let Some(cx) = cx.if_enabled_for(id, self.id()) {
self.layer.on_exit(id, cx)
self.filter.on_enter(id, cx.clone());
self.layer.on_exit(id, cx);
}
}

fn on_close(&self, id: span::Id, cx: Context<'_, S>) {
if let Some(cx) = cx.if_enabled_for(&id, self.id()) {
self.layer.on_close(id, cx)
self.filter.on_close(id.clone(), cx.clone());
self.layer.on_close(id, cx);
}
}

Expand Down
36 changes: 35 additions & 1 deletion tracing-subscriber/src/layer/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@
//! .json()
//! .with_writer(Arc::new(file));
//! Some(json_log)
//! } else {
//! } else {
//! None
//! };
//!
Expand Down Expand Up @@ -1054,6 +1054,40 @@ feature! {
fn max_level_hint(&self) -> Option<LevelFilter> {
None
}

/// Notifies this filter that a new span was constructed with the given
hawkw marked this conversation as resolved.
Show resolved Hide resolved
/// `Attributes` and `Id`.
tfreiberg-fastly marked this conversation as resolved.
Show resolved Hide resolved
///
/// By default, this method does nothing. `Filter` implementations that
/// need to be notified when new spans are created can override this
/// method.
fn on_new_span(&self, attrs: &span::Attributes<'_>, id: &span::Id, ctx: Context<'_, S>) {
let _ = (attrs, id, ctx);
}

/// Notifies this filter that a span with the given ID was entered.
tfreiberg-fastly marked this conversation as resolved.
Show resolved Hide resolved
///
/// By default, this method does nothing. `Filter` implementations that
/// need to be notified when a span is entered can override this method.
fn on_enter(&self, id: &span::Id, ctx: Context<'_, S>) {
let _ = (id, ctx);
}

/// Notifies this filter that a span with the given ID was exited.
///
/// By default, this method does nothing. `Filter` implementations that
/// need to be notified when a span is exited can override this method.
fn on_exit(&self, id: &span::Id, ctx: Context<'_, S>) {
let _ = (id, ctx);
}

/// Notifies this filter that a span with the given ID has been closed.
///
/// By default, this method does nothing. `Filter` implementations that
/// need to be notified when a span is closed can override this method.
fn on_close(&self, id: span::Id, ctx: Context<'_, S>) {
let _ = (id, ctx);
}
}
}

Expand Down