Skip to content

macros: Handle constant field names#2617

Merged
hawkw merged 8 commits intotokio-rs:masterfrom
wyfo:constant_field_name
Sep 5, 2023
Merged

macros: Handle constant field names#2617
hawkw merged 8 commits intotokio-rs:masterfrom
wyfo:constant_field_name

Conversation

@wyfo
Copy link
Copy Markdown
Contributor

@wyfo wyfo commented Jun 15, 2023

Motivation

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.

Solution

This PR proposes a new syntax for using constant field names:

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.

@wyfo wyfo requested review from a team, davidbarsky and hawkw as code owners June 15, 2023 16:23
hawkw
hawkw reviewed Jun 15, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

Thanks! I'm definitely open to adding this. It would be nice to add tests for a few more cases, if you don't mind?

We may also want to allow constants to be used as span names and/or targets for spans and events, but we could look into this in a follow-up PR.

Comment thread tracing/src/lib.rs Outdated
Comment thread tracing/tests/event.rs Outdated
.run_with_handle();
with_default(collector, || {
const FOO: &str = "foo";
tracing::event!(Level::INFO, { FOO } = "bar");
Copy link
Copy Markdown
Member

@hawkw hawkw Jun 15, 2023

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
Copy Markdown
Member

@hawkw hawkw Jun 15, 2023

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 }?

Comment thread tracing/tests/span.rs Outdated
.run_with_handle();
with_default(collector, || {
const FOO: &str = "foo";
tracing::span!(Level::TRACE, "my_span", { FOO } = "bar");
Copy link
Copy Markdown
Member

@hawkw hawkw Jun 15, 2023

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

wyfo and others added 2 commits June 22, 2023 09:17
@wyfo @hawkw
doc: fix typo
38a7232 
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
macros: refine constant field names tests
8f3d935
@wyfo
Copy link
Copy Markdown
Contributor Author

wyfo commented Jun 22, 2023 •
edited
Loading

Sorry for the delay, hard to find time for open-source at this moment...
I've updated the tests, it's indeed better now.

We may also want to allow constants to be used as span names and/or targets for spans and events, but we could look into this in a follow-up PR.

Actually, it's already possible, because they are all expr designators;

tracing::span!(std::convert::identity(tracing::Level::INFO), stringify!(my_span))

works fine for example.
On the other hand, an additional syntax was needed for fields because they use ident designator by default.

hawkw
hawkw approved these changes Sep 5, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

this looks good to me, thank you! i had one suggestion for making the documentation a little bit more obvious.

Comment thread tracing/src/lib.rs Outdated
Comment thread tracing/tests/event.rs
Comment on lines +426 to +441
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"
);
Copy link
Copy Markdown
Member

@hawkw hawkw Sep 5, 2023

Choose a reason for hiding this comment

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

thanks for testing all these cases!

@hawkw hawkw enabled auto-merge (squash) September 5, 2023 21:15
@hawkw hawkw mentioned this pull request Sep 5, 2023
Open
@hawkw hawkw merged commit 245d607 into tokio-rs:master Sep 5, 2023
@stormshield-frb
Copy link
Copy Markdown

stormshield-frb commented Sep 14, 2023

Hi. Great useful feature, thanks !!

While testing it out, I've noticed that it seems to work only for the tracing::event! and tracing::span! macro. Is there any plan to integrate this feature with the #[instrument] macro and all the other 'classic' macros (debug, info, warn, etc) ? Thanks

davidbarsky pushed a commit that referenced this pull request Sep 26, 2023
@wyfo @hawkw @davidbarsky
tracing: allow constant field names in macros (#2617)
c25bd39 
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 <joseph.perez@numberly.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
@davidbarsky davidbarsky mentioned this pull request Sep 26, 2023
Merged
davidbarsky pushed a commit that referenced this pull request Sep 27, 2023
@wyfo @hawkw @davidbarsky
tracing: allow constant field names in macros (#2617)
0cca212 
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 <joseph.perez@numberly.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
davidbarsky pushed a commit that referenced this pull request Sep 27, 2023
@wyfo @hawkw @davidbarsky
tracing: allow constant field names in macros (#2617)
4b19632 
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 <joseph.perez@numberly.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
davidbarsky pushed a commit that referenced this pull request Sep 27, 2023
@wyfo @hawkw @davidbarsky
tracing: allow constant field names in macros (#2617)
f23de16 
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 <joseph.perez@numberly.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
davidbarsky pushed a commit that referenced this pull request Sep 27, 2023
@wyfo @hawkw @davidbarsky
tracing: allow constant field names in macros (#2617)
c56682a 
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 <joseph.perez@numberly.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
davidbarsky pushed a commit that referenced this pull request Sep 27, 2023
@wyfo @hawkw @davidbarsky
tracing: allow constant field names in macros (#2617)
107ab37 
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 <joseph.perez@numberly.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
davidbarsky pushed a commit that referenced this pull request Sep 27, 2023
@wyfo @hawkw @davidbarsky
tracing: allow constant field names in macros (#2617)
75e4f1e 
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 <joseph.perez@numberly.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
davidbarsky pushed a commit that referenced this pull request Sep 29, 2023
@wyfo @hawkw @davidbarsky
tracing: allow constant field names in macros (#2617)
0687403 
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 <joseph.perez@numberly.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Oct 1, 2023
@wyfo @hawkw
tracing: allow constant field names in macros (#2617)
1d8861d 
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 <joseph.perez@numberly.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
hawkw pushed a commit that referenced this pull request Oct 13, 2023
@davidbarsky @hawkw
chore: prepare tracing 0.1.39 (#2755)
4b99457 
# 0.1.39 (October 12, 2023)

This release adds several additional features to the `tracing` macros.
In addition, it updates the `tracing-core` dependency to
[v0.1.32][core-0.1.32] and the `tracing-attributes` dependency to
[v0.1.27][attrs-0.1.27].

### Added

- Allow constant field names in macros ([#2617])
- Allow setting event names in macros ([#2699])
- **core**: Allow `ValueSet`s of any length ([#2508])

### Changed

- `tracing-attributes`: updated to [0.1.27][attrs-0.1.27]
- `tracing-core`: updated to [0.1.32][core-0.1.32]
- **attributes**: Bump minimum version of proc-macro2 to 1.0.60
  ([#2732])
-  **attributes**: Generate less dead code for async block return type
   hint ([#2709])

### Fixed

- Use fully qualified names in macros for items exported from std
  prelude ([#2621], [#2757])
- **attributes**: Allow [`clippy::let_with_type_underscore`] in
  macro-generated code ([#2609])
- **attributes**: Allow `unknown_lints` in macro-generated code
  ([#2626])
- **attributes**: Fix a compilation error in `#[instrument]` when the
  `"log"` feature is enabled ([#2599])

### Documented

- Add `axum-insights` to relevant crates. ([#2713])
- Fix link to RAI pattern crate documentation ([#2612])
- Fix docs typos and warnings ([#2581])
- Add `clippy-tracing` to related crates ([#2628])
- Add `tracing-cloudwatch` to related crates ([#2667])
- Fix deadlink to `tracing-etw` repo ([#2602])

[#2617]: #2617
[#2699]: #2699
[#2508]: #2508
[#2621]: #2621
[#2713]: #2713
[#2581]: #2581
[#2628]: #2628
[#2667]: #2667
[#2602]: #2602
[#2626]: #2626
[#2757]: #2757
[#2732]: #2732
[#2709]: #2709
[#2599]: #2599
[`let_with_type_underscore`]: http://rust-lang.github.io/rust-clippy/rust-1.70.0/index.html#let_with_type_underscore
[attrs-0.1.27]:
    https://github.com/tokio-rs/tracing/releases/tag/tracing-attributes-0.1.27
[core-0.1.32]:
    https://github.com/tokio-rs/tracing/releases/tag/tracing-core-0.1.32
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
@wyfo @hawkw @kaffarell
tracing: allow constant field names in macros (tokio-rs#2617)
a8b5de9 
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 <joseph.perez@numberly.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
@davidbarsky @kaffarell
chore: prepare tracing 0.1.39 (tokio-rs#2755)
4003892 
# 0.1.39 (October 12, 2023)

This release adds several additional features to the `tracing` macros.
In addition, it updates the `tracing-core` dependency to
[v0.1.32][core-0.1.32] and the `tracing-attributes` dependency to
[v0.1.27][attrs-0.1.27].

### Added

- Allow constant field names in macros ([tokio-rs#2617])
- Allow setting event names in macros ([tokio-rs#2699])
- **core**: Allow `ValueSet`s of any length ([tokio-rs#2508])

### Changed

- `tracing-attributes`: updated to [0.1.27][attrs-0.1.27]
- `tracing-core`: updated to [0.1.32][core-0.1.32]
- **attributes**: Bump minimum version of proc-macro2 to 1.0.60
  ([tokio-rs#2732])
-  **attributes**: Generate less dead code for async block return type
   hint ([tokio-rs#2709])

### Fixed

- Use fully qualified names in macros for items exported from std
  prelude ([tokio-rs#2621], [tokio-rs#2757])
- **attributes**: Allow [`clippy::let_with_type_underscore`] in
  macro-generated code ([tokio-rs#2609])
- **attributes**: Allow `unknown_lints` in macro-generated code
  ([tokio-rs#2626])
- **attributes**: Fix a compilation error in `#[instrument]` when the
  `"log"` feature is enabled ([tokio-rs#2599])

### Documented

- Add `axum-insights` to relevant crates. ([tokio-rs#2713])
- Fix link to RAI pattern crate documentation ([tokio-rs#2612])
- Fix docs typos and warnings ([tokio-rs#2581])
- Add `clippy-tracing` to related crates ([tokio-rs#2628])
- Add `tracing-cloudwatch` to related crates ([tokio-rs#2667])
- Fix deadlink to `tracing-etw` repo ([tokio-rs#2602])

[tokio-rs#2617]: tokio-rs#2617
[tokio-rs#2699]: tokio-rs#2699
[tokio-rs#2508]: tokio-rs#2508
[tokio-rs#2621]: tokio-rs#2621
[tokio-rs#2713]: tokio-rs#2713
[tokio-rs#2581]: tokio-rs#2581
[tokio-rs#2628]: tokio-rs#2628
[tokio-rs#2667]: tokio-rs#2667
[tokio-rs#2602]: tokio-rs#2602
[tokio-rs#2626]: tokio-rs#2626
[tokio-rs#2757]: tokio-rs#2757
[tokio-rs#2732]: tokio-rs#2732
[tokio-rs#2709]: tokio-rs#2709
[tokio-rs#2599]: tokio-rs#2599
[`let_with_type_underscore`]: http://rust-lang.github.io/rust-clippy/rust-1.70.0/index.html#let_with_type_underscore
[attrs-0.1.27]:
    https://github.com/tokio-rs/tracing/releases/tag/tracing-attributes-0.1.27
[core-0.1.32]:
    https://github.com/tokio-rs/tracing/releases/tag/tracing-core-0.1.32
This was referenced Nov 26, 2024
Closed
Merged
hds added a commit that referenced this pull request Nov 17, 2025
@nico-incubiq @hds
Support constant expressions as instrument field names (#3158)
ee0713d 
## Motivation

The `#[instrument]` proc macro only allows setting field names as identifiers, which is not on par with other macros such as `span!` or `event!` that allow constant expressions. Using constants for example can ensure consistency across a project for the field names used.

This addresses #2969, which was already mentioned in this [comment here](#2617 (comment)) on #2617 (note that "classic macros" mentioned there already work, there are tests attesting of it).
I also updated the doc block to mention field values can be any arbitrary expressions, which was implemented by #672 but never updated.

## Solution

I updated the instrument proc macro to allow constant expressions (const, const fn, literal str, ...) as field names, using the same syntax as #2617, **enclosed in curly braces**.
```rust
const CONST_FIELD_NAME: &str = "foo.bar";

#[instrument(fields({CONST_FIELD_NAME} = "quux"))]
fn fn_const_field_name() {}
```

The behavior around overriding function parameters needed to be
a bit different with const field names, where duplicate fields will
occur if the function parameter isn't explicitly skipped. This is
to avoid any runtime check and may be able to be improved in
future Rust versions.

---------

Co-authored-by: Hayden Stainsby <hds@caffeineconcepts.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hawkw hawkw hawkw approved these changes

@davidbarsky davidbarsky Awaiting requested review from davidbarsky davidbarsky is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@wyfo @stormshield-frb @hawkw