tracing

tracing copied to clipboard

Feature Request

Crates

• tracing-subscriber

Motivation

Right now the interface for setting up only allows for setting up complex directives with more than just a Level via the parse interface. It should be possible to add alternative methods of constructing Directives such as from tuples, via a constructor, etc.

Proposal

Add a new from impl for Directives impl From<(&str, Level)> for Directive so that the following becomes valid.

filter.add_directive((module_path!(), Level::INFO).into());

Alternatives

The main goal of this change is to make it easier to setup a logger where it gets the filter from the RUST_LOG env variable, or defaults to some level otherwise. A function that directly addresses this need could function as an alternative to this UI improvement. This would also make it possible to use Into<Directive> as the argument type rather than Directive, which I believe would be a breaking change if applied to the add_directive function.

let filter = EnvFilter::from_default_env_or((module_path!(), Level::INFO));

Add a new from impl for Directives impl From<(&str, Level)> for Directive so that the following becomes valid.

filter.add_directive((module_path!(), Level::INFO).into());

I'm not convinced that just adding a From<(&str, Level)> constructor is the best approach for adding programmatic filter construction. Bear in mind that a filtering directive might also include a span name or field matching expression. We might also want to consider adding a directive builder API, so you could say something like this

filter.add_directive(
    Level::INFO
        .with_target(module_path!())
        .in_span("foo")
        .with_field("bar=1")
);

This would also make it possible to use Into<Directive> as the argument type rather than Directive, which I believe would be a breaking change if applied to the add_directive function.

If memory serves, the reason add_directive takes Directive rather than Into<Directive> is because taking a Directive allows code like

filter.add_directive("foo=trace".parse()?);

whereas if the method took Into<Directive>, the type for parse couldn't be inferred, so you'd have to say

filter.add_directive("foo=trace".parse::<Directive>()?);

If we add APIs for constructing directives programmatically, I think ergonomics for parsing them from strings in add_directive is less of a priority, so I'd be willing to consider changing the argument to add_directive to take Into<Directive>. Since we're hoping to get a tracing-subscriber 0.2 release out in the near future, we could do the breaking change in that release.

Alternatively we could have add_directive use TryFrom which should make it possible to write the following

filter.add_directive("foo=trace");

filter.try_add_directive("foo=trace")?;

In this world I'm imagining add_directive would be a panicking function where as try_add_directive would forward the conversion error back to the caller. I'm very happy with the idea for builder patterns from Levels though, so this is just me tossing ideas around not really asserting that I like this more.

Personally, I'd prefer not to have an infallible (panicking) add_directive and a fallible try_add_directive, I'd prefer to just put the decision of whether to bubble up the error or panic at the call site...

I'm not sure what you mean by "putting the decision at the call site"

@yaahc I think it's better to have one add_directive that's infallible and does not panic, and let the user use ? or expect/unwrap if they want to call it with something fallible, instead of having an add_directive that might panic and a try_add_directive that returns a Result. IMO, the user is constructing the directive that's being passed in, they should be able to decide how to handle it if that construction fails at the site where that directive is being constructed, rather than based on what API method it's passed into.

I think we should probably make a separate issue for potential changes to the add_directive API --- adding a programmatic builder for filters is not a breaking change, but reworking add_directive probably is.

I have a need to build these since I have a custom logging interface which has a map of module path -> Level, and turning that into a string just to then re-parse it moments later is a bit silly.

Even just adding a Directive::new function would fix that particular issue (but maybe isn't the most desirable) - but I'd be happy to contribute something that allows building them without parsing, and doesn't change the add_directive API.

@Sushisource I would be happy to merge any PRs you open for this, and if you have any questions or need guidance, please let me know! :)

I added Directive::builder() which in combination with the already existing EnvFilter::add_directive() should resolve this issue.

PR: https://github.com/tokio-rs/tracing/pull/3018