From 53f3617011a66f03fb9c8d45d654bfba6ae9b7eb Mon Sep 17 00:00:00 2001 From: om7057 Date: Sun, 26 Jul 2026 10:41:58 +0530 Subject: [PATCH 01/21] Implement EventToSpanEventBridge log record processor Adds the ExperimentalEventToSpanEventBridgeLogRecordProcessor type from the declarative configuration spec. When a log record carries an event name and matches the trace and span id of the currently active span, this processor copies it over as a span event, mirroring the existing implementation in the Java SDK. Fixes #4155 --- CHANGELOG.md | 4 + .../sdk/configuration/configuration_parser.h | 5 + ...ridge_log_record_processor_configuration.h | 29 +++ ...g_record_processor_configuration_visitor.h | 7 +- .../sdk/configuration/sdk_builder.h | 5 + .../event_to_span_event_bridge_processor.h | 58 +++++ ...t_to_span_event_bridge_processor_factory.h | 31 +++ sdk/src/configuration/configuration_parser.cc | 11 + sdk/src/configuration/sdk_builder.cc | 22 +- sdk/src/logs/CMakeLists.txt | 2 + .../event_to_span_event_bridge_processor.cc | 219 ++++++++++++++++++ ..._to_span_event_bridge_processor_factory.cc | 25 ++ sdk/test/configuration/yaml_logs_test.cc | 22 ++ sdk/test/logs/BUILD | 18 ++ sdk/test/logs/CMakeLists.txt | 15 ++ ...ent_to_span_event_bridge_processor_test.cc | 131 +++++++++++ 16 files changed, 599 insertions(+), 5 deletions(-) create mode 100644 sdk/include/opentelemetry/sdk/configuration/event_to_span_event_bridge_log_record_processor_configuration.h create mode 100644 sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h create mode 100644 sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor_factory.h create mode 100644 sdk/src/logs/event_to_span_event_bridge_processor.cc create mode 100644 sdk/src/logs/event_to_span_event_bridge_processor_factory.cc create mode 100644 sdk/test/logs/event_to_span_event_bridge_processor_test.cc diff --git a/CHANGELOG.md b/CHANGELOG.md index 819a741b73..9cb4a124aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ Increment the: ## [Unreleased] +* [CONFIGURATION] Implement EventToSpanEventBridge log record processor for + declarative configuration + [#4155](https://github.com/open-telemetry/opentelemetry-cpp/issues/4155) + * [CODE HEALTH] Move SDK trace and metrics test helpers into anonymous namespaces [#4303](https://github.com/open-telemetry/opentelemetry-cpp/pull/4303) diff --git a/sdk/include/opentelemetry/sdk/configuration/configuration_parser.h b/sdk/include/opentelemetry/sdk/configuration/configuration_parser.h index 84f5811874..93903c756c 100644 --- a/sdk/include/opentelemetry/sdk/configuration/configuration_parser.h +++ b/sdk/include/opentelemetry/sdk/configuration/configuration_parser.h @@ -36,6 +36,7 @@ #include "opentelemetry/sdk/configuration/double_array_attribute_value_configuration.h" #include "opentelemetry/sdk/configuration/double_attribute_value_configuration.h" #include "opentelemetry/sdk/configuration/drop_aggregation_configuration.h" +#include "opentelemetry/sdk/configuration/event_to_span_event_bridge_log_record_processor_configuration.h" #include "opentelemetry/sdk/configuration/exemplar_filter.h" #include "opentelemetry/sdk/configuration/explicit_bucket_histogram_aggregation_configuration.h" #include "opentelemetry/sdk/configuration/extension_log_record_exporter_configuration.h" @@ -167,6 +168,10 @@ class ConfigurationParser std::unique_ptr ParseSimpleLogRecordProcessorConfiguration( const std::unique_ptr &node) const; + std::unique_ptr + ParseEventToSpanEventBridgeLogRecordProcessorConfiguration( + const std::unique_ptr &node) const; + std::unique_ptr ParseExtensionLogRecordProcessorConfiguration(const std::string &name, std::unique_ptr node) const; diff --git a/sdk/include/opentelemetry/sdk/configuration/event_to_span_event_bridge_log_record_processor_configuration.h b/sdk/include/opentelemetry/sdk/configuration/event_to_span_event_bridge_log_record_processor_configuration.h new file mode 100644 index 0000000000..687b416e30 --- /dev/null +++ b/sdk/include/opentelemetry/sdk/configuration/event_to_span_event_bridge_log_record_processor_configuration.h @@ -0,0 +1,29 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +#include "opentelemetry/sdk/configuration/log_record_processor_configuration.h" +#include "opentelemetry/sdk/configuration/log_record_processor_configuration_visitor.h" +#include "opentelemetry/version.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace configuration +{ + +// YAML-SCHEMA: schema/logger_provider.json +// YAML-NODE: ExperimentalEventToSpanEventBridgeLogRecordProcessor +class EventToSpanEventBridgeLogRecordProcessorConfiguration : public LogRecordProcessorConfiguration +{ +public: + void Accept(LogRecordProcessorConfigurationVisitor *visitor) const override + { + visitor->VisitEventToSpanEventBridge(this); + } +}; + +} // namespace configuration +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/configuration/log_record_processor_configuration_visitor.h b/sdk/include/opentelemetry/sdk/configuration/log_record_processor_configuration_visitor.h index ac4dd52d23..320a8fe4b1 100644 --- a/sdk/include/opentelemetry/sdk/configuration/log_record_processor_configuration_visitor.h +++ b/sdk/include/opentelemetry/sdk/configuration/log_record_processor_configuration_visitor.h @@ -13,6 +13,7 @@ namespace configuration class BatchLogRecordProcessorConfiguration; class SimpleLogRecordProcessorConfiguration; +class EventToSpanEventBridgeLogRecordProcessorConfiguration; class ExtensionLogRecordProcessorConfiguration; class LogRecordProcessorConfigurationVisitor @@ -27,8 +28,10 @@ class LogRecordProcessorConfigurationVisitor const LogRecordProcessorConfigurationVisitor &other) = default; virtual ~LogRecordProcessorConfigurationVisitor() = default; - virtual void VisitBatch(const BatchLogRecordProcessorConfiguration *model) = 0; - virtual void VisitSimple(const SimpleLogRecordProcessorConfiguration *model) = 0; + virtual void VisitBatch(const BatchLogRecordProcessorConfiguration *model) = 0; + virtual void VisitSimple(const SimpleLogRecordProcessorConfiguration *model) = 0; + virtual void VisitEventToSpanEventBridge( + const EventToSpanEventBridgeLogRecordProcessorConfiguration *model) = 0; virtual void VisitExtension(const ExtensionLogRecordProcessorConfiguration *model) = 0; }; diff --git a/sdk/include/opentelemetry/sdk/configuration/sdk_builder.h b/sdk/include/opentelemetry/sdk/configuration/sdk_builder.h index 6897c54c9f..3507367fe8 100644 --- a/sdk/include/opentelemetry/sdk/configuration/sdk_builder.h +++ b/sdk/include/opentelemetry/sdk/configuration/sdk_builder.h @@ -249,6 +249,11 @@ class SdkBuilder std::unique_ptr CreateSimpleLogRecordProcessor( const opentelemetry::sdk::configuration::SimpleLogRecordProcessorConfiguration *model) const; + std::unique_ptr + CreateEventToSpanEventBridgeLogRecordProcessor( + const opentelemetry::sdk::configuration::EventToSpanEventBridgeLogRecordProcessorConfiguration + *model) const; + std::unique_ptr CreateExtensionLogRecordProcessor( const opentelemetry::sdk::configuration::ExtensionLogRecordProcessorConfiguration *model) const; diff --git a/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h b/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h new file mode 100644 index 0000000000..8eba69894a --- /dev/null +++ b/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h @@ -0,0 +1,58 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +#include +#include + +#include "opentelemetry/sdk/logs/processor.h" +#include "opentelemetry/sdk/logs/recordable.h" +#include "opentelemetry/version.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace logs +{ + +/** + * A log record processor that bridges log records representing events (i.e. log records with a + * non-empty event name) onto the current active span as span events. + * + * A log record is bridged to a span event if and only if: + * - the log record has a non-empty event name, + * - the log record carries a valid trace id and span id, + * - the current active span (from the ambient context at the time OnEmit is called) is + * recording, and + * - the current active span's trace id and span id match those on the log record. + * + * This processor does not export log records and does not stop the log record from continuing + * through the rest of the configured processing pipeline. + */ +class EventToSpanEventBridgeProcessor : public LogRecordProcessor +{ +public: + EventToSpanEventBridgeProcessor() = default; + EventToSpanEventBridgeProcessor(const EventToSpanEventBridgeProcessor &) = delete; + EventToSpanEventBridgeProcessor(EventToSpanEventBridgeProcessor &&) = delete; + EventToSpanEventBridgeProcessor &operator=(const EventToSpanEventBridgeProcessor &) = delete; + EventToSpanEventBridgeProcessor &operator=(EventToSpanEventBridgeProcessor &&) = delete; + ~EventToSpanEventBridgeProcessor() override = default; + + std::unique_ptr MakeRecordable() noexcept override; + + void OnEmit(std::unique_ptr &&record) noexcept override; + + bool ForceFlush( + std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override; + + bool Shutdown( + std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override; + + bool HasEnabledFilter() const noexcept override { return false; } +}; + +} // namespace logs +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor_factory.h b/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor_factory.h new file mode 100644 index 0000000000..d5a1b7378b --- /dev/null +++ b/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor_factory.h @@ -0,0 +1,31 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +#include + +#include "opentelemetry/sdk/logs/processor.h" +#include "opentelemetry/version.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace logs +{ + +/** + * Factory class for EventToSpanEventBridgeProcessor. + */ +class OPENTELEMETRY_EXPORT EventToSpanEventBridgeProcessorFactory +{ +public: + /** + * Create an EventToSpanEventBridgeProcessor. + */ + static std::unique_ptr Create(); +}; + +} // namespace logs +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/src/configuration/configuration_parser.cc b/sdk/src/configuration/configuration_parser.cc index bfe4dfa373..b5793b53ff 100644 --- a/sdk/src/configuration/configuration_parser.cc +++ b/sdk/src/configuration/configuration_parser.cc @@ -570,6 +570,13 @@ ConfigurationParser::ParseSimpleLogRecordProcessorConfiguration( return model; } +std::unique_ptr +ConfigurationParser::ParseEventToSpanEventBridgeLogRecordProcessorConfiguration( + const std::unique_ptr & /* node */) const +{ + return std::make_unique(); +} + std::unique_ptr ConfigurationParser::ParseExtensionLogRecordProcessorConfiguration( const std::string &name, @@ -615,6 +622,10 @@ ConfigurationParser::ParseLogRecordProcessorConfiguration( { model = ParseSimpleLogRecordProcessorConfiguration(child); } + else if (name == "event_to_span_event_bridge/development") + { + model = ParseEventToSpanEventBridgeLogRecordProcessorConfiguration(child); + } else { model = ParseExtensionLogRecordProcessorConfiguration(name, std::move(child)); diff --git a/sdk/src/configuration/sdk_builder.cc b/sdk/src/configuration/sdk_builder.cc index b94ccd2799..2baafa68cb 100644 --- a/sdk/src/configuration/sdk_builder.cc +++ b/sdk/src/configuration/sdk_builder.cc @@ -137,6 +137,7 @@ #include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" #include "opentelemetry/sdk/logs/batch_log_record_processor_factory.h" #include "opentelemetry/sdk/logs/batch_log_record_processor_options.h" +#include "opentelemetry/sdk/logs/event_to_span_event_bridge_processor_factory.h" #include "opentelemetry/sdk/logs/exporter.h" #include "opentelemetry/sdk/logs/log_record_limits.h" #include "opentelemetry/sdk/logs/logger_config.h" @@ -756,6 +757,13 @@ class LogRecordProcessorBuilder processor = sdk_builder_->CreateSimpleLogRecordProcessor(model); } + void VisitEventToSpanEventBridge( + const opentelemetry::sdk::configuration::EventToSpanEventBridgeLogRecordProcessorConfiguration + *model) override + { + processor = sdk_builder_->CreateEventToSpanEventBridgeLogRecordProcessor(model); + } + void VisitExtension( const opentelemetry::sdk::configuration::ExtensionLogRecordProcessorConfiguration *model) override @@ -1195,8 +1203,8 @@ std::unique_ptr SdkBuilder::CreateTra auto tracer_configurator = CreateTracerConfigurator(model->tracer_configurator); auto id_generator = opentelemetry::sdk::trace::RandomIdGeneratorFactory::Create(); sdk = opentelemetry::sdk::trace::TracerProviderFactory::Create( - std::move(sdk_processors), resource, std::move(sampler), std::move(id_generator), - std::move(tracer_configurator), span_limits); + std::move(sdk_processors), resource, std::move(sampler), std::move(id_generator), + std::move(tracer_configurator), span_limits); } else { @@ -1762,7 +1770,7 @@ std::unique_ptr SdkBuilder::CreateMe { auto meter_configurator = CreateMeterConfigurator(model->meter_configurator); meter_context = opentelemetry::sdk::metrics::MeterContextFactory::Create( - std::move(view_registry), resource, std::move(meter_configurator)); + std::move(view_registry), resource, std::move(meter_configurator)); } else { @@ -1930,6 +1938,14 @@ SdkBuilder::CreateSimpleLogRecordProcessor( return sdk; } +std::unique_ptr +SdkBuilder::CreateEventToSpanEventBridgeLogRecordProcessor( + const opentelemetry::sdk::configuration::EventToSpanEventBridgeLogRecordProcessorConfiguration + * /* model */) const +{ + return opentelemetry::sdk::logs::EventToSpanEventBridgeProcessorFactory::Create(); +} + std::unique_ptr SdkBuilder::CreateExtensionLogRecordProcessor( const opentelemetry::sdk::configuration::ExtensionLogRecordProcessorConfiguration *model) const diff --git a/sdk/src/logs/CMakeLists.txt b/sdk/src/logs/CMakeLists.txt index 338fa8e8f1..e28f4e1638 100644 --- a/sdk/src/logs/CMakeLists.txt +++ b/sdk/src/logs/CMakeLists.txt @@ -13,6 +13,8 @@ add_library( event_logger.cc simple_log_record_processor.cc simple_log_record_processor_factory.cc + event_to_span_event_bridge_processor.cc + event_to_span_event_bridge_processor_factory.cc batch_log_record_processor.cc batch_log_record_processor_options.cc batch_log_record_processor_factory.cc diff --git a/sdk/src/logs/event_to_span_event_bridge_processor.cc b/sdk/src/logs/event_to_span_event_bridge_processor.cc new file mode 100644 index 0000000000..e31547fc5d --- /dev/null +++ b/sdk/src/logs/event_to_span_event_bridge_processor.cc @@ -0,0 +1,219 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#include +#include +#include +#include +#include +#include +#include + +#include "opentelemetry/common/attribute_value.h" +#include "opentelemetry/common/key_value_iterable.h" +#include "opentelemetry/common/timestamp.h" +#include "opentelemetry/nostd/function_ref.h" +#include "opentelemetry/nostd/span.h" +#include "opentelemetry/nostd/string_view.h" +#include "opentelemetry/nostd/variant.h" +#include "opentelemetry/sdk/common/attribute_utils.h" +#include "opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h" +#include "opentelemetry/sdk/logs/read_write_log_record.h" +#include "opentelemetry/sdk/logs/recordable.h" +#include "opentelemetry/trace/span.h" +#include "opentelemetry/trace/span_context.h" +#include "opentelemetry/trace/span_id.h" +#include "opentelemetry/trace/trace_id.h" +#include "opentelemetry/trace/tracer.h" +#include "opentelemetry/version.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace logs +{ + +namespace +{ + +// Converts a single OwnedAttributeValue into the corresponding AttributeValue and forwards it +// to the KeyValueIterable callback. Array-of-string and array-of-bool attributes need a +// temporary contiguous buffer (std::vector does not decay to a span of +// string_view, and std::vector is not backed by a real bool array), so those buffers are +// kept alive for the duration of a single callback invocation only. +class EventAttributeEmitter +{ +public: + EventAttributeEmitter( + nostd::string_view key, + nostd::function_ref callback, + bool *keep_going) + : key_(key), callback_(callback), keep_going_(keep_going) + {} + + void operator()(bool v) { Emit(v); } + void operator()(int32_t v) { Emit(v); } + void operator()(uint32_t v) { Emit(v); } + void operator()(int64_t v) { Emit(v); } + void operator()(uint64_t v) { Emit(v); } + void operator()(double v) { Emit(v); } + void operator()(const std::string &v) { Emit(nostd::string_view(v)); } + + void operator()(const std::vector &v) + { + std::unique_ptr buffer(new bool[v.size()]); + for (std::size_t i = 0; i < v.size(); ++i) + { + buffer[i] = v[i]; + } + Emit(nostd::span(buffer.get(), v.size())); + } + + void operator()(const std::vector &v) + { + Emit(nostd::span(v.data(), v.size())); + } + + void operator()(const std::vector &v) + { + Emit(nostd::span(v.data(), v.size())); + } + + void operator()(const std::vector &v) + { + Emit(nostd::span(v.data(), v.size())); + } + + void operator()(const std::vector &v) + { + Emit(nostd::span(v.data(), v.size())); + } + + void operator()(const std::vector &v) + { + Emit(nostd::span(v.data(), v.size())); + } + + void operator()(const std::vector &v) + { + Emit(nostd::span(v.data(), v.size())); + } + + void operator()(const std::vector &v) + { + std::vector views(v.begin(), v.end()); + Emit(nostd::span(views.data(), views.size())); + } + +private: + template + void Emit(const T &value) + { + if (*keep_going_) + { + *keep_going_ = callback_(key_, opentelemetry::common::AttributeValue(value)); + } + } + + nostd::string_view key_; + nostd::function_ref callback_; + bool *keep_going_; +}; + +class EventAttributesKeyValueIterable final : public opentelemetry::common::KeyValueIterable +{ +public: + explicit EventAttributesKeyValueIterable( + const std::unordered_map + &attributes) + : attributes_(attributes) + {} + + bool ForEachKeyValue( + nostd::function_ref callback) + const noexcept override + { + bool keep_going = true; + for (const auto &kv : attributes_) + { + if (!keep_going) + { + break; + } + EventAttributeEmitter emitter(kv.first, callback, &keep_going); + opentelemetry::nostd::visit(emitter, kv.second); + } + return keep_going; + } + + std::size_t size() const noexcept override { return attributes_.size(); } + +private: + const std::unordered_map + &attributes_; +}; + +} // namespace + +std::unique_ptr EventToSpanEventBridgeProcessor::MakeRecordable() noexcept +{ + return std::unique_ptr(new ReadWriteLogRecord()); +} + +void EventToSpanEventBridgeProcessor::OnEmit(std::unique_ptr &&record) noexcept +{ + std::unique_ptr log_record( + static_cast(record.release())); + if (!log_record) + { + return; + } + + nostd::string_view event_name = log_record->GetEventName(); + if (event_name.empty()) + { + return; + } + + const opentelemetry::trace::TraceId &log_trace_id = log_record->GetTraceId(); + const opentelemetry::trace::SpanId &log_span_id = log_record->GetSpanId(); + if (!log_trace_id.IsValid() || !log_span_id.IsValid()) + { + return; + } + + auto current_span = opentelemetry::trace::Tracer::GetCurrentSpan(); + if (!current_span || !current_span->IsRecording()) + { + return; + } + + opentelemetry::trace::SpanContext current_context = current_span->GetContext(); + if (!(current_context.trace_id() == log_trace_id) || !(current_context.span_id() == log_span_id)) + { + return; + } + + opentelemetry::common::SystemTimestamp event_timestamp = log_record->GetTimestamp(); + if (event_timestamp.time_since_epoch().count() == 0) + { + event_timestamp = log_record->GetObservedTimestamp(); + } + + EventAttributesKeyValueIterable attributes(log_record->GetAttributes()); + current_span->AddEvent(event_name, event_timestamp, attributes); +} + +bool EventToSpanEventBridgeProcessor::ForceFlush(std::chrono::microseconds /* timeout */) noexcept +{ + return true; +} + +bool EventToSpanEventBridgeProcessor::Shutdown(std::chrono::microseconds /* timeout */) noexcept +{ + return true; +} + +} // namespace logs +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/src/logs/event_to_span_event_bridge_processor_factory.cc b/sdk/src/logs/event_to_span_event_bridge_processor_factory.cc new file mode 100644 index 0000000000..4c339aa9e8 --- /dev/null +++ b/sdk/src/logs/event_to_span_event_bridge_processor_factory.cc @@ -0,0 +1,25 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#include + +#include "opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h" +#include "opentelemetry/sdk/logs/event_to_span_event_bridge_processor_factory.h" +#include "opentelemetry/sdk/logs/processor.h" +#include "opentelemetry/version.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace logs +{ + +std::unique_ptr EventToSpanEventBridgeProcessorFactory::Create() +{ + std::unique_ptr processor(new EventToSpanEventBridgeProcessor()); + return processor; +} + +} // namespace logs +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/test/configuration/yaml_logs_test.cc b/sdk/test/configuration/yaml_logs_test.cc index f885f85434..0a036cc49d 100644 --- a/sdk/test/configuration/yaml_logs_test.cc +++ b/sdk/test/configuration/yaml_logs_test.cc @@ -9,6 +9,7 @@ #include "opentelemetry/sdk/configuration/batch_log_record_processor_configuration.h" #include "opentelemetry/sdk/configuration/configuration.h" +#include "opentelemetry/sdk/configuration/event_to_span_event_bridge_log_record_processor_configuration.h" #include "opentelemetry/sdk/configuration/grpc_tls_configuration.h" #include "opentelemetry/sdk/configuration/headers_configuration.h" #include "opentelemetry/sdk/configuration/http_tls_configuration.h" @@ -100,6 +101,27 @@ file_format: "1.0-logs" ASSERT_NE(exporter, nullptr); } +TEST(YamlLogs, event_to_span_event_bridge_processor) +{ + std::string yaml = R"( +file_format: "1.0-logs" +logger_provider: + processors: + - event_to_span_event_bridge/development: +)"; + + auto config = DoParse(yaml); + ASSERT_NE(config, nullptr); + ASSERT_NE(config->logger_provider, nullptr); + ASSERT_EQ(config->logger_provider->processors.size(), 1); + auto *processor = config->logger_provider->processors[0].get(); + ASSERT_NE(processor, nullptr); + auto *bridge = dynamic_cast< + opentelemetry::sdk::configuration::EventToSpanEventBridgeLogRecordProcessorConfiguration *>( + processor); + ASSERT_NE(bridge, nullptr); +} + TEST(YamlLogs, default_batch_processor) { std::string yaml = R"( diff --git a/sdk/test/logs/BUILD b/sdk/test/logs/BUILD index 8e2198c318..4e9ffa0da6 100644 --- a/sdk/test/logs/BUILD +++ b/sdk/test/logs/BUILD @@ -66,6 +66,24 @@ cc_test( ], ) +cc_test( + name = "event_to_span_event_bridge_processor_test", + srcs = [ + "event_to_span_event_bridge_processor_test.cc", + ], + tags = [ + "logs", + "test", + ], + deps = [ + "//exporters/memory:in_memory_span_exporter", + "//sdk/src/logs", + "//sdk/src/resource", + "//sdk/src/trace", + "@com_google_googletest//:gtest_main", + ], +) + cc_test( name = "log_record_test", srcs = [ diff --git a/sdk/test/logs/CMakeLists.txt b/sdk/test/logs/CMakeLists.txt index 1fcbc3bb27..e0de53ed30 100644 --- a/sdk/test/logs/CMakeLists.txt +++ b/sdk/test/logs/CMakeLists.txt @@ -20,3 +20,18 @@ foreach( TEST_PREFIX logs. TEST_LIST ${testname}) endforeach() + +add_executable(event_to_span_event_bridge_processor_test + event_to_span_event_bridge_processor_test.cc) +target_link_libraries( + event_to_span_event_bridge_processor_test + ${GTEST_BOTH_LIBRARIES} + ${CMAKE_THREAD_LIBS_INIT} + opentelemetry_logs + opentelemetry_trace + opentelemetry_resources + opentelemetry_exporter_in_memory) +gtest_add_tests( + TARGET event_to_span_event_bridge_processor_test + TEST_PREFIX logs. + TEST_LIST event_to_span_event_bridge_processor_test) diff --git a/sdk/test/logs/event_to_span_event_bridge_processor_test.cc b/sdk/test/logs/event_to_span_event_bridge_processor_test.cc new file mode 100644 index 0000000000..b1c9ef2ed9 --- /dev/null +++ b/sdk/test/logs/event_to_span_event_bridge_processor_test.cc @@ -0,0 +1,131 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#include +#include +#include +#include +#include + +#include "opentelemetry/exporters/memory/in_memory_span_data.h" +#include "opentelemetry/exporters/memory/in_memory_span_exporter.h" +#include "opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h" +#include "opentelemetry/sdk/logs/read_write_log_record.h" +#include "opentelemetry/sdk/logs/recordable.h" +#include "opentelemetry/sdk/trace/simple_processor.h" +#include "opentelemetry/sdk/trace/tracer.h" +#include "opentelemetry/sdk/trace/tracer_context.h" +#include "opentelemetry/trace/scope.h" +#include "opentelemetry/trace/span.h" +#include "opentelemetry/trace/span_context.h" +#include "opentelemetry/trace/tracer.h" + +using namespace opentelemetry::sdk::logs; +using opentelemetry::exporter::memory::InMemorySpanData; +using opentelemetry::exporter::memory::InMemorySpanExporter; + +namespace trace_sdk = opentelemetry::sdk::trace; +namespace trace_api = opentelemetry::trace; + +namespace +{ + +std::shared_ptr MakeTracer(std::shared_ptr *out_span_data) +{ + InMemorySpanExporter *exporter = new InMemorySpanExporter(); + *out_span_data = exporter->GetData(); + + auto processor = std::unique_ptr( + new trace_sdk::SimpleSpanProcessor(std::unique_ptr(exporter))); + std::vector> processors; + processors.push_back(std::move(processor)); + auto context = std::make_shared(std::move(processors)); + return std::shared_ptr(new trace_sdk::Tracer(context)); +} + +} // namespace + +TEST(EventToSpanEventBridgeProcessorTest, BridgesEventOntoCurrentMatchingSpan) +{ + std::shared_ptr span_data; + auto tracer = MakeTracer(&span_data); + + auto span = tracer->StartSpan("test-span"); + auto scope = tracer->WithActiveSpan(span); + + EventToSpanEventBridgeProcessor processor; + auto recordable = processor.MakeRecordable(); + auto *log_record = static_cast(recordable.get()); + + trace_api::SpanContext span_context = span->GetContext(); + log_record->SetTraceId(span_context.trace_id()); + log_record->SetSpanId(span_context.span_id()); + log_record->SetEventId(0, "my.event"); + log_record->SetAttribute("key1", "value1"); + + processor.OnEmit(std::move(recordable)); + + span->End(); + + auto spans = span_data->GetSpans(); + ASSERT_EQ(1, spans.size()); + ASSERT_EQ(1, spans.at(0)->GetEvents().size()); + EXPECT_EQ("my.event", spans.at(0)->GetEvents().at(0).GetName()); +} + +TEST(EventToSpanEventBridgeProcessorTest, IgnoresLogRecordWithoutEventName) +{ + std::shared_ptr span_data; + auto tracer = MakeTracer(&span_data); + + auto span = tracer->StartSpan("test-span"); + auto scope = tracer->WithActiveSpan(span); + + EventToSpanEventBridgeProcessor processor; + auto recordable = processor.MakeRecordable(); + auto *log_record = static_cast(recordable.get()); + + trace_api::SpanContext span_context = span->GetContext(); + log_record->SetTraceId(span_context.trace_id()); + log_record->SetSpanId(span_context.span_id()); + + processor.OnEmit(std::move(recordable)); + + span->End(); + + auto spans = span_data->GetSpans(); + ASSERT_EQ(1, spans.size()); + EXPECT_EQ(0, spans.at(0)->GetEvents().size()); +} + +TEST(EventToSpanEventBridgeProcessorTest, IgnoresEventFromDifferentSpan) +{ + std::shared_ptr span_data; + auto tracer = MakeTracer(&span_data); + + auto span = tracer->StartSpan("test-span"); + auto other_span = tracer->StartSpan("other-span"); + auto scope = tracer->WithActiveSpan(span); + + EventToSpanEventBridgeProcessor processor; + auto recordable = processor.MakeRecordable(); + auto *log_record = static_cast(recordable.get()); + + // The log record belongs to other_span, not the current active span. + trace_api::SpanContext other_context = other_span->GetContext(); + log_record->SetTraceId(other_context.trace_id()); + log_record->SetSpanId(other_context.span_id()); + log_record->SetEventId(0, "my.event"); + + processor.OnEmit(std::move(recordable)); + + span->End(); + other_span->End(); + + auto spans = span_data->GetSpans(); + ASSERT_EQ(2, spans.size()); + for (auto &exported_span : spans) + { + EXPECT_EQ(0, exported_span->GetEvents().size()); + } +} From bbb273f761608136c961d2dddfbe81763f53346a Mon Sep 17 00:00:00 2001 From: om7057 Date: Sun, 26 Jul 2026 10:42:54 +0530 Subject: [PATCH 02/21] Point changelog entry at the actual PR number --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9cb4a124aa..93774e4a06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,7 @@ Increment the: * [CONFIGURATION] Implement EventToSpanEventBridge log record processor for declarative configuration - [#4155](https://github.com/open-telemetry/opentelemetry-cpp/issues/4155) + [#4309](https://github.com/open-telemetry/opentelemetry-cpp/pull/4309) * [CODE HEALTH] Move SDK trace and metrics test helpers into anonymous namespaces From c8e67dceba5ffe6111d9eaa4511840d494c5e2b1 Mon Sep 17 00:00:00 2001 From: om7057 Date: Sun, 26 Jul 2026 11:27:16 +0530 Subject: [PATCH 03/21] Fix CI failures: RTTI-free cast and unrelated formatting drift The new yaml_logs_test.cc test case used dynamic_cast, but one CI config builds with -fno-rtti; switched to reinterpret_cast to match the rest of the file. Also reverted an unrelated indentation change in sdk_builder.cc that a local clang-format 14 run introduced on lines I didn't touch; CI runs clang-format 18, which formats these continuation lines differently. --- sdk/src/configuration/sdk_builder.cc | 6 +++--- sdk/test/configuration/yaml_logs_test.cc | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/src/configuration/sdk_builder.cc b/sdk/src/configuration/sdk_builder.cc index 2baafa68cb..7c1a2edf12 100644 --- a/sdk/src/configuration/sdk_builder.cc +++ b/sdk/src/configuration/sdk_builder.cc @@ -1203,8 +1203,8 @@ std::unique_ptr SdkBuilder::CreateTra auto tracer_configurator = CreateTracerConfigurator(model->tracer_configurator); auto id_generator = opentelemetry::sdk::trace::RandomIdGeneratorFactory::Create(); sdk = opentelemetry::sdk::trace::TracerProviderFactory::Create( - std::move(sdk_processors), resource, std::move(sampler), std::move(id_generator), - std::move(tracer_configurator), span_limits); + std::move(sdk_processors), resource, std::move(sampler), std::move(id_generator), + std::move(tracer_configurator), span_limits); } else { @@ -1770,7 +1770,7 @@ std::unique_ptr SdkBuilder::CreateMe { auto meter_configurator = CreateMeterConfigurator(model->meter_configurator); meter_context = opentelemetry::sdk::metrics::MeterContextFactory::Create( - std::move(view_registry), resource, std::move(meter_configurator)); + std::move(view_registry), resource, std::move(meter_configurator)); } else { diff --git a/sdk/test/configuration/yaml_logs_test.cc b/sdk/test/configuration/yaml_logs_test.cc index 0a036cc49d..f2d0d83e7e 100644 --- a/sdk/test/configuration/yaml_logs_test.cc +++ b/sdk/test/configuration/yaml_logs_test.cc @@ -116,7 +116,7 @@ file_format: "1.0-logs" ASSERT_EQ(config->logger_provider->processors.size(), 1); auto *processor = config->logger_provider->processors[0].get(); ASSERT_NE(processor, nullptr); - auto *bridge = dynamic_cast< + auto *bridge = reinterpret_cast< opentelemetry::sdk::configuration::EventToSpanEventBridgeLogRecordProcessorConfiguration *>( processor); ASSERT_NE(bridge, nullptr); From 46fab04b40e28183b6d0951b491d53e00f624495 Mon Sep 17 00:00:00 2001 From: om7057 Date: Sun, 26 Jul 2026 12:50:35 +0530 Subject: [PATCH 04/21] Fix clang-tidy and iwyu findings on the new processor clang-tidy flagged three things that pushed the abiv2-preview preset over its warning budget: the attributes_ member stored a reference instead of a pointer, OnEmit released the incoming unique_ptr without an explicit std::move, and ForEachKeyValue could let an allocation failure escape a noexcept override. Switched the member to a pointer, added std::move, and wrapped the iteration in a try/catch guarded by OPENTELEMETRY_HAVE_EXCEPTIONS, matching the pattern already used in periodic_exporting_metric_reader.cc. iwyu wanted in both files for std::move, plus a few trace headers directly included in the test file rather than pulled in transitively. --- .../event_to_span_event_bridge_processor.cc | 37 +++++++++++++------ ...ent_to_span_event_bridge_processor_test.cc | 6 ++- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/sdk/src/logs/event_to_span_event_bridge_processor.cc b/sdk/src/logs/event_to_span_event_bridge_processor.cc index e31547fc5d..9cb34f6240 100644 --- a/sdk/src/logs/event_to_span_event_bridge_processor.cc +++ b/sdk/src/logs/event_to_span_event_bridge_processor.cc @@ -7,6 +7,7 @@ #include #include #include +#include #include #include "opentelemetry/common/attribute_value.h" @@ -126,31 +127,45 @@ class EventAttributesKeyValueIterable final : public opentelemetry::common::KeyV explicit EventAttributesKeyValueIterable( const std::unordered_map &attributes) - : attributes_(attributes) + : attributes_(&attributes) {} bool ForEachKeyValue( nostd::function_ref callback) const noexcept override { - bool keep_going = true; - for (const auto &kv : attributes_) + // Building temporary buffers for array-of-bool and array-of-string attributes (see + // EventAttributeEmitter above) can allocate, so guard against a thrown bad_alloc escaping + // this noexcept override; conservatively stop iterating rather than terminate the process. +#if OPENTELEMETRY_HAVE_EXCEPTIONS + try { - if (!keep_going) +#endif + bool keep_going = true; + for (const auto &kv : *attributes_) { - break; + if (!keep_going) + { + break; + } + EventAttributeEmitter emitter(kv.first, callback, &keep_going); + opentelemetry::nostd::visit(emitter, kv.second); } - EventAttributeEmitter emitter(kv.first, callback, &keep_going); - opentelemetry::nostd::visit(emitter, kv.second); + return keep_going; +#if OPENTELEMETRY_HAVE_EXCEPTIONS } - return keep_going; + catch (...) + { + return false; + } +#endif } - std::size_t size() const noexcept override { return attributes_.size(); } + std::size_t size() const noexcept override { return attributes_->size(); } private: const std::unordered_map - &attributes_; + *attributes_; }; } // namespace @@ -163,7 +178,7 @@ std::unique_ptr EventToSpanEventBridgeProcessor::MakeRecordable() no void EventToSpanEventBridgeProcessor::OnEmit(std::unique_ptr &&record) noexcept { std::unique_ptr log_record( - static_cast(record.release())); + static_cast(std::move(record).release())); if (!log_record) { return; diff --git a/sdk/test/logs/event_to_span_event_bridge_processor_test.cc b/sdk/test/logs/event_to_span_event_bridge_processor_test.cc index b1c9ef2ed9..80a43ac740 100644 --- a/sdk/test/logs/event_to_span_event_bridge_processor_test.cc +++ b/sdk/test/logs/event_to_span_event_bridge_processor_test.cc @@ -2,9 +2,9 @@ // SPDX-License-Identifier: Apache-2.0 #include -#include #include #include +#include #include #include "opentelemetry/exporters/memory/in_memory_span_data.h" @@ -12,10 +12,12 @@ #include "opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h" #include "opentelemetry/sdk/logs/read_write_log_record.h" #include "opentelemetry/sdk/logs/recordable.h" +#include "opentelemetry/sdk/trace/exporter.h" +#include "opentelemetry/sdk/trace/processor.h" #include "opentelemetry/sdk/trace/simple_processor.h" +#include "opentelemetry/sdk/trace/span_data.h" #include "opentelemetry/sdk/trace/tracer.h" #include "opentelemetry/sdk/trace/tracer_context.h" -#include "opentelemetry/trace/scope.h" #include "opentelemetry/trace/span.h" #include "opentelemetry/trace/span_context.h" #include "opentelemetry/trace/tracer.h" From 41f9b896d1d7f3ae74da25a7f242de6cd310db64 Mon Sep 17 00:00:00 2001 From: om7057 Date: Sun, 26 Jul 2026 19:06:02 +0530 Subject: [PATCH 05/21] Fix iwyu: include event_to_span_event_bridge configuration header directly configuration_parser.cc used EventToSpanEventBridgeLogRecordProcessorConfiguration via a transitive include only, which iwyu flagged in both abiv1-preview and abiv2-preview jobs. --- sdk/src/configuration/configuration_parser.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/src/configuration/configuration_parser.cc b/sdk/src/configuration/configuration_parser.cc index b5793b53ff..3cf0f68230 100644 --- a/sdk/src/configuration/configuration_parser.cc +++ b/sdk/src/configuration/configuration_parser.cc @@ -49,6 +49,7 @@ #include "opentelemetry/sdk/configuration/double_array_attribute_value_configuration.h" #include "opentelemetry/sdk/configuration/double_attribute_value_configuration.h" #include "opentelemetry/sdk/configuration/drop_aggregation_configuration.h" +#include "opentelemetry/sdk/configuration/event_to_span_event_bridge_log_record_processor_configuration.h" #include "opentelemetry/sdk/configuration/exemplar_filter.h" #include "opentelemetry/sdk/configuration/explicit_bucket_histogram_aggregation_configuration.h" #include "opentelemetry/sdk/configuration/extension_log_record_exporter_configuration.h" From ee5769b3f0225a1326dcef025345ff72881d7453 Mon Sep 17 00:00:00 2001 From: om7057 Date: Tue, 28 Jul 2026 19:56:50 +0530 Subject: [PATCH 06/21] refactor(logs): add OnEmitWithContext to LogRecordProcessor API Add OnEmitWithContext() method to LogRecordProcessor interface to pass the resolved context (explicit if provided, else ambient) to processors that need it. The context is valid only for the duration of the call and must not be retained. Updates: - LogRecordProcessor: new OnEmitWithContext() virtual method (default forwards to OnEmit) - Logger: call OnEmitWithContext with resolved context - MultiLogRecordProcessor: implement OnEmitWithContext to forward to child processors This enables processors like EventToSpanEventBridge to access the log's resolved context for proper span matching, addressing spec compliance for explicit contexts. --- .../sdk/logs/multi_log_record_processor.h | 9 ++++++++ .../opentelemetry/sdk/logs/processor.h | 20 +++++++++++++++++ sdk/src/logs/logger.cc | 6 +++-- sdk/src/logs/multi_log_record_processor.cc | 22 +++++++++++++++++++ 4 files changed, 55 insertions(+), 2 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/logs/multi_log_record_processor.h b/sdk/include/opentelemetry/sdk/logs/multi_log_record_processor.h index b7f73ec0c1..41143fa78a 100644 --- a/sdk/include/opentelemetry/sdk/logs/multi_log_record_processor.h +++ b/sdk/include/opentelemetry/sdk/logs/multi_log_record_processor.h @@ -44,6 +44,15 @@ class MultiLogRecordProcessor : public LogRecordProcessor */ void OnEmit(std::unique_ptr &&record) noexcept override; + /** + * OnEmitWithContext forwards the context to child processors. + */ + void OnEmitWithContext( + std::unique_ptr &&record, + const opentelemetry::nostd::variant &context) + noexcept override; + /** * Exports all log records that have not yet been exported to the configured Exporter. * @param timeout that the forceflush is required to finish within. diff --git a/sdk/include/opentelemetry/sdk/logs/processor.h b/sdk/include/opentelemetry/sdk/logs/processor.h index 41e47d1a27..de9f6c62d3 100644 --- a/sdk/include/opentelemetry/sdk/logs/processor.h +++ b/sdk/include/opentelemetry/sdk/logs/processor.h @@ -60,6 +60,26 @@ class LogRecordProcessor */ virtual void OnEmit(std::unique_ptr &&record) noexcept = 0; + /** + * OnEmitWithContext is called by the SDK once a log record has been successfully created, + * with access to the resolved context (explicit if provided, else ambient). + * The resolved context is valid only for the duration of this call and must not be retained. + * + * The default implementation forwards to OnEmit() for backward compatibility. + * Processors that need access to the log's resolved context should override this method. + * + * @param record the log recordable object + * @param context the resolved context (SpanContext or Context variant) + */ + virtual void OnEmitWithContext( + std::unique_ptr &&record, + const opentelemetry::nostd::variant & /*context*/) + noexcept + { + OnEmit(std::move(record)); + } + /** * Enabled returns whether this processor is interested in a log with the given inputs. * The default implementation is permissive and returns true. diff --git a/sdk/src/logs/logger.cc b/sdk/src/logs/logger.cc index 20bc6e4562..eda0a2c635 100644 --- a/sdk/src/logs/logger.cc +++ b/sdk/src/logs/logger.cc @@ -197,8 +197,10 @@ void Logger::EmitLogRecord( auto &processor = context_->GetProcessor(); - // Send the log recordable to the processor - processor.OnEmit(std::move(recordable)); + // Send the log recordable to the processor with the resolved context + const nostd::variant resolved_context{ + context::RuntimeContext::GetCurrent()}; + processor.OnEmitWithContext(std::move(recordable), resolved_context); } bool Logger::EnabledImplementation(opentelemetry::logs::Severity severity, diff --git a/sdk/src/logs/multi_log_record_processor.cc b/sdk/src/logs/multi_log_record_processor.cc index 8ebb004213..87f1095502 100644 --- a/sdk/src/logs/multi_log_record_processor.cc +++ b/sdk/src/logs/multi_log_record_processor.cc @@ -74,6 +74,28 @@ void MultiLogRecordProcessor::OnEmit(std::unique_ptr &&record) noexc } } +void MultiLogRecordProcessor::OnEmitWithContext( + std::unique_ptr &&record, + const opentelemetry::nostd::variant &context) noexcept +{ + auto log_record = std::move(record); + if (!log_record) + { + return; + } + auto multi_recordable = static_cast(log_record.get()); + + for (auto &processor : processors_) + { + auto recordable = multi_recordable->ReleaseRecordable(*processor); + if (recordable) + { + processor->OnEmitWithContext(std::move(recordable), context); + } + } +} + bool MultiLogRecordProcessor::EnabledImplementation( const opentelemetry::nostd::variant &context_or_span, From caf5e607c19ef6ccfb17b3e53ef6c5a1b3800346 Mon Sep 17 00:00:00 2001 From: om7057 Date: Tue, 28 Jul 2026 19:58:31 +0530 Subject: [PATCH 07/21] feat(logs): implement EventToSpanEventBridge with resolved context support Update EventToSpanEventBridgeProcessor to override OnEmitWithContext and use the log's resolved context (explicit if provided, else ambient) for proper span matching. Changes: - Override OnEmitWithContext to receive the resolved context from the logger - Extract span context from the variant (SpanContext or Context) - Compare resolved span context to log's trace/span IDs - Verify current active span matches resolved context before adding event - Update processor docstring to clarify context usage This ensures spec compliance for explicitly-supplied log contexts where the log and ambient span might differ. --- .../event_to_span_event_bridge_processor.h | 16 +++- .../event_to_span_event_bridge_processor.cc | 74 +++++++++++++++++++ 2 files changed, 86 insertions(+), 4 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h b/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h index 8eba69894a..308f5d3514 100644 --- a/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h +++ b/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h @@ -18,14 +18,16 @@ namespace logs /** * A log record processor that bridges log records representing events (i.e. log records with a - * non-empty event name) onto the current active span as span events. + * non-empty event name) onto the corresponding span as span events. * * A log record is bridged to a span event if and only if: * - the log record has a non-empty event name, * - the log record carries a valid trace id and span id, - * - the current active span (from the ambient context at the time OnEmit is called) is - * recording, and - * - the current active span's trace id and span id match those on the log record. + * - the span with those IDs is the current active span (from the log's resolved context), and + * - that span is currently recording. + * + * The resolved context (explicit if provided to the logger, else ambient) is used to determine + * which span to bridge to, ensuring spec compliance for explicitly-supplied contexts. * * This processor does not export log records and does not stop the log record from continuing * through the rest of the configured processing pipeline. @@ -44,6 +46,12 @@ class EventToSpanEventBridgeProcessor : public LogRecordProcessor void OnEmit(std::unique_ptr &&record) noexcept override; + void OnEmitWithContext( + std::unique_ptr &&record, + const opentelemetry::nostd::variant &context) + noexcept override; + bool ForceFlush( std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override; diff --git a/sdk/src/logs/event_to_span_event_bridge_processor.cc b/sdk/src/logs/event_to_span_event_bridge_processor.cc index 9cb34f6240..9afc9e1587 100644 --- a/sdk/src/logs/event_to_span_event_bridge_processor.cc +++ b/sdk/src/logs/event_to_span_event_bridge_processor.cc @@ -219,6 +219,80 @@ void EventToSpanEventBridgeProcessor::OnEmit(std::unique_ptr &&recor current_span->AddEvent(event_name, event_timestamp, attributes); } +void EventToSpanEventBridgeProcessor::OnEmitWithContext( + std::unique_ptr &&record, + const opentelemetry::nostd::variant &context) noexcept +{ + std::unique_ptr log_record( + static_cast(std::move(record).release())); + if (!log_record) + { + return; + } + + nostd::string_view event_name = log_record->GetEventName(); + if (event_name.empty()) + { + return; + } + + const opentelemetry::trace::TraceId &log_trace_id = log_record->GetTraceId(); + const opentelemetry::trace::SpanId &log_span_id = log_record->GetSpanId(); + if (!log_trace_id.IsValid() || !log_span_id.IsValid()) + { + return; + } + + // Extract span context from the resolved context + opentelemetry::trace::SpanContext resolved_span_context{}; + if (nostd::holds_alternative(context)) + { + resolved_span_context = nostd::get(context); + } + else + { + // Context variant contains a Context object; get the active span from it + const auto &ctx = nostd::get(context); + auto span = opentelemetry::trace::GetSpan(ctx); + if (!span) + { + return; + } + resolved_span_context = span->GetContext(); + } + + // Check if the resolved span context matches the log's trace/span IDs + if (!(resolved_span_context.trace_id() == log_trace_id) || + !(resolved_span_context.span_id() == log_span_id)) + { + return; + } + + // Get the span object for the resolved context and add the event + auto current_span = opentelemetry::trace::Tracer::GetCurrentSpan(); + if (!current_span || !current_span->IsRecording()) + { + return; + } + + // Verify that the current span matches the resolved context + if (!(current_span->GetContext().trace_id() == log_trace_id) || + !(current_span->GetContext().span_id() == log_span_id)) + { + return; + } + + opentelemetry::common::SystemTimestamp event_timestamp = log_record->GetTimestamp(); + if (event_timestamp.time_since_epoch().count() == 0) + { + event_timestamp = log_record->GetObservedTimestamp(); + } + + EventAttributesKeyValueIterable attributes(log_record->GetAttributes()); + current_span->AddEvent(event_name, event_timestamp, attributes); +} + bool EventToSpanEventBridgeProcessor::ForceFlush(std::chrono::microseconds /* timeout */) noexcept { return true; From ec30ecbb9b8cd1fcd5aa0879c50d5bbdc00a7043 Mon Sep 17 00:00:00 2001 From: om7057 Date: Thu, 30 Jul 2026 00:18:19 +0530 Subject: [PATCH 08/21] fix: add missing includes for OnEmitWithContext --- .../sdk/logs/event_to_span_event_bridge_processor.h | 3 +++ sdk/src/logs/event_to_span_event_bridge_processor.cc | 1 + 2 files changed, 4 insertions(+) diff --git a/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h b/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h index 308f5d3514..3b2e856e04 100644 --- a/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h +++ b/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h @@ -6,8 +6,11 @@ #include #include +#include "opentelemetry/context/context.h" +#include "opentelemetry/nostd/variant.h" #include "opentelemetry/sdk/logs/processor.h" #include "opentelemetry/sdk/logs/recordable.h" +#include "opentelemetry/trace/span_context.h" #include "opentelemetry/version.h" OPENTELEMETRY_BEGIN_NAMESPACE diff --git a/sdk/src/logs/event_to_span_event_bridge_processor.cc b/sdk/src/logs/event_to_span_event_bridge_processor.cc index 9afc9e1587..8dcd749c31 100644 --- a/sdk/src/logs/event_to_span_event_bridge_processor.cc +++ b/sdk/src/logs/event_to_span_event_bridge_processor.cc @@ -21,6 +21,7 @@ #include "opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h" #include "opentelemetry/sdk/logs/read_write_log_record.h" #include "opentelemetry/sdk/logs/recordable.h" +#include "opentelemetry/trace/context.h" #include "opentelemetry/trace/span.h" #include "opentelemetry/trace/span_context.h" #include "opentelemetry/trace/span_id.h" From a6ecfdac5d7bd1e03b26075b725a86c7514ba170 Mon Sep 17 00:00:00 2001 From: om7057 Date: Thu, 30 Jul 2026 07:52:29 +0530 Subject: [PATCH 09/21] Address review: fix explicit-context bridging, gate context resolution, harden tests Bridge onto the span from the resolved context (ThomsonTan review): OnEmitWithContext() looked up the live span from the resolved Context and then discarded it in favour of the ambient span, so an event whose ids referred to an explicitly supplied span was dropped whenever a different span was ambient. The span from the resolved Context is now the span the event is added to. A bare SpanContext carries no live Span, so that alternative is handled separately and still resolves through the ambient span, with the id check rejecting a mismatch. Remove the duplicated emit pipeline: OnEmit() and OnEmitWithContext() had drifted apart. Both now share BridgeRecordToSpan(), which holds all preconditions and the emission in one place. Keep the emit hot path free of context resolution: LogRecordProcessor gains ConsumesResolvedContext() (default false), MultiLogRecordProcessor reports true when any child does, and LoggerContext caches the answer the same way it already caches RecordableEnforcesLogRecordLimits(). Logger only resolves the ambient context and dispatches through OnEmitWithContext() when some processor will actually read it, so simple/batch pipelines keep dispatching through OnEmit() with no extra RuntimeContext::GetCurrent() call. Document compatibility: OnEmitWithContext() and ConsumesResolvedContext() are source compatible via default implementations but change the LogRecordProcessor vtable, so subclasses must be recompiled. Noted in the header and in the CHANGELOG breaking-changes section. VisitEventToSpanEventBridge() was pure virtual, which broke existing visitors, and now defaults to a no-op. Make the YAML assertion type-aware: the reinterpret_cast only repeated the preceding non-null check and passed for a misspelled key that fell through to the extension configuration. The test now dispatches through Accept() with a recording visitor, and a companion test pins the misspelled-key fallback. Tests: explicit span_a with ambient span_b asserting only span_a receives the event (fails against the previous code), explicit-context mismatch, bare SpanContext, and Logger-level tests that a context-ignoring processor dispatches through OnEmit() and adds no GetCurrent() call, using a counting RuntimeContextStorage. --- CHANGELOG.md | 13 ++ ...g_record_processor_configuration_visitor.h | 12 +- .../event_to_span_event_bridge_processor.h | 11 +- .../opentelemetry/sdk/logs/logger_context.h | 14 ++ .../sdk/logs/multi_log_record_processor.h | 11 +- .../opentelemetry/sdk/logs/processor.h | 35 ++++- .../event_to_span_event_bridge_processor.cc | 119 ++++++---------- sdk/src/logs/logger.cc | 17 ++- sdk/src/logs/logger_context.cc | 9 +- sdk/src/logs/multi_log_record_processor.cc | 12 ++ sdk/test/configuration/yaml_logs_test.cc | 84 ++++++++++- ...ent_to_span_event_bridge_processor_test.cc | 131 +++++++++++++++++ sdk/test/logs/logger_sdk_test.cc | 133 ++++++++++++++++++ 13 files changed, 497 insertions(+), 104 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 85f8e96577..dc9c3d4efa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,19 @@ Breaking changes: * The public configuration member `max_buckets_` was renamed to `max_size_` to match the configuration schema. Please adjust SDK configuration accordingly. +* [LOGS SDK] `LogRecordProcessor` gained the virtual methods + `OnEmitWithContext()` and `ConsumesResolvedContext()` + [#4309](https://github.com/open-telemetry/opentelemetry-cpp/pull/4309) + * Source compatible: both methods have default implementations, so existing + processors continue to compile and behave as before. `OnEmitWithContext()` + forwards to `OnEmit()` and `ConsumesResolvedContext()` returns `false`. + * Not ABI compatible: the added virtuals change the vtable layout of + `LogRecordProcessor`, so code that subclasses it must be recompiled against + this version of the SDK. + * `LogRecordProcessorConfigurationVisitor::VisitEventToSpanEventBridge()` was + added with a default no-op implementation, so existing visitor + implementations remain source compatible. + ## [1.28.0] 2026-07-16 * [RELEASE] Bump main branch to 1.28.0-dev diff --git a/sdk/include/opentelemetry/sdk/configuration/log_record_processor_configuration_visitor.h b/sdk/include/opentelemetry/sdk/configuration/log_record_processor_configuration_visitor.h index 320a8fe4b1..94701e9dda 100644 --- a/sdk/include/opentelemetry/sdk/configuration/log_record_processor_configuration_visitor.h +++ b/sdk/include/opentelemetry/sdk/configuration/log_record_processor_configuration_visitor.h @@ -30,8 +30,18 @@ class LogRecordProcessorConfigurationVisitor virtual void VisitBatch(const BatchLogRecordProcessorConfiguration *model) = 0; virtual void VisitSimple(const SimpleLogRecordProcessorConfiguration *model) = 0; + + /** + * Visit an event_to_span_event_bridge/development processor configuration. + * + * Unlike its siblings this method is not pure virtual: it was added after the visitor was + * already public, and defaulting it to a no-op keeps existing visitor implementations + * compiling. Visitors that support this processor type should override it. + */ virtual void VisitEventToSpanEventBridge( - const EventToSpanEventBridgeLogRecordProcessorConfiguration *model) = 0; + const EventToSpanEventBridgeLogRecordProcessorConfiguration * /* model */) + {} + virtual void VisitExtension(const ExtensionLogRecordProcessorConfiguration *model) = 0; }; diff --git a/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h b/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h index 3b2e856e04..1272547187 100644 --- a/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h +++ b/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h @@ -49,11 +49,10 @@ class EventToSpanEventBridgeProcessor : public LogRecordProcessor void OnEmit(std::unique_ptr &&record) noexcept override; - void OnEmitWithContext( - std::unique_ptr &&record, - const opentelemetry::nostd::variant &context) - noexcept override; + void OnEmitWithContext(std::unique_ptr &&record, + const opentelemetry::nostd::variant + &context) noexcept override; bool ForceFlush( std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override; @@ -62,6 +61,8 @@ class EventToSpanEventBridgeProcessor : public LogRecordProcessor std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override; bool HasEnabledFilter() const noexcept override { return false; } + + bool ConsumesResolvedContext() const noexcept override { return true; } }; } // namespace logs diff --git a/sdk/include/opentelemetry/sdk/logs/logger_context.h b/sdk/include/opentelemetry/sdk/logs/logger_context.h index bf19fa879c..dc93bb7eb9 100644 --- a/sdk/include/opentelemetry/sdk/logs/logger_context.h +++ b/sdk/include/opentelemetry/sdk/logs/logger_context.h @@ -93,6 +93,15 @@ class LoggerContext */ bool RecordableEnforcesLogRecordLimits() const noexcept; + /** + * Whether any configured processor reads the resolved context passed to + * LogRecordProcessor::OnEmitWithContext(). Computed once at construction (and refreshed by + * AddProcessor) so the Logger hot path can dispatch through plain OnEmit() -- and skip + * resolving the ambient context entirely -- when no processor needs it. + * @return true if at least one processor consumes the resolved context. + */ + bool ConsumesResolvedContext() const noexcept; + /** * Replace the ScopeConfigurator for this logger context. * @param logger_configurator The new configurator. @@ -124,6 +133,11 @@ class LoggerContext // recordable enforces the LogRecord attribute limits. Refreshed by // AddProcessor so the Logger hot path reads a plain bool. bool recordable_enforces_limits_{false}; + + // Cached capability flag: true when at least one configured processor reads the resolved + // context. Refreshed by AddProcessor so the Logger hot path reads a plain bool and only + // resolves the ambient context when some processor will actually look at it. + bool consumes_resolved_context_{false}; }; } // namespace logs } // namespace sdk diff --git a/sdk/include/opentelemetry/sdk/logs/multi_log_record_processor.h b/sdk/include/opentelemetry/sdk/logs/multi_log_record_processor.h index 41143fa78a..e8a3b7c2ec 100644 --- a/sdk/include/opentelemetry/sdk/logs/multi_log_record_processor.h +++ b/sdk/include/opentelemetry/sdk/logs/multi_log_record_processor.h @@ -47,11 +47,10 @@ class MultiLogRecordProcessor : public LogRecordProcessor /** * OnEmitWithContext forwards the context to child processors. */ - void OnEmitWithContext( - std::unique_ptr &&record, - const opentelemetry::nostd::variant &context) - noexcept override; + void OnEmitWithContext(std::unique_ptr &&record, + const opentelemetry::nostd::variant + &context) noexcept override; /** * Exports all log records that have not yet been exported to the configured Exporter. @@ -75,6 +74,8 @@ class MultiLogRecordProcessor : public LogRecordProcessor bool RecordableEnforcesLogRecordLimits() const noexcept override; + bool ConsumesResolvedContext() const noexcept override; + protected: /** * Exports all log records that have not yet been exported to the configured Exporter. diff --git a/sdk/include/opentelemetry/sdk/logs/processor.h b/sdk/include/opentelemetry/sdk/logs/processor.h index de9f6c62d3..99688c3157 100644 --- a/sdk/include/opentelemetry/sdk/logs/processor.h +++ b/sdk/include/opentelemetry/sdk/logs/processor.h @@ -61,12 +61,24 @@ class LogRecordProcessor virtual void OnEmit(std::unique_ptr &&record) noexcept = 0; /** - * OnEmitWithContext is called by the SDK once a log record has been successfully created, - * with access to the resolved context (explicit if provided, else ambient). - * The resolved context is valid only for the duration of this call and must not be retained. + * OnEmitWithContext is called by the SDK instead of OnEmit() when some processor in the + * configured pipeline reports ConsumesResolvedContext(), giving the processor access to the + * log record's resolved context: the explicitly supplied context when one was provided to the + * Logger, otherwise the ambient context. * - * The default implementation forwards to OnEmit() for backward compatibility. - * Processors that need access to the log's resolved context should override this method. + * The supplied context is valid only for the duration of this call. Implementations must not + * retain it (or anything obtained from it, such as a Span) beyond the call, because doing so + * would keep the referenced span alive past its intended lifetime. + * + * Compatibility notes: + * - Source compatibility is preserved. This method has a default implementation that forwards + * to OnEmit(), so existing processors continue to compile and behave exactly as before. + * A distinct name (rather than an OnEmit() overload) is used so that a derived class + * overriding only OnEmit() does not hide this method. + * - ABI compatibility is NOT preserved. Adding a virtual method changes the vtable layout of + * LogRecordProcessor, so downstream code that subclasses LogRecordProcessor must be + * recompiled against this version of the SDK. Mixing a processor built against an older + * header with this SDK is undefined behavior. * * @param record the log recordable object * @param context the resolved context (SpanContext or Context variant) @@ -74,12 +86,21 @@ class LogRecordProcessor virtual void OnEmitWithContext( std::unique_ptr &&record, const opentelemetry::nostd::variant & /*context*/) - noexcept + opentelemetry::context::Context> & /*context*/) noexcept { OnEmit(std::move(record)); } + /** + * Returns true when this processor reads the resolved context passed to + * OnEmitWithContext(). The default returns false, so the SDK Logger keeps dispatching through + * OnEmit() and does not resolve the ambient context on the emit path. + * + * Only processors that override OnEmitWithContext() should return true. Composite processors + * return true if any child does. + */ + virtual bool ConsumesResolvedContext() const noexcept { return false; } + /** * Enabled returns whether this processor is interested in a log with the given inputs. * The default implementation is permissive and returns true. diff --git a/sdk/src/logs/event_to_span_event_bridge_processor.cc b/sdk/src/logs/event_to_span_event_bridge_processor.cc index 8dcd749c31..def44b4d28 100644 --- a/sdk/src/logs/event_to_span_event_bridge_processor.cc +++ b/sdk/src/logs/event_to_span_event_bridge_processor.cc @@ -14,6 +14,7 @@ #include "opentelemetry/common/key_value_iterable.h" #include "opentelemetry/common/timestamp.h" #include "opentelemetry/nostd/function_ref.h" +#include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/span.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/nostd/variant.h" @@ -169,18 +170,15 @@ class EventAttributesKeyValueIterable final : public opentelemetry::common::KeyV *attributes_; }; -} // namespace - -std::unique_ptr EventToSpanEventBridgeProcessor::MakeRecordable() noexcept -{ - return std::unique_ptr(new ReadWriteLogRecord()); -} - -void EventToSpanEventBridgeProcessor::OnEmit(std::unique_ptr &&record) noexcept +// Bridges `record` onto `span` as a span event, provided every precondition holds: +// the record is an event (non-empty event name), it carries a valid trace id and span id, +// `span` is a live recording span, and `span`'s ids match the ones on the record. +// Any failed precondition means the record is not an event for this span, and it is left +// to continue through the rest of the pipeline untouched. +void BridgeRecordToSpan(const ReadWriteLogRecord *log_record, + const nostd::shared_ptr &span) noexcept { - std::unique_ptr log_record( - static_cast(std::move(record).release())); - if (!log_record) + if (log_record == nullptr) { return; } @@ -198,14 +196,15 @@ void EventToSpanEventBridgeProcessor::OnEmit(std::unique_ptr &&recor return; } - auto current_span = opentelemetry::trace::Tracer::GetCurrentSpan(); - if (!current_span || !current_span->IsRecording()) + // trace::GetSpan() yields a non-recording DefaultSpan when the context carries no span, + // so the IsRecording() check also covers the "no span in context" case. + if (!span || !span->IsRecording()) { return; } - opentelemetry::trace::SpanContext current_context = current_span->GetContext(); - if (!(current_context.trace_id() == log_trace_id) || !(current_context.span_id() == log_span_id)) + const opentelemetry::trace::SpanContext span_context = span->GetContext(); + if (!(span_context.trace_id() == log_trace_id) || !(span_context.span_id() == log_span_id)) { return; } @@ -217,7 +216,23 @@ void EventToSpanEventBridgeProcessor::OnEmit(std::unique_ptr &&recor } EventAttributesKeyValueIterable attributes(log_record->GetAttributes()); - current_span->AddEvent(event_name, event_timestamp, attributes); + span->AddEvent(event_name, event_timestamp, attributes); +} + +} // namespace + +std::unique_ptr EventToSpanEventBridgeProcessor::MakeRecordable() noexcept +{ + return std::unique_ptr(new ReadWriteLogRecord()); +} + +void EventToSpanEventBridgeProcessor::OnEmit(std::unique_ptr &&record) noexcept +{ + const std::unique_ptr log_record( + static_cast(std::move(record).release())); + + // No resolved context supplied, so the ambient span is the only span reachable here. + BridgeRecordToSpan(log_record.get(), opentelemetry::trace::Tracer::GetCurrentSpan()); } void EventToSpanEventBridgeProcessor::OnEmitWithContext( @@ -225,73 +240,23 @@ void EventToSpanEventBridgeProcessor::OnEmitWithContext( const opentelemetry::nostd::variant &context) noexcept { - std::unique_ptr log_record( + const std::unique_ptr log_record( static_cast(std::move(record).release())); - if (!log_record) - { - return; - } - - nostd::string_view event_name = log_record->GetEventName(); - if (event_name.empty()) - { - return; - } - - const opentelemetry::trace::TraceId &log_trace_id = log_record->GetTraceId(); - const opentelemetry::trace::SpanId &log_span_id = log_record->GetSpanId(); - if (!log_trace_id.IsValid() || !log_span_id.IsValid()) - { - return; - } - - // Extract span context from the resolved context - opentelemetry::trace::SpanContext resolved_span_context{}; - if (nostd::holds_alternative(context)) - { - resolved_span_context = nostd::get(context); - } - else - { - // Context variant contains a Context object; get the active span from it - const auto &ctx = nostd::get(context); - auto span = opentelemetry::trace::GetSpan(ctx); - if (!span) - { - return; - } - resolved_span_context = span->GetContext(); - } - // Check if the resolved span context matches the log's trace/span IDs - if (!(resolved_span_context.trace_id() == log_trace_id) || - !(resolved_span_context.span_id() == log_span_id)) + if (nostd::holds_alternative(context)) { + // The resolved context carries the live Span the record belongs to. Add the event to that + // span directly, which is what makes an explicitly supplied context work even when a + // different span is ambient. + BridgeRecordToSpan(log_record.get(), opentelemetry::trace::GetSpan( + nostd::get(context))); return; } - // Get the span object for the resolved context and add the event - auto current_span = opentelemetry::trace::Tracer::GetCurrentSpan(); - if (!current_span || !current_span->IsRecording()) - { - return; - } - - // Verify that the current span matches the resolved context - if (!(current_span->GetContext().trace_id() == log_trace_id) || - !(current_span->GetContext().span_id() == log_span_id)) - { - return; - } - - opentelemetry::common::SystemTimestamp event_timestamp = log_record->GetTimestamp(); - if (event_timestamp.time_since_epoch().count() == 0) - { - event_timestamp = log_record->GetObservedTimestamp(); - } - - EventAttributesKeyValueIterable attributes(log_record->GetAttributes()); - current_span->AddEvent(event_name, event_timestamp, attributes); + // A bare SpanContext carries only ids, not a live Span, and the SDK offers no way to look up + // a Span by id. The ambient span is therefore the only span that can be written to; the id + // check in BridgeRecordToSpan drops the record when it is not the one the ids refer to. + BridgeRecordToSpan(log_record.get(), opentelemetry::trace::Tracer::GetCurrentSpan()); } bool EventToSpanEventBridgeProcessor::ForceFlush(std::chrono::microseconds /* timeout */) noexcept diff --git a/sdk/src/logs/logger.cc b/sdk/src/logs/logger.cc index eda0a2c635..da07b5c66d 100644 --- a/sdk/src/logs/logger.cc +++ b/sdk/src/logs/logger.cc @@ -197,10 +197,19 @@ void Logger::EmitLogRecord( auto &processor = context_->GetProcessor(); - // Send the log recordable to the processor with the resolved context - const nostd::variant resolved_context{ - context::RuntimeContext::GetCurrent()}; - processor.OnEmitWithContext(std::move(recordable), resolved_context); + // Only resolve the context when a processor will actually read it: resolving means a + // RuntimeContext::GetCurrent() call, which would otherwise be pure overhead on every log for + // the common pipelines (simple/batch) that ignore the context entirely. + if (context_->ConsumesResolvedContext()) + { + const nostd::variant resolved_context{ + context::RuntimeContext::GetCurrent()}; + processor.OnEmitWithContext(std::move(recordable), resolved_context); + return; + } + + // Send the log recordable to the processor + processor.OnEmit(std::move(recordable)); } bool Logger::EnabledImplementation(opentelemetry::logs::Severity severity, diff --git a/sdk/src/logs/logger_context.cc b/sdk/src/logs/logger_context.cc index 7f6d5315ec..6f7eba304c 100644 --- a/sdk/src/logs/logger_context.cc +++ b/sdk/src/logs/logger_context.cc @@ -33,7 +33,8 @@ LoggerContext::LoggerContext( std::unique_ptr(new MultiLogRecordProcessor(std::move(processors)))), logger_configurator_(std::move(logger_configurator)), log_record_limits_(log_record_limits), - recordable_enforces_limits_(processor_->RecordableEnforcesLogRecordLimits()) + recordable_enforces_limits_(processor_->RecordableEnforcesLogRecordLimits()), + consumes_resolved_context_(processor_->ConsumesResolvedContext()) {} void LoggerContext::AddProcessor(std::unique_ptr processor) noexcept @@ -41,6 +42,7 @@ void LoggerContext::AddProcessor(std::unique_ptr processor) auto multi_processor = static_cast(processor_.get()); multi_processor->AddProcessor(std::move(processor)); recordable_enforces_limits_ = processor_->RecordableEnforcesLogRecordLimits(); + consumes_resolved_context_ = processor_->ConsumesResolvedContext(); } LogRecordProcessor &LoggerContext::GetProcessor() const noexcept @@ -69,6 +71,11 @@ bool LoggerContext::RecordableEnforcesLogRecordLimits() const noexcept return recordable_enforces_limits_; } +bool LoggerContext::ConsumesResolvedContext() const noexcept +{ + return consumes_resolved_context_; +} + void LoggerContext::SetLoggerConfigurator( std::unique_ptr> logger_configurator) noexcept diff --git a/sdk/src/logs/multi_log_record_processor.cc b/sdk/src/logs/multi_log_record_processor.cc index 87f1095502..1d2d34347a 100644 --- a/sdk/src/logs/multi_log_record_processor.cc +++ b/sdk/src/logs/multi_log_record_processor.cc @@ -144,6 +144,18 @@ bool MultiLogRecordProcessor::RecordableEnforcesLogRecordLimits() const noexcept return false; } +bool MultiLogRecordProcessor::ConsumesResolvedContext() const noexcept +{ + for (const auto &processor : processors_) + { + if (processor != nullptr && processor->ConsumesResolvedContext()) + { + return true; + } + } + return false; +} + bool MultiLogRecordProcessor::ForceFlush(std::chrono::microseconds timeout) noexcept { return InternalForceFlush(timeout); diff --git a/sdk/test/configuration/yaml_logs_test.cc b/sdk/test/configuration/yaml_logs_test.cc index f2d0d83e7e..3a3edd6c90 100644 --- a/sdk/test/configuration/yaml_logs_test.cc +++ b/sdk/test/configuration/yaml_logs_test.cc @@ -10,10 +10,12 @@ #include "opentelemetry/sdk/configuration/batch_log_record_processor_configuration.h" #include "opentelemetry/sdk/configuration/configuration.h" #include "opentelemetry/sdk/configuration/event_to_span_event_bridge_log_record_processor_configuration.h" +#include "opentelemetry/sdk/configuration/extension_log_record_processor_configuration.h" #include "opentelemetry/sdk/configuration/grpc_tls_configuration.h" #include "opentelemetry/sdk/configuration/headers_configuration.h" #include "opentelemetry/sdk/configuration/http_tls_configuration.h" #include "opentelemetry/sdk/configuration/log_record_limits_configuration.h" +#include "opentelemetry/sdk/configuration/log_record_processor_configuration_visitor.h" #include "opentelemetry/sdk/configuration/logger_config_configuration.h" #include "opentelemetry/sdk/configuration/logger_configurator_configuration.h" #include "opentelemetry/sdk/configuration/logger_matcher_and_config_configuration.h" @@ -101,6 +103,51 @@ file_format: "1.0-logs" ASSERT_NE(exporter, nullptr); } +namespace +{ + +// Records which LogRecordProcessorConfigurationVisitor callback a parsed model dispatches to. +// The bridge configuration carries no properties of its own, so there is no parsed field to +// assert on; dispatching through Accept() is what actually proves the YAML key selected the +// bridge configuration rather than falling through to the extension configuration. This also +// avoids needing RTTI. +class RecordingLogRecordProcessorVisitor final + : public opentelemetry::sdk::configuration::LogRecordProcessorConfigurationVisitor +{ +public: + void VisitBatch( + const opentelemetry::sdk::configuration::BatchLogRecordProcessorConfiguration *) override + { + ++batch_count; + } + + void VisitSimple( + const opentelemetry::sdk::configuration::SimpleLogRecordProcessorConfiguration *) override + { + ++simple_count; + } + + void VisitEventToSpanEventBridge( + const opentelemetry::sdk::configuration::EventToSpanEventBridgeLogRecordProcessorConfiguration + *) override + { + ++bridge_count; + } + + void VisitExtension( + const opentelemetry::sdk::configuration::ExtensionLogRecordProcessorConfiguration *) override + { + ++extension_count; + } + + int batch_count{0}; + int simple_count{0}; + int bridge_count{0}; + int extension_count{0}; +}; + +} // namespace + TEST(YamlLogs, event_to_span_event_bridge_processor) { std::string yaml = R"( @@ -116,10 +163,39 @@ file_format: "1.0-logs" ASSERT_EQ(config->logger_provider->processors.size(), 1); auto *processor = config->logger_provider->processors[0].get(); ASSERT_NE(processor, nullptr); - auto *bridge = reinterpret_cast< - opentelemetry::sdk::configuration::EventToSpanEventBridgeLogRecordProcessorConfiguration *>( - processor); - ASSERT_NE(bridge, nullptr); + + RecordingLogRecordProcessorVisitor visitor; + processor->Accept(&visitor); + + EXPECT_EQ(visitor.bridge_count, 1); + EXPECT_EQ(visitor.extension_count, 0); + EXPECT_EQ(visitor.batch_count, 0); + EXPECT_EQ(visitor.simple_count, 0); +} + +TEST(YamlLogs, event_to_span_event_bridge_processor_misspelled_key_is_not_the_bridge) +{ + // A misspelled key falls through to the extension processor configuration. Guards against the + // bridge assertion above passing for a model that is not actually the bridge. + std::string yaml = R"( +file_format: "1.0-logs" +logger_provider: + processors: + - event_to_span_event_brige/development: +)"; + + auto config = DoParse(yaml); + ASSERT_NE(config, nullptr); + ASSERT_NE(config->logger_provider, nullptr); + ASSERT_EQ(config->logger_provider->processors.size(), 1); + auto *processor = config->logger_provider->processors[0].get(); + ASSERT_NE(processor, nullptr); + + RecordingLogRecordProcessorVisitor visitor; + processor->Accept(&visitor); + + EXPECT_EQ(visitor.bridge_count, 0); + EXPECT_EQ(visitor.extension_count, 1); } TEST(YamlLogs, default_batch_processor) diff --git a/sdk/test/logs/event_to_span_event_bridge_processor_test.cc b/sdk/test/logs/event_to_span_event_bridge_processor_test.cc index 80a43ac740..c8d64426f6 100644 --- a/sdk/test/logs/event_to_span_event_bridge_processor_test.cc +++ b/sdk/test/logs/event_to_span_event_bridge_processor_test.cc @@ -7,8 +7,10 @@ #include #include +#include "opentelemetry/context/context.h" #include "opentelemetry/exporters/memory/in_memory_span_data.h" #include "opentelemetry/exporters/memory/in_memory_span_exporter.h" +#include "opentelemetry/nostd/variant.h" #include "opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h" #include "opentelemetry/sdk/logs/read_write_log_record.h" #include "opentelemetry/sdk/logs/recordable.h" @@ -18,6 +20,7 @@ #include "opentelemetry/sdk/trace/span_data.h" #include "opentelemetry/sdk/trace/tracer.h" #include "opentelemetry/sdk/trace/tracer_context.h" +#include "opentelemetry/trace/context.h" #include "opentelemetry/trace/span.h" #include "opentelemetry/trace/span_context.h" #include "opentelemetry/trace/tracer.h" @@ -131,3 +134,131 @@ TEST(EventToSpanEventBridgeProcessorTest, IgnoresEventFromDifferentSpan) EXPECT_EQ(0, exported_span->GetEvents().size()); } } + +// The case that motivated OnEmitWithContext: the record belongs to span_a, which is supplied +// explicitly, while an unrelated span_b is ambient. The event must land on span_a and only on +// span_a. Dispatching through OnEmit() (ambient only) cannot satisfy this. +TEST(EventToSpanEventBridgeProcessorTest, BridgesOntoExplicitContextSpanWhileDifferentSpanIsAmbient) +{ + std::shared_ptr span_data; + auto tracer = MakeTracer(&span_data); + + auto span_a = tracer->StartSpan("span-a"); + auto span_b = tracer->StartSpan("span-b"); + + // span_b is ambient; span_a is only reachable through the explicitly supplied context. + auto scope = tracer->WithActiveSpan(span_b); + + opentelemetry::context::Context explicit_context = + opentelemetry::context::Context().SetValue(trace_api::kSpanKey, span_a); + + EventToSpanEventBridgeProcessor processor; + auto recordable = processor.MakeRecordable(); + auto *log_record = static_cast(recordable.get()); + + trace_api::SpanContext span_a_context = span_a->GetContext(); + log_record->SetTraceId(span_a_context.trace_id()); + log_record->SetSpanId(span_a_context.span_id()); + log_record->SetEventId(0, "my.event"); + log_record->SetAttribute("key1", "value1"); + + const opentelemetry::nostd::variant + resolved_context{explicit_context}; + processor.OnEmitWithContext(std::move(recordable), resolved_context); + + span_a->End(); + span_b->End(); + + auto spans = span_data->GetSpans(); + ASSERT_EQ(2, spans.size()); + + int spans_with_event = 0; + for (auto &exported_span : spans) + { + if (exported_span->GetName() == "span-a") + { + ASSERT_EQ(1, exported_span->GetEvents().size()); + EXPECT_EQ("my.event", exported_span->GetEvents().at(0).GetName()); + ++spans_with_event; + } + else + { + EXPECT_EQ(0, exported_span->GetEvents().size()); + } + } + EXPECT_EQ(1, spans_with_event); +} + +// An explicitly supplied context whose span does not match the record's ids must not bridge. +TEST(EventToSpanEventBridgeProcessorTest, IgnoresEventWhenExplicitContextSpanDoesNotMatch) +{ + std::shared_ptr span_data; + auto tracer = MakeTracer(&span_data); + + auto span_a = tracer->StartSpan("span-a"); + auto span_b = tracer->StartSpan("span-b"); + + opentelemetry::context::Context explicit_context = + opentelemetry::context::Context().SetValue(trace_api::kSpanKey, span_b); + + EventToSpanEventBridgeProcessor processor; + auto recordable = processor.MakeRecordable(); + auto *log_record = static_cast(recordable.get()); + + // Record carries span_a's ids, but span_b is the span in the resolved context. + trace_api::SpanContext span_a_context = span_a->GetContext(); + log_record->SetTraceId(span_a_context.trace_id()); + log_record->SetSpanId(span_a_context.span_id()); + log_record->SetEventId(0, "my.event"); + + const opentelemetry::nostd::variant + resolved_context{explicit_context}; + processor.OnEmitWithContext(std::move(recordable), resolved_context); + + span_a->End(); + span_b->End(); + + auto spans = span_data->GetSpans(); + ASSERT_EQ(2, spans.size()); + for (auto &exported_span : spans) + { + EXPECT_EQ(0, exported_span->GetEvents().size()); + } +} + +// A bare SpanContext carries only ids, so the ambient span is the only writable span. It must +// still bridge when the ambient span is the one the ids refer to. +TEST(EventToSpanEventBridgeProcessorTest, BridgesOntoAmbientSpanForBareSpanContext) +{ + std::shared_ptr span_data; + auto tracer = MakeTracer(&span_data); + + auto span = tracer->StartSpan("test-span"); + auto scope = tracer->WithActiveSpan(span); + + EventToSpanEventBridgeProcessor processor; + auto recordable = processor.MakeRecordable(); + auto *log_record = static_cast(recordable.get()); + + trace_api::SpanContext span_context = span->GetContext(); + log_record->SetTraceId(span_context.trace_id()); + log_record->SetSpanId(span_context.span_id()); + log_record->SetEventId(0, "my.event"); + + const opentelemetry::nostd::variant + resolved_context{span_context}; + processor.OnEmitWithContext(std::move(recordable), resolved_context); + + span->End(); + + auto spans = span_data->GetSpans(); + ASSERT_EQ(1, spans.size()); + ASSERT_EQ(1, spans.at(0)->GetEvents().size()); + EXPECT_EQ("my.event", spans.at(0)->GetEvents().at(0).GetName()); +} + +TEST(EventToSpanEventBridgeProcessorTest, ConsumesResolvedContext) +{ + EventToSpanEventBridgeProcessor processor; + EXPECT_TRUE(processor.ConsumesResolvedContext()); +} diff --git a/sdk/test/logs/logger_sdk_test.cc b/sdk/test/logs/logger_sdk_test.cc index f85b7ef0d7..ce664f0f10 100644 --- a/sdk/test/logs/logger_sdk_test.cc +++ b/sdk/test/logs/logger_sdk_test.cc @@ -12,6 +12,8 @@ #include "opentelemetry/common/attribute_value.h" #include "opentelemetry/common/timestamp.h" +#include "opentelemetry/context/context.h" +#include "opentelemetry/context/runtime_context.h" #include "opentelemetry/logs/event_id.h" #include "opentelemetry/logs/event_logger.h" // IWYU pragma: keep #include "opentelemetry/logs/event_logger_provider.h" // IWYU pragma: keep @@ -986,4 +988,135 @@ TEST(LoggerSDK, EventLog) } #endif +// Records which emit entry point the Logger dispatched through, and whether the processor +// declares that it reads the resolved context. +struct EmitDispatchState +{ + size_t on_emit_calls = 0; + size_t on_emit_with_context_calls = 0; +}; + +class DispatchRecordingProcessor final : public LogRecordProcessor +{ +public: + DispatchRecordingProcessor(bool consumes_resolved_context, + std::shared_ptr state) noexcept + : consumes_resolved_context_(consumes_resolved_context), state_(std::move(state)) + {} + + std::unique_ptr MakeRecordable() noexcept override + { + return std::unique_ptr(new MockLogRecordable()); + } + + void OnEmit(std::unique_ptr && /* record */) noexcept override + { + ++state_->on_emit_calls; + } + + void OnEmitWithContext( + std::unique_ptr &&record, + const nostd::variant + & /* context */) noexcept override + { + ++state_->on_emit_with_context_calls; + // Deliberately does not chain to OnEmit(), so the two counters stay independent. + static_cast(std::move(record)); + } + + bool ConsumesResolvedContext() const noexcept override { return consumes_resolved_context_; } + + bool ForceFlush(std::chrono::microseconds /* timeout */) noexcept override { return true; } + bool Shutdown(std::chrono::microseconds /* timeout */) noexcept override { return true; } + +private: + bool consumes_resolved_context_; + std::shared_ptr state_; +}; + +// A RuntimeContextStorage that counts GetCurrent() calls, so the test can assert that a +// context-ignoring pipeline adds no context lookups on the emit path. +class CountingRuntimeContextStorage final : public opentelemetry::context::RuntimeContextStorage +{ +public: + opentelemetry::context::Context GetCurrent() noexcept override + { + ++get_current_calls; + return context_; + } + + bool Detach(opentelemetry::context::Token & /* token */) noexcept override { return true; } + + nostd::unique_ptr Attach( + const opentelemetry::context::Context &context) noexcept override + { + context_ = context; + return CreateToken(context); + } + + size_t get_current_calls = 0; + +private: + opentelemetry::context::Context context_; +}; + +std::shared_ptr MakeProviderWithProcessor( + std::unique_ptr processor) +{ + return std::shared_ptr(new LoggerProvider(std::move(processor))); +} + +TEST(LoggerSDK, EmitDispatchesThroughOnEmitWhenProcessorIgnoresResolvedContext) +{ + auto state = std::make_shared(); + auto lp = MakeProviderWithProcessor(std::unique_ptr( + new DispatchRecordingProcessor(/* consumes_resolved_context */ false, state))); + auto logger = lp->GetLogger("logger", "opentelemetry_library"); + + logger->EmitLogRecord(logger->CreateLogRecord()); + + EXPECT_EQ(state->on_emit_calls, 1u); + EXPECT_EQ(state->on_emit_with_context_calls, 0u); +} + +TEST(LoggerSDK, EmitDispatchesThroughOnEmitWithContextWhenProcessorConsumesResolvedContext) +{ + auto state = std::make_shared(); + auto lp = MakeProviderWithProcessor(std::unique_ptr( + new DispatchRecordingProcessor(/* consumes_resolved_context */ true, state))); + auto logger = lp->GetLogger("logger", "opentelemetry_library"); + + logger->EmitLogRecord(logger->CreateLogRecord()); + + EXPECT_EQ(state->on_emit_with_context_calls, 1u); + EXPECT_EQ(state->on_emit_calls, 0u); +} + +// A processor that ignores the resolved context must not cause the emit path to resolve the +// ambient context, so that adding OnEmitWithContext() costs nothing for existing pipelines. +TEST(LoggerSDK, EmitDoesNotResolveContextWhenProcessorIgnoresResolvedContext) +{ + auto *counting_storage = new CountingRuntimeContextStorage(); + opentelemetry::context::RuntimeContext::SetRuntimeContextStorage( + nostd::shared_ptr(counting_storage)); + + auto state = std::make_shared(); + auto lp = MakeProviderWithProcessor(std::unique_ptr( + new DispatchRecordingProcessor(/* consumes_resolved_context */ false, state))); + auto logger = lp->GetLogger("logger", "opentelemetry_library"); + + auto record = logger->CreateLogRecord(); + + const size_t calls_after_create = counting_storage->get_current_calls; + logger->EmitLogRecord(std::move(record)); + + EXPECT_EQ(state->on_emit_calls, 1u); + // EmitLogRecord() itself must not add a context lookup. + EXPECT_EQ(counting_storage->get_current_calls, calls_after_create); + + opentelemetry::context::RuntimeContext::SetRuntimeContextStorage( + nostd::shared_ptr( + new CountingRuntimeContextStorage())); +} + } // namespace From af2e49c86c6b591cd165bda5c3d39461e5094f9e Mon Sep 17 00:00:00 2001 From: om7057 Date: Thu, 30 Jul 2026 08:12:14 +0530 Subject: [PATCH 10/21] Fix Doxygen lint: name the OnEmitWithContext context parameter The base declaration left the second parameter unnamed while the comment documented @param context, so Doxygen warned that the documented argument was not in the argument list -- once for LogRecordProcessor and again for the EventToSpanEventBridgeProcessor override that inherits the comment. Name the parameter and mark it OPENTELEMETRY_MAYBE_UNUSED, since the default implementation forwards to OnEmit() without reading it. Keeps the parameter documented rather than dropping the @param line. Verified with doxygen against docs/public/Doxyfile.lint: no warnings remain for any file this PR touches. The warnings still reported for api logs logger.h, noop.h and sdk resource.h are pre-existing on main, are not emitted by the Doxygen version CI uses, and are untouched here. --- sdk/include/opentelemetry/sdk/logs/processor.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/logs/processor.h b/sdk/include/opentelemetry/sdk/logs/processor.h index 99688c3157..49bf143dee 100644 --- a/sdk/include/opentelemetry/sdk/logs/processor.h +++ b/sdk/include/opentelemetry/sdk/logs/processor.h @@ -6,6 +6,7 @@ #include #include +#include "opentelemetry/common/macros.h" #include "opentelemetry/context/context.h" #include "opentelemetry/logs/severity.h" #include "opentelemetry/nostd/string_view.h" @@ -83,10 +84,10 @@ class LogRecordProcessor * @param record the log recordable object * @param context the resolved context (SpanContext or Context variant) */ - virtual void OnEmitWithContext( - std::unique_ptr &&record, - const opentelemetry::nostd::variant & /*context*/) noexcept + virtual void OnEmitWithContext(std::unique_ptr &&record, + OPENTELEMETRY_MAYBE_UNUSED const opentelemetry::nostd::variant< + opentelemetry::trace::SpanContext, + opentelemetry::context::Context> &context) noexcept { OnEmit(std::move(record)); } From 01009bbc013952a090df224de43e9de5fb4f54d2 Mon Sep 17 00:00:00 2001 From: om7057 Date: Fri, 31 Jul 2026 02:19:22 +0530 Subject: [PATCH 11/21] Fix iwyu findings from the review-response commit - event_to_span_event_bridge_processor.cc: drop , no longer needed directly now that the shared BridgeRecordToSpan() helper pulls unique_ptr transitively through recordable.h. - event_to_span_event_bridge_processor_test.cc: swap trace/context.h for nostd/string_view.h + trace/span_metadata.h; the new regression tests use trace_api::kSpanKey directly and never call GetSpan()/SetSpan(). - logger_sdk_test.cc: add stddef.h (size_t) and nostd/unique_ptr.h (CountingRuntimeContextStorage::Attach's return type), both used directly by the new dispatch tests. - yaml_logs_test.cc: swap the two concrete {event_to_span_event_bridge,extension}_log_record_processor_configuration.h includes for the base log_record_processor_configuration.h; the parsed processor pointer is typed as the base class and only Accept() is called on it, the concrete types are only named as visitor-callback parameters, which the visitor header already forward-declares. Verified with 'iwyu' locally is not available in this environment, so confirmed by inspection against the CI diagnostics and by rebuilding: full build is clean and ctest is 931/931 passing. doxygen docs/public/Doxyfile.lint still reports zero warnings for any file this PR touches. --- sdk/src/logs/event_to_span_event_bridge_processor.cc | 1 - sdk/test/configuration/yaml_logs_test.cc | 3 +-- sdk/test/logs/event_to_span_event_bridge_processor_test.cc | 3 ++- sdk/test/logs/logger_sdk_test.cc | 2 ++ 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/sdk/src/logs/event_to_span_event_bridge_processor.cc b/sdk/src/logs/event_to_span_event_bridge_processor.cc index def44b4d28..445e7e3362 100644 --- a/sdk/src/logs/event_to_span_event_bridge_processor.cc +++ b/sdk/src/logs/event_to_span_event_bridge_processor.cc @@ -4,7 +4,6 @@ #include #include #include -#include #include #include #include diff --git a/sdk/test/configuration/yaml_logs_test.cc b/sdk/test/configuration/yaml_logs_test.cc index 3a3edd6c90..d7cc3038c9 100644 --- a/sdk/test/configuration/yaml_logs_test.cc +++ b/sdk/test/configuration/yaml_logs_test.cc @@ -9,12 +9,11 @@ #include "opentelemetry/sdk/configuration/batch_log_record_processor_configuration.h" #include "opentelemetry/sdk/configuration/configuration.h" -#include "opentelemetry/sdk/configuration/event_to_span_event_bridge_log_record_processor_configuration.h" -#include "opentelemetry/sdk/configuration/extension_log_record_processor_configuration.h" #include "opentelemetry/sdk/configuration/grpc_tls_configuration.h" #include "opentelemetry/sdk/configuration/headers_configuration.h" #include "opentelemetry/sdk/configuration/http_tls_configuration.h" #include "opentelemetry/sdk/configuration/log_record_limits_configuration.h" +#include "opentelemetry/sdk/configuration/log_record_processor_configuration.h" #include "opentelemetry/sdk/configuration/log_record_processor_configuration_visitor.h" #include "opentelemetry/sdk/configuration/logger_config_configuration.h" #include "opentelemetry/sdk/configuration/logger_configurator_configuration.h" diff --git a/sdk/test/logs/event_to_span_event_bridge_processor_test.cc b/sdk/test/logs/event_to_span_event_bridge_processor_test.cc index c8d64426f6..7cfb21dcd8 100644 --- a/sdk/test/logs/event_to_span_event_bridge_processor_test.cc +++ b/sdk/test/logs/event_to_span_event_bridge_processor_test.cc @@ -10,6 +10,7 @@ #include "opentelemetry/context/context.h" #include "opentelemetry/exporters/memory/in_memory_span_data.h" #include "opentelemetry/exporters/memory/in_memory_span_exporter.h" +#include "opentelemetry/nostd/string_view.h" #include "opentelemetry/nostd/variant.h" #include "opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h" #include "opentelemetry/sdk/logs/read_write_log_record.h" @@ -20,9 +21,9 @@ #include "opentelemetry/sdk/trace/span_data.h" #include "opentelemetry/sdk/trace/tracer.h" #include "opentelemetry/sdk/trace/tracer_context.h" -#include "opentelemetry/trace/context.h" #include "opentelemetry/trace/span.h" #include "opentelemetry/trace/span_context.h" +#include "opentelemetry/trace/span_metadata.h" #include "opentelemetry/trace/tracer.h" using namespace opentelemetry::sdk::logs; diff --git a/sdk/test/logs/logger_sdk_test.cc b/sdk/test/logs/logger_sdk_test.cc index ce664f0f10..31104a3b24 100644 --- a/sdk/test/logs/logger_sdk_test.cc +++ b/sdk/test/logs/logger_sdk_test.cc @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 #include +#include #include #include #include @@ -24,6 +25,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 7cccee799aacaad6d03a99a900282515bc8f8113 Mon Sep 17 00:00:00 2001 From: om7057 Date: Fri, 31 Jul 2026 09:26:34 +0530 Subject: [PATCH 12/21] Fix clang-tidy and iwyu findings on the new processor clang-tidy (both abiv1-preview and abiv2-preview pushed the warning count over the job's threshold by 2): - event_to_span_event_bridge_processor.cc:237 bugprone-exception-escape: OnEmitWithContext() is noexcept but used holds_alternative() + get(context), and nostd::get() throws bad_variant_access on a mismatched alternative. Even though the holds_alternative check made that path unreachable, clang-tidy can't prove it statically. Switched to nostd::get_if(&context), which returns nullptr instead of throwing -- the same pattern Logger::ExtractSpanContext already uses for this exact variant in logger.cc. - logger_sdk_test.cc:1014 cppcoreguidelines-rvalue-reference-param-not-moved: DispatchRecordingProcessor::OnEmit() took its unique_ptr&& by dropping it unnamed. Named the parameter and moved it into a static_cast(...), mirroring the sibling OnEmitWithContext() override a few lines below that already does this. iwyu (abiv2-preview only; abiv1/abiv1-preview already required these directly and passed): - logger_sdk_test.cc: marked the and opentelemetry/context/ --- sdk/src/logs/event_to_span_event_bridge_processor.cc | 5 ++--- sdk/test/logs/logger_sdk_test.cc | 8 +++++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/sdk/src/logs/event_to_span_event_bridge_processor.cc b/sdk/src/logs/event_to_span_event_bridge_processor.cc index 445e7e3362..1da173085c 100644 --- a/sdk/src/logs/event_to_span_event_bridge_processor.cc +++ b/sdk/src/logs/event_to_span_event_bridge_processor.cc @@ -242,13 +242,12 @@ void EventToSpanEventBridgeProcessor::OnEmitWithContext( const std::unique_ptr log_record( static_cast(std::move(record).release())); - if (nostd::holds_alternative(context)) + if (const auto *resolved_context = nostd::get_if(&context)) { // The resolved context carries the live Span the record belongs to. Add the event to that // span directly, which is what makes an explicitly supplied context work even when a // different span is ambient. - BridgeRecordToSpan(log_record.get(), opentelemetry::trace::GetSpan( - nostd::get(context))); + BridgeRecordToSpan(log_record.get(), opentelemetry::trace::GetSpan(*resolved_context)); return; } diff --git a/sdk/test/logs/logger_sdk_test.cc b/sdk/test/logs/logger_sdk_test.cc index 31104a3b24..9512a5bcb1 100644 --- a/sdk/test/logs/logger_sdk_test.cc +++ b/sdk/test/logs/logger_sdk_test.cc @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 #include -#include +#include // IWYU pragma: keep #include #include #include @@ -13,7 +13,7 @@ #include "opentelemetry/common/attribute_value.h" #include "opentelemetry/common/timestamp.h" -#include "opentelemetry/context/context.h" +#include "opentelemetry/context/context.h" // IWYU pragma: keep #include "opentelemetry/context/runtime_context.h" #include "opentelemetry/logs/event_id.h" #include "opentelemetry/logs/event_logger.h" // IWYU pragma: keep @@ -1011,9 +1011,11 @@ class DispatchRecordingProcessor final : public LogRecordProcessor return std::unique_ptr(new MockLogRecordable()); } - void OnEmit(std::unique_ptr && /* record */) noexcept override + void OnEmit(std::unique_ptr &&record) noexcept override { ++state_->on_emit_calls; + // Deliberately does not chain to OnEmitWithContext(), so the two counters stay independent. + static_cast(std::move(record)); } void OnEmitWithContext( From 8f94856fa45d28346215cf73c147a2f8dbaebba7 Mon Sep 17 00:00:00 2001 From: om7057 Date: Sun, 2 Aug 2026 08:59:58 +0530 Subject: [PATCH 13/21] Trigger CI re-run CI / CMake gcc 14 (maintainer mode, sync) failed with a glibc heap-corruption abort (double free or corruption (fasttop)) inside the functional/otlp mTLS integration test binary during the mtls-ok test case. Both full ctest runs in that job completed 100% passing (1332/1332) beforehand; the crash is in a separate OTLP gRPC exporter mTLS test harness untouched by this PR's diff. Not a Required check. Retriggering to see if it clears as a transient flake. From 1f1ced6cc36792d40204d8951e91d9ebb6d182ee Mon Sep 17 00:00:00 2001 From: om7057 Date: Sun, 2 Aug 2026 16:43:09 +0530 Subject: [PATCH 14/21] Trigger CI re-run Two unrelated flakes seen across recent runs: - CI / CMake gcc 14 (maintainer mode, sync): glibc heap-corruption abort in the functional/otlp mTLS integration test (mtls-ok case), after both ctest suites in that job passed 100%. Not touched by this PR's diff. - CI / CMake FetchContent usage with opentelemetry-cpp: BasicCurlHttpTests. ElegantQuitQuick failed a hardcoded 20ms shutdown-latency assertion (actual 247ms) in ext/test/http/curl_http_test.cc, whose own comment already documents this margin as CI-host-load-sensitive. Not touched by this PR's diff. Retriggering to see if both clear as transient CI-host flakes. From dd1cb0e5d84ff53cd0f1318f4830800517e3c5f7 Mon Sep 17 00:00:00 2001 From: om7057 Date: Sun, 2 Aug 2026 20:56:17 +0530 Subject: [PATCH 15/21] Trigger CI re-run CI / CMake gcc 14 (maintainer mode, async) failed after both ctest suites passed 100% (1334/1334). The failure is in functional/otlp/run_test.sh's docker build step, timing out while pulling otel/opentelemetry-collector:0.123.0 from Docker Hub: dial tcp 54.164.79.128:443: i/o timeout Network/registry timeout on the runner, unrelated to this PR's diff (no functional/otlp or Dockerfile changes here). Retriggering. From ac1b1c548c4dfd178f9a9f4f6f9e5954508957a6 Mon Sep 17 00:00:00 2001 From: om7057 Date: Fri, 7 Aug 2026 08:54:49 +0530 Subject: [PATCH 16/21] Retrigger CI (Windows jobs hit a vcpkg benchmark build flake then got stuck/cancelled) From 7696c6b107d28bd16cbc997c33dffebb76ecfa2f Mon Sep 17 00:00:00 2001 From: om7057 Date: Tue, 11 Aug 2026 20:08:25 +0530 Subject: [PATCH 17/21] Carry the resolved context through Logger::EmitLogRecord() end-to-end Fixes the gap lalitb flagged: EmitLogRecord() was re-deriving the resolved context from RuntimeContext::GetCurrent() (ambient) instead of using the one EmitLogRecord(args...) had already resolved (explicit if supplied). If a log used an explicit context for span_a while span_b was ambient, the processor was handed span_b's context and the event bridge silently dropped it. api/include/opentelemetry/logs/logger.h (ABI v2): - Added Logger::EmitLogRecordWithContext(record, resolved_context), a new virtual (distinct name, not an EmitLogRecord() overload, so it can't be hidden by a derived class that only overrides the existing overloads -- same reasoning already applied to LogRecordProcessor::OnEmitWithContext). Default implementation forwards to EmitLogRecord(record), so existing Logger implementations keep compiling and behaving as before. - Extracted the per-argument LogRecordSetterTrait loop, previously inlined in EmitLogRecord(record, args...), into a private StampLogRecordFields() helper shared by both call sites. - EmitLogRecord(args...)'s ABI v2 branch now stamps the record via that helper and calls EmitLogRecordWithContext(record, context_or_span) with the context_or_span it just resolved, instead of forwarding to EmitLogRecord(record, args...) and losing context_or_span at the end of the function. sdk/include+src/logs/logger.h/.cc: - Logger overrides EmitLogRecordWithContext() (ABI v2) and passes the supplied resolved_context straight to the processor, no RuntimeContext lookup involved. - EmitLogRecord(record) (the ABI-version-agnostic override, used when no context was ever resolved) keeps the existing ambient-context fallback. - Both now share EmitToProcessor(recordable, resolved_context), which keeps the existing gate on LoggerContext::ConsumesResolvedContext(): a processor that ignores context still costs nothing on the emit path. sdk/test/logs/logger_sdk_test.cc: added ContextCapturingProcessor and LoggerEmitWithExplicitTraceTest.ContextAwareProcessorReceivesExplicitContextNotAmbientSpan, reproducing lalitb's exact scenario (explicit span_a, ambient span_b) and asserting the processor receives span_a's context. Confirmed this fails against the pre-fix code (received span_b instead). Verified in a from-scratch ABI v2 build (this repo's only local build directories were ABI v1, which compiles out everything gated behind OPENTELEMETRY_ABI_VERSION_NO >= 2 and would have silently skipped this code and the new test): logger_sdk_test, event_to_span_event_bridge_processor_test, and sdk_builder_test all pass. Also verified the existing ABI v1 build (build_qualify) still builds clean and passes its full 1353-test ctest suite. doxygen docs/public/Doxyfile.lint reports no new warnings. --- api/include/opentelemetry/logs/logger.h | 81 ++++++++++++++++----- sdk/include/opentelemetry/sdk/logs/logger.h | 25 +++++-- sdk/src/logs/logger.cc | 50 +++++++++++-- sdk/test/logs/logger_sdk_test.cc | 75 +++++++++++++++++++ 4 files changed, 202 insertions(+), 29 deletions(-) diff --git a/api/include/opentelemetry/logs/logger.h b/api/include/opentelemetry/logs/logger.h index cc48be1a72..625fa9d3e1 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -80,6 +80,30 @@ class Logger */ virtual void EmitLogRecord(nostd::unique_ptr &&log_record) noexcept = 0; +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + /** + * Emit a Log Record object together with the context it was resolved against: the explicitly + * supplied context when @c EmitLogRecord(args...) below was given one, otherwise the ambient + * context captured at that time. + * + * The default implementation ignores @p resolved_context and forwards to + * @c EmitLogRecord(std::move(log_record)), so existing @c Logger implementations that only + * override the record-only overload keep compiling and behaving exactly as before. Only a + * @c Logger that needs the resolved context (for example, to hand it to a + * context-aware @c LogRecordProcessor) needs to override this as well. + * + * @param log_record Log record + * @param resolved_context the resolved context (SpanContext or Context variant) + */ + virtual void EmitLogRecordWithContext( + nostd::unique_ptr &&log_record, + const nostd::variant + & /*resolved_context*/) noexcept + { + EmitLogRecord(std::move(log_record)); + } +#endif // OPENTELEMETRY_ABI_VERSION_NO >= 2 + /** * Emit a Log Record object with arguments. * @@ -115,23 +139,7 @@ class Logger return; } - // - // Keep the parameter pack unpacking order from left to right because left - // ones are usually more important like severity and event_id than the - // attributes. The left to right unpack order could pass the more important - // data to processors to avoid caching and memory allocating. - // -#if __cplusplus <= 201402L - // C++14 does not support fold expressions for parameter pack expansion. - int dummy[] = {(detail::LogRecordSetterTrait::type>::Set( - log_record.get(), std::forward(args)), - 0)...}; - IgnoreTraitResult(dummy); -#else - IgnoreTraitResult((detail::LogRecordSetterTrait::type>::Set( - log_record.get(), std::forward(args)), - ...)); -#endif + StampLogRecordFields(log_record.get(), std::forward(args)...); EmitLogRecord(std::move(log_record)); } @@ -215,11 +223,21 @@ class Logger } nostd::unique_ptr log_record = CreateLogRecord(context_or_span); + if (!log_record) + { + return; + } + + StampLogRecordFields(log_record.get(), std::forward(args)...); + + // Carries the already-resolved context_or_span through, rather than letting the record-only + // EmitLogRecord() overload re-derive it (which would silently fall back to the ambient + // context and lose an explicitly supplied one). + EmitLogRecordWithContext(std::move(log_record), context_or_span); #else nostd::unique_ptr log_record = CreateLogRecord(); -#endif // OPENTELEMETRY_ABI_VERSION_NO >= 2 - EmitLogRecord(std::move(log_record), std::forward(args)...); +#endif // OPENTELEMETRY_ABI_VERSION_NO >= 2 } /** @@ -643,6 +661,31 @@ class Logger void IgnoreTraitResult(ValueType &&...) {} + // Applies each of args to log_record via LogRecordSetterTrait, left to right. Shared by every + // EmitLogRecord(...) overload that stamps fields from a caller-supplied argument pack, so the + // stamping logic has one definition regardless of which overload resolved the record. + template + void StampLogRecordFields(LogRecord *log_record, ArgumentType &&...args) + { + // + // Keep the parameter pack unpacking order from left to right because left + // ones are usually more important like severity and event_id than the + // attributes. The left to right unpack order could pass the more important + // data to processors to avoid caching and memory allocating. + // +#if __cplusplus <= 201402L + // C++14 does not support fold expressions for parameter pack expansion. + int dummy[] = {(detail::LogRecordSetterTrait::type>::Set( + log_record, std::forward(args)), + 0)...}; + IgnoreTraitResult(dummy); +#else + IgnoreTraitResult((detail::LogRecordSetterTrait::type>::Set( + log_record, std::forward(args)), + ...)); +#endif + } + // // minimum_severity_ can be updated concurrently by multiple threads/cores, so race condition on // read/write should be handled. And std::atomic can not be used here because it is not ABI diff --git a/sdk/include/opentelemetry/sdk/logs/logger.h b/sdk/include/opentelemetry/sdk/logs/logger.h index 44ed4ec1ee..547d142421 100644 --- a/sdk/include/opentelemetry/sdk/logs/logger.h +++ b/sdk/include/opentelemetry/sdk/logs/logger.h @@ -14,20 +14,18 @@ #include "opentelemetry/logs/noop.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/logs/logger_context.h" +#include "opentelemetry/trace/span_context.h" #include "opentelemetry/version.h" -#if OPENTELEMETRY_ABI_VERSION_NO >= 2 -# include "opentelemetry/nostd/variant.h" -# include "opentelemetry/trace/span_context.h" -#endif // OPENTELEMETRY_ABI_VERSION_NO >= 2 - OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { namespace logs { +class Recordable; class Logger final : public opentelemetry::logs::Logger { @@ -62,6 +60,13 @@ class Logger final : public opentelemetry::logs::Logger void EmitLogRecord( nostd::unique_ptr &&log_record) noexcept override; +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + void EmitLogRecordWithContext( + nostd::unique_ptr &&log_record, + const nostd::variant + &resolved_context) noexcept override; +#endif // OPENTELEMETRY_ABI_VERSION_NO >= 2 + /** Returns the associated instrumentation scope */ const opentelemetry::sdk::instrumentationscope::InstrumentationScope &GetInstrumentationScope() const noexcept; @@ -103,6 +108,16 @@ class Logger final : public opentelemetry::logs::Logger */ void UpdateLoggerConfig(LoggerConfig config) noexcept; + /** + * Shared tail of EmitLogRecord()/EmitLogRecordWithContext(): dispatches recordable to the + * configured processor, resolving the ambient context only if resolved_context is nullptr + * and some processor actually needs it (per LoggerContext::ConsumesResolvedContext()). + */ + void EmitToProcessor( + std::unique_ptr &&recordable, + const nostd::variant + *resolved_context) noexcept; + // The name of this logger std::string logger_name_; diff --git a/sdk/src/logs/logger.cc b/sdk/src/logs/logger.cc index 019972ee22..35215497c7 100644 --- a/sdk/src/logs/logger.cc +++ b/sdk/src/logs/logger.cc @@ -195,16 +195,56 @@ void Logger::EmitLogRecord( recordable->SetResource(context_->GetResource()); recordable->SetInstrumentationScope(GetInstrumentationScope()); + // No resolved context was ever produced for this record (it did not go through + // EmitLogRecord(args...)/EmitLogRecordWithContext()), so the ambient context -- resolved + // below only if a processor actually needs it -- is the best available approximation. + EmitToProcessor(std::move(recordable), nullptr); +} + +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 +void Logger::EmitLogRecordWithContext( + opentelemetry::nostd::unique_ptr &&log_record, + const nostd::variant &resolved_context) noexcept +{ + if (!logger_enabled_.load(std::memory_order_relaxed)) + { + return kNoopLogger.EmitLogRecordWithContext(std::move(log_record), resolved_context); + } + + if (!log_record) + { + return; + } + + std::unique_ptr recordable = + std::unique_ptr(static_cast(log_record.release())); + recordable->SetResource(context_->GetResource()); + recordable->SetInstrumentationScope(GetInstrumentationScope()); + + EmitToProcessor(std::move(recordable), &resolved_context); +} +#endif // OPENTELEMETRY_ABI_VERSION_NO >= 2 + +void Logger::EmitToProcessor( + std::unique_ptr &&recordable, + const nostd::variant *resolved_context) noexcept +{ auto &processor = context_->GetProcessor(); - // Only resolve the context when a processor will actually read it: resolving means a - // RuntimeContext::GetCurrent() call, which would otherwise be pure overhead on every log for - // the common pipelines (simple/batch) that ignore the context entirely. + // Only resolve/forward a context when a processor will actually read it: resolving the + // ambient context means a RuntimeContext::GetCurrent() call, which would otherwise be pure + // overhead on every log for the common pipelines (simple/batch) that ignore it entirely. if (context_->ConsumesResolvedContext()) { - const nostd::variant resolved_context{ + if (resolved_context != nullptr) + { + processor.OnEmitWithContext(std::move(recordable), *resolved_context); + return; + } + + const nostd::variant ambient_context{ context::RuntimeContext::GetCurrent()}; - processor.OnEmitWithContext(std::move(recordable), resolved_context); + processor.OnEmitWithContext(std::move(recordable), ambient_context); return; } diff --git a/sdk/test/logs/logger_sdk_test.cc b/sdk/test/logs/logger_sdk_test.cc index 064a5fe232..c06640d1e6 100644 --- a/sdk/test/logs/logger_sdk_test.cc +++ b/sdk/test/logs/logger_sdk_test.cc @@ -675,6 +675,52 @@ TEST(LoggerSDK, LoggerTraceBasedConfigAllowsSampledExplicitContextWithNamedEvent EXPECT_EQ(call_state->call_count, 1U); } +// Captures the resolved context a context-aware processor was actually handed, so a test can +// assert it is the explicitly supplied one rather than whatever happens to be ambient. +class ContextCapturingProcessor final : public LogRecordProcessor +{ +public: + std::unique_ptr MakeRecordable() noexcept override + { + return std::unique_ptr(new MockLogRecordable()); + } + + void OnEmit(std::unique_ptr &&record) noexcept override + { + static_cast(std::move(record)); + ++on_emit_calls; + } + + void OnEmitWithContext( + std::unique_ptr &&record, + const nostd::variant + &context) noexcept override + { + static_cast(std::move(record)); + ++on_emit_with_context_calls; + + if (const auto *sc = nostd::get_if(&context)) + { + received_span_context = *sc; + return; + } + if (const auto *ctx = nostd::get_if(&context)) + { + received_span_context = opentelemetry::trace::GetSpanContext(*ctx); + } + } + + bool ConsumesResolvedContext() const noexcept override { return true; } + + bool ForceFlush(std::chrono::microseconds /* timeout */) noexcept override { return true; } + bool Shutdown(std::chrono::microseconds /* timeout */) noexcept override { return true; } + + size_t on_emit_calls = 0; + size_t on_emit_with_context_calls = 0; + opentelemetry::trace::SpanContext received_span_context = + opentelemetry::trace::SpanContext::GetInvalid(); +}; + class LoggerEmitWithExplicitTraceTest : public ::testing::Test { protected: @@ -689,6 +735,10 @@ class LoggerEmitWithExplicitTraceTest : public ::testing::Test sdk_lp->AddProcessor( std::unique_ptr(new MockProcessor(shared_recordable_))); + auto *context_processor = new ContextCapturingProcessor(); + context_processor_ = context_processor; + sdk_lp->AddProcessor(std::unique_ptr(context_processor)); + { std::vector> sp; auto tp = opentelemetry::sdk::trace::TracerProviderFactory::Create(std::move(sp)); @@ -700,6 +750,7 @@ class LoggerEmitWithExplicitTraceTest : public ::testing::Test std::shared_ptr api_lp_; nostd::shared_ptr logger_; std::shared_ptr shared_recordable_; + ContextCapturingProcessor *context_processor_ = nullptr; nostd::shared_ptr runtime_span_; std::unique_ptr runtime_scope_; }; @@ -744,6 +795,30 @@ TEST_F(LoggerEmitWithExplicitTraceTest, ExplicitTracePartsStampsTraceFields) EXPECT_EQ(shared_recordable_->GetTraceFlags(), explicit_span_ctx.trace_flags()); EXPECT_NE(shared_recordable_->GetTraceId(), runtime_span_->GetContext().trace_id()); } + +// The end-to-end case lalitb flagged on review: a log is emitted with an explicit context for +// span_a while span_b (runtime_span_, from the fixture's Scope) is ambient. A processor that +// declares ConsumesResolvedContext() must receive span_a's context, not span_b's -- if the +// resolved context were re-derived from RuntimeContext::GetCurrent() instead of carried through +// from EmitLogRecord(args...), it would receive span_b here and silently misattribute the log. +TEST_F(LoggerEmitWithExplicitTraceTest, ContextAwareProcessorReceivesExplicitContextNotAmbientSpan) +{ + auto explicit_span = MakeTestSpan(/*sampled=*/true); + context::Context explicit_context; + explicit_context = opentelemetry::trace::SetSpan(explicit_context, explicit_span); + const auto explicit_span_ctx = explicit_span->GetContext(); + + ASSERT_NE(explicit_span_ctx.trace_id(), runtime_span_->GetContext().trace_id()); + + logger_->EmitLogRecord(logs_api::Severity::kInfo, nostd::string_view{"msg"}, explicit_context); + + ASSERT_EQ(context_processor_->on_emit_with_context_calls, 1u); + EXPECT_EQ(context_processor_->on_emit_calls, 0u); + EXPECT_EQ(context_processor_->received_span_context.trace_id(), explicit_span_ctx.trace_id()); + EXPECT_EQ(context_processor_->received_span_context.span_id(), explicit_span_ctx.span_id()); + EXPECT_NE(context_processor_->received_span_context.trace_id(), + runtime_span_->GetContext().trace_id()); +} #endif // OPENTELEMETRY_ABI_VERSION_NO >= 2 static std::unique_ptr create_mock_log_recordable( From 2f8c5c21abc55fad3bd65a26443da20914309217 Mon Sep 17 00:00:00 2001 From: om7057 Date: Wed, 19 Aug 2026 23:04:28 +0530 Subject: [PATCH 18/21] Reduce scope to just the configuration model and YAML parser Per @dbarker's review: the bridge processor needs new SDK architecture (a read-write recordable, a composable pipeline processor interface, and a root processor supporting it) to be spec compliant and avoid the fan-out duplication/data-loss problem of the current LogRecordProcessor interface. That work is tracked separately in https://github.com/open-telemetry/opentelemetry-cpp/issues/4454. This PR now only parses and models the event_to_span_event_bridge/development processor configuration. SdkBuilder logs a warning and skips it rather than building a processor, with a FIXME-SDK comment pointing at the tracking issue. The Logger API/SDK resolved-context plumbing (EmitLogRecordWithContext, OnEmitWithContext) is reverted, since it existed only to serve the now-removed processor implementation. --- CHANGELOG.md | 28 +- api/include/opentelemetry/logs/logger.h | 81 ++---- .../event_to_span_event_bridge_processor.h | 70 ----- ...t_to_span_event_bridge_processor_factory.h | 31 -- sdk/include/opentelemetry/sdk/logs/logger.h | 25 +- .../opentelemetry/sdk/logs/logger_context.h | 14 - .../sdk/logs/multi_log_record_processor.h | 10 - .../opentelemetry/sdk/logs/processor.h | 42 --- sdk/src/configuration/sdk_builder.cc | 13 +- sdk/src/logs/CMakeLists.txt | 2 - .../event_to_span_event_bridge_processor.cc | 272 ------------------ ..._to_span_event_bridge_processor_factory.cc | 25 -- sdk/src/logs/logger.cc | 51 ---- sdk/src/logs/logger_context.cc | 9 +- sdk/src/logs/multi_log_record_processor.cc | 34 --- sdk/test/configuration/sdk_builder_test.cc | 28 ++ sdk/test/logs/BUILD | 18 -- sdk/test/logs/CMakeLists.txt | 15 - ...ent_to_span_event_bridge_processor_test.cc | 265 ----------------- sdk/test/logs/logger_sdk_test.cc | 212 -------------- 20 files changed, 79 insertions(+), 1166 deletions(-) delete mode 100644 sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h delete mode 100644 sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor_factory.h delete mode 100644 sdk/src/logs/event_to_span_event_bridge_processor.cc delete mode 100644 sdk/src/logs/event_to_span_event_bridge_processor_factory.cc delete mode 100644 sdk/test/logs/event_to_span_event_bridge_processor_test.cc diff --git a/CHANGELOG.md b/CHANGELOG.md index 6be60bc66c..83d87c37ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,8 +15,13 @@ Increment the: ## [Unreleased] -* [CONFIGURATION] Implement EventToSpanEventBridge log record processor for - declarative configuration +* [CONFIGURATION] Parse the `event_to_span_event_bridge/development` log + record processor configuration in declarative configuration. The SDK + implementation of this processor is not yet available; `SdkBuilder` logs a + warning and skips it when configured. See + [#4454](https://github.com/open-telemetry/opentelemetry-cpp/issues/4454) + for the SDK implementation, which is blocked on new pipeline processor + architecture. [#4309](https://github.com/open-telemetry/opentelemetry-cpp/pull/4309) * [CONFIGURATION] Build the configured resource detectors in SdkBuilder, apply the `detection.attributes` include/exclude filter to the detected attributes, @@ -275,18 +280,15 @@ Breaking changes: * The public configuration member `max_buckets_` was renamed to `max_size_` to match the configuration schema. Please adjust SDK configuration accordingly. -* [LOGS SDK] `LogRecordProcessor` gained the virtual methods - `OnEmitWithContext()` and `ConsumesResolvedContext()` +* [CONFIGURATION] `LogRecordProcessorConfigurationVisitor` gained the virtual + method `VisitEventToSpanEventBridge()` [#4309](https://github.com/open-telemetry/opentelemetry-cpp/pull/4309) - * Source compatible: both methods have default implementations, so existing - processors continue to compile and behave as before. `OnEmitWithContext()` - forwards to `OnEmit()` and `ConsumesResolvedContext()` returns `false`. - * Not ABI compatible: the added virtuals change the vtable layout of - `LogRecordProcessor`, so code that subclasses it must be recompiled against - this version of the SDK. - * `LogRecordProcessorConfigurationVisitor::VisitEventToSpanEventBridge()` was - added with a default no-op implementation, so existing visitor - implementations remain source compatible. + * Source compatible: the method has a default no-op implementation, so + existing visitor implementations continue to compile and behave as + before. + * Not ABI compatible: the added virtual changes the vtable layout of + `LogRecordProcessorConfigurationVisitor`, so code that subclasses it must + be recompiled against this version of the SDK. * [CONFIGURATION] Update prometheus config to schema v1.1.0 [#4383](https://github.com/open-telemetry/opentelemetry-cpp/pull/4383) * The following YAML values for `translation_strategy` have been replaced diff --git a/api/include/opentelemetry/logs/logger.h b/api/include/opentelemetry/logs/logger.h index 625fa9d3e1..cc48be1a72 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -80,30 +80,6 @@ class Logger */ virtual void EmitLogRecord(nostd::unique_ptr &&log_record) noexcept = 0; -#if OPENTELEMETRY_ABI_VERSION_NO >= 2 - /** - * Emit a Log Record object together with the context it was resolved against: the explicitly - * supplied context when @c EmitLogRecord(args...) below was given one, otherwise the ambient - * context captured at that time. - * - * The default implementation ignores @p resolved_context and forwards to - * @c EmitLogRecord(std::move(log_record)), so existing @c Logger implementations that only - * override the record-only overload keep compiling and behaving exactly as before. Only a - * @c Logger that needs the resolved context (for example, to hand it to a - * context-aware @c LogRecordProcessor) needs to override this as well. - * - * @param log_record Log record - * @param resolved_context the resolved context (SpanContext or Context variant) - */ - virtual void EmitLogRecordWithContext( - nostd::unique_ptr &&log_record, - const nostd::variant - & /*resolved_context*/) noexcept - { - EmitLogRecord(std::move(log_record)); - } -#endif // OPENTELEMETRY_ABI_VERSION_NO >= 2 - /** * Emit a Log Record object with arguments. * @@ -139,7 +115,23 @@ class Logger return; } - StampLogRecordFields(log_record.get(), std::forward(args)...); + // + // Keep the parameter pack unpacking order from left to right because left + // ones are usually more important like severity and event_id than the + // attributes. The left to right unpack order could pass the more important + // data to processors to avoid caching and memory allocating. + // +#if __cplusplus <= 201402L + // C++14 does not support fold expressions for parameter pack expansion. + int dummy[] = {(detail::LogRecordSetterTrait::type>::Set( + log_record.get(), std::forward(args)), + 0)...}; + IgnoreTraitResult(dummy); +#else + IgnoreTraitResult((detail::LogRecordSetterTrait::type>::Set( + log_record.get(), std::forward(args)), + ...)); +#endif EmitLogRecord(std::move(log_record)); } @@ -223,21 +215,11 @@ class Logger } nostd::unique_ptr log_record = CreateLogRecord(context_or_span); - if (!log_record) - { - return; - } - - StampLogRecordFields(log_record.get(), std::forward(args)...); - - // Carries the already-resolved context_or_span through, rather than letting the record-only - // EmitLogRecord() overload re-derive it (which would silently fall back to the ambient - // context and lose an explicitly supplied one). - EmitLogRecordWithContext(std::move(log_record), context_or_span); #else nostd::unique_ptr log_record = CreateLogRecord(); - EmitLogRecord(std::move(log_record), std::forward(args)...); #endif // OPENTELEMETRY_ABI_VERSION_NO >= 2 + + EmitLogRecord(std::move(log_record), std::forward(args)...); } /** @@ -661,31 +643,6 @@ class Logger void IgnoreTraitResult(ValueType &&...) {} - // Applies each of args to log_record via LogRecordSetterTrait, left to right. Shared by every - // EmitLogRecord(...) overload that stamps fields from a caller-supplied argument pack, so the - // stamping logic has one definition regardless of which overload resolved the record. - template - void StampLogRecordFields(LogRecord *log_record, ArgumentType &&...args) - { - // - // Keep the parameter pack unpacking order from left to right because left - // ones are usually more important like severity and event_id than the - // attributes. The left to right unpack order could pass the more important - // data to processors to avoid caching and memory allocating. - // -#if __cplusplus <= 201402L - // C++14 does not support fold expressions for parameter pack expansion. - int dummy[] = {(detail::LogRecordSetterTrait::type>::Set( - log_record, std::forward(args)), - 0)...}; - IgnoreTraitResult(dummy); -#else - IgnoreTraitResult((detail::LogRecordSetterTrait::type>::Set( - log_record, std::forward(args)), - ...)); -#endif - } - // // minimum_severity_ can be updated concurrently by multiple threads/cores, so race condition on // read/write should be handled. And std::atomic can not be used here because it is not ABI diff --git a/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h b/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h deleted file mode 100644 index 1272547187..0000000000 --- a/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h +++ /dev/null @@ -1,70 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -#pragma once - -#include -#include - -#include "opentelemetry/context/context.h" -#include "opentelemetry/nostd/variant.h" -#include "opentelemetry/sdk/logs/processor.h" -#include "opentelemetry/sdk/logs/recordable.h" -#include "opentelemetry/trace/span_context.h" -#include "opentelemetry/version.h" - -OPENTELEMETRY_BEGIN_NAMESPACE -namespace sdk -{ -namespace logs -{ - -/** - * A log record processor that bridges log records representing events (i.e. log records with a - * non-empty event name) onto the corresponding span as span events. - * - * A log record is bridged to a span event if and only if: - * - the log record has a non-empty event name, - * - the log record carries a valid trace id and span id, - * - the span with those IDs is the current active span (from the log's resolved context), and - * - that span is currently recording. - * - * The resolved context (explicit if provided to the logger, else ambient) is used to determine - * which span to bridge to, ensuring spec compliance for explicitly-supplied contexts. - * - * This processor does not export log records and does not stop the log record from continuing - * through the rest of the configured processing pipeline. - */ -class EventToSpanEventBridgeProcessor : public LogRecordProcessor -{ -public: - EventToSpanEventBridgeProcessor() = default; - EventToSpanEventBridgeProcessor(const EventToSpanEventBridgeProcessor &) = delete; - EventToSpanEventBridgeProcessor(EventToSpanEventBridgeProcessor &&) = delete; - EventToSpanEventBridgeProcessor &operator=(const EventToSpanEventBridgeProcessor &) = delete; - EventToSpanEventBridgeProcessor &operator=(EventToSpanEventBridgeProcessor &&) = delete; - ~EventToSpanEventBridgeProcessor() override = default; - - std::unique_ptr MakeRecordable() noexcept override; - - void OnEmit(std::unique_ptr &&record) noexcept override; - - void OnEmitWithContext(std::unique_ptr &&record, - const opentelemetry::nostd::variant - &context) noexcept override; - - bool ForceFlush( - std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override; - - bool Shutdown( - std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override; - - bool HasEnabledFilter() const noexcept override { return false; } - - bool ConsumesResolvedContext() const noexcept override { return true; } -}; - -} // namespace logs -} // namespace sdk -OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor_factory.h b/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor_factory.h deleted file mode 100644 index d5a1b7378b..0000000000 --- a/sdk/include/opentelemetry/sdk/logs/event_to_span_event_bridge_processor_factory.h +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -#pragma once - -#include - -#include "opentelemetry/sdk/logs/processor.h" -#include "opentelemetry/version.h" - -OPENTELEMETRY_BEGIN_NAMESPACE -namespace sdk -{ -namespace logs -{ - -/** - * Factory class for EventToSpanEventBridgeProcessor. - */ -class OPENTELEMETRY_EXPORT EventToSpanEventBridgeProcessorFactory -{ -public: - /** - * Create an EventToSpanEventBridgeProcessor. - */ - static std::unique_ptr Create(); -}; - -} // namespace logs -} // namespace sdk -OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/logs/logger.h b/sdk/include/opentelemetry/sdk/logs/logger.h index 547d142421..44ed4ec1ee 100644 --- a/sdk/include/opentelemetry/sdk/logs/logger.h +++ b/sdk/include/opentelemetry/sdk/logs/logger.h @@ -14,18 +14,20 @@ #include "opentelemetry/logs/noop.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/logs/logger_context.h" -#include "opentelemetry/trace/span_context.h" #include "opentelemetry/version.h" +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 +# include "opentelemetry/nostd/variant.h" +# include "opentelemetry/trace/span_context.h" +#endif // OPENTELEMETRY_ABI_VERSION_NO >= 2 + OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { namespace logs { -class Recordable; class Logger final : public opentelemetry::logs::Logger { @@ -60,13 +62,6 @@ class Logger final : public opentelemetry::logs::Logger void EmitLogRecord( nostd::unique_ptr &&log_record) noexcept override; -#if OPENTELEMETRY_ABI_VERSION_NO >= 2 - void EmitLogRecordWithContext( - nostd::unique_ptr &&log_record, - const nostd::variant - &resolved_context) noexcept override; -#endif // OPENTELEMETRY_ABI_VERSION_NO >= 2 - /** Returns the associated instrumentation scope */ const opentelemetry::sdk::instrumentationscope::InstrumentationScope &GetInstrumentationScope() const noexcept; @@ -108,16 +103,6 @@ class Logger final : public opentelemetry::logs::Logger */ void UpdateLoggerConfig(LoggerConfig config) noexcept; - /** - * Shared tail of EmitLogRecord()/EmitLogRecordWithContext(): dispatches recordable to the - * configured processor, resolving the ambient context only if resolved_context is nullptr - * and some processor actually needs it (per LoggerContext::ConsumesResolvedContext()). - */ - void EmitToProcessor( - std::unique_ptr &&recordable, - const nostd::variant - *resolved_context) noexcept; - // The name of this logger std::string logger_name_; diff --git a/sdk/include/opentelemetry/sdk/logs/logger_context.h b/sdk/include/opentelemetry/sdk/logs/logger_context.h index dc93bb7eb9..bf19fa879c 100644 --- a/sdk/include/opentelemetry/sdk/logs/logger_context.h +++ b/sdk/include/opentelemetry/sdk/logs/logger_context.h @@ -93,15 +93,6 @@ class LoggerContext */ bool RecordableEnforcesLogRecordLimits() const noexcept; - /** - * Whether any configured processor reads the resolved context passed to - * LogRecordProcessor::OnEmitWithContext(). Computed once at construction (and refreshed by - * AddProcessor) so the Logger hot path can dispatch through plain OnEmit() -- and skip - * resolving the ambient context entirely -- when no processor needs it. - * @return true if at least one processor consumes the resolved context. - */ - bool ConsumesResolvedContext() const noexcept; - /** * Replace the ScopeConfigurator for this logger context. * @param logger_configurator The new configurator. @@ -133,11 +124,6 @@ class LoggerContext // recordable enforces the LogRecord attribute limits. Refreshed by // AddProcessor so the Logger hot path reads a plain bool. bool recordable_enforces_limits_{false}; - - // Cached capability flag: true when at least one configured processor reads the resolved - // context. Refreshed by AddProcessor so the Logger hot path reads a plain bool and only - // resolves the ambient context when some processor will actually look at it. - bool consumes_resolved_context_{false}; }; } // namespace logs } // namespace sdk diff --git a/sdk/include/opentelemetry/sdk/logs/multi_log_record_processor.h b/sdk/include/opentelemetry/sdk/logs/multi_log_record_processor.h index e8a3b7c2ec..b7f73ec0c1 100644 --- a/sdk/include/opentelemetry/sdk/logs/multi_log_record_processor.h +++ b/sdk/include/opentelemetry/sdk/logs/multi_log_record_processor.h @@ -44,14 +44,6 @@ class MultiLogRecordProcessor : public LogRecordProcessor */ void OnEmit(std::unique_ptr &&record) noexcept override; - /** - * OnEmitWithContext forwards the context to child processors. - */ - void OnEmitWithContext(std::unique_ptr &&record, - const opentelemetry::nostd::variant - &context) noexcept override; - /** * Exports all log records that have not yet been exported to the configured Exporter. * @param timeout that the forceflush is required to finish within. @@ -74,8 +66,6 @@ class MultiLogRecordProcessor : public LogRecordProcessor bool RecordableEnforcesLogRecordLimits() const noexcept override; - bool ConsumesResolvedContext() const noexcept override; - protected: /** * Exports all log records that have not yet been exported to the configured Exporter. diff --git a/sdk/include/opentelemetry/sdk/logs/processor.h b/sdk/include/opentelemetry/sdk/logs/processor.h index 49bf143dee..41e47d1a27 100644 --- a/sdk/include/opentelemetry/sdk/logs/processor.h +++ b/sdk/include/opentelemetry/sdk/logs/processor.h @@ -6,7 +6,6 @@ #include #include -#include "opentelemetry/common/macros.h" #include "opentelemetry/context/context.h" #include "opentelemetry/logs/severity.h" #include "opentelemetry/nostd/string_view.h" @@ -61,47 +60,6 @@ class LogRecordProcessor */ virtual void OnEmit(std::unique_ptr &&record) noexcept = 0; - /** - * OnEmitWithContext is called by the SDK instead of OnEmit() when some processor in the - * configured pipeline reports ConsumesResolvedContext(), giving the processor access to the - * log record's resolved context: the explicitly supplied context when one was provided to the - * Logger, otherwise the ambient context. - * - * The supplied context is valid only for the duration of this call. Implementations must not - * retain it (or anything obtained from it, such as a Span) beyond the call, because doing so - * would keep the referenced span alive past its intended lifetime. - * - * Compatibility notes: - * - Source compatibility is preserved. This method has a default implementation that forwards - * to OnEmit(), so existing processors continue to compile and behave exactly as before. - * A distinct name (rather than an OnEmit() overload) is used so that a derived class - * overriding only OnEmit() does not hide this method. - * - ABI compatibility is NOT preserved. Adding a virtual method changes the vtable layout of - * LogRecordProcessor, so downstream code that subclasses LogRecordProcessor must be - * recompiled against this version of the SDK. Mixing a processor built against an older - * header with this SDK is undefined behavior. - * - * @param record the log recordable object - * @param context the resolved context (SpanContext or Context variant) - */ - virtual void OnEmitWithContext(std::unique_ptr &&record, - OPENTELEMETRY_MAYBE_UNUSED const opentelemetry::nostd::variant< - opentelemetry::trace::SpanContext, - opentelemetry::context::Context> &context) noexcept - { - OnEmit(std::move(record)); - } - - /** - * Returns true when this processor reads the resolved context passed to - * OnEmitWithContext(). The default returns false, so the SDK Logger keeps dispatching through - * OnEmit() and does not resolve the ambient context on the emit path. - * - * Only processors that override OnEmitWithContext() should return true. Composite processors - * return true if any child does. - */ - virtual bool ConsumesResolvedContext() const noexcept { return false; } - /** * Enabled returns whether this processor is interested in a log with the given inputs. * The default implementation is permissive and returns true. diff --git a/sdk/src/configuration/sdk_builder.cc b/sdk/src/configuration/sdk_builder.cc index b77bc6c936..c439b3a8ce 100644 --- a/sdk/src/configuration/sdk_builder.cc +++ b/sdk/src/configuration/sdk_builder.cc @@ -169,7 +169,6 @@ #include "opentelemetry/sdk/configuration/view_selector_configuration.h" #include "opentelemetry/sdk/configuration/view_stream_configuration.h" #include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" -#include "opentelemetry/sdk/logs/event_to_span_event_bridge_processor_factory.h" #include "opentelemetry/sdk/logs/exporter.h" #include "opentelemetry/sdk/logs/log_record_limits.h" #include "opentelemetry/sdk/logs/logger_config.h" @@ -2307,7 +2306,17 @@ SdkBuilder::CreateEventToSpanEventBridgeLogRecordProcessor( const opentelemetry::sdk::configuration::EventToSpanEventBridgeLogRecordProcessorConfiguration * /* model */) const { - return opentelemetry::sdk::logs::EventToSpanEventBridgeProcessorFactory::Create(); + // FIXME-SDK: the event_to_span_event_bridge/development processor has no SDK implementation + // yet. It requires new pipeline processor architecture (a read-write recordable, a composable + // pipeline processor interface, and a root processor supporting it) so it can read the + // resolved context and mutate a record without taking exclusive ownership away from + // downstream processors/exporters. Tracked by + // https://github.com/open-telemetry/opentelemetry-cpp/issues/4454; implement this once that + // architecture lands. + OTEL_INTERNAL_LOG_WARN( + "event_to_span_event_bridge/development processor is not yet supported, ignoring. See " + "https://github.com/open-telemetry/opentelemetry-cpp/issues/4454"); + return nullptr; } std::unique_ptr diff --git a/sdk/src/logs/CMakeLists.txt b/sdk/src/logs/CMakeLists.txt index e28f4e1638..338fa8e8f1 100644 --- a/sdk/src/logs/CMakeLists.txt +++ b/sdk/src/logs/CMakeLists.txt @@ -13,8 +13,6 @@ add_library( event_logger.cc simple_log_record_processor.cc simple_log_record_processor_factory.cc - event_to_span_event_bridge_processor.cc - event_to_span_event_bridge_processor_factory.cc batch_log_record_processor.cc batch_log_record_processor_options.cc batch_log_record_processor_factory.cc diff --git a/sdk/src/logs/event_to_span_event_bridge_processor.cc b/sdk/src/logs/event_to_span_event_bridge_processor.cc deleted file mode 100644 index 1da173085c..0000000000 --- a/sdk/src/logs/event_to_span_event_bridge_processor.cc +++ /dev/null @@ -1,272 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -#include -#include -#include -#include -#include -#include -#include - -#include "opentelemetry/common/attribute_value.h" -#include "opentelemetry/common/key_value_iterable.h" -#include "opentelemetry/common/timestamp.h" -#include "opentelemetry/nostd/function_ref.h" -#include "opentelemetry/nostd/shared_ptr.h" -#include "opentelemetry/nostd/span.h" -#include "opentelemetry/nostd/string_view.h" -#include "opentelemetry/nostd/variant.h" -#include "opentelemetry/sdk/common/attribute_utils.h" -#include "opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h" -#include "opentelemetry/sdk/logs/read_write_log_record.h" -#include "opentelemetry/sdk/logs/recordable.h" -#include "opentelemetry/trace/context.h" -#include "opentelemetry/trace/span.h" -#include "opentelemetry/trace/span_context.h" -#include "opentelemetry/trace/span_id.h" -#include "opentelemetry/trace/trace_id.h" -#include "opentelemetry/trace/tracer.h" -#include "opentelemetry/version.h" - -OPENTELEMETRY_BEGIN_NAMESPACE -namespace sdk -{ -namespace logs -{ - -namespace -{ - -// Converts a single OwnedAttributeValue into the corresponding AttributeValue and forwards it -// to the KeyValueIterable callback. Array-of-string and array-of-bool attributes need a -// temporary contiguous buffer (std::vector does not decay to a span of -// string_view, and std::vector is not backed by a real bool array), so those buffers are -// kept alive for the duration of a single callback invocation only. -class EventAttributeEmitter -{ -public: - EventAttributeEmitter( - nostd::string_view key, - nostd::function_ref callback, - bool *keep_going) - : key_(key), callback_(callback), keep_going_(keep_going) - {} - - void operator()(bool v) { Emit(v); } - void operator()(int32_t v) { Emit(v); } - void operator()(uint32_t v) { Emit(v); } - void operator()(int64_t v) { Emit(v); } - void operator()(uint64_t v) { Emit(v); } - void operator()(double v) { Emit(v); } - void operator()(const std::string &v) { Emit(nostd::string_view(v)); } - - void operator()(const std::vector &v) - { - std::unique_ptr buffer(new bool[v.size()]); - for (std::size_t i = 0; i < v.size(); ++i) - { - buffer[i] = v[i]; - } - Emit(nostd::span(buffer.get(), v.size())); - } - - void operator()(const std::vector &v) - { - Emit(nostd::span(v.data(), v.size())); - } - - void operator()(const std::vector &v) - { - Emit(nostd::span(v.data(), v.size())); - } - - void operator()(const std::vector &v) - { - Emit(nostd::span(v.data(), v.size())); - } - - void operator()(const std::vector &v) - { - Emit(nostd::span(v.data(), v.size())); - } - - void operator()(const std::vector &v) - { - Emit(nostd::span(v.data(), v.size())); - } - - void operator()(const std::vector &v) - { - Emit(nostd::span(v.data(), v.size())); - } - - void operator()(const std::vector &v) - { - std::vector views(v.begin(), v.end()); - Emit(nostd::span(views.data(), views.size())); - } - -private: - template - void Emit(const T &value) - { - if (*keep_going_) - { - *keep_going_ = callback_(key_, opentelemetry::common::AttributeValue(value)); - } - } - - nostd::string_view key_; - nostd::function_ref callback_; - bool *keep_going_; -}; - -class EventAttributesKeyValueIterable final : public opentelemetry::common::KeyValueIterable -{ -public: - explicit EventAttributesKeyValueIterable( - const std::unordered_map - &attributes) - : attributes_(&attributes) - {} - - bool ForEachKeyValue( - nostd::function_ref callback) - const noexcept override - { - // Building temporary buffers for array-of-bool and array-of-string attributes (see - // EventAttributeEmitter above) can allocate, so guard against a thrown bad_alloc escaping - // this noexcept override; conservatively stop iterating rather than terminate the process. -#if OPENTELEMETRY_HAVE_EXCEPTIONS - try - { -#endif - bool keep_going = true; - for (const auto &kv : *attributes_) - { - if (!keep_going) - { - break; - } - EventAttributeEmitter emitter(kv.first, callback, &keep_going); - opentelemetry::nostd::visit(emitter, kv.second); - } - return keep_going; -#if OPENTELEMETRY_HAVE_EXCEPTIONS - } - catch (...) - { - return false; - } -#endif - } - - std::size_t size() const noexcept override { return attributes_->size(); } - -private: - const std::unordered_map - *attributes_; -}; - -// Bridges `record` onto `span` as a span event, provided every precondition holds: -// the record is an event (non-empty event name), it carries a valid trace id and span id, -// `span` is a live recording span, and `span`'s ids match the ones on the record. -// Any failed precondition means the record is not an event for this span, and it is left -// to continue through the rest of the pipeline untouched. -void BridgeRecordToSpan(const ReadWriteLogRecord *log_record, - const nostd::shared_ptr &span) noexcept -{ - if (log_record == nullptr) - { - return; - } - - nostd::string_view event_name = log_record->GetEventName(); - if (event_name.empty()) - { - return; - } - - const opentelemetry::trace::TraceId &log_trace_id = log_record->GetTraceId(); - const opentelemetry::trace::SpanId &log_span_id = log_record->GetSpanId(); - if (!log_trace_id.IsValid() || !log_span_id.IsValid()) - { - return; - } - - // trace::GetSpan() yields a non-recording DefaultSpan when the context carries no span, - // so the IsRecording() check also covers the "no span in context" case. - if (!span || !span->IsRecording()) - { - return; - } - - const opentelemetry::trace::SpanContext span_context = span->GetContext(); - if (!(span_context.trace_id() == log_trace_id) || !(span_context.span_id() == log_span_id)) - { - return; - } - - opentelemetry::common::SystemTimestamp event_timestamp = log_record->GetTimestamp(); - if (event_timestamp.time_since_epoch().count() == 0) - { - event_timestamp = log_record->GetObservedTimestamp(); - } - - EventAttributesKeyValueIterable attributes(log_record->GetAttributes()); - span->AddEvent(event_name, event_timestamp, attributes); -} - -} // namespace - -std::unique_ptr EventToSpanEventBridgeProcessor::MakeRecordable() noexcept -{ - return std::unique_ptr(new ReadWriteLogRecord()); -} - -void EventToSpanEventBridgeProcessor::OnEmit(std::unique_ptr &&record) noexcept -{ - const std::unique_ptr log_record( - static_cast(std::move(record).release())); - - // No resolved context supplied, so the ambient span is the only span reachable here. - BridgeRecordToSpan(log_record.get(), opentelemetry::trace::Tracer::GetCurrentSpan()); -} - -void EventToSpanEventBridgeProcessor::OnEmitWithContext( - std::unique_ptr &&record, - const opentelemetry::nostd::variant &context) noexcept -{ - const std::unique_ptr log_record( - static_cast(std::move(record).release())); - - if (const auto *resolved_context = nostd::get_if(&context)) - { - // The resolved context carries the live Span the record belongs to. Add the event to that - // span directly, which is what makes an explicitly supplied context work even when a - // different span is ambient. - BridgeRecordToSpan(log_record.get(), opentelemetry::trace::GetSpan(*resolved_context)); - return; - } - - // A bare SpanContext carries only ids, not a live Span, and the SDK offers no way to look up - // a Span by id. The ambient span is therefore the only span that can be written to; the id - // check in BridgeRecordToSpan drops the record when it is not the one the ids refer to. - BridgeRecordToSpan(log_record.get(), opentelemetry::trace::Tracer::GetCurrentSpan()); -} - -bool EventToSpanEventBridgeProcessor::ForceFlush(std::chrono::microseconds /* timeout */) noexcept -{ - return true; -} - -bool EventToSpanEventBridgeProcessor::Shutdown(std::chrono::microseconds /* timeout */) noexcept -{ - return true; -} - -} // namespace logs -} // namespace sdk -OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/src/logs/event_to_span_event_bridge_processor_factory.cc b/sdk/src/logs/event_to_span_event_bridge_processor_factory.cc deleted file mode 100644 index 4c339aa9e8..0000000000 --- a/sdk/src/logs/event_to_span_event_bridge_processor_factory.cc +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -#include - -#include "opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h" -#include "opentelemetry/sdk/logs/event_to_span_event_bridge_processor_factory.h" -#include "opentelemetry/sdk/logs/processor.h" -#include "opentelemetry/version.h" - -OPENTELEMETRY_BEGIN_NAMESPACE -namespace sdk -{ -namespace logs -{ - -std::unique_ptr EventToSpanEventBridgeProcessorFactory::Create() -{ - std::unique_ptr processor(new EventToSpanEventBridgeProcessor()); - return processor; -} - -} // namespace logs -} // namespace sdk -OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/src/logs/logger.cc b/sdk/src/logs/logger.cc index 35215497c7..c04b1dfe68 100644 --- a/sdk/src/logs/logger.cc +++ b/sdk/src/logs/logger.cc @@ -195,59 +195,8 @@ void Logger::EmitLogRecord( recordable->SetResource(context_->GetResource()); recordable->SetInstrumentationScope(GetInstrumentationScope()); - // No resolved context was ever produced for this record (it did not go through - // EmitLogRecord(args...)/EmitLogRecordWithContext()), so the ambient context -- resolved - // below only if a processor actually needs it -- is the best available approximation. - EmitToProcessor(std::move(recordable), nullptr); -} - -#if OPENTELEMETRY_ABI_VERSION_NO >= 2 -void Logger::EmitLogRecordWithContext( - opentelemetry::nostd::unique_ptr &&log_record, - const nostd::variant &resolved_context) noexcept -{ - if (!logger_enabled_.load(std::memory_order_relaxed)) - { - return kNoopLogger.EmitLogRecordWithContext(std::move(log_record), resolved_context); - } - - if (!log_record) - { - return; - } - - std::unique_ptr recordable = - std::unique_ptr(static_cast(log_record.release())); - recordable->SetResource(context_->GetResource()); - recordable->SetInstrumentationScope(GetInstrumentationScope()); - - EmitToProcessor(std::move(recordable), &resolved_context); -} -#endif // OPENTELEMETRY_ABI_VERSION_NO >= 2 - -void Logger::EmitToProcessor( - std::unique_ptr &&recordable, - const nostd::variant *resolved_context) noexcept -{ auto &processor = context_->GetProcessor(); - // Only resolve/forward a context when a processor will actually read it: resolving the - // ambient context means a RuntimeContext::GetCurrent() call, which would otherwise be pure - // overhead on every log for the common pipelines (simple/batch) that ignore it entirely. - if (context_->ConsumesResolvedContext()) - { - if (resolved_context != nullptr) - { - processor.OnEmitWithContext(std::move(recordable), *resolved_context); - return; - } - - const nostd::variant ambient_context{ - context::RuntimeContext::GetCurrent()}; - processor.OnEmitWithContext(std::move(recordable), ambient_context); - return; - } - // Send the log recordable to the processor processor.OnEmit(std::move(recordable)); } diff --git a/sdk/src/logs/logger_context.cc b/sdk/src/logs/logger_context.cc index 6f7eba304c..7f6d5315ec 100644 --- a/sdk/src/logs/logger_context.cc +++ b/sdk/src/logs/logger_context.cc @@ -33,8 +33,7 @@ LoggerContext::LoggerContext( std::unique_ptr(new MultiLogRecordProcessor(std::move(processors)))), logger_configurator_(std::move(logger_configurator)), log_record_limits_(log_record_limits), - recordable_enforces_limits_(processor_->RecordableEnforcesLogRecordLimits()), - consumes_resolved_context_(processor_->ConsumesResolvedContext()) + recordable_enforces_limits_(processor_->RecordableEnforcesLogRecordLimits()) {} void LoggerContext::AddProcessor(std::unique_ptr processor) noexcept @@ -42,7 +41,6 @@ void LoggerContext::AddProcessor(std::unique_ptr processor) auto multi_processor = static_cast(processor_.get()); multi_processor->AddProcessor(std::move(processor)); recordable_enforces_limits_ = processor_->RecordableEnforcesLogRecordLimits(); - consumes_resolved_context_ = processor_->ConsumesResolvedContext(); } LogRecordProcessor &LoggerContext::GetProcessor() const noexcept @@ -71,11 +69,6 @@ bool LoggerContext::RecordableEnforcesLogRecordLimits() const noexcept return recordable_enforces_limits_; } -bool LoggerContext::ConsumesResolvedContext() const noexcept -{ - return consumes_resolved_context_; -} - void LoggerContext::SetLoggerConfigurator( std::unique_ptr> logger_configurator) noexcept diff --git a/sdk/src/logs/multi_log_record_processor.cc b/sdk/src/logs/multi_log_record_processor.cc index 1d2d34347a..8ebb004213 100644 --- a/sdk/src/logs/multi_log_record_processor.cc +++ b/sdk/src/logs/multi_log_record_processor.cc @@ -74,28 +74,6 @@ void MultiLogRecordProcessor::OnEmit(std::unique_ptr &&record) noexc } } -void MultiLogRecordProcessor::OnEmitWithContext( - std::unique_ptr &&record, - const opentelemetry::nostd::variant &context) noexcept -{ - auto log_record = std::move(record); - if (!log_record) - { - return; - } - auto multi_recordable = static_cast(log_record.get()); - - for (auto &processor : processors_) - { - auto recordable = multi_recordable->ReleaseRecordable(*processor); - if (recordable) - { - processor->OnEmitWithContext(std::move(recordable), context); - } - } -} - bool MultiLogRecordProcessor::EnabledImplementation( const opentelemetry::nostd::variant &context_or_span, @@ -144,18 +122,6 @@ bool MultiLogRecordProcessor::RecordableEnforcesLogRecordLimits() const noexcept return false; } -bool MultiLogRecordProcessor::ConsumesResolvedContext() const noexcept -{ - for (const auto &processor : processors_) - { - if (processor != nullptr && processor->ConsumesResolvedContext()) - { - return true; - } - } - return false; -} - bool MultiLogRecordProcessor::ForceFlush(std::chrono::microseconds timeout) noexcept { return InternalForceFlush(timeout); diff --git a/sdk/test/configuration/sdk_builder_test.cc b/sdk/test/configuration/sdk_builder_test.cc index 7550c44fd1..a842f970de 100644 --- a/sdk/test/configuration/sdk_builder_test.cc +++ b/sdk/test/configuration/sdk_builder_test.cc @@ -81,9 +81,13 @@ #include "opentelemetry/nostd/function_ref.h" #include "opentelemetry/nostd/variant.h" +#include "opentelemetry/sdk/common/global_log_handler.h" +#include "opentelemetry/sdk/configuration/event_to_span_event_bridge_log_record_processor_configuration.h" +#include "opentelemetry/sdk/configuration/log_record_processor_configuration.h" #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" #include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" #include "opentelemetry/sdk/logs/logger_config.h" +#include "opentelemetry/sdk/logs/processor.h" #include "opentelemetry/sdk/metrics/aggregation/aggregation.h" #include "opentelemetry/sdk/metrics/aggregation/aggregation_config.h" #include "opentelemetry/sdk/metrics/aggregation/default_aggregation.h" @@ -99,6 +103,7 @@ #include "opentelemetry/sdk/trace/samplers/composable_always_on.h" #include "opentelemetry/sdk/trace/span_limits.h" #include "opentelemetry/sdk/trace/tracer_provider.h" +#include "opentelemetry/test_common/sdk/common/scoped_test_log_handler.h" #include "opentelemetry/trace/span_context.h" #include "opentelemetry/trace/span_context_kv_iterable_view.h" #include "opentelemetry/trace/span_id.h" @@ -280,6 +285,29 @@ TEST_F(SdkBuilderExemplarFilterEnvironmentTest, DeclarativeExemplarFilterDoesNot } #endif +// The event_to_span_event_bridge/development processor has no SDK implementation yet (see +// https://github.com/open-telemetry/opentelemetry-cpp/issues/4454): SdkBuilder should log a +// warning and skip it rather than build a processor, so a pipeline like +// [event_to_span_event_bridge, batch] just runs batch instead of failing or crashing. +TEST(SdkBuilder, CreateEventToSpanEventBridgeLogRecordProcessorWarnsAndSkips) +{ + opentelemetry::test_common::ScopedTestLogHandler log_handler{ + opentelemetry::sdk::common::internal_log::LogLevel::Warning}; + + std::unique_ptr model = + std::make_unique(); + + SdkBuilder builder(RegistryFactory::Create()); + auto processor = builder.CreateLogRecordProcessor(model); + + EXPECT_EQ(processor, nullptr); + + auto entries = log_handler.Drain(); + ASSERT_EQ(entries.size(), 1U); + EXPECT_EQ(entries[0].level, opentelemetry::sdk::common::internal_log::LogLevel::Warning); + EXPECT_NE(entries[0].msg.find("event_to_span_event_bridge"), std::string::npos); +} + TEST(SdkBuilder, CreateLoggerConfigurator) { config_sdk::LoggerConfigConfiguration default_config; diff --git a/sdk/test/logs/BUILD b/sdk/test/logs/BUILD index 4e9ffa0da6..8e2198c318 100644 --- a/sdk/test/logs/BUILD +++ b/sdk/test/logs/BUILD @@ -66,24 +66,6 @@ cc_test( ], ) -cc_test( - name = "event_to_span_event_bridge_processor_test", - srcs = [ - "event_to_span_event_bridge_processor_test.cc", - ], - tags = [ - "logs", - "test", - ], - deps = [ - "//exporters/memory:in_memory_span_exporter", - "//sdk/src/logs", - "//sdk/src/resource", - "//sdk/src/trace", - "@com_google_googletest//:gtest_main", - ], -) - cc_test( name = "log_record_test", srcs = [ diff --git a/sdk/test/logs/CMakeLists.txt b/sdk/test/logs/CMakeLists.txt index e0de53ed30..1fcbc3bb27 100644 --- a/sdk/test/logs/CMakeLists.txt +++ b/sdk/test/logs/CMakeLists.txt @@ -20,18 +20,3 @@ foreach( TEST_PREFIX logs. TEST_LIST ${testname}) endforeach() - -add_executable(event_to_span_event_bridge_processor_test - event_to_span_event_bridge_processor_test.cc) -target_link_libraries( - event_to_span_event_bridge_processor_test - ${GTEST_BOTH_LIBRARIES} - ${CMAKE_THREAD_LIBS_INIT} - opentelemetry_logs - opentelemetry_trace - opentelemetry_resources - opentelemetry_exporter_in_memory) -gtest_add_tests( - TARGET event_to_span_event_bridge_processor_test - TEST_PREFIX logs. - TEST_LIST event_to_span_event_bridge_processor_test) diff --git a/sdk/test/logs/event_to_span_event_bridge_processor_test.cc b/sdk/test/logs/event_to_span_event_bridge_processor_test.cc deleted file mode 100644 index 7cfb21dcd8..0000000000 --- a/sdk/test/logs/event_to_span_event_bridge_processor_test.cc +++ /dev/null @@ -1,265 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -#include -#include -#include -#include -#include - -#include "opentelemetry/context/context.h" -#include "opentelemetry/exporters/memory/in_memory_span_data.h" -#include "opentelemetry/exporters/memory/in_memory_span_exporter.h" -#include "opentelemetry/nostd/string_view.h" -#include "opentelemetry/nostd/variant.h" -#include "opentelemetry/sdk/logs/event_to_span_event_bridge_processor.h" -#include "opentelemetry/sdk/logs/read_write_log_record.h" -#include "opentelemetry/sdk/logs/recordable.h" -#include "opentelemetry/sdk/trace/exporter.h" -#include "opentelemetry/sdk/trace/processor.h" -#include "opentelemetry/sdk/trace/simple_processor.h" -#include "opentelemetry/sdk/trace/span_data.h" -#include "opentelemetry/sdk/trace/tracer.h" -#include "opentelemetry/sdk/trace/tracer_context.h" -#include "opentelemetry/trace/span.h" -#include "opentelemetry/trace/span_context.h" -#include "opentelemetry/trace/span_metadata.h" -#include "opentelemetry/trace/tracer.h" - -using namespace opentelemetry::sdk::logs; -using opentelemetry::exporter::memory::InMemorySpanData; -using opentelemetry::exporter::memory::InMemorySpanExporter; - -namespace trace_sdk = opentelemetry::sdk::trace; -namespace trace_api = opentelemetry::trace; - -namespace -{ - -std::shared_ptr MakeTracer(std::shared_ptr *out_span_data) -{ - InMemorySpanExporter *exporter = new InMemorySpanExporter(); - *out_span_data = exporter->GetData(); - - auto processor = std::unique_ptr( - new trace_sdk::SimpleSpanProcessor(std::unique_ptr(exporter))); - std::vector> processors; - processors.push_back(std::move(processor)); - auto context = std::make_shared(std::move(processors)); - return std::shared_ptr(new trace_sdk::Tracer(context)); -} - -} // namespace - -TEST(EventToSpanEventBridgeProcessorTest, BridgesEventOntoCurrentMatchingSpan) -{ - std::shared_ptr span_data; - auto tracer = MakeTracer(&span_data); - - auto span = tracer->StartSpan("test-span"); - auto scope = tracer->WithActiveSpan(span); - - EventToSpanEventBridgeProcessor processor; - auto recordable = processor.MakeRecordable(); - auto *log_record = static_cast(recordable.get()); - - trace_api::SpanContext span_context = span->GetContext(); - log_record->SetTraceId(span_context.trace_id()); - log_record->SetSpanId(span_context.span_id()); - log_record->SetEventId(0, "my.event"); - log_record->SetAttribute("key1", "value1"); - - processor.OnEmit(std::move(recordable)); - - span->End(); - - auto spans = span_data->GetSpans(); - ASSERT_EQ(1, spans.size()); - ASSERT_EQ(1, spans.at(0)->GetEvents().size()); - EXPECT_EQ("my.event", spans.at(0)->GetEvents().at(0).GetName()); -} - -TEST(EventToSpanEventBridgeProcessorTest, IgnoresLogRecordWithoutEventName) -{ - std::shared_ptr span_data; - auto tracer = MakeTracer(&span_data); - - auto span = tracer->StartSpan("test-span"); - auto scope = tracer->WithActiveSpan(span); - - EventToSpanEventBridgeProcessor processor; - auto recordable = processor.MakeRecordable(); - auto *log_record = static_cast(recordable.get()); - - trace_api::SpanContext span_context = span->GetContext(); - log_record->SetTraceId(span_context.trace_id()); - log_record->SetSpanId(span_context.span_id()); - - processor.OnEmit(std::move(recordable)); - - span->End(); - - auto spans = span_data->GetSpans(); - ASSERT_EQ(1, spans.size()); - EXPECT_EQ(0, spans.at(0)->GetEvents().size()); -} - -TEST(EventToSpanEventBridgeProcessorTest, IgnoresEventFromDifferentSpan) -{ - std::shared_ptr span_data; - auto tracer = MakeTracer(&span_data); - - auto span = tracer->StartSpan("test-span"); - auto other_span = tracer->StartSpan("other-span"); - auto scope = tracer->WithActiveSpan(span); - - EventToSpanEventBridgeProcessor processor; - auto recordable = processor.MakeRecordable(); - auto *log_record = static_cast(recordable.get()); - - // The log record belongs to other_span, not the current active span. - trace_api::SpanContext other_context = other_span->GetContext(); - log_record->SetTraceId(other_context.trace_id()); - log_record->SetSpanId(other_context.span_id()); - log_record->SetEventId(0, "my.event"); - - processor.OnEmit(std::move(recordable)); - - span->End(); - other_span->End(); - - auto spans = span_data->GetSpans(); - ASSERT_EQ(2, spans.size()); - for (auto &exported_span : spans) - { - EXPECT_EQ(0, exported_span->GetEvents().size()); - } -} - -// The case that motivated OnEmitWithContext: the record belongs to span_a, which is supplied -// explicitly, while an unrelated span_b is ambient. The event must land on span_a and only on -// span_a. Dispatching through OnEmit() (ambient only) cannot satisfy this. -TEST(EventToSpanEventBridgeProcessorTest, BridgesOntoExplicitContextSpanWhileDifferentSpanIsAmbient) -{ - std::shared_ptr span_data; - auto tracer = MakeTracer(&span_data); - - auto span_a = tracer->StartSpan("span-a"); - auto span_b = tracer->StartSpan("span-b"); - - // span_b is ambient; span_a is only reachable through the explicitly supplied context. - auto scope = tracer->WithActiveSpan(span_b); - - opentelemetry::context::Context explicit_context = - opentelemetry::context::Context().SetValue(trace_api::kSpanKey, span_a); - - EventToSpanEventBridgeProcessor processor; - auto recordable = processor.MakeRecordable(); - auto *log_record = static_cast(recordable.get()); - - trace_api::SpanContext span_a_context = span_a->GetContext(); - log_record->SetTraceId(span_a_context.trace_id()); - log_record->SetSpanId(span_a_context.span_id()); - log_record->SetEventId(0, "my.event"); - log_record->SetAttribute("key1", "value1"); - - const opentelemetry::nostd::variant - resolved_context{explicit_context}; - processor.OnEmitWithContext(std::move(recordable), resolved_context); - - span_a->End(); - span_b->End(); - - auto spans = span_data->GetSpans(); - ASSERT_EQ(2, spans.size()); - - int spans_with_event = 0; - for (auto &exported_span : spans) - { - if (exported_span->GetName() == "span-a") - { - ASSERT_EQ(1, exported_span->GetEvents().size()); - EXPECT_EQ("my.event", exported_span->GetEvents().at(0).GetName()); - ++spans_with_event; - } - else - { - EXPECT_EQ(0, exported_span->GetEvents().size()); - } - } - EXPECT_EQ(1, spans_with_event); -} - -// An explicitly supplied context whose span does not match the record's ids must not bridge. -TEST(EventToSpanEventBridgeProcessorTest, IgnoresEventWhenExplicitContextSpanDoesNotMatch) -{ - std::shared_ptr span_data; - auto tracer = MakeTracer(&span_data); - - auto span_a = tracer->StartSpan("span-a"); - auto span_b = tracer->StartSpan("span-b"); - - opentelemetry::context::Context explicit_context = - opentelemetry::context::Context().SetValue(trace_api::kSpanKey, span_b); - - EventToSpanEventBridgeProcessor processor; - auto recordable = processor.MakeRecordable(); - auto *log_record = static_cast(recordable.get()); - - // Record carries span_a's ids, but span_b is the span in the resolved context. - trace_api::SpanContext span_a_context = span_a->GetContext(); - log_record->SetTraceId(span_a_context.trace_id()); - log_record->SetSpanId(span_a_context.span_id()); - log_record->SetEventId(0, "my.event"); - - const opentelemetry::nostd::variant - resolved_context{explicit_context}; - processor.OnEmitWithContext(std::move(recordable), resolved_context); - - span_a->End(); - span_b->End(); - - auto spans = span_data->GetSpans(); - ASSERT_EQ(2, spans.size()); - for (auto &exported_span : spans) - { - EXPECT_EQ(0, exported_span->GetEvents().size()); - } -} - -// A bare SpanContext carries only ids, so the ambient span is the only writable span. It must -// still bridge when the ambient span is the one the ids refer to. -TEST(EventToSpanEventBridgeProcessorTest, BridgesOntoAmbientSpanForBareSpanContext) -{ - std::shared_ptr span_data; - auto tracer = MakeTracer(&span_data); - - auto span = tracer->StartSpan("test-span"); - auto scope = tracer->WithActiveSpan(span); - - EventToSpanEventBridgeProcessor processor; - auto recordable = processor.MakeRecordable(); - auto *log_record = static_cast(recordable.get()); - - trace_api::SpanContext span_context = span->GetContext(); - log_record->SetTraceId(span_context.trace_id()); - log_record->SetSpanId(span_context.span_id()); - log_record->SetEventId(0, "my.event"); - - const opentelemetry::nostd::variant - resolved_context{span_context}; - processor.OnEmitWithContext(std::move(recordable), resolved_context); - - span->End(); - - auto spans = span_data->GetSpans(); - ASSERT_EQ(1, spans.size()); - ASSERT_EQ(1, spans.at(0)->GetEvents().size()); - EXPECT_EQ("my.event", spans.at(0)->GetEvents().at(0).GetName()); -} - -TEST(EventToSpanEventBridgeProcessorTest, ConsumesResolvedContext) -{ - EventToSpanEventBridgeProcessor processor; - EXPECT_TRUE(processor.ConsumesResolvedContext()); -} diff --git a/sdk/test/logs/logger_sdk_test.cc b/sdk/test/logs/logger_sdk_test.cc index c06640d1e6..2955049d0f 100644 --- a/sdk/test/logs/logger_sdk_test.cc +++ b/sdk/test/logs/logger_sdk_test.cc @@ -4,7 +4,6 @@ #include #include #include -#include // IWYU pragma: keep #include #include #include @@ -13,8 +12,6 @@ #include "opentelemetry/common/attribute_value.h" #include "opentelemetry/common/timestamp.h" -#include "opentelemetry/context/context.h" // IWYU pragma: keep -#include "opentelemetry/context/runtime_context.h" #include "opentelemetry/logs/event_id.h" #include "opentelemetry/logs/event_logger.h" // IWYU pragma: keep #include "opentelemetry/logs/event_logger_provider.h" // IWYU pragma: keep @@ -25,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" @@ -675,52 +671,6 @@ TEST(LoggerSDK, LoggerTraceBasedConfigAllowsSampledExplicitContextWithNamedEvent EXPECT_EQ(call_state->call_count, 1U); } -// Captures the resolved context a context-aware processor was actually handed, so a test can -// assert it is the explicitly supplied one rather than whatever happens to be ambient. -class ContextCapturingProcessor final : public LogRecordProcessor -{ -public: - std::unique_ptr MakeRecordable() noexcept override - { - return std::unique_ptr(new MockLogRecordable()); - } - - void OnEmit(std::unique_ptr &&record) noexcept override - { - static_cast(std::move(record)); - ++on_emit_calls; - } - - void OnEmitWithContext( - std::unique_ptr &&record, - const nostd::variant - &context) noexcept override - { - static_cast(std::move(record)); - ++on_emit_with_context_calls; - - if (const auto *sc = nostd::get_if(&context)) - { - received_span_context = *sc; - return; - } - if (const auto *ctx = nostd::get_if(&context)) - { - received_span_context = opentelemetry::trace::GetSpanContext(*ctx); - } - } - - bool ConsumesResolvedContext() const noexcept override { return true; } - - bool ForceFlush(std::chrono::microseconds /* timeout */) noexcept override { return true; } - bool Shutdown(std::chrono::microseconds /* timeout */) noexcept override { return true; } - - size_t on_emit_calls = 0; - size_t on_emit_with_context_calls = 0; - opentelemetry::trace::SpanContext received_span_context = - opentelemetry::trace::SpanContext::GetInvalid(); -}; - class LoggerEmitWithExplicitTraceTest : public ::testing::Test { protected: @@ -735,10 +685,6 @@ class LoggerEmitWithExplicitTraceTest : public ::testing::Test sdk_lp->AddProcessor( std::unique_ptr(new MockProcessor(shared_recordable_))); - auto *context_processor = new ContextCapturingProcessor(); - context_processor_ = context_processor; - sdk_lp->AddProcessor(std::unique_ptr(context_processor)); - { std::vector> sp; auto tp = opentelemetry::sdk::trace::TracerProviderFactory::Create(std::move(sp)); @@ -750,7 +696,6 @@ class LoggerEmitWithExplicitTraceTest : public ::testing::Test std::shared_ptr api_lp_; nostd::shared_ptr logger_; std::shared_ptr shared_recordable_; - ContextCapturingProcessor *context_processor_ = nullptr; nostd::shared_ptr runtime_span_; std::unique_ptr runtime_scope_; }; @@ -795,30 +740,6 @@ TEST_F(LoggerEmitWithExplicitTraceTest, ExplicitTracePartsStampsTraceFields) EXPECT_EQ(shared_recordable_->GetTraceFlags(), explicit_span_ctx.trace_flags()); EXPECT_NE(shared_recordable_->GetTraceId(), runtime_span_->GetContext().trace_id()); } - -// The end-to-end case lalitb flagged on review: a log is emitted with an explicit context for -// span_a while span_b (runtime_span_, from the fixture's Scope) is ambient. A processor that -// declares ConsumesResolvedContext() must receive span_a's context, not span_b's -- if the -// resolved context were re-derived from RuntimeContext::GetCurrent() instead of carried through -// from EmitLogRecord(args...), it would receive span_b here and silently misattribute the log. -TEST_F(LoggerEmitWithExplicitTraceTest, ContextAwareProcessorReceivesExplicitContextNotAmbientSpan) -{ - auto explicit_span = MakeTestSpan(/*sampled=*/true); - context::Context explicit_context; - explicit_context = opentelemetry::trace::SetSpan(explicit_context, explicit_span); - const auto explicit_span_ctx = explicit_span->GetContext(); - - ASSERT_NE(explicit_span_ctx.trace_id(), runtime_span_->GetContext().trace_id()); - - logger_->EmitLogRecord(logs_api::Severity::kInfo, nostd::string_view{"msg"}, explicit_context); - - ASSERT_EQ(context_processor_->on_emit_with_context_calls, 1u); - EXPECT_EQ(context_processor_->on_emit_calls, 0u); - EXPECT_EQ(context_processor_->received_span_context.trace_id(), explicit_span_ctx.trace_id()); - EXPECT_EQ(context_processor_->received_span_context.span_id(), explicit_span_ctx.span_id()); - EXPECT_NE(context_processor_->received_span_context.trace_id(), - runtime_span_->GetContext().trace_id()); -} #endif // OPENTELEMETRY_ABI_VERSION_NO >= 2 static std::unique_ptr create_mock_log_recordable( @@ -1065,137 +986,4 @@ TEST(LoggerSDK, EventLog) } #endif -// Records which emit entry point the Logger dispatched through, and whether the processor -// declares that it reads the resolved context. -struct EmitDispatchState -{ - size_t on_emit_calls = 0; - size_t on_emit_with_context_calls = 0; -}; - -class DispatchRecordingProcessor final : public LogRecordProcessor -{ -public: - DispatchRecordingProcessor(bool consumes_resolved_context, - std::shared_ptr state) noexcept - : consumes_resolved_context_(consumes_resolved_context), state_(std::move(state)) - {} - - std::unique_ptr MakeRecordable() noexcept override - { - return std::unique_ptr(new MockLogRecordable()); - } - - void OnEmit(std::unique_ptr &&record) noexcept override - { - ++state_->on_emit_calls; - // Deliberately does not chain to OnEmitWithContext(), so the two counters stay independent. - static_cast(std::move(record)); - } - - void OnEmitWithContext( - std::unique_ptr &&record, - const nostd::variant - & /* context */) noexcept override - { - ++state_->on_emit_with_context_calls; - // Deliberately does not chain to OnEmit(), so the two counters stay independent. - static_cast(std::move(record)); - } - - bool ConsumesResolvedContext() const noexcept override { return consumes_resolved_context_; } - - bool ForceFlush(std::chrono::microseconds /* timeout */) noexcept override { return true; } - bool Shutdown(std::chrono::microseconds /* timeout */) noexcept override { return true; } - -private: - bool consumes_resolved_context_; - std::shared_ptr state_; -}; - -// A RuntimeContextStorage that counts GetCurrent() calls, so the test can assert that a -// context-ignoring pipeline adds no context lookups on the emit path. -class CountingRuntimeContextStorage final : public opentelemetry::context::RuntimeContextStorage -{ -public: - opentelemetry::context::Context GetCurrent() noexcept override - { - ++get_current_calls; - return context_; - } - - bool Detach(opentelemetry::context::Token & /* token */) noexcept override { return true; } - - nostd::unique_ptr Attach( - const opentelemetry::context::Context &context) noexcept override - { - context_ = context; - return CreateToken(context); - } - - size_t get_current_calls = 0; - -private: - opentelemetry::context::Context context_; -}; - -std::shared_ptr MakeProviderWithProcessor( - std::unique_ptr processor) -{ - return std::shared_ptr(new LoggerProvider(std::move(processor))); -} - -TEST(LoggerSDK, EmitDispatchesThroughOnEmitWhenProcessorIgnoresResolvedContext) -{ - auto state = std::make_shared(); - auto lp = MakeProviderWithProcessor(std::unique_ptr( - new DispatchRecordingProcessor(/* consumes_resolved_context */ false, state))); - auto logger = lp->GetLogger("logger", "opentelemetry_library"); - - logger->EmitLogRecord(logger->CreateLogRecord()); - - EXPECT_EQ(state->on_emit_calls, 1u); - EXPECT_EQ(state->on_emit_with_context_calls, 0u); -} - -TEST(LoggerSDK, EmitDispatchesThroughOnEmitWithContextWhenProcessorConsumesResolvedContext) -{ - auto state = std::make_shared(); - auto lp = MakeProviderWithProcessor(std::unique_ptr( - new DispatchRecordingProcessor(/* consumes_resolved_context */ true, state))); - auto logger = lp->GetLogger("logger", "opentelemetry_library"); - - logger->EmitLogRecord(logger->CreateLogRecord()); - - EXPECT_EQ(state->on_emit_with_context_calls, 1u); - EXPECT_EQ(state->on_emit_calls, 0u); -} - -// A processor that ignores the resolved context must not cause the emit path to resolve the -// ambient context, so that adding OnEmitWithContext() costs nothing for existing pipelines. -TEST(LoggerSDK, EmitDoesNotResolveContextWhenProcessorIgnoresResolvedContext) -{ - auto *counting_storage = new CountingRuntimeContextStorage(); - opentelemetry::context::RuntimeContext::SetRuntimeContextStorage( - nostd::shared_ptr(counting_storage)); - - auto state = std::make_shared(); - auto lp = MakeProviderWithProcessor(std::unique_ptr( - new DispatchRecordingProcessor(/* consumes_resolved_context */ false, state))); - auto logger = lp->GetLogger("logger", "opentelemetry_library"); - - auto record = logger->CreateLogRecord(); - - const size_t calls_after_create = counting_storage->get_current_calls; - logger->EmitLogRecord(std::move(record)); - - EXPECT_EQ(state->on_emit_calls, 1u); - // EmitLogRecord() itself must not add a context lookup. - EXPECT_EQ(counting_storage->get_current_calls, calls_after_create); - - opentelemetry::context::RuntimeContext::SetRuntimeContextStorage( - nostd::shared_ptr( - new CountingRuntimeContextStorage())); -} - } // namespace From 80333dd9f3545cf5749b8e32c1a0b4c1e1a7739e Mon Sep 17 00:00:00 2001 From: om7057 Date: Thu, 20 Aug 2026 08:18:45 +0530 Subject: [PATCH 19/21] Address review: make visitor method pure virtual, trim comment/changelog Per @dbarker: - VisitEventToSpanEventBridge() is now pure virtual instead of defaulting to a no-op. Configuration classes under this directory are internal (used only by tests and SdkBuilder) and experimental, so this breaking change is fine without a changelog entry. - Shortened the FIXME-SDK comment in sdk_builder.cc to just point at the tracking issue, leaving implementation details to the issue itself. - Trimmed the CHANGELOG entry to describe only what this PR changes. --- CHANGELOG.md | 14 +------------- .../log_record_processor_configuration_visitor.h | 12 +----------- sdk/src/configuration/sdk_builder.cc | 12 +++--------- sdk/test/configuration/sdk_builder_test.cc | 2 -- 4 files changed, 5 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dbb4242511..0008268cc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,10 +18,7 @@ Increment the: * [CONFIGURATION] Parse the `event_to_span_event_bridge/development` log record processor configuration in declarative configuration. The SDK implementation of this processor is not yet available; `SdkBuilder` logs a - warning and skips it when configured. See - [#4454](https://github.com/open-telemetry/opentelemetry-cpp/issues/4454) - for the SDK implementation, which is blocked on new pipeline processor - architecture. + warning and skips it when configured. [#4309](https://github.com/open-telemetry/opentelemetry-cpp/pull/4309) * [CONFIGURATION] Build the configured resource detectors in SdkBuilder, apply the `detection.attributes` include/exclude filter to the detected attributes, @@ -296,15 +293,6 @@ Breaking changes: * The public configuration member `max_buckets_` was renamed to `max_size_` to match the configuration schema. Please adjust SDK configuration accordingly. -* [CONFIGURATION] `LogRecordProcessorConfigurationVisitor` gained the virtual - method `VisitEventToSpanEventBridge()` - [#4309](https://github.com/open-telemetry/opentelemetry-cpp/pull/4309) - * Source compatible: the method has a default no-op implementation, so - existing visitor implementations continue to compile and behave as - before. - * Not ABI compatible: the added virtual changes the vtable layout of - `LogRecordProcessorConfigurationVisitor`, so code that subclasses it must - be recompiled against this version of the SDK. * [CONFIGURATION] Update prometheus config to schema v1.1.0 [#4383](https://github.com/open-telemetry/opentelemetry-cpp/pull/4383) * The following YAML values for `translation_strategy` have been replaced diff --git a/sdk/include/opentelemetry/sdk/configuration/log_record_processor_configuration_visitor.h b/sdk/include/opentelemetry/sdk/configuration/log_record_processor_configuration_visitor.h index 94701e9dda..320a8fe4b1 100644 --- a/sdk/include/opentelemetry/sdk/configuration/log_record_processor_configuration_visitor.h +++ b/sdk/include/opentelemetry/sdk/configuration/log_record_processor_configuration_visitor.h @@ -30,18 +30,8 @@ class LogRecordProcessorConfigurationVisitor virtual void VisitBatch(const BatchLogRecordProcessorConfiguration *model) = 0; virtual void VisitSimple(const SimpleLogRecordProcessorConfiguration *model) = 0; - - /** - * Visit an event_to_span_event_bridge/development processor configuration. - * - * Unlike its siblings this method is not pure virtual: it was added after the visitor was - * already public, and defaulting it to a no-op keeps existing visitor implementations - * compiling. Visitors that support this processor type should override it. - */ virtual void VisitEventToSpanEventBridge( - const EventToSpanEventBridgeLogRecordProcessorConfiguration * /* model */) - {} - + const EventToSpanEventBridgeLogRecordProcessorConfiguration *model) = 0; virtual void VisitExtension(const ExtensionLogRecordProcessorConfiguration *model) = 0; }; diff --git a/sdk/src/configuration/sdk_builder.cc b/sdk/src/configuration/sdk_builder.cc index c439b3a8ce..834cba3684 100644 --- a/sdk/src/configuration/sdk_builder.cc +++ b/sdk/src/configuration/sdk_builder.cc @@ -2306,16 +2306,10 @@ SdkBuilder::CreateEventToSpanEventBridgeLogRecordProcessor( const opentelemetry::sdk::configuration::EventToSpanEventBridgeLogRecordProcessorConfiguration * /* model */) const { - // FIXME-SDK: the event_to_span_event_bridge/development processor has no SDK implementation - // yet. It requires new pipeline processor architecture (a read-write recordable, a composable - // pipeline processor interface, and a root processor supporting it) so it can read the - // resolved context and mutate a record without taking exclusive ownership away from - // downstream processors/exporters. Tracked by - // https://github.com/open-telemetry/opentelemetry-cpp/issues/4454; implement this once that - // architecture lands. + // FIXME-SDK: not yet implemented. + // https://github.com/open-telemetry/opentelemetry-cpp/issues/4454 OTEL_INTERNAL_LOG_WARN( - "event_to_span_event_bridge/development processor is not yet supported, ignoring. See " - "https://github.com/open-telemetry/opentelemetry-cpp/issues/4454"); + "event_to_span_event_bridge/development processor is not yet supported, ignoring"); return nullptr; } diff --git a/sdk/test/configuration/sdk_builder_test.cc b/sdk/test/configuration/sdk_builder_test.cc index a842f970de..240382c052 100644 --- a/sdk/test/configuration/sdk_builder_test.cc +++ b/sdk/test/configuration/sdk_builder_test.cc @@ -112,11 +112,9 @@ #include "opentelemetry/trace/trace_id.h" #if defined(ENABLE_METRICS_EXEMPLAR_PREVIEW) && !defined(NO_GETENV) -# include "opentelemetry/sdk/common/global_log_handler.h" # include "opentelemetry/sdk/configuration/exemplar_filter.h" # include "opentelemetry/sdk/configuration/meter_provider_configuration.h" # include "opentelemetry/sdk/metrics/meter_provider.h" -# include "opentelemetry/test_common/sdk/common/scoped_test_log_handler.h" # if defined(_MSC_VER) # include "opentelemetry/sdk/common/env_variables.h" From 54fb9fcbb5021be2fbaf62992660974aa51f60a3 Mon Sep 17 00:00:00 2001 From: Om Kulkarni Date: Wed, 16 Sep 2026 19:08:53 +0530 Subject: [PATCH 20/21] Fix include order in logs_builder_utils.cc clang-format expects includes sorted alphabetically; the newly added event_to_span_event_bridge_log_record_processor_configuration.h include was placed after extension_log_record_processor_builder.h instead of before extension_log_record_exporter_builder.h. --- sdk/src/configuration/logs_builder_utils.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/configuration/logs_builder_utils.cc b/sdk/src/configuration/logs_builder_utils.cc index 1e9a5dceb1..31d9f0bc27 100644 --- a/sdk/src/configuration/logs_builder_utils.cc +++ b/sdk/src/configuration/logs_builder_utils.cc @@ -14,10 +14,10 @@ #include "opentelemetry/sdk/configuration/batch_log_record_processor_configuration.h" #include "opentelemetry/sdk/configuration/console_log_record_exporter_builder.h" #include "opentelemetry/sdk/configuration/console_log_record_exporter_configuration.h" +#include "opentelemetry/sdk/configuration/event_to_span_event_bridge_log_record_processor_configuration.h" #include "opentelemetry/sdk/configuration/extension_log_record_exporter_builder.h" #include "opentelemetry/sdk/configuration/extension_log_record_exporter_configuration.h" #include "opentelemetry/sdk/configuration/extension_log_record_processor_builder.h" -#include "opentelemetry/sdk/configuration/event_to_span_event_bridge_log_record_processor_configuration.h" #include "opentelemetry/sdk/configuration/extension_log_record_processor_configuration.h" #include "opentelemetry/sdk/configuration/log_record_exporter_configuration.h" #include "opentelemetry/sdk/configuration/log_record_exporter_configuration_visitor.h" From 4df009b889c853b3669be3e390ede2ae699a8c2d Mon Sep 17 00:00:00 2001 From: Om Kulkarni Date: Mon, 5 Oct 2026 23:31:12 +0530 Subject: [PATCH 21/21] Remove extra blank line in changelog --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f458910b22..d3f9091319 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,7 +63,6 @@ Increment the: warning and skips it when configured. [#4309](https://github.com/open-telemetry/opentelemetry-cpp/pull/4309) - Breaking changes: * [SDK] Remove `noexcept` from the public SDK `TracerProvider`,