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

subscriber: add missing Filter::on_record for to EnvFilter #2058

Merged
merged 4 commits into from Apr 8, 2022

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Apr 8, 2022

Depends on #2057

Motivation

Currently, EnvFilter's Subscribe implementation provides a
Subscribe::on_record method, but its Filter implementation is
missing the corresponding Filter::on_record implementation. This means
that when using EnvFilter as a per-subscriber filter, recording span
fields after the spans were created will not update the filter state.

Solution

This commit factors out the on_record implementation for Subscribe
into an inherent method, and adds a new Filter::on_record method that
calls it as well.

## Motivation

Currently, there is a potential namespace resolution issue when calling
`EnvFilter` methods that have the same name in the `Filter` and
`Subscribe` traits (such as `enabled` and `max_level_hint`). When both
`Filter` and `Subscribe` are in scope, the method resolution is
ambiguous.

See #1983 (comment)

## Solution

This commit solves the problem by making the inherent method versions of
these methods public. When the traits are in scope, name resolution will
always select the inherent method prefer`entially, preventing the name
clash.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Depends on #2057

## Motivation

Currently, `EnvFilter`'s `Subscribe` implementation provides a
`Subscribe::on_record` method, but its `Filter` implementation is
missing the corresponding `Filter::on_record` implementation. This means
that when using `EnvFilter` as a per-subscriber filter, recording span
fields after the spans were created will not update the filter state.

## Solution

This commit factors out the `on_record` implementation for `Subscribe`
into an inherent method, and adds a new `Filter::on_record` method that
calls it as well.
@hawkw hawkw requested review from davidbarsky and a team as code owners April 8, 2022 16:32
Base automatically changed from eliza/inherent-enabled to master April 8, 2022 17:05
@hawkw hawkw merged commit 43f1f73 into master Apr 8, 2022
@hawkw hawkw deleted the eliza/envfilter-on-record branch April 8, 2022 17:32
hawkw added a commit that referenced this pull request Apr 8, 2022
Depends on #2057

## Motivation

Currently, `EnvFilter`'s `Layer` implementation provides a
`Layer::on_record` method, but its `Filter` implementation is
missing the corresponding `Filter::on_record` implementation. This means
that when using `EnvFilter` as a per-layer filter, recording span
fields after the spans were created will not update the filter state.

## Solution

This commit factors out the `on_record` implementation for `Layer`
into an inherent method, and adds a new `Filter::on_record` method that
calls it as well.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@hawkw hawkw mentioned this pull request Apr 8, 2022
hawkw added a commit that referenced this pull request Apr 9, 2022
Depends on #2057

## Motivation

Currently, `EnvFilter`'s `Layer` implementation provides a
`Layer::on_record` method, but its `Filter` implementation is
missing the corresponding `Filter::on_record` implementation. This means
that when using `EnvFilter` as a per-layer filter, recording span
fields after the spans were created will not update the filter state.

## Solution

This commit factors out the `on_record` implementation for `Layer`
into an inherent method, and adds a new `Filter::on_record` method that
calls it as well.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Apr 9, 2022
# 0.3.11 (Apr 9, 2022)

This is a bugfix release for the `Filter` implementation for `EnvFilter` added
in [v0.3.10].

### Fixed

- **env-filter**: Added missing `Filter::on_record` callback to `EnvFilter`'s
  `Filter` impl ([#2058])
- **env-filter**: Fixed method resolution issues when calling `EnvFilter`
  methods with both the `Filter` and `Layer` traits in scope ([#2057])
- **env-filter**: Fixed `EnvFilter::builder().parse()` and other parsing methods
  returning an error when parsing an empty string ([#2052])

Thanks to new contributor @Ma124 for contributing to this release!

[v0.3.10]: https://github.com/tokio-rs/tracing/releases/tag/tracing-subscriber-0.3.10
[#2058]: #2058
[#2057]: #2057
[#2052]: #2052
hawkw added a commit that referenced this pull request Apr 9, 2022
# 0.3.11 (Apr 9, 2022)

This is a bugfix release for the `Filter` implementation for `EnvFilter` added
in [v0.3.10].

### Fixed

- **env-filter**: Added missing `Filter::on_record` callback to `EnvFilter`'s
  `Filter` impl ([#2058])
- **env-filter**: Fixed method resolution issues when calling `EnvFilter`
  methods with both the `Filter` and `Layer` traits in scope ([#2057])
- **env-filter**: Fixed `EnvFilter::builder().parse()` and other parsing methods
  returning an error when parsing an empty string ([#2052])

Thanks to new contributor @Ma124 for contributing to this release!

[v0.3.10]: https://github.com/tokio-rs/tracing/releases/tag/tracing-subscriber-0.3.10
[#2058]: #2058
[#2057]: #2057
[#2052]: #2052
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants