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.

Sign up for GitHub

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

Jump to bottom

macros: Handle constant field names #2617

Merged
merged 8 commits into from
Sep 5, 2023
Merged

macros: Handle constant field names #2617

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
48495eb
macros: Handle constant field names
Jun 15, 2023
a3cc393
Add documentation
Jun 15, 2023
38a7232
doc: fix typo
wyfo Jun 22, 2023
8f3d935
macros: refine constant field names tests
Jun 22, 2023
a680b4c
expand doc comment
hawkw Sep 5, 2023
cb5b499
Merge branch 'master' into constant_field_name
hawkw Sep 5, 2023
5d65b45
rename callsite in new macro cases from #2699
hawkw Sep 5, 2023
5092c87
rustfmt
hawkw Sep 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions tracing/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,16 @@
//! # }
//!```
//!
//! Constant expressions can also be used as field name.
wyfo marked this conversation as resolved.
Show resolved Hide resolved
//! ```
//! # 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:
//! ```
Expand Down
138 changes: 95 additions & 43 deletions tracing/src/macros.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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::__macro_support::MacroCallsite = $crate::callsite2! {
static __CALLSITE: $crate::__macro_support::MacroCallsite = $crate::callsite2! {
name: $name,
kind: $crate::metadata::Kind::SPAN,
target: $target,
Expand All @@ -33,20 +33,20 @@ macro_rules! span {
};
let mut interest = $crate::collect::Interest::never();
if $crate::level_enabled!($lvl)
&& { interest = CALLSITE.interest(); !interest.is_never() }
&& CALLSITE.is_enabled(interest)
&& { interest = __CALLSITE.interest(); !interest.is_never() }
&& __CALLSITE.is_enabled(interest)
{
let meta = CALLSITE.metadata();
let meta = __CALLSITE.metadata();
// span with explicit parent
$crate::Span::child_of(
$parent,
meta,
&$crate::valueset!(meta.fields(), $($fields)*),
)
} else {
let span = CALLSITE.disabled_span();
let span = __CALLSITE.disabled_span();
$crate::if_log_enabled! { $lvl, {
span.record_all(&$crate::valueset!(CALLSITE.metadata().fields(), $($fields)*));
span.record_all(&$crate::valueset!(__CALLSITE.metadata().fields(), $($fields)*));
}};
span
}
Expand All @@ -55,7 +55,7 @@ macro_rules! span {
(target: $target:expr, $lvl:expr, $name:expr, $($fields:tt)*) => {
{
use $crate::__macro_support::{Callsite as _, Registration};
static CALLSITE: $crate::__macro_support::MacroCallsite = $crate::callsite2! {
static __CALLSITE: $crate::__macro_support::MacroCallsite = $crate::callsite2! {
name: $name,
kind: $crate::metadata::Kind::SPAN,
target: $target,
Expand All @@ -65,19 +65,19 @@ macro_rules! span {

let mut interest = $crate::collect::Interest::never();
if $crate::level_enabled!($lvl)
&& { interest = CALLSITE.interest(); !interest.is_never() }
&& CALLSITE.is_enabled(interest)
&& { interest = __CALLSITE.interest(); !interest.is_never() }
&& __CALLSITE.is_enabled(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 = CALLSITE.disabled_span();
let span = __CALLSITE.disabled_span();
$crate::if_log_enabled! { $lvl, {
span.record_all(&$crate::valueset!(CALLSITE.metadata().fields(), $($fields)*));
span.record_all(&$crate::valueset!(__CALLSITE.metadata().fields(), $($fields)*));
}};
span
}
Expand Down Expand Up @@ -585,7 +585,7 @@ macro_rules! error_span {
macro_rules! event {
(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: concat!(
"event ",
file!(),
Expand All @@ -599,29 +599,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)*)
);
}
});
Expand All @@ -642,7 +642,7 @@ macro_rules! event {
);
(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: concat!(
"event ",
file!(),
Expand All @@ -655,28 +655,28 @@ 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() && __CALLSITE.is_enabled(interest)
};
if enabled {
(|value_set: $crate::field::ValueSet| {
let meta = CALLSITE.metadata();
let meta = __CALLSITE.metadata();
// event with contextual parent
$crate::Event::dispatch(
meta,
&value_set
);
$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)*)
);
}
});
Expand Down Expand Up @@ -968,7 +968,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::__macro_support::MacroCallsite = $crate::callsite2! {
static __CALLSITE: $crate::__macro_support::MacroCallsite = $crate::callsite2! {
name: concat!(
"enabled ",
file!(),
Expand All @@ -980,9 +980,9 @@ macro_rules! enabled {
level: $lvl,
fields: $($fields)*
};
let interest = CALLSITE.interest();
if !interest.is_never() && CALLSITE.is_enabled(interest) {
let meta = CALLSITE.metadata();
let interest = __CALLSITE.interest();
if !interest.is_never() && __CALLSITE.is_enabled(interest) {
let meta = __CALLSITE.metadata();
$crate::dispatch::get_default(|current| current.enabled(meta))
} else {
false
Expand Down Expand Up @@ -2100,20 +2100,20 @@ macro_rules! callsite {
fields: $($fields:tt)*
) => {{
use $crate::__macro_support::{MacroCallsite, Registration};
static META: $crate::Metadata<'static> = {
static __META: $crate::Metadata<'static> = {
$crate::metadata! {
name: $name,
target: $target,
level: $lvl,
fields: $crate::fieldset!( $($fields)* ),
callsite: &CALLSITE,
callsite: &__CALLSITE,
kind: $kind,
}
};
static REG: Registration = Registration::new(&CALLSITE);
static CALLSITE: MacroCallsite = MacroCallsite::new(&META, &REG);
CALLSITE.register();
&CALLSITE
static REG: Registration = Registration::new(&__CALLSITE);
static __CALLSITE: MacroCallsite = MacroCallsite::new(&__META, &REG);
__CALLSITE.register();
&__CALLSITE
}};
}

Expand Down Expand Up @@ -2152,19 +2152,19 @@ macro_rules! callsite2 {
fields: $($fields:tt)*
) => {{
use $crate::__macro_support::{MacroCallsite, Registration};
static META: $crate::Metadata<'static> = {
static __META: $crate::Metadata<'static> = {
$crate::metadata! {
name: $name,
target: $target,
level: $lvl,
fields: $crate::fieldset!( $($fields)* ),
callsite: &CALLSITE,
callsite: &__CALLSITE,
kind: $kind,
}
};
static REG: Registration = Registration::new(&CALLSITE);
static REG: Registration = Registration::new(&__CALLSITE);

MacroCallsite::new(&META, &REG)
MacroCallsite::new(&__META, &REG)
}};
}

Expand Down Expand Up @@ -2314,6 +2314,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, Some(&format_args!($($rest)+) as &dyn Value)), $($out),* }, $next, )
Expand Down Expand Up @@ -2383,6 +2424,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),*, })
Expand Down Expand Up @@ -2436,7 +2488,7 @@ macro_rules! __tracing_log {
if level <= log::max_level() {
let log_meta = log::Metadata::builder()
.level(level)
.target(CALLSITE.metadata().target())
.target(__CALLSITE.metadata().target())
.build();
let logger = log::logger();
if logger.enabled(&log_meta) {
Expand Down
15 changes: 15 additions & 0 deletions tracing/tests/event.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,3 +401,18 @@ fn string_field() {

handle.assert_finished();
}

#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)]
#[test]
fn constant_field_name() {
let (collector, handle) = collector::mock()
.event(expect::event().with_fields(expect::field("foo").with_value(&"bar").only()))
.only()
.run_with_handle();
with_default(collector, || {
const FOO: &str = "foo";
tracing::event!(Level::INFO, { FOO } = "bar");
Copy link
Member

Choose a reason for hiding this comment

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

can we also add a test which mixes constant and non-constant field names? i'd like to ensure that the macro parses unambiguously when the two forms are mixed.

Copy link
Member

Choose a reason for hiding this comment

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

we may also want to test constant field names that are not in the curren scope, like { some_module::FOO } and { some_module::SomeType::FOO }?

});

handle.assert_finished();
}
19 changes: 19 additions & 0 deletions tracing/tests/span.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -831,3 +831,22 @@ fn both_shorthands() {

handle.assert_finished();
}

#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)]
#[test]
fn constant_field_name() {
let (collector, handle) = collector::mock()
.new_span(
expect::span()
.named("my_span")
.with_field(expect::field("foo").with_value(&"bar").only()),
)
.only()
.run_with_handle();
with_default(collector, || {
const FOO: &str = "foo";
tracing::span!(Level::TRACE, "my_span", { FOO } = "bar");
Copy link
Member

Choose a reason for hiding this comment

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

similarly, let's also test some spans with mixed constant and ident field names

});

handle.assert_finished();
}