Skip to content

Conversation

@Jesse-Bakker
Copy link
Contributor

@Jesse-Bakker Jesse-Bakker commented Dec 5, 2025

The Connection struct contains a tracing::Span tracking its
lifetime. This can be useful. The problem is that this span has as its
parent the span within which the connection is created, meaning that it
keeps that span alive (not closed) while it is alive. In practice that
means, for example, instrumented functions that create connections and
store those connections for reuse will not have their spans closed and
exported when they end.

This shows up in tonic, where the first RPC call to any server will have its containing span alive for the remainder of the RPC client's lifetime.

This commit sets the Connection's span's parent to None and gives it
a "follows-from" relation to the span in which it was created instead.
This shows the causal relationship between them, without tying the
creating span's lifetime to the connection's lifetime.

The `Connection` struct contains a `tracing::Span` tracking its
lifetime. This can be useful. The problem is that this span has as its
parent the span within which the connection is created, meaning that it
keeps that span alive (not closed) while it is alive. In practice that
means, for example, instrumented functions that create connections and
store those connections for reuse will not have their spans closed and
exported when they end.

This commit sets the `Connection`'s span's parent to `None` and gives it
a "follows-from" relation to the span in which it was created instead.
This shows the causal relationship between them, without tying the
creating span's lifetime to the connection's lifetime.
@seanmonstar
Copy link
Member

@cratelyn hey, I know linkerd2-proxy was originally interested in tracing with h2, how would this affect your workflow?

Copy link
Member

@cratelyn cratelyn left a comment

Choose a reason for hiding this comment

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

thanks for flagging me @seanmonstar! this looks like a great change to me, and i agree with the rationale outlined in the description here. this would be an ergonomic improvement for span lifetimes, and i support this change!

@seanmonstar
Copy link
Member

Right on, I'm good with releasing this and if causes issues, someone can tell us and we can evaluate more closely. :)

@seanmonstar seanmonstar merged commit 7c9a874 into hyperium:master Dec 5, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants