-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
trace: Span API polish #988
Conversation
In practice, there wasn't really a use-case for this, and it complicates the implementation. We can always add it back later. Signed-off-by: Eliza Weisman <[email protected]>'
Again, there wasn't actually a use-case for spans with metadata that doesn't live for the static lifetime, and it made using `Span`s in other types somewhat more challenging. It's also possible to implement an alternative API for non-static spans on top of the `tokio-trace-core` primitives. Signed-off-by: Eliza Weisman <[email protected]>
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.
im all for this, im using static stuff anyways so 👍
@@ -154,18 +142,11 @@ use { | |||
/// span will silently do nothing. Thus, the handle can be used in the same | |||
/// manner regardless of whether or not the trace is currently being collected. | |||
#[derive(Clone, PartialEq, Hash)] | |||
pub struct Span<'a> { | |||
pub struct Span { |
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 means we can drop the lifetime from tokio_trace_futures::Instrumented
?
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.
Yes; in fact, this will break the current Instrumented
.
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.
Works for me! its an easy upgrade
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.
+1 on tests passing
tokio-rs/tokio#988 removed the generic lifetime parameter from `Span`. This branch updates the nursery crates to match. Signed-off-by: Eliza Weisman <[email protected]>
This branch makes the following changes to `tokio-trace`'s `Span` type: * **Remove manual close API from spans** In practice, there wasn't really a use-case for this, and it complicates the implementation a bit. We can always add it back later. * **Remove generic lifetime from `Span`** Again, there wasn't actually a use-case for spans with metadata that doesn't live for the static lifetime, and it made using `Span`s in other types somewhat inconvenient. It's also possible to implement an alternative API for non-static spans on top of the `tokio-trace-core` primitives. Signed-off-by: Eliza Weisman <[email protected]>
This branch makes the following changes to
tokio-trace
'sSpan
type:Remove manual close API from spans
In practice, there wasn't really a use-case for this, and it
complicates the implementation a bit. We can always add it back later.
Remove generic lifetime from
Span
Again, there wasn't actually a use-case for spans with metadata that
doesn't live for the static lifetime, and it made using
Span
s inother types somewhat inconvenient. It's also possible to implement an
alternative API for non-static spans on top of the
tokio-trace-core
primitives.
Signed-off-by: Eliza Weisman [email protected]