Skip to content

GH-4398: link each retry attempt to the failed attempt before it - #4405

Merged
jeremydmiller merged 2 commits into
mainfrom
gh-4398-retry-activity-links
Sep 10, 2026
Merged

jeremydmiller merged 2 commits into
mainfrom
gh-4398-retry-activity-links

Conversation

@jeremydmiller

@jeremydmiller jeremydmiller commented Sep 10, 2026

Copy link
Copy Markdown
Member

Closes #4398. Supersedes #4402 by @outofrange-consulting, who is credited as co-author on the commits. That PR got the design right: link the next attempt to the failed one with an ActivityLink instead of reparenting it, which matches the OpenTelemetry messaging conventions and what MassTransit and Brighter do. This PR keeps that design and changes two things: where the link lands, and how it is carried.

What changed compared with #4402

1. The link goes on the span the handler runs in. #4402 used up the link in StartEnvelopeActivity. That method serves send, receive and execute spans, and every receiver starts its receive span before the pipeline starts the execute span:

  • BufferedReceiver
  • DurableReceiver
  • NativeAckReceiver
  • InlineReceiver

So on a buffered, durable or NativeAck listener, the short receive span took the link and cleared it, and the process span got nothing. Here the link is attached in HandlerPipeline.InvokeAsync(envelope, channel, activity), the one place every attempt goes through, using .NET 9's Activity.AddLink. On Inline and NativeAck listeners the receiver passes its receive span in as the processing span, so this covers those too.

2. A loose header, not a mapped Envelope property, so messages that are never retried pay nothing. A mapped property adds one more reserved-header read for every incoming message in every transport mapper, plus a new Envelope field and serializer work. Custom headers already round-trip through every mapper and through EnvelopeSerializer. The pipeline check is HasHeaders first, so it never creates the lazy headers dictionary. It also makes Envelope.Reset() pool-safe without a new line, because _headers is already cleared there.

The header is removed before the handler runs. So a handler, and anything it cascades or propagates with PropagateIncomingHeadersToOutgoing, never sees it.

Known limitation

A durable-inbox reschedule (RescheduleExistingEnvelopeForRetryAsync) only UPDATEs the row's time, attempts and status. It does not rewrite the stored body, so the link does not survive that path. Supporting it would mean rewriting the body on every durable retry across all the message stores. That is a separate decision about cost, and this PR does not make it. The in-memory scheduler, local queues and broker requeues all carry the link.

Tests

  • End to end: TracingTests/retry_attempt_links. A real Wolverine host runs through the real OpenTelemetry SDK into OpenTelemetry.Exporter.InMemory, a new test-only dependency pinned to 1.15.3 like the other OTel packages. The assertions are on the spans that were actually exported.
    • The matrix covers every failure policy on every listener shape:

      Local queue (Buffered) RabbitMQ Inline RabbitMQ Buffered
      Retry now
      Scheduled retry
      Requeue

      Inline and Buffered reach the pipeline by different routes: Inline hands its receive span over as the handler's span, while Buffered starts a separate receive span first. Durable behaves like Buffered here, so Buffered stands in for both. Local queues can't run Inline, so the Inline column is RabbitMQ.

    • Each cell checks that:

    • Attempts are identified by the span the handler actually ran in, not by span name. A retry-now on an Inline listener reruns the pipeline in a new process span, and with no message store a scheduled retry comes back through the local replies queue.

    • A first attempt that succeeds has no links, on each listener.

    • Negative control: against origin/main's HandlerPipeline, exactly the 9 link cells fail and the 3 first-attempt cells pass.

  • Unit tests, adapted from Link retry attempts to the prior failed attempt's Activity via ActivityLink #4402: each continuation records the failed attempt, and LinkToPreviousAttempt links, uses up the header, ignores malformed ids and handles a missing activity.
  • Local runs:
    • CoreTests: 2910 passed, 2 skipped, 0 failed
    • TracingTests: 19 of 19 passed
    • dotnet build wolverine.slnx -c Release -f net9.0: 0 errors, 0 warnings

🤖 Generated with Claude Code

https://claude.ai/code/session_01VDUrBeB4tTnKj4AExCS1nj

Supersedes #4402. Keeps that PR's design -- the next attempt carries an
ActivityLink to the failed one rather than being reparented under it, per the
OpenTelemetry messaging conventions -- and changes where the link lands and
how it is carried.

- RetryInlineContinuation, ScheduledRetryContinuation and RequeueContinuation
  record the failed attempt's Activity.Id in a loose Envelope.Headers entry.
- HandlerPipeline.InvokeAsync(envelope, channel, activity) links it onto the
  span the handler runs in, then drops the header so the link is single-use.
  That overload is the one place every attempt converges; linking at span
  start (StartEnvelopeActivity) handed the link to whichever span a receiver
  started first -- the short "receive" span on buffered, durable and
  NativeAck listeners -- and left the process span with none.
- A loose header instead of a mapped Envelope property, so an envelope that
  is never retried pays nothing: no extra reserved-header read per incoming
  message in every transport mapper, no new Envelope field, nothing in the
  serializer. Custom headers already round-trip through every mapper and
  EnvelopeSerializer.

Known limit: a durable-inbox reschedule only UPDATEs the row's time/status,
it does not rewrite the stored body, so the link does not survive that path.

Tests: TracingTests/retry_attempt_links runs a real host through the
OpenTelemetry SDK into the in-memory exporter (new
OpenTelemetry.Exporter.InMemory test dependency) and asserts on exported
spans for retry-now, scheduled retry and requeue on a local queue, and
requeue across Rabbit MQ on both Inline and BufferedInMemory listeners.
With the pipeline hook removed, all five link tests fail.

Co-authored-by: Geoffrey Marc <7069588+outofrange-consulting@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VDUrBeB4tTnKj4AExCS1nj
Turns the end-to-end retry-link tests into a matrix: retry now, scheduled
retry and requeue, each against a buffered local queue and against Rabbit MQ
listeners in both Inline and BufferedInMemory mode. The two listener shapes
reach the handler pipeline by different doors -- Inline hands its receive
span to the pipeline as the handler's span, Buffered starts a separate
receive span first -- and Durable behaves like Buffered here, so Buffered
stands in for both. Local queues cannot run Inline, so Rabbit MQ covers that
half.

Which span an attempt "is" differs by path: a retry-now on an Inline
listener re-runs the pipeline in a new process span, and with no message
store a scheduled retry comes back through the local replies queue. So the
handler now records the span it actually ran in, and the tests assert those
spans chain -- and that no other span for the message carries a link.

Negative control against origin/main's HandlerPipeline: exactly the 9 link
cells fail, the 3 first-attempt cells pass.

Co-authored-by: Geoffrey Marc <7069588+outofrange-consulting@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VDUrBeB4tTnKj4AExCS1nj
@jeremydmiller
jeremydmiller merged commit 54838c2 into main Sep 10, 2026
42 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.

[OTEL] Retries don't correlate to the failed attempt they follow - sibling spans, not linked spans

1 participant