feat(eventhubs): add distributed tracing to the clients - #7339
feat(eventhubs): add distributed tracing to the clients#7339Johnathan W (j7nw4r) wants to merge 12 commits into
Conversation
Add eventhubs_tracing_test.cpp with the recording tracer, span, and attribute set doubles for the Event Hubs tracing work in issue 7336. The tests pin the TracingProvider option on ProducerClientOptions and ConsumerClientOptions, the eager tracer creation in both clients, the ProducerClient.Send span shape on a cancelled context and on a non-cancel exception, and the shape of the shared StartSpan helper for PartitionClient.ReceiveEvents. Tests 1 to 7 do not compile until the tracing source lands. The TracingProvider member does not exist yet, and the private header src/private/eventhubs_tracing.hpp does not exist yet. SendWithoutProviderIsUnchanged is a characterization test. It passes today. It guards the no-provider path against a null dereference after the source lands.
ProducerClientOptions and ConsumerClientOptions gain a TracingProvider field. Each client builds a TracingContextFactory from that field at construction time. When the field is empty, the factory has no tracer and the clients create no spans. ProducerClient::Send creates one Producer span named ProducerClient.Send. PartitionClient::ReceiveEvents creates one Client span named PartitionClient.ReceiveEvents. Both spans carry the messaging attributes, the az.namespace attribute, and the message count. Both spans record an exception event and an error status, then rethrow the original exception. A new private helper, src/private/eventhubs_tracing.hpp, holds the factory creation and the span shape, so the producer and the partition client emit the same attributes. EventDataBatch::NumberOfEvents is now const, and its mutex is mutable, because Send takes the batch by const reference. Trace-context propagation and span links stay deferred, because Azure Core has no API for them.
Run clang-format-11 over eventhubs_tracing_test.cpp. The declarations of AddAttributes and SetStatus on the recording span double broke the Validate Clang Format step in CI. The change is formatting only. No assertion, no test name, and no behavior changed.
|
Azure Pipelines: Successfully started running 3 pipeline(s). 7 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Adds provider-neutral distributed tracing to Event Hubs producer and consumer operations.
Changes:
- Adds tracing-provider configuration and shared span helpers.
- Instruments send and receive operations with messaging attributes and error recording.
- Adds tracing tests, build integration, and changelog documentation.
Public API and tracing architecture changes require maintainer review.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
CHANGELOG.md |
Documents distributed tracing support. |
CMakeLists.txt |
Builds tracing helper sources. |
inc/azure/messaging/eventhubs/consumer_client.hpp |
Adds consumer tracing configuration. |
inc/azure/messaging/eventhubs/event_data_batch.hpp |
Makes event-count access const-safe. |
inc/azure/messaging/eventhubs/partition_client.hpp |
Stores receive tracing context. |
inc/azure/messaging/eventhubs/producer_client.hpp |
Adds producer tracing configuration. |
src/consumer_client.cpp |
Propagates tracing into partition clients. |
src/eventhubs_tracing.cpp |
Implements span creation and attributes. |
src/partition_client.cpp |
Instruments receive operations. |
src/private/eventhubs_tracing.hpp |
Declares shared tracing helpers. |
src/private/eventhubs_utilities.hpp |
Extends partition-client factory inputs. |
src/producer_client.cpp |
Instruments send operations. |
test/ut/CMakeLists.txt |
Registers tracing tests. |
test/ut/eventhubs_tracing_test.cpp |
Tests tracing options and span behavior. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
ProducerClient::Send converted the batch to an AMQP message before it started the span. An empty batch makes that conversion throw, so the send failed before a span existed and nothing recorded the failure. The conversion now runs at the top of the try block, after the span starts. The existing catch clause adds the exception to the span and rethrows, the same as every other send failure.
The OpenTelemetry semantic conventions at schema 1.17.0 define messaging.batch.message_count as an int. The Event Hubs tracing helper records it as a string, and the test doubles could not show the difference, because they stringified every overload into one map. TestAttributeSet and TestSpan now record a type tag for each attribute. The tag names the overload that delivered the value. Three tests use it: the receive span count and the send span count must arrive through the uint64 overload, and SetMessageCount must stay quiet and safe when the factory has no tracer. The first two tests are red today. The third one is a characterization test that guards the null attribute set hazard.
The batch message count now goes out as an unsigned integer attribute, which matches the OpenTelemetry semantic conventions at schema 1.17.0. Before this change the helper wrote the count as a string. SetMessageCount takes the tracing context factory, because the typed attribute needs an attribute set. The helper guards the null attribute set on the no-tracer path, because the factory returns nullptr when it has no tracer and that pointer is not null safe.
The span helper built a std::string temporary for each attribute name before it added the attribute. Two of the names are longer than the libc++ small-string limit, so each call heap allocated even when the client had no tracer and dropped every attribute. The span helper now returns the tracing context immediately when the factory has no tracer. The message count helper keeps its own null attribute set guard, because PartitionClient::ReceiveEvents calls it directly. The remaining two allocations per operation come from azure-core and are tracked in issue 7340.
The SingleSpan helper called front() on the tracer list after a non-fatal EXPECT_EQ on the list size. When no tracer was created, that call dereferenced an empty std::list and the test process ended in a segmentation fault. The crash stopped the run, so the tests after the first affected one never reported at all. The helper now returns null when the tracer list is not the expected size, so a failure reports through the caller's ASSERT_NE assertion rather than a crash. This mirrors the guard that already protects the spans list two lines later.
The receive span of PartitionClient::ReceiveEvents has no offline coverage. A partition client comes only from a factory that opens a live AMQP link, so no unit test can reach the span, the message count, or the exception path. This test runs against a real namespace. It gives the consumer client a recording tracing provider, receives events from a partition, and makes sure the span name, the span kind, the messaging attributes, and the message count are correct. The count assertion compares the span attribute with the size of the vector that ReceiveEvents returned. The recording tracing test doubles move from eventhubs_tracing_test.cpp into a shared header, because the live test lives in a different file.
The two convenience Send overloads build the batch before they send it, and CreateBatch opens the connection, the session and the AMQP link. A bad host fails there. Only the batch overload starts a span today, so that failure unwinds with no span at all. Add two offline tests that send one event and a vector of three events against a fake connection string. Each test asserts one span named ProducerClient.Send with the kind Producer, the message count as a uint64 attribute, one recorded event and the Error status. Both fail today because the provider holds zero spans. Add a characterization test that keeps the no-provider path unchanged, and a live test that sends one event and asserts a single span. The live test catches a nested second span, which an offline test cannot reach because CreateBatch always throws offline.
The convenience Send overloads made the batch first, and the batch creation opens the connection, the session, and the AMQP link. A bad host or a failed authentication threw there, and the stack unwound before a span existed. A caller then saw no producer span for the failed send. Each public Send overload now starts one span, and one try/catch records the exception on that span. A new private method, SendBatchInSpan, holds the send body and starts no span, so one logical send still makes one span. The convenience overloads pass the span context to CreateBatch, which puts the link open under the span.
Johnathan W (j7nw4r)
left a comment
There was a problem hiding this comment.
Every review finding is fixed, and each fix is in its own commit. The empty-batch send now records a span, the batch message count goes out as an unsigned integer, the no-tracer path skips the attribute work, the test helper no longer crashes on an empty tracer list, and every ProducerClient::Send overload now creates exactly one span that covers the batch creation. The playback suite went from 94 tests to 111, all passing.
Two findings need work that does not belong here. Azure Core injects the tracing factory into the context even with no tracer, which costs 2 heap allocations for each operation, and the short-circuit cannot be written from a service package because ServiceSpan's default constructor is private. That is issue #7340. The send and receive paths also have no offline test seam, because MessageSender and MessageReceiver are both final types that only a live AMQP link produces, so azure-core-amqp has to publish its existing mock server before those spans can be checked in CI. That is issue #7341.
Two gaps stay open in this pull request, and I would rather state them than leave them implied. The receive span is covered only by a live-only test that CI skips. The rule that one logical Send makes one span is also live-only, because CreateBatch throws offline before the send logic runs, so no test that CI runs can tell a correct implementation from one that nests a second span. Both close when #7341 lands.
The measurements behind these claims are real rather than reasoned. I counted the allocations with a probe against the compiled library, reproduced the test crash and then removed it, and deleted the receive span block to confirm the suite never noticed.
|
Azure Pipelines: Successfully started running 3 pipeline(s). 7 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
The tracing change added a TracingProvider field to ProducerClientOptions and ConsumerClientOptions, but no document told a user how to set it. The only repo document on tracing, doc/DistributedTracing.md, showed clientOptions.Telemetry.TracingProvider. That line does not compile for Event Hubs, because the two option structs do not derive from Azure::Core::_internal::ClientOptions and have no Telemetry field. - Adds a "Distributed tracing" section to the Event Hubs README with a snippet that builds an OpenTelemetryProvider and sets TracingProvider on both client options. - Lists the span names, the span kinds, and the attributes that the clients emit, and names the 1.17.0 semantic conventions schema. - States that the client creates no spans when TracingProvider is not set, because there is no global fallback provider. - Adds a note to doc/DistributedTracing.md for the clients that declare TracingProvider at the top level of their own options structure.
Summary
The Event Hubs C++ SDK emits no distributed tracing spans. Producer and consumer operations stay invisible to OpenTelemetry-based observability pipelines. This change adds provider-neutral spans on send and receive, built only on the azure-core tracing abstraction.
Motivation
Distributed tracing is a GA requirement in the Azure SDK guidelines. Without spans on send and receive, users cannot correlate Event Hubs traffic with the rest of a traced application. The azure-core tracing abstraction already provides a provider-neutral API, so the instrumentation adds no new dependency.
Changes
std::shared_ptr<Azure::Core::Tracing::TracerProvider> TracingProviderfield toProducerClientOptionsandConsumerClientOptions.ProducerClient::Sendoverload creates exactly one spanProducerClient.SendwithSpanKind::Producer. The span covers the batch creation, which opens the AMQP link, and the whole retry loop, so a connection failure is recorded and retries add no extra top-level span.PartitionClient::ReceiveEventscreates one spanPartitionClient.ReceiveEventswithSpanKind::Client; the receive count is the size of the returned vector, not the internal prefetch count.az.namespace,messaging.system,messaging.destination.name,messaging.operation,messaging.batch.message_count,net.peer.name.server.addressis not used because it post-dates schema 1.17.0.azure-messaging-eventhubs-cppwith the package version.TracingProviderset, no span is created and no tracing data is recorded. The span helper returns before it adds any attribute, so the path costs 2 heap allocations for each operation. Both come from the child context that azure-core derives, and issue TracingContextFactory allocates when no tracer is configured #7340 tracks removing them.src/private/eventhubs_tracing.hppandsrc/eventhubs_tracing.cpp.EventDataBatch::NumberOfEvents()becomesconstand its mutex becomesmutable, becauseSendtakesEventDataBatch const&and must read the batch count. This is source-compatible.messaging.batch.message_countgoes out as an unsigned integer attribute, which matches the type the 1.17.0 convention defines.test/ut/eventhubs_tracing_test.cppwith local test doubles; they need no OpenTelemetry and no live service.test/ut/consumer_client_test.cpp, because aPartitionClientneeds a live AMQP link.vcpkg.jsonis untouched.Deferred to follow-up work: cross-message trace-context propagation, remote-parent extraction, and span links for batches are not in this pull request. Azure Core has no API for them today; the only propagation API is
Span::PropagateToHttpHeaders, andCreateSpanOptionshas noLinksmember. Issue #7336 asks for these to be designed in Azure Core or tracked as a follow-up rather than bypassed with Event Hubs-specific OpenTelemetry calls.Cross-language note: the .NET SDK emits a send span for Event Hubs and sets
az.namespacetoMicrosoft.EventHub, which this change matches. The .NET SDK creates no receive span on the pull path, so thePartitionClient.ReceiveEventsspan follows the OpenTelemetry messaging conventions rather than a .NET precedent. The Goazeventhubspackage ships no tracing today.Known limitations: the
PartitionClient::ReceiveEventsspan is covered only by a live-only test, because aPartitionClientis reachable only through a factory that opens a real AMQP link. CI runs in playback and skips that test, so the receive span has no coverage in CI. An offline seam needs a change in azure-core-amqp, tracked in issue #7341.Test plan
AZURE_TEST_MODE=PLAYBACK: 100% tests passed, 0 tests failed out of 111 (the 94-test baseline plus the 17 new tests). The live-only receive-span test is skipped in playback and is unverified.clang-format-11 --dry-run -Werroris clean over every changed C++ file.Closes #7336