diff --git a/sdk/include/opentelemetry/sdk/trace/multi_span_processor.h b/sdk/include/opentelemetry/sdk/trace/multi_span_processor.h index 7b0172dc8e..85caf5f1a1 100644 --- a/sdk/include/opentelemetry/sdk/trace/multi_span_processor.h +++ b/sdk/include/opentelemetry/sdk/trace/multi_span_processor.h @@ -194,6 +194,9 @@ class MultiSpanProcessor : public SpanProcessor ProcessorNode *head_{nullptr}; ProcessorNode *tail_{nullptr}; size_t count_{0}; + + // For testing + friend class MultiSpanProcessorTestPeer; }; } // namespace trace } // namespace sdk diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_context.h b/sdk/include/opentelemetry/sdk/trace/tracer_context.h index 518c1cbdf6..8d3d226790 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_context.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_context.h @@ -23,6 +23,9 @@ namespace sdk namespace trace { +// forward declare to be able to have a raw pointer to it +class MultiSpanProcessor; + /** * A class which stores the TracerProvider context. * @@ -129,6 +132,8 @@ class TracerContext std::unique_ptr id_generator_; std::unique_ptr processor_; std::unique_ptr> tracer_configurator_; + // shares the pointer with processor_ if it is a MultiSpanProcessor, null otherwise + MultiSpanProcessor *multi_processor_; }; } // namespace trace diff --git a/sdk/src/trace/span.cc b/sdk/src/trace/span.cc index d764540875..9c31930600 100644 --- a/sdk/src/trace/span.cc +++ b/sdk/src/trace/span.cc @@ -89,7 +89,11 @@ Span::Span(std::shared_ptr &&tracer, recordable_->SetStartTime(NowOr(options.start_system_time)); start_steady_time = NowOr(options.start_steady_time); recordable_->SetResource(tracer_->GetResource()); - tracer_->GetProcessor().OnStart(*recordable_, parent_span_context); + + // store the span processor, this is important to use the same processor even + // in case the tracer changes from single processor to a MultiSpanProcessor + span_processor_ = &tracer_->GetProcessor(); + span_processor_->OnStart(*recordable_, parent_span_context); } Span::~Span() @@ -203,6 +207,8 @@ void Span::End(const opentelemetry::trace::EndSpanOptions &options) noexcept { std::lock_guard lock_guard{mu_}; + // has_ended_ could be removed and instead rely on span_processor_, + // resetting it after calling OnEnd. it would save 8 bytes in each Span object. if (has_ended_ == true) { return; @@ -218,7 +224,7 @@ void Span::End(const opentelemetry::trace::EndSpanOptions &options) noexcept recordable_->SetDuration(std::chrono::steady_clock::time_point(end_steady_time) - std::chrono::steady_clock::time_point(start_steady_time)); - tracer_->GetProcessor().OnEnd(std::move(recordable_)); + span_processor_->OnEnd(std::move(recordable_)); recordable_.reset(); } diff --git a/sdk/src/trace/span.h b/sdk/src/trace/span.h index 2a5614821d..e9bd1fd88a 100644 --- a/sdk/src/trace/span.h +++ b/sdk/src/trace/span.h @@ -10,6 +10,7 @@ #include "opentelemetry/common/key_value_iterable.h" #include "opentelemetry/common/timestamp.h" #include "opentelemetry/nostd/string_view.h" +#include "opentelemetry/sdk/trace/processor.h" #include "opentelemetry/sdk/trace/recordable.h" #include "opentelemetry/sdk/trace/tracer.h" #include "opentelemetry/trace/span.h" @@ -79,11 +80,12 @@ class Span final : public opentelemetry::trace::Span } private: - std::shared_ptr tracer_; + std::shared_ptr tracer_; // also keeps span_processor_ alive mutable std::mutex mu_; std::unique_ptr recordable_; opentelemetry::common::SteadyTimestamp start_steady_time; std::unique_ptr span_context_; + SpanProcessor *span_processor_{nullptr}; // kept alive by tracer_ bool has_ended_{false}; }; } // namespace trace diff --git a/sdk/src/trace/tracer_context.cc b/sdk/src/trace/tracer_context.cc index adb3cfe538..dd1eb5eaf7 100644 --- a/sdk/src/trace/tracer_context.cc +++ b/sdk/src/trace/tracer_context.cc @@ -33,9 +33,22 @@ TracerContext::TracerContext(std::vector> &&proce : resource_(resource), sampler_(std::move(sampler)), id_generator_(std::move(id_generator)), - processor_(std::unique_ptr(new MultiSpanProcessor(std::move(processors)))), - tracer_configurator_(std::move(tracer_configurator)) -{} + tracer_configurator_(std::move(tracer_configurator)), + multi_processor_(nullptr) +{ + if (processors.empty()) + { + processor_ = std::unique_ptr(new MultiSpanProcessor(std::move(processors))); + } + else + { + // at least one processor is available here + for (auto &&processor : processors) + { + AddProcessor(std::move(processor)); + } + } +} Sampler &TracerContext::GetSampler() const noexcept { @@ -60,9 +73,31 @@ opentelemetry::sdk::trace::IdGenerator &TracerContext::GetIdGenerator() const no void TracerContext::AddProcessor(std::unique_ptr processor) noexcept { + if (!processor) + { + return; + } + + if (!processor_) + { + // this is the first processor to be added + processor_ = std::move(processor); + } + else if (multi_processor_ == nullptr) + { + // a processor exists, but it's not a MultiSpanProcessor. make a new MultiSpanProcessor + multi_processor_ = new MultiSpanProcessor({}); + std::unique_ptr multi_processor(multi_processor_); + multi_processor->AddProcessor(std::move(processor_)); + multi_processor->AddProcessor(std::move(processor)); - auto multi_processor = static_cast(processor_.get()); - multi_processor->AddProcessor(std::move(processor)); + processor_ = std::move(multi_processor); + } + else /*if (multi_processor_ != nullptr)*/ + { + // already have a MultiSpanProcessor, add the processor to it + multi_processor_->AddProcessor(std::move(processor)); + } } void TracerContext::SetTracerConfigurator( diff --git a/sdk/test/trace/tracer_provider_test.cc b/sdk/test/trace/tracer_provider_test.cc index 687f63d07f..0ae7767672 100644 --- a/sdk/test/trace/tracer_provider_test.cc +++ b/sdk/test/trace/tracer_provider_test.cc @@ -19,6 +19,7 @@ #include "opentelemetry/sdk/resource/resource.h" #include "opentelemetry/sdk/trace/exporter.h" #include "opentelemetry/sdk/trace/id_generator.h" +#include "opentelemetry/sdk/trace/multi_span_processor.h" #include "opentelemetry/sdk/trace/processor.h" #include "opentelemetry/sdk/trace/random_id_generator.h" #include "opentelemetry/sdk/trace/sampler.h" @@ -49,6 +50,33 @@ using namespace opentelemetry::sdk::trace; using namespace opentelemetry::sdk::resource; using opentelemetry::sdk::instrumentationscope::ScopeConfigurator; +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace trace +{ +class MultiSpanProcessorTestPeer +{ +public: + static std::vector GetProcessors(MultiSpanProcessor *multi_span_processor) + { + std::vector res; + + MultiSpanProcessor::ProcessorNode *node = multi_span_processor->head_; + while (node != nullptr) + { + auto processor = node->value_.get(); + res.emplace_back(processor); + node = node->next_; + } + + return res; + } +}; +} // namespace trace +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE + TEST(TracerProvider, GetTracer) { std::unique_ptr processor(new SimpleSpanProcessor(nullptr)); @@ -329,6 +357,90 @@ TEST(TracerProvider, GetTracerAbiv2) } #endif /* OPENTELEMETRY_ABI_VERSION_NO >= 2 */ +// get the same processor back, not wrapped in a MultiSpanProcessor +TEST(TracerProvider, GetProcessor) +{ + std::unique_ptr processor(new SimpleSpanProcessor(nullptr)); + std::vector> processors; + processors.push_back(std::move(processor)); + + std::unique_ptr context1(new TracerContext(std::move(processors))); + + auto &span_processor = context1->GetProcessor(); + + // Should be the SimpleSpanProcessor processor that was created above. +#ifdef OPENTELEMETRY_RTTI_ENABLED + auto processor_typeed = dynamic_cast(&span_processor); +#else + auto processor_typeed = static_cast(&span_processor); +#endif + ASSERT_NE(nullptr, processor_typeed); +} + +// get a MultiSpanProcessor back that wraps both processors +TEST(TracerProvider, GetProcessorsTwo) +{ + std::vector processors_raw(2); + processors_raw[0] = new SimpleSpanProcessor(nullptr); // deleted via unique_ptr + processors_raw[1] = new SimpleSpanProcessor(nullptr); // deleted via unique_ptr + + std::unique_ptr processor1(processors_raw[0]); + std::unique_ptr processor2(processors_raw[1]); + + std::vector> processors; + processors.push_back(std::move(processor1)); + processors.push_back(std::move(processor2)); + + std::unique_ptr context1(new TracerContext(std::move(processors))); + + auto &span_processor = context1->GetProcessor(); + + // Should be the SimpleSpanProcessor processor that was created above. +#ifdef OPENTELEMETRY_RTTI_ENABLED + auto processor_typeed = dynamic_cast(&span_processor); +#else + auto processor_typeed = static_cast(&span_processor); +#endif + ASSERT_NE(nullptr, processor_typeed); + + std::vector contained_processors = + MultiSpanProcessorTestPeer::GetProcessors(processor_typeed); + EXPECT_EQ(processors_raw, contained_processors); +} + +// get a MultiSpanProcessor back that wraps all three processors +TEST(TracerProvider, GetProcessorsThree) +{ + std::vector processors_raw(3); + processors_raw[0] = new SimpleSpanProcessor(nullptr); // deleted via unique_ptr + processors_raw[1] = new SimpleSpanProcessor(nullptr); // deleted via unique_ptr + processors_raw[2] = new SimpleSpanProcessor(nullptr); // deleted via unique_ptr + + std::unique_ptr processor1(processors_raw[0]); + std::unique_ptr processor2(processors_raw[1]); + std::unique_ptr processor3(processors_raw[2]); + std::vector> processors; + processors.push_back(std::move(processor1)); + processors.push_back(std::move(processor2)); + processors.push_back(std::move(processor3)); + + std::unique_ptr context1(new TracerContext(std::move(processors))); + + auto &span_processor = context1->GetProcessor(); + + // Should be the SimpleSpanProcessor processor that was created above. +#ifdef OPENTELEMETRY_RTTI_ENABLED + auto processor_typeed = dynamic_cast(&span_processor); +#else + auto processor_typeed = static_cast(&span_processor); +#endif + ASSERT_NE(nullptr, processor_typeed); + + std::vector contained_processors = + MultiSpanProcessorTestPeer::GetProcessors(processor_typeed); + EXPECT_EQ(processors_raw, contained_processors); +} + TEST(TracerProvider, Shutdown) { std::unique_ptr processor(new SimpleSpanProcessor(nullptr));