Targets::with_targets ignores default #1790
Comments
|
I just ran into this too! I'd be happy to prepare a PR if someone has an opinion on what the appropriate fix is. |
|
What do you think the expected behavior for the default directive when merging Another potential option is to always pick the more verbose default, rather than always overwriting the default from the first set of targets with the default from the second one. Alternatively, we could add an accessor that returns the default level, so that the user can use that to explicitly implement whatever merging logic they want, and document in the How do others feel about that solution? |
|
FWIW, the behavior I expected is that the last target to specify a "thing" wins, whether that "thing" is a specific target or the default. Isn't that the way all the other merging works? (If not then I am misinformed.)
This works for me, though! |
## Motivation This makes it possible to fully "override" some base `Targets` filter with another (e.g. user-supplied) filter. Without some way to obtain the default, only explicit targets can be overridden (via `IntoIter` and friends). See also #1790 (comment) ## Solution We can add a method to `Targets` that filters the underlying `DirectiveSet` for the default directive. This works because `DirectiveSet::add` will replace directives with the same `target`/`field_names`, which is always `None`/`vec![]` for the directive added by `with_default` (and in fact we are only concerned with `target`, since no other `Targets` API allows adding directives with a `None` target). Ideally the method would be named `default`, corresponding to `with_default`, however this conflicts with `Default::default` and so would be a breaking change (and harm ergonomics). `default_level` seemed a better name than `get_default`, since "getters" of this style are generally considered unidiomatic<sup>[citation needed]</sup>. Example usage: ```rust let mut filter = Targets::new().with_target("some_module", LevelFilter::INFO); // imagine this came from `RUST_LOG` or similar let override: Targets = "trace".parse().unwrap(); // merge the override if let Some(default) = override.default_level() { filter = filter.with_default(default); } for (target, level) in override.iter() { filter = filter.with_target(target, level); } ``` Closes #1790
## Motivation This makes it possible to fully "override" some base `Targets` filter with another (e.g. user-supplied) filter. Without some way to obtain the default, only explicit targets can be overridden (via `IntoIter` and friends). See also #1790 (comment) ## Solution We can add a method to `Targets` that filters the underlying `DirectiveSet` for the default directive. This works because `DirectiveSet::add` will replace directives with the same `target`/`field_names`, which is always `None`/`vec![]` for the directive added by `with_default` (and in fact we are only concerned with `target`, since no other `Targets` API allows adding directives with a `None` target). Ideally the method would be named `default`, corresponding to `with_default`, however this conflicts with `Default::default` and so would be a breaking change (and harm ergonomics). `default_level` seemed a better name than `get_default`, since "getters" of this style are generally considered unidiomatic<sup>[citation needed]</sup>. Example usage: ```rust let mut filter = Targets::new().with_target("some_module", LevelFilter::INFO); // imagine this came from `RUST_LOG` or similar let override: Targets = "trace".parse().unwrap(); // merge the override if let Some(default) = override.default_level() { filter = filter.with_default(default); } for (target, level) in override.iter() { filter = filter.with_target(target, level); } ``` Closes #1790
## Motivation This makes it possible to fully "override" some base `Targets` filter with another (e.g. user-supplied) filter. Without some way to obtain the default, only explicit targets can be overridden (via `IntoIter` and friends). See also tokio-rs/tracing#1790 (comment) ## Solution We can add a method to `Targets` that filters the underlying `DirectiveSet` for the default directive. This works because `DirectiveSet::add` will replace directives with the same `target`/`field_names`, which is always `None`/`vec![]` for the directive added by `with_default` (and in fact we are only concerned with `target`, since no other `Targets` API allows adding directives with a `None` target). Ideally the method would be named `default`, corresponding to `with_default`, however this conflicts with `Default::default` and so would be a breaking change (and harm ergonomics). `default_level` seemed a better name than `get_default`, since "getters" of this style are generally considered unidiomatic<sup>[citation needed]</sup>. Example usage: ```rust let mut filter = Targets::new().with_target("some_module", LevelFilter::INFO); // imagine this came from `RUST_LOG` or similar let override: Targets = "trace".parse().unwrap(); // merge the override if let Some(default) = override.default_level() { filter = filter.with_default(default); } for (target, level) in override.iter() { filter = filter.with_target(target, level); } ``` Closes #1790
Bug Report
Version
`cargo tree | grep tracing`
Platform
Darwin MacBook-Pro-16-inch-2021-2.local 21.2.0 Darwin Kernel Version 21.2.0: Sun Nov 28 20:28:41 PST 2021; root:xnu-8019.61.5~1/RELEASE_ARM64_T6000 arm64Crates
tracing_subscriberDescription
Merging two
Targetsobjects usingfirst.with_targets(second)ignores the default value set in second.Example
results in
where the
appdirective is correctly overridden by second, but the original default directive is retained.In fact, there appears to be no method to access the default level on a
Targets, so I can not even write a workaround.I'm using this pattern to combine the
--verboseCLI argument (first) with a--log-filterargument (second). I want the log-filter to take precedence for all directives, including default.The text was updated successfully, but these errors were encountered: