Allow setting event names in macros. #2699
Conversation
There was a problem hiding this comment.
The macro and test changes look good to me --- thank you for working on this! However, I notice that the new macro syntax is not documented anywhere. Can we add it to the the RustDoc comments on the individual event macros, and also mention it in the top-level "using the macros" section in the lib.rs docs? It would be nice to include examples. Thank you!
|
Will do. Likely will get to this tonight (PDT). |
There was a problem hiding this comment.
the docs here describe what can be overridden, and they don't mention event names:
https://github.com/tokio-rs/tracing/pull/2699/files#diff-ea2f30245316830135d5b43541a7be0930b4bfded454b5396509770c39a59c7cL216-L220
can we also update this paragraph?
tracing/src/lib.rs
Outdated
| @@ -421,6 +421,7 @@ | |||
| //! // - "message", with the value "the answer to the ultimate question of life, the | |||
| //! // universe, and everything is 42." | |||
| //! event!( | |||
| //! name: "life", | |||
There was a problem hiding this comment.
this example has a block comment explaining the span that will be recorded, and it doesn't mention the overridden name:
https://github.com/tokio-rs/tracing/pull/2699/files#diff-ea2f30245316830135d5b43541a7be0930b4bfded454b5396509770c39a59c7cR417-R422
can we either update that comment, or remove the overridden name from this example?
my preference would probably be to add new examples for overriding event names, rather than including them in existing examples. this example is intended to demonstrate the way that the message field is generated, and overriding the name is not related to message generation. my preference would be to not include things not directly related to the specific thing being demonstrated, so that the user doesn't think they're related...
|
@hawkw, I don't necessarily want to re-do, but how do you feel about the The current change is fine with me, but just wanted to check that this convention aligns well with your wishes. |
hmm, my preference is for events to not require a name...i feel like users often just want to record a single field value or an unstructured textual log message, and having to think up a name for each event seems like an extra speed bump to just recording the piece of data you want. i agree that this is somewhat inconsistent with how spans are recorded, though, and i'm open to being convinced otherwise. i feel like in the common case, an event will have a textual message, and the name will likely end up duplicating some or all of that text. i wonder if (as a separate change), we might want to change the default generated name to be the format string for the textual message, if there is a format string. that way, the default name is maybe a bit more useful than the source location. in any case, this PR is definitely going to be backported to v0.1.x, so i'd like to continue with the non-breaking change so we can release this in v0.1.x soon. we could reconsider the conventions on v0.2.x after this merges. |
|
Cool: makes sense. Just wanted to make sure I wasn't pushing something that would make future plans harder. I agree that it does seem to make sense to not require |
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
## Motivation The motivation is #1426. Currently, event names are set to a default value of `event file:line`, which (1) doesn't allow for customization, and (2) prevents events from working for some opentelemetry endpoints (they often use the event name `exception` to designate something like a panic). ## Solution Went through the event macros, and added new parameterization that allows for setting the `name`. In addition, added some comments, and reordering, to make life a bit better for the next person that comes along to edit those macros. Finally, added tests for the macro expansion alongside the existing tests with a reasonable amount of coverage (though, admittedly, more could be added for all of the macro invocation types Fixes #1426
## Motivation The motivation is #1426. Currently, event names are set to a default value of `event file:line`, which (1) doesn't allow for customization, and (2) prevents events from working for some opentelemetry endpoints (they often use the event name `exception` to designate something like a panic). ## Solution Went through the event macros, and added new parameterization that allows for setting the `name`. In addition, added some comments, and reordering, to make life a bit better for the next person that comes along to edit those macros. Finally, added tests for the macro expansion alongside the existing tests with a reasonable amount of coverage (though, admittedly, more could be added for all of the macro invocation types Fixes #1426
## Motivation The motivation is #1426. Currently, event names are set to a default value of `event file:line`, which (1) doesn't allow for customization, and (2) prevents events from working for some opentelemetry endpoints (they often use the event name `exception` to designate something like a panic). ## Solution Went through the event macros, and added new parameterization that allows for setting the `name`. In addition, added some comments, and reordering, to make life a bit better for the next person that comes along to edit those macros. Finally, added tests for the macro expansion alongside the existing tests with a reasonable amount of coverage (though, admittedly, more could be added for all of the macro invocation types Fixes #1426
## Motivation The motivation is #1426. Currently, event names are set to a default value of `event file:line`, which (1) doesn't allow for customization, and (2) prevents events from working for some opentelemetry endpoints (they often use the event name `exception` to designate something like a panic). ## Solution Went through the event macros, and added new parameterization that allows for setting the `name`. In addition, added some comments, and reordering, to make life a bit better for the next person that comes along to edit those macros. Finally, added tests for the macro expansion alongside the existing tests with a reasonable amount of coverage (though, admittedly, more could be added for all of the macro invocation types Fixes #1426
## Motivation The motivation is #1426. Currently, event names are set to a default value of `event file:line`, which (1) doesn't allow for customization, and (2) prevents events from working for some opentelemetry endpoints (they often use the event name `exception` to designate something like a panic). ## Solution Went through the event macros, and added new parameterization that allows for setting the `name`. In addition, added some comments, and reordering, to make life a bit better for the next person that comes along to edit those macros. Finally, added tests for the macro expansion alongside the existing tests with a reasonable amount of coverage (though, admittedly, more could be added for all of the macro invocation types Fixes #1426
## Motivation The motivation is #1426. Currently, event names are set to a default value of `event file:line`, which (1) doesn't allow for customization, and (2) prevents events from working for some opentelemetry endpoints (they often use the event name `exception` to designate something like a panic). ## Solution Went through the event macros, and added new parameterization that allows for setting the `name`. In addition, added some comments, and reordering, to make life a bit better for the next person that comes along to edit those macros. Finally, added tests for the macro expansion alongside the existing tests with a reasonable amount of coverage (though, admittedly, more could be added for all of the macro invocation types Fixes #1426
## Motivation The motivation is #1426. Currently, event names are set to a default value of `event file:line`, which (1) doesn't allow for customization, and (2) prevents events from working for some opentelemetry endpoints (they often use the event name `exception` to designate something like a panic). ## Solution Went through the event macros, and added new parameterization that allows for setting the `name`. In addition, added some comments, and reordering, to make life a bit better for the next person that comes along to edit those macros. Finally, added tests for the macro expansion alongside the existing tests with a reasonable amount of coverage (though, admittedly, more could be added for all of the macro invocation types Fixes #1426
# 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
Motivation
The motivation is #1426. Currently, event names are set to a default value of
event file:line, which (1) doesn't allow for customization, and (2) prevents events from working for some opentelemetry endpoints (they often use the event nameexceptionto designate something like a panic).Solution
Went through the event macros, and added new parameterization that allows for setting the
name. In addition, added some comments, and reordering, to make life a bit better for the next person that comes along to edit those macros. Finally, added tests for the macro expansion alongside the existing tests with a reasonable amount of coverage (though, admittedly, more could be added for all of the macro invocation types).