Add tracing activities for topology and channel recovery

[lukebakken]

Note

This issue was written by Claude (Anthropic's Claude Code) under the direction of @lukebakken, who reviewed it before filing. The code references and line numbers were verified against main, but the design options below are AI-drafted starting points, not settled decisions — treat them as such.

Background

#1731 asked for an Activity per connection attempt, with the exception recorded on failure. #1734 delivered that: a RabbitMQ.Client.Connection activity source emitting a connection attempt span per CreateConnectionAsync, a child tcp connection attempt span per endpoint tried, and a separate connection attempt span tagged is_reconnection=true for auto-recovery (AutorecoveringConnection.Recovery.cs).

That issue's text also said "This also applies to auto-recovering channels." The connection half of recovery is instrumented; the topology recovery half is not. Filing separately rather than holding #1731 open, because the span shape here needs design work that the connection case did not.

Current state

TryRecoverConnectionDelegateAsync (AutorecoveringConnection.Recovery.cs:254) is the only recovery method with an Activity. Everything that happens after the socket is re-established is untraced:

• TryPerformAutomaticRecoveryAsync (:180) — the orchestrator; runs the documented 4-step sequence (exchanges, queues, bindings, consumers) plus channel recovery
• RecoverExchangesAsync (:298)
• RecoverQueuesAsync (:350)
• RecoverBindingsAsync (:464)
• RecoverConsumersAsync (:516)
• RecoverChannelsAndItsConsumersAsync (:603)
• AutorecoveringChannel.AutomaticallyRecoverAsync (AutorecoveringChannel.cs:153) — re-opens the channel, replays BasicQos / TxSelect, then recovers that channel's consumers

grep -n Activity projects/RabbitMQ.Client/Impl/AutorecoveringChannel.cs returns nothing today.

So from a telemetry consumer's point of view, a recovery currently looks like a single connection attempt span that succeeds, followed by silence — even though the interesting failures happen afterwards. Topology recovery is exactly where recovery goes wrong in practice: a queue that no longer exists, a binding to a deleted exchange, a consumer whose queue was removed. Those go to HandleTopologyRecoveryException and the ConnectionRecoveryError / TopologyRecovery* event handlers, and are invisible to a tracing backend.

The design problem

The reason this is not a mechanical copy of the connection instrumentation: cardinality. A single recovery can fan out to hundreds of spans — one connection has up to 2047 channels, and each recorded exchange, queue, binding, and consumer is recovered in its own loop iteration. Naively wrapping every Recover* loop body in an activity would make a recovery on a large topology dominate the trace and could be a real cost during an outage, when recovery is retried on an interval.

Options worth weighing:

1. One span for the whole topology recovery, child of the connection attempt span, with counts as tags (exchanges.recovered, queues.recovered, bindings.recovered, consumers.recovered, and the corresponding .failed counts). Cheapest, bounded, and probably answers "did recovery fully succeed?" — which is the actual operational question.
2. A span per phase (exchanges / queues / bindings / channels+consumers), 4-5 spans per recovery, each with counts. Still bounded; pinpoints which phase failed.
3. A span per recovered entity. Highest fidelity, unbounded cardinality. If offered at all it should be opt-in via RabbitMQTracingOptions, not default-on.
4. Events rather than spans on the parent recovery span for individual failures — records which entity failed without a span per entity. Composes with 1 or 2.

My leaning is 2 + 4: per-phase spans with counts, and an exception event per failed entity on the enclosing phase span. But this needs a decision, not just an implementation.

Additional questions to settle

• Parentage. Recovery is driven by an internal recovery loop task, not by user code, so there is no meaningful ambient parent. Should the topology spans be children of the is_reconnection=true connection attempt span (which is currently using-disposed at the end of TryRecoverConnectionDelegateAsync, i.e. before topology recovery even starts), or should the parent span be widened to span the whole recovery? Widening it changes the meaning of the existing span's duration, so this is a compatibility consideration if it lands after 7.3.0 ships.
• Status semantics. feat: add activity on connection #1734's review found that recording an exception without also calling SetStatus(ActivityStatusCode.Error) leaves the span looking successful to backends. Same trap applies here, with an extra wrinkle: topology recovery deliberately tolerates some failures (TopologyRecoveryFilter, TopologyRecoveryExceptionHandler). A handled-and-tolerated failure arguably should not mark the span Error. Needs an explicit rule.
• Per-channel recovery. Should AutomaticallyRecoverAsync get its own span? It does real broker round-trips (channel.open, up to two basic.qos, optionally tx.select) and is the natural parent for that channel's consumer recovery. This is per-channel, so cardinality is bounded by RequestedChannelMax.
• New tag names. The connection spans already needed client-specific tags (messaging.rabbitmq.connection.is_reconnection, messaging.rabbitmq.connection.automatic_recovery) because the OpenTelemetry messaging conventions do not cover connection establishment — they cover recovery even less. Any new tags are effectively our own convention and should be named deliberately.

Testing

Tests belong in projects/Test/SequentialIntegration/ (ActivityRecorder installs a process-global ActivityListener; the Integration project runs its tests in parallel, so recorders there see other tests' activities). Toxiproxy-based tests in projects/Test/Integration/TestToxiproxy.cs are the existing way to force a real recovery — that tension will need resolving, most likely by driving recovery from the sequential project.

Related

• Consider adding connection Activities to RabbitMQActivitySource #1731 — the original connection-activity request (connection half done, closed)
• feat: add activity on connection #1734 — the implementation
• Review the entire OpenTelemetry tracing implementation #1967 — review of the entire OpenTelemetry tracing implementation; should land first, since it may change the tracing public surface and establish the guard/status conventions this work would follow

[lukebakken]

Note

This comment was written by Claude (Anthropic's Claude Code) under the direction of @lukebakken, who asked for the most active tracing contributors to be brought in. The ranking below was computed from the GitHub API and git log; the choice of who made the cut was Claude's.

Pulling in the people with the most history on this client's tracing implementation, since the open question here is a design decision (span granularity and parentage for topology recovery) rather than an implementation detail, and getting it wrong bakes unbounded span cardinality into a released library.

Ranked by interactions across the tracing-related issues and PRs in this repo (#776, #1261, #1478, #1528, #1533, #1591, #1592, #1595, #1621, #1633, #1666, #1715, #1716, #1717, #1728, #1731, #1734, #1779, #1849, #1852, #1967), counting authorship, comments, and commits to the tracing source files:

• @stebet — built the original implementation (Add OpenTelemetry support via ActivitySource #1261) and the OpenTelemetry registration/propagation work (Adding proper OpenTelemetry integration via. registration helpers and better context propagation #1528); by far the most history here, across 7 separate threads.
• @michaelklishin — second most active after me, across 7 threads including the conventions discussions (OpenTelemetry: Update messaging.operation span attribute to latest OTel Semantic Conventions #1715, Change OTel attribute messaging.operation to messaging.operation.type #1716, More updates for the current OTel (OpenTelemetry) conventions #1717) and Ship RabbitMQ.Client.OpenTelemetry 1.0.0 #1728.
• @iinuwa — brought the attributes in line with current OTel messaging semantic conventions (OpenTelemetry: Update messaging.operation span attribute to latest OTel Semantic Conventions #1715, Change OTel attribute messaging.operation to messaging.operation.type #1716, More updates for the current OTel (OpenTelemetry) conventions #1717). Directly relevant, since recovery is not covered by those conventions at all, so anything added here becomes our own convention.
• @lmolkova — weighed in on the spec/conventions questions in Add OpenTelemetry support via ActivitySource #1261, Consider allowing to set remote span as parent instead of link when propagating context via RabbitMQ #1666 and OpenTelemetry: Update messaging.operation span attribute to latest OTel Semantic Conventions #1715.
• @davidfowl — early tracing design discussion in Evaluating how to support tracing and OpenTelemetry #776 and Add OpenTelemetry support via ActivitySource #1261.
• @eerhardt — opened Consider adding connection Activities to RabbitMQActivitySource #1731, the parent of this issue, based on the .NET Aspire connection-attempt span.
• @aygalinc — implemented the connection spans in feat: add activity on connection #1734 that this builds on.
• @bollhals — Evaluating how to support tracing and OpenTelemetry #776, Add OpenTelemetry support via ActivitySource #1261, RabbitMQActivitySource.Deliver cannot be used by types that implement Consumer #1621.

No obligation to engage — but if any of you have opinions on the four options in the description (one span per recovery / per phase / per entity / events on a parent span), or on whether the existing is_reconnection=true span should be widened to cover topology recovery rather than ending before it starts, that input is worth more now than after 7.3.0 locks the surface in.

Related: @Kielek, @Tornhoof, @josh-romanowski, @meum, @martinjt, @cijothomas, @lailabougria, @danielmarbach and @YayBurritos have all filed or reviewed tracing work here too — feel free to chime in.

[danielmarbach]

I would recommend @tmasternak