Attribute non-span tail events to the current user span#6656
Conversation
|
UnknownError: ProviderInitError |
|
@danlapid Bonk workflow failed. Check the logs for details. View workflow run · To retry, trigger Bonk again. |
IoContext::getInvocationSpanContext() now substitutes in the currently-active user span's spanId (from the AsyncContextFrame pushed by Tracing::enterSpan), falling back to the invocation root when none is active. Previously Log/Exception/DiagnosticChannelEvent/JsRpc Attribute tail events always reported the root spanId, even when emitted inside a ctx.tracing.enterSpan() callback.
dfe92a5 to
a0fb590
Compare
|
/bonk review this PR |
There was a problem hiding this comment.
getInvocationSpanContext()return-type change from&to value is safe: all call-sites pass toconst InvocationSpanContext¶meters. The constructed context lacks an entropy source (sonewChild()would assert), but no call-site invokesnewChild()on it.- The
getCurrentUserTraceSpan()refactor correctly short-circuits when user tracing is unobserved, avoiding the AsyncContextFrame probe on the hot path when there is no tail worker. SpanObserver::getSpanId()+SpanParent::getSpanId()are clean additions. AddingoverridetoUserSpanObserver::getSpanId()is a correctness fix.- Test coverage is good (sync, async/await, nested, outside-span, diagnostics channel). One gap noted in an inline comment.
This review was generated by an AI code review assistant and may contain inaccuracies.
|
Review posted on PR #6656. Summary of findings: C++ changes (correct, safe):
Two inline comments posted:
|
Merging this PR will not alter performance
Comparing Footnotes
|
|
Implementation changes LGTM. The tests feel too verbose, but that's non-blocking/can be hard to avoid with LLM output |
IoContext::getInvocationSpanContext() now substitutes in the currently-active user span's spanId (from the AsyncContextFrame pushed by Tracing::enterSpan), falling back to the invocation root when none is active. Previously Log/Exception/DiagnosticChannelEvent/JsRpc Attribute tail events always reported the root spanId, even when emitted inside a ctx.tracing.enterSpan() callback.