[SDK] Handle instrumentation object construction failures safely - #4469
[SDK] Handle instrumentation object construction failures safely#446922elix3r wants to merge 6 commits into
Conversation
Remove noexcept from SDK TracerProvider, LoggerProvider, MeterProvider, Tracer, Logger, and Meter constructors so initialization-time allocation failures can propagate. Keep GetTracer/GetLogger/GetMeter noexcept and return a pre-allocated noop object if constructing a new instrumentation object fails, without caching the failed attempt. Fixes open-telemetry#4361 Signed-off-by: elix3r <157088510+22elix3r@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4469 +/- ##
==========================================
- Coverage 83.12% 83.08% -0.03%
==========================================
Files 519 519
Lines 20256 20312 +56
==========================================
+ Hits 16835 16875 +40
- Misses 3421 3437 +16
🚀 New features to boost your workflow:
|
Signed-off-by: elix3r <157088510+22elix3r@users.noreply.github.com>
|
@dbarker Latest commits are up (format/IWYU follow-up + merge from main). CI is waiting on workflow approval again. |
There was a problem hiding this comment.
Pull request overview
This PR updates the OpenTelemetry C++ SDK to safely handle failures during instrumentation object construction while preserving noexcept guarantees for runtime GetTracer/GetLogger/GetMeter calls by returning a pre-allocated noop fallback when construction fails (with exception handling compiled in only when exceptions are enabled).
Changes:
- Remove
noexceptfrom SDK provider and instrumentation constructors so initialization failures can propagate to the caller. - Keep provider
Get*methodsnoexcept, adding guarded try/catch to log construction failures and return a pre-allocated noop object without poisoning caches. - Add unit tests for constructor exception specs and deterministic fault injection/recovery behavior (skipped when exceptions are disabled).
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| sdk/test/trace/tracer_provider_test.cc | Adds static_asserts for constructor exception specs and tests noop fallback + recovery for GetTracer. |
| sdk/test/metrics/meter_provider_sdk_test.cc | Adds static_asserts and tests noop fallback + recovery for GetMeter. |
| sdk/test/logs/logger_provider_sdk_test.cc | Adds static_asserts and tests noop fallback + recovery for GetLogger; introduces a new test processor. |
| sdk/src/trace/tracer.cc | Removes noexcept from Tracer constructor implementation. |
| sdk/src/trace/tracer_provider.cc | Adds pre-allocated noop tracer fallback and exception-guarded construction in GetTracer. |
| sdk/src/metrics/meter.cc | Removes noexcept from Meter constructor implementation. |
| sdk/src/metrics/meter_provider.cc | Adds pre-allocated noop meter fallback and exception-guarded construction in GetMeter. |
| sdk/src/logs/logger.cc | Removes noexcept from Logger constructor implementation. |
| sdk/src/logs/logger_provider.cc | Adds pre-allocated noop logger fallback and exception-guarded construction in GetLogger. |
| sdk/include/opentelemetry/sdk/trace/tracer.h | Removes noexcept from Tracer constructor declaration. |
| sdk/include/opentelemetry/sdk/trace/tracer_provider.h | Removes noexcept from TracerProvider constructors; adds stored noop tracer member. |
| sdk/include/opentelemetry/sdk/metrics/meter.h | Removes noexcept from Meter constructor declaration. |
| sdk/include/opentelemetry/sdk/metrics/meter_provider.h | Removes noexcept from MeterProvider constructors; adds stored noop meter member. |
| sdk/include/opentelemetry/sdk/logs/logger.h | Removes noexcept from Logger constructor declaration. |
| sdk/include/opentelemetry/sdk/logs/logger_provider.h | Removes noexcept from LoggerProvider constructors; adds stored noop logger member. |
| CHANGELOG.md | Documents the SDK behavior change for constructor exception specs and noop fallback behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: elix3r <157088510+22elix3r@users.noreply.github.com>
Restore CountingProcessor ownership of the OnEmit record via std::move. Add the includes IWYU asked for, and mark empty logging catches so clang-tidy does not count them against the unique-warning limit. Signed-off-by: elix3r <157088510+22elix3r@users.noreply.github.com>
…er-construction-4361 Signed-off-by: elix3r <157088510+22elix3r@users.noreply.github.com>
|
@dbarker IWYU and clang-tidy should be clean now. CountingProcessor uses std::move again, and the branch is merged onto latest main. Ready for another look when you have a minute. |
Fixes #4361
Changes
Provider construction happens during SDK initialization. Runtime
GetTracer/GetLogger/GetMetercalls must not throw. This change follows the post-SIG direction from @dbarker:noexceptfrom the SDK constructors forTracerProvider,LoggerProvider,MeterProvider,Tracer,Logger, andMeter. Allocation or initialization failures during provider construction can now propagate so the caller (often the configuration library) can handle them.GetTracer,GetLogger, andGetMeternoexcept. If constructing a previously unseen tracer/logger/meter fails, the provider catches the exception (when exceptions are enabled), logs viaOTEL_INTERNAL_LOG_ERRORwithout letting logging escape, and returns a valid noop API object.new NoopTracer/NoopLogger/NoopMeterstored as anostd::shared_ptr). Runtime failure handling does not allocate a fresh noop object.Get*call can retry and return a normal SDK object once the failure is gone. Existing cached objects continue to be returned unchanged.shared_ptrcontrol-block allocation, cache insertion, and metricsAddMeter.Get*signatures are unchanged. Try/catch is compiled only whenOPENTELEMETRY_HAVE_EXCEPTIONSis set.Tests added
noexceptstatic_asserts for each signal.ScopeConfiguratormatcher that throws for a named scope.Get*returns a valid noop-compatible object, using it produces no telemetry, the cache is not poisoned, cached objects are unchanged, and a later call recovers when the injected failure is removed.Validation
tools/format.shwas not run:clang-formatis not installed in this environment.CHANGELOG.mdupdated for non-trivial changesGet*contracts unchanged)