From 07a0e4703d0f3e0ec78b92df53c8c8a5ac8474c1 Mon Sep 17 00:00:00 2001 From: Om Kulkarni Date: Mon, 21 Sep 2026 23:51:47 +0530 Subject: [PATCH 1/8] [LOGS] Fix Logger::EmitLogRecord unsafe static_cast to Recordable EmitLogRecord() unconditionally static_cast a caller-supplied LogRecord to the SDK's internal Recordable. static_cast between unrelated polymorphic types performs no runtime check, so a LogRecord that is not actually a Recordable (a bridge, or a caller-supplied MakeRecordable() override, both of which are legitimate given LogRecord/MakeRecordable() are public, overridable API surface) hits undefined behavior the moment the mismatched vtable/layout is used. dynamic_cast is not an option as a guard: this project supports building with RTTI disabled (the "Bazel nortti" CI job builds and tests the whole tree, sdk/logs included, with -fno-rtti), and a dynamic_cast would fail to compile there. Added LogRecord::IsRecordable(), a virtual capability query with a default false implementation, overridden by Recordable to return true. EmitLogRecord() checks it before casting and drops (with a warning) a LogRecord that isn't a Recordable, rather than forwarding it. Added a regression test with a minimal foreign LogRecord implementation that does not derive from Recordable, confirming it is dropped cleanly rather than reaching the processor. Verified under both ABI v1 and ABI v2, plus the rest of the sdk/logs test suite for regressions. --- CHANGELOG.md | 14 ++++++ api/include/opentelemetry/logs/log_record.h | 12 +++++ .../opentelemetry/sdk/logs/recordable.h | 2 + sdk/src/logs/logger.cc | 13 ++++++ sdk/test/logs/logger_sdk_test.cc | 45 +++++++++++++++++++ 5 files changed, 86 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e203290157..afc3fb1041 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,20 @@ Increment the: ## [Unreleased] +* [LOGS] Fix `Logger::EmitLogRecord()` unconditionally `static_cast`-ing a + caller-supplied `LogRecord` to the SDK's internal `Recordable`, which is + undefined behavior when the `LogRecord` (e.g. from a bridge, or a custom + `MakeRecordable()` override) is not actually a `Recordable`. Added + `LogRecord::IsRecordable()`, a virtual capability check (not `dynamic_cast`, + since this project supports building with RTTI disabled), that + `EmitLogRecord()` now consults before casting; a `LogRecord` that is not a + `Recordable` is dropped with a warning instead of being forwarded. + [#4537](https://github.com/open-telemetry/opentelemetry-cpp/issues/4537) + * This adds a new virtual method to the public `opentelemetry::logs::LogRecord` + class, changing its vtable layout. It has a default implementation, so no + source changes are needed for existing `LogRecord` implementations, but + they must be rebuilt against this version. + * [EXAMPLES] Fix random attribute selection in metrics foo example to include all key-value pairs [#4585](https://github.com/open-telemetry/opentelemetry-cpp/pull/4585) diff --git a/api/include/opentelemetry/logs/log_record.h b/api/include/opentelemetry/logs/log_record.h index 1e90960769..fee3b01ea6 100644 --- a/api/include/opentelemetry/logs/log_record.h +++ b/api/include/opentelemetry/logs/log_record.h @@ -92,6 +92,18 @@ class LogRecord * @param trace_flags the trace flags to set */ virtual void SetTraceFlags(const trace::TraceFlags &trace_flags) noexcept = 0; + + /** + * Whether this LogRecord is also an opentelemetry::sdk::logs::Recordable. The SDK's + * Logger::EmitLogRecord() accepts a LogRecord and needs to downcast it to the SDK's own + * Recordable to attach resource/scope and hand it to a processor, but a caller (or another + * SDK/wrapper built on the API) can supply a LogRecord implementation that is not a + * Recordable, and this project supports building with RTTI disabled, so a checked + * dynamic_cast is not available as a guard. Overriding this to return true is how a + * Recordable implementation asserts that such a downcast is safe. Do not override this in + * any type that is not actually a Recordable. + */ + virtual bool IsRecordable() const noexcept { return false; } }; } // namespace logs OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/logs/recordable.h b/sdk/include/opentelemetry/sdk/logs/recordable.h index d7ddb3b34a..5ead0fb43e 100644 --- a/sdk/include/opentelemetry/sdk/logs/recordable.h +++ b/sdk/include/opentelemetry/sdk/logs/recordable.h @@ -55,6 +55,8 @@ class Recordable : public opentelemetry::logs::LogRecord * supplied object alive. */ virtual void SetLogRecordLimits(const LogRecordLimits & /* limits */) noexcept {} + + bool IsRecordable() const noexcept final { return true; } }; } // namespace logs diff --git a/sdk/src/logs/logger.cc b/sdk/src/logs/logger.cc index c04b1dfe68..d86cd1c3a5 100644 --- a/sdk/src/logs/logger.cc +++ b/sdk/src/logs/logger.cc @@ -17,6 +17,7 @@ #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/nostd/unique_ptr.h" #include "opentelemetry/nostd/variant.h" +#include "opentelemetry/sdk/common/global_log_handler.h" #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" #include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" #include "opentelemetry/sdk/logs/logger.h" @@ -190,6 +191,18 @@ void Logger::EmitLogRecord( return; } + // MakeRecordable() is a public, overridable entry point, so a caller (or another SDK/wrapper + // built on the API) can hand this a LogRecord implementation that is not actually a + // Recordable. static_cast between unrelated polymorphic types performs no runtime check, so + // the guard below has to come first: it is a virtual capability query rather than a + // dynamic_cast because this project supports building with RTTI disabled. + if (!log_record->IsRecordable()) + { + OTEL_INTERNAL_LOG_WARN( + "[Logger::EmitLogRecord] Dropping log record: not a Recordable implementation."); + return; + } + std::unique_ptr recordable = std::unique_ptr(static_cast(log_record.release())); recordable->SetResource(context_->GetResource()); diff --git a/sdk/test/logs/logger_sdk_test.cc b/sdk/test/logs/logger_sdk_test.cc index 2955049d0f..fc7109c12f 100644 --- a/sdk/test/logs/logger_sdk_test.cc +++ b/sdk/test/logs/logger_sdk_test.cc @@ -407,6 +407,51 @@ TEST(LoggerSDK, LogToAProcessor) now); } +namespace +{ +// A LogRecord implementation that is not an opentelemetry::sdk::logs::Recordable, simulating a +// caller or bridge that hands EmitLogRecord() a foreign LogRecord. IsRecordable() correctly +// defaults to false since this class does not override it. +class ForeignLogRecord final : public logs_api::LogRecord +{ +public: + void SetTimestamp(opentelemetry::common::SystemTimestamp) noexcept override {} + void SetObservedTimestamp(opentelemetry::common::SystemTimestamp) noexcept override {} + void SetSeverity(logs_api::Severity) noexcept override {} + void SetBody(const opentelemetry::common::AttributeValue &) noexcept override {} + void SetAttribute(nostd::string_view, const opentelemetry::common::AttributeValue &) noexcept override + {} + void SetEventId(int64_t, nostd::string_view) noexcept override {} + void SetTraceId(const opentelemetry::trace::TraceId &) noexcept override {} + void SetSpanId(const opentelemetry::trace::SpanId &) noexcept override {} + void SetTraceFlags(const opentelemetry::trace::TraceFlags &) noexcept override {} +}; +} // namespace + +// Regression test: EmitLogRecord() used to static_cast any LogRecord straight to Recordable +// with no runtime check, so a LogRecord implementation that is not actually a Recordable (a +// bridge, or a caller-supplied MakeRecordable() override) hit undefined behavior the moment the +// mismatched vtable/layout was used. IsRecordable() now gates the cast; a foreign LogRecord +// must be dropped rather than forwarded to the processor. +TEST(LoggerSDK, EmitLogRecordDropsNonRecordableLogRecord) +{ + auto api_lp = std::shared_ptr(new LoggerProvider()); + auto logger = api_lp->GetLogger("logger", "opentelelemtry_library"); + auto lp = static_cast(api_lp.get()); + + auto shared_recordable = std::shared_ptr(new MockLogRecordable()); + lp->AddProcessor(std::unique_ptr( + new MockProcessor(shared_recordable))); + + logger->EmitLogRecord( + nostd::unique_ptr(new ForeignLogRecord())); + + // The processor's MockProcessor::OnEmit() would have run through a mismatched vtable/layout + // had the cast not been guarded; instead, shared_recordable must be untouched. + EXPECT_EQ(shared_recordable->GetSeverity(), logs_api::Severity::kInvalid); + EXPECT_EQ(shared_recordable->GetBody(), ""); +} + TEST(LoggerSDK, LoggerWithDisabledConfig) { ScopeConfigurator disabled_all_scopes = From 63a307b28c418dbfd1c16dab1bd701c1007c04fc Mon Sep 17 00:00:00 2001 From: Om Kulkarni Date: Mon, 21 Sep 2026 23:52:30 +0530 Subject: [PATCH 2/8] Update CHANGELOG entry with PR number --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index afc3fb1041..cdf7dbc2cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,7 @@ Increment the: since this project supports building with RTTI disabled), that `EmitLogRecord()` now consults before casting; a `LogRecord` that is not a `Recordable` is dropped with a warning instead of being forwarded. - [#4537](https://github.com/open-telemetry/opentelemetry-cpp/issues/4537) + [#4624](https://github.com/open-telemetry/opentelemetry-cpp/pull/4624) * This adds a new virtual method to the public `opentelemetry::logs::LogRecord` class, changing its vtable layout. It has a default implementation, so no source changes are needed for existing `LogRecord` implementations, but From 225e51a2f20f290d7c10ac2c30533d58bdf65573 Mon Sep 17 00:00:00 2001 From: Om Kulkarni Date: Tue, 22 Sep 2026 00:01:21 +0530 Subject: [PATCH 3/8] Fix clang-format on the new regression test --- sdk/test/logs/logger_sdk_test.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/test/logs/logger_sdk_test.cc b/sdk/test/logs/logger_sdk_test.cc index fc7109c12f..b3aa427930 100644 --- a/sdk/test/logs/logger_sdk_test.cc +++ b/sdk/test/logs/logger_sdk_test.cc @@ -419,7 +419,8 @@ class ForeignLogRecord final : public logs_api::LogRecord void SetObservedTimestamp(opentelemetry::common::SystemTimestamp) noexcept override {} void SetSeverity(logs_api::Severity) noexcept override {} void SetBody(const opentelemetry::common::AttributeValue &) noexcept override {} - void SetAttribute(nostd::string_view, const opentelemetry::common::AttributeValue &) noexcept override + void SetAttribute(nostd::string_view, + const opentelemetry::common::AttributeValue &) noexcept override {} void SetEventId(int64_t, nostd::string_view) noexcept override {} void SetTraceId(const opentelemetry::trace::TraceId &) noexcept override {} @@ -443,8 +444,7 @@ TEST(LoggerSDK, EmitLogRecordDropsNonRecordableLogRecord) lp->AddProcessor(std::unique_ptr( new MockProcessor(shared_recordable))); - logger->EmitLogRecord( - nostd::unique_ptr(new ForeignLogRecord())); + logger->EmitLogRecord(nostd::unique_ptr(new ForeignLogRecord())); // The processor's MockProcessor::OnEmit() would have run through a mismatched vtable/layout // had the cast not been guarded; instead, shared_recordable must be untouched. From b72a588d5926d6593831a51ca0e7bd5e57fd2afe Mon Sep 17 00:00:00 2001 From: Om Kulkarni Date: Tue, 22 Sep 2026 08:52:54 +0530 Subject: [PATCH 4/8] Add missing nostd/unique_ptr.h include for IWYU --- sdk/test/logs/logger_sdk_test.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/test/logs/logger_sdk_test.cc b/sdk/test/logs/logger_sdk_test.cc index b3aa427930..57981eb9b2 100644 --- a/sdk/test/logs/logger_sdk_test.cc +++ b/sdk/test/logs/logger_sdk_test.cc @@ -22,6 +22,7 @@ #include "opentelemetry/logs/severity.h" #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/string_view.h" +#include "opentelemetry/nostd/unique_ptr.h" #include "opentelemetry/nostd/variant.h" #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" #include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" From ef64107f824f5c86ff33d37ab9c58d0e36aa561b Mon Sep 17 00:00:00 2001 From: Om Kulkarni Date: Tue, 22 Sep 2026 09:01:56 +0530 Subject: [PATCH 5/8] [LOGS] Rework to match community-converged design (empty MultiRecordable) Per discussion on #4537 and lalitb's review on this PR, rework from the originally proposed virtual LogRecord::IsRecordable() (ABI-changing) to having Logger::CreateLogRecord() return an empty MultiRecordable while the logger is disabled. This is a safe target for both Logger::EmitLogRecord()'s static_cast and MultiLogRecordProcessor::OnEmit()'s static_cast if the logger is enabled between record creation and emission, with no API/ABI change, since an empty MultiRecordable's Set* calls and ReleaseRecordable() simply loop over zero wrapped recordables. --- CHANGELOG.md | 24 +++---- api/include/opentelemetry/logs/log_record.h | 12 ---- .../opentelemetry/sdk/logs/recordable.h | 2 - sdk/src/logs/logger.cc | 24 +++---- sdk/test/logs/logger_sdk_test.cc | 64 +++++++++---------- 5 files changed, 50 insertions(+), 76 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cdf7dbc2cb..284434c990 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,19 +15,19 @@ Increment the: ## [Unreleased] -* [LOGS] Fix `Logger::EmitLogRecord()` unconditionally `static_cast`-ing a - caller-supplied `LogRecord` to the SDK's internal `Recordable`, which is - undefined behavior when the `LogRecord` (e.g. from a bridge, or a custom - `MakeRecordable()` override) is not actually a `Recordable`. Added - `LogRecord::IsRecordable()`, a virtual capability check (not `dynamic_cast`, - since this project supports building with RTTI disabled), that - `EmitLogRecord()` now consults before casting; a `LogRecord` that is not a - `Recordable` is dropped with a warning instead of being forwarded. +* [LOGS] Fix undefined behavior in `Logger::EmitLogRecord()` when a logger is + enabled (e.g. via `LoggerProvider::UpdateLoggerConfigurator()`) after + `CreateLogRecord()` was called while it was still disabled. The disabled + path previously returned a `NoopLogRecord`, which is not the SDK's internal + `Recordable`; `EmitLogRecord()` re-checks the enabled state at emit time, so + such a record could reach its unconditional `static_cast` and + then `MultiLogRecordProcessor::OnEmit()`'s own `static_cast`, both undefined + behavior. `CreateLogRecord()` now returns an empty `MultiRecordable` while + disabled instead, which is a safe target either way: every `Set*` call and + `ReleaseRecordable()` on it simply loop over zero wrapped recordables, so + the record is dropped without reaching a real processor. No API or ABI + change. [#4624](https://github.com/open-telemetry/opentelemetry-cpp/pull/4624) - * This adds a new virtual method to the public `opentelemetry::logs::LogRecord` - class, changing its vtable layout. It has a default implementation, so no - source changes are needed for existing `LogRecord` implementations, but - they must be rebuilt against this version. * [EXAMPLES] Fix random attribute selection in metrics foo example to include all key-value pairs diff --git a/api/include/opentelemetry/logs/log_record.h b/api/include/opentelemetry/logs/log_record.h index fee3b01ea6..1e90960769 100644 --- a/api/include/opentelemetry/logs/log_record.h +++ b/api/include/opentelemetry/logs/log_record.h @@ -92,18 +92,6 @@ class LogRecord * @param trace_flags the trace flags to set */ virtual void SetTraceFlags(const trace::TraceFlags &trace_flags) noexcept = 0; - - /** - * Whether this LogRecord is also an opentelemetry::sdk::logs::Recordable. The SDK's - * Logger::EmitLogRecord() accepts a LogRecord and needs to downcast it to the SDK's own - * Recordable to attach resource/scope and hand it to a processor, but a caller (or another - * SDK/wrapper built on the API) can supply a LogRecord implementation that is not a - * Recordable, and this project supports building with RTTI disabled, so a checked - * dynamic_cast is not available as a guard. Overriding this to return true is how a - * Recordable implementation asserts that such a downcast is safe. Do not override this in - * any type that is not actually a Recordable. - */ - virtual bool IsRecordable() const noexcept { return false; } }; } // namespace logs OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/logs/recordable.h b/sdk/include/opentelemetry/sdk/logs/recordable.h index 5ead0fb43e..d7ddb3b34a 100644 --- a/sdk/include/opentelemetry/sdk/logs/recordable.h +++ b/sdk/include/opentelemetry/sdk/logs/recordable.h @@ -55,8 +55,6 @@ class Recordable : public opentelemetry::logs::LogRecord * supplied object alive. */ virtual void SetLogRecordLimits(const LogRecordLimits & /* limits */) noexcept {} - - bool IsRecordable() const noexcept final { return true; } }; } // namespace logs diff --git a/sdk/src/logs/logger.cc b/sdk/src/logs/logger.cc index d86cd1c3a5..db7137cdbe 100644 --- a/sdk/src/logs/logger.cc +++ b/sdk/src/logs/logger.cc @@ -17,12 +17,12 @@ #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/nostd/unique_ptr.h" #include "opentelemetry/nostd/variant.h" -#include "opentelemetry/sdk/common/global_log_handler.h" #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" #include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" #include "opentelemetry/sdk/logs/logger.h" #include "opentelemetry/sdk/logs/logger_config.h" #include "opentelemetry/sdk/logs/logger_context.h" +#include "opentelemetry/sdk/logs/multi_recordable.h" #include "opentelemetry/sdk/logs/processor.h" #include "opentelemetry/sdk/logs/recordable.h" #include "opentelemetry/trace/context.h" @@ -135,7 +135,12 @@ opentelemetry::nostd::unique_ptr Logger::CreateL { if (!logger_enabled_.load(std::memory_order_relaxed)) { - return kNoopLogger.CreateLogRecord(); + // Returns an empty MultiRecordable rather than a NoopLogRecord: the logger's enabled state + // can flip between this call and EmitLogRecord() (e.g. via UpdateLoggerConfig()), and + // MultiLogRecordProcessor::OnEmit() unconditionally static_casts whatever it receives to + // MultiRecordable. An empty one is a safe target either way, since every Set* call and + // ReleaseRecordable() loop over zero wrapped recordables. + return opentelemetry::nostd::unique_ptr(new MultiRecordable()); } auto recordable = context_->GetProcessor().MakeRecordable(); @@ -161,7 +166,8 @@ opentelemetry::nostd::unique_ptr Logger::CreateL { if (!logger_enabled_.load(std::memory_order_relaxed)) { - return kNoopLogger.CreateLogRecord(); + // See the matching comment in the no-argument CreateLogRecord() overload above. + return opentelemetry::nostd::unique_ptr(new MultiRecordable()); } auto recordable = context_->GetProcessor().MakeRecordable(); @@ -191,18 +197,6 @@ void Logger::EmitLogRecord( return; } - // MakeRecordable() is a public, overridable entry point, so a caller (or another SDK/wrapper - // built on the API) can hand this a LogRecord implementation that is not actually a - // Recordable. static_cast between unrelated polymorphic types performs no runtime check, so - // the guard below has to come first: it is a virtual capability query rather than a - // dynamic_cast because this project supports building with RTTI disabled. - if (!log_record->IsRecordable()) - { - OTEL_INTERNAL_LOG_WARN( - "[Logger::EmitLogRecord] Dropping log record: not a Recordable implementation."); - return; - } - std::unique_ptr recordable = std::unique_ptr(static_cast(log_record.release())); recordable->SetResource(context_->GetResource()); diff --git a/sdk/test/logs/logger_sdk_test.cc b/sdk/test/logs/logger_sdk_test.cc index 57981eb9b2..80e96417da 100644 --- a/sdk/test/logs/logger_sdk_test.cc +++ b/sdk/test/logs/logger_sdk_test.cc @@ -408,47 +408,41 @@ TEST(LoggerSDK, LogToAProcessor) now); } -namespace -{ -// A LogRecord implementation that is not an opentelemetry::sdk::logs::Recordable, simulating a -// caller or bridge that hands EmitLogRecord() a foreign LogRecord. IsRecordable() correctly -// defaults to false since this class does not override it. -class ForeignLogRecord final : public logs_api::LogRecord +// Regression test: while the logger is disabled, CreateLogRecord() used to hand back a +// kNoopLogger-created NoopLogRecord, which is not an opentelemetry::sdk::logs::Recordable. +// EmitLogRecord() re-checks the enabled state at emit time, so enabling the logger in between +// (e.g. via UpdateLoggerConfig()) made that NoopLogRecord reach Logger::EmitLogRecord()'s +// static_cast and then MultiLogRecordProcessor::OnEmit()'s own +// static_cast, both undefined behavior. CreateLogRecord() now returns an +// empty MultiRecordable while disabled, which is a safe target for both casts either way: an +// empty MultiRecordable's Set* calls and ReleaseRecordable() simply loop over zero wrapped +// recordables, so the record is dropped without ever reaching a real processor. +TEST(LoggerSDK, EmitLogRecordSafeWhenEnabledBetweenCreateAndEmit) { -public: - void SetTimestamp(opentelemetry::common::SystemTimestamp) noexcept override {} - void SetObservedTimestamp(opentelemetry::common::SystemTimestamp) noexcept override {} - void SetSeverity(logs_api::Severity) noexcept override {} - void SetBody(const opentelemetry::common::AttributeValue &) noexcept override {} - void SetAttribute(nostd::string_view, - const opentelemetry::common::AttributeValue &) noexcept override - {} - void SetEventId(int64_t, nostd::string_view) noexcept override {} - void SetTraceId(const opentelemetry::trace::TraceId &) noexcept override {} - void SetSpanId(const opentelemetry::trace::SpanId &) noexcept override {} - void SetTraceFlags(const opentelemetry::trace::TraceFlags &) noexcept override {} -}; -} // namespace + ScopeConfigurator disabled_all_scopes = + ScopeConfigurator::Builder(LoggerConfig::Disabled()).Build(); + auto shared_recordable = std::shared_ptr(new MockLogRecordable()); + auto log_processor = std::unique_ptr(new MockProcessor(shared_recordable)); -// Regression test: EmitLogRecord() used to static_cast any LogRecord straight to Recordable -// with no runtime check, so a LogRecord implementation that is not actually a Recordable (a -// bridge, or a caller-supplied MakeRecordable() override) hit undefined behavior the moment the -// mismatched vtable/layout was used. IsRecordable() now gates the cast; a foreign LogRecord -// must be dropped rather than forwarded to the processor. -TEST(LoggerSDK, EmitLogRecordDropsNonRecordableLogRecord) -{ - auto api_lp = std::shared_ptr(new LoggerProvider()); + const auto resource = opentelemetry::sdk::resource::Resource::Create({}); + auto scope_configurator = + std::make_unique>(disabled_all_scopes); + auto api_lp = std::shared_ptr( + new LoggerProvider(std::move(log_processor), resource, std::move(scope_configurator))); auto logger = api_lp->GetLogger("logger", "opentelelemtry_library"); - auto lp = static_cast(api_lp.get()); + auto sdk_lp = static_cast(api_lp.get()); - auto shared_recordable = std::shared_ptr(new MockLogRecordable()); - lp->AddProcessor(std::unique_ptr( - new MockProcessor(shared_recordable))); + // Created while disabled: this must be an empty MultiRecordable, not a NoopLogRecord. + auto log_record = logger->CreateLogRecord(); - logger->EmitLogRecord(nostd::unique_ptr(new ForeignLogRecord())); + // Enable the logger before the record is emitted. + sdk_lp->UpdateLoggerConfigurator(std::make_unique>( + ScopeConfigurator::Builder(LoggerConfig::Enabled()).Build())); + ASSERT_TRUE(logger->Enabled(logs_api::Severity::kInvalid)); - // The processor's MockProcessor::OnEmit() would have run through a mismatched vtable/layout - // had the cast not been guarded; instead, shared_recordable must be untouched. + // Must not crash, and the record has no wrapped recordable for this processor, so it is + // dropped rather than delivered. + logger->EmitLogRecord(std::move(log_record)); EXPECT_EQ(shared_recordable->GetSeverity(), logs_api::Severity::kInvalid); EXPECT_EQ(shared_recordable->GetBody(), ""); } From 5f93be7c83d6dee1c374e57306c6b4889da36824 Mon Sep 17 00:00:00 2001 From: Om Kulkarni Date: Tue, 22 Sep 2026 19:44:25 +0530 Subject: [PATCH 6/8] Fix Format and IWYU CI failures in logger_sdk_test.cc clang-format alignment for the new test's variable declarations, and remove the now-unused nostd/unique_ptr.h include after the previous rework dropped the test that referenced it directly. --- sdk/test/logs/logger_sdk_test.cc | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/sdk/test/logs/logger_sdk_test.cc b/sdk/test/logs/logger_sdk_test.cc index 80e96417da..444477085b 100644 --- a/sdk/test/logs/logger_sdk_test.cc +++ b/sdk/test/logs/logger_sdk_test.cc @@ -22,7 +22,6 @@ #include "opentelemetry/logs/severity.h" #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/string_view.h" -#include "opentelemetry/nostd/unique_ptr.h" #include "opentelemetry/nostd/variant.h" #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" #include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" @@ -424,10 +423,9 @@ TEST(LoggerSDK, EmitLogRecordSafeWhenEnabledBetweenCreateAndEmit) auto shared_recordable = std::shared_ptr(new MockLogRecordable()); auto log_processor = std::unique_ptr(new MockProcessor(shared_recordable)); - const auto resource = opentelemetry::sdk::resource::Resource::Create({}); - auto scope_configurator = - std::make_unique>(disabled_all_scopes); - auto api_lp = std::shared_ptr( + const auto resource = opentelemetry::sdk::resource::Resource::Create({}); + auto scope_configurator = std::make_unique>(disabled_all_scopes); + auto api_lp = std::shared_ptr( new LoggerProvider(std::move(log_processor), resource, std::move(scope_configurator))); auto logger = api_lp->GetLogger("logger", "opentelelemtry_library"); auto sdk_lp = static_cast(api_lp.get()); From a1bcbf4c40f87d0dc86bdd74d39e0d0cbfef73c3 Mon Sep 17 00:00:00 2001 From: Om Kulkarni Date: Wed, 23 Sep 2026 08:19:07 +0530 Subject: [PATCH 7/8] Cover the ABI v2 CreateLogRecord overload in the regression test mateenali66 confirmed EmitLogRecordSafeWhenEnabledBetweenCreateAndEmit reproduces a SIGBUS on plain main and passes on this PR's head, but only the no-argument CreateLogRecord() overload was exercised. The v2 CreateLogRecord(context_or_span) overload takes the identical fix, so extend the same test to create and emit through it too. Verified locally under both ABIv1 (10/10 pass, v2 branch compiled out) and ABIv2 (19/19 pass). Co-Authored-By: Mateen Anjum --- sdk/test/logs/logger_sdk_test.cc | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/sdk/test/logs/logger_sdk_test.cc b/sdk/test/logs/logger_sdk_test.cc index 444477085b..1a30f15e80 100644 --- a/sdk/test/logs/logger_sdk_test.cc +++ b/sdk/test/logs/logger_sdk_test.cc @@ -415,7 +415,8 @@ TEST(LoggerSDK, LogToAProcessor) // static_cast, both undefined behavior. CreateLogRecord() now returns an // empty MultiRecordable while disabled, which is a safe target for both casts either way: an // empty MultiRecordable's Set* calls and ReleaseRecordable() simply loop over zero wrapped -// recordables, so the record is dropped without ever reaching a real processor. +// recordables, so the record is dropped without ever reaching a real processor. Covers both +// CreateLogRecord() overloads, since the fix applies to each independently. TEST(LoggerSDK, EmitLogRecordSafeWhenEnabledBetweenCreateAndEmit) { ScopeConfigurator disabled_all_scopes = @@ -432,8 +433,15 @@ TEST(LoggerSDK, EmitLogRecordSafeWhenEnabledBetweenCreateAndEmit) // Created while disabled: this must be an empty MultiRecordable, not a NoopLogRecord. auto log_record = logger->CreateLogRecord(); +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + // The v2 overload takes the same fix; nothing exercises it otherwise, since the no-argument + // overload above is the only one a disabled logger reaches by default. + auto log_record_v2 = logger->CreateLogRecord( + nostd::variant{ + opentelemetry::trace::SpanContext::GetInvalid()}); +#endif // OPENTELEMETRY_ABI_VERSION_NO >= 2 - // Enable the logger before the record is emitted. + // Enable the logger before the records are emitted. sdk_lp->UpdateLoggerConfigurator(std::make_unique>( ScopeConfigurator::Builder(LoggerConfig::Enabled()).Build())); ASSERT_TRUE(logger->Enabled(logs_api::Severity::kInvalid)); @@ -443,6 +451,12 @@ TEST(LoggerSDK, EmitLogRecordSafeWhenEnabledBetweenCreateAndEmit) logger->EmitLogRecord(std::move(log_record)); EXPECT_EQ(shared_recordable->GetSeverity(), logs_api::Severity::kInvalid); EXPECT_EQ(shared_recordable->GetBody(), ""); + +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + logger->EmitLogRecord(std::move(log_record_v2)); + EXPECT_EQ(shared_recordable->GetSeverity(), logs_api::Severity::kInvalid); + EXPECT_EQ(shared_recordable->GetBody(), ""); +#endif // OPENTELEMETRY_ABI_VERSION_NO >= 2 } TEST(LoggerSDK, LoggerWithDisabledConfig) From a33863c9feb331d1b62ebced6fbc448376b99058 Mon Sep 17 00:00:00 2001 From: Om Kulkarni Date: Wed, 23 Sep 2026 19:59:05 +0530 Subject: [PATCH 8/8] Fix clang-format wrapping on the v2 CreateLogRecord test line --- sdk/test/logs/logger_sdk_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/test/logs/logger_sdk_test.cc b/sdk/test/logs/logger_sdk_test.cc index 1a30f15e80..65720baddd 100644 --- a/sdk/test/logs/logger_sdk_test.cc +++ b/sdk/test/logs/logger_sdk_test.cc @@ -436,8 +436,8 @@ TEST(LoggerSDK, EmitLogRecordSafeWhenEnabledBetweenCreateAndEmit) #if OPENTELEMETRY_ABI_VERSION_NO >= 2 // The v2 overload takes the same fix; nothing exercises it otherwise, since the no-argument // overload above is the only one a disabled logger reaches by default. - auto log_record_v2 = logger->CreateLogRecord( - nostd::variant{ + auto log_record_v2 = + logger->CreateLogRecord(nostd::variant{ opentelemetry::trace::SpanContext::GetInvalid()}); #endif // OPENTELEMETRY_ABI_VERSION_NO >= 2