tracing: generate macro tests #3437
Open
+741
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hds
force-pushed
the
hds/macro-gen
branch
6 times, most recently
from
0cb912b to
3fbc0b9
Compare
hds
added a commit
that referenced
this pull request
Dec 17, 2025
The level event macros (e.g. `trace!`, `info!`) call into the `event!` macro with the level set appropriately. However, these macros have become complex, covering many different cases and reaching aroudn 250 lines of patterns. As discovered in #3437, there were many cases where these level event macros didn't correctly handle patterns which the `event!` macro does handle. This change simplifies the event macros to delegate more parsing to the common `event!` macro patterns. Rather than matching all the possible field patterns, the level event macros only match the directives (name, target, and parent or some combination thereof). The remainder of the macro invocation is matched as a token tree (`tt`) and passed as is to the `event!` macro. This reduces the patterns to only the 8 combinations of directives (including no directives at all), reducing the previous 250 lines of patterns for each macro down to around 25 lines instead. Additionally, an unqualified use of `Some` in the `valueset` macro has been fixed, as this affected all event macros (`event!` and all the level event macros). With these changes, the comprehensive checks introduced in #3437 now all pass and so the job can be fully enabled to fail on CI.
hds
added a commit
that referenced
this pull request
Dec 17, 2025
The level event macros (e.g. `trace!`, `info!`) call into the `event!` macro with the level set appropriately. However, these macros have become complex, covering many different cases and reaching aroudn 250 lines of patterns. As discovered in #3437, there were many cases where these level event macros didn't correctly handle patterns which the `event!` macro does handle. This change simplifies the event macros to delegate more parsing to the common `event!` macro patterns. Rather than matching all the possible field patterns, the level event macros only match the directives (name, target, and parent or some combination thereof). The remainder of the macro invocation is matched as a token tree (`tt`) and passed as is to the `event!` macro. This reduces the patterns to only the 8 combinations of directives (including no directives at all), reducing the previous 250 lines of patterns for each macro down to around 25 lines instead. Additionally, an unqualified use of `Some` in the `valueset` macro has been fixed, as this affected all event macros (`event!` and all the level event macros). With these changes, the comprehensive checks introduced in #3437 now all pass and so the job can be fully enabled to fail on CI.
hds
added a commit
that referenced
this pull request
Dec 17, 2025
The level event macros (e.g. `trace!`, `info!`) call into the `event!` macro with the level set appropriately. However, these macros have become complex, covering many different cases and reaching aroudn 250 lines of patterns. As discovered in #3437, there were many cases where these level event macros didn't correctly handle patterns which the `event!` macro does handle. This change simplifies the event macros to delegate more parsing to the common `event!` macro patterns. Rather than matching all the possible field patterns, the level event macros only match the directives (name, target, and parent or some combination thereof). The remainder of the macro invocation is matched as a token tree (`tt`) and passed as is to the `event!` macro. This reduces the patterns to only the 8 combinations of directives (including no directives at all), reducing the previous 250 lines of patterns for each macro down to around 25 lines instead. Additionally, an unqualified use of `Some` in the `valueset` macro has been fixed, as this affected all event macros (`event!` and all the level event macros). With these changes, the comprehensive checks introduced in #3437 now all pass and so the job can be fully enabled to fail on CI.
There are a large number of combinations of field types which currently are not matched by the level event macros (e.g. `info!`). A recent example from #3407 is the following invocation that doesn't compile: ```rust info!(name: "order.received.ok", order.id = 123, "order received"); ``` However, the corresponding `event!` macro does compile: ```rust event!(name: "order.received.ok", Level::INFO, order.id = 123, "order received"); ``` And other variants also compile: ```rust // Without `name:` directive info!(order.id = 123, "order received"); // With another field before the dotted one info!(name: "order.received.ok", foo = true, order.id = 123, "order received"); ``` Many such cases have been fixed in the past (#2983, #2883, #2879). However, this has been a bit like wack-a-mole, where we keep finding issues and then fixing those issues, adding extra tests for them and then going on our way. Since the complexity is often in combinations (as above, only when using the `name:` directive together with a dotted field name on the first field), it would be useful to have some extensive tests that attempt to cover all possible combinations. It turns out that there are **a lot** of combiantions. This change adds an `xtask` that generates tests for event macros (span macros are out of scope for this change) similar to the ones found in `tracing/tests/macros.rs`. Which is to say, tests containing macros which should compile, but don't need to run. Owing to the large number of combinations, the tests are split into test files by macro (e.g. `event!`, `info!`) and directive combination (e.g. no directives, just `name:`, `name:` and `target:`). The tests are kept in a separate crate outside the main workspace to avoid rust-analyzer trying to parse the files - as they are quite large. Specifically, there are 1220 macro invocations per test file. For each macro, there are 9760 invocations generated (including all combinatiosn of directives). When run against the current `tracing` macros, this resulted in the following failure counts: * `event!`: 705 * `info!`, `warn!`: 1683 (each) * `trace!`, `debug!`, `error!`: 1652 (each) The test files are generated (no need to check them in) and then validated by `cargo check` on CI. The CI job has not been made blocking because none of the errors have been fixed yet!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
There are a large number of combinations of field types which currently
are not matched by the level event macros (e.g.
info!).A recent example from #3407 is the following invocation that doesn't
compile:
However, the corresponding
event!macro does compile:And other variants also compile:
Many such cases have been fixed in the past (#2983, #2883, #2879).
However, this has been a bit like wack-a-mole, where we keep finding
issues and then fixing those issues, adding extra tests for them and
then going on our way.
Since the complexity is often in combinations (as above, only when using
the
name:directive together with a dotted field name on the firstfield), it would be useful to have some extensive tests that attempt to
cover all possible combinations.
It turns out that there are a lot of combiantions.
Solution
This change adds an
xtaskthat generates tests for event macros (spanmacros are out of scope for this change) similar to the ones found in
tracing/tests/macros.rs. Which is to say, tests containing macroswhich should compile, but don't need to run.
Owing to the large number of combinations, the tests are split into test
files by macro (e.g.
event!,info!) and directive combination (e.g.no directives, just
name:,name:andtarget:). The tests are keptin a separate crate outside the main workspace to avoid rust-analyzer
trying to parse the files - as they are quite large. Specifically, there
are 1220 macro invocations per test file.
For each macro, there are 9760 invocations generated (including all
combinatiosn of directives). When run against the current
tracingmacros, this resulted in the following failure counts:
event!: 705info!,warn!: 1683 (each)trace!,debug!,error!: 1652 (each)The test files are generated (no need to check them in) and then
validated by
cargo checkon CI.The CI job has not been made blocking because none of the errors have
been fixed yet!