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

Jump to bottom

forward-port Filter impl for EnvFilter #2031

Merged
merged 3 commits into from Mar 29, 2022
Merged

forward-port Filter impl for EnvFilter #2031

merged 3 commits into from Mar 29, 2022

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Mar 29, 2022

This ports the following changes from v0.1.x to master:

@hawkw
subscriber: fix Filtered::on_exit calling on_enter by mistake (#2018
70fcfce 
)

## Motivation

Currently, the `Filtered` type's `Subscribe` impl accidentally calls the
inner `Filter`'s `on_enter` hook in the `on_exit` hook. This
is...obviously wrong, lol, and means that filters like `EnvFilter` will
never consider spans to have exited, breaking filtering behavior.

@tfreiberg-fastly originally noticed this bug (see
#1983 (comment)),
but I wanted to make the change separately from PR #1983 so that we can
fix it on the main branch without waiting for #1983 to merge first.

## Solution

This branch, uh, you know... fixes that.
@hawkw
subscriber: add Filter::on_record callback
7e4724d 
Currently, `Filter` only has the `on_new_span`, `on_enter`, `on_exit`,
and `on_close` callbacks. This means it won't handle cases where a
span records a new field value that changes its filter state.

This branch adds the missing `on_record` callback.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@hawkw hawkw requested review from davidbarsky and a team as code owners March 29, 2022 21:46
@hawkw hawkw enabled auto-merge (rebase) March 29, 2022 21:46
@hawkw hawkw force-pushed the eliza/forward-port-envfilter branch 2 times, most recently from 5ccadf1 to 9b7bab1 Compare March 29, 2022 21:48
davidbarsky
davidbarsky reviewed Mar 29, 2022
View reviewed changes
tracing-subscriber/src/filter/env/mod.rs Outdated
/// use tracing_subscriber::{EnvFilter, fmt, prelude::*};
///
/// tracing_subscriber::registry()
/// .with(fmt::layer())
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// .with(fmt::layer())
/// .with(fmt::subscriber())

tracing-subscriber/src/filter/env/mod.rs Outdated Show resolved Hide resolved
tracing-subscriber/src/filter/env/mod.rs Outdated Show resolved Hide resolved
tracing-subscriber/tests/env_filter/per_subscriber.rs Outdated Show resolved Hide resolved
tracing-subscriber/tests/env_filter/per_subscriber.rs Outdated Show resolved Hide resolved
tracing-subscriber/tests/env_filter/per_subscriber.rs Outdated Show resolved Hide resolved
@tfreiberg-fastly @hawkw
subscriber: impl Filter for EnvFilter (#1983)
d0a73e2 
## 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 <eliza@buoyant.io>
@hawkw hawkw force-pushed the eliza/forward-port-envfilter branch from 9b7bab1 to d0a73e2 Compare March 29, 2022 22:06
@hawkw hawkw requested a review from davidbarsky March 29, 2022 22:11
@hawkw hawkw merged commit f58019e into master Mar 29, 2022
@hawkw hawkw deleted the eliza/forward-port-envfilter branch March 29, 2022 22:24
hawkw added a commit that referenced this pull request Apr 1, 2022
@hawkw
subscriber: prepare to release v0.3.10
2a8eab8 
# 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
@hawkw hawkw mentioned this pull request Apr 1, 2022
Merged
hawkw added a commit that referenced this pull request Apr 1, 2022
@hawkw
subscriber: prepare to release v0.3.10
2963995 
# 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
hawkw added a commit that referenced this pull request Apr 1, 2022
@hawkw
subscriber: prepare to release v0.3.10
ec96350 
# 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
hawkw added a commit that referenced this pull request Apr 1, 2022
@hawkw
subscriber: prepare to release v0.3.10
c14a60d 
# 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
hawkw added a commit that referenced this pull request Apr 1, 2022
@hawkw
subscriber: prepare to release v0.3.10 (#2041)
42888dc 
# 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidbarsky davidbarsky davidbarsky approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@hawkw @davidbarsky @tfreiberg-fastly