Skip to content

Commit

Permalink
tracing: fix event macros with constant field names in the first posi…
Browse files Browse the repository at this point in the history 
…tion (#2883)


## Motivation

Const argumetns in `level!` macros do not work when in the first
position.

This also seems to have fixed #2748 where literals for fields names like
`info!("foo" = 2)` could not be used outside the `event!` macro.


Fixes #2837
Fixes #2738

## Solution

Previsously, `level!($(args:tt))` was forwarded to `event!(target: ...,
level: ..., { $(args:tt) })` but the added curly braces seem to have
prevented the `event` macro from correctly understanding the arguments
and it tried to pass them to `format!`.

With this change there may have some performance impact when expanding
the macros in most cases where the braces could have been added as it
will take one more step.

These are the two relevant `event!` blocks I believe, the new tests used
to expand to the first one (with empty fields), now they expand to the
latter:
```
    (target: $target:expr, $lvl:expr, { $($fields:tt)* }, $($arg:tt)+ ) => (
        $crate::event!(
            target: $target,
            $lvl,
            { message = $crate::__macro_support::format_args!($($arg)+), $($fields)* }
        )
    );
    (target: $target:expr, $lvl:expr, $($arg:tt)+ ) => (
        $crate::event!(target: $target, $lvl, { $($arg)+ })
    );
```
  • Loading branch information
@mladedav
mladedav authored Mar 1, 2024
1 parent 0e3577f commit 908cc43
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 6 deletions.
5 changes: 0 additions & 5 deletions tracing/src/macros.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1545,7 +1545,6 @@ macro_rules! trace {
$crate::event!(
target: module_path!(),
$crate::Level::TRACE,
{},
$($arg)+
)
);
Expand Down Expand Up @@ -1822,7 +1821,6 @@ macro_rules! debug {
$crate::event!(
target: module_path!(),
$crate::Level::DEBUG,
{},
$($arg)+
)
);
Expand Down Expand Up @@ -2110,7 +2108,6 @@ macro_rules! info {
$crate::event!(
target: module_path!(),
$crate::Level::INFO,
{},
$($arg)+
)
);
Expand Down Expand Up @@ -2391,7 +2388,6 @@ macro_rules! warn {
$crate::event!(
target: module_path!(),
$crate::Level::WARN,
{},
$($arg)+
)
);
Expand Down Expand Up @@ -2668,7 +2664,6 @@ macro_rules! error {
$crate::event!(
target: module_path!(),
$crate::Level::ERROR,
{},
$($arg)+
)
);
Expand Down
54 changes: 54 additions & 0 deletions tracing/tests/event.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,12 @@ fn constant_field_name() {
)
};
let (collector, handle) = collector::mock()
.event(expect_event())
.event(expect_event())
.event(expect_event())
.event(expect_event())
.event(expect_event())
.event(expect_event())
.event(expect_event())
.event(expect_event())
.only()
Expand All @@ -439,6 +445,54 @@ fn constant_field_name() {
},
"quux"
);
tracing::info!(
{ std::convert::identity(FOO) } = "bar",
{ "constant string" } = "also works",
foo.bar = "baz",
"quux"
);
tracing::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"
);
tracing::event!(
Level::INFO,
{
{ std::convert::identity(FOO) } = "bar",
{ "constant string" } = "also works",
foo.bar = "baz",
},
"{}",
"quux"
);
tracing::info!(
{ std::convert::identity(FOO) } = "bar",
{ "constant string" } = "also works",
foo.bar = "baz",
"{}",
"quux"
);
tracing::info!(
{
{ std::convert::identity(FOO) } = "bar",
{ "constant string" } = "also works",
foo.bar = "baz",
},
"{}",
"quux"
);
});

handle.assert_finished();
Expand Down
32 changes: 31 additions & 1 deletion tracing/tests/macros.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -555,12 +555,15 @@ fn trace() {
trace!(foo = ?3, bar.baz = %2, quux = false);
trace!(foo = 3, bar.baz = 2, quux = false);
trace!(foo = 3, bar.baz = 3,);
trace!("foo" = 3, bar.baz = 3,);
trace!(foo = 3, "bar.baz" = 3,);
trace!("foo");
trace!("foo: {}", 3);
trace!(foo = ?3, bar.baz = %2, quux = false, "hello world {:?}", 42);
trace!(foo = 3, bar.baz = 2, quux = false, "hello world {:?}", 42);
trace!(foo = 3, bar.baz = 3, "hello world {:?}", 42,);
trace!({ foo = 3, bar.baz = 80 }, "quux");
trace!({ "foo" = 3, "bar.baz" = 80 }, "quux");
trace!({ foo = 2, bar.baz = 79 }, "quux {:?}", true);
trace!({ foo = 2, bar.baz = 79 }, "quux {:?}, {quux}", true, quux = false);
trace!({ foo = 2, bar.baz = 78 }, "quux");
Expand All @@ -579,6 +582,9 @@ fn trace() {
trace!(?foo);
trace!(%foo);
trace!(foo);
trace!("foo" = ?foo);
trace!("foo" = %foo);
trace!("foo" = foo);
trace!(name: "foo", ?foo);
trace!(name: "foo", %foo);
trace!(name: "foo", foo);
Expand Down Expand Up @@ -615,12 +621,15 @@ fn debug() {
debug!(foo = ?3, bar.baz = %2, quux = false);
debug!(foo = 3, bar.baz = 2, quux = false);
debug!(foo = 3, bar.baz = 3,);
debug!("foo" = 3, bar.baz = 3,);
debug!(foo = 3, "bar.baz" = 3,);
debug!("foo");
debug!("foo: {}", 3);
debug!(foo = ?3, bar.baz = %2, quux = false, "hello world {:?}", 42);
debug!(foo = 3, bar.baz = 2, quux = false, "hello world {:?}", 42);
debug!(foo = 3, bar.baz = 3, "hello world {:?}", 42,);
debug!({ foo = 3, bar.baz = 80 }, "quux");
debug!({ "foo" = 3, "bar.baz" = 80 }, "quux");
debug!({ foo = 2, bar.baz = 79 }, "quux {:?}", true);
debug!({ foo = 2, bar.baz = 79 }, "quux {:?}, {quux}", true, quux = false);
debug!({ foo = 2, bar.baz = 78 }, "quux");
Expand All @@ -639,6 +648,9 @@ fn debug() {
debug!(?foo);
debug!(%foo);
debug!(foo);
debug!("foo" = ?foo);
debug!("foo" = %foo);
debug!("foo" = foo);
debug!(name: "foo", ?foo);
debug!(name: "foo", %foo);
debug!(name: "foo", foo);
Expand Down Expand Up @@ -675,12 +687,15 @@ fn info() {
info!(foo = ?3, bar.baz = %2, quux = false);
info!(foo = 3, bar.baz = 2, quux = false);
info!(foo = 3, bar.baz = 3,);
info!("foo" = 3, bar.baz = 3,);
info!(foo = 3, "bar.baz" = 3,);
info!("foo");
info!("foo: {}", 3);
info!(foo = ?3, bar.baz = %2, quux = false, "hello world {:?}", 42);
info!(foo = 3, bar.baz = 2, quux = false, "hello world {:?}", 42);
info!(foo = 3, bar.baz = 3, "hello world {:?}", 42,);
info!({ foo = 3, bar.baz = 80 }, "quux");
info!({ "foo" = 3, "bar.baz" = 80 }, "quux");
info!({ foo = 2, bar.baz = 79 }, "quux {:?}", true);
info!({ foo = 2, bar.baz = 79 }, "quux {:?}, {quux}", true, quux = false);
info!({ foo = 2, bar.baz = 78 }, "quux");
Expand All @@ -699,6 +714,9 @@ fn info() {
info!(?foo);
info!(%foo);
info!(foo);
info!("foo" = ?foo);
info!("foo" = %foo);
info!("foo" = foo);
info!(name: "foo", ?foo);
info!(name: "foo", %foo);
info!(name: "foo", foo);
Expand Down Expand Up @@ -735,12 +753,15 @@ fn warn() {
warn!(foo = ?3, bar.baz = %2, quux = false);
warn!(foo = 3, bar.baz = 2, quux = false);
warn!(foo = 3, bar.baz = 3,);
warn!("foo" = 3, bar.baz = 3,);
warn!(foo = 3, "bar.baz" = 3,);
warn!("foo");
warn!("foo: {}", 3);
warn!(foo = ?3, bar.baz = %2, quux = false, "hello world {:?}", 42);
warn!(foo = 3, bar.baz = 2, quux = false, "hello world {:?}", 42);
warn!(foo = 3, bar.baz = 3, "hello world {:?}", 42,);
warn!({ foo = 3, bar.baz = 80 }, "quux");
warn!({ "foo" = 3, "bar.baz" = 80 }, "quux");
warn!({ foo = 2, bar.baz = 79 }, "quux {:?}", true);
warn!({ foo = 2, bar.baz = 79 }, "quux {:?}, {quux}", true, quux = false);
warn!({ foo = 2, bar.baz = 78 }, "quux");
Expand All @@ -759,6 +780,9 @@ fn warn() {
warn!(?foo);
warn!(%foo);
warn!(foo);
warn!("foo" = ?foo);
warn!("foo" = %foo);
warn!("foo" = foo);
warn!(name: "foo", ?foo);
warn!(name: "foo", %foo);
warn!(name: "foo", foo);
Expand Down Expand Up @@ -795,15 +819,18 @@ fn error() {
error!(foo = ?3, bar.baz = %2, quux = false);
error!(foo = 3, bar.baz = 2, quux = false);
error!(foo = 3, bar.baz = 3,);
error!("foo" = 3, bar.baz = 3,);
error!(foo = 3, "bar.baz" = 3,);
error!("foo");
error!("foo: {}", 3);
error!(foo = ?3, bar.baz = %2, quux = false, "hello world {:?}", 42);
error!(foo = 3, bar.baz = 2, quux = false, "hello world {:?}", 42);
error!(foo = 3, bar.baz = 3, "hello world {:?}", 42,);
error!({ foo = 3, bar.baz = 80 }, "quux");
error!({ "foo" = 3, "bar.baz" = 80 }, "quux");
error!({ foo = 2, bar.baz = 79 }, "quux {:?}", true);
error!({ foo = 2, bar.baz = 79 }, "quux {:?}, {quux}", true, quux = false);
error!({ foo = 2, bar.baz = 78, }, "quux");
error!({ foo = 2, bar.baz = 78 }, "quux");
error!({ foo = ?2, bar.baz = %78 }, "quux");
error!(name: "foo", foo = 3, bar.baz = 2, quux = false);
error!(name: "foo", target: "foo_events", foo = 3, bar.baz = 2, quux = false);
Expand All @@ -819,6 +846,9 @@ fn error() {
error!(?foo);
error!(%foo);
error!(foo);
error!("foo" = ?foo);
error!("foo" = %foo);
error!("foo" = foo);
error!(name: "foo", ?foo);
error!(name: "foo", %foo);
error!(name: "foo", foo);
Expand Down

0 comments on commit 908cc43

Please sign in to comment.