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

Manually creating a Directive for a specific target #2507

Open
ten3roberts opened this issue Mar 14, 2023 · 4 comments · May be fixed by #3018
Open

Manually creating a Directive for a specific target #2507

ten3roberts opened this issue Mar 14, 2023 · 4 comments · May be fixed by #3018

Comments

@ten3roberts
Copy link

Feature Request

The ability to manually create a Directive with a target.

It is currently only possible to construct a Directive without a specific target using From<Level>

Crates

  • tracing_subscriber

Motivation

This would allow adding more specific filtering directives to the EnvFilter programmatically, to for example silence chatty crates.

Using Targets is not possible as it only provides a subset of the EnvFilter functionality, as well as not ordering the directives by specificity, using the EnvFilter is preferable.

Proposal

Directive::new().with_target("my_crate");

Alternatives

Using the more limited Targets which allow for add_directive(target, level:

let filter = Targets::new();
filter.extend([("my_crate", Level::INFO]);

if let Ok(env_filter) = std::env::var("RUST_LOG").unwrap_or_default().parse::<tracing_subscriber::filter::Targets>() {
    filter.extend(env_filter.iter());
}

eprintln!("Using filter: {filter:?}");

This doesn't work for RUST_LOG=info since iter only returns for Some(targets), so "general" directives will be missed.

Doing it the other way:

    let mut filter = std::env::var("RUST_LOG").unwrap_or_default().parse::<tracing_subscriber::filter::Targets>().unwrap_or_default();
    filter.extend(([("my_crate", Level::INFO]);

does not work with RUST_LOG="my_crate=debug since the my_crate=info will be ordered before my_crate=debug in the DirectiveSet due to stable sorting.

@ten3roberts
Copy link
Author

Additionally:

Is there a way to add a directive before the env, as adding them afterwards override, even the existing directive (from env) is more verbose?

Short exerpt from env_logger which perfectly replicates the desired behavior.

        let mut builder = env_logger::builder();
        builder.filter_level(LevelFilter::Info);

        for (level, modules) in MODULES {
            for module in *modules {
                builder.filter_module(module, *level);
            }
        }

        builder.parse_default_env().try_init()?;

@ten3roberts
Copy link
Author

Would a PR be appreciated?

@hawkw
Copy link
Member

hawkw commented May 3, 2023

Would a PR be appreciated?

I would be happy to merge a pull request for programmatically creating directives --- this has been discussed previously a bit in #404, so you may want to look at that issue as well.

There is also a builder API for env_filter

Additionally:

Is there a way to add a directive before the env, as adding them afterwards override, even the existing directive (from env) is more verbose?

There is already a builder API for EnvFilter, which is used to pre-configure parsing options. We could add the capability to add directives to this builder as well, replicating the env_logger functionality you'd like. I'd be happy to merge a PR for that as well.

@ten3roberts
Copy link
Author

Alright, that sounds like a good idea

rustonaut added a commit to rustonaut/tracing that referenced this issue Jun 24, 2024
\## Motivation

1. There are some filters which can not be created with the current
   Directive string repr syntax, like e.g. the strings `bool`, `1`
   or `2.3` or values containing characters including but not limited
   to `}],` (for some of this there are partial fixes in other PRS).
   Furthermore updating the syntax the accomadate this is always a
   potential braking change, one which also can be subtle to detect.

2. Sometimes there is need to create `Directives` progrematically,
   e.g. from a different kind of logging config or to set more
   complicated defaults.

3. We might want to add other matching capabilities in the future
   which might not be representable in the string syntax (or at
   least entail a braking change). Like e.g. matching fields regex,
   matching based on `dyn Value`, a field matching the string `true`
   and boolean `true` at the same time,  etc.

By allowing programatic creation of `Directive`s we allow downstream
users to work around existing issues, experiement with solution to
such issues in external crates and in addition set the fundation for
adding future matching capabilities in the future.

\## Solution

- adds `Directive::builder()` / `DirectiveBuilder`
- `ValueMatch` is now public, but wrapped to that we can freely extend and change it without braking changes

Workaround For: tokio-rs#1181
Fixes: tokio-rs#2507, tokio-rs#404
Refs: tokio-rs#3016, tokio-rs#1584, tokio-rs#1181
rustonaut added a commit to rustonaut/tracing that referenced this issue Jun 24, 2024
Motivation
----------

1. There are some filters which can not be created with the current
   Directive string repr syntax, like e.g. the strings `bool`, `1`
   or `2.3` or values containing characters including but not limited
   to `}],` (for some of this there are partial fixes in other PRS).
   Furthermore updating the syntax the accomadate this is always a
   potential braking change, one which also can be subtle to detect.

2. Sometimes there is need to create `Directives` progrematically,
   e.g. from a different kind of logging config or to set more
   complicated defaults.

3. We might want to add other matching capabilities in the future
   which might not be representable in the string syntax (or at
   least entail a braking change). Like e.g. matching fields regex,
   matching based on `dyn Value`, a field matching the string `true`
   and boolean `true` at the same time,  etc.

By allowing programatic creation of `Directive`s we allow downstream
users to work around existing issues, experiement with solution to
such issues in external crates and in addition set the fundation for
adding future matching capabilities in the future.

Solution
--------

- adds `Directive::builder()` / `DirectiveBuilder`
- `ValueMatch` is now public, but wrapped to that we can freely extend and change it without braking changes

Workaround For: tokio-rs#1181
Fixes: tokio-rs#2507, tokio-rs#404
Refs: tokio-rs#3016, tokio-rs#1584, tokio-rs#1181
rustonaut added a commit to rustonaut/tracing that referenced this issue Jun 24, 2024
Motivation
----------

1. There are some filters which can not be created with the current
   Directive string repr syntax, like e.g. the strings `bool`, `1`
   or `2.3` or values containing characters including but not limited
   to `}],` (for some of this there are partial fixes in other PRS).
   Furthermore updating the syntax the accomadate this is always a
   potential braking change, one which also can be subtle to detect.

2. Sometimes there is need to create `Directives` progrematically,
   e.g. from a different kind of logging config or to set more
   complicated defaults.

3. We might want to add other matching capabilities in the future
   which might not be representable in the string syntax (or at
   least entail a braking change). Like e.g. matching fields regex,
   matching based on `dyn Value`, a field matching the string `true`
   and boolean `true` at the same time,  etc.

By allowing programatic creation of `Directive`s we allow downstream
users to work around existing issues, experiement with solution to
such issues in external crates and in addition set the fundation for
adding future matching capabilities in the future.

Solution
--------

- adds `Directive::builder()` / `DirectiveBuilder`
- `ValueMatch` is now public, but wrapped to that we can freely extend and change it without braking changes

Workaround For: tokio-rs#1584, tokio-rs#1181
Fixes: tokio-rs#2507, tokio-rs#404
Refs: tokio-rs#1584, tokio-rs#1181
rustonaut added a commit to rustonaut/tracing that referenced this issue Jun 24, 2024
Motivation
----------

1. There are some filters which can not be created with the current Directive string repr syntax, like e.g. the strings `bool`, `1` or `2.3` or values containing characters including but not limited to `}],` (for some of this there are partial fixes in other PRS). Furthermore updating the syntax the accommodate this is always a potential braking change, one which also can be subtle to detect.

2. Sometimes there is need to create `Directives` programmatically, e.g. from a different kind of logging config or to set more complicated defaults.

3. We might want to add other matching capabilities in the future which might not be re presentable in the string syntax (or at least entail a braking change). Like e.g. matching fields regex, matching based on `dyn Value`, a field matching the string `true` and boolean `true` at the same time,  etc.

By allowing programmatic creation of `Directive`s we allow downstream users to work around existing issues, experiment with solution to such issues in external crates and in addition set the foundation for adding future matching capabilities in the future.

Solution
--------

- adds `Directive::builder()` / `DirectiveBuilder`
- `ValueMatch` is now public, but wrapped to that we can freely extend and change it without braking changes

Workaround For: tokio-rs#1584, tokio-rs#1181
Fixes: tokio-rs#2507, tokio-rs#404
Refs: tokio-rs#1584, tokio-rs#1181
@rustonaut rustonaut linked a pull request Jun 24, 2024 that will close this issue
rustonaut added a commit to rustonaut/tracing that referenced this issue Jun 24, 2024
Motivation
----------

1. There are some filters which can not be created with the current Directive string repr syntax, like e.g. the strings `bool`, `1` or `2.3` or values containing characters including but not limited to `}],` (for some of this there are partial fixes in other PRS). Furthermore updating the syntax the accommodate this is always a potential braking change, one which also can be subtle to detect.

2. Sometimes there is need to create `Directives` programmatically, e.g. from a different kind of logging config or to set more complicated defaults.

3. We might want to add other matching capabilities in the future which might not be re presentable in the string syntax (or at least entail a braking change). Like e.g. matching fields regex, matching based on `dyn Value`, a field matching the string `true` and boolean `true` at the same time,  etc.

By allowing programmatic creation of `Directive`s we allow downstream users to work around existing issues, experiment with solution to such issues in external crates and in addition set the foundation for adding future matching capabilities in the future.

Solution
--------

- adds `Directive::builder()` / `DirectiveBuilder`
- `ValueMatch` is now public, but wrapped to that we can freely extend and change it without braking changes

Workaround For: tokio-rs#1584, tokio-rs#1181
Fixes: tokio-rs#2507, tokio-rs#404
Refs: tokio-rs#1584, tokio-rs#1181
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 a pull request may close this issue.

2 participants