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

Jump to bottom

Add Filter::event_enabled #2245

Merged
merged 7 commits into from Jul 27, 2022
Merged

Add Filter::event_enabled #2245

merged 7 commits into from Jul 27, 2022

Conversation

CAD97
Copy link
Contributor

@CAD97 CAD97 commented Jul 26, 2022

Motivation

Like for Collect and Subscribe, allow per-layer Filters to filter based on event fields.

Solution

Add Filter::event_enabled, plumb it through the combinator implementations, and call it from Filtered.

The bit I'm the least confident about is the check in Registry's implementation, but I think it matches what event is doing and everything seems to function correctly.

@CAD97 CAD97 requested review from hawkw, davidbarsky and a team as code owners July 26, 2022 04:29
@CAD97
Copy link
Contributor Author

CAD97 commented Jul 26, 2022

Btw, GitHub Copilot threw some shade while working on this:

image

hawkw
hawkw reviewed Jul 26, 2022
View reviewed changes
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

overall, this looks good! mostly, i commented on a couple things that we ought to add to the docs; beyond that, my other comments were just style suggestions that you can feel free to ignore.

tracing-subscriber/src/filter/subscriber_filters/combinator.rs Show resolved Hide resolved
tracing-subscriber/src/subscribe/mod.rs
/// This gives a chance to filter events based on their fields.
///
/// [`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.

:)

tracing-subscriber/src/filter/subscriber_filters/mod.rs Outdated Show resolved Hide resolved
tracing-subscriber/src/filter/subscriber_filters/mod.rs Show resolved Hide resolved
tracing-subscriber/src/subscribe/mod.rs Show resolved Hide resolved
CAD97 and others added 2 commits July 26, 2022 21:56
hawkw
hawkw approved these changes Jul 27, 2022
View reviewed changes
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

Happy to merge this as soon as the docs changes are addressed --- let me know when that's done?

@CAD97
Copy link
Contributor Author

CAD97 commented Jul 27, 2022

That should be docs changes addressed

@hawkw modify labels -S-waiting-on-author +S-waiting-on-reviewer 😜

hawkw
hawkw approved these changes Jul 27, 2022
View reviewed changes
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

corrected one slight docs inaccuracy, going to go ahead and merge now! thanks!

@hawkw hawkw enabled auto-merge (squash) July 27, 2022 19:05
@hawkw hawkw merged commit 5706844 into tokio-rs:master Jul 27, 2022
@CAD97 CAD97 deleted the filter-event-enabled branch July 27, 2022 19:51
hawkw pushed a commit that referenced this pull request Jul 29, 2022
@CAD97 @hawkw
subscriber: add Filter::event_enabled (#2245)
f0663e8 
## Motivation

Like for `Subscriber` and `Layer`, allow per-layer `Filter`s to filter
based on event fields.

## Solution

Add `Filter::event_enabled`, plumb it through the combinator
implementations, and call it from `Filtered`.

The bit I'm the least confident about is the check in `Registry`'s
implementation, but I *think* it matches what `event` is doing and
everything seems to function correctly.
@hawkw hawkw mentioned this pull request Jul 29, 2022
Merged
hawkw pushed a commit that referenced this pull request Jul 29, 2022
@CAD97 @hawkw
subscriber: add Filter::event_enabled (#2245)
ffe4eb4 
## Motivation

Like for `Subscriber` and `Layer`, allow per-layer `Filter`s to filter
based on event fields.

## Solution

Add `Filter::event_enabled`, plumb it through the combinator
implementations, and call it from `Filtered`.

The bit I'm the least confident about is the check in `Registry`'s
implementation, but I *think* it matches what `event` is doing and
everything seems to function correctly.
hawkw pushed a commit that referenced this pull request Jul 29, 2022
@CAD97 @hawkw
subscriber: add Filter::event_enabled (#2245)
09da422 
## Motivation

Like for `Subscriber` and `Layer`, allow per-layer `Filter`s to filter
based on event fields.

## Solution

Add `Filter::event_enabled`, plumb it through the combinator
implementations, and call it from `Filtered`.

The bit I'm the least confident about is the check in `Registry`'s
implementation, but I *think* it matches what `event` is doing and
everything seems to function correctly.
hawkw added a commit that referenced this pull request Oct 6, 2022
@hawkw
subscriber: prepare to release v0.3.16
a92c013 
# 0.3.16 (October 6, 2022)

This release of `tracing-subscriber` fixes a regression introduced in
[v0.3.15][subscriber-0.3.15] where `Option::None`'s `Layer`
implementation would set the max level hint to `OFF`. In addition, it
adds several new APIs, including the `Filter::event_enabled` method for
filtering events based on fields values, and the ability to log internal
errors that occur when writing a log line.

This release also replaces the dependency on the unmaintained
[`ansi-term`] crate with the [`nu-ansi-term`] crate, resolving an
*informational* security advisory ([RUSTSEC-2021-0139] for
[`ansi-term`]'s maintainance status. This increases the minimum
supported Rust version (MSRV) to Rust 1.50+, although the crate should
still compile for the previous MSRV of Rust 1.49+ when the `ansi`
feature is not enabled.

### Fixed

- **layer**: `Option::None`'s `Layer` impl always setting the
  `max_level_hint` to `LevelFilter::OFF` (#2321)
- Compilation with `-Z minimal versions` (#2246)
- **env-filter**: Clarify that disabled level warnings are emitted by
  `tracing-subscriber` (#2285)

### Added

- **fmt**: Log internal errors to `stderr` if writing a log line fails
  (#2102)
- **fmt**: `FmtLayer::log_internal_errors` and
  `FmtSubscriber::log_internal_errors` methods for configuring whether
  internal writer errors are printed to `stderr` (#2102)
- **fmt**: `#[must_use]` attributes on builders to warn if a
  `Subscriber` is configured but not set as the default subscriber
  (#2239)
- **filter**: `Filter::event_enabled` method for filtering an event
  based on its fields (#2245, #2251)
- **filter**: `Targets::default_level` accessor (#2242)

### Changed

- **ansi**: Replaced dependency on unmaintained `ansi-term` crate with
  `nu-ansi-term` ((#2287, fixes informational advisory
  [RUSTSEC-2021-0139])
- `tracing-core`: updated to [0.1.30][core-0.1.30]
- Minimum Supported Rust Version (MSRV) increased to Rust 1.50+ (when
  the `ansi`) feature flag is enabled (#2287)

### Documented

- **fmt**: Correct inaccuracies in `fmt::init` documentation (#2224)
- **filter**: Fix incorrect doc link in `filter::Not` combinator
  (#2249)

Thanks to new contributors @cgbur, @DesmondWillowbrook, @RalfJung, and
@poliorcetics, as well as returning contributors @CAD97, @connec,
@jswrenn, @guswynn, and @bryangarza, for contributing to this release!

[nu-ansi-term]: https://github.com/nushell/nu-ansi-term
[ansi_term]: https://github.com/ogham/rust-ansi-term
[RUSTSEC-2021-0139]: https://rustsec.org/advisories/RUSTSEC-2021-0139.html
[core-0.1.30]: https://github.com/tokio-rs/tracing/releases/tag/tracing-core-0.1.30
[subscriber-0.3.15]: https://github.com/tokio-rs/tracing/releases/tag/tracing-subscriber-0.3.15
@hawkw hawkw mentioned this pull request Oct 6, 2022
Merged
hawkw added a commit that referenced this pull request Oct 6, 2022
@hawkw
subscriber: prepare to release v0.3.16
d1f401a 
# 0.3.16 (October 6, 2022)

This release of `tracing-subscriber` fixes a regression introduced in
[v0.3.15][subscriber-0.3.15] where `Option::None`'s `Layer`
implementation would set the max level hint to `OFF`. In addition, it
adds several new APIs, including the `Filter::event_enabled` method for
filtering events based on fields values, and the ability to log internal
errors that occur when writing a log line.

This release also replaces the dependency on the unmaintained
[`ansi-term`] crate with the [`nu-ansi-term`] crate, resolving an
*informational* security advisory ([RUSTSEC-2021-0139] for
[`ansi-term`]'s maintainance status. This increases the minimum
supported Rust version (MSRV) to Rust 1.50+, although the crate should
still compile for the previous MSRV of Rust 1.49+ when the `ansi`
feature is not enabled.

### Fixed

- **layer**: `Option::None`'s `Layer` impl always setting the
  `max_level_hint` to `LevelFilter::OFF` (#2321)
- Compilation with `-Z minimal versions` (#2246)
- **env-filter**: Clarify that disabled level warnings are emitted by
  `tracing-subscriber` (#2285)

### Added

- **fmt**: Log internal errors to `stderr` if writing a log line fails
  (#2102)
- **fmt**: `FmtLayer::log_internal_errors` and
  `FmtSubscriber::log_internal_errors` methods for configuring whether
  internal writer errors are printed to `stderr` (#2102)
- **fmt**: `#[must_use]` attributes on builders to warn if a
  `Subscriber` is configured but not set as the default subscriber
  (#2239)
- **filter**: `Filter::event_enabled` method for filtering an event
  based on its fields (#2245, #2251)
- **filter**: `Targets::default_level` accessor (#2242)

### Changed

- **ansi**: Replaced dependency on unmaintained `ansi-term` crate with
  `nu-ansi-term` ((#2287, fixes informational advisory
  [RUSTSEC-2021-0139])
- `tracing-core`: updated to [0.1.30][core-0.1.30]
- Minimum Supported Rust Version (MSRV) increased to Rust 1.50+ (when
  the `ansi`) feature flag is enabled (#2287)

### Documented

- **fmt**: Correct inaccuracies in `fmt::init` documentation (#2224)
- **filter**: Fix incorrect doc link in `filter::Not` combinator
  (#2249)

Thanks to new contributors @cgbur, @DesmondWillowbrook, @RalfJung, and
@poliorcetics, as well as returning contributors @CAD97, @connec,
@jswrenn, @guswynn, and @bryangarza, for contributing to this release!

[nu-ansi-term]: https://github.com/nushell/nu-ansi-term
[ansi_term]: https://github.com/ogham/rust-ansi-term
[RUSTSEC-2021-0139]: https://rustsec.org/advisories/RUSTSEC-2021-0139.html
[core-0.1.30]: https://github.com/tokio-rs/tracing/releases/tag/tracing-core-0.1.30
[subscriber-0.3.15]: https://github.com/tokio-rs/tracing/releases/tag/tracing-subscriber-0.3.15
hawkw added a commit that referenced this pull request Oct 6, 2022
@hawkw
subscriber: prepare to release v0.3.16 (#2342)
b28c935 
# 0.3.16 (October 6, 2022)

This release of `tracing-subscriber` fixes a regression introduced in
[v0.3.15][subscriber-0.3.15] where `Option::None`'s `Layer`
implementation would set the max level hint to `OFF`. In addition, it
adds several new APIs, including the `Filter::event_enabled` method for
filtering events based on fields values, and the ability to log internal
errors that occur when writing a log line.

This release also replaces the dependency on the unmaintained
[`ansi-term`] crate with the [`nu-ansi-term`] crate, resolving an
*informational* security advisory ([RUSTSEC-2021-0139] for
[`ansi-term`]'s maintainance status. This increases the minimum
supported Rust version (MSRV) to Rust 1.50+, although the crate should
still compile for the previous MSRV of Rust 1.49+ when the `ansi`
feature is not enabled.

### Fixed

- **layer**: `Option::None`'s `Layer` impl always setting the
  `max_level_hint` to `LevelFilter::OFF` (#2321)
- Compilation with `-Z minimal versions` (#2246)
- **env-filter**: Clarify that disabled level warnings are emitted by
  `tracing-subscriber` (#2285)

### Added

- **fmt**: Log internal errors to `stderr` if writing a log line fails
  (#2102)
- **fmt**: `FmtLayer::log_internal_errors` and
  `FmtSubscriber::log_internal_errors` methods for configuring whether
  internal writer errors are printed to `stderr` (#2102)
- **fmt**: `#[must_use]` attributes on builders to warn if a
  `Subscriber` is configured but not set as the default subscriber
  (#2239)
- **filter**: `Filter::event_enabled` method for filtering an event
  based on its fields (#2245, #2251)
- **filter**: `Targets::default_level` accessor (#2242)

### Changed

- **ansi**: Replaced dependency on unmaintained `ansi-term` crate with
  `nu-ansi-term` ((#2287, fixes informational advisory
  [RUSTSEC-2021-0139])
- `tracing-core`: updated to [0.1.30][core-0.1.30]
- Minimum Supported Rust Version (MSRV) increased to Rust 1.50+ (when
  the `ansi`) feature flag is enabled (#2287)

### Documented

- **fmt**: Correct inaccuracies in `fmt::init` documentation (#2224)
- **filter**: Fix incorrect doc link in `filter::Not` combinator
  (#2249)

Thanks to new contributors @cgbur, @DesmondWillowbrook, @RalfJung, and
@poliorcetics, as well as returning contributors @CAD97, @connec,
@jswrenn, @guswynn, and @bryangarza, for contributing to this release!

[nu-ansi-term]: https://github.com/nushell/nu-ansi-term
[ansi_term]: https://github.com/ogham/rust-ansi-term
[RUSTSEC-2021-0139]: https://rustsec.org/advisories/RUSTSEC-2021-0139.html
[core-0.1.30]: https://github.com/tokio-rs/tracing/releases/tag/tracing-core-0.1.30
[subscriber-0.3.15]: https://github.com/tokio-rs/tracing/releases/tag/tracing-subscriber-0.3.15
@davidbarsky davidbarsky mentioned this pull request Sep 26, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hawkw hawkw hawkw approved these changes

@davidbarsky davidbarsky Awaiting requested review from davidbarsky davidbarsky is a code owner

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
2 participants
@CAD97 @hawkw