From 107ab37e0899a6e6e7d1baa19ec236729d3a2065 Mon Sep 17 00:00:00 2001 From: Joseph Perez Date: Wed, 6 Sep 2023 00:33:44 +0200 Subject: [PATCH] tracing: allow constant field names in macros (#2617) I've found myself in the case where I wanted to have customized event field name for different trait implementations. In fact, these implementations are completely unrelated (in separate applications), so, in this use case, I find more readable to have `foo="some_id"` and `bar=16` instead of `resource="foo" value="some_id"` and `resource=bar value=16` Because events only accept identifier or literal as field name, this is quite cumbersome/impossible to do. A simple solution could be to make events accept constant expression too; in my use case, I could then add a associated constant to my trait. This PR proposes a new syntax for using constant field names: ```rust tracing::debug!({ CONSTANT_EXPR } = "foo"); ``` This is the same syntax than constant expression, so it should be quite intuitive. To avoid constant expression names conflict, internal variables of macro expansion have been prefixed with `__`, e.g. `__CALLSITE`. Co-authored-by: Joseph Perez Co-authored-by: Eliza Weisman --- tracing/src/lib.rs | 13 +++ tracing/src/macros.rs | 190 ++++++++++++++++++++++++++--------------- tracing/tests/event.rs | 42 +++++++++ tracing/tests/span.rs | 30 +++++++ 4 files changed, 206 insertions(+), 69 deletions(-) diff --git a/tracing/src/lib.rs b/tracing/src/lib.rs index bcd0c6356c..679ca155ea 100644 --- a/tracing/src/lib.rs +++ b/tracing/src/lib.rs @@ -319,6 +319,19 @@ //! # } //!``` //! +//! Constant expressions can also be used as field names. Constants +//! must be enclosed in curly braces (`{}`) to indicate that the *value* +//! of the constant is to be used as the field name, rather than the +//! constant's name. For example: +//! ``` +//! # use tracing::{span, Level}; +//! # fn main() { +//! const RESOURCE_NAME: &str = "foo"; +//! // this span will have the field `foo = "some_id"` +//! span!(Level::TRACE, "get", { RESOURCE_NAME } = "some_id"); +//! # } +//!``` +//! //! The `?` sigil is shorthand that specifies a field should be recorded using //! its [`fmt::Debug`] implementation: //! ``` diff --git a/tracing/src/macros.rs b/tracing/src/macros.rs index b5f232bf0e..5aa2afe2e5 100644 --- a/tracing/src/macros.rs +++ b/tracing/src/macros.rs @@ -24,7 +24,7 @@ macro_rules! span { (target: $target:expr, parent: $parent:expr, $lvl:expr, $name:expr, $($fields:tt)*) => { { use $crate::__macro_support::Callsite as _; - static CALLSITE: $crate::callsite::DefaultCallsite = $crate::callsite2! { + static __CALLSITE: $crate::__macro_support::MacroCallsite = $crate::callsite2! { name: $name, kind: $crate::metadata::Kind::SPAN, target: $target, @@ -33,10 +33,10 @@ macro_rules! span { }; let mut interest = $crate::subscriber::Interest::never(); if $crate::level_enabled!($lvl) - && { interest = CALLSITE.interest(); !interest.is_never() } - && $crate::__macro_support::__is_enabled(CALLSITE.metadata(), interest) + && { interest = __CALLSITE.interest(); !interest.is_never() } + && $crate::__macro_support::__is_enabled(__CALLSITE.metadata(), interest) { - let meta = CALLSITE.metadata(); + let meta = __CALLSITE.metadata(); // span with explicit parent $crate::Span::child_of( $parent, @@ -44,9 +44,9 @@ macro_rules! span { &$crate::valueset!(meta.fields(), $($fields)*), ) } else { - let span = $crate::__macro_support::__disabled_span(CALLSITE.metadata()); + let span = $crate::__macro_support::__disabled_span(__CALLSITE.metadata()); $crate::if_log_enabled! { $lvl, { - span.record_all(&$crate::valueset!(CALLSITE.metadata().fields(), $($fields)*)); + span.record_all(&$crate::valueset!(__CALLSITE.metadata().fields(), $($fields)*)); }}; span } @@ -55,7 +55,7 @@ macro_rules! span { (target: $target:expr, $lvl:expr, $name:expr, $($fields:tt)*) => { { use $crate::__macro_support::Callsite as _; - static CALLSITE: $crate::callsite::DefaultCallsite = $crate::callsite2! { + static __CALLSITE: $crate::callsite::DefaultCallsite = $crate::callsite2! { name: $name, kind: $crate::metadata::Kind::SPAN, target: $target, @@ -64,19 +64,19 @@ macro_rules! span { }; let mut interest = $crate::subscriber::Interest::never(); if $crate::level_enabled!($lvl) - && { interest = CALLSITE.interest(); !interest.is_never() } - && $crate::__macro_support::__is_enabled(CALLSITE.metadata(), interest) + && { interest = __CALLSITE.interest(); !interest.is_never() } + && $crate::__macro_support::__is_enabled(__CALLSITE.metadata(), interest) { - let meta = CALLSITE.metadata(); + let meta = __CALLSITE.metadata(); // span with contextual parent $crate::Span::new( meta, &$crate::valueset!(meta.fields(), $($fields)*), ) } else { - let span = $crate::__macro_support::__disabled_span(CALLSITE.metadata()); + let span = $crate::__macro_support::__disabled_span(__CALLSITE.metadata()); $crate::if_log_enabled! { $lvl, { - span.record_all(&$crate::valueset!(CALLSITE.metadata().fields(), $($fields)*)); + span.record_all(&$crate::valueset!(__CALLSITE.metadata().fields(), $($fields)*)); }}; span } @@ -586,7 +586,7 @@ macro_rules! event { // Name / target / parent. (name: $name:expr, target: $target:expr, parent: $parent:expr, $lvl:expr, { $($fields:tt)* } )=> ({ use $crate::__macro_support::Callsite as _; - static CALLSITE: $crate::__macro_support::MacroCallsite = $crate::callsite2! { + static __CALLSITE: $crate::__macro_support::MacroCallsite = $crate::callsite2! { name: $name, kind: $crate::metadata::Kind::EVENT, target: $target, @@ -595,29 +595,29 @@ macro_rules! event { }; let enabled = $crate::level_enabled!($lvl) && { - let interest = CALLSITE.interest(); - !interest.is_never() && CALLSITE.is_enabled(interest) + let interest = __CALLSITE.interest(); + !interest.is_never() && $crate::__macro_support::__is_enabled(__CALLSITE.metadata(), interest) }; if enabled { (|value_set: $crate::field::ValueSet| { $crate::__tracing_log!( $lvl, - CALLSITE, + __CALLSITE, &value_set ); - let meta = CALLSITE.metadata(); + let meta = __CALLSITE.metadata(); // event with explicit parent $crate::Event::child_of( $parent, meta, &value_set ); - })($crate::valueset!(CALLSITE.metadata().fields(), $($fields)*)); + })($crate::valueset!(__CALLSITE.metadata().fields(), $($fields)*)); } else { $crate::__tracing_log!( $lvl, - CALLSITE, - &$crate::valueset!(CALLSITE.metadata().fields(), $($fields)*) + __CALLSITE, + &$crate::valueset!(__CALLSITE.metadata().fields(), $($fields)*) ); } }); @@ -640,7 +640,7 @@ macro_rules! event { // Name / target. (name: $name:expr, target: $target:expr, $lvl:expr, { $($fields:tt)* } )=> ({ use $crate::__macro_support::Callsite as _; - static CALLSITE: $crate::__macro_support::MacroCallsite = $crate::callsite2! { + static __CALLSITE: $crate::__macro_support::MacroCallsite = $crate::callsite2! { name: $name, kind: $crate::metadata::Kind::EVENT, target: $target, @@ -648,12 +648,12 @@ macro_rules! event { fields: $($fields)* }; let enabled = $crate::level_enabled!($lvl) && { - let interest = CALLSITE.interest(); - !interest.is_never() && CALLSITE.is_enabled(interest) + let interest = __CALLSITE.interest(); + !interest.is_never() && $crate::__macro_support::__is_enabled(__CALLSITE.metadata(), interest) }; if enabled { (|value_set: $crate::field::ValueSet| { - let meta = CALLSITE.metadata(); + let meta = __CALLSITE.metadata(); // event with contextual parent $crate::Event::dispatch( meta, @@ -661,15 +661,15 @@ macro_rules! event { ); $crate::__tracing_log!( $lvl, - CALLSITE, + __CALLSITE, &value_set ); - })($crate::valueset!(CALLSITE.metadata().fields(), $($fields)*)); + })($crate::valueset!(__CALLSITE.metadata().fields(), $($fields)*)); } else { $crate::__tracing_log!( $lvl, - CALLSITE, - &$crate::valueset!(CALLSITE.metadata().fields(), $($fields)*) + __CALLSITE, + &$crate::valueset!(__CALLSITE.metadata().fields(), $($fields)*) ); } }); @@ -691,7 +691,7 @@ macro_rules! event { // Target / parent. (target: $target:expr, parent: $parent:expr, $lvl:expr, { $($fields:tt)* } )=> ({ use $crate::__macro_support::Callsite as _; - static CALLSITE: $crate::callsite::DefaultCallsite = $crate::callsite2! { + static __CALLSITE: $crate::callsite::DefaultCallsite = $crate::callsite2! { name: $crate::__macro_support::concat!( "event ", file!(), @@ -705,29 +705,29 @@ macro_rules! event { }; let enabled = $crate::level_enabled!($lvl) && { - let interest = CALLSITE.interest(); - !interest.is_never() && $crate::__macro_support::__is_enabled(CALLSITE.metadata(), interest) + let interest = __CALLSITE.interest(); + !interest.is_never() && $crate::__macro_support::__is_enabled(__CALLSITE.metadata(), interest) }; if enabled { (|value_set: $crate::field::ValueSet| { $crate::__tracing_log!( $lvl, - CALLSITE, + __CALLSITE, &value_set ); - let meta = CALLSITE.metadata(); + let meta = __CALLSITE.metadata(); // event with explicit parent $crate::Event::child_of( $parent, meta, &value_set ); - })($crate::valueset!(CALLSITE.metadata().fields(), $($fields)*)); + })($crate::valueset!(__CALLSITE.metadata().fields(), $($fields)*)); } else { $crate::__tracing_log!( $lvl, - CALLSITE, - &$crate::valueset!(CALLSITE.metadata().fields(), $($fields)*) + __CALLSITE, + &$crate::valueset!(__CALLSITE.metadata().fields(), $($fields)*) ); } }); @@ -749,7 +749,7 @@ macro_rules! event { // Name / parent. (name: $name:expr, parent: $parent:expr, $lvl:expr, { $($fields:tt)* } )=> ({ use $crate::__macro_support::Callsite as _; - static CALLSITE: $crate::__macro_support::MacroCallsite = $crate::callsite2! { + static __CALLSITE: $crate::__macro_support::MacroCallsite = $crate::callsite2! { name: $name, kind: $crate::metadata::Kind::EVENT, target: module_path!(), @@ -758,29 +758,29 @@ macro_rules! event { }; let enabled = $crate::level_enabled!($lvl) && { - let interest = CALLSITE.interest(); - !interest.is_never() && CALLSITE.is_enabled(interest) + let interest = __CALLSITE.interest(); + !interest.is_never() && __CALLSITE.is_enabled(interest) }; if enabled { (|value_set: $crate::field::ValueSet| { $crate::__tracing_log!( $lvl, - CALLSITE, + __CALLSITE, &value_set ); - let meta = CALLSITE.metadata(); + let meta = __CALLSITE.metadata(); // event with explicit parent $crate::Event::child_of( $parent, meta, &value_set ); - })($crate::valueset!(CALLSITE.metadata().fields(), $($fields)*)); + })($crate::valueset!(__CALLSITE.metadata().fields(), $($fields)*)); } else { $crate::__tracing_log!( $lvl, - CALLSITE, - &$crate::valueset!(CALLSITE.metadata().fields(), $($fields)*) + __CALLSITE, + &$crate::valueset!(__CALLSITE.metadata().fields(), $($fields)*) ); } }); @@ -802,7 +802,7 @@ macro_rules! event { // Name. (name: $name:expr, $lvl:expr, { $($fields:tt)* } )=> ({ use $crate::__macro_support::Callsite as _; - static CALLSITE: $crate::__macro_support::MacroCallsite = $crate::callsite2! { + static __CALLSITE: $crate::__macro_support::MacroCallsite = $crate::callsite2! { name: $name, kind: $crate::metadata::Kind::EVENT, target: module_path!(), @@ -810,12 +810,12 @@ macro_rules! event { fields: $($fields)* }; let enabled = $crate::level_enabled!($lvl) && { - let interest = CALLSITE.interest(); - !interest.is_never() && CALLSITE.is_enabled(interest) + let interest = __CALLSITE.interest(); + !interest.is_never() && $crate::__macro_support::__is_enabled(__CALLSITE.metadata(), interest) }; if enabled { (|value_set: $crate::field::ValueSet| { - let meta = CALLSITE.metadata(); + let meta = __CALLSITE.metadata(); // event with contextual parent $crate::Event::dispatch( meta, @@ -823,15 +823,15 @@ macro_rules! event { ); $crate::__tracing_log!( $lvl, - CALLSITE, + __CALLSITE, &value_set ); - })($crate::valueset!(CALLSITE.metadata().fields(), $($fields)*)); + })($crate::valueset!(__CALLSITE.metadata().fields(), $($fields)*)); } else { $crate::__tracing_log!( $lvl, - CALLSITE, - &$crate::valueset!(CALLSITE.metadata().fields(), $($fields)*) + __CALLSITE, + &$crate::valueset!(__CALLSITE.metadata().fields(), $($fields)*) ); } }); @@ -852,7 +852,7 @@ macro_rules! event { // Target. (target: $target:expr, $lvl:expr, { $($fields:tt)* } )=> ({ use $crate::__macro_support::Callsite as _; - static CALLSITE: $crate::callsite::DefaultCallsite = $crate::callsite2! { + static __CALLSITE: $crate::callsite::DefaultCallsite = $crate::callsite2! { name: $crate::__macro_support::concat!( "event ", file!(), @@ -865,12 +865,12 @@ macro_rules! event { fields: $($fields)* }; let enabled = $crate::level_enabled!($lvl) && { - let interest = CALLSITE.interest(); - !interest.is_never() && $crate::__macro_support::__is_enabled(CALLSITE.metadata(), interest) + let interest = __CALLSITE.interest(); + !interest.is_never() && $crate::__macro_support::__is_enabled(__CALLSITE.metadata(), interest) }; if enabled { (|value_set: $crate::field::ValueSet| { - let meta = CALLSITE.metadata(); + let meta = __CALLSITE.metadata(); // event with contextual parent $crate::Event::dispatch( meta, @@ -878,15 +878,15 @@ macro_rules! event { ); $crate::__tracing_log!( $lvl, - CALLSITE, + __CALLSITE, &value_set ); - })($crate::valueset!(CALLSITE.metadata().fields(), $($fields)*)); + })($crate::valueset!(__CALLSITE.metadata().fields(), $($fields)*)); } else { $crate::__tracing_log!( $lvl, - CALLSITE, - &$crate::valueset!(CALLSITE.metadata().fields(), $($fields)*) + __CALLSITE, + &$crate::valueset!(__CALLSITE.metadata().fields(), $($fields)*) ); } }); @@ -1185,7 +1185,7 @@ macro_rules! enabled { (kind: $kind:expr, target: $target:expr, $lvl:expr, { $($fields:tt)* } )=> ({ if $crate::level_enabled!($lvl) { use $crate::__macro_support::Callsite as _; - static CALLSITE: $crate::callsite::DefaultCallsite = $crate::callsite2! { + static __CALLSITE: $crate::callsite::DefaultCallsite = $crate::callsite2! { name: $crate::__macro_support::concat!( "enabled ", file!(), @@ -1197,9 +1197,9 @@ macro_rules! enabled { level: $lvl, fields: $($fields)* }; - let interest = CALLSITE.interest(); - if !interest.is_never() && $crate::__macro_support::__is_enabled(CALLSITE.metadata(), interest) { - let meta = CALLSITE.metadata(); + let interest = __CALLSITE.interest(); + if !interest.is_never() && $crate::__macro_support::__is_enabled(__CALLSITE.metadata(), interest) { + let meta = __CALLSITE.metadata(); $crate::dispatcher::get_default(|current| current.enabled(meta)) } else { false @@ -2716,13 +2716,13 @@ macro_rules! callsite { target: $target, level: $lvl, fields: $crate::fieldset!( $($fields)* ), - callsite: &CALLSITE, + callsite: &__CALLSITE, kind: $kind, } }; - static CALLSITE: $crate::callsite::DefaultCallsite = $crate::callsite::DefaultCallsite::new(&META); - CALLSITE.register(); - &CALLSITE + static __CALLSITE: $crate::callsite::DefaultCallsite = $crate::callsite::DefaultCallsite::new(&META); + __CALLSITE.register(); + &__CALLSITE }}; } @@ -2766,7 +2766,7 @@ macro_rules! callsite2 { target: $target, level: $lvl, fields: $crate::fieldset!( $($fields)* ), - callsite: &CALLSITE, + callsite: &__CALLSITE, kind: $kind, } }; @@ -2920,6 +2920,47 @@ macro_rules! valueset { ) }; + // Handle constant names + (@ { $(,)* $($out:expr),* }, $next:expr, { $k:expr } = ?$val:expr, $($rest:tt)*) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&debug(&$val) as &dyn Value)) }, + $next, + $($rest)* + ) + }; + (@ { $(,)* $($out:expr),* }, $next:expr, { $k:expr } = %$val:expr, $($rest:tt)*) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&display(&$val) as &dyn Value)) }, + $next, + $($rest)* + ) + }; + (@ { $(,)* $($out:expr),* }, $next:expr, { $k:expr } = $val:expr, $($rest:tt)*) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&$val as &dyn Value)) }, + $next, + $($rest)* + ) + }; + (@ { $(,)* $($out:expr),* }, $next:expr, { $k:expr } = ?$val:expr) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&debug(&$val) as &dyn Value)) }, + $next, + ) + }; + (@ { $(,)* $($out:expr),* }, $next:expr, { $k:expr } = %$val:expr) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&display(&$val) as &dyn Value)) }, + $next, + ) + }; + (@ { $(,)* $($out:expr),* }, $next:expr, { $k:expr } = $val:expr) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&$val as &dyn Value)) }, + $next, + ) + }; + // Remainder is unparsable, but exists --- must be format args! (@ { $(,)* $($out:expr),* }, $next:expr, $($rest:tt)+) => { $crate::valueset!(@ { (&$next, ::core::option::Option::Some(&format_args!($($rest)+) as &dyn Value)), $($out),* }, $next, ) @@ -2989,6 +3030,17 @@ macro_rules! fieldset { $crate::fieldset!(@ { $($out),*, $k } $($rest)*) }; + // Handle constant names + (@ { $(,)* $($out:expr),* } { $k:expr } = ?$val:expr, $($rest:tt)*) => { + $crate::fieldset!(@ { $($out),*, $k } $($rest)*) + }; + (@ { $(,)* $($out:expr),* } { $k:expr } = %$val:expr, $($rest:tt)*) => { + $crate::fieldset!(@ { $($out),*, $k } $($rest)*) + }; + (@ { $(,)* $($out:expr),* } { $k:expr } = $val:expr, $($rest:tt)*) => { + $crate::fieldset!(@ { $($out),*, $k } $($rest)*) + }; + // Remainder is unparseable, but exists --- must be format args! (@ { $(,)* $($out:expr),* } $($rest:tt)+) => { $crate::fieldset!(@ { "message", $($out),*, }) diff --git a/tracing/tests/event.rs b/tracing/tests/event.rs index 5a0dc9ce4a..0be7c0bc56 100644 --- a/tracing/tests/event.rs +++ b/tracing/tests/event.rs @@ -510,3 +510,45 @@ fn string_field() { handle.assert_finished(); } + +#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)] +#[test] +fn constant_field_name() { + let expect_event = || { + expect::event().with_fields( + expect::field("foo") + .with_value(&"bar") + .and(expect::field("constant string").with_value(&"also works")) + .and(expect::field("foo.bar").with_value(&"baz")) + .and(expect::field("message").with_value(&debug(format_args!("quux")))) + .only(), + ) + }; + let (subscriber, handle) = subscriber::mock() + .event(expect_event()) + .event(expect_event()) + .only() + .run_with_handle(); + + with_default(subscriber, || { + const FOO: &str = "foo"; + tracing::event!( + Level::INFO, + { std::convert::identity(FOO) } = "bar", + { "constant string" } = "also works", + foo.bar = "baz", + "quux" + ); + tracing::event!( + Level::INFO, + { + { std::convert::identity(FOO) } = "bar", + { "constant string" } = "also works", + foo.bar = "baz", + }, + "quux" + ); + }); + + handle.assert_finished(); +} diff --git a/tracing/tests/span.rs b/tracing/tests/span.rs index d75f3a89a8..09f1be8954 100644 --- a/tracing/tests/span.rs +++ b/tracing/tests/span.rs @@ -836,3 +836,33 @@ fn both_shorthands() { handle.assert_finished(); } + +#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)] +#[test] +fn constant_field_name() { + let (subscriber, handle) = subscriber::mock() + .new_span( + expect::span().named("my_span").with_field( + expect::field("foo") + .with_value(&"bar") + .and(expect::field("constant string").with_value(&"also works")) + .and(expect::field("foo.bar").with_value(&"baz")) + .only(), + ), + ) + .only() + .run_with_handle(); + + with_default(subscriber, || { + const FOO: &str = "foo"; + tracing::span!( + Level::TRACE, + "my_span", + { std::convert::identity(FOO) } = "bar", + { "constant string" } = "also works", + foo.bar = "baz", + ); + }); + + handle.assert_finished(); +}