fix(subscriptions): link to the restored trace context, don't parent - #570
fix(subscriptions): link to the restored trace context, don't parent#570Inok wants to merge 1 commit into
Conversation
The consume span was made a child of the append that produced the event, using the tracing context restored from the event's metadata. That context is durable data on the event, so every redelivery re-joined the same trace, with no live root to end it and no sampling decision left to take. A hot resubscribe loop grew one trace to 251MB and OOM-killed a shared Tempo twice. The consume span now roots its own trace and links to the restored context. Tags are unchanged. Parent-child is kept where the causality is in-process: an ambient Eventuous activity, or the activity stored for the async path. The SignalR client had the same bug and is fixed alongside. Rooting takes two steps, because .NET has no parentless argument and binds the parent from Activity.Current at Start() — which on the async path happens far down the pipe. Activity.Current is suppressed at creation, and the span is pinned to a zero parent span id so a late Start cannot re-parent it. The trace id is the sampler's own where it read one (only ratio-based samplers do) and generated otherwise, since an unstarted activity reports all zeros. Three defects found alongside, each wrong on its own: - ToActivityContext returned a zeroed context instead of null, so both callers' null checks never fired. - AddTracingMeta compared a 32-char trace id to a 16-char constant, so its empty-id guard never fired and all-zero contexts were written to events. - AddEventuousTracing called SetSampler, replacing the application's sampler for the whole provider; its drop branch matched no activity that exists, so it only ever returned RecordAndSample. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PR Summary by QodoFix subscription/SignalR consume tracing to link remote context instead of parenting
AI Description
Diagram
High-Level Assessment
Files changed (10)
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR |
Consume spans were parented to the tracing context restored from the event's
metadata, making each consume a child of the append that produced the event.
That context is durable data on the event, so every redelivery re-joined the
same trace — no live root to end it, no sampling decision left to take. A hot
resubscribe loop grew one trace to 251MB and OOM-killed a shared Tempo twice.
The consume span now roots its own trace and carries an
ActivityLinkto therestored context, which is also what the OpenTelemetry messaging conventions
prescribe for an asynchronous "process" operation. Tags are unchanged.
Parent-child is kept where the causality is in-process: an ambient Eventuous
activity, or the activity stored for the async handling path. The SignalR
client had the same bug and is fixed alongside.
Three defects found alongside, each wrong on its own:
ToActivityContextreturned a zeroed context instead ofnull, so bothcallers' null checks never fired.
AddTracingMetacompared a 32-char trace id to a 16-char constant, so itsempty-id guard never fired and all-zero contexts were written to events.
AddEventuousTracingcalledSetSampler, replacing the application's samplerfor the whole provider; its drop branch matched no activity that exists, so it
only ever returned
RecordAndSample.Breaking change for consumers of the traces
Navigating from a producer span to its consumers by parentage no longer works —
follow links instead. Applications that relied on
AddEventuousTracingtosample everything now need an explicit
AlwaysOnSampler.Testing
18 new unit tests across subscriptions, SignalR and tracing metadata; each fix
red-checked by reverting it.
TracesTestsverified against a real KurrentDB.Suites: subscriptions 50/50, core 29/29, application 21/21, gateway 10/10,
SignalR 16/16, analyzers 1/1, Sqlite 27/27.
Docs still to update in
eventuous-docs— not in this repo.🤖 Generated with Claude Code