-
Notifications
You must be signed in to change notification settings - Fork 734
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 new_span
, on_enter
, on_exit
, and on_close
hooks to Filter
#1955
Labels
Comments
hawkw
added
kind/feature
New feature or request
crate/subscriber
Related to the `tracing-subscriber` crate
labels
Feb 22, 2022
Merged
hawkw
pushed a commit
that referenced
this issue
Mar 8, 2022
Closes #1955 Call those methods in the `Layer` methods for `Filtered` and delegate them in the filter combinators ## Motivation Currently, if a `Filter` is going to implement dynamic filtering, it must do this by querying the wrapped Subscriber for the current span context. Since `Filter` only has callsite_enabled and enabled methods, a `Filter` implementation cannot track the span context internally, the way a Layer can. Unfortunately, this means that the current implementation of `EnvFilter` can only implement `Layer` (and provide global filtering). It cannot currently implement `Filter`, because it stores span context data internally. See #1868 for details. ## Proposal We should add `on_new_span`, `on_enter`, `on_exit`, and `on_close` methods to `Filter`. This would allow implementing `Filter`s (such as `EnvFilter`) that internally track span states.
hawkw
pushed a commit
that referenced
this issue
Mar 23, 2022
Closes #1955 Call those methods in the `Subscribe` methods for `Filtered` and delegate them in the filter combinators ## Motivation Currently, if a `Filter` is going to implement dynamic filtering, it must do this by querying the wrapped Subscriber for the current span context. Since `Filter` only has callsite_enabled and enabled methods, a `Filter` implementation cannot track the span context internally, the way a subscriber can. Unfortunately, this means that the current implementation of `EnvFilter` can only implement `Subscribe` (and provide global filtering). It cannot currently implement `Filter`, because it stores span context data internally. See #1868 for details. ## Proposal We should add `on_new_span`, `on_enter`, `on_exit`, and `on_close` methods to `Filter`. This would allow implementing `Filter`s (such as `EnvFilter`) that internally track span states.
hawkw
pushed a commit
that referenced
this issue
Mar 23, 2022
Closes #1955 Call those methods in the `Subscribe` methods for `Filtered` and delegate them in the filter combinators ## Motivation Currently, if a `Filter` is going to implement dynamic filtering, it must do this by querying the wrapped Subscriber for the current span context. Since `Filter` only has callsite_enabled and enabled methods, a `Filter` implementation cannot track the span context internally, the way a subscriber can. Unfortunately, this means that the current implementation of `EnvFilter` can only implement `Subscribe` (and provide global filtering). It cannot currently implement `Filter`, because it stores span context data internally. See #1868 for details. ## Proposal We should add `on_new_span`, `on_enter`, `on_exit`, and `on_close` methods to `Filter`. This would allow implementing `Filter`s (such as `EnvFilter`) that internally track span states.
hawkw
pushed a commit
that referenced
this issue
Mar 23, 2022
Closes #1955 Call those methods in the `Subscribe` methods for `Filtered` and delegate them in the filter combinators ## Motivation Currently, if a `Filter` is going to implement dynamic filtering, it must do this by querying the wrapped Subscriber for the current span context. Since `Filter` only has callsite_enabled and enabled methods, a `Filter` implementation cannot track the span context internally, the way a subscriber can. Unfortunately, this means that the current implementation of `EnvFilter` can only implement `Subscribe` (and provide global filtering). It cannot currently implement `Filter`, because it stores span context data internally. See #1868 for details. ## Proposal We should add `on_new_span`, `on_enter`, `on_exit`, and `on_close` methods to `Filter`. This would allow implementing `Filter`s (such as `EnvFilter`) that internally track span states.
hawkw
pushed a commit
that referenced
this issue
Mar 24, 2022
Closes #1955 Call those methods in the `Subscribe` methods for `Filtered` and delegate them in the filter combinators ## Motivation Currently, if a `Filter` is going to implement dynamic filtering, it must do this by querying the wrapped Subscriber for the current span context. Since `Filter` only has callsite_enabled and enabled methods, a `Filter` implementation cannot track the span context internally, the way a subscriber can. Unfortunately, this means that the current implementation of `EnvFilter` can only implement `Subscribe` (and provide global filtering). It cannot currently implement `Filter`, because it stores span context data internally. See #1868 for details. ## Proposal We should add `on_new_span`, `on_enter`, `on_exit`, and `on_close` methods to `Filter`. This would allow implementing `Filter`s (such as `EnvFilter`) that internally track span states.
hawkw
pushed a commit
that referenced
this issue
Mar 24, 2022
Closes #1955 Call those methods in the `Subscribe` methods for `Filtered` and delegate them in the filter combinators ## Motivation Currently, if a `Filter` is going to implement dynamic filtering, it must do this by querying the wrapped Subscriber for the current span context. Since `Filter` only has callsite_enabled and enabled methods, a `Filter` implementation cannot track the span context internally, the way a subscriber can. Unfortunately, this means that the current implementation of `EnvFilter` can only implement `Subscribe` (and provide global filtering). It cannot currently implement `Filter`, because it stores span context data internally. See #1868 for details. ## Proposal We should add `on_new_span`, `on_enter`, `on_exit`, and `on_close` methods to `Filter`. This would allow implementing `Filter`s (such as `EnvFilter`) that internally track span states.
hawkw
pushed a commit
that referenced
this issue
Mar 24, 2022
Closes #1955 Call those methods in the `Subscribe` methods for `Filtered` and delegate them in the filter combinators ## Motivation Currently, if a `Filter` is going to implement dynamic filtering, it must do this by querying the wrapped Subscriber for the current span context. Since `Filter` only has callsite_enabled and enabled methods, a `Filter` implementation cannot track the span context internally, the way a subscriber can. Unfortunately, this means that the current implementation of `EnvFilter` can only implement `Subscribe` (and provide global filtering). It cannot currently implement `Filter`, because it stores span context data internally. See #1868 for details. ## Proposal We should add `on_new_span`, `on_enter`, `on_exit`, and `on_close` methods to `Filter`. This would allow implementing `Filter`s (such as `EnvFilter`) that internally track span states.
hawkw
pushed a commit
that referenced
this issue
Mar 24, 2022
Closes #1955 Call those methods in the `Subscribe` methods for `Filtered` and delegate them in the filter combinators ## Motivation Currently, if a `Filter` is going to implement dynamic filtering, it must do this by querying the wrapped Subscriber for the current span context. Since `Filter` only has callsite_enabled and enabled methods, a `Filter` implementation cannot track the span context internally, the way a subscriber can. Unfortunately, this means that the current implementation of `EnvFilter` can only implement `Subscribe` (and provide global filtering). It cannot currently implement `Filter`, because it stores span context data internally. See #1868 for details. ## Proposal We should add `on_new_span`, `on_enter`, `on_exit`, and `on_close` methods to `Filter`. This would allow implementing `Filter`s (such as `EnvFilter`) that internally track span states.
hawkw
pushed a commit
that referenced
this issue
Mar 24, 2022
Closes #1955 Call those methods in the `Subscribe` methods for `Filtered` and delegate them in the filter combinators ## Motivation Currently, if a `Filter` is going to implement dynamic filtering, it must do this by querying the wrapped Subscriber for the current span context. Since `Filter` only has callsite_enabled and enabled methods, a `Filter` implementation cannot track the span context internally, the way a subscriber can. Unfortunately, this means that the current implementation of `EnvFilter` can only implement `Subscribe` (and provide global filtering). It cannot currently implement `Filter`, because it stores span context data internally. See #1868 for details. ## Proposal We should add `on_new_span`, `on_enter`, `on_exit`, and `on_close` methods to `Filter`. This would allow implementing `Filter`s (such as `EnvFilter`) that internally track span states.
hawkw
pushed a commit
that referenced
this issue
Mar 24, 2022
Closes #1955 Call those methods in the `Subscribe` methods for `Filtered` and delegate them in the filter combinators ## Motivation Currently, if a `Filter` is going to implement dynamic filtering, it must do this by querying the wrapped Subscriber for the current span context. Since `Filter` only has callsite_enabled and enabled methods, a `Filter` implementation cannot track the span context internally, the way a subscriber can. Unfortunately, this means that the current implementation of `EnvFilter` can only implement `Subscribe` (and provide global filtering). It cannot currently implement `Filter`, because it stores span context data internally. See #1868 for details. ## Proposal We should add `on_new_span`, `on_enter`, `on_exit`, and `on_close` methods to `Filter`. This would allow implementing `Filter`s (such as `EnvFilter`) that internally track span states.
hawkw
pushed a commit
that referenced
this issue
Mar 24, 2022
Closes #1955 Call those methods in the `Subscribe` methods for `Filtered` and delegate them in the filter combinators ## Motivation Currently, if a `Filter` is going to implement dynamic filtering, it must do this by querying the wrapped Subscriber for the current span context. Since `Filter` only has callsite_enabled and enabled methods, a `Filter` implementation cannot track the span context internally, the way a subscriber can. Unfortunately, this means that the current implementation of `EnvFilter` can only implement `Subscribe` (and provide global filtering). It cannot currently implement `Filter`, because it stores span context data internally. See #1868 for details. ## Proposal We should add `on_new_span`, `on_enter`, `on_exit`, and `on_close` methods to `Filter`. This would allow implementing `Filter`s (such as `EnvFilter`) that internally track span states.
kaffarell
pushed a commit
to kaffarell/tracing
that referenced
this issue
May 22, 2024
…#1973) Closes tokio-rs#1955 Call those methods in the `Layer` methods for `Filtered` and delegate them in the filter combinators ## Motivation Currently, if a `Filter` is going to implement dynamic filtering, it must do this by querying the wrapped Subscriber for the current span context. Since `Filter` only has callsite_enabled and enabled methods, a `Filter` implementation cannot track the span context internally, the way a Layer can. Unfortunately, this means that the current implementation of `EnvFilter` can only implement `Layer` (and provide global filtering). It cannot currently implement `Filter`, because it stores span context data internally. See tokio-rs#1868 for details. ## Proposal We should add `on_new_span`, `on_enter`, `on_exit`, and `on_close` methods to `Filter`. This would allow implementing `Filter`s (such as `EnvFilter`) that internally track span states.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Feature Request
Crates
tracing-subscriber
Motivation
Currently, if a
Filter
is going to implement dynamic filtering, it must do this by querying the wrappedSubscriber
for the current span context. SinceFilter
only hascallsite_enabled
andenabled
methods, aFilter
implementation cannot track the span context internally, the way aLayer
can.Unfortunately, this means that the current implementation of
EnvFilter
can only implementLayer
(and provide global filtering). It cannot currently implementFilter
, because it stores span context data internally. See #1868 for details.Proposal
We should add
on_new_span
,on_enter
,on_exit
, andon_close
methods toFilter
. This would allow implementingFilter
s (such asEnvFilter
) that internally track span states.Alternatives
There are essentially two other options for implementing
Filter
forEnvFilter
:We could rewrite the implementation to use the
LookupSpan
APIs to store span data in the registry, and query that instead of internally. This might potentially require a breaking change, asEnvFilter
would require theSubscriber
to implementLookupSpan
.Another hurdle is that the way
EnvFilter
is currently implemented also requires anew_span
notification. Currently, theFilter
trait cannot store data for a span when that span is created, even if that data is used for filtering later. This is because it only receivesenabled
method calls when spans are created. It cannot store data in the span for filtering events, because the span has not yet been created yet. Potentially, if we prefer to make a smaller, more targeted modification to theFilter
trait, we could just add anew_span
method.Or, we could use an approach where the
EnvFilter
is shared between a struct implementingFilter
and a struct implementingLayer
, where theLayer
does not perform any actual filtering, but simply mutates the sharedEnvFilter
's span state. I discussed that inEnvFilter
does not implementFilter
#1868 (comment). This would allow aFilter
impl forEnvFilter
without modifying theFilter
trait, but it seems quite gross and makes the API worse.The text was updated successfully, but these errors were encountered: