-
Notifications
You must be signed in to change notification settings - Fork 731
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.
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
tracing, core: re-implement empty fields #548
Conversation
Right now, there is no way to indicate that a span field _currently_ has no value but that a value will be recorded later. Tracing used to support this concept, but when we added local-variable shorthand, it was removed from the macros, as we needed to use the previous syntax for indicating empty fields for local variable shorthand. See #77 for details. This commit side-steps the problem of determining an appropriate _syntax_ for empty fields by adding a special value _type_, called `Empty`. Now, empty values can be indicated like ```rust span!("my_span", foo = 5, bar = tracing::field::Empty); ``` The `Empty` type implements `Value`, but its' `record` method is a no-op that doesn't' call any functions on the visitor. Therefore, when a field is empty, the visitor will not see it. An empty field can then be recorded later using `Subscriber::record` (or the corresponding `Span` method in `tracing`). Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
/// // ... | ||
/// | ||
/// // Now, record a value for parting as well. | ||
/// span.record("parting", &"goodbye world!"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since "parting" needs to referenced as a string, I'd add a comment noting that difference between macros and normal function parameters as it took me a few seconds to parse the example in https://deploy-preview-548--tracing-rs.netlify.com/tracing/span/struct.span?search=#method.record.
//! // Now, record a value for parting as well. | ||
//! span.record("parting", &"goodbye world!"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as I mentioned prior would be handy here too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few non-blocking documentation nits.
@tokio-rs/tracing does anyone else want to weigh in on this before I merge it? |
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Added: - `field::Empty` type for declaring empty fields whose values will be recorded later (#548) - `field::Value` implementations for `Wrapping` and `NonZero*` numbers (#538) Fixed: - Broken and unresolvable links in RustDoc (#595) Thanks to @oli-cosmian for contributing to this release! Signed-off-by: Eliza Weisman <eliza@buoyant.io>
### Added - `field::Empty` type for declaring empty fields whose values will be recorded later (#548) - `field::Value` implementations for `Wrapping` and `NonZero*` numbers (#538) ### Fixed - Broken and unresolvable links in RustDoc (#595) Thanks to @oli-cosmian for contributing to this release! Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Added: - **field**: `field::Empty` type for declaring empty fields whose values will be recorded later (#548) - **field**: `field::Value` implementations for `Wrapping` and `NonZero*` numbers (#538) - **attributes**: Support for adding arbitrary literal fields to spans generated by `#[instrument]` (#569) - **attributes**: `#[instrument]` now emits a helpful compiler error when attempting to skip a function parameter (#600) Changed: - **attributes**: The `#[instrument]` attribute was placed under an on-by-default feature flag "attributes" (#603) Fixed: - Broken and unresolvable links in RustDoc (#595) Thanks to @oli-cosmian and @Kobzol for contributing to this release! Signed-off-by: Eliza Weisman <eliza@buoyant.io>
### Added - **field**: `field::Empty` type for declaring empty fields whose values will be recorded later (#548) - **field**: `field::Value` implementations for `Wrapping` and `NonZero*` numbers (#538) - **attributes**: Support for adding arbitrary literal fields to spans generated by `#[instrument]` (#569) - **attributes**: `#[instrument]` now emits a helpful compiler error when attempting to skip a function parameter (#600) ### Changed - **attributes**: The `#[instrument]` attribute was placed under an on-by-default feature flag "attributes" (#603) ### Fixed - Broken and unresolvable links in RustDoc (#595) Thanks to @oli-cosmian and @Kobzol for contributing to this release! Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Right now, there is no way to indicate that a span field currently
has no value but that a value will be recorded later. Tracing used to
support this concept, but when we added local-variable shorthand, it was
removed from the macros, as we needed to use the previous syntax for
indicating empty fields for local variable shorthand. See #77 for
details.
This commit side-steps the problem of determining an appropriate
syntax for empty fields by adding a special value type, called
Empty
. Now, empty values can be indicated likeThe
Empty
type implementsValue
, but its'record
method is a no-opthat doesn't' call any functions on the visitor. Therefore, when a field
is empty, the visitor will not see it. An empty field can then be
recorded later using
Subscriber::record
(or the correspondingSpan
method in
tracing
).Closes #77
Signed-off-by: Eliza Weisman eliza@buoyant.io