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

attributes: make #[instrument] support string literal fields #2924

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

svix-aaron1011
Copy link

Motivation

The 'tracing::span!' macro supports passing in field names as string literals, which can be used to use an invalid Rust identifier as a field name (e.g. "type"). However, the 'instrument!' macro required that arguments to 'fields' be period-separated Rust identifiers.

Solution

This commit extends the #[instrument] macro to support both period-separated identifiers and single string-literal fields. We preserve the original argument and pass it to the tracing::span! macro to preserve span/hygiene information.

Fixes #2438

The 'tracing::span!' macro supports passing in field names as string
literals, which can be used to use an invalid Rust identifier as
a field name (e.g. "type"). However, the 'instrument!' macro required
that arguments to 'fields' be period-separated Rust identifiers.

This commit extends the `#[instrument]` macro to support both
period-separated identifiers and single string-literal fields.
We preserve the original argument and pass it to the `tracing::span!`
macro to preserve span/hygiene information.
@svix-aaron1011 svix-aaron1011 requested review from hawkw, davidbarsky and a team as code owners April 2, 2024 16:45
@svix-jplatte
Copy link
Contributor

svix-jplatte commented Apr 3, 2024

I found it weird that this would be required and checked why foo.type didn't work without quoting in the first place. Turns out the proc-macro code is actually fine (it uses parse_any) to allow keywords, but one internal macro_rules! that was used by the proc-macro required the field path expression to match the expr fragment-specifier. #2925 fixes that and should thus supersede this PR (though maybe it doesn't hurt to support both if the span! macro also supports literals?).

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 this pull request may close these issues.

tracing::instrument doesn't support fields as strings or raw identifiers when keywords are involved
2 participants