Skip to content

tracing-subscriber: implement Clone for EnvFilter #2398

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
Closed
wants to merge 1 commit into from
Closed

tracing-subscriber: implement Clone for EnvFilter #2398

wants to merge 1 commit into from

Conversation

gelfand
Copy link

@gelfand gelfand commented Nov 25, 2022

Motivation

As of clap v4, there's Clone bound, thus making it impossible for EnvFilter to be a clap command line argument.

Solution

Implement Clone for EnvFilter.

@gelfand gelfand requested review from hawkw, davidbarsky and a team as code owners November 25, 2022 19:07
@gelfand gelfand closed this Mar 22, 2023
@gelfand gelfand deleted the tracing-subscriber/env-filter-clone branch March 22, 2023 16:39
@olivia-fl olivia-fl mentioned this pull request Apr 25, 2024
Closed
olivia-fl added a commit to olivia-fl/tracing that referenced this pull request Apr 26, 2024
@olivia-fl
subscriber: impl Clone for EnvFilter
bf4acba 
This is useful when using `EnvFilter` for multiple identical per-layer
filters, as well as with clap and similar libraries that have `Clone`
bounds.

We generally expect users to be cloning an `EnvFilter` before attaching it
to a subscriber, rather than cloning `EnvFilters` that are already
attached. Because of this, we reset all the accumulated dynamic state
when cloning. This means that some spans and callsites might be missed
when an already-attached `EnvFilter` is cloned, but the presence of the
dynamic state mean that detaching and attaching `EnvFilter`s to existing
subscribers (e.g. with `reload`) already doesn't work very well. This
isn't a new class of problem.

There was a previous implementation of this in tokio-rs#2398, that shared the
dynamic state between all cloned filters behind an `Arc`. I chose
not do go for that approach because it causes inconsistencies if the
cloned filters are attached to different subscribers.

Fixes: tokio-rs#2360
@olivia-fl olivia-fl mentioned this pull request Apr 26, 2024
Merged
olivia-fl added a commit to olivia-fl/tracing that referenced this pull request Apr 26, 2024
@olivia-fl
subscriber: impl Clone for EnvFilter
b348dca 
This is useful when using `EnvFilter` for multiple identical per-layer
filters, as well as with clap and similar libraries that have `Clone`
bounds.

We generally expect users to be cloning an `EnvFilter` before attaching it
to a subscriber, rather than cloning `EnvFilters` that are already
attached. Because of this, we reset all the accumulated dynamic state
when cloning. This means that some spans and callsites might be missed
when an already-attached `EnvFilter` is cloned, but the presence of the
dynamic state mean that detaching and attaching `EnvFilter`s to existing
subscribers (e.g. with `reload`) already doesn't work very well. This
isn't a new class of problem.

There was a previous implementation of this in tokio-rs#2398, that shared the
dynamic state between all cloned filters behind an `Arc`. I chose
not do go for that approach because it causes inconsistencies if the
cloned filters are attached to different subscribers.

Fixes: tokio-rs#2360
olivia-fl added a commit to olivia-fl/tracing that referenced this pull request Aug 13, 2025
@olivia-fl
subscriber: impl Clone for EnvFilter
c6567e8 
This is useful when using `EnvFilter` for multiple identical per-layer
filters, as well as with clap and similar libraries that have `Clone`
bounds.

We generally expect users to be cloning an `EnvFilter` before attaching it
to a subscriber, rather than cloning `EnvFilters` that are already
attached. Because of this, we reset all the accumulated dynamic state
when cloning. This means that some spans and callsites might be missed
when an already-attached `EnvFilter` is cloned, but the presence of the
dynamic state mean that detaching and attaching `EnvFilter`s to existing
subscribers (e.g. with `reload`) already doesn't work very well. This
isn't a new class of problem.

There was a previous implementation of this in tokio-rs#2398, that shared the
dynamic state between all cloned filters behind an `Arc`. I chose
not do go for that approach because it causes inconsistencies if the
cloned filters are attached to different subscribers.

Fixes: tokio-rs#2360
olivia-fl added a commit to olivia-fl/tracing that referenced this pull request Aug 13, 2025
@olivia-fl
subscriber: impl Clone for EnvFilter
8f8f26e 
This is useful when using `EnvFilter` for multiple identical per-layer
filters, as well as with clap and similar libraries that have `Clone`
bounds.

We generally expect users to be cloning an `EnvFilter` before attaching it
to a subscriber, rather than cloning `EnvFilters` that are already
attached. Because of this, we reset all the accumulated dynamic state
when cloning. This means that some spans and callsites might be missed
when an already-attached `EnvFilter` is cloned, but the presence of the
dynamic state mean that detaching and attaching `EnvFilter`s to existing
subscribers (e.g. with `reload`) already doesn't work very well. This
isn't a new class of problem.

There was a previous implementation of this in tokio-rs#2398, that shared the
dynamic state between all cloned filters behind an `Arc`. I chose
not do go for that approach because it causes inconsistencies if the
cloned filters are attached to different subscribers.

Fixes: tokio-rs#2360
olivia-fl added a commit to olivia-fl/tracing that referenced this pull request Aug 14, 2025
@olivia-fl
subscriber: impl Clone for EnvFilter
c478a5b 
This is useful when using `EnvFilter` for multiple identical per-layer
filters, as well as with clap and similar libraries that have `Clone`
bounds.

We generally expect users to be cloning an `EnvFilter` before attaching it
to a subscriber, rather than cloning `EnvFilters` that are already
attached. Because of this, we reset all the accumulated dynamic state
when cloning. This means that some spans and callsites might be missed
when an already-attached `EnvFilter` is cloned, but the presence of the
dynamic state mean that detaching and attaching `EnvFilter`s to existing
subscribers (e.g. with `reload`) already doesn't work very well. This
isn't a new class of problem.

There was a previous implementation of this in tokio-rs#2398, that shared the
dynamic state between all cloned filters behind an `Arc`. I chose
not do go for that approach because it causes inconsistencies if the
cloned filters are attached to different subscribers.

Fixes: tokio-rs#2360
jplatte pushed a commit that referenced this pull request Aug 14, 2025
@olivia-fl
subscriber: impl Clone for EnvFilter (#2956)
74fc079 
This is useful when using `EnvFilter` for multiple identical per-layer
filters, as well as with clap and similar libraries that have `Clone`
bounds.

We generally expect users to be cloning an `EnvFilter` before attaching it
to a subscriber, rather than cloning `EnvFilters` that are already
attached. Because of this, we reset all the accumulated dynamic state
when cloning. This means that some spans and callsites might be missed
when an already-attached `EnvFilter` is cloned, but the presence of the
dynamic state mean that detaching and attaching `EnvFilter`s to existing
subscribers (e.g. with `reload`) already doesn't work very well. This
isn't a new class of problem.

There was a previous implementation of this in #2398, that shared the
dynamic state between all cloned filters behind an `Arc`. I chose
not do go for that approach because it causes inconsistencies if the
cloned filters are attached to different subscribers.

Fixes: #2360
olivia-fl added a commit to olivia-fl/tracing that referenced this pull request Aug 14, 2025
@olivia-fl
subscriber: impl Clone for EnvFilter
0f6fc18 
This is useful when using `EnvFilter` for multiple identical per-layer
filters, as well as with clap and similar libraries that have `Clone`
bounds.

We generally expect users to be cloning an `EnvFilter` before attaching it
to a subscriber, rather than cloning `EnvFilters` that are already
attached. Because of this, we reset all the accumulated dynamic state
when cloning. This means that some spans and callsites might be missed
when an already-attached `EnvFilter` is cloned, but the presence of the
dynamic state mean that detaching and attaching `EnvFilter`s to existing
subscribers (e.g. with `reload`) already doesn't work very well. This
isn't a new class of problem.

There was a previous implementation of this in tokio-rs#2398, that shared the
dynamic state between all cloned filters behind an `Arc`. I chose
not do go for that approach because it causes inconsistencies if the
cloned filters are attached to different subscribers.

Fixes: tokio-rs#2360
jplatte pushed a commit that referenced this pull request Aug 15, 2025
@olivia-fl
subscriber: impl Clone for EnvFilter (#3360)
f71cebe 
This is useful when using `EnvFilter` for multiple identical per-layer
filters, as well as with clap and similar libraries that have `Clone`
bounds.

We generally expect users to be cloning an `EnvFilter` before attaching it
to a subscriber, rather than cloning `EnvFilters` that are already
attached. Because of this, we reset all the accumulated dynamic state
when cloning. This means that some spans and callsites might be missed
when an already-attached `EnvFilter` is cloned, but the presence of the
dynamic state mean that detaching and attaching `EnvFilter`s to existing
subscribers (e.g. with `reload`) already doesn't work very well. This
isn't a new class of problem.

There was a previous implementation of this in #2398, that shared the
dynamic state between all cloned filters behind an `Arc`. I chose
not do go for that approach because it causes inconsistencies if the
cloned filters are attached to different subscribers.

Fixes: #2360
jevolk pushed a commit to matrix-construct/tracing that referenced this pull request Aug 30, 2025
@olivia-fl @jevolk
subscriber: impl Clone for EnvFilter
831fcf8 
This is useful when using `EnvFilter` for multiple identical per-layer
filters, as well as with clap and similar libraries that have `Clone`
bounds.

We generally expect users to be cloning an `EnvFilter` before attaching it
to a subscriber, rather than cloning `EnvFilters` that are already
attached. Because of this, we reset all the accumulated dynamic state
when cloning. This means that some spans and callsites might be missed
when an already-attached `EnvFilter` is cloned, but the presence of the
dynamic state mean that detaching and attaching `EnvFilter`s to existing
subscribers (e.g. with `reload`) already doesn't work very well. This
isn't a new class of problem.

There was a previous implementation of this in tokio-rs#2398, that shared the
dynamic state between all cloned filters behind an `Arc`. I chose
not do go for that approach because it causes inconsistencies if the
cloned filters are attached to different subscribers.

Fixes: tokio-rs#2360
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hawkw hawkw Awaiting requested review from hawkw hawkw is a code owner

@davidbarsky davidbarsky Awaiting requested review from davidbarsky davidbarsky is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@gelfand