diff --git a/.github/workflows/clang-tidy.yaml b/.github/workflows/clang-tidy.yaml index c405aaefdd..f4ec876e30 100644 --- a/.github/workflows/clang-tidy.yaml +++ b/.github/workflows/clang-tidy.yaml @@ -17,9 +17,9 @@ jobs: matrix: include: - cmake_options: all-options-abiv1-preview - warning_limit: 73 + warning_limit: 63 - cmake_options: all-options-abiv2-preview - warning_limit: 76 + warning_limit: 66 env: CC: /usr/bin/clang-22 CXX: /usr/bin/clang++-22 diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ef13659ef..cc2e9c251d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,17 +73,9 @@ Increment the: conversion. No API or ABI change. [#4347](https://github.com/open-telemetry/opentelemetry-cpp/issues/4347) -Breaking changes: +* [CODE HEALTH] Fix bugprone-exception-escape in NoopTracer::StartSpan() + [#4673](https://github.com/open-telemetry/opentelemetry-cpp/pull/4673) -* [SDK] Remove `noexcept` from the public SDK `TracerProvider`, - `LoggerProvider`, `MeterProvider`, `Tracer`, `Logger`, and `Meter` - constructors so initialization failures can propagate to callers. - `GetTracer`, `GetLogger`, and `GetMeter` remain `noexcept` and return a - pre-allocated noop object if constructing a new instrumentation object - fails. After the first failure, later uncached `Get*` calls return the same - noop immediately without retrying construction. Existing cached objects - continue to be returned. - [#4361](https://github.com/open-telemetry/opentelemetry-cpp/issues/4361) * [BUG] Send one request per curl session, rather than replacing the operation a running request still belongs to [#4396](https://github.com/open-telemetry/opentelemetry-cpp/issues/4396) @@ -95,6 +87,7 @@ Breaking changes: * [BUG] Do not queue a curl session closed by the Retry-After cap [#4632](https://github.com/open-telemetry/opentelemetry-cpp/pull/4632) + * [BUG] Fix a data race on the curl operation's last result code [#4618](https://github.com/open-telemetry/opentelemetry-cpp/pull/4618) @@ -114,14 +107,42 @@ Breaking changes: * [BUG] Keep the curl retry deadline stable within each attempt [#4452](https://github.com/open-telemetry/opentelemetry-cpp/pull/4452) +Breaking changes: + +* [SDK] Handle instrumentation object construction failures safely + [#4469](https://github.com/open-telemetry/opentelemetry-cpp/pull/4469) + * Removed `noexcept` from the public SDK `TracerProvider`, + `LoggerProvider`, `MeterProvider`, `Tracer`, `Logger`, and `Meter` + constructors so initialization failures can propagate to callers. + * `GetTracer`, `GetLogger`, and `GetMeter` remain `noexcept` and return a + pre-allocated noop object if constructing a new instrumentation object + fails. + * After the first failure, later uncached `Get*` calls return the same + noop immediately without retrying construction. Existing cached objects + continue to be returned. + * [REMOVAL] Remove WITH_OTLP_RETRY_PREVIEW - #4668 [#4668](https://github.com/open-telemetry/opentelemetry-cpp/pull/4668) + * CMake flags WITH_OTLP_RETRY_PREVIEW is removed, + the feature is no longer in preview status. * [API] Remove opentelemetry::plugin [#4660](https://github.com/open-telemetry/opentelemetry-cpp/pull/4660) + * The unused opentelemetry::plugin framework is removed. * [REMOVAL] Remove WITH_OTLP_GRPC_SSL_MTLS_PREVIEW [#4666](https://github.com/open-telemetry/opentelemetry-cpp/pull/4666) + * CMake flags WITH_OTLP_GRPC_SSL_MTLS_PREVIEW is removed, + the feature is no longer in preview status. + +* [CODE HEALTH] Fix bugprone-exception-escape in NoopTracer::StartSpan() + [#4673](https://github.com/open-telemetry/opentelemetry-cpp/pull/4673) + * The NoopSpan class in the API has changed. + * For applications only instrumenting code, this has no impact, + as this class is not used for instrumentation. + * For applications implementing a custom trace exporter, + the exporter implementation needs to be adjusted, + when using the NoopSpan. ## [1.29.0] 2026-09-13 diff --git a/api/include/opentelemetry/trace/noop.h b/api/include/opentelemetry/trace/noop.h index 8d5d1032a7..0ec0f4fe4c 100644 --- a/api/include/opentelemetry/trace/noop.h +++ b/api/include/opentelemetry/trace/noop.h @@ -37,13 +37,10 @@ namespace trace class OPENTELEMETRY_EXPORT NoopSpan final : public Span { public: - explicit NoopSpan(const std::shared_ptr &tracer) noexcept - : tracer_{tracer}, span_context_{new SpanContext(false, false)} - {} + explicit NoopSpan() : span_context_{new SpanContext(false, false)} {} - explicit NoopSpan(const std::shared_ptr &tracer, - nostd::unique_ptr span_context) noexcept - : tracer_{tracer}, span_context_{std::move(span_context)} + explicit NoopSpan(nostd::unique_ptr span_context) + : span_context_{std::move(span_context)} {} void SetAttribute(nostd::string_view /*key*/, @@ -84,7 +81,6 @@ class OPENTELEMETRY_EXPORT NoopSpan final : public Span SpanContext GetContext() const noexcept override { return *span_context_.get(); } private: - std::shared_ptr tracer_; nostd::unique_ptr span_context_; }; @@ -96,7 +92,7 @@ class OPENTELEMETRY_EXPORT NoopTracer final : public Tracer, { public: // Tracer - NoopTracer() + NoopTracer() : noop_span_(new trace::NoopSpan()) { #if OPENTELEMETRY_ABI_VERSION_NO >= 2 UpdateEnabled(false); @@ -108,11 +104,7 @@ class OPENTELEMETRY_EXPORT NoopTracer final : public Tracer, const SpanContextKeyValueIterable & /*links*/, const StartSpanOptions & /*options*/) noexcept override { - // Don't allocate a no-op span for every StartSpan call, but use a static - // singleton for this case. - static nostd::shared_ptr noop_span(new trace::NoopSpan{this->shared_from_this()}); - - return noop_span; + return noop_span_; } #if OPENTELEMETRY_ABI_VERSION_NO == 1 @@ -122,6 +114,9 @@ class OPENTELEMETRY_EXPORT NoopTracer final : public Tracer, void CloseWithMicroseconds(uint64_t /*timeout*/) noexcept override {} #endif /* OPENTELEMETRY_ABI_VERSION_NO */ + +private: + std::shared_ptr noop_span_; }; /** @@ -130,9 +125,7 @@ class OPENTELEMETRY_EXPORT NoopTracer final : public Tracer, class OPENTELEMETRY_EXPORT NoopTracerProvider final : public trace::TracerProvider { public: - NoopTracerProvider() noexcept - : tracer_{nostd::shared_ptr(new trace::NoopTracer)} - {} + NoopTracerProvider() : tracer_{nostd::shared_ptr(new trace::NoopTracer)} {} #if OPENTELEMETRY_ABI_VERSION_NO >= 2 nostd::shared_ptr GetTracer( diff --git a/api/test/trace/noop_test.cc b/api/test/trace/noop_test.cc index 9828154fe4..7f6f8bf0d1 100644 --- a/api/test/trace/noop_test.cc +++ b/api/test/trace/noop_test.cc @@ -99,9 +99,7 @@ TEST(NoopTest, CreateSpanValidSpanContext) auto span_id = trace_api::SpanId{buf_span}; auto span_context = nonstd::unique_ptr( new trace_api::SpanContext{trace_id, span_id, trace_api::TraceFlags{true}, false}); - std::shared_ptr tracer{new trace_api::NoopTracer{}}; - auto s1 = - nonstd::shared_ptr(new trace_api::NoopSpan(tracer, std::move(span_context))); + auto s1 = nonstd::shared_ptr(new trace_api::NoopSpan(std::move(span_context))); auto stored_span_context = s1->GetContext(); EXPECT_EQ(stored_span_context.span_id(), span_id); EXPECT_EQ(stored_span_context.trace_id(), trace_id); diff --git a/api/test/trace/scope_test.cc b/api/test/trace/scope_test.cc index 51550394e4..929cfe4532 100644 --- a/api/test/trace/scope_test.cc +++ b/api/test/trace/scope_test.cc @@ -22,7 +22,7 @@ namespace context = opentelemetry::context; TEST(ScopeTest, Construct) { - nostd::shared_ptr span(new NoopSpan(nullptr)); + nostd::shared_ptr span(new NoopSpan()); Scope scope(span); context::ContextValue active_span_value = context::RuntimeContext::GetValue(kSpanKey); @@ -34,11 +34,11 @@ TEST(ScopeTest, Construct) TEST(ScopeTest, Destruct) { - nostd::shared_ptr span(new NoopSpan(nullptr)); + nostd::shared_ptr span(new NoopSpan()); Scope scope(span); { - nostd::shared_ptr span_nested(new NoopSpan(nullptr)); + nostd::shared_ptr span_nested(new NoopSpan()); Scope scope_nested(span_nested); context::ContextValue active_span_value = context::RuntimeContext::GetValue(kSpanKey); diff --git a/api/test/trace/tracer_test.cc b/api/test/trace/tracer_test.cc index 1fba78149d..90d1a014d9 100644 --- a/api/test/trace/tracer_test.cc +++ b/api/test/trace/tracer_test.cc @@ -16,8 +16,8 @@ namespace nostd = opentelemetry::nostd; TEST(TracerTest, GetCurrentSpan) { std::unique_ptr tracer(new trace_api::NoopTracer()); - nostd::shared_ptr span_first(new trace_api::NoopSpan(nullptr)); - nostd::shared_ptr span_second(new trace_api::NoopSpan(nullptr)); + nostd::shared_ptr span_first(new trace_api::NoopSpan()); + nostd::shared_ptr span_second(new trace_api::NoopSpan()); auto current = tracer->GetCurrentSpan(); ASSERT_FALSE(current->GetContext().IsValid()); diff --git a/exporters/etw/include/opentelemetry/exporters/etw/etw_tracer.h b/exporters/etw/include/opentelemetry/exporters/etw/etw_tracer.h index 67745a99cd..91cadfbe7e 100644 --- a/exporters/etw/include/opentelemetry/exporters/etw/etw_tracer.h +++ b/exporters/etw/include/opentelemetry/exporters/etw/etw_tracer.h @@ -510,8 +510,7 @@ class Tracer : public opentelemetry::trace::Tracer, if (sampling_result.decision == sdk::trace::Decision::DROP) { auto noopSpan = nostd::shared_ptr{ - new (std::nothrow) - opentelemetry::trace::NoopSpan(this->shared_from_this(), std::move(spanContext))}; + new (std::nothrow) opentelemetry::trace::NoopSpan(std::move(spanContext))}; return noopSpan; }