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

subscriber: clear per-layer interest when short circuiting #1569

Merged
merged 5 commits into from
Sep 16, 2021
Merged

subscriber: clear per-layer interest when short circuiting #1569

merged 5 commits into from
Sep 16, 2021

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Sep 15, 2021

Currently, when evaluating register_callsite for a stack containing
per-layer filters, the intermediate Interest from combining the per
layer filters' Interests is stored in the thread-local FilterState.
When all per-layer filters have been evaluated, we reach the Registry,
which clears the FilterState and bubbles the Interest back up.
However, when a global filter in the stack returns Interest::never,
we short-circuit, and don't reach the Registry. This means the
Interest state is not cleared.

This branch adds code in Layered to ensure the per-layer filter state
is cleared when a global filter short circuits Interest evaluation.

This fixes #1563.

@hawkw
add repro for #1563
3537afb 
Signed-off-by: Eliza Weisman <[email protected]>
@hawkw
minimize test a bit
2949cc2 
Signed-off-by: Eliza Weisman <[email protected]>
@hawkw
add a test to catch the *actual* bug
1033190 
This test repros the bug the assertion was *supposed* to prevent.

Signed-off-by: Eliza Weisman <[email protected]>
@hawkw
clear interest when a global filter short circuits
ba0b424 
Signed-off-by: Eliza Weisman <[email protected]>
@hawkw hawkw requested a review from a team September 15, 2021 21:00
@hawkw hawkw merged commit 3b8e680 into v0.1.x Sep 16, 2021
@hawkw hawkw deleted the eliza/fix-interest-panic branch September 16, 2021 02:45
hawkw added a commit that referenced this pull request Sep 16, 2021
@hawkw
subscriber: prepare to release v0.2.23
c9f8af6 
# 0.2.23 (September 16, 2021)

This release fixes a few bugs in the per-layer filtering API added in
v0.2.21.

### Fixed

- **env-filter**: Fixed excessive `EnvFilter` memory use ([#1568])
- **filter**: Fixed a panic that may occur in debug mode when using
  per-layer filters together with global filters ([#1569])
- Fixed incorrect documentation formatting ([#1572])

[#1568]: #1568
[#1569]: #1569
[#1572]: #1572
@hawkw hawkw mentioned this pull request Sep 16, 2021
Merged
hawkw added a commit that referenced this pull request Sep 16, 2021
@hawkw
subscriber: prepare to release v0.2.23 (#1573)
a792aa8 
# 0.2.23 (September 16, 2021)

This release fixes a few bugs in the per-layer filtering API added in
v0.2.21.

### Fixed

- **env-filter**: Fixed excessive `EnvFilter` memory use ([#1568])
- **filter**: Fixed a panic that may occur in debug mode when using
  per-layer filters together with global filters ([#1569])
- Fixed incorrect documentation formatting ([#1572])

[#1568]: #1568
[#1569]: #1569
[#1572]: #1572
hawkw added a commit that referenced this pull request Sep 17, 2021
@hawkw
subscriber: clear enabled filter map when short circuiting
9ff04fd 
This is essentially the same change as #1569, but for `enabled` states
rather than `register_callsite`. When a global filter returns `false`
from `enabled`, ensure that the per-layer filter `FilterMap` and debug
counters are cleared, so that they are empty on the next `enabled` call.

See #1563
@hawkw hawkw mentioned this pull request Sep 17, 2021
Merged
hawkw added a commit that referenced this pull request Sep 17, 2021
@hawkw
subscriber: clear enabled filter map when short circuiting
1202a86 
This is essentially the same change as #1569, but for `enabled` states
rather than `register_callsite`. When a global filter returns `false`
from `enabled`, ensure that the per-layer filter `FilterMap` and debug
counters are cleared, so that they are empty on the next `enabled` call.

See #1563
hawkw added a commit that referenced this pull request Sep 19, 2021
@hawkw
subscriber: clear enabled filter map when short circuiting (#1575)
b09b368 
This is essentially the same change as #1569, but for `enabled` states
rather than `register_callsite`. When a global filter returns `false`
from `enabled`, ensure that the per-layer filter `FilterMap` and debug
counters are cleared, so that they are empty on the next `enabled` call.

See #1563
davidbarsky pushed a commit that referenced this pull request Nov 29, 2021
@hawkw @davidbarsky
subscriber: clear per-layer interest when short circuiting (#1569)
133be44 
Currently, when evaluating `register_callsite` for a stack containing
per-layer filters, the intermediate `Interest` from combining the per
layer filters' `Interest`s is stored in the thread-local `FilterState`.
When all per-layer filters have been evaluated, we reach the `Registry`,
which clears the `FilterState` and bubbles the `Interest` back up.
However, when a _global_ filter in the stack returns `Interest::never`,
we short-circuit, and don't reach the `Registry`. This means the
`Interest` state is not cleared.

This branch adds code in `Layered` to ensure the per-layer filter state
is cleared when a global filter short circuits `Interest` evaluation.

This fixes #1563.

Signed-off-by: Eliza Weisman <[email protected]>
davidbarsky pushed a commit that referenced this pull request Nov 29, 2021
@hawkw @davidbarsky
subscriber: clear enabled filter map when short circuiting (#1575)
320a0f4 
This is essentially the same change as #1569, but for `enabled` states
rather than `register_callsite`. When a global filter returns `false`
from `enabled`, ensure that the per-layer filter `FilterMap` and debug
counters are cleared, so that they are empty on the next `enabled` call.

See #1563
davidbarsky pushed a commit that referenced this pull request Mar 18, 2022
@hawkw @davidbarsky
subscriber: clear per-layer interest when short circuiting (#1569)
c26ae06 
Currently, when evaluating `register_callsite` for a stack containing
per-layer filters, the intermediate `Interest` from combining the per
layer filters' `Interest`s is stored in the thread-local `FilterState`.
When all per-layer filters have been evaluated, we reach the `Registry`,
which clears the `FilterState` and bubbles the `Interest` back up.
However, when a _global_ filter in the stack returns `Interest::never`,
we short-circuit, and don't reach the `Registry`. This means the
`Interest` state is not cleared.

This branch adds code in `Layered` to ensure the per-layer filter state
is cleared when a global filter short circuits `Interest` evaluation.

This fixes #1563.

Signed-off-by: Eliza Weisman <[email protected]>
davidbarsky pushed a commit that referenced this pull request Mar 18, 2022
@hawkw @davidbarsky
subscriber: clear enabled filter map when short circuiting (#1575)
4a25297 
This is essentially the same change as #1569, but for `enabled` states
rather than `register_callsite`. When a global filter returns `false`
from `enabled`, ensure that the per-layer filter `FilterMap` and debug
counters are cleared, so that they are empty on the next `enabled` call.

See #1563
hawkw added a commit that referenced this pull request Mar 23, 2022
@hawkw
subscriber: clear per-layer interest when short circuiting (#1569)
10f7747 
Currently, when evaluating `register_callsite` for a stack containing
per-layer filters, the intermediate `Interest` from combining the per
layer filters' `Interest`s is stored in the thread-local `FilterState`.
When all per-layer filters have been evaluated, we reach the `Registry`,
which clears the `FilterState` and bubbles the `Interest` back up.
However, when a _global_ filter in the stack returns `Interest::never`,
we short-circuit, and don't reach the `Registry`. This means the
`Interest` state is not cleared.

This branch adds code in `Layered` to ensure the per-layer filter state
is cleared when a global filter short circuits `Interest` evaluation.

This fixes #1563.

Signed-off-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Mar 23, 2022
@hawkw
subscriber: clear enabled filter map when short circuiting (#1575)
0428412 
This is essentially the same change as #1569, but for `enabled` states
rather than `register_callsite`. When a global filter returns `false`
from `enabled`, ensure that the per-layer filter `FilterMap` and debug
counters are cleared, so that they are empty on the next `enabled` call.

See #1563
hawkw added a commit that referenced this pull request Mar 23, 2022
@hawkw
subscriber: clear per-subscriber interest when short circuiting (#1569)
9f1e803 
Currently, when evaluating `register_callsite` for a stack containing
per-subscriber filters, the intermediate `Interest` from combining the
per-subscriber filters' `Interest`s is stored in the thread-local
`FilterState`. When all per-subscriber filters have been evaluated, we
reach the `Registry`, which clears the `FilterState` and bubbles the
`Interest` back up. However, when a _global_ filter in the stack returns
`Interest::never`, we short-circuit, and don't reach the `Registry`.
This means the `Interest` state is not cleared.

This branch adds code in `Layered` to ensure the per-subscriber filter
state is cleared when a global filter short circuits `Interest`
evaluation.

This fixes #1563.

Signed-off-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Mar 23, 2022
@hawkw
subscriber: clear enabled filter map when short circuiting (#1575)
667d365 
This is essentially the same change as #1569, but for `enabled` states
rather than `register_callsite`. When a global filter returns `false`
from `enabled`, ensure that the per-subscriber filter `FilterMap` and
debug counters are cleared, so that they are empty on the next `enabled`
call.

See #1563
hawkw added a commit that referenced this pull request Mar 23, 2022
@hawkw
subscriber: clear per-subscriber interest when short circuiting (#1569)
ea667ab 
Currently, when evaluating `register_callsite` for a stack containing
per-subscriber filters, the intermediate `Interest` from combining the
per-subscriber filters' `Interest`s is stored in the thread-local
`FilterState`. When all per-subscriber filters have been evaluated, we
reach the `Registry`, which clears the `FilterState` and bubbles the
`Interest` back up. However, when a _global_ filter in the stack returns
`Interest::never`, we short-circuit, and don't reach the `Registry`.
This means the `Interest` state is not cleared.

This branch adds code in `Layered` to ensure the per-subscriber filter
state is cleared when a global filter short circuits `Interest`
evaluation.

This fixes #1563.

Signed-off-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Mar 23, 2022
@hawkw
subscriber: clear enabled filter map when short circuiting (#1575)
db72031 
This is essentially the same change as #1569, but for `enabled` states
rather than `register_callsite`. When a global filter returns `false`
from `enabled`, ensure that the per-subscriber filter `FilterMap` and
debug counters are cleared, so that they are empty on the next `enabled`
call.

See #1563
hawkw added a commit that referenced this pull request Mar 24, 2022
@hawkw
subscriber: clear per-subscriber interest when short circuiting (#1569)
7f3763b 
Currently, when evaluating `register_callsite` for a stack containing
per-subscriber filters, the intermediate `Interest` from combining the
per-subscriber filters' `Interest`s is stored in the thread-local
`FilterState`. When all per-subscriber filters have been evaluated, we
reach the `Registry`, which clears the `FilterState` and bubbles the
`Interest` back up. However, when a _global_ filter in the stack returns
`Interest::never`, we short-circuit, and don't reach the `Registry`.
This means the `Interest` state is not cleared.

This branch adds code in `Layered` to ensure the per-subscriber filter
state is cleared when a global filter short circuits `Interest`
evaluation.

This fixes #1563.

Signed-off-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Mar 24, 2022
@hawkw
subscriber: clear enabled filter map when short circuiting (#1575)
8e6f0ed 
This is essentially the same change as #1569, but for `enabled` states
rather than `register_callsite`. When a global filter returns `false`
from `enabled`, ensure that the per-subscriber filter `FilterMap` and
debug counters are cleared, so that they are empty on the next `enabled`
call.

See #1563
hawkw added a commit that referenced this pull request Mar 24, 2022
@hawkw
subscriber: clear per-subscriber interest when short circuiting (#1569)
4e386ca 
Currently, when evaluating `register_callsite` for a stack containing
per-subscriber filters, the intermediate `Interest` from combining the
per-subscriber filters' `Interest`s is stored in the thread-local
`FilterState`. When all per-subscriber filters have been evaluated, we
reach the `Registry`, which clears the `FilterState` and bubbles the
`Interest` back up. However, when a _global_ filter in the stack returns
`Interest::never`, we short-circuit, and don't reach the `Registry`.
This means the `Interest` state is not cleared.

This branch adds code in `Layered` to ensure the per-subscriber filter
state is cleared when a global filter short circuits `Interest`
evaluation.

This fixes #1563.

Signed-off-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Mar 24, 2022
@hawkw
subscriber: clear enabled filter map when short circuiting (#1575)
9c39495 
This is essentially the same change as #1569, but for `enabled` states
rather than `register_callsite`. When a global filter returns `false`
from `enabled`, ensure that the per-subscriber filter `FilterMap` and
debug counters are cleared, so that they are empty on the next `enabled`
call.

See #1563
hawkw added a commit that referenced this pull request Mar 24, 2022
@hawkw
subscriber: clear per-subscriber interest when short circuiting (#1569)
0c56df2 
Currently, when evaluating `register_callsite` for a stack containing
per-subscriber filters, the intermediate `Interest` from combining the
per-subscriber filters' `Interest`s is stored in the thread-local
`FilterState`. When all per-subscriber filters have been evaluated, we
reach the `Registry`, which clears the `FilterState` and bubbles the
`Interest` back up. However, when a _global_ filter in the stack returns
`Interest::never`, we short-circuit, and don't reach the `Registry`.
This means the `Interest` state is not cleared.

This branch adds code in `Layered` to ensure the per-subscriber filter
state is cleared when a global filter short circuits `Interest`
evaluation.

This fixes #1563.

Signed-off-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Mar 24, 2022
@hawkw
subscriber: clear enabled filter map when short circuiting (#1575)
fa53463 
This is essentially the same change as #1569, but for `enabled` states
rather than `register_callsite`. When a global filter returns `false`
from `enabled`, ensure that the per-subscriber filter `FilterMap` and
debug counters are cleared, so that they are empty on the next `enabled`
call.

See #1563
hawkw added a commit that referenced this pull request Mar 24, 2022
@hawkw
subscriber: clear per-subscriber interest when short circuiting (#1569)
b3602fe 
Currently, when evaluating `register_callsite` for a stack containing
per-subscriber filters, the intermediate `Interest` from combining the
per-subscriber filters' `Interest`s is stored in the thread-local
`FilterState`. When all per-subscriber filters have been evaluated, we
reach the `Registry`, which clears the `FilterState` and bubbles the
`Interest` back up. However, when a _global_ filter in the stack returns
`Interest::never`, we short-circuit, and don't reach the `Registry`.
This means the `Interest` state is not cleared.

This branch adds code in `Layered` to ensure the per-subscriber filter
state is cleared when a global filter short circuits `Interest`
evaluation.

This fixes #1563.

Signed-off-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Mar 24, 2022
@hawkw
subscriber: clear enabled filter map when short circuiting (#1575)
a572fe4 
This is essentially the same change as #1569, but for `enabled` states
rather than `register_callsite`. When a global filter returns `false`
from `enabled`, ensure that the per-subscriber filter `FilterMap` and
debug counters are cleared, so that they are empty on the next `enabled`
call.

See #1563
hawkw added a commit that referenced this pull request Mar 24, 2022
@hawkw
subscriber: clear per-subscriber interest when short circuiting (#1569)
a6abb07 
Currently, when evaluating `register_callsite` for a stack containing
per-subscriber filters, the intermediate `Interest` from combining the
per-subscriber filters' `Interest`s is stored in the thread-local
`FilterState`. When all per-subscriber filters have been evaluated, we
reach the `Registry`, which clears the `FilterState` and bubbles the
`Interest` back up. However, when a _global_ filter in the stack returns
`Interest::never`, we short-circuit, and don't reach the `Registry`.
This means the `Interest` state is not cleared.

This branch adds code in `Layered` to ensure the per-subscriber filter
state is cleared when a global filter short circuits `Interest`
evaluation.

This fixes #1563.

Signed-off-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Mar 24, 2022
@hawkw
subscriber: clear enabled filter map when short circuiting (#1575)
de067c7 
This is essentially the same change as #1569, but for `enabled` states
rather than `register_callsite`. When a global filter returns `false`
from `enabled`, ensure that the per-subscriber filter `FilterMap` and
debug counters are cleared, so that they are empty on the next `enabled`
call.

See #1563
hawkw added a commit that referenced this pull request Mar 24, 2022
@hawkw
subscriber: clear per-subscriber interest when short circuiting (#1569)
b26119c 
Currently, when evaluating `register_callsite` for a stack containing
per-subscriber filters, the intermediate `Interest` from combining the
per-subscriber filters' `Interest`s is stored in the thread-local
`FilterState`. When all per-subscriber filters have been evaluated, we
reach the `Registry`, which clears the `FilterState` and bubbles the
`Interest` back up. However, when a _global_ filter in the stack returns
`Interest::never`, we short-circuit, and don't reach the `Registry`.
This means the `Interest` state is not cleared.

This branch adds code in `Layered` to ensure the per-subscriber filter
state is cleared when a global filter short circuits `Interest`
evaluation.

This fixes #1563.

Signed-off-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Mar 24, 2022
@hawkw
subscriber: clear enabled filter map when short circuiting (#1575)
5593680 
This is essentially the same change as #1569, but for `enabled` states
rather than `register_callsite`. When a global filter returns `false`
from `enabled`, ensure that the per-subscriber filter `FilterMap` and
debug counters are cleared, so that they are empty on the next `enabled`
call.

See #1563
hawkw added a commit that referenced this pull request Mar 24, 2022
@hawkw
subscriber: clear per-subscriber interest when short circuiting (#1569)
dc310a1 
Currently, when evaluating `register_callsite` for a stack containing
per-subscriber filters, the intermediate `Interest` from combining the
per-subscriber filters' `Interest`s is stored in the thread-local
`FilterState`. When all per-subscriber filters have been evaluated, we
reach the `Registry`, which clears the `FilterState` and bubbles the
`Interest` back up. However, when a _global_ filter in the stack returns
`Interest::never`, we short-circuit, and don't reach the `Registry`.
This means the `Interest` state is not cleared.

This branch adds code in `Layered` to ensure the per-subscriber filter
state is cleared when a global filter short circuits `Interest`
evaluation.

This fixes #1563.

Signed-off-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Mar 24, 2022
@hawkw
subscriber: clear enabled filter map when short circuiting (#1575)
b9f4468 
This is essentially the same change as #1569, but for `enabled` states
rather than `register_callsite`. When a global filter returns `false`
from `enabled`, ensure that the per-subscriber filter `FilterMap` and
debug counters are cleared, so that they are empty on the next `enabled`
call.

See #1563
hawkw added a commit that referenced this pull request Mar 24, 2022
@hawkw
subscriber: clear per-subscriber interest when short circuiting (#1569)
301313a 
Currently, when evaluating `register_callsite` for a stack containing
per-subscriber filters, the intermediate `Interest` from combining the
per-subscriber filters' `Interest`s is stored in the thread-local
`FilterState`. When all per-subscriber filters have been evaluated, we
reach the `Registry`, which clears the `FilterState` and bubbles the
`Interest` back up. However, when a _global_ filter in the stack returns
`Interest::never`, we short-circuit, and don't reach the `Registry`.
This means the `Interest` state is not cleared.

This branch adds code in `Layered` to ensure the per-subscriber filter
state is cleared when a global filter short circuits `Interest`
evaluation.

This fixes #1563.

Signed-off-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Mar 24, 2022
@hawkw
subscriber: clear enabled filter map when short circuiting (#1575)
6a4d2f2 
This is essentially the same change as #1569, but for `enabled` states
rather than `register_callsite`. When a global filter returns `false`
from `enabled`, ensure that the per-subscriber filter `FilterMap` and
debug counters are cleared, so that they are empty on the next `enabled`
call.

See #1563
@davidbarsky davidbarsky mentioned this pull request Sep 26, 2023
Merged
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
@hawkw @kaffarell
subscriber: clear per-layer interest when short circuiting (tokio-rs#…
76355a6 
…1569)

Currently, when evaluating `register_callsite` for a stack containing
per-layer filters, the intermediate `Interest` from combining the per
layer filters' `Interest`s is stored in the thread-local `FilterState`.
When all per-layer filters have been evaluated, we reach the `Registry`,
which clears the `FilterState` and bubbles the `Interest` back up.
However, when a _global_ filter in the stack returns `Interest::never`,
we short-circuit, and don't reach the `Registry`. This means the
`Interest` state is not cleared.

This branch adds code in `Layered` to ensure the per-layer filter state
is cleared when a global filter short circuits `Interest` evaluation.

This fixes tokio-rs#1563.

Signed-off-by: Eliza Weisman <[email protected]>
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
@hawkw @kaffarell
subscriber: prepare to release v0.2.23 (tokio-rs#1573)
f7de843 
# 0.2.23 (September 16, 2021)

This release fixes a few bugs in the per-layer filtering API added in
v0.2.21.

### Fixed

- **env-filter**: Fixed excessive `EnvFilter` memory use ([tokio-rs#1568])
- **filter**: Fixed a panic that may occur in debug mode when using
  per-layer filters together with global filters ([tokio-rs#1569])
- Fixed incorrect documentation formatting ([tokio-rs#1572])

[tokio-rs#1568]: tokio-rs#1568
[tokio-rs#1569]: tokio-rs#1569
[tokio-rs#1572]: tokio-rs#1572
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
@hawkw @kaffarell
subscriber: clear enabled filter map when short circuiting (tokio-r…
ba1c36b 
…s#1575)

This is essentially the same change as tokio-rs#1569, but for `enabled` states
rather than `register_callsite`. When a global filter returns `false`
from `enabled`, ensure that the per-layer filter `FilterMap` and debug
counters are cleared, so that they are empty on the next `enabled` call.

See tokio-rs#1563
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.
2 participants
@hawkw @davidbarsky