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

forward-port Filter impl for EnvFilter #2031

Merged
merged 3 commits into from
Mar 29, 2022
Merged

forward-port Filter impl for EnvFilter #2031

merged 3 commits into from
Mar 29, 2022

Commits on Mar 29, 2022

  1. subscriber: fix Filtered::on_exit calling on_enter by mistake (#2018 

    )

## 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
    hawkw committed Mar 29, 2022
    Configuration menu
    Copy the full SHA
    70fcfce View commit details
    Browse the repository at this point in the history

  2. subscriber: add Filter::on_record callback 

    Currently, `Filter` only has the `on_new_span`, `on_enter`, `on_exit`,
and `on_close` callbacks. This means it won't handle cases where a
span records a new field value that changes its filter state.

This branch adds the missing `on_record` callback.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
    @hawkw
    hawkw committed Mar 29, 2022
    Configuration menu
    Copy the full SHA
    7e4724d View commit details
    Browse the repository at this point in the history

  3. subscriber: impl Filter for EnvFilter (#1983) 

    ## Motivation

Filtering by span and field requires using `EnvFilter` rather than
`Targets`. Per-subscriber filtering requires the `Filter` trait, which
`EnvFilter` does not implement.

## Solution

Implement the `Filter` trait for `EnvFilter`. PR #1973 adds additiional
methods to the `Filter` trait, which are necessary for `EnvFilter` to
implement dynamic span filtering. Now that those methods are added, we
can provide a `Filter` impl for `EnvFilter`.

In addition, we changed the globally-scoped `thread_local!` macro to use
a `ThreadLocal` struct as a field, so that multiple `EnvFilter`s used as
per-subscriber filters don't share a single span scope.

Fixes #1868
Follow-up on #1973

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
    @tfreiberg-fastly @hawkw
    tfreiberg-fastly and hawkw committed Mar 29, 2022
    Configuration menu
    Copy the full SHA
    d0a73e2 View commit details
    Browse the repository at this point in the history