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: fix Filtered::on_exit calling on_enter by mistake #2018

Merged
merged 1 commit into from
Mar 24, 2022

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Mar 24, 2022

Motivation

Currently, the Filtered type's Layer impl accidentally calls the
inner Filter's on_enter hook in the on_exit hook. This
is...obviously wrong, lol, and means that filters like EnvFilter will
never consider spans to have exited, breaking filtering behavior.

@tfreiberg-fastly originally noticed this bug (see
#1983 (comment)),
but I wanted to make the change separately from PR #1983 so that we can
fix it on the main branch without waiting for #1983 to merge first.

Solution

This branch, uh, you know... fixes that.

## Motivation

Currently, the `Filtered` type's `Layer` impl accidentally calls the
inner `Filter`'s `on_enter` hook in the `on_exit` hook. This
is...obviously wrong, lol, and means that filters like `EnvFilter` will
never consider spans to have exited, breaking filtering behavior.

@tfreiberg-fastly originally noticed this bug (see
#1983 (comment)),
but I wanted to make the change separately from PR #1983 so that we can
fix it on the main branch without waiting for #1983 to merge first.

## Solution

This branch, uh, you know... fixes that.
@hawkw hawkw requested a review from a team as a code owner March 24, 2022 18:27
@hawkw hawkw enabled auto-merge (squash) March 24, 2022 18:28
Copy link
Member

@davidbarsky davidbarsky left a comment

Choose a reason for hiding this comment

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

oops

@hawkw hawkw merged commit 5813899 into v0.1.x Mar 24, 2022
@hawkw hawkw deleted the eliza/fix-filter-on-exit branch March 24, 2022 18:57
hawkw added a commit that referenced this pull request Mar 29, 2022
)

## Motivation

Currently, the `Filtered` type's `Subscribe` impl accidentally calls the
inner `Filter`'s `on_enter` hook in the `on_exit` hook. This
is...obviously wrong, lol, and means that filters like `EnvFilter` will
never consider spans to have exited, breaking filtering behavior.

@tfreiberg-fastly originally noticed this bug (see
#1983 (comment)),
but I wanted to make the change separately from PR #1983 so that we can
fix it on the main branch without waiting for #1983 to merge first.

## Solution

This branch, uh, you know... fixes that.
hawkw added a commit that referenced this pull request Mar 29, 2022
)

## Motivation

Currently, the `Filtered` type's `Subscribe` impl accidentally calls the
inner `Filter`'s `on_enter` hook in the `on_exit` hook. This
is...obviously wrong, lol, and means that filters like `EnvFilter` will
never consider spans to have exited, breaking filtering behavior.

@tfreiberg-fastly originally noticed this bug (see
#1983 (comment)),
but I wanted to make the change separately from PR #1983 so that we can
fix it on the main branch without waiting for #1983 to merge first.

## Solution

This branch, uh, you know... fixes that.
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
…kio-rs#2018)

## Motivation

Currently, the `Filtered` type's `Layer` impl accidentally calls the
inner `Filter`'s `on_enter` hook in the `on_exit` hook. This
is...obviously wrong, lol, and means that filters like `EnvFilter` will
never consider spans to have exited, breaking filtering behavior.

@tfreiberg-fastly originally noticed this bug (see
tokio-rs#1983 (comment)),
but I wanted to make the change separately from PR tokio-rs#1983 so that we can
fix it on the main branch without waiting for tokio-rs#1983 to merge first.

## Solution

This branch, uh, you know... fixes that.
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.

2 participants