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

tracing override for #[instrument] #1818

Open
Tamschi opened this issue Jan 5, 2022 · 3 comments · May be fixed by #1819
Open

tracing override for #[instrument] #1818

Tamschi opened this issue Jan 5, 2022 · 3 comments · May be fixed by #1819
Labels
crate/attributes Related to the `tracing-attributes` crate kind/feature New feature or request

Comments

@Tamschi
Copy link
Contributor

Tamschi commented Jan 5, 2022

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
Copy link
Member

hawkw commented Jan 5, 2022

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.

@hawkw hawkw added crate/attributes Related to the `tracing-attributes` crate kind/feature New feature or request labels Jan 5, 2022
@Tamschi
Copy link
Contributor Author

Tamschi commented Jan 5, 2022

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
Copy link
Contributor Author

Tamschi commented Jan 5, 2022

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.

@Tamschi Tamschi linked a pull request Jan 5, 2022 that will close this issue
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate/attributes Related to the `tracing-attributes` crate kind/feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants