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
subscriber: handle explicit event parents properly in formatters #767
Conversation
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
.and_then(|id| self.ctx.ctx.span(&id)) | ||
.or_else(|| self.ctx.ctx.lookup_current()); |
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.
This is needed because this an option and we don't have stable try blocks, right?
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.
What do you mean? If there is no explicitly set parent, we want to look at the context to determine the current span.
Changed: - **parking_lot**: Updated the optional `parking_lot` dependency to accept the latest `parking_lot` version (#774) Fixed - **fmt**: Fixed events with explicitly overridden parent spans being formatted as though they were children of the current span (#767) Added - **fmt**: Added the option to print synthesized events when spans are created, entered, exited, and closed, including span durations (#761) - Documentation clarification and improvement (#762, #769) Thanks to @rkuhn, @greenwoodcm, and @Ralith for contributing to this release! Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Changed: - **parking_lot**: Updated the optional `parking_lot` dependency to accept the latest `parking_lot` version (#774) Fixed - **fmt**: Fixed events with explicitly overridden parent spans being formatted as though they were children of the current span (#767) Added - **fmt**: Added the option to print synthesized events when spans are created, entered, exited, and closed, including span durations (#761) - Documentation clarification and improvement (#762, #769) Thanks to @rkuhn, @greenwoodcm, and @Ralith for contributing to this release! Signed-off-by: Eliza Weisman <eliza@buoyant.io>
### Changed - **parking_lot**: Updated the optional `parking_lot` dependency to accept the latest `parking_lot` version (#774) ### Fixed - **fmt**: Fixed events with explicitly overridden parent spans being formatted as though they were children of the current span (#767) ### Added - **fmt**: Added the option to print synthesized events when spans are created, entered, exited, and closed, including span durations (#761) - Documentation clarification and improvement (#762, #769) Thanks to @rkuhn, @greenwoodcm, and @Ralith for contributing to this release! Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Motivation
Currently, the default formatter implementations in
tracing-subscriber
'sfmt
module do not handle explicitly set parentspans for events, such as
Instead, when formatting the span context of an event, the context is
always generated from the current span, even when the event has an
overridden parent. This is not correct.
Solution
This branch changes the default context formatters to use the explicit
parent ID, if it is present. Otherwise, the contexual parent is used, as
it was previously.
I've also added tests ensuring that this works correctly, and removed
some workarounds for the previous incorrect behavior from the examples.
Fixes #766
Signed-off-by: Eliza Weisman eliza@buoyant.io