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: implement #[instrument(tracing)] #1819

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
78b4e37
add `#[instrument(tracing = Expr)]` parameter
Tamschi Jan 5, 2022
087e369
[WIP] implement `tracing` override for `#[instrument]`
Tamschi Jan 5, 2022
fabca1a
implement `tracing` override for `#[instrument]`
Tamschi Jan 8, 2022
aadd850
add the first set of `#[instrument(tracing)]` tests
Tamschi Jan 8, 2022
b7f105e
limit `#[instrument(tracing)]` to `syn::Path`s immediately, and docum…
Tamschi Jan 8, 2022
1684e54
link to symbols in `#[instrument(tracing)]` documentation
Tamschi Jan 8, 2022
b91ee93
add tests for remaining `#[instrument(tracing)]` code paths
Tamschi Jan 8, 2022
899714d
check `#[instrument(tracing, level)]` with a level ident, for good me…
Tamschi Jan 8, 2022
96f016f
Merge branch 'master' into instrument-tracing-override
davidbarsky Jan 11, 2022
40ad784
Merge branch 'master' into instrument-tracing-override
davidbarsky Jan 11, 2022
f4b31c5
Merge branch 'master' into instrument-tracing-override
Tamschi May 9, 2022
a185b25
Merge branch 'master' into instrument-tracing-override
Tamschi Sep 24, 2022
a17b04d
Merge branch 'master' into instrument-tracing-override
Tamschi Apr 1, 2023
3b5d4c3
Moved `ToTokens` implementation of `Level` to new `LevelWithTracing` …
Tamschi Apr 1, 2023
fc39c55
Merge branch 'master' into instrument-tracing-override
Tamschi Sep 23, 2024
12b854e
attributes: fix copy-paste mistake in error message
Tamschi Sep 23, 2024
3802e68
Merge branch 'master' into instrument-tracing-override
Tamschi Oct 3, 2024
72bc05c
Merge branch 'master' into instrument-tracing-override
Tamschi Nov 26, 2024
bc1789c
Merge branch 'master' into instrument-tracing-override
Tamschi Dec 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 35 additions & 12 deletions tracing-attributes/src/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub(crate) struct InstrumentArgs {
pub(crate) fields: Option<Fields>,
pub(crate) err_mode: Option<FormatMode>,
pub(crate) ret_mode: Option<FormatMode>,
tracing: Option<Expr>,
/// Errors describing any unrecognized parse inputs that we skipped.
parse_warnings: Vec<syn::Error>,
}
Expand All @@ -28,35 +29,36 @@ impl InstrumentArgs {
}
}

let tracing = self.tracing();
match &self.level {
Some(Level::Str(ref lit)) if lit.value().eq_ignore_ascii_case("trace") => {
quote!(tracing::Level::TRACE)
quote!(#tracing::Level::TRACE)
}
Some(Level::Str(ref lit)) if lit.value().eq_ignore_ascii_case("debug") => {
quote!(tracing::Level::DEBUG)
quote!(#tracing::Level::DEBUG)
}
Some(Level::Str(ref lit)) if lit.value().eq_ignore_ascii_case("info") => {
quote!(tracing::Level::INFO)
quote!(#tracing::Level::INFO)
}
Some(Level::Str(ref lit)) if lit.value().eq_ignore_ascii_case("warn") => {
quote!(tracing::Level::WARN)
quote!(#tracing::Level::WARN)
}
Some(Level::Str(ref lit)) if lit.value().eq_ignore_ascii_case("error") => {
quote!(tracing::Level::ERROR)
quote!(#tracing::Level::ERROR)
}
Some(Level::Int(ref lit)) if is_level(lit, 1) => quote!(tracing::Level::TRACE),
Some(Level::Int(ref lit)) if is_level(lit, 2) => quote!(tracing::Level::DEBUG),
Some(Level::Int(ref lit)) if is_level(lit, 3) => quote!(tracing::Level::INFO),
Some(Level::Int(ref lit)) if is_level(lit, 4) => quote!(tracing::Level::WARN),
Some(Level::Int(ref lit)) if is_level(lit, 5) => quote!(tracing::Level::ERROR),
Some(Level::Int(ref lit)) if is_level(lit, 1) => quote!(#tracing::Level::TRACE),
Some(Level::Int(ref lit)) if is_level(lit, 2) => quote!(#tracing::Level::DEBUG),
Some(Level::Int(ref lit)) if is_level(lit, 3) => quote!(#tracing::Level::INFO),
Some(Level::Int(ref lit)) if is_level(lit, 4) => quote!(#tracing::Level::WARN),
Some(Level::Int(ref lit)) if is_level(lit, 5) => quote!(#tracing::Level::ERROR),
Some(Level::Path(ref pat)) => quote!(#pat),
Some(_) => quote! {
compile_error!(
"unknown verbosity level, expected one of \"trace\", \
\"debug\", \"info\", \"warn\", or \"error\", or a number 1-5"
)
},
None => quote!(tracing::Level::INFO),
None => quote!(#tracing::Level::INFO),
}
}

Expand All @@ -68,6 +70,17 @@ impl InstrumentArgs {
}
}

/// The location of the `tracing` crate.
///
/// For backwards-compatibility, this must default to the plain `tracing` identifier with call site resolution.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure if this should go into the doc comment, on second thought. A comment above the quote!(tracing) may be more apt.

I'd actually like to suggest adding one of those breaking change comments there: A default of quote!(::tracing) would avoid collisions with consumer-defined names just about entirely by default, but unfortunately it would also break the "hidden override" that's theoretically available right now.

pub(crate) fn tracing(&self) -> impl ToTokens {
if let Some(ref tracing) = self.tracing {
quote!(#tracing)
} else {
quote!(tracing)
}
}

/// Generate "deprecation" warnings for any unrecognized attribute inputs
/// that we skipped.
///
Expand Down Expand Up @@ -146,6 +159,14 @@ impl Parse for InstrumentArgs {
let _ = input.parse::<kw::ret>()?;
let mode = FormatMode::parse(input)?;
args.ret_mode = Some(mode);
} else if lookahead.peek(kw::tracing) {
if args.tracing.is_some() {
return Err(input.error("expected only a single `fields` argument"));
}
let _ = input.parse::<kw::tracing>()?;
let _ = input.parse::<Token![=]>()?;
let tracing = Expr::parse(input)?;
args.tracing = Some(tracing);
} else if lookahead.peek(Token![,]) {
let _ = input.parse::<Token![,]>()?;
} else {
Expand Down Expand Up @@ -313,7 +334,8 @@ impl ToTokens for Field {
// `instrument` produce empty field values, so changing it now
// is a breaking change. agh.
let name = &self.name;
tokens.extend(quote!(#name = tracing::field::Empty))
let tracing = todo!();
tokens.extend(quote!(#name = #tracing::field::Empty))
Tamschi marked this conversation as resolved.
Show resolved Hide resolved
} else {
self.kind.to_tokens(tokens);
self.name.to_tokens(tokens);
Expand Down Expand Up @@ -363,4 +385,5 @@ mod kw {
syn::custom_keyword!(name);
syn::custom_keyword!(err);
syn::custom_keyword!(ret);
syn::custom_keyword!(tracing);
}
17 changes: 9 additions & 8 deletions tracing-attributes/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ fn gen_block<B: ToTokens>(
.unwrap_or_else(|| quote!(#instrumented_function_name));

let level = args.level();
let tracing = args.tracing();

// generate this inside a closure, so we can return early on errors.
let span = (|| {
Expand Down Expand Up @@ -156,7 +157,7 @@ fn gen_block<B: ToTokens>(
})
.map(|(user_name, (real_name, record_type))| match record_type {
RecordType::Value => quote!(#user_name = #real_name),
RecordType::Debug => quote!(#user_name = tracing::field::debug(&#real_name)),
RecordType::Debug => quote!(#user_name = #tracing::field::debug(&#real_name)),
})
.collect();

Expand All @@ -180,7 +181,7 @@ fn gen_block<B: ToTokens>(

let custom_fields = &args.fields;

quote!(tracing::span!(
quote!(#tracing::span!(
target: #target,
#level,
#span_name,
Expand All @@ -192,16 +193,16 @@ fn gen_block<B: ToTokens>(

let err_event = match args.err_mode {
Some(FormatMode::Default) | Some(FormatMode::Display) => {
Some(quote!(tracing::error!(error = %e)))
Some(quote!(#tracing::error!(error = %e)))
}
Some(FormatMode::Debug) => Some(quote!(tracing::error!(error = ?e))),
Some(FormatMode::Debug) => Some(quote!(#tracing::error!(error = ?e))),
_ => None,
};

let ret_event = match args.ret_mode {
Some(FormatMode::Display) => Some(quote!(tracing::event!(#level, return = %x))),
Some(FormatMode::Display) => Some(quote!(#tracing::event!(#level, return = %x))),
Some(FormatMode::Default) | Some(FormatMode::Debug) => {
Some(quote!(tracing::event!(#level, return = ?x)))
Some(quote!(#tracing::event!(#level, return = ?x)))
}
_ => None,
};
Expand Down Expand Up @@ -258,7 +259,7 @@ fn gen_block<B: ToTokens>(
let __tracing_attr_span = #span;
let __tracing_instrument_future = #mk_fut;
if !__tracing_attr_span.is_disabled() {
tracing::Instrument::instrument(
#tracing::Instrument::instrument(
__tracing_instrument_future,
__tracing_attr_span
)
Expand All @@ -282,7 +283,7 @@ fn gen_block<B: ToTokens>(
// regression in case the level is enabled.
let __tracing_attr_span;
let __tracing_attr_guard;
if tracing::level_enabled!(#level) {
if #tracing::level_enabled!(#level) {
__tracing_attr_span = #span;
__tracing_attr_guard = __tracing_attr_span.enter();
}
Expand Down