Repository navigation
Restore the ambient activity after a recoverability span starts a new trace - #7969
Open
tmasternak wants to merge 4 commits into
Open
tmasternak wants to merge 4 commits into
tmasternak wants to merge 4 commits into
Conversation
The recoverability activity ended as soon as the recoverability policy had decided, so its duration covered the decision and not the retry dispatch or the move to the error queue that followed. The activity now wraps the whole executor: the decision, the tag that records it and the recoverability pipeline. A failure of that pipeline is recorded on the activity before the exception propagates.
…d from The incoming and handler activities record the failure, then the recoverability activity starts from the message headers alone, so a trace shows no relation between the failed processing and what recoverability did about it. RecordError knows the innermost activity an exception escaped from: the first one to record it. When it is handed the pipeline context, it writes that activity's context to the root context bag, the one the transport hands to OnMessage and floats to OnError as ErrorContext.Extensions. The recoverability activity takes it from there and adds it as a link. An outgoing activity that fails inside a handler records without a bag and is skipped, so the link always points at a pipeline activity. Nothing is linked when no processing activity was created, when the failure happened outside the activities or when the transport does not float the bag. The context is consumed on read so a reused bag cannot carry it into the next attempt.
In Logs mode ActivityFactory.RecordError logged the exception from the innermost activity it escaped from, at Error level regardless of what recoverability was going to do, and the recoverability log entry carried the exception only in SpanAndLogs mode. A retried message produced an Error log entry for a failure that was handled. For failures in the message processing pipeline the exception is now logged once, when recoverability has decided, with the exception attached whatever the ExceptionRecordingMode: Information for an immediate retry or a discard, Warning for a delayed retry, Error for a move to the error queue. The entry is written while the recoverability activity is current, which links to the activity the exception escaped from. Failures outside that pipeline have no recoverability to log them: a failed operation on the message session and a failure of the recoverability pipeline itself are logged right away, at Error level, while their activity is current. RecordError tells the two apart by whether it is handed a context bag. ExceptionRecordingMode only controls whether the exception is also recorded as an event on the activity it escaped from.
tmasternak
force-pushed
the
otel-restore-ambient-activity
branch
from
October 16, 2026 12:41
71c5304 to
e587eec
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #7965 (base branch
otel-legacy-status-ok).RecoverabilityPipelineExecutordisposes the recoverability activity before invoking the recoverability pipeline. When that activity started a new trace, the recoverability pipeline — delayed retries and moves to the error queue — ran withActivity.Current == nullinstead of the ambient activity, for example the transport's receive span.Why
Activity.Stop()makes the activity that was current whenStart()ran current again (Activity._previousActiveActivity), notActivity.Parent. To start a new trace,ActivityFactory.StartActivityFromIncomingMessagesetsActivity.Current = nullbeforeStart()so the new span becomes a root — see the runtime discussion referenced in the code. That also makes the saved "previous" activitynull, so afterStop()nothing brings the ambient span back. The restore cannot happen inside the factory beforeStart(), becauseStart()would then adopt the ambient activity as the parent and the trace would not be new.The
ContinueExistingpath is unaffected: its activity has a remote parent butActivity.Currentwas not cleared, soStop()restores the ambient activity on its own.MainPipelineExecutoris also unaffected: its activity is disposed as the last statement ofInvoke, andAsyncLocalchanges do not flow back to the awaiting transport pump.Changes
ActivityExtensions.RestoreAmbientActivity(Activity?): setsActivity.Currentback, skipping a stopped activity, which the runtime would reject.RecoverabilityPipelineExecutor: capturesActivity.CurrentbeforeStartRecoverabilityActivityand restores it in afinallyonce the activity is disposed, so the restore also happens when the policy or the logger throws.RecoverabilityExecutorTests.Should_run_recoverability_pipeline_under_the_ambient_activity: starts an ambient activity, invokes the executor withtraceparentandNServiceBus.OpenTelemetry.StartNewTraceheaders, and asserts the recoverability pipeline observes the ambient activity asActivity.Current. Verified to fail without the fix.Testing
NServiceBus.Core.Testspass locally except the NativeAOT/packaging tests, which need a Release analyzer build not present in my checkout.