Add new filter methods #1973

tfreiberg-fastly · 2022-03-08T15:30:34Z

Related to #1955

Call those methods in the Layer methods for Filtered and delegate them in the filter combinators

Motivation

Currently, if a Filter is going to implement dynamic filtering, it must do this by querying the wrapped Subscriber for the current span context. Since Filter only has callsite_enabled and enabled methods, a Filter implementation cannot track the span context internally, the way a Layer can.

Unfortunately, this means that the current implementation of EnvFilter can only implement Layer (and provide global filtering). It cannot currently implement Filter, because it stores span context data internally. See #1868 for details.

Proposal

We should add on_new_span, on_enter, on_exit, and on_close methods to Filter. This would allow implementing Filters (such as EnvFilter) that internally track span states.

tracing-subscriber/src/filter/layer_filters/combinator.rs

tfreiberg-fastly · 2022-03-08T16:37:34Z

tracing-subscriber/src/filter/layer_filters/mod.rs

@@ -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());


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

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.

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!

tracing-subscriber/src/layer/mod.rs

hawkw

Thanks for working on this --- this looks great to me overall! I commented on some minor style nits, but the implementation seems correct.

It would be nice to add a bit more documentation on the new methods; perhaps we should put something explaining the general use of these methods in the top-level documentation for the Filter trait? But, that's not a blocker for merging this PR, I would be fine with going back and expanding on it later.

Once this is merged, I'll be happy to add a Filter implementation for EnvFilter using this code!

tracing-subscriber/src/filter/layer_filters/combinator.rs

tracing-subscriber/src/layer/mod.rs

tracing-subscriber/src/filter/layer_filters/combinator.rs

Co-authored-by: Eliza Weisman <[email protected]>

hawkw

okay, looks great to me --- thanks!

tracing-subscriber/src/filter/layer_filters/combinator.rs

Closes #1955 Call those methods in the `Subscribe` methods for `Filtered` and delegate them in the filter combinators ## Motivation Currently, if a `Filter` is going to implement dynamic filtering, it must do this by querying the wrapped Subscriber for the current span context. Since `Filter` only has callsite_enabled and enabled methods, a `Filter` implementation cannot track the span context internally, the way a subscriber can. Unfortunately, this means that the current implementation of `EnvFilter` can only implement `Subscribe` (and provide global filtering). It cannot currently implement `Filter`, because it stores span context data internally. See #1868 for details. ## Proposal We should add `on_new_span`, `on_enter`, `on_exit`, and `on_close` methods to `Filter`. This would allow implementing `Filter`s (such as `EnvFilter`) that internally track span states.

## Motivation Filtering by span and field requires using `EnvFilter` rather than `Targets`. Per-layer filtering requires the `Filter` trait, which `EnvFilter` does not implement. ## Solution Implement the `Filter` trait for `EnvFilter`. PR #1973 adds additiional methods to the `Filter` trait, which are necessary for `EnvFilter` to implement dynamic span filtering. Now that those methods are added, we can provide a `Filter` impl for `EnvFilter`. In addition, we changed the globally-scoped `thread_local!` macro to use a `ThreadLocal` struct as a field, so that multiple `EnvFilter`s used as per-layer filters don't share a single span scope. Fixes #1868 Follow-up on #1973 Co-authored-by: Eliza Weisman <[email protected]>

## Motivation Filtering by span and field requires using `EnvFilter` rather than `Targets`. Per-subscriber filtering requires the `Filter` trait, which `EnvFilter` does not implement. ## Solution Implement the `Filter` trait for `EnvFilter`. PR #1973 adds additiional methods to the `Filter` trait, which are necessary for `EnvFilter` to implement dynamic span filtering. Now that those methods are added, we can provide a `Filter` impl for `EnvFilter`. In addition, we changed the globally-scoped `thread_local!` macro to use a `ThreadLocal` struct as a field, so that multiple `EnvFilter`s used as per-subscriber filters don't share a single span scope. Fixes #1868 Follow-up on #1973 Co-authored-by: Eliza Weisman <[email protected]>

@TimoFreiberg

# 0.3.10 (Apr 1, 2022) This release adds several new features, including a `Filter` implementation and new builder API for `EnvFilter`, support for using a `Vec<L> where L: Layer` as a `Layer`, and a number of smaller API improvements to make working with dynamic and reloadable layers easier. ### Added - **registry**: Implement `Filter` for `EnvFilter`, allowing it to be used with per-layer filtering ([#1983]) - **registry**: `Filter::on_new_span`, `Filter::on_enter`, `Filter::on_exit`, `Filter::on_close` and `Filter::on_record` callbacks to allow `Filter`s to track span states internally ([#1973], [#2017], [#2031]) - **registry**: `Filtered::filter` and `Filtered::filter_mut` accessors ([#1959]) - **registry**: `Filtered::inner` and `Filtered::inner_mut` accessors to borrow the wrapped `Layer` ([#2034]) - **layer**: Implement `Layer` for `Vec<L: Layer>`, to allow composing together a dynamically sized list of `Layer`s ([#2027]) - **layer**: `Layer::boxed` method to make type-erasing `Layer`s easier ([#2026]) - **fmt**: `fmt::Layer::writer` and `fmt::Layer::writer_mut` accessors ([#2034]) - **fmt**: `fmt::Layer::set_ansi` method to allow changing the ANSI formatting configuration at runtime ([#2034]) - **env-filter**: `EnvFilter::builder` to configure a new `EnvFilter` prior to parsing it ([#2035]) - Several documentation fixes and improvements ([#1972], [#1971], [#2023], [#2023]) ### Fixed - **fmt**: `fmt::Layer`'s auto traits no longer depend on the `Subscriber` type parameter's auto traits ([2025]) - **env-filter**: Fixed missing help text when the `ansi` feature is disabled ([#2029]) Thanks to new contributors @TimoFreiberg and @wagenet, as well as @CAD97 for contributing to this release! [#1983]: #1983 [#1973]: #1973 [#2017]: #2017 [#2031]: #2031 [#1959]: #1959 [#2034]: #2034 [#2027]: #2027 [#2026]: #2026 [#2035]: #2035 [#1972]: #1972 [#1971]: #1971 [#2023]: #2023

@TimoFreiberg

# 0.3.10 (Apr 1, 2022) This release adds several new features, including a `Filter` implementation and new builder API for `EnvFilter`, support for using a `Vec<L> where L: Layer` as a `Layer`, and a number of smaller API improvements to make working with dynamic and reloadable layers easier. ### Added - **registry**: Implement `Filter` for `EnvFilter`, allowing it to be used with per-layer filtering ([#1983]) - **registry**: `Filter::on_new_span`, `Filter::on_enter`, `Filter::on_exit`, `Filter::on_close` and `Filter::on_record` callbacks to allow `Filter`s to track span states internally ([#1973], [#2017], [#2031]) - **registry**: `Filtered::filter` and `Filtered::filter_mut` accessors ([#1959]) - **registry**: `Filtered::inner` and `Filtered::inner_mut` accessors to borrow the wrapped `Layer` ([#2034]) - **layer**: Implement `Layer` for `Vec<L: Layer>`, to allow composing together a dynamically sized list of `Layer`s ([#2027]) - **layer**: `Layer::boxed` method to make type-erasing `Layer`s easier ([#2026]) - **fmt**: `fmt::Layer::writer` and `fmt::Layer::writer_mut` accessors ([#2034]) - **fmt**: `fmt::Layer::set_ansi` method to allow changing the ANSI formatting configuration at runtime ([#2034]) - **env-filter**: `EnvFilter::builder` to configure a new `EnvFilter` prior to parsing it ([#2035]) - Several documentation fixes and improvements ([#1972], [#1971], [#2023], [#2023]) ### Fixed - **fmt**: `fmt::Layer`'s auto traits no longer depend on the `Subscriber` type parameter's auto traits ([2025]) - **env-filter**: Fixed missing help text when the `ansi` feature is disabled ([#2029]) Thanks to new contributors @TimoFreiberg and @wagenet, as well as @CAD97 for contributing to this release! [#1983]: #1983 [#1973]: #1973 [#2017]: #2017 [#2031]: #2031 [#1959]: #1959 [#2034]: #2034 [#2027]: #2027 [#2026]: #2026 [#2035]: #2035 [#1972]: #1972 [#1971]: #1971 [#2023]: #2023

@TimoFreiberg

# 0.3.10 (Apr 1, 2022) This release adds several new features, including a `Filter` implementation and new builder API for `EnvFilter`, support for using a `Vec<L> where L: Layer` as a `Layer`, and a number of smaller API improvements to make working with dynamic and reloadable layers easier. ### Added - **registry**: Implement `Filter` for `EnvFilter`, allowing it to be used with per-layer filtering ([#1983]) - **registry**: `Filter::on_new_span`, `Filter::on_enter`, `Filter::on_exit`, `Filter::on_close` and `Filter::on_record` callbacks to allow `Filter`s to track span states internally ([#1973], [#2017], [#2031]) - **registry**: `Filtered::filter` and `Filtered::filter_mut` accessors ([#1959]) - **registry**: `Filtered::inner` and `Filtered::inner_mut` accessors to borrow the wrapped `Layer` ([#2034]) - **layer**: Implement `Layer` for `Vec<L: Layer>`, to allow composing together a dynamically sized list of `Layer`s ([#2027]) - **layer**: `Layer::boxed` method to make type-erasing `Layer`s easier ([#2026]) - **fmt**: `fmt::Layer::writer` and `fmt::Layer::writer_mut` accessors ([#2034]) - **fmt**: `fmt::Layer::set_ansi` method to allow changing the ANSI formatting configuration at runtime ([#2034]) - **env-filter**: `EnvFilter::builder` to configure a new `EnvFilter` prior to parsing it ([#2035]) - Several documentation fixes and improvements ([#1972], [#1971], [#2023], [#2023]) ### Fixed - **fmt**: `fmt::Layer`'s auto traits no longer depend on the `Subscriber` type parameter's auto traits ([2025]) - **env-filter**: Fixed missing help text when the `ansi` feature is disabled ([#2029]) Thanks to new contributors @TimoFreiberg and @wagenet, as well as @CAD97 for contributing to this release! [#1983]: #1983 [#1973]: #1973 [#2017]: #2017 [#2031]: #2031 [#1959]: #1959 [#2034]: #2034 [#2027]: #2027 [#2026]: #2026 [#2035]: #2035 [#1972]: #1972 [#1971]: #1971 [#2023]: #2023

@TimoFreiberg

# 0.3.10 (Apr 1, 2022) This release adds several new features, including a `Filter` implementation and new builder API for `EnvFilter`, support for using a `Vec<L> where L: Layer` as a `Layer`, and a number of smaller API improvements to make working with dynamic and reloadable layers easier. ### Added - **registry**: Implement `Filter` for `EnvFilter`, allowing it to be used with per-layer filtering ([#1983]) - **registry**: `Filter::on_new_span`, `Filter::on_enter`, `Filter::on_exit`, `Filter::on_close` and `Filter::on_record` callbacks to allow `Filter`s to track span states internally ([#1973], [#2017], [#2031]) - **registry**: `Filtered::filter` and `Filtered::filter_mut` accessors ([#1959]) - **registry**: `Filtered::inner` and `Filtered::inner_mut` accessors to borrow the wrapped `Layer` ([#2034]) - **layer**: Implement `Layer` for `Vec<L: Layer>`, to allow composing together a dynamically sized list of `Layer`s ([#2027]) - **layer**: `Layer::boxed` method to make type-erasing `Layer`s easier ([#2026]) - **fmt**: `fmt::Layer::writer` and `fmt::Layer::writer_mut` accessors ([#2034]) - **fmt**: `fmt::Layer::set_ansi` method to allow changing the ANSI formatting configuration at runtime ([#2034]) - **env-filter**: `EnvFilter::builder` to configure a new `EnvFilter` prior to parsing it ([#2035]) - Several documentation fixes and improvements ([#1972], [#1971], [#2023], [#2023]) ### Fixed - **fmt**: `fmt::Layer`'s auto traits no longer depend on the `Subscriber` type parameter's auto traits ([2025]) - **env-filter**: Fixed missing help text when the `ansi` feature is disabled ([#2029]) Thanks to new contributors @TimoFreiberg and @wagenet, as well as @CAD97 for contributing to this release! [#1983]: #1983 [#1973]: #1973 [#2017]: #2017 [#2031]: #2031 [#1959]: #1959 [#2034]: #2034 [#2027]: #2027 [#2026]: #2026 [#2035]: #2035 [#1972]: #1972 [#1971]: #1971 [#2023]: #2023

@TimoFreiberg

# 0.3.10 (Apr 1, 2022) This release adds several new features, including a `Filter` implementation and new builder API for `EnvFilter`, support for using a `Vec<L> where L: Layer` as a `Layer`, and a number of smaller API improvements to make working with dynamic and reloadable layers easier. ### Added - **registry**: Implement `Filter` for `EnvFilter`, allowing it to be used with per-layer filtering ([#1983]) - **registry**: `Filter::on_new_span`, `Filter::on_enter`, `Filter::on_exit`, `Filter::on_close` and `Filter::on_record` callbacks to allow `Filter`s to track span states internally ([#1973], [#2017], [#2031]) - **registry**: `Filtered::filter` and `Filtered::filter_mut` accessors ([#1959]) - **registry**: `Filtered::inner` and `Filtered::inner_mut` accessors to borrow the wrapped `Layer` ([#2034]) - **layer**: Implement `Layer` for `Vec<L: Layer>`, to allow composing together a dynamically sized list of `Layer`s ([#2027]) - **layer**: `Layer::boxed` method to make type-erasing `Layer`s easier ([#2026]) - **fmt**: `fmt::Layer::writer` and `fmt::Layer::writer_mut` accessors ([#2034]) - **fmt**: `fmt::Layer::set_ansi` method to allow changing the ANSI formatting configuration at runtime ([#2034]) - **env-filter**: `EnvFilter::builder` to configure a new `EnvFilter` prior to parsing it ([#2035]) - Several documentation fixes and improvements ([#1972], [#1971], [#2023], [#2023]) ### Fixed - **fmt**: `fmt::Layer`'s auto traits no longer depend on the `Subscriber` type parameter's auto traits ([#2025]) - **env-filter**: Fixed missing help text when the `ansi` feature is disabled ([#2029]) Thanks to new contributors @TimoFreiberg and @wagenet, as well as @CAD97 for contributing to this release! [#1983]: #1983 [#1973]: #1973 [#2017]: #2017 [#2031]: #2031 [#1959]: #1959 [#2034]: #2034 [#2027]: #2027 [#2026]: #2026 [#2035]: #2035 [#1972]: #1972 [#1971]: #1971 [#2023]: #2023 [#2025]: #2025 [#2029]: #2029

…#1973) Closes tokio-rs#1955 Call those methods in the `Layer` methods for `Filtered` and delegate them in the filter combinators ## Motivation Currently, if a `Filter` is going to implement dynamic filtering, it must do this by querying the wrapped Subscriber for the current span context. Since `Filter` only has callsite_enabled and enabled methods, a `Filter` implementation cannot track the span context internally, the way a Layer can. Unfortunately, this means that the current implementation of `EnvFilter` can only implement `Layer` (and provide global filtering). It cannot currently implement `Filter`, because it stores span context data internally. See tokio-rs#1868 for details. ## Proposal We should add `on_new_span`, `on_enter`, `on_exit`, and `on_close` methods to `Filter`. This would allow implementing `Filter`s (such as `EnvFilter`) that internally track span states.

Filtering by span and field requires using `EnvFilter` rather than `Targets`. Per-layer filtering requires the `Filter` trait, which `EnvFilter` does not implement. Implement the `Filter` trait for `EnvFilter`. PR tokio-rs#1973 adds additiional methods to the `Filter` trait, which are necessary for `EnvFilter` to implement dynamic span filtering. Now that those methods are added, we can provide a `Filter` impl for `EnvFilter`. In addition, we changed the globally-scoped `thread_local!` macro to use a `ThreadLocal` struct as a field, so that multiple `EnvFilter`s used as per-layer filters don't share a single span scope. Fixes tokio-rs#1868 Follow-up on tokio-rs#1973 Co-authored-by: Eliza Weisman <[email protected]>

@TimoFreiberg

# 0.3.10 (Apr 1, 2022) This release adds several new features, including a `Filter` implementation and new builder API for `EnvFilter`, support for using a `Vec<L> where L: Layer` as a `Layer`, and a number of smaller API improvements to make working with dynamic and reloadable layers easier. ### Added - **registry**: Implement `Filter` for `EnvFilter`, allowing it to be used with per-layer filtering ([tokio-rs#1983]) - **registry**: `Filter::on_new_span`, `Filter::on_enter`, `Filter::on_exit`, `Filter::on_close` and `Filter::on_record` callbacks to allow `Filter`s to track span states internally ([tokio-rs#1973], [tokio-rs#2017], [tokio-rs#2031]) - **registry**: `Filtered::filter` and `Filtered::filter_mut` accessors ([tokio-rs#1959]) - **registry**: `Filtered::inner` and `Filtered::inner_mut` accessors to borrow the wrapped `Layer` ([tokio-rs#2034]) - **layer**: Implement `Layer` for `Vec<L: Layer>`, to allow composing together a dynamically sized list of `Layer`s ([tokio-rs#2027]) - **layer**: `Layer::boxed` method to make type-erasing `Layer`s easier ([tokio-rs#2026]) - **fmt**: `fmt::Layer::writer` and `fmt::Layer::writer_mut` accessors ([tokio-rs#2034]) - **fmt**: `fmt::Layer::set_ansi` method to allow changing the ANSI formatting configuration at runtime ([tokio-rs#2034]) - **env-filter**: `EnvFilter::builder` to configure a new `EnvFilter` prior to parsing it ([tokio-rs#2035]) - Several documentation fixes and improvements ([tokio-rs#1972], [tokio-rs#1971], [tokio-rs#2023], [tokio-rs#2023]) ### Fixed - **fmt**: `fmt::Layer`'s auto traits no longer depend on the `Subscriber` type parameter's auto traits ([tokio-rs#2025]) - **env-filter**: Fixed missing help text when the `ansi` feature is disabled ([tokio-rs#2029]) Thanks to new contributors @TimoFreiberg and @wagenet, as well as @CAD97 for contributing to this release! [tokio-rs#1983]: tokio-rs#1983 [tokio-rs#1973]: tokio-rs#1973 [tokio-rs#2017]: tokio-rs#2017 [tokio-rs#2031]: tokio-rs#2031 [tokio-rs#1959]: tokio-rs#1959 [tokio-rs#2034]: tokio-rs#2034 [tokio-rs#2027]: tokio-rs#2027 [tokio-rs#2026]: tokio-rs#2026 [tokio-rs#2035]: tokio-rs#2035 [tokio-rs#1972]: tokio-rs#1972 [tokio-rs#1971]: tokio-rs#1971 [tokio-rs#2023]: tokio-rs#2023 [tokio-rs#2025]: tokio-rs#2025 [tokio-rs#2029]: tokio-rs#2029

Add on_{new_span, enter, exit, close} methods to Filter

e66104b

tfreiberg-fastly requested review from hawkw and a team as code owners March 8, 2022 15:30

tfreiberg-fastly added 2 commits March 8, 2022 17:30

Use new Filter methods in filter combinators

a12c96f

Call new Filter methods in impl Layer for Filtered

0510795

tfreiberg-fastly commented Mar 8, 2022

View reviewed changes

tracing-subscriber/src/filter/layer_filters/combinator.rs Show resolved Hide resolved

tfreiberg-fastly commented Mar 8, 2022

View reviewed changes

tracing-subscriber/src/filter/layer_filters/combinator.rs Show resolved Hide resolved

tfreiberg-fastly commented Mar 8, 2022

View reviewed changes

tracing-subscriber/src/layer/mod.rs Show resolved Hide resolved

tfreiberg-fastly commented Mar 8, 2022

View reviewed changes

tracing-subscriber/src/layer/mod.rs Outdated Show resolved Hide resolved

hawkw approved these changes Mar 8, 2022

View reviewed changes

tfreiberg-fastly and others added 10 commits March 8, 2022 19:24

Update tracing-subscriber/src/filter/layer_filters/combinator.rs

7b104a1

Co-authored-by: Eliza Weisman <[email protected]>

Update tracing-subscriber/src/filter/layer_filters/combinator.rs

8f26227

Co-authored-by: Eliza Weisman <[email protected]>

Update tracing-subscriber/src/layer/mod.rs

e45da8f

Co-authored-by: Eliza Weisman <[email protected]>

Update tracing-subscriber/src/layer/mod.rs

e8f10d9

Co-authored-by: Eliza Weisman <[email protected]>

Update tracing-subscriber/src/layer/mod.rs

c3da622

Co-authored-by: Eliza Weisman <[email protected]>

Update tracing-subscriber/src/layer/mod.rs

b1aad56

Co-authored-by: Eliza Weisman <[email protected]>

Update tracing-subscriber/src/filter/layer_filters/combinator.rs

9242b5d

Co-authored-by: Eliza Weisman <[email protected]>

Merge branch 'v0.1.x' into 1955-new-filter-methods

ed10ba3

Import instead of fully qualify

6371aa0

Remove underscores from trait method arg names

7d35fc5

hawkw approved these changes Mar 8, 2022

View reviewed changes

tracing-subscriber/src/filter/layer_filters/combinator.rs Outdated Show resolved Hide resolved

Update tracing-subscriber/src/filter/layer_filters/combinator.rs

a162473

hawkw enabled auto-merge (squash) March 8, 2022 20:38

hawkw merged commit 2d27332 into tokio-rs:v0.1.x Mar 8, 2022

tfreiberg-fastly deleted the 1955-new-filter-methods branch March 9, 2022 10:55

tfreiberg-fastly mentioned this pull request Mar 10, 2022

Impl filter for EnvFilter #1983

Merged

davidbarsky mentioned this pull request Mar 16, 2022

chore: port filters to master #1796

Closed

hawkw mentioned this pull request Apr 1, 2022

subscriber: prepare to release v0.3.10 #2041

Merged

davidbarsky mentioned this pull request Sep 26, 2023

chore: backport roughly a year's worth of changes #2728

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new filter methods #1973

Add new filter methods #1973

tfreiberg-fastly commented Mar 8, 2022 •

edited

Loading

tfreiberg-fastly Mar 8, 2022 •

edited

Loading

hawkw Mar 8, 2022

tfreiberg-fastly Mar 8, 2022

hawkw left a comment

hawkw left a comment

Add new filter methods #1973

Add new filter methods #1973

Conversation

tfreiberg-fastly commented Mar 8, 2022 • edited Loading

Motivation

Proposal

tfreiberg-fastly Mar 8, 2022 • edited Loading

Choose a reason for hiding this comment

hawkw Mar 8, 2022

Choose a reason for hiding this comment

tfreiberg-fastly Mar 8, 2022

Choose a reason for hiding this comment

hawkw left a comment

Choose a reason for hiding this comment

hawkw left a comment

Choose a reason for hiding this comment

tfreiberg-fastly commented Mar 8, 2022 •

edited

Loading

tfreiberg-fastly Mar 8, 2022 •

edited

Loading