tokio-rs / tracing

Application level tracing for Rust.
https://tracing.rs
MIT License
5.5k stars 722 forks source link

`tracing` override for `#[instrument]` #1818

Open Tamschi opened 2 years ago

Tamschi commented 2 years ago

Feature Request

Crates

tracing (more specifically tracing_attributes only)

Motivation

As the #[instrument] macro is unable to use $crate, it currently (unhygienically) grabs tracing from the locally available names. This generally works fine if the crate where that attribute ends up actually depends on tracing directly, but it fails when tracing is only a transitive dependency and has not been imported.

Proposal

I would like to see/implement an additional parameter for #[instrument] like this:

#[instrument(tracing = some_path)]

where some_path is a path expression leading to the tracing module.

When no tracing argument is available, some_path should default to just tracing with call site resolution, as before.

The hygiene information on those path tokens should be preserved to make it easier to use in other macros, but this is the default when pasting into a quote!(…) anyway, so I don't see a reason not to handle it like that.

Alternatives

Currently, my documentation says to add tracing = { version = "0.1", default-features = false } to the consumer's dependencies to use the "tracing" feature, which is less than ideal because the crate that uses my macro could be a library not otherwise depending on tracing itself, and would then have to expose that as feature in turn in order to not be outright incompatible with crates that use the "tracing" feature in mine.

I could also emit a use … as tracing; import statement, but this adds an item to the consumer-visible namespace and can lead to unexpected collisions (or shadowing, if I rewrite some of my code to scope it).

hawkw commented 2 years ago

This generally works fine if the crate where that attribute ends up actually depends on tracing directly, but it fails when tracing is only a transitive dependency and has not been imported.

Hmm, i'm quite curious about when this would actually occur. The main scenarios I can think of is when you have a macro that expands to code including a #[tracing::instrument] attribute in a downstream crate, or if tracing is imported via a renaming import in Cargo.toml. Are you doing one of those things? I just want to make sure I understand the use-case for this change.

I think adding something like this makes sense, although it seems relatively niche. Are you interested in opening a PR to add it? I'm happy to provide advice.

Tamschi commented 2 years ago

The main scenarios I can think of is when you have a macro that expands to code including a #[tracing::instrument] attribute in a downstream crate, […]

Yes, it's that case. I wrote a web frontend framework that can be used to generate MVC component implementations like this (though most are more concise than that right now, since I'm the only user and only practically using it as static site generator at the moment).

The problematic case appears when I try to write component libraries or unit tests and enable the tracing feature. (The attribute is unconditional, but I replace the tracing with a stand-in that does nothing if the "asteracea/tracing" features isn't enabled.)


(more context on usage)

The generated components have ::new and .render functions each that I optionally instrument with tracing. It gives me a flame graph with default browser tooling out of the box if I use tracing-wasm and I think I can eventually capture the spans for error backtraces in Wasm too.

The way component arguments are passed to ::new and .render isn't entirely straightforward because I'm emulating named parameters. I'd also like to duck-type the output depending on whether individual parameters are Debug, which should be straightforward but comes with a little boilerplate.

I think it makes sense for me to generate the #[instrument] attribute on my end for those reasons, and also because I can add the component name to the span name = this way.

Tamschi commented 2 years ago

I'll try to make a PR some time this week. I think the one aspect I'm not entirely clear about is how you parse the arguments right now, but I haven't looked through the source code too thoroughly.

Edit: I found it. I'll make sure to add relevant tests and documentation, too.