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

pass through max_level_hint in reload::Layer #2204

Merged
merged 1 commit into from Jul 1, 2022

Conversation

guswynn
Copy link
Contributor

@guswynn guswynn commented Jul 1, 2022

Motivation

When using a reload layer, the fast-path current level check doesn't work, as the max_level_hint is just None, which rebuild_interest interprets as TRACE

Solution

Pass through to the underlying layer/filter. On poisons, when already panicking, just return None

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.

looks good to me!

Comment on lines +150 to +152
assert_eq!(LevelFilter::current(), LevelFilter::INFO);
handle.reload(Filter::Two).expect("should reload");
assert_eq!(LevelFilter::current(), LevelFilter::DEBUG);
Copy link
Member

Choose a reason for hiding this comment

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

slight pref for having a separate test for max level hinting, but 🤷‍♀️ i don't care that much

@hawkw hawkw merged commit 2aa0cb0 into tokio-rs:master Jul 1, 2022
kmahar pushed a commit to kmahar/tracing that referenced this pull request Jul 7, 2022
## Motivation

When using a `reload` layer, the fast-path current level check doesn't
work, as the `max_level_hint` is just `None`, which `rebuild_interest`
interprets as `TRACE`

## Solution

Pass through to the underlying layer/filter. On poisons, when already
panicking, just return `None`
hawkw pushed a commit that referenced this pull request Jul 20, 2022
## Motivation

When using a `reload` layer, the fast-path current level check doesn't
work, as the `max_level_hint` is just `None`, which `rebuild_interest`
interprets as `TRACE`

## Solution

Pass through to the underlying layer/filter. On poisons, when already
panicking, just return `None`
hawkw pushed a commit that referenced this pull request Jul 20, 2022
## Motivation

When using a `reload` layer, the fast-path current level check doesn't
work, as the `max_level_hint` is just `None`, which `rebuild_interest`
interprets as `TRACE`

## Solution

Pass through to the underlying layer/filter. On poisons, when already
panicking, just return `None`
hawkw pushed a commit that referenced this pull request Jul 20, 2022
## Motivation

When using a `reload` layer, the fast-path current level check doesn't
work, as the `max_level_hint` is just `None`, which `rebuild_interest`
interprets as `TRACE`

## Solution

Pass through to the underlying layer/filter. On poisons, when already
panicking, just return `None`
hawkw added a commit that referenced this pull request Jul 20, 2022
# 0.3.15 (Jul 20, 2022)

This release fixes a bug where the `reload` layer would fail to pass
through  `max_level_hint` to the underlying layer, potentially breaking
filtering.

### Fixed

- **reload**: pass through `max_level_hint` to the inner `Layer`
  ([#2204])

Thanks to @guswynn for contributing to this release!

[#2204]: #2204
hawkw added a commit that referenced this pull request Jul 20, 2022
# 0.3.15 (Jul 20, 2022)

This release fixes a bug where the `reload` layer would fail to pass
through  `max_level_hint` to the underlying layer, potentially breaking
filtering.

### Fixed

- **reload**: pass through `max_level_hint` to the inner `Layer`
  ([#2204])

Thanks to @guswynn for contributing to this release!

[#2204]: #2204
@guswynn guswynn deleted the reload-hint branch September 21, 2022 20:06
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