From aa55bea25b04f478f2633a7d3c321ddd6d791de8 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Mon, 19 Jul 2021 20:31:12 +0530 Subject: [PATCH 1/4] reorder member variable definition --- examples/batch/main.cc | 6 +++++- sdk/include/opentelemetry/sdk/resource/resource.h | 4 ++++ sdk/include/opentelemetry/sdk/trace/tracer.h | 2 +- sdk/include/opentelemetry/sdk/trace/tracer_context.h | 3 +-- sdk/include/opentelemetry/sdk/trace/tracer_provider.h | 2 +- sdk/src/trace/batch_span_processor.cc | 1 + sdk/src/trace/tracer.cc | 2 +- sdk/src/trace/tracer_context.cc | 6 +++--- 8 files changed, 17 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/resource/resource.h b/sdk/include/opentelemetry/sdk/resource/resource.h index 03aebcd84a..a820d4cb4e 100644 --- a/sdk/include/opentelemetry/sdk/resource/resource.h +++ b/sdk/include/opentelemetry/sdk/resource/resource.h @@ -15,6 +15,8 @@ #include #include +#include + OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { @@ -62,6 +64,8 @@ class Resource static Resource &GetDefault(); + ~Resource() { std::cout << "\n resource Shutdown called " << std::hex << this << "\n"; } + protected: /** * The constructor is protected and only for use internally by the class and diff --git a/sdk/include/opentelemetry/sdk/trace/tracer.h b/sdk/include/opentelemetry/sdk/trace/tracer.h index 6aeb19fa57..df28379ccb 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer.h @@ -60,8 +60,8 @@ class Tracer final : public trace_api::Tracer, public std::enable_shared_from_th Sampler &GetSampler() { return context_->GetSampler(); } private: - std::shared_ptr 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..bd093b701a 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_context.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_context.h @@ -88,11 +88,10 @@ class TracerContext bool Shutdown() noexcept; private: - // This is an atomic pointer so we can adapt the processor pipeline dynamically. - std::unique_ptr 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..ae88d954d2 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h @@ -96,8 +96,8 @@ class TracerProvider final : public opentelemetry::trace::TracerProvider bool ForceFlush(std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept; private: - std::shared_ptr context_; std::vector> tracers_; + std::shared_ptr context_; std::mutex lock_; }; } // namespace trace diff --git a/sdk/src/trace/batch_span_processor.cc b/sdk/src/trace/batch_span_processor.cc index e9fdef7647..47dda1ac80 100644 --- a/sdk/src/trace/batch_span_processor.cc +++ b/sdk/src/trace/batch_span_processor.cc @@ -194,6 +194,7 @@ bool BatchSpanProcessor::Shutdown(std::chrono::microseconds timeout) noexcept BatchSpanProcessor::~BatchSpanProcessor() { + std::cout << " \n batchshutdown called\n"; if (is_shutdown_.load() == false) { Shutdown(); 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 From 1927b06985358bf47a41bd3f4264352d18debe29 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Mon, 19 Jul 2021 20:39:42 +0530 Subject: [PATCH 2/4] remove debug --- sdk/include/opentelemetry/sdk/resource/resource.h | 2 -- sdk/src/trace/batch_span_processor.cc | 1 - 2 files changed, 3 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/resource/resource.h b/sdk/include/opentelemetry/sdk/resource/resource.h index a820d4cb4e..482fb3e00a 100644 --- a/sdk/include/opentelemetry/sdk/resource/resource.h +++ b/sdk/include/opentelemetry/sdk/resource/resource.h @@ -64,8 +64,6 @@ class Resource static Resource &GetDefault(); - ~Resource() { std::cout << "\n resource Shutdown called " << std::hex << this << "\n"; } - protected: /** * The constructor is protected and only for use internally by the class and diff --git a/sdk/src/trace/batch_span_processor.cc b/sdk/src/trace/batch_span_processor.cc index 47dda1ac80..e9fdef7647 100644 --- a/sdk/src/trace/batch_span_processor.cc +++ b/sdk/src/trace/batch_span_processor.cc @@ -194,7 +194,6 @@ bool BatchSpanProcessor::Shutdown(std::chrono::microseconds timeout) noexcept BatchSpanProcessor::~BatchSpanProcessor() { - std::cout << " \n batchshutdown called\n"; if (is_shutdown_.load() == false) { Shutdown(); From 41e0b0095fd9fa0a5fa406e00b4cd3cea62d1f18 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Mon, 19 Jul 2021 20:42:23 +0530 Subject: [PATCH 3/4] remove iostream header --- sdk/include/opentelemetry/sdk/resource/resource.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/resource/resource.h b/sdk/include/opentelemetry/sdk/resource/resource.h index 482fb3e00a..03aebcd84a 100644 --- a/sdk/include/opentelemetry/sdk/resource/resource.h +++ b/sdk/include/opentelemetry/sdk/resource/resource.h @@ -15,8 +15,6 @@ #include #include -#include - OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { From 96fdcc4be59cb8d532b57d81e2ab40d43994be5b Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Wed, 21 Jul 2021 23:37:30 +0530 Subject: [PATCH 4/4] document the ordering of declaration --- sdk/include/opentelemetry/sdk/trace/tracer.h | 2 ++ sdk/include/opentelemetry/sdk/trace/tracer_context.h | 1 + sdk/include/opentelemetry/sdk/trace/tracer_provider.h | 1 + 3 files changed, 4 insertions(+) diff --git a/sdk/include/opentelemetry/sdk/trace/tracer.h b/sdk/include/opentelemetry/sdk/trace/tracer.h index df28379ccb..fd9f2c8e8d 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer.h @@ -60,6 +60,8 @@ class Tracer final : public trace_api::Tracer, public std::enable_shared_from_th Sampler &GetSampler() { return context_->GetSampler(); } private: + // order of declaration is important here - instrumentation library should destroy after + // tracer-context. std::shared_ptr instrumentation_library_; std::shared_ptr context_; }; diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_context.h b/sdk/include/opentelemetry/sdk/trace/tracer_context.h index bd093b701a..572f60cafe 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_context.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_context.h @@ -88,6 +88,7 @@ class TracerContext bool Shutdown() noexcept; private: + // 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_; diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h index ae88d954d2..ec825b6e3c 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h @@ -96,6 +96,7 @@ class TracerProvider final : public opentelemetry::trace::TracerProvider bool ForceFlush(std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept; private: + // order of declaration is important here - tracers should destroy only after context. std::vector> tracers_; std::shared_ptr context_; std::mutex lock_;