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 inherent impls for EnvFilter methods #2057

Merged
merged 4 commits into from Apr 8, 2022

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Apr 8, 2022

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.

## 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.
@hawkw hawkw requested review from davidbarsky and a team as code owners April 8, 2022 00:13
@hawkw
Copy link
Member Author

hawkw commented Apr 8, 2022

bleh, i broke the docs. hang on...

@hawkw hawkw enabled auto-merge (squash) April 8, 2022 16:29
hawkw added a commit that referenced this pull request 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.
@hawkw hawkw merged commit de364fb into master Apr 8, 2022
@hawkw hawkw deleted the eliza/inherent-enabled branch April 8, 2022 17:05
hawkw added a commit that referenced this pull request 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.

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

Currently, there is a potential namespace resolution issue when calling
`EnvFilter` methods that have the same name in the `Filter` and
`Layer` traits (such as `enabled` and `max_level_hint`). When both
`Filter` and `Layer` 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>
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
## Motivation

Currently, there is a potential namespace resolution issue when calling
`EnvFilter` methods that have the same name in the `Filter` and
`Layer` traits (such as `enabled` and `max_level_hint`). When both
`Filter` and `Layer` 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>
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