From 083cf026ac3b0bff7f98ceef8bf8e4b9a3fe4238 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 22 Jul 2021 07:32:37 +0530 Subject: [PATCH] Fix for resource deletion after tracer provider shutdown (#911) --- examples/batch/main.cc | 6 +++++- sdk/include/opentelemetry/sdk/trace/tracer.h | 4 +++- sdk/include/opentelemetry/sdk/trace/tracer_context.h | 4 ++-- sdk/include/opentelemetry/sdk/trace/tracer_provider.h | 3 ++- sdk/src/trace/tracer.cc | 2 +- sdk/src/trace/tracer_context.cc | 6 +++--- 6 files changed, 16 insertions(+), 9 deletions(-) diff --git a/examples/batch/main.cc b/examples/batch/main.cc index 5c007dfbc7..3c9a168071 100644 --- a/examples/batch/main.cc +++ b/examples/batch/main.cc @@ -31,11 +31,15 @@ void initTracer() // We export `kNumSpans` after every `schedule_delay_millis` milliseconds. options.max_export_batch_size = kNumSpans; + opentelemetry::sdk::resource::ResourceAttributes attributes = {{"service", "test_service"}, + {"version", (uint32_t)1}}; + auto resource = opentelemetry::sdk::resource::Resource::Create(attributes); + auto processor = std::unique_ptr( new sdktrace::BatchSpanProcessor(std::move(exporter), options)); auto provider = nostd::shared_ptr( - new sdktrace::TracerProvider(std::move(processor))); + new sdktrace::TracerProvider(std::move(processor), resource)); // Set the global trace provider. opentelemetry::trace::Provider::SetTracerProvider(provider); } diff --git a/sdk/include/opentelemetry/sdk/trace/tracer.h b/sdk/include/opentelemetry/sdk/trace/tracer.h index 6aeb19fa57..fd9f2c8e8d 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer.h @@ -60,8 +60,10 @@ class Tracer final : public trace_api::Tracer, public std::enable_shared_from_th Sampler &GetSampler() { return context_->GetSampler(); } private: - std::shared_ptr context_; + // order of declaration is important here - instrumentation library should destroy after + // tracer-context. std::shared_ptr instrumentation_library_; + std::shared_ptr context_; }; } // 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 e4d968d2b0..572f60cafe 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_context.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_context.h @@ -88,11 +88,11 @@ class TracerContext bool Shutdown() noexcept; private: - // This is an atomic pointer so we can adapt the processor pipeline dynamically. - std::unique_ptr processor_; + // order of declaration is important here - resource object should be destroyed after processor. opentelemetry::sdk::resource::Resource resource_; std::unique_ptr sampler_; std::unique_ptr id_generator_; + std::unique_ptr processor_; }; } // namespace trace diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h index bd131704d5..ec825b6e3c 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h @@ -96,8 +96,9 @@ class TracerProvider final : public opentelemetry::trace::TracerProvider bool ForceFlush(std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept; private: - std::shared_ptr context_; + // order of declaration is important here - tracers should destroy only after context. std::vector> tracers_; + std::shared_ptr context_; std::mutex lock_; }; } // namespace trace diff --git a/sdk/src/trace/tracer.cc b/sdk/src/trace/tracer.cc index c84d847bd9..977722d0ca 100644 --- a/sdk/src/trace/tracer.cc +++ b/sdk/src/trace/tracer.cc @@ -18,7 +18,7 @@ namespace trace Tracer::Tracer(std::shared_ptr context, std::unique_ptr instrumentation_library) noexcept - : context_{context}, instrumentation_library_{std::move(instrumentation_library)} + : instrumentation_library_{std::move(instrumentation_library)}, context_{context} {} nostd::shared_ptr Tracer::StartSpan( diff --git a/sdk/src/trace/tracer_context.cc b/sdk/src/trace/tracer_context.cc index 196cecd6a0..eaab96154c 100644 --- a/sdk/src/trace/tracer_context.cc +++ b/sdk/src/trace/tracer_context.cc @@ -14,10 +14,10 @@ TracerContext::TracerContext(std::vector> &&proce opentelemetry::sdk::resource::Resource resource, std::unique_ptr sampler, std::unique_ptr id_generator) noexcept - : processor_(std::unique_ptr(new MultiSpanProcessor(std::move(processors)))), - resource_(resource), + : resource_(resource), sampler_(std::move(sampler)), - id_generator_(std::move(id_generator)) + id_generator_(std::move(id_generator)), + processor_(std::unique_ptr(new MultiSpanProcessor(std::move(processors)))) {} Sampler &TracerContext::GetSampler() const noexcept