Azure / Azure/azure-sdk-for-cpp
TracingContextFactory allocates when no tracer is configured
- Dominant language
- C++
- Stars
- 205
- Forks
- 172
- Avg merge
- 1d 3h
- Merged PRs (30d)
- 37
Description
## Summary
`Azure::Core::Tracing::_internal::TracingContextFactory::CreateTracingContext` allocates on the no-tracer path. Every call that receives a caller context without the factory performs two heap allocations, even when the application configured no tracing provider. A service that does not use the HTTP pipeline pays that cost for every top-level operation and gets nothing back.
## Motivation
`CreateTracingContext` calls `context.WithValue(TracingFactoryContextKey, this)` at `sdk/core/azure-core/src/tracing/tracing.cpp:66-68` before it tests `HasTracer()`, and `Context::WithValue` performs two `make_shared` calls at `sdk/core/azure-core/inc/azure/core/context.hpp:289-293`. The comment at `tracing.cpp:61-64` justifies the unconditional injection by saying the factory also builds the User-Agent header. That justification is stale. At commit 99f45a5f1 `RequestActivityPolicy` read `GetUserAgent()` from the injected factory, but `GetUserAgent` no longer exists anywhere in `sdk/core`, and `TelemetryPolicy` now produces the header on its own at `sdk/core/azure-core/src/http/telemetry_policy.cpp:16-21`. The only remaining consumer of the injected factory is `RequestActivityPolicy`, which acts only when `tracingFactory && tracingFactory->HasTracer()` at `sdk/core/azure-core/src/http/request_activity_policy.cpp:28`, so a factory without a tracer has no observable effect. `WithValue` returns a child context and never changes the caller context, so two calls that share one caller context both miss the guard and both allocate. An AMQP service such as `azure-messaging-eventhubs` builds a fresh caller context for each operation, which measures at two allocations for every send and every receive.
## Proposal
- Return `TracingContext{context, ServiceSpan{}}` at the top of `CreateTracingContext` in `sdk/core/azure-core/src/tracing/tracing.cpp` when `HasTracer()` is false, and delete the dead `else` branch at lines 98-101. Copying the context copies one `shared_ptr` and does not reach the heap.
- Keep the guarded injection and the span creation unchanged for the tracer-configured path, so HTTP clients behave exactly as they do today.
- Replace the stale comment at `tracing.cpp:61-64` with one that names `RequestActivityPolicy` as the consumer and records that `TelemetryPolicy` owns the User-Agent header.
- Correct the `TelemetryPolicy` doc comment at `sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp:546-550`, which claims User-Agent generation moved into `RequestActivityPolicy`. That merger was reverted, and the stale claim invites a future reader to restore the injection.
- Add a test near `BasicServiceSpanTests` in `sdk/core/azure-core/test/ut/service_tracing_test.cpp` that makes sure `CreateFromContext` returns null for a context built with no provider, and returns a factory with `HasTracer()` true when a provider is set.
- Add a CHANGELOG entry under "Other Changes" recording that `CreateTracingContext` no longer allocates when no tracer is configured.
Rejected alternatives. An opt-out constructor flag works, but it adds a data member to a class whose inline definition ships in a public header, and it forces every non-HTTP service to opt in by hand. Passing the factory to `RequestActivityPolicy` outside the context would let the injection be deleted for everyone, but the pipeline constructor carries no service name, which `az.namespace` needs, so it requires plumbing through every service that builds a pipeline. That route remains the right long-term direction if User-Agent duty ever returns to `RequestActivityPolicy`.
## Validation
- Both symbols live in `Azure::Core::Tracing::_internal`, and a repository-wide search for `TracingFactoryContextKey` and `CreateFromContext` finds one non-test consumer, so no public header changes and no API or ABI break.
- Every existing `RequestActivityPolicy` test configures a tracing provider, at `sdk/core/azure-core/test/ut/request_activity_policy_test.cpp:161-246`, so no current test pins the no-tracer injection.
Contributor guide
Assessment
This issue has not been assessed yet.