From ae5cdf4f7d86f84a3b9fb00be1a86751279094f4 Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Fri, 8 Nov 2024 22:27:17 +0000 Subject: [PATCH 01/32] Add tracer_config as per spec --- .../opentelemetry/sdk/trace/tracer_config.h | 36 +++++++++++++++++++ sdk/src/trace/CMakeLists.txt | 4 ++- sdk/src/trace/tracer_config.cc | 35 ++++++++++++++++++ 3 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 sdk/include/opentelemetry/sdk/trace/tracer_config.h create mode 100644 sdk/src/trace/tracer_config.cc diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_config.h b/sdk/include/opentelemetry/sdk/trace/tracer_config.h new file mode 100644 index 0000000000..0aac91b4d2 --- /dev/null +++ b/sdk/include/opentelemetry/sdk/trace/tracer_config.h @@ -0,0 +1,36 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +#include "opentelemetry/version.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace trace +{ +/** + * TracerConfig defines various configurable aspects of a Tracer's behavior. + */ +class TracerConfig { +public: + /** + * Returns if the Tracer is enabled or disabled. Tracers are enabled by default. + * @return a boolean indicating if the Tracer is enabled. Defaults to true. + */ + bool IsEnabled() const noexcept; + + static TracerConfig Disabled(); + static TracerConfig Enabled(); + static TracerConfig Default(); + +private: + explicit TracerConfig(const bool disabled = false) : disabled_(disabled) {} + bool disabled_; + static TracerConfig kDefaultConfig; + static TracerConfig kDisabledConfig; +}; +} // namespace trace +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE \ No newline at end of file diff --git a/sdk/src/trace/CMakeLists.txt b/sdk/src/trace/CMakeLists.txt index 939a6ed2bc..ec975fd2b7 100644 --- a/sdk/src/trace/CMakeLists.txt +++ b/sdk/src/trace/CMakeLists.txt @@ -20,7 +20,9 @@ add_library( samplers/trace_id_ratio.cc samplers/trace_id_ratio_factory.cc random_id_generator.cc - random_id_generator_factory.cc) + random_id_generator_factory.cc + tracer_config.cc +) set_target_properties(opentelemetry_trace PROPERTIES EXPORT_NAME trace) set_target_version(opentelemetry_trace) diff --git a/sdk/src/trace/tracer_config.cc b/sdk/src/trace/tracer_config.cc new file mode 100644 index 0000000000..540d91ef3a --- /dev/null +++ b/sdk/src/trace/tracer_config.cc @@ -0,0 +1,35 @@ +// +// Created by sharmapranav on 11/8/24. +// + +#include "opentelemetry/sdk/trace/tracer_config.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace trace +{ + +TracerConfig TracerConfig::kDefaultConfig = TracerConfig(); +TracerConfig TracerConfig::kDisabledConfig = TracerConfig(true); + +TracerConfig TracerConfig::Disabled() { + return kDisabledConfig; +} + +TracerConfig TracerConfig::Enabled() { + return kDefaultConfig; +} + +TracerConfig TracerConfig::Default() +{ + return kDefaultConfig; +} + +bool TracerConfig::IsEnabled() const noexcept +{ + return !disabled_; +} +}// namespace trace +}// namespace sdk +OPENTELEMETRY_END_NAMESPACE \ No newline at end of file From 9117eff1315a4caf21488e027a6a49163bfa5617 Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Mon, 11 Nov 2024 17:11:59 +0000 Subject: [PATCH 02/32] Update formatting --- sdk/include/opentelemetry/sdk/trace/tracer.h | 5 ++++- sdk/include/opentelemetry/sdk/trace/tracer_config.h | 7 ++++--- .../opentelemetry/sdk/trace/tracer_provider.h | 8 ++++++++ sdk/src/trace/tracer.cc | 7 +++++-- sdk/src/trace/tracer_config.cc | 12 +++++++----- sdk/src/trace/tracer_provider.cc | 4 +--- 6 files changed, 29 insertions(+), 14 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/trace/tracer.h b/sdk/include/opentelemetry/sdk/trace/tracer.h index f738203b4f..7f8f11ecb3 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer.h @@ -13,6 +13,7 @@ #include "opentelemetry/sdk/trace/id_generator.h" #include "opentelemetry/sdk/trace/processor.h" #include "opentelemetry/sdk/trace/sampler.h" +#include "opentelemetry/sdk/trace/tracer_config.h" #include "opentelemetry/sdk/trace/tracer_context.h" #include "opentelemetry/trace/span.h" #include "opentelemetry/trace/span_context_kv_iterable.h" @@ -35,7 +36,8 @@ class Tracer final : public opentelemetry::trace::Tracer, /** Construct a new Tracer with the given context pipeline. */ explicit Tracer(std::shared_ptr context, std::unique_ptr instrumentation_scope = - InstrumentationScope::Create("")) noexcept; + InstrumentationScope::Create(""), + TracerConfig tracer_config = TracerConfig::Default()) noexcept; nostd::shared_ptr StartSpan( nostd::string_view name, @@ -105,6 +107,7 @@ class Tracer final : public opentelemetry::trace::Tracer, // tracer-context. std::shared_ptr instrumentation_scope_; std::shared_ptr context_; + TracerConfig tracer_config_; }; } // namespace trace } // namespace sdk diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_config.h b/sdk/include/opentelemetry/sdk/trace/tracer_config.h index 0aac91b4d2..ae8b2248f7 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_config.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_config.h @@ -13,7 +13,8 @@ namespace trace /** * TracerConfig defines various configurable aspects of a Tracer's behavior. */ -class TracerConfig { +class TracerConfig +{ public: /** * Returns if the Tracer is enabled or disabled. Tracers are enabled by default. @@ -31,6 +32,6 @@ class TracerConfig { static TracerConfig kDefaultConfig; static TracerConfig kDisabledConfig; }; -} // namespace trace -} // namespace sdk +} // namespace trace +} // namespace sdk OPENTELEMETRY_END_NAMESPACE \ No newline at end of file diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h index 44d5840558..55e996ebdf 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h @@ -56,6 +56,14 @@ class OPENTELEMETRY_EXPORT TracerProvider final : public opentelemetry::trace::T std::unique_ptr id_generator = std::unique_ptr(new RandomIdGenerator())) noexcept; + // explicit TracerProvider( + // std::vector> &&processors, + // const opentelemetry::sdk::resource::Resource &resource = + // opentelemetry::sdk::resource::Resource::Create({}), + // std::unique_ptr sampler = std::unique_ptr(new AlwaysOnSampler), + // std::unique_ptr id_generator = + // std::unique_ptr(new RandomIdGenerator())) noexcept; + /** * Initialize a new tracer provider with a specified context * @param context The owned tracer configuration/pipeline for this provider. diff --git a/sdk/src/trace/tracer.cc b/sdk/src/trace/tracer.cc index 73fc08d5dc..a803dc6880 100644 --- a/sdk/src/trace/tracer.cc +++ b/sdk/src/trace/tracer.cc @@ -38,8 +38,11 @@ namespace trace { Tracer::Tracer(std::shared_ptr context, - std::unique_ptr instrumentation_scope) noexcept - : instrumentation_scope_{std::move(instrumentation_scope)}, context_{std::move(context)} + std::unique_ptr instrumentation_scope, + TracerConfig tracer_config) noexcept + : instrumentation_scope_{std::move(instrumentation_scope)}, + context_{std::move(context)}, + tracer_config_(tracer_config) {} nostd::shared_ptr Tracer::StartSpan( diff --git a/sdk/src/trace/tracer_config.cc b/sdk/src/trace/tracer_config.cc index 540d91ef3a..3f66c2670c 100644 --- a/sdk/src/trace/tracer_config.cc +++ b/sdk/src/trace/tracer_config.cc @@ -10,14 +10,16 @@ namespace sdk namespace trace { -TracerConfig TracerConfig::kDefaultConfig = TracerConfig(); +TracerConfig TracerConfig::kDefaultConfig = TracerConfig(); TracerConfig TracerConfig::kDisabledConfig = TracerConfig(true); -TracerConfig TracerConfig::Disabled() { +TracerConfig TracerConfig::Disabled() +{ return kDisabledConfig; } -TracerConfig TracerConfig::Enabled() { +TracerConfig TracerConfig::Enabled() +{ return kDefaultConfig; } @@ -30,6 +32,6 @@ bool TracerConfig::IsEnabled() const noexcept { return !disabled_; } -}// namespace trace -}// namespace sdk +} // namespace trace +} // namespace sdk OPENTELEMETRY_END_NAMESPACE \ No newline at end of file diff --git a/sdk/src/trace/tracer_provider.cc b/sdk/src/trace/tracer_provider.cc index 3b5462a083..bda0ab2928 100644 --- a/sdk/src/trace/tracer_provider.cc +++ b/sdk/src/trace/tracer_provider.cc @@ -1,7 +1,6 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -#include #include #include #include @@ -19,7 +18,6 @@ #include "opentelemetry/sdk/trace/tracer.h" #include "opentelemetry/sdk/trace/tracer_context.h" #include "opentelemetry/sdk/trace/tracer_provider.h" -#include "opentelemetry/trace/span_id.h" #include "opentelemetry/trace/tracer.h" #include "opentelemetry/version.h" @@ -60,7 +58,7 @@ TracerProvider::TracerProvider(std::vector> &&pro TracerProvider::~TracerProvider() { // Tracer hold the shared pointer to the context. So we can not use destructor of TracerContext to - // Shutdown and flush all pending recordables when we have more than one tracers.These recordables + // Shutdown and flush all pending recordables when we have more than one tracer.These recordables // may use the raw pointer of instrumentation_scope_ in Tracer if (context_) { From 127311b1be25be9d0941945d02ead54c0fda02a0 Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Tue, 12 Nov 2024 23:34:29 +0000 Subject: [PATCH 03/32] TracerProvider uses TracerConfig as per spec --- .../instrumentationscope/scope_configurator.h | 39 +++++++++++++++++ sdk/include/opentelemetry/sdk/trace/tracer.h | 3 +- .../opentelemetry/sdk/trace/tracer_config.h | 9 ++-- .../opentelemetry/sdk/trace/tracer_context.h | 12 +++++- .../opentelemetry/sdk/trace/tracer_provider.h | 26 +++++++----- sdk/src/trace/CMakeLists.txt | 3 +- sdk/src/trace/tracer.cc | 8 +++- sdk/src/trace/tracer_config.cc | 21 +++++++--- sdk/src/trace/tracer_context.cc | 18 +++++--- sdk/src/trace/tracer_provider.cc | 42 +++++++++++++------ 10 files changed, 138 insertions(+), 43 deletions(-) create mode 100644 sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h diff --git a/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h b/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h new file mode 100644 index 0000000000..6ffac33bca --- /dev/null +++ b/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h @@ -0,0 +1,39 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +#include +#include "instrumentation_scope.h" +#include "opentelemetry/version.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace instrumentationscope +{ +template +class ScopeConfigurator +{ +public: + static nostd::unique_ptr Create( + std::function scope_configurator) + { + return nostd::unique_ptr(new ScopeConfigurator(scope_configurator)); + } + + T ComputeConfig(const InstrumentationScope &instrumentation_scope) + { + return scope_configurator_(instrumentation_scope); + } + +private: + explicit ScopeConfigurator( + const std::function scope_configurator) + : scope_configurator_(scope_configurator) + {} + const std::function scope_configurator_; +}; +} // namespace instrumentationscope +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/trace/tracer.h b/sdk/include/opentelemetry/sdk/trace/tracer.h index 7f8f11ecb3..77c1fd0861 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer.h @@ -15,8 +15,8 @@ #include "opentelemetry/sdk/trace/sampler.h" #include "opentelemetry/sdk/trace/tracer_config.h" #include "opentelemetry/sdk/trace/tracer_context.h" +#include "opentelemetry/trace/noop.h" #include "opentelemetry/trace/span.h" -#include "opentelemetry/trace/span_context_kv_iterable.h" #include "opentelemetry/trace/span_startoptions.h" #include "opentelemetry/trace/tracer.h" #include "opentelemetry/version.h" @@ -108,6 +108,7 @@ class Tracer final : public opentelemetry::trace::Tracer, std::shared_ptr instrumentation_scope_; std::shared_ptr context_; TracerConfig tracer_config_; + static const std::shared_ptr kNoopTracer; }; } // namespace trace } // namespace sdk diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_config.h b/sdk/include/opentelemetry/sdk/trace/tracer_config.h index ae8b2248f7..eb8f4a80c3 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_config.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_config.h @@ -3,6 +3,7 @@ #pragma once +#include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" #include "opentelemetry/version.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -25,13 +26,15 @@ class TracerConfig static TracerConfig Disabled(); static TracerConfig Enabled(); static TracerConfig Default(); + static const instrumentationscope::ScopeConfigurator &DefaultConfigurator(); private: explicit TracerConfig(const bool disabled = false) : disabled_(disabled) {} bool disabled_; - static TracerConfig kDefaultConfig; - static TracerConfig kDisabledConfig; + static const TracerConfig kDefaultConfig; + static const TracerConfig kDisabledConfig; + static const instrumentationscope::ScopeConfigurator kDefaultTracerConfigurator; }; } // namespace trace } // namespace sdk -OPENTELEMETRY_END_NAMESPACE \ No newline at end of file +OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_context.h b/sdk/include/opentelemetry/sdk/trace/tracer_context.h index e1b2cb25d2..b49d45444f 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_context.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_context.h @@ -7,12 +7,14 @@ #include #include +#include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" #include "opentelemetry/sdk/resource/resource.h" #include "opentelemetry/sdk/trace/id_generator.h" #include "opentelemetry/sdk/trace/processor.h" #include "opentelemetry/sdk/trace/random_id_generator.h" #include "opentelemetry/sdk/trace/sampler.h" #include "opentelemetry/sdk/trace/samplers/always_on.h" +#include "opentelemetry/sdk/trace/tracer_config.h" #include "opentelemetry/version.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -21,6 +23,8 @@ namespace sdk namespace trace { +using namespace opentelemetry::sdk::instrumentationscope; + /** * A class which stores the TracerProvider context. * @@ -43,7 +47,10 @@ class TracerContext opentelemetry::sdk::resource::Resource::Create({}), std::unique_ptr sampler = std::unique_ptr(new AlwaysOnSampler), std::unique_ptr id_generator = - std::unique_ptr(new RandomIdGenerator())) noexcept; + std::unique_ptr(new RandomIdGenerator()), + std::unique_ptr> tracer_configurator = + std::make_unique>( + TracerConfig::DefaultConfigurator())) noexcept; virtual ~TracerContext() = default; @@ -77,6 +84,8 @@ class TracerContext */ const opentelemetry::sdk::resource::Resource &GetResource() const noexcept; + ScopeConfigurator &GetTracerConfigurator() const noexcept; + /** * Obtain the Id Generator associated with this tracer context. * @return The ID Generator for this tracer context. @@ -100,6 +109,7 @@ class TracerContext std::unique_ptr sampler_; std::unique_ptr id_generator_; std::unique_ptr processor_; + std::unique_ptr> tracer_configurator_; }; } // namespace trace diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h index 55e996ebdf..fe73083584 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h @@ -9,6 +9,7 @@ #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/string_view.h" +#include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" #include "opentelemetry/sdk/resource/resource.h" #include "opentelemetry/sdk/trace/id_generator.h" #include "opentelemetry/sdk/trace/processor.h" @@ -27,6 +28,8 @@ namespace sdk namespace trace { +using namespace opentelemetry::sdk::instrumentationscope; + class OPENTELEMETRY_EXPORT TracerProvider final : public opentelemetry::trace::TracerProvider { public: @@ -39,6 +42,8 @@ class OPENTELEMETRY_EXPORT TracerProvider final : public opentelemetry::trace::T * not be a nullptr. * @param id_generator The custom id generator for this tracer provider. This must * not be a nullptr + * @param tracer_configurator Provides access to a function that computes the TracerConfig for + * Tracers provided by this TracerProvider. */ explicit TracerProvider( std::unique_ptr processor, @@ -46,7 +51,10 @@ class OPENTELEMETRY_EXPORT TracerProvider final : public opentelemetry::trace::T opentelemetry::sdk::resource::Resource::Create({}), std::unique_ptr sampler = std::unique_ptr(new AlwaysOnSampler), std::unique_ptr id_generator = - std::unique_ptr(new RandomIdGenerator())) noexcept; + std::unique_ptr(new RandomIdGenerator()), + std::unique_ptr> tracer_configurator = + std::make_unique>( + TracerConfig::DefaultConfigurator())) noexcept; explicit TracerProvider( std::vector> &&processors, @@ -54,15 +62,10 @@ class OPENTELEMETRY_EXPORT TracerProvider final : public opentelemetry::trace::T opentelemetry::sdk::resource::Resource::Create({}), std::unique_ptr sampler = std::unique_ptr(new AlwaysOnSampler), std::unique_ptr id_generator = - std::unique_ptr(new RandomIdGenerator())) noexcept; - - // explicit TracerProvider( - // std::vector> &&processors, - // const opentelemetry::sdk::resource::Resource &resource = - // opentelemetry::sdk::resource::Resource::Create({}), - // std::unique_ptr sampler = std::unique_ptr(new AlwaysOnSampler), - // std::unique_ptr id_generator = - // std::unique_ptr(new RandomIdGenerator())) noexcept; + std::unique_ptr(new RandomIdGenerator()), + std::unique_ptr> tracer_configurator = + std::make_unique>( + TracerConfig::DefaultConfigurator())) noexcept; /** * Initialize a new tracer provider with a specified context @@ -121,6 +124,9 @@ class OPENTELEMETRY_EXPORT TracerProvider final : public opentelemetry::trace::T std::vector> tracers_; std::shared_ptr context_; std::mutex lock_; + + // private helper to get TracerConfig from InstrumentationScope using tracer configurator. + TracerConfig GetTracerConfig(const InstrumentationScope &instrumentation_scope) const; }; } // namespace trace } // namespace sdk diff --git a/sdk/src/trace/CMakeLists.txt b/sdk/src/trace/CMakeLists.txt index ec975fd2b7..b99c2ca42c 100644 --- a/sdk/src/trace/CMakeLists.txt +++ b/sdk/src/trace/CMakeLists.txt @@ -21,8 +21,7 @@ add_library( samplers/trace_id_ratio_factory.cc random_id_generator.cc random_id_generator_factory.cc - tracer_config.cc -) + tracer_config.cc) set_target_properties(opentelemetry_trace PROPERTIES EXPORT_NAME trace) set_target_version(opentelemetry_trace) diff --git a/sdk/src/trace/tracer.cc b/sdk/src/trace/tracer.cc index a803dc6880..d4c6507839 100644 --- a/sdk/src/trace/tracer.cc +++ b/sdk/src/trace/tracer.cc @@ -3,7 +3,6 @@ #include #include -#include #include #include @@ -21,7 +20,6 @@ #include "opentelemetry/trace/noop.h" #include "opentelemetry/trace/span.h" #include "opentelemetry/trace/span_context.h" -#include "opentelemetry/trace/span_context_kv_iterable.h" #include "opentelemetry/trace/span_id.h" #include "opentelemetry/trace/span_startoptions.h" #include "opentelemetry/trace/trace_flags.h" @@ -36,6 +34,8 @@ namespace sdk { namespace trace { +const std::shared_ptr Tracer::kNoopTracer = + std::make_shared(); Tracer::Tracer(std::shared_ptr context, std::unique_ptr instrumentation_scope, @@ -54,6 +54,10 @@ nostd::shared_ptr Tracer::StartSpan( opentelemetry::trace::SpanContext parent_context = GetCurrentSpan()->GetContext(); if (nostd::holds_alternative(options.parent)) { + if (!tracer_config_.IsEnabled()) + { + return kNoopTracer->StartSpan(name, attributes, links, options); + } auto span_context = nostd::get(options.parent); if (span_context.IsValid()) { diff --git a/sdk/src/trace/tracer_config.cc b/sdk/src/trace/tracer_config.cc index 3f66c2670c..461e6c935c 100644 --- a/sdk/src/trace/tracer_config.cc +++ b/sdk/src/trace/tracer_config.cc @@ -1,6 +1,5 @@ -// -// Created by sharmapranav on 11/8/24. -// +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 #include "opentelemetry/sdk/trace/tracer_config.h" @@ -10,8 +9,13 @@ namespace sdk namespace trace { -TracerConfig TracerConfig::kDefaultConfig = TracerConfig(); -TracerConfig TracerConfig::kDisabledConfig = TracerConfig(true); +const TracerConfig TracerConfig::kDefaultConfig = TracerConfig(); +const TracerConfig TracerConfig::kDisabledConfig = TracerConfig(true); + +const instrumentationscope::ScopeConfigurator + TracerConfig::kDefaultTracerConfigurator = + *instrumentationscope::ScopeConfigurator::Create( + [](const instrumentationscope::InstrumentationScope &) { return Default(); }); TracerConfig TracerConfig::Disabled() { @@ -28,10 +32,15 @@ TracerConfig TracerConfig::Default() return kDefaultConfig; } +const instrumentationscope::ScopeConfigurator &TracerConfig::DefaultConfigurator() +{ + return kDefaultTracerConfigurator; +} + bool TracerConfig::IsEnabled() const noexcept { return !disabled_; } } // namespace trace } // namespace sdk -OPENTELEMETRY_END_NAMESPACE \ No newline at end of file +OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/src/trace/tracer_context.cc b/sdk/src/trace/tracer_context.cc index 65300d6f22..138abb8377 100644 --- a/sdk/src/trace/tracer_context.cc +++ b/sdk/src/trace/tracer_context.cc @@ -21,14 +21,17 @@ namespace trace { namespace resource = opentelemetry::sdk::resource; -TracerContext::TracerContext(std::vector> &&processors, - const resource::Resource &resource, - std::unique_ptr sampler, - std::unique_ptr id_generator) noexcept +TracerContext::TracerContext( + std::vector> &&processors, + const resource::Resource &resource, + std::unique_ptr sampler, + std::unique_ptr id_generator, + std::unique_ptr> tracer_configurator) noexcept : resource_(resource), sampler_(std::move(sampler)), id_generator_(std::move(id_generator)), - processor_(std::unique_ptr(new MultiSpanProcessor(std::move(processors)))) + processor_(std::unique_ptr(new MultiSpanProcessor(std::move(processors)))), + tracer_configurator_(std::move(tracer_configurator)) {} Sampler &TracerContext::GetSampler() const noexcept @@ -41,6 +44,11 @@ const resource::Resource &TracerContext::GetResource() const noexcept return resource_; } +ScopeConfigurator &TracerContext::GetTracerConfigurator() const noexcept +{ + return *tracer_configurator_; +} + opentelemetry::sdk::trace::IdGenerator &TracerContext::GetIdGenerator() const noexcept { return *id_generator_; diff --git a/sdk/src/trace/tracer_provider.cc b/sdk/src/trace/tracer_provider.cc index bda0ab2928..5824120422 100644 --- a/sdk/src/trace/tracer_provider.cc +++ b/sdk/src/trace/tracer_provider.cc @@ -16,6 +16,7 @@ #include "opentelemetry/sdk/trace/processor.h" #include "opentelemetry/sdk/trace/sampler.h" #include "opentelemetry/sdk/trace/tracer.h" +#include "opentelemetry/sdk/trace/tracer_config.h" #include "opentelemetry/sdk/trace/tracer_context.h" #include "opentelemetry/sdk/trace/tracer_provider.h" #include "opentelemetry/trace/tracer.h" @@ -35,24 +36,30 @@ TracerProvider::TracerProvider(std::unique_ptr context) noexcept OTEL_INTERNAL_LOG_DEBUG("[TracerProvider] TracerProvider created."); } -TracerProvider::TracerProvider(std::unique_ptr processor, - const resource::Resource &resource, - std::unique_ptr sampler, - std::unique_ptr id_generator) noexcept +TracerProvider::TracerProvider( + std::unique_ptr processor, + const resource::Resource &resource, + std::unique_ptr sampler, + std::unique_ptr id_generator, + std::unique_ptr> tracer_configurator) noexcept { std::vector> processors; processors.push_back(std::move(processor)); - context_ = std::make_shared(std::move(processors), resource, std::move(sampler), - std::move(id_generator)); + context_ = + std::make_shared(std::move(processors), resource, std::move(sampler), + std::move(id_generator), std::move(tracer_configurator)); } -TracerProvider::TracerProvider(std::vector> &&processors, - const resource::Resource &resource, - std::unique_ptr sampler, - std::unique_ptr id_generator) noexcept +TracerProvider::TracerProvider( + std::vector> &&processors, + const resource::Resource &resource, + std::unique_ptr sampler, + std::unique_ptr id_generator, + std::unique_ptr> tracer_configurator) noexcept { - context_ = std::make_shared(std::move(processors), resource, std::move(sampler), - std::move(id_generator)); + context_ = + std::make_shared(std::move(processors), resource, std::move(sampler), + std::move(id_generator), std::move(tracer_configurator)); } TracerProvider::~TracerProvider() @@ -107,8 +114,11 @@ nostd::shared_ptr TracerProvider::GetTracer( instrumentationscope::InstrumentationScopeAttributes attrs_map(attributes); auto scope = instrumentationscope::InstrumentationScope::Create(name, version, schema_url, attrs_map); + const instrumentationscope::InstrumentationScope &scope_reference = + instrumentationscope::InstrumentationScope(*scope); + auto tracer_config = context_->GetTracerConfigurator().ComputeConfig(scope_reference); - auto tracer = std::shared_ptr(new Tracer(context_, std::move(scope))); + auto tracer = std::shared_ptr(new Tracer(context_, std::move(scope), tracer_config)); tracers_.push_back(tracer); return nostd::shared_ptr{tracer}; } @@ -133,6 +143,12 @@ bool TracerProvider::ForceFlush(std::chrono::microseconds timeout) noexcept return context_->ForceFlush(timeout); } +TracerConfig TracerProvider::GetTracerConfig( + const InstrumentationScope &instrumentation_scope) const +{ + return context_->GetTracerConfigurator().ComputeConfig(instrumentation_scope); +} + } // namespace trace } // namespace sdk OPENTELEMETRY_END_NAMESPACE From 20cb4e9c755803ce4daf6d6c17aec36db4ac6d8a Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Wed, 13 Nov 2024 19:02:51 +0000 Subject: [PATCH 04/32] Fix iwyu warnings generated --- sdk/include/opentelemetry/sdk/trace/tracer.h | 1 + sdk/src/trace/tracer.cc | 3 +++ sdk/src/trace/tracer_config.cc | 2 ++ sdk/src/trace/tracer_context.cc | 2 ++ sdk/src/trace/tracer_provider.cc | 3 +++ 5 files changed, 11 insertions(+) diff --git a/sdk/include/opentelemetry/sdk/trace/tracer.h b/sdk/include/opentelemetry/sdk/trace/tracer.h index 77c1fd0861..64cd69afdc 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer.h @@ -17,6 +17,7 @@ #include "opentelemetry/sdk/trace/tracer_context.h" #include "opentelemetry/trace/noop.h" #include "opentelemetry/trace/span.h" +#include "opentelemetry/trace/span_context_kv_iterable.h" #include "opentelemetry/trace/span_startoptions.h" #include "opentelemetry/trace/tracer.h" #include "opentelemetry/version.h" diff --git a/sdk/src/trace/tracer.cc b/sdk/src/trace/tracer.cc index d4c6507839..8a06df69ca 100644 --- a/sdk/src/trace/tracer.cc +++ b/sdk/src/trace/tracer.cc @@ -3,6 +3,7 @@ #include #include +#include #include #include @@ -20,12 +21,14 @@ #include "opentelemetry/trace/noop.h" #include "opentelemetry/trace/span.h" #include "opentelemetry/trace/span_context.h" +#include "opentelemetry/trace/span_context_kv_iterable.h" #include "opentelemetry/trace/span_id.h" #include "opentelemetry/trace/span_startoptions.h" #include "opentelemetry/trace/trace_flags.h" #include "opentelemetry/trace/trace_id.h" #include "opentelemetry/trace/trace_state.h" #include "opentelemetry/version.h" +#include "opentelemetry/sdk/trace/tracer_config.h" #include "src/trace/span.h" diff --git a/sdk/src/trace/tracer_config.cc b/sdk/src/trace/tracer_config.cc index 461e6c935c..3cdbc8983c 100644 --- a/sdk/src/trace/tracer_config.cc +++ b/sdk/src/trace/tracer_config.cc @@ -1,7 +1,9 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +#include #include "opentelemetry/sdk/trace/tracer_config.h" +#include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk diff --git a/sdk/src/trace/tracer_context.cc b/sdk/src/trace/tracer_context.cc index 138abb8377..c4283cb376 100644 --- a/sdk/src/trace/tracer_context.cc +++ b/sdk/src/trace/tracer_context.cc @@ -13,6 +13,8 @@ #include "opentelemetry/sdk/trace/sampler.h" #include "opentelemetry/sdk/trace/tracer_context.h" #include "opentelemetry/version.h" +#include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" +#include "opentelemetry/sdk/trace/tracer_config.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk diff --git a/sdk/src/trace/tracer_provider.cc b/sdk/src/trace/tracer_provider.cc index 5824120422..f1c027da54 100644 --- a/sdk/src/trace/tracer_provider.cc +++ b/sdk/src/trace/tracer_provider.cc @@ -1,6 +1,7 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +#include #include #include #include @@ -11,6 +12,7 @@ #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/sdk/common/global_log_handler.h" #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" +#include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" #include "opentelemetry/sdk/resource/resource.h" #include "opentelemetry/sdk/trace/id_generator.h" #include "opentelemetry/sdk/trace/processor.h" @@ -19,6 +21,7 @@ #include "opentelemetry/sdk/trace/tracer_config.h" #include "opentelemetry/sdk/trace/tracer_context.h" #include "opentelemetry/sdk/trace/tracer_provider.h" +#include "opentelemetry/trace/span_id.h" #include "opentelemetry/trace/tracer.h" #include "opentelemetry/version.h" From df36a54b1cdddf3c49063b77672fa2f228e4fdaa Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Wed, 13 Nov 2024 20:07:36 +0000 Subject: [PATCH 05/32] Fix formatting --- sdk/src/trace/tracer.cc | 2 +- sdk/src/trace/tracer_config.cc | 2 +- sdk/src/trace/tracer_context.cc | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/src/trace/tracer.cc b/sdk/src/trace/tracer.cc index 8a06df69ca..aadea2a19d 100644 --- a/sdk/src/trace/tracer.cc +++ b/sdk/src/trace/tracer.cc @@ -16,6 +16,7 @@ #include "opentelemetry/sdk/trace/id_generator.h" #include "opentelemetry/sdk/trace/sampler.h" #include "opentelemetry/sdk/trace/tracer.h" +#include "opentelemetry/sdk/trace/tracer_config.h" #include "opentelemetry/sdk/trace/tracer_context.h" #include "opentelemetry/trace/context.h" #include "opentelemetry/trace/noop.h" @@ -28,7 +29,6 @@ #include "opentelemetry/trace/trace_id.h" #include "opentelemetry/trace/trace_state.h" #include "opentelemetry/version.h" -#include "opentelemetry/sdk/trace/tracer_config.h" #include "src/trace/span.h" diff --git a/sdk/src/trace/tracer_config.cc b/sdk/src/trace/tracer_config.cc index 3cdbc8983c..70e911f828 100644 --- a/sdk/src/trace/tracer_config.cc +++ b/sdk/src/trace/tracer_config.cc @@ -1,8 +1,8 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -#include #include "opentelemetry/sdk/trace/tracer_config.h" +#include #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" OPENTELEMETRY_BEGIN_NAMESPACE diff --git a/sdk/src/trace/tracer_context.cc b/sdk/src/trace/tracer_context.cc index c4283cb376..2f57124a18 100644 --- a/sdk/src/trace/tracer_context.cc +++ b/sdk/src/trace/tracer_context.cc @@ -6,15 +6,15 @@ #include #include +#include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" #include "opentelemetry/sdk/resource/resource.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/sampler.h" +#include "opentelemetry/sdk/trace/tracer_config.h" #include "opentelemetry/sdk/trace/tracer_context.h" #include "opentelemetry/version.h" -#include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" -#include "opentelemetry/sdk/trace/tracer_config.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk From a80800e6b6301f17d58a8a2c768b839b8006f10a Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Wed, 13 Nov 2024 22:33:19 +0000 Subject: [PATCH 06/32] Replace scope_configurator class with typedef --- .../instrumentationscope/scope_configurator.h | 39 ------------------- .../opentelemetry/sdk/trace/tracer_config.h | 11 ++++-- .../opentelemetry/sdk/trace/tracer_context.h | 10 ++--- .../opentelemetry/sdk/trace/tracer_provider.h | 11 ++---- sdk/src/trace/tracer_config.cc | 8 ++-- sdk/src/trace/tracer_context.cc | 14 +++---- sdk/src/trace/tracer_provider.cc | 29 +++++++------- 7 files changed, 38 insertions(+), 84 deletions(-) delete mode 100644 sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h diff --git a/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h b/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h deleted file mode 100644 index 6ffac33bca..0000000000 --- a/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -#pragma once - -#include -#include "instrumentation_scope.h" -#include "opentelemetry/version.h" - -OPENTELEMETRY_BEGIN_NAMESPACE -namespace sdk -{ -namespace instrumentationscope -{ -template -class ScopeConfigurator -{ -public: - static nostd::unique_ptr Create( - std::function scope_configurator) - { - return nostd::unique_ptr(new ScopeConfigurator(scope_configurator)); - } - - T ComputeConfig(const InstrumentationScope &instrumentation_scope) - { - return scope_configurator_(instrumentation_scope); - } - -private: - explicit ScopeConfigurator( - const std::function scope_configurator) - : scope_configurator_(scope_configurator) - {} - const std::function scope_configurator_; -}; -} // namespace instrumentationscope -} // namespace sdk -OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_config.h b/sdk/include/opentelemetry/sdk/trace/tracer_config.h index eb8f4a80c3..2a0d226992 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_config.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_config.h @@ -3,7 +3,7 @@ #pragma once -#include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" +#include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" #include "opentelemetry/version.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -11,6 +11,11 @@ namespace sdk { namespace trace { +// Forward declaration to support typedef creation +class TracerConfig; + +typedef std::function + TracerConfigurator; /** * TracerConfig defines various configurable aspects of a Tracer's behavior. */ @@ -26,14 +31,14 @@ class TracerConfig static TracerConfig Disabled(); static TracerConfig Enabled(); static TracerConfig Default(); - static const instrumentationscope::ScopeConfigurator &DefaultConfigurator(); + static const TracerConfigurator &DefaultConfigurator(); private: explicit TracerConfig(const bool disabled = false) : disabled_(disabled) {} bool disabled_; static const TracerConfig kDefaultConfig; static const TracerConfig kDisabledConfig; - static const instrumentationscope::ScopeConfigurator kDefaultTracerConfigurator; + static const TracerConfigurator kDefaultTracerConfigurator; }; } // 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 b49d45444f..8f6641e1e7 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_context.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_context.h @@ -7,7 +7,6 @@ #include #include -#include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" #include "opentelemetry/sdk/resource/resource.h" #include "opentelemetry/sdk/trace/id_generator.h" #include "opentelemetry/sdk/trace/processor.h" @@ -48,9 +47,8 @@ class TracerContext std::unique_ptr sampler = std::unique_ptr(new AlwaysOnSampler), std::unique_ptr id_generator = std::unique_ptr(new RandomIdGenerator()), - std::unique_ptr> tracer_configurator = - std::make_unique>( - TracerConfig::DefaultConfigurator())) noexcept; + std::unique_ptr tracer_configurator = + std::make_unique(TracerConfig::DefaultConfigurator())) noexcept; virtual ~TracerContext() = default; @@ -84,7 +82,7 @@ class TracerContext */ const opentelemetry::sdk::resource::Resource &GetResource() const noexcept; - ScopeConfigurator &GetTracerConfigurator() const noexcept; + TracerConfigurator &GetTracerConfigurator() const noexcept; /** * Obtain the Id Generator associated with this tracer context. @@ -109,7 +107,7 @@ class TracerContext std::unique_ptr sampler_; std::unique_ptr id_generator_; std::unique_ptr processor_; - std::unique_ptr> tracer_configurator_; + std::unique_ptr tracer_configurator_; }; } // namespace trace diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h index fe73083584..8c8c58c650 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h @@ -9,7 +9,6 @@ #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/string_view.h" -#include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" #include "opentelemetry/sdk/resource/resource.h" #include "opentelemetry/sdk/trace/id_generator.h" #include "opentelemetry/sdk/trace/processor.h" @@ -52,9 +51,8 @@ class OPENTELEMETRY_EXPORT TracerProvider final : public opentelemetry::trace::T std::unique_ptr sampler = std::unique_ptr(new AlwaysOnSampler), std::unique_ptr id_generator = std::unique_ptr(new RandomIdGenerator()), - std::unique_ptr> tracer_configurator = - std::make_unique>( - TracerConfig::DefaultConfigurator())) noexcept; + std::unique_ptr tracer_configurator = + std::make_unique(TracerConfig::DefaultConfigurator())) noexcept; explicit TracerProvider( std::vector> &&processors, @@ -63,9 +61,8 @@ class OPENTELEMETRY_EXPORT TracerProvider final : public opentelemetry::trace::T std::unique_ptr sampler = std::unique_ptr(new AlwaysOnSampler), std::unique_ptr id_generator = std::unique_ptr(new RandomIdGenerator()), - std::unique_ptr> tracer_configurator = - std::make_unique>( - TracerConfig::DefaultConfigurator())) noexcept; + std::unique_ptr tracer_configurator = + std::make_unique(TracerConfig::DefaultConfigurator())) noexcept; /** * Initialize a new tracer provider with a specified context diff --git a/sdk/src/trace/tracer_config.cc b/sdk/src/trace/tracer_config.cc index 70e911f828..c0c6b7ccfc 100644 --- a/sdk/src/trace/tracer_config.cc +++ b/sdk/src/trace/tracer_config.cc @@ -14,10 +14,8 @@ namespace trace const TracerConfig TracerConfig::kDefaultConfig = TracerConfig(); const TracerConfig TracerConfig::kDisabledConfig = TracerConfig(true); -const instrumentationscope::ScopeConfigurator - TracerConfig::kDefaultTracerConfigurator = - *instrumentationscope::ScopeConfigurator::Create( - [](const instrumentationscope::InstrumentationScope &) { return Default(); }); +const TracerConfigurator TracerConfig::kDefaultTracerConfigurator = + [](const instrumentationscope::InstrumentationScope &) { return Default(); }; TracerConfig TracerConfig::Disabled() { @@ -34,7 +32,7 @@ TracerConfig TracerConfig::Default() return kDefaultConfig; } -const instrumentationscope::ScopeConfigurator &TracerConfig::DefaultConfigurator() +const TracerConfigurator &TracerConfig::DefaultConfigurator() { return kDefaultTracerConfigurator; } diff --git a/sdk/src/trace/tracer_context.cc b/sdk/src/trace/tracer_context.cc index 2f57124a18..b6b1e3fda2 100644 --- a/sdk/src/trace/tracer_context.cc +++ b/sdk/src/trace/tracer_context.cc @@ -6,7 +6,6 @@ #include #include -#include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" #include "opentelemetry/sdk/resource/resource.h" #include "opentelemetry/sdk/trace/id_generator.h" #include "opentelemetry/sdk/trace/multi_span_processor.h" @@ -23,12 +22,11 @@ namespace trace { namespace resource = opentelemetry::sdk::resource; -TracerContext::TracerContext( - std::vector> &&processors, - const resource::Resource &resource, - std::unique_ptr sampler, - std::unique_ptr id_generator, - std::unique_ptr> tracer_configurator) noexcept +TracerContext::TracerContext(std::vector> &&processors, + const resource::Resource &resource, + std::unique_ptr sampler, + std::unique_ptr id_generator, + std::unique_ptr tracer_configurator) noexcept : resource_(resource), sampler_(std::move(sampler)), id_generator_(std::move(id_generator)), @@ -46,7 +44,7 @@ const resource::Resource &TracerContext::GetResource() const noexcept return resource_; } -ScopeConfigurator &TracerContext::GetTracerConfigurator() const noexcept +TracerConfigurator &TracerContext::GetTracerConfigurator() const noexcept { return *tracer_configurator_; } diff --git a/sdk/src/trace/tracer_provider.cc b/sdk/src/trace/tracer_provider.cc index f1c027da54..8dd91aa8a6 100644 --- a/sdk/src/trace/tracer_provider.cc +++ b/sdk/src/trace/tracer_provider.cc @@ -12,7 +12,6 @@ #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/sdk/common/global_log_handler.h" #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" -#include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" #include "opentelemetry/sdk/resource/resource.h" #include "opentelemetry/sdk/trace/id_generator.h" #include "opentelemetry/sdk/trace/processor.h" @@ -39,12 +38,11 @@ TracerProvider::TracerProvider(std::unique_ptr context) noexcept OTEL_INTERNAL_LOG_DEBUG("[TracerProvider] TracerProvider created."); } -TracerProvider::TracerProvider( - std::unique_ptr processor, - const resource::Resource &resource, - std::unique_ptr sampler, - std::unique_ptr id_generator, - std::unique_ptr> tracer_configurator) noexcept +TracerProvider::TracerProvider(std::unique_ptr processor, + const resource::Resource &resource, + std::unique_ptr sampler, + std::unique_ptr id_generator, + std::unique_ptr tracer_configurator) noexcept { std::vector> processors; processors.push_back(std::move(processor)); @@ -53,12 +51,11 @@ TracerProvider::TracerProvider( std::move(id_generator), std::move(tracer_configurator)); } -TracerProvider::TracerProvider( - std::vector> &&processors, - const resource::Resource &resource, - std::unique_ptr sampler, - std::unique_ptr id_generator, - std::unique_ptr> tracer_configurator) noexcept +TracerProvider::TracerProvider(std::vector> &&processors, + const resource::Resource &resource, + std::unique_ptr sampler, + std::unique_ptr id_generator, + std::unique_ptr tracer_configurator) noexcept { context_ = std::make_shared(std::move(processors), resource, std::move(sampler), @@ -119,9 +116,9 @@ nostd::shared_ptr TracerProvider::GetTracer( instrumentationscope::InstrumentationScope::Create(name, version, schema_url, attrs_map); const instrumentationscope::InstrumentationScope &scope_reference = instrumentationscope::InstrumentationScope(*scope); - auto tracer_config = context_->GetTracerConfigurator().ComputeConfig(scope_reference); - auto tracer = std::shared_ptr(new Tracer(context_, std::move(scope), tracer_config)); + auto tracer = std::shared_ptr( + new Tracer(context_, std::move(scope), GetTracerConfig(scope_reference))); tracers_.push_back(tracer); return nostd::shared_ptr{tracer}; } @@ -149,7 +146,7 @@ bool TracerProvider::ForceFlush(std::chrono::microseconds timeout) noexcept TracerConfig TracerProvider::GetTracerConfig( const InstrumentationScope &instrumentation_scope) const { - return context_->GetTracerConfigurator().ComputeConfig(instrumentation_scope); + return context_->GetTracerConfigurator()(instrumentation_scope); } } // namespace trace From 694f9c0fcedfc96aa13ea73f2ad65186ef96d967 Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Thu, 14 Nov 2024 17:29:20 +0000 Subject: [PATCH 07/32] Update factories to accept TracerConfigurator --- .../sdk/trace/tracer_context_factory.h | 10 ++++++ .../sdk/trace/tracer_provider_factory.h | 18 +++++++++-- sdk/src/trace/tracer_context_factory.cc | 18 +++++++++-- sdk/src/trace/tracer_provider_factory.cc | 32 +++++++++++++++++-- 4 files changed, 72 insertions(+), 6 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_context_factory.h b/sdk/include/opentelemetry/sdk/trace/tracer_context_factory.h index 4954fd7c3f..0f06b1106d 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_context_factory.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_context_factory.h @@ -54,6 +54,16 @@ class OPENTELEMETRY_EXPORT TracerContextFactory const opentelemetry::sdk::resource::Resource &resource, std::unique_ptr sampler, std::unique_ptr id_generator); + + /** + * Create a TracerContext. + */ + static std::unique_ptr Create( + std::vector> &&processors, + const opentelemetry::sdk::resource::Resource &resource, + std::unique_ptr sampler, + std::unique_ptr id_generator, + std::unique_ptr tracer_configurator); }; } // namespace trace diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_provider_factory.h b/sdk/include/opentelemetry/sdk/trace/tracer_provider_factory.h index ed451a8c99..c8f2bfdb50 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_provider_factory.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_provider_factory.h @@ -28,7 +28,7 @@ namespace trace class OPENTELEMETRY_EXPORT TracerProviderFactory { public: - /* Serie of builders with a single processor. */ + /* Series of creator methods with a single processor. */ static std::unique_ptr Create( std::unique_ptr processor); @@ -48,7 +48,14 @@ class OPENTELEMETRY_EXPORT TracerProviderFactory std::unique_ptr sampler, std::unique_ptr id_generator); - /* Serie of builders with a vector of processor. */ + static std::unique_ptr Create( + std::unique_ptr processor, + const opentelemetry::sdk::resource::Resource &resource, + std::unique_ptr sampler, + std::unique_ptr id_generator, + std::unique_ptr tracer_configurator); + + /* Series of creator methods with a single processor. */ static std::unique_ptr Create( std::vector> &&processors); @@ -68,6 +75,13 @@ class OPENTELEMETRY_EXPORT TracerProviderFactory std::unique_ptr sampler, std::unique_ptr id_generator); + static std::unique_ptr Create( + std::vector> &&processors, + const opentelemetry::sdk::resource::Resource &resource, + std::unique_ptr sampler, + std::unique_ptr id_generator, + std::unique_ptr tracer_configurator); + /* Create with a tracer context. */ static std::unique_ptr Create( diff --git a/sdk/src/trace/tracer_context_factory.cc b/sdk/src/trace/tracer_context_factory.cc index 68ea39349e..0c581a44e5 100644 --- a/sdk/src/trace/tracer_context_factory.cc +++ b/sdk/src/trace/tracer_context_factory.cc @@ -51,8 +51,22 @@ std::unique_ptr TracerContextFactory::Create( std::unique_ptr sampler, std::unique_ptr id_generator) { - std::unique_ptr context(new TracerContext( - std::move(processors), resource, std::move(sampler), std::move(id_generator))); + auto tracer_configurator = + std::make_unique(TracerConfig::DefaultConfigurator()); + return Create(std::move(processors), resource, std::move(sampler), std::move(id_generator), + std::move(tracer_configurator)); +} + +std::unique_ptr TracerContextFactory::Create( + std::vector> &&processors, + const opentelemetry::sdk::resource::Resource &resource, + std::unique_ptr sampler, + std::unique_ptr id_generator, + std::unique_ptr tracer_configurator) +{ + std::unique_ptr context( + new TracerContext(std::move(processors), resource, std::move(sampler), + std::move(id_generator), std::move(tracer_configurator))); return context; } diff --git a/sdk/src/trace/tracer_provider_factory.cc b/sdk/src/trace/tracer_provider_factory.cc index eaeb669b91..7a40a5a3b8 100644 --- a/sdk/src/trace/tracer_provider_factory.cc +++ b/sdk/src/trace/tracer_provider_factory.cc @@ -51,10 +51,24 @@ std::unique_ptr TracerProviderFactory const opentelemetry::sdk::resource::Resource &resource, std::unique_ptr sampler, std::unique_ptr id_generator) +{ + auto tracer_configurator = + std::make_unique(TracerConfig::DefaultConfigurator()); + return Create(std::move(processor), resource, std::move(sampler), std::move(id_generator), + std::move(tracer_configurator)); +} + +std::unique_ptr TracerProviderFactory::Create( + std::unique_ptr processor, + const opentelemetry::sdk::resource::Resource &resource, + std::unique_ptr sampler, + std::unique_ptr id_generator, + std::unique_ptr tracer_configurator) { std::unique_ptr provider( new opentelemetry::sdk::trace::TracerProvider(std::move(processor), resource, - std::move(sampler), std::move(id_generator))); + std::move(sampler), std::move(id_generator), + std::move(tracer_configurator))); return provider; } @@ -87,10 +101,24 @@ std::unique_ptr TracerProviderFactory const opentelemetry::sdk::resource::Resource &resource, std::unique_ptr sampler, std::unique_ptr id_generator) +{ + auto tracer_configurator = + std::make_unique(TracerConfig::DefaultConfigurator()); + return Create(std::move(processors), resource, std::move(sampler), std::move(id_generator), + std::move(tracer_configurator)); +} + +std::unique_ptr TracerProviderFactory::Create( + std::vector> &&processors, + const opentelemetry::sdk::resource::Resource &resource, + std::unique_ptr sampler, + std::unique_ptr id_generator, + std::unique_ptr tracer_configurator) { std::unique_ptr provider( new opentelemetry::sdk::trace::TracerProvider(std::move(processors), resource, - std::move(sampler), std::move(id_generator))); + std::move(sampler), std::move(id_generator), + std::move(tracer_configurator))); return provider; } From f815c3e804de99a9b1ef847d937637790b6480e8 Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Thu, 14 Nov 2024 17:42:03 +0000 Subject: [PATCH 08/32] Fix IWYU warnings --- sdk/include/opentelemetry/sdk/trace/tracer_config.h | 1 + sdk/src/trace/tracer_config.cc | 1 - sdk/src/trace/tracer_context_factory.cc | 1 + sdk/src/trace/tracer_provider.cc | 1 + sdk/src/trace/tracer_provider_factory.cc | 1 + 5 files changed, 4 insertions(+), 1 deletion(-) diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_config.h b/sdk/include/opentelemetry/sdk/trace/tracer_config.h index 2a0d226992..14c6d59ca7 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_config.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_config.h @@ -3,6 +3,7 @@ #pragma once +#include #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" #include "opentelemetry/version.h" diff --git a/sdk/src/trace/tracer_config.cc b/sdk/src/trace/tracer_config.cc index c0c6b7ccfc..8089dfd44c 100644 --- a/sdk/src/trace/tracer_config.cc +++ b/sdk/src/trace/tracer_config.cc @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 #include "opentelemetry/sdk/trace/tracer_config.h" -#include #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" OPENTELEMETRY_BEGIN_NAMESPACE diff --git a/sdk/src/trace/tracer_context_factory.cc b/sdk/src/trace/tracer_context_factory.cc index 0c581a44e5..f5441a323f 100644 --- a/sdk/src/trace/tracer_context_factory.cc +++ b/sdk/src/trace/tracer_context_factory.cc @@ -11,6 +11,7 @@ #include "opentelemetry/sdk/trace/random_id_generator_factory.h" #include "opentelemetry/sdk/trace/sampler.h" #include "opentelemetry/sdk/trace/samplers/always_on_factory.h" +#include "opentelemetry/sdk/trace/tracer_config.h" #include "opentelemetry/sdk/trace/tracer_context.h" #include "opentelemetry/sdk/trace/tracer_context_factory.h" #include "opentelemetry/version.h" diff --git a/sdk/src/trace/tracer_provider.cc b/sdk/src/trace/tracer_provider.cc index 8dd91aa8a6..4394cda2a9 100644 --- a/sdk/src/trace/tracer_provider.cc +++ b/sdk/src/trace/tracer_provider.cc @@ -3,6 +3,7 @@ #include #include +#include #include #include #include diff --git a/sdk/src/trace/tracer_provider_factory.cc b/sdk/src/trace/tracer_provider_factory.cc index 7a40a5a3b8..a196d9e26e 100644 --- a/sdk/src/trace/tracer_provider_factory.cc +++ b/sdk/src/trace/tracer_provider_factory.cc @@ -11,6 +11,7 @@ #include "opentelemetry/sdk/trace/random_id_generator_factory.h" #include "opentelemetry/sdk/trace/sampler.h" #include "opentelemetry/sdk/trace/samplers/always_on_factory.h" +#include "opentelemetry/sdk/trace/tracer_config.h" #include "opentelemetry/sdk/trace/tracer_context.h" #include "opentelemetry/sdk/trace/tracer_provider.h" #include "opentelemetry/sdk/trace/tracer_provider_factory.h" From 9b897862ab5728ded248eaef0d2229e39624382d Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Thu, 14 Nov 2024 20:19:25 +0000 Subject: [PATCH 09/32] Add operator overload to check for equality --- sdk/include/opentelemetry/sdk/trace/tracer_config.h | 1 + sdk/src/trace/tracer_config.cc | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_config.h b/sdk/include/opentelemetry/sdk/trace/tracer_config.h index 14c6d59ca7..d098f2c1c5 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_config.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_config.h @@ -28,6 +28,7 @@ class TracerConfig * @return a boolean indicating if the Tracer is enabled. Defaults to true. */ bool IsEnabled() const noexcept; + bool operator==(const TracerConfig& other) const noexcept; static TracerConfig Disabled(); static TracerConfig Enabled(); diff --git a/sdk/src/trace/tracer_config.cc b/sdk/src/trace/tracer_config.cc index 8089dfd44c..22a9419076 100644 --- a/sdk/src/trace/tracer_config.cc +++ b/sdk/src/trace/tracer_config.cc @@ -40,6 +40,12 @@ bool TracerConfig::IsEnabled() const noexcept { return !disabled_; } + +bool TracerConfig::operator==(const TracerConfig &other) const noexcept +{ + return disabled_ == other.disabled_; +} + } // namespace trace } // namespace sdk OPENTELEMETRY_END_NAMESPACE From 541c0eb6626b67ac53cf7802502cdf880a90e0db Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Thu, 14 Nov 2024 20:21:21 +0000 Subject: [PATCH 10/32] Add tests for TracerConfig --- sdk/test/trace/BUILD | 15 +++++++ sdk/test/trace/CMakeLists.txt | 3 +- sdk/test/trace/tracer_config_test.cc | 62 ++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 sdk/test/trace/tracer_config_test.cc diff --git a/sdk/test/trace/BUILD b/sdk/test/trace/BUILD index 9aa8a95cdf..455b24964d 100644 --- a/sdk/test/trace/BUILD +++ b/sdk/test/trace/BUILD @@ -143,6 +143,21 @@ cc_test( ], ) +cc_test( + name = "tracer_config_test", + srcs = [ + "tracer_config_test.cc", + ], + tags = [ + "test", + "trace", + ], + deps = [ + "//sdk/src/trace", + "@com_google_googletest//:gtest_main", + ], +) + otel_cc_benchmark( name = "sampler_benchmark", srcs = ["sampler_benchmark.cc"], diff --git a/sdk/test/trace/CMakeLists.txt b/sdk/test/trace/CMakeLists.txt index 385c8eaacd..7c1b75400c 100644 --- a/sdk/test/trace/CMakeLists.txt +++ b/sdk/test/trace/CMakeLists.txt @@ -11,7 +11,8 @@ foreach( always_on_sampler_test parent_sampler_test trace_id_ratio_sampler_test - batch_span_processor_test) + batch_span_processor_test + tracer_config_test) add_executable(${testname} "${testname}.cc") target_link_libraries( ${testname} diff --git a/sdk/test/trace/tracer_config_test.cc b/sdk/test/trace/tracer_config_test.cc new file mode 100644 index 0000000000..dff28f3fa0 --- /dev/null +++ b/sdk/test/trace/tracer_config_test.cc @@ -0,0 +1,62 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#include +#include + +namespace trace_sdk = opentelemetry::sdk::trace; +namespace instrumentation_scope = opentelemetry::sdk::instrumentationscope; + +/** Tests to verify the basic behavior of trace_sdk::TracerConfig */ + +TEST(TracerConfig, CheckDisabledWorksAsExpected) +{ + trace_sdk::TracerConfig disabled_config = trace_sdk::TracerConfig::Disabled(); + ASSERT_FALSE(disabled_config.IsEnabled()); +} + +TEST(TracerConfig, CheckEnabledWorksAsExpected) +{ + trace_sdk::TracerConfig enabled_config = trace_sdk::TracerConfig::Enabled(); + ASSERT_TRUE(enabled_config.IsEnabled()); +} + +TEST(TracerConfig, CheckDefaultConfigWorksAccToSpec) +{ + trace_sdk::TracerConfig default_config = trace_sdk::TracerConfig::Default(); + ASSERT_TRUE(default_config.IsEnabled()); +} + +/** Tests to verify the behavior of trace_sdk::TracerConfig::DefaultConfigurator */ + +std::pair attr1 = { + "accept_single_attr", true}; +std::pair attr2 = { + "accept_second_attr", "some other attr"}; +std::pair attr3 = { + "accept_third_attr", 3}; + +const instrumentation_scope::InstrumentationScope instrumentation_scopes[] = { + *instrumentation_scope::InstrumentationScope::Create("test_scope_1").get(), + *instrumentation_scope::InstrumentationScope::Create("test_scope_2", "1.0").get(), + *instrumentation_scope::InstrumentationScope::Create("test_scope_3", "0", "https://opentelemetry.io/schemas/v1.18.0").get(), + *instrumentation_scope::InstrumentationScope::Create("test_scope_3", "0", "https://opentelemetry.io/schemas/v1.18.0", {attr1}) + .get(), + *instrumentation_scope::InstrumentationScope::Create("test_scope_4", "0", "https://opentelemetry.io/schemas/v1.18.0", {attr1, attr2, attr3}) + .get(), +}; + +// Test fixture for VerifyDefaultConfiguratorBehavior +class DefaultTracerConfiguratorTestFixture : public ::testing::TestWithParam +{}; + +// verifies that the default configurator always returns the default tracer config +TEST_P(DefaultTracerConfiguratorTestFixture, VerifyDefaultConfiguratorBehavior) +{ + const instrumentation_scope::InstrumentationScope& scope = GetParam(); + trace_sdk::TracerConfigurator default_configurator = trace_sdk::TracerConfig::DefaultConfigurator(); + + ASSERT_EQ(default_configurator(scope), trace_sdk::TracerConfig::Default()); +} + +INSTANTIATE_TEST_SUITE_P(InstrumentationScopes, DefaultTracerConfiguratorTestFixture, ::testing::ValuesIn(instrumentation_scopes)); \ No newline at end of file From a67ea858180ca42edc60745077976dbb30aa155d Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Thu, 14 Nov 2024 20:30:34 +0000 Subject: [PATCH 11/32] Fix formatting --- .../opentelemetry/sdk/trace/tracer_config.h | 2 +- sdk/test/trace/tracer_config_test.cc | 43 ++++++++++++------- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_config.h b/sdk/include/opentelemetry/sdk/trace/tracer_config.h index d098f2c1c5..1bbc2786cc 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_config.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_config.h @@ -28,7 +28,7 @@ class TracerConfig * @return a boolean indicating if the Tracer is enabled. Defaults to true. */ bool IsEnabled() const noexcept; - bool operator==(const TracerConfig& other) const noexcept; + bool operator==(const TracerConfig &other) const noexcept; static TracerConfig Disabled(); static TracerConfig Enabled(); diff --git a/sdk/test/trace/tracer_config_test.cc b/sdk/test/trace/tracer_config_test.cc index dff28f3fa0..c11ee8a81e 100644 --- a/sdk/test/trace/tracer_config_test.cc +++ b/sdk/test/trace/tracer_config_test.cc @@ -4,7 +4,7 @@ #include #include -namespace trace_sdk = opentelemetry::sdk::trace; +namespace trace_sdk = opentelemetry::sdk::trace; namespace instrumentation_scope = opentelemetry::sdk::instrumentationscope; /** Tests to verify the basic behavior of trace_sdk::TracerConfig */ @@ -30,33 +30,46 @@ TEST(TracerConfig, CheckDefaultConfigWorksAccToSpec) /** Tests to verify the behavior of trace_sdk::TracerConfig::DefaultConfigurator */ std::pair attr1 = { - "accept_single_attr", true}; + "accept_single_attr", true}; std::pair attr2 = { - "accept_second_attr", "some other attr"}; + "accept_second_attr", "some other attr"}; std::pair attr3 = { - "accept_third_attr", 3}; + "accept_third_attr", 3}; const instrumentation_scope::InstrumentationScope instrumentation_scopes[] = { - *instrumentation_scope::InstrumentationScope::Create("test_scope_1").get(), - *instrumentation_scope::InstrumentationScope::Create("test_scope_2", "1.0").get(), - *instrumentation_scope::InstrumentationScope::Create("test_scope_3", "0", "https://opentelemetry.io/schemas/v1.18.0").get(), - *instrumentation_scope::InstrumentationScope::Create("test_scope_3", "0", "https://opentelemetry.io/schemas/v1.18.0", {attr1}) - .get(), - *instrumentation_scope::InstrumentationScope::Create("test_scope_4", "0", "https://opentelemetry.io/schemas/v1.18.0", {attr1, attr2, attr3}) - .get(), + *instrumentation_scope::InstrumentationScope::Create("test_scope_1").get(), + *instrumentation_scope::InstrumentationScope::Create("test_scope_2", "1.0").get(), + *instrumentation_scope::InstrumentationScope::Create("test_scope_3", + "0", + "https://opentelemetry.io/schemas/v1.18.0") + .get(), + *instrumentation_scope::InstrumentationScope::Create("test_scope_3", + "0", + "https://opentelemetry.io/schemas/v1.18.0", + {attr1}) + .get(), + *instrumentation_scope::InstrumentationScope::Create("test_scope_4", + "0", + "https://opentelemetry.io/schemas/v1.18.0", + {attr1, attr2, attr3}) + .get(), }; // Test fixture for VerifyDefaultConfiguratorBehavior -class DefaultTracerConfiguratorTestFixture : public ::testing::TestWithParam +class DefaultTracerConfiguratorTestFixture + : public ::testing::TestWithParam {}; // verifies that the default configurator always returns the default tracer config TEST_P(DefaultTracerConfiguratorTestFixture, VerifyDefaultConfiguratorBehavior) { - const instrumentation_scope::InstrumentationScope& scope = GetParam(); - trace_sdk::TracerConfigurator default_configurator = trace_sdk::TracerConfig::DefaultConfigurator(); + const instrumentation_scope::InstrumentationScope &scope = GetParam(); + trace_sdk::TracerConfigurator default_configurator = + trace_sdk::TracerConfig::DefaultConfigurator(); ASSERT_EQ(default_configurator(scope), trace_sdk::TracerConfig::Default()); } -INSTANTIATE_TEST_SUITE_P(InstrumentationScopes, DefaultTracerConfiguratorTestFixture, ::testing::ValuesIn(instrumentation_scopes)); \ No newline at end of file +INSTANTIATE_TEST_SUITE_P(InstrumentationScopes, + DefaultTracerConfiguratorTestFixture, + ::testing::ValuesIn(instrumentation_scopes)); \ No newline at end of file From bcd6e948dc4307d3107d7427d889ea76d87dbe28 Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Thu, 14 Nov 2024 23:48:51 +0000 Subject: [PATCH 12/32] Fix memory leak issue in test --- sdk/test/trace/tracer_config_test.cc | 45 +++++++++++++++------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/sdk/test/trace/tracer_config_test.cc b/sdk/test/trace/tracer_config_test.cc index c11ee8a81e..8d78e87ab5 100644 --- a/sdk/test/trace/tracer_config_test.cc +++ b/sdk/test/trace/tracer_config_test.cc @@ -36,40 +36,43 @@ std::pair attr3 = { "accept_third_attr", 3}; -const instrumentation_scope::InstrumentationScope instrumentation_scopes[] = { - *instrumentation_scope::InstrumentationScope::Create("test_scope_1").get(), - *instrumentation_scope::InstrumentationScope::Create("test_scope_2", "1.0").get(), - *instrumentation_scope::InstrumentationScope::Create("test_scope_3", - "0", - "https://opentelemetry.io/schemas/v1.18.0") - .get(), - *instrumentation_scope::InstrumentationScope::Create("test_scope_3", - "0", - "https://opentelemetry.io/schemas/v1.18.0", - {attr1}) - .get(), - *instrumentation_scope::InstrumentationScope::Create("test_scope_4", - "0", - "https://opentelemetry.io/schemas/v1.18.0", - {attr1, attr2, attr3}) - .get(), +const std::array instrumentation_scopes = { + std::move(instrumentation_scope::InstrumentationScope::Create("test_scope_1")).get(), + std::move(instrumentation_scope::InstrumentationScope::Create("test_scope_2", "1.0")).get(), + std::move(instrumentation_scope::InstrumentationScope::Create( + "test_scope_3", + "0", + "https://opentelemetry.io/schemas/v1.18.0")) + .get(), + std::move(instrumentation_scope::InstrumentationScope::Create( + "test_scope_3", + "0", + "https://opentelemetry.io/schemas/v1.18.0", + {attr1})) + .get(), + std::move(instrumentation_scope::InstrumentationScope::Create( + "test_scope_4", + "0", + "https://opentelemetry.io/schemas/v1.18.0", + {attr1, attr2, attr3})) + .get(), }; // Test fixture for VerifyDefaultConfiguratorBehavior class DefaultTracerConfiguratorTestFixture - : public ::testing::TestWithParam + : public ::testing::TestWithParam {}; // verifies that the default configurator always returns the default tracer config TEST_P(DefaultTracerConfiguratorTestFixture, VerifyDefaultConfiguratorBehavior) { - const instrumentation_scope::InstrumentationScope &scope = GetParam(); + instrumentation_scope::InstrumentationScope *scope = GetParam(); trace_sdk::TracerConfigurator default_configurator = trace_sdk::TracerConfig::DefaultConfigurator(); - ASSERT_EQ(default_configurator(scope), trace_sdk::TracerConfig::Default()); + ASSERT_EQ(default_configurator(*scope), trace_sdk::TracerConfig::Default()); } INSTANTIATE_TEST_SUITE_P(InstrumentationScopes, DefaultTracerConfiguratorTestFixture, - ::testing::ValuesIn(instrumentation_scopes)); \ No newline at end of file + ::testing::ValuesIn(instrumentation_scopes)); From 2e2afd4738fb1271062a6792b5b9cee0580d9b31 Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Fri, 15 Nov 2024 00:24:13 +0000 Subject: [PATCH 13/32] Replace typedef with type alias --- .../instrumentation_scope.h | 3 +++ .../opentelemetry/sdk/trace/tracer_config.h | 10 ++------ .../opentelemetry/sdk/trace/tracer_context.h | 9 +++---- .../sdk/trace/tracer_context_factory.h | 2 +- .../opentelemetry/sdk/trace/tracer_provider.h | 10 ++++---- .../sdk/trace/tracer_provider_factory.h | 4 ++-- sdk/src/trace/tracer_config.cc | 7 +++--- sdk/src/trace/tracer_context.cc | 6 +++-- sdk/src/trace/tracer_context_factory.cc | 5 ++-- sdk/src/trace/tracer_provider.cc | 24 +++++++++++-------- sdk/src/trace/tracer_provider_factory.cc | 10 ++++---- sdk/test/trace/tracer_config_test.cc | 2 +- 12 files changed, 51 insertions(+), 41 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/instrumentationscope/instrumentation_scope.h b/sdk/include/opentelemetry/sdk/instrumentationscope/instrumentation_scope.h index 1bf0facaa6..595e4cc4ba 100644 --- a/sdk/include/opentelemetry/sdk/instrumentationscope/instrumentation_scope.h +++ b/sdk/include/opentelemetry/sdk/instrumentationscope/instrumentation_scope.h @@ -3,6 +3,7 @@ #pragma once +#include #include #include @@ -158,6 +159,8 @@ class InstrumentationScope InstrumentationScopeAttributes attributes_; }; +template +using ScopeConfigurator = std::function; } // namespace instrumentationscope } // namespace sdk diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_config.h b/sdk/include/opentelemetry/sdk/trace/tracer_config.h index 1bbc2786cc..b5115f7c0c 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_config.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_config.h @@ -3,7 +3,6 @@ #pragma once -#include #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" #include "opentelemetry/version.h" @@ -12,11 +11,6 @@ namespace sdk { namespace trace { -// Forward declaration to support typedef creation -class TracerConfig; - -typedef std::function - TracerConfigurator; /** * TracerConfig defines various configurable aspects of a Tracer's behavior. */ @@ -33,14 +27,14 @@ class TracerConfig static TracerConfig Disabled(); static TracerConfig Enabled(); static TracerConfig Default(); - static const TracerConfigurator &DefaultConfigurator(); + static const instrumentationscope::ScopeConfigurator &DefaultConfigurator(); private: explicit TracerConfig(const bool disabled = false) : disabled_(disabled) {} bool disabled_; static const TracerConfig kDefaultConfig; static const TracerConfig kDisabledConfig; - static const TracerConfigurator kDefaultTracerConfigurator; + static const instrumentationscope::ScopeConfigurator kDefaultTracerConfigurator; }; } // 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 8f6641e1e7..25314fa846 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_context.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_context.h @@ -47,8 +47,9 @@ class TracerContext std::unique_ptr sampler = std::unique_ptr(new AlwaysOnSampler), std::unique_ptr id_generator = std::unique_ptr(new RandomIdGenerator()), - std::unique_ptr tracer_configurator = - std::make_unique(TracerConfig::DefaultConfigurator())) noexcept; + std::unique_ptr> tracer_configurator = + std::make_unique>( + TracerConfig::DefaultConfigurator())) noexcept; virtual ~TracerContext() = default; @@ -82,7 +83,7 @@ class TracerContext */ const opentelemetry::sdk::resource::Resource &GetResource() const noexcept; - TracerConfigurator &GetTracerConfigurator() const noexcept; + instrumentationscope::ScopeConfigurator &GetTracerConfigurator() const noexcept; /** * Obtain the Id Generator associated with this tracer context. @@ -107,7 +108,7 @@ class TracerContext std::unique_ptr sampler_; std::unique_ptr id_generator_; std::unique_ptr processor_; - std::unique_ptr tracer_configurator_; + std::unique_ptr> tracer_configurator_; }; } // namespace trace diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_context_factory.h b/sdk/include/opentelemetry/sdk/trace/tracer_context_factory.h index 0f06b1106d..0279bee93a 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_context_factory.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_context_factory.h @@ -63,7 +63,7 @@ class OPENTELEMETRY_EXPORT TracerContextFactory const opentelemetry::sdk::resource::Resource &resource, std::unique_ptr sampler, std::unique_ptr id_generator, - std::unique_ptr tracer_configurator); + std::unique_ptr> tracer_configurator); }; } // namespace trace diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h index 8c8c58c650..838c46e37b 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h @@ -51,8 +51,9 @@ class OPENTELEMETRY_EXPORT TracerProvider final : public opentelemetry::trace::T std::unique_ptr sampler = std::unique_ptr(new AlwaysOnSampler), std::unique_ptr id_generator = std::unique_ptr(new RandomIdGenerator()), - std::unique_ptr tracer_configurator = - std::make_unique(TracerConfig::DefaultConfigurator())) noexcept; + std::unique_ptr> tracer_configurator = + std::make_unique>( + TracerConfig::DefaultConfigurator())) noexcept; explicit TracerProvider( std::vector> &&processors, @@ -61,8 +62,9 @@ class OPENTELEMETRY_EXPORT TracerProvider final : public opentelemetry::trace::T std::unique_ptr sampler = std::unique_ptr(new AlwaysOnSampler), std::unique_ptr id_generator = std::unique_ptr(new RandomIdGenerator()), - std::unique_ptr tracer_configurator = - std::make_unique(TracerConfig::DefaultConfigurator())) noexcept; + std::unique_ptr> tracer_configurator = + std::make_unique>( + TracerConfig::DefaultConfigurator())) noexcept; /** * Initialize a new tracer provider with a specified context diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_provider_factory.h b/sdk/include/opentelemetry/sdk/trace/tracer_provider_factory.h index c8f2bfdb50..92140c188c 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_provider_factory.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_provider_factory.h @@ -53,7 +53,7 @@ class OPENTELEMETRY_EXPORT TracerProviderFactory const opentelemetry::sdk::resource::Resource &resource, std::unique_ptr sampler, std::unique_ptr id_generator, - std::unique_ptr tracer_configurator); + std::unique_ptr> tracer_configurator); /* Series of creator methods with a single processor. */ @@ -80,7 +80,7 @@ class OPENTELEMETRY_EXPORT TracerProviderFactory const opentelemetry::sdk::resource::Resource &resource, std::unique_ptr sampler, std::unique_ptr id_generator, - std::unique_ptr tracer_configurator); + std::unique_ptr> tracer_configurator); /* Create with a tracer context. */ diff --git a/sdk/src/trace/tracer_config.cc b/sdk/src/trace/tracer_config.cc index 22a9419076..17382b65c2 100644 --- a/sdk/src/trace/tracer_config.cc +++ b/sdk/src/trace/tracer_config.cc @@ -13,8 +13,9 @@ namespace trace const TracerConfig TracerConfig::kDefaultConfig = TracerConfig(); const TracerConfig TracerConfig::kDisabledConfig = TracerConfig(true); -const TracerConfigurator TracerConfig::kDefaultTracerConfigurator = - [](const instrumentationscope::InstrumentationScope &) { return Default(); }; +const instrumentationscope::ScopeConfigurator + TracerConfig::kDefaultTracerConfigurator = + [](const instrumentationscope::InstrumentationScope &) { return Default(); }; TracerConfig TracerConfig::Disabled() { @@ -31,7 +32,7 @@ TracerConfig TracerConfig::Default() return kDefaultConfig; } -const TracerConfigurator &TracerConfig::DefaultConfigurator() +const instrumentationscope::ScopeConfigurator &TracerConfig::DefaultConfigurator() { return kDefaultTracerConfigurator; } diff --git a/sdk/src/trace/tracer_context.cc b/sdk/src/trace/tracer_context.cc index b6b1e3fda2..e5f55897ff 100644 --- a/sdk/src/trace/tracer_context.cc +++ b/sdk/src/trace/tracer_context.cc @@ -26,7 +26,8 @@ TracerContext::TracerContext(std::vector> &&proce const resource::Resource &resource, std::unique_ptr sampler, std::unique_ptr id_generator, - std::unique_ptr tracer_configurator) noexcept + std::unique_ptr> + tracer_configurator) noexcept : resource_(resource), sampler_(std::move(sampler)), id_generator_(std::move(id_generator)), @@ -44,7 +45,8 @@ const resource::Resource &TracerContext::GetResource() const noexcept return resource_; } -TracerConfigurator &TracerContext::GetTracerConfigurator() const noexcept +instrumentationscope::ScopeConfigurator &TracerContext::GetTracerConfigurator() + const noexcept { return *tracer_configurator_; } diff --git a/sdk/src/trace/tracer_context_factory.cc b/sdk/src/trace/tracer_context_factory.cc index f5441a323f..3caa4168f5 100644 --- a/sdk/src/trace/tracer_context_factory.cc +++ b/sdk/src/trace/tracer_context_factory.cc @@ -53,7 +53,8 @@ std::unique_ptr TracerContextFactory::Create( std::unique_ptr id_generator) { auto tracer_configurator = - std::make_unique(TracerConfig::DefaultConfigurator()); + std::make_unique>( + TracerConfig::DefaultConfigurator()); return Create(std::move(processors), resource, std::move(sampler), std::move(id_generator), std::move(tracer_configurator)); } @@ -63,7 +64,7 @@ std::unique_ptr TracerContextFactory::Create( const opentelemetry::sdk::resource::Resource &resource, std::unique_ptr sampler, std::unique_ptr id_generator, - std::unique_ptr tracer_configurator) + std::unique_ptr> tracer_configurator) { std::unique_ptr context( new TracerContext(std::move(processors), resource, std::move(sampler), diff --git a/sdk/src/trace/tracer_provider.cc b/sdk/src/trace/tracer_provider.cc index 4394cda2a9..9c6bc77c9b 100644 --- a/sdk/src/trace/tracer_provider.cc +++ b/sdk/src/trace/tracer_provider.cc @@ -39,11 +39,13 @@ TracerProvider::TracerProvider(std::unique_ptr context) noexcept OTEL_INTERNAL_LOG_DEBUG("[TracerProvider] TracerProvider created."); } -TracerProvider::TracerProvider(std::unique_ptr processor, - const resource::Resource &resource, - std::unique_ptr sampler, - std::unique_ptr id_generator, - std::unique_ptr tracer_configurator) noexcept +TracerProvider::TracerProvider( + std::unique_ptr processor, + const resource::Resource &resource, + std::unique_ptr sampler, + std::unique_ptr id_generator, + std::unique_ptr> + tracer_configurator) noexcept { std::vector> processors; processors.push_back(std::move(processor)); @@ -52,11 +54,13 @@ TracerProvider::TracerProvider(std::unique_ptr processor, std::move(id_generator), std::move(tracer_configurator)); } -TracerProvider::TracerProvider(std::vector> &&processors, - const resource::Resource &resource, - std::unique_ptr sampler, - std::unique_ptr id_generator, - std::unique_ptr tracer_configurator) noexcept +TracerProvider::TracerProvider( + std::vector> &&processors, + const resource::Resource &resource, + std::unique_ptr sampler, + std::unique_ptr id_generator, + std::unique_ptr> + tracer_configurator) noexcept { context_ = std::make_shared(std::move(processors), resource, std::move(sampler), diff --git a/sdk/src/trace/tracer_provider_factory.cc b/sdk/src/trace/tracer_provider_factory.cc index a196d9e26e..f13ce2395e 100644 --- a/sdk/src/trace/tracer_provider_factory.cc +++ b/sdk/src/trace/tracer_provider_factory.cc @@ -54,7 +54,8 @@ std::unique_ptr TracerProviderFactory std::unique_ptr id_generator) { auto tracer_configurator = - std::make_unique(TracerConfig::DefaultConfigurator()); + std::make_unique>( + TracerConfig::DefaultConfigurator()); return Create(std::move(processor), resource, std::move(sampler), std::move(id_generator), std::move(tracer_configurator)); } @@ -64,7 +65,7 @@ std::unique_ptr TracerProviderFactory const opentelemetry::sdk::resource::Resource &resource, std::unique_ptr sampler, std::unique_ptr id_generator, - std::unique_ptr tracer_configurator) + std::unique_ptr> tracer_configurator) { std::unique_ptr provider( new opentelemetry::sdk::trace::TracerProvider(std::move(processor), resource, @@ -104,7 +105,8 @@ std::unique_ptr TracerProviderFactory std::unique_ptr id_generator) { auto tracer_configurator = - std::make_unique(TracerConfig::DefaultConfigurator()); + std::make_unique>( + TracerConfig::DefaultConfigurator()); return Create(std::move(processors), resource, std::move(sampler), std::move(id_generator), std::move(tracer_configurator)); } @@ -114,7 +116,7 @@ std::unique_ptr TracerProviderFactory const opentelemetry::sdk::resource::Resource &resource, std::unique_ptr sampler, std::unique_ptr id_generator, - std::unique_ptr tracer_configurator) + std::unique_ptr> tracer_configurator) { std::unique_ptr provider( new opentelemetry::sdk::trace::TracerProvider(std::move(processors), resource, diff --git a/sdk/test/trace/tracer_config_test.cc b/sdk/test/trace/tracer_config_test.cc index 8d78e87ab5..32f509cb26 100644 --- a/sdk/test/trace/tracer_config_test.cc +++ b/sdk/test/trace/tracer_config_test.cc @@ -67,7 +67,7 @@ class DefaultTracerConfiguratorTestFixture TEST_P(DefaultTracerConfiguratorTestFixture, VerifyDefaultConfiguratorBehavior) { instrumentation_scope::InstrumentationScope *scope = GetParam(); - trace_sdk::TracerConfigurator default_configurator = + instrumentation_scope::ScopeConfigurator default_configurator = trace_sdk::TracerConfig::DefaultConfigurator(); ASSERT_EQ(default_configurator(*scope), trace_sdk::TracerConfig::Default()); From 4de93cb7adee44fd285d2178e98e70b62a8df789 Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Fri, 15 Nov 2024 19:50:18 +0000 Subject: [PATCH 14/32] Infer trace_config from the context --- sdk/include/opentelemetry/sdk/trace/tracer.h | 5 ++--- .../opentelemetry/sdk/trace/tracer_provider.h | 3 --- sdk/src/trace/tracer.cc | 14 +++++++------- sdk/src/trace/tracer_provider.cc | 12 +----------- 4 files changed, 10 insertions(+), 24 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/trace/tracer.h b/sdk/include/opentelemetry/sdk/trace/tracer.h index 64cd69afdc..52ad8f24f9 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer.h @@ -37,8 +37,7 @@ class Tracer final : public opentelemetry::trace::Tracer, /** Construct a new Tracer with the given context pipeline. */ explicit Tracer(std::shared_ptr context, std::unique_ptr instrumentation_scope = - InstrumentationScope::Create(""), - TracerConfig tracer_config = TracerConfig::Default()) noexcept; + InstrumentationScope::Create("")) noexcept; nostd::shared_ptr StartSpan( nostd::string_view name, @@ -106,9 +105,9 @@ class Tracer final : public opentelemetry::trace::Tracer, private: // order of declaration is important here - instrumentation scope should destroy after // tracer-context. + std::unique_ptr tracer_config_; std::shared_ptr instrumentation_scope_; std::shared_ptr context_; - TracerConfig tracer_config_; static const std::shared_ptr kNoopTracer; }; } // namespace trace diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h index 838c46e37b..ac018cf47a 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h @@ -123,9 +123,6 @@ class OPENTELEMETRY_EXPORT TracerProvider final : public opentelemetry::trace::T std::vector> tracers_; std::shared_ptr context_; std::mutex lock_; - - // private helper to get TracerConfig from InstrumentationScope using tracer configurator. - TracerConfig GetTracerConfig(const InstrumentationScope &instrumentation_scope) const; }; } // namespace trace } // namespace sdk diff --git a/sdk/src/trace/tracer.cc b/sdk/src/trace/tracer.cc index aadea2a19d..3989081b75 100644 --- a/sdk/src/trace/tracer.cc +++ b/sdk/src/trace/tracer.cc @@ -41,12 +41,12 @@ const std::shared_ptr Tracer::kNoopTracer = std::make_shared(); Tracer::Tracer(std::shared_ptr context, - std::unique_ptr instrumentation_scope, - TracerConfig tracer_config) noexcept - : instrumentation_scope_{std::move(instrumentation_scope)}, - context_{std::move(context)}, - tracer_config_(tracer_config) -{} + std::unique_ptr instrumentation_scope) noexcept + : instrumentation_scope_{std::move(instrumentation_scope)}, context_{std::move(context)} +{ + tracer_config_ = + std::make_unique(context_->GetTracerConfigurator()(*instrumentation_scope_)); +} nostd::shared_ptr Tracer::StartSpan( nostd::string_view name, @@ -57,7 +57,7 @@ nostd::shared_ptr Tracer::StartSpan( opentelemetry::trace::SpanContext parent_context = GetCurrentSpan()->GetContext(); if (nostd::holds_alternative(options.parent)) { - if (!tracer_config_.IsEnabled()) + if (!tracer_config_->IsEnabled()) { return kNoopTracer->StartSpan(name, attributes, links, options); } diff --git a/sdk/src/trace/tracer_provider.cc b/sdk/src/trace/tracer_provider.cc index 9c6bc77c9b..d12ad37d31 100644 --- a/sdk/src/trace/tracer_provider.cc +++ b/sdk/src/trace/tracer_provider.cc @@ -119,11 +119,8 @@ nostd::shared_ptr TracerProvider::GetTracer( instrumentationscope::InstrumentationScopeAttributes attrs_map(attributes); auto scope = instrumentationscope::InstrumentationScope::Create(name, version, schema_url, attrs_map); - const instrumentationscope::InstrumentationScope &scope_reference = - instrumentationscope::InstrumentationScope(*scope); - auto tracer = std::shared_ptr( - new Tracer(context_, std::move(scope), GetTracerConfig(scope_reference))); + auto tracer = std::shared_ptr(new Tracer(context_, std::move(scope))); tracers_.push_back(tracer); return nostd::shared_ptr{tracer}; } @@ -147,13 +144,6 @@ bool TracerProvider::ForceFlush(std::chrono::microseconds timeout) noexcept { return context_->ForceFlush(timeout); } - -TracerConfig TracerProvider::GetTracerConfig( - const InstrumentationScope &instrumentation_scope) const -{ - return context_->GetTracerConfigurator()(instrumentation_scope); -} - } // namespace trace } // namespace sdk OPENTELEMETRY_END_NAMESPACE From cfd9730901ed6c8e090755b308a0a27572297c3e Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Fri, 15 Nov 2024 22:12:08 +0000 Subject: [PATCH 15/32] Add tests for Tracer --- sdk/test/trace/tracer_test.cc | 53 ++++++++++++++++++++++++++++++++--- 1 file changed, 49 insertions(+), 4 deletions(-) diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index 2c4de8e83c..a26f93552b 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -54,8 +54,11 @@ #include "opentelemetry/trace/trace_state.h" #include "opentelemetry/trace/tracer.h" +#include + using namespace opentelemetry::sdk::trace; using namespace opentelemetry::sdk::resource; +using namespace opentelemetry::sdk::instrumentationscope; using opentelemetry::common::SteadyTimestamp; using opentelemetry::common::SystemTimestamp; namespace nostd = opentelemetry::nostd; @@ -142,15 +145,18 @@ std::shared_ptr initTracer( std::unique_ptr &&exporter, // For testing, just shove a pointer over, we'll take it over. Sampler *sampler, - IdGenerator *id_generator = new RandomIdGenerator) + IdGenerator *id_generator = new RandomIdGenerator, + const ScopeConfigurator &tracer_configurator = + TracerConfig::DefaultConfigurator()) { auto processor = std::unique_ptr(new SimpleSpanProcessor(std::move(exporter))); std::vector> processors; processors.push_back(std::move(processor)); auto resource = Resource::Create({}); - auto context = std::make_shared(std::move(processors), resource, - std::unique_ptr(sampler), - std::unique_ptr(id_generator)); + auto context = std::make_shared( + std::move(processors), resource, std::unique_ptr(sampler), + std::unique_ptr(id_generator), + std::make_unique>(tracer_configurator)); return std::shared_ptr(new Tracer(context)); } @@ -488,6 +494,45 @@ TEST(Tracer, SpanSetEvents) ASSERT_EQ(1, span_data_events[2].GetAttributes().size()); } +TEST(Tracer, StartSpanWithDisabledConfig) +{ +#ifdef OPENTELEMETRY_RTTI_ENABLED + InMemorySpanExporter *exporter = new InMemorySpanExporter(); + std::shared_ptr span_data = exporter->GetData(); + ScopeConfigurator disable_tracer = [](const InstrumentationScope &) { + return TracerConfig::Disabled(); + }; + auto tracer = initTracer(std::unique_ptr{exporter}, new AlwaysOnSampler(), + new RandomIdGenerator(), disable_tracer); + auto span = tracer->StartSpan("span 1"); + auto &span_ref = *span.get(); + + EXPECT_NE(typeid(span_ref), typeid(trace_api::Span)); + EXPECT_EQ(typeid(span_ref), typeid(trace_api::NoopSpan)); +#else + GTEST_SKIP() << "cannot use 'typeid' with '-fno-rtti'"; +#endif +} + +TEST(Tracer, StartSpanWithEnabledConfig) +{ +#ifdef OPENTELEMETRY_RTTI_ENABLED + InMemorySpanExporter *exporter = new InMemorySpanExporter(); + std::shared_ptr span_data = exporter->GetData(); + ScopeConfigurator enable_tracer = [](const InstrumentationScope &) { + return TracerConfig::Enabled(); + }; + auto tracer = initTracer(std::unique_ptr{exporter}, new AlwaysOnSampler(), + new RandomIdGenerator(), enable_tracer); + auto span = tracer->StartSpan("span 1"); + auto &span_ref = *span.get(); + + EXPECT_EQ(typeid(span_ref), typeid(Span)); +#else + GTEST_SKIP() << "cannot use 'typeid' with '-fno-rtti'"; +#endif +} + TEST(Tracer, SpanSetLinks) { InMemorySpanExporter *exporter = new InMemorySpanExporter(); From 47acb3f8f51e67972b0c32a021daa6452daeacf1 Mon Sep 17 00:00:00 2001 From: psx95 Date: Sat, 16 Nov 2024 13:42:56 -0500 Subject: [PATCH 16/32] Fix iwyu warnings --- sdk/src/trace/tracer.cc | 1 + sdk/src/trace/tracer_context.cc | 1 + sdk/src/trace/tracer_context_factory.cc | 1 + sdk/src/trace/tracer_provider.cc | 1 - sdk/src/trace/tracer_provider_factory.cc | 1 + sdk/test/trace/tracer_config_test.cc | 10 +++++++++- sdk/test/trace/tracer_test.cc | 3 +++ 7 files changed, 16 insertions(+), 2 deletions(-) diff --git a/sdk/src/trace/tracer.cc b/sdk/src/trace/tracer.cc index 3989081b75..98247bdf93 100644 --- a/sdk/src/trace/tracer.cc +++ b/sdk/src/trace/tracer.cc @@ -3,6 +3,7 @@ #include #include +#include #include #include #include diff --git a/sdk/src/trace/tracer_context.cc b/sdk/src/trace/tracer_context.cc index e5f55897ff..22ff46dae0 100644 --- a/sdk/src/trace/tracer_context.cc +++ b/sdk/src/trace/tracer_context.cc @@ -6,6 +6,7 @@ #include #include +#include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" #include "opentelemetry/sdk/resource/resource.h" #include "opentelemetry/sdk/trace/id_generator.h" #include "opentelemetry/sdk/trace/multi_span_processor.h" diff --git a/sdk/src/trace/tracer_context_factory.cc b/sdk/src/trace/tracer_context_factory.cc index 3caa4168f5..6d16493ac0 100644 --- a/sdk/src/trace/tracer_context_factory.cc +++ b/sdk/src/trace/tracer_context_factory.cc @@ -5,6 +5,7 @@ #include #include +#include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" #include "opentelemetry/sdk/resource/resource.h" #include "opentelemetry/sdk/trace/id_generator.h" #include "opentelemetry/sdk/trace/processor.h" diff --git a/sdk/src/trace/tracer_provider.cc b/sdk/src/trace/tracer_provider.cc index d12ad37d31..6d16882511 100644 --- a/sdk/src/trace/tracer_provider.cc +++ b/sdk/src/trace/tracer_provider.cc @@ -3,7 +3,6 @@ #include #include -#include #include #include #include diff --git a/sdk/src/trace/tracer_provider_factory.cc b/sdk/src/trace/tracer_provider_factory.cc index f13ce2395e..993646dd60 100644 --- a/sdk/src/trace/tracer_provider_factory.cc +++ b/sdk/src/trace/tracer_provider_factory.cc @@ -5,6 +5,7 @@ #include #include +#include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" #include "opentelemetry/sdk/resource/resource.h" #include "opentelemetry/sdk/trace/id_generator.h" #include "opentelemetry/sdk/trace/processor.h" diff --git a/sdk/test/trace/tracer_config_test.cc b/sdk/test/trace/tracer_config_test.cc index 32f509cb26..d1fe3259e1 100644 --- a/sdk/test/trace/tracer_config_test.cc +++ b/sdk/test/trace/tracer_config_test.cc @@ -1,8 +1,16 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +#include "opentelemetry/sdk/trace/tracer_config.h" #include -#include +#include +#include +#include +#include +#include "opentelemetry/common/attribute_value.h" +#include "opentelemetry/nostd/string_view.h" +#include "opentelemetry/nostd/unique_ptr.h" +#include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" namespace trace_sdk = opentelemetry::sdk::trace; namespace instrumentation_scope = opentelemetry::sdk::instrumentationscope; diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index a26f93552b..feffeffbdf 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -28,6 +28,7 @@ #include "opentelemetry/nostd/unique_ptr.h" #include "opentelemetry/nostd/utility.h" #include "opentelemetry/nostd/variant.h" +#include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" #include "opentelemetry/sdk/resource/resource.h" #include "opentelemetry/sdk/trace/exporter.h" #include "opentelemetry/sdk/trace/id_generator.h" @@ -41,8 +42,10 @@ #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_config.h" #include "opentelemetry/sdk/trace/tracer_context.h" #include "opentelemetry/trace/context.h" +#include "opentelemetry/trace/noop.h" #include "opentelemetry/trace/scope.h" #include "opentelemetry/trace/span.h" #include "opentelemetry/trace/span_context.h" From f7dde197d1ecb6bf320353e4acee869c015b2f34 Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Mon, 18 Nov 2024 15:30:43 +0000 Subject: [PATCH 17/32] Add unit test for custom configurator --- sdk/test/trace/tracer_test.cc | 43 +++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index feffeffbdf..118f18ba83 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -150,7 +150,8 @@ std::shared_ptr initTracer( Sampler *sampler, IdGenerator *id_generator = new RandomIdGenerator, const ScopeConfigurator &tracer_configurator = - TracerConfig::DefaultConfigurator()) + TracerConfig::DefaultConfigurator(), + std::unique_ptr scope = InstrumentationScope::Create("")) { auto processor = std::unique_ptr(new SimpleSpanProcessor(std::move(exporter))); std::vector> processors; @@ -160,7 +161,7 @@ std::shared_ptr initTracer( std::move(processors), resource, std::unique_ptr(sampler), std::unique_ptr(id_generator), std::make_unique>(tracer_configurator)); - return std::shared_ptr(new Tracer(context)); + return std::shared_ptr(new Tracer(context, std::move(scope))); } } // namespace @@ -536,6 +537,44 @@ TEST(Tracer, StartSpanWithEnabledConfig) #endif } +TEST(Tracer, StartSpanWithCustomConfig) +{ +#ifdef OPENTELEMETRY_RTTI_ENABLED + ScopeConfigurator custom_configurator = [](const InstrumentationScope &scope) { + if (scope.GetName() == "" || scope.GetName() == "foo_library") + { + return TracerConfig::Disabled(); + } + return TracerConfig::Enabled(); + }; + + const auto tracer_default_scope = + initTracer(std::unique_ptr{new InMemorySpanExporter()}, new AlwaysOnSampler(), + new RandomIdGenerator(), custom_configurator); + const auto span_default_scope = tracer_default_scope->StartSpan("span 1"); + const auto &span_default_scope_ref = *span_default_scope.get(); + EXPECT_EQ(typeid(span_default_scope_ref), typeid(trace_api::NoopSpan)); + + auto foo_scope = InstrumentationScope::Create("foo_library"); + const auto tracer_foo_scope = + initTracer(std::unique_ptr{new InMemorySpanExporter()}, new AlwaysOnSampler(), + new RandomIdGenerator(), custom_configurator, std::move(foo_scope)); + const auto span_foo_scope = tracer_foo_scope->StartSpan("span 1"); + auto &span_foo_scope_ref = *span_foo_scope.get(); + EXPECT_EQ(typeid(span_foo_scope_ref), typeid(trace_api::NoopSpan)); + + auto bar_scope = InstrumentationScope::Create("bar_library"); + auto tracer_bar_scope = + initTracer(std::unique_ptr{new InMemorySpanExporter()}, new AlwaysOnSampler(), + new RandomIdGenerator(), custom_configurator, std::move(bar_scope)); + auto span_bar_scope = tracer_bar_scope->StartSpan("span 1"); + auto &span_bar_scope_ref = *span_bar_scope.get(); + EXPECT_EQ(typeid(span_bar_scope_ref), typeid(opentelemetry::sdk::trace::Span)); +#else + GTEST_SKIP() << "cannot use 'typeid' with '-fno-rtti'"; +#endif +} + TEST(Tracer, SpanSetLinks) { InMemorySpanExporter *exporter = new InMemorySpanExporter(); From d87997d0507314a97afb886f5ae351b9b80204dc Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Mon, 18 Nov 2024 20:18:38 +0000 Subject: [PATCH 18/32] Add documentation for the tracer_config.h --- .../opentelemetry/sdk/trace/tracer_config.h | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_config.h b/sdk/include/opentelemetry/sdk/trace/tracer_config.h index b5115f7c0c..d32a7dce2d 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_config.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_config.h @@ -13,20 +13,50 @@ namespace trace { /** * TracerConfig defines various configurable aspects of a Tracer's behavior. + * This class should not be used directly to configure a Tracer's behavior, instead a + * ScopeConfigurator should be used to compute the desired TracerConfig which can then be used to + * configure a Tracer. */ class TracerConfig { public: + bool operator==(const TracerConfig &other) const noexcept; + /** * Returns if the Tracer is enabled or disabled. Tracers are enabled by default. * @return a boolean indicating if the Tracer is enabled. Defaults to true. */ bool IsEnabled() const noexcept; - bool operator==(const TracerConfig &other) const noexcept; + /** + * Returns a TracerConfig that represents a disabled Tracer. A disabled tracer behaves like a + * no-op tracer. + * @return a static constant TracerConfig that represents a disabled tracer. + */ static TracerConfig Disabled(); + + /** + * Returns a TracerConfig that represents an enabled Tracer. + * @return a static constant TracerConfig that represents an enabled tracer. + */ static TracerConfig Enabled(); + + /** + * Returns a TracerConfig that represents a Tracer configured with the default behavior. + * The default behavior is guided by the OpenTelemetry specification. + * @return a static constant TracerConfig that represents a tracer configured with default + * behavior. + */ static TracerConfig Default(); + + /** + * Returns a ScopeConfigurator that can be used to get the default tracer configurator. + * A tracer configurator computes TracerConfig based on the InstrumentationScope. The default + * tracer configurator unconditionally computes the default TracerConfig for all scopes. + * + * @return a static constant TracerConfig that represents a tracer configurator with default + * behavior. + */ static const instrumentationscope::ScopeConfigurator &DefaultConfigurator(); private: From 2f1ee3889e6ce3216ae75402bbf2f30085abaa1e Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Mon, 18 Nov 2024 20:35:56 +0000 Subject: [PATCH 19/32] Update Changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2025c36b59..43e34243f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -149,6 +149,9 @@ Increment the: * [bazel] Update opentelemetry-proto in MODULE.bazel [#3163](https://github.com/open-telemetry/opentelemetry-cpp/pull/3163) +* [SDK] Add tracer scope configurator + [#3137](https://github.com/open-telemetry/opentelemetry-cpp/pull/3137) + Important changes: * [API] Jaeger Propagator should not be deprecated From 31c73a30a30af334dd87617f103d0f1f2926e501 Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Wed, 4 Dec 2024 21:22:44 +0000 Subject: [PATCH 20/32] Address comments in tracer: fix logic --- sdk/include/opentelemetry/sdk/trace/tracer.h | 2 +- sdk/src/trace/tracer.cc | 19 +++++++++---------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/trace/tracer.h b/sdk/include/opentelemetry/sdk/trace/tracer.h index 52ad8f24f9..26693e1896 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer.h @@ -105,9 +105,9 @@ class Tracer final : public opentelemetry::trace::Tracer, private: // order of declaration is important here - instrumentation scope should destroy after // tracer-context. - std::unique_ptr tracer_config_; std::shared_ptr instrumentation_scope_; std::shared_ptr context_; + TracerConfig tracer_config_; static const std::shared_ptr kNoopTracer; }; } // namespace trace diff --git a/sdk/src/trace/tracer.cc b/sdk/src/trace/tracer.cc index 98247bdf93..36d97ee26f 100644 --- a/sdk/src/trace/tracer.cc +++ b/sdk/src/trace/tracer.cc @@ -43,11 +43,10 @@ const std::shared_ptr Tracer::kNoopTracer = Tracer::Tracer(std::shared_ptr context, std::unique_ptr instrumentation_scope) noexcept - : instrumentation_scope_{std::move(instrumentation_scope)}, context_{std::move(context)} -{ - tracer_config_ = - std::make_unique(context_->GetTracerConfigurator()(*instrumentation_scope_)); -} + : instrumentation_scope_{std::move(instrumentation_scope)}, + context_{std::move(context)}, + tracer_config_(context_->GetTracerConfigurator()(*instrumentation_scope_)) +{} nostd::shared_ptr Tracer::StartSpan( nostd::string_view name, @@ -55,13 +54,13 @@ nostd::shared_ptr Tracer::StartSpan( const opentelemetry::trace::SpanContextKeyValueIterable &links, const opentelemetry::trace::StartSpanOptions &options) noexcept { + if (!tracer_config_.IsEnabled()) + { + return kNoopTracer->StartSpan(name, attributes, links, options); + } opentelemetry::trace::SpanContext parent_context = GetCurrentSpan()->GetContext(); if (nostd::holds_alternative(options.parent)) { - if (!tracer_config_->IsEnabled()) - { - return kNoopTracer->StartSpan(name, attributes, links, options); - } auto span_context = nostd::get(options.parent); if (span_context.IsValid()) { @@ -176,7 +175,7 @@ void Tracer::ForceFlushWithMicroseconds(uint64_t timeout) noexcept void Tracer::CloseWithMicroseconds(uint64_t timeout) noexcept { // Trace context is shared by many tracers.So we just call ForceFlush to flush all pending spans - // and do not shutdown it. + // and do not shutdown it. if (context_) { context_->ForceFlush( From 5f6a95f91f4a614965d3f1b742d2283116163cd4 Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Thu, 5 Dec 2024 21:35:24 +0000 Subject: [PATCH 21/32] Add scope_configurator_builder for better class structure --- .../instrumentation_scope.h | 3 - .../scope_configurator_builder.h | 115 ++++++++++++++++++ .../opentelemetry/sdk/trace/tracer_config.h | 11 -- .../opentelemetry/sdk/trace/tracer_context.h | 13 +- .../opentelemetry/sdk/trace/tracer_provider.h | 8 +- sdk/src/trace/tracer_config.cc | 9 -- sdk/src/trace/tracer_context.cc | 2 +- sdk/src/trace/tracer_context_factory.cc | 3 +- sdk/src/trace/tracer_provider_factory.cc | 6 +- sdk/test/trace/tracer_config_test.cc | 6 +- sdk/test/trace/tracer_test.cc | 5 +- 11 files changed, 142 insertions(+), 39 deletions(-) create mode 100644 sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator_builder.h diff --git a/sdk/include/opentelemetry/sdk/instrumentationscope/instrumentation_scope.h b/sdk/include/opentelemetry/sdk/instrumentationscope/instrumentation_scope.h index 595e4cc4ba..6b565dbc5a 100644 --- a/sdk/include/opentelemetry/sdk/instrumentationscope/instrumentation_scope.h +++ b/sdk/include/opentelemetry/sdk/instrumentationscope/instrumentation_scope.h @@ -158,9 +158,6 @@ class InstrumentationScope InstrumentationScopeAttributes attributes_; }; - -template -using ScopeConfigurator = std::function; } // namespace instrumentationscope } // namespace sdk diff --git a/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator_builder.h b/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator_builder.h new file mode 100644 index 0000000000..85f273cb8c --- /dev/null +++ b/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator_builder.h @@ -0,0 +1,115 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#pragma once +#include + +#include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" +#include "opentelemetry/version.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace instrumentationscope +{ + +// A scope configurator is a function that returns the scope config for a given instrumentation +// scope. +template +using ScopeConfigurator = std::function; + +template +class ScopeConfiguratorBuilder +{ +public: + /** + * Constructor for a builder object that cam be used to create a scope configurator. A minimally + * configured builder would build a ScopeConfigurator that applies the default_scope_config to + * every instrumentation scope. + * @param default_scope_config The default scope config that the built configurator should fall + * back on. + */ + explicit ScopeConfiguratorBuilder(T default_scope_config) noexcept + : default_scope_config_(default_scope_config) + {} + + /** + * Allows the user to pass a generic function that evaluates an instrumentation scope through a + * boolean check. If the check passes, the provided config is applied. Conditions are evaluated in + * order. + * @param scope_matcher a function that returns true if the scope being evaluated matches the + * criteria defined by the function. + * @param scope_config the scope configuration to return for the matched scope. + * @return this + */ + ScopeConfiguratorBuilder AddCondition( + std::function scope_matcher, + T scope_config) + { + conditions_.push_back(ScopeConfiguratorBuilder::Condition(scope_matcher, scope_config)); + return this; + } + + /** + * A convenience condition that specifically matches the scope name of the scope being evaluated. + * If the scope name matches to the provided string, then the provided scope configuration is + * applied to the scope. + * @param scope_name The scope name to which the config needs to be applied. + * @param scope_config The scope config for the matching scopes. + * @return this + */ + ScopeConfiguratorBuilder AddConditionNameEquals(nostd::string_view scope_name, T scope_config) + { + auto name_equals_matcher = [scope_name](const InstrumentationScope &scope_info) { + return scope_info.GetName() == scope_name; + }; + conditions_.push_back(ScopeConfiguratorBuilder::Condition(name_equals_matcher, scope_config)); + return this; + } + + /** + * Constructs the scope configurator object that can be used to retrieve scope config depending on + * the instrumentation scope. + * @return a configured scope configurator. + */ + ScopeConfigurator Build() + { + if (conditions_.size() == 0) + { + return [default_scope_config_ = this->default_scope_config_](const InstrumentationScope &) { + return default_scope_config_; + }; + } + + // Return a configurator that processes all the conditions + return [conditions_ = this->conditions_, default_scope_config_ = this->default_scope_config_]( + const InstrumentationScope &scope_info) { + for (Condition condition : conditions_) + { + if (condition.scope_matcher(scope_info)) + { + return condition.scope_config; + } + } + return default_scope_config_; + }; + } + +private: + /** + * An internal struct to encapsulate 'conditions' that can be applied to a + * ScopeConfiguratorBuilder. The applied conditions influence the behavior of the generatred + * ScopeConfigurator. + */ + struct Condition + { + std::function scope_matcher; + T scope_config; + }; + + T default_scope_config_; + std::vector conditions_; +}; +} // namespace instrumentationscope +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_config.h b/sdk/include/opentelemetry/sdk/trace/tracer_config.h index d32a7dce2d..69125f97d0 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_config.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_config.h @@ -49,22 +49,11 @@ class TracerConfig */ static TracerConfig Default(); - /** - * Returns a ScopeConfigurator that can be used to get the default tracer configurator. - * A tracer configurator computes TracerConfig based on the InstrumentationScope. The default - * tracer configurator unconditionally computes the default TracerConfig for all scopes. - * - * @return a static constant TracerConfig that represents a tracer configurator with default - * behavior. - */ - static const instrumentationscope::ScopeConfigurator &DefaultConfigurator(); - private: explicit TracerConfig(const bool disabled = false) : disabled_(disabled) {} bool disabled_; static const TracerConfig kDefaultConfig; static const TracerConfig kDisabledConfig; - static const instrumentationscope::ScopeConfigurator kDefaultTracerConfigurator; }; } // 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 25314fa846..fbbd1e9ee0 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_context.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_context.h @@ -7,6 +7,7 @@ #include #include +#include "opentelemetry/sdk/instrumentationscope/scope_configurator_builder.h" #include "opentelemetry/sdk/resource/resource.h" #include "opentelemetry/sdk/trace/id_generator.h" #include "opentelemetry/sdk/trace/processor.h" @@ -22,8 +23,6 @@ namespace sdk namespace trace { -using namespace opentelemetry::sdk::instrumentationscope; - /** * A class which stores the TracerProvider context. * @@ -49,7 +48,8 @@ class TracerContext std::unique_ptr(new RandomIdGenerator()), std::unique_ptr> tracer_configurator = std::make_unique>( - TracerConfig::DefaultConfigurator())) noexcept; + instrumentationscope::ScopeConfiguratorBuilder(TracerConfig::Default()) + .Build())) noexcept; virtual ~TracerContext() = default; @@ -83,7 +83,12 @@ class TracerContext */ const opentelemetry::sdk::resource::Resource &GetResource() const noexcept; - instrumentationscope::ScopeConfigurator &GetTracerConfigurator() const noexcept; + /** + * Obtain the ScopeConfigurator with this tracer context. + * @return The ScopeConfigurator for this tracer context. + */ + const instrumentationscope::ScopeConfigurator &GetTracerConfigurator() + const noexcept; /** * Obtain the Id Generator associated with this tracer context. diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h index ac018cf47a..e4bf63f29c 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h @@ -27,8 +27,6 @@ namespace sdk namespace trace { -using namespace opentelemetry::sdk::instrumentationscope; - class OPENTELEMETRY_EXPORT TracerProvider final : public opentelemetry::trace::TracerProvider { public: @@ -53,7 +51,8 @@ class OPENTELEMETRY_EXPORT TracerProvider final : public opentelemetry::trace::T std::unique_ptr(new RandomIdGenerator()), std::unique_ptr> tracer_configurator = std::make_unique>( - TracerConfig::DefaultConfigurator())) noexcept; + instrumentationscope::ScopeConfiguratorBuilder(TracerConfig::Default()) + .Build())) noexcept; explicit TracerProvider( std::vector> &&processors, @@ -64,7 +63,8 @@ class OPENTELEMETRY_EXPORT TracerProvider final : public opentelemetry::trace::T std::unique_ptr(new RandomIdGenerator()), std::unique_ptr> tracer_configurator = std::make_unique>( - TracerConfig::DefaultConfigurator())) noexcept; + instrumentationscope::ScopeConfiguratorBuilder(TracerConfig::Default()) + .Build())) noexcept; /** * Initialize a new tracer provider with a specified context diff --git a/sdk/src/trace/tracer_config.cc b/sdk/src/trace/tracer_config.cc index 17382b65c2..2e94f6ea5c 100644 --- a/sdk/src/trace/tracer_config.cc +++ b/sdk/src/trace/tracer_config.cc @@ -13,10 +13,6 @@ namespace trace const TracerConfig TracerConfig::kDefaultConfig = TracerConfig(); const TracerConfig TracerConfig::kDisabledConfig = TracerConfig(true); -const instrumentationscope::ScopeConfigurator - TracerConfig::kDefaultTracerConfigurator = - [](const instrumentationscope::InstrumentationScope &) { return Default(); }; - TracerConfig TracerConfig::Disabled() { return kDisabledConfig; @@ -32,11 +28,6 @@ TracerConfig TracerConfig::Default() return kDefaultConfig; } -const instrumentationscope::ScopeConfigurator &TracerConfig::DefaultConfigurator() -{ - return kDefaultTracerConfigurator; -} - bool TracerConfig::IsEnabled() const noexcept { return !disabled_; diff --git a/sdk/src/trace/tracer_context.cc b/sdk/src/trace/tracer_context.cc index 22ff46dae0..37d4d00299 100644 --- a/sdk/src/trace/tracer_context.cc +++ b/sdk/src/trace/tracer_context.cc @@ -46,7 +46,7 @@ const resource::Resource &TracerContext::GetResource() const noexcept return resource_; } -instrumentationscope::ScopeConfigurator &TracerContext::GetTracerConfigurator() +const instrumentationscope::ScopeConfigurator &TracerContext::GetTracerConfigurator() const noexcept { return *tracer_configurator_; diff --git a/sdk/src/trace/tracer_context_factory.cc b/sdk/src/trace/tracer_context_factory.cc index 6d16493ac0..209ef0ff6d 100644 --- a/sdk/src/trace/tracer_context_factory.cc +++ b/sdk/src/trace/tracer_context_factory.cc @@ -55,7 +55,8 @@ std::unique_ptr TracerContextFactory::Create( { auto tracer_configurator = std::make_unique>( - TracerConfig::DefaultConfigurator()); + instrumentationscope::ScopeConfiguratorBuilder(TracerConfig::Default()) + .Build()); return Create(std::move(processors), resource, std::move(sampler), std::move(id_generator), std::move(tracer_configurator)); } diff --git a/sdk/src/trace/tracer_provider_factory.cc b/sdk/src/trace/tracer_provider_factory.cc index 993646dd60..58ab2b8e1c 100644 --- a/sdk/src/trace/tracer_provider_factory.cc +++ b/sdk/src/trace/tracer_provider_factory.cc @@ -56,7 +56,8 @@ std::unique_ptr TracerProviderFactory { auto tracer_configurator = std::make_unique>( - TracerConfig::DefaultConfigurator()); + instrumentationscope::ScopeConfiguratorBuilder(TracerConfig::Default()) + .Build()); return Create(std::move(processor), resource, std::move(sampler), std::move(id_generator), std::move(tracer_configurator)); } @@ -107,7 +108,8 @@ std::unique_ptr TracerProviderFactory { auto tracer_configurator = std::make_unique>( - TracerConfig::DefaultConfigurator()); + instrumentationscope::ScopeConfiguratorBuilder(TracerConfig::Default()) + .Build()); return Create(std::move(processors), resource, std::move(sampler), std::move(id_generator), std::move(tracer_configurator)); } diff --git a/sdk/test/trace/tracer_config_test.cc b/sdk/test/trace/tracer_config_test.cc index d1fe3259e1..53d640d253 100644 --- a/sdk/test/trace/tracer_config_test.cc +++ b/sdk/test/trace/tracer_config_test.cc @@ -10,7 +10,7 @@ #include "opentelemetry/common/attribute_value.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/nostd/unique_ptr.h" -#include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" +#include "opentelemetry/sdk/instrumentationscope/scope_configurator_builder.h" namespace trace_sdk = opentelemetry::sdk::trace; namespace instrumentation_scope = opentelemetry::sdk::instrumentationscope; @@ -76,7 +76,9 @@ TEST_P(DefaultTracerConfiguratorTestFixture, VerifyDefaultConfiguratorBehavior) { instrumentation_scope::InstrumentationScope *scope = GetParam(); instrumentation_scope::ScopeConfigurator default_configurator = - trace_sdk::TracerConfig::DefaultConfigurator(); + instrumentation_scope::ScopeConfiguratorBuilder( + trace_sdk::TracerConfig::Default()) + .Build(); ASSERT_EQ(default_configurator(*scope), trace_sdk::TracerConfig::Default()); } diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index 118f18ba83..5afe622dc9 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -29,6 +29,7 @@ #include "opentelemetry/nostd/utility.h" #include "opentelemetry/nostd/variant.h" #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" +#include "opentelemetry/sdk/instrumentationscope/scope_configurator_builder.h" #include "opentelemetry/sdk/resource/resource.h" #include "opentelemetry/sdk/trace/exporter.h" #include "opentelemetry/sdk/trace/id_generator.h" @@ -113,7 +114,7 @@ class MockSampler final : public Sampler }; /** - * A Mock Custom Id Generator + * A Mock Custom ID Generator */ class MockIdGenerator : public IdGenerator { @@ -150,7 +151,7 @@ std::shared_ptr initTracer( Sampler *sampler, IdGenerator *id_generator = new RandomIdGenerator, const ScopeConfigurator &tracer_configurator = - TracerConfig::DefaultConfigurator(), + ScopeConfiguratorBuilder(TracerConfig::Default()).Build(), std::unique_ptr scope = InstrumentationScope::Create("")) { auto processor = std::unique_ptr(new SimpleSpanProcessor(std::move(exporter))); From 501fa473dcfca938e6a0eed428edf2f958704d3e Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Fri, 6 Dec 2024 02:09:04 +0000 Subject: [PATCH 22/32] ScopeConfigurator: make builder inner class Converting ScopeConfigurator from type alias to a dedicated class allows control over how it is initialized. This commits makes it so that ScopeConfigurator can only be initialized using the inner Builder class. This will prevent future confusion on how to initialize ScopeConfigurators. --- .../instrumentationscope/scope_configurator.h | 141 ++++++++++++++++++ .../scope_configurator_builder.h | 115 -------------- .../opentelemetry/sdk/trace/tracer_context.h | 5 +- .../opentelemetry/sdk/trace/tracer_provider.h | 6 +- sdk/src/trace/tracer.cc | 2 +- sdk/src/trace/tracer_context_factory.cc | 2 +- sdk/src/trace/tracer_provider_factory.cc | 4 +- sdk/test/trace/tracer_config_test.cc | 6 +- sdk/test/trace/tracer_test.cc | 34 ++--- 9 files changed, 170 insertions(+), 145 deletions(-) create mode 100644 sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h delete mode 100644 sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator_builder.h diff --git a/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h b/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h new file mode 100644 index 0000000000..a26bf10282 --- /dev/null +++ b/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h @@ -0,0 +1,141 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#pragma once +#include + +#include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" +#include "opentelemetry/version.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace instrumentationscope +{ +/** + * A scope configurator is a function that returns the scope config for a given instrumentation + * scope. + */ +template +class ScopeConfigurator +{ +public: + /** + * A builder class for the ScopeConfigurator that facilitates the creation of ScopeConfigurators. + */ + class Builder + { + public: + /** + * Constructor for a builder object that cam be used to create a scope configurator. A minimally + * configured builder would build a ScopeConfigurator that applies the default_scope_config to + * every instrumentation scope. + * @param default_scope_config The default scope config that the built configurator should fall + * back on. + */ + explicit Builder(T default_scope_config) noexcept : default_scope_config_(default_scope_config) + {} + + /** + * Allows the user to pass a generic function that evaluates an instrumentation scope through a + * boolean check. If the check passes, the provided config is applied. Conditions are evaluated + * in order. + * @param scope_matcher a function that returns true if the scope being evaluated matches the + * criteria defined by the function. + * @param scope_config the scope configuration to return for the matched scope. + * @return this + */ + Builder AddCondition(std::function scope_matcher, + T scope_config) + { + conditions_.push_back(Condition{scope_matcher, scope_config}); + return *this; + } + + /** + * A convenience condition that specifically matches the scope name of the scope being + * evaluated. If the scope name matches to the provided string, then the provided scope + * configuration is applied to the scope. + * @param scope_name The scope name to which the config needs to be applied. + * @param scope_config The scope config for the matching scopes. + * @return this + */ + Builder AddConditionNameEquals(nostd::string_view scope_name, T scope_config) + { + std::function name_equals_matcher = + [scope_name](const InstrumentationScope &scope_info) { + return scope_info.GetName() == scope_name; + }; + conditions_.push_back(Condition{name_equals_matcher, scope_config}); + return *this; + } + + /** + * Constructs the scope configurator object that can be used to retrieve scope config depending + * on the instrumentation scope. + * @return a configured scope configurator. + */ + ScopeConfigurator Build() + { + if (conditions_.size() == 0) + { + return ScopeConfigurator( + [default_scope_config_ = this->default_scope_config_](const InstrumentationScope &) { + return default_scope_config_; + }); + } + + // Return a configurator that processes all the conditions + return ScopeConfigurator( + [conditions_ = this->conditions_, default_scope_config_ = this->default_scope_config_]( + const InstrumentationScope &scope_info) { + for (Condition condition : conditions_) + { + if (condition.scope_matcher(scope_info)) + { + return condition.scope_config; + } + } + return default_scope_config_; + }); + } + + private: + /** + * An internal struct to encapsulate 'conditions' that can be applied to a + * ScopeConfiguratorBuilder. The applied conditions influence the behavior of the generatred + * ScopeConfigurator. + */ + struct Condition + { + std::function scope_matcher; + T scope_config; + }; + + T default_scope_config_; + std::vector conditions_; + }; + + // Public methods for ScopeConfigurator + + /** + * Invokes the underlying configurator function to get a valid scope configuration. + * @param scope_info The InstrumentationScope containing scope information for which configuration + * needs to be retrieved. + */ + T ComputeConfig(const InstrumentationScope &scope_info) const + { + return this->configurator_(scope_info); + } + +private: + // Prevent direct initialization of ScopeConfigurator objects. + explicit ScopeConfigurator(std::function configurator) + : configurator_(configurator) + {} + + std::function configurator_; +}; +} // namespace instrumentationscope +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator_builder.h b/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator_builder.h deleted file mode 100644 index 85f273cb8c..0000000000 --- a/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator_builder.h +++ /dev/null @@ -1,115 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -#pragma once -#include - -#include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" -#include "opentelemetry/version.h" - -OPENTELEMETRY_BEGIN_NAMESPACE -namespace sdk -{ -namespace instrumentationscope -{ - -// A scope configurator is a function that returns the scope config for a given instrumentation -// scope. -template -using ScopeConfigurator = std::function; - -template -class ScopeConfiguratorBuilder -{ -public: - /** - * Constructor for a builder object that cam be used to create a scope configurator. A minimally - * configured builder would build a ScopeConfigurator that applies the default_scope_config to - * every instrumentation scope. - * @param default_scope_config The default scope config that the built configurator should fall - * back on. - */ - explicit ScopeConfiguratorBuilder(T default_scope_config) noexcept - : default_scope_config_(default_scope_config) - {} - - /** - * Allows the user to pass a generic function that evaluates an instrumentation scope through a - * boolean check. If the check passes, the provided config is applied. Conditions are evaluated in - * order. - * @param scope_matcher a function that returns true if the scope being evaluated matches the - * criteria defined by the function. - * @param scope_config the scope configuration to return for the matched scope. - * @return this - */ - ScopeConfiguratorBuilder AddCondition( - std::function scope_matcher, - T scope_config) - { - conditions_.push_back(ScopeConfiguratorBuilder::Condition(scope_matcher, scope_config)); - return this; - } - - /** - * A convenience condition that specifically matches the scope name of the scope being evaluated. - * If the scope name matches to the provided string, then the provided scope configuration is - * applied to the scope. - * @param scope_name The scope name to which the config needs to be applied. - * @param scope_config The scope config for the matching scopes. - * @return this - */ - ScopeConfiguratorBuilder AddConditionNameEquals(nostd::string_view scope_name, T scope_config) - { - auto name_equals_matcher = [scope_name](const InstrumentationScope &scope_info) { - return scope_info.GetName() == scope_name; - }; - conditions_.push_back(ScopeConfiguratorBuilder::Condition(name_equals_matcher, scope_config)); - return this; - } - - /** - * Constructs the scope configurator object that can be used to retrieve scope config depending on - * the instrumentation scope. - * @return a configured scope configurator. - */ - ScopeConfigurator Build() - { - if (conditions_.size() == 0) - { - return [default_scope_config_ = this->default_scope_config_](const InstrumentationScope &) { - return default_scope_config_; - }; - } - - // Return a configurator that processes all the conditions - return [conditions_ = this->conditions_, default_scope_config_ = this->default_scope_config_]( - const InstrumentationScope &scope_info) { - for (Condition condition : conditions_) - { - if (condition.scope_matcher(scope_info)) - { - return condition.scope_config; - } - } - return default_scope_config_; - }; - } - -private: - /** - * An internal struct to encapsulate 'conditions' that can be applied to a - * ScopeConfiguratorBuilder. The applied conditions influence the behavior of the generatred - * ScopeConfigurator. - */ - struct Condition - { - std::function scope_matcher; - T scope_config; - }; - - T default_scope_config_; - std::vector conditions_; -}; -} // namespace instrumentationscope -} // namespace sdk -OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_context.h b/sdk/include/opentelemetry/sdk/trace/tracer_context.h index fbbd1e9ee0..90beb9a2ca 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_context.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_context.h @@ -7,7 +7,7 @@ #include #include -#include "opentelemetry/sdk/instrumentationscope/scope_configurator_builder.h" +#include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" #include "opentelemetry/sdk/resource/resource.h" #include "opentelemetry/sdk/trace/id_generator.h" #include "opentelemetry/sdk/trace/processor.h" @@ -48,7 +48,8 @@ class TracerContext std::unique_ptr(new RandomIdGenerator()), std::unique_ptr> tracer_configurator = std::make_unique>( - instrumentationscope::ScopeConfiguratorBuilder(TracerConfig::Default()) + instrumentationscope::ScopeConfigurator::Builder( + TracerConfig::Default()) .Build())) noexcept; virtual ~TracerContext() = default; diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h index e4bf63f29c..210228713c 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h @@ -51,7 +51,8 @@ class OPENTELEMETRY_EXPORT TracerProvider final : public opentelemetry::trace::T std::unique_ptr(new RandomIdGenerator()), std::unique_ptr> tracer_configurator = std::make_unique>( - instrumentationscope::ScopeConfiguratorBuilder(TracerConfig::Default()) + instrumentationscope::ScopeConfigurator::Builder( + TracerConfig::Default()) .Build())) noexcept; explicit TracerProvider( @@ -63,7 +64,8 @@ class OPENTELEMETRY_EXPORT TracerProvider final : public opentelemetry::trace::T std::unique_ptr(new RandomIdGenerator()), std::unique_ptr> tracer_configurator = std::make_unique>( - instrumentationscope::ScopeConfiguratorBuilder(TracerConfig::Default()) + instrumentationscope::ScopeConfigurator::Builder( + TracerConfig::Default()) .Build())) noexcept; /** diff --git a/sdk/src/trace/tracer.cc b/sdk/src/trace/tracer.cc index 36d97ee26f..02f32feff9 100644 --- a/sdk/src/trace/tracer.cc +++ b/sdk/src/trace/tracer.cc @@ -45,7 +45,7 @@ Tracer::Tracer(std::shared_ptr context, std::unique_ptr instrumentation_scope) noexcept : instrumentation_scope_{std::move(instrumentation_scope)}, context_{std::move(context)}, - tracer_config_(context_->GetTracerConfigurator()(*instrumentation_scope_)) + tracer_config_(context_->GetTracerConfigurator().ComputeConfig(*instrumentation_scope_)) {} nostd::shared_ptr Tracer::StartSpan( diff --git a/sdk/src/trace/tracer_context_factory.cc b/sdk/src/trace/tracer_context_factory.cc index 209ef0ff6d..a2585ce378 100644 --- a/sdk/src/trace/tracer_context_factory.cc +++ b/sdk/src/trace/tracer_context_factory.cc @@ -55,7 +55,7 @@ std::unique_ptr TracerContextFactory::Create( { auto tracer_configurator = std::make_unique>( - instrumentationscope::ScopeConfiguratorBuilder(TracerConfig::Default()) + instrumentationscope::ScopeConfigurator::Builder(TracerConfig::Default()) .Build()); return Create(std::move(processors), resource, std::move(sampler), std::move(id_generator), std::move(tracer_configurator)); diff --git a/sdk/src/trace/tracer_provider_factory.cc b/sdk/src/trace/tracer_provider_factory.cc index 58ab2b8e1c..8219d54866 100644 --- a/sdk/src/trace/tracer_provider_factory.cc +++ b/sdk/src/trace/tracer_provider_factory.cc @@ -56,7 +56,7 @@ std::unique_ptr TracerProviderFactory { auto tracer_configurator = std::make_unique>( - instrumentationscope::ScopeConfiguratorBuilder(TracerConfig::Default()) + instrumentationscope::ScopeConfigurator::Builder(TracerConfig::Default()) .Build()); return Create(std::move(processor), resource, std::move(sampler), std::move(id_generator), std::move(tracer_configurator)); @@ -108,7 +108,7 @@ std::unique_ptr TracerProviderFactory { auto tracer_configurator = std::make_unique>( - instrumentationscope::ScopeConfiguratorBuilder(TracerConfig::Default()) + instrumentationscope::ScopeConfigurator::Builder(TracerConfig::Default()) .Build()); return Create(std::move(processors), resource, std::move(sampler), std::move(id_generator), std::move(tracer_configurator)); diff --git a/sdk/test/trace/tracer_config_test.cc b/sdk/test/trace/tracer_config_test.cc index 53d640d253..c5b9fb95c3 100644 --- a/sdk/test/trace/tracer_config_test.cc +++ b/sdk/test/trace/tracer_config_test.cc @@ -10,7 +10,7 @@ #include "opentelemetry/common/attribute_value.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/nostd/unique_ptr.h" -#include "opentelemetry/sdk/instrumentationscope/scope_configurator_builder.h" +#include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" namespace trace_sdk = opentelemetry::sdk::trace; namespace instrumentation_scope = opentelemetry::sdk::instrumentationscope; @@ -76,11 +76,11 @@ TEST_P(DefaultTracerConfiguratorTestFixture, VerifyDefaultConfiguratorBehavior) { instrumentation_scope::InstrumentationScope *scope = GetParam(); instrumentation_scope::ScopeConfigurator default_configurator = - instrumentation_scope::ScopeConfiguratorBuilder( + instrumentation_scope::ScopeConfigurator::Builder( trace_sdk::TracerConfig::Default()) .Build(); - ASSERT_EQ(default_configurator(*scope), trace_sdk::TracerConfig::Default()); + ASSERT_EQ(default_configurator.ComputeConfig(*scope), trace_sdk::TracerConfig::Default()); } INSTANTIATE_TEST_SUITE_P(InstrumentationScopes, diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index 5afe622dc9..a4c2c0ffc4 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -29,7 +29,7 @@ #include "opentelemetry/nostd/utility.h" #include "opentelemetry/nostd/variant.h" #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" -#include "opentelemetry/sdk/instrumentationscope/scope_configurator_builder.h" +#include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" #include "opentelemetry/sdk/resource/resource.h" #include "opentelemetry/sdk/trace/exporter.h" #include "opentelemetry/sdk/trace/id_generator.h" @@ -151,7 +151,7 @@ std::shared_ptr initTracer( Sampler *sampler, IdGenerator *id_generator = new RandomIdGenerator, const ScopeConfigurator &tracer_configurator = - ScopeConfiguratorBuilder(TracerConfig::Default()).Build(), + ScopeConfigurator::Builder(TracerConfig::Default()).Build(), std::unique_ptr scope = InstrumentationScope::Create("")) { auto processor = std::unique_ptr(new SimpleSpanProcessor(std::move(exporter))); @@ -502,11 +502,10 @@ TEST(Tracer, SpanSetEvents) TEST(Tracer, StartSpanWithDisabledConfig) { #ifdef OPENTELEMETRY_RTTI_ENABLED - InMemorySpanExporter *exporter = new InMemorySpanExporter(); - std::shared_ptr span_data = exporter->GetData(); - ScopeConfigurator disable_tracer = [](const InstrumentationScope &) { - return TracerConfig::Disabled(); - }; + InMemorySpanExporter *exporter = new InMemorySpanExporter(); + std::shared_ptr span_data = exporter->GetData(); + ScopeConfigurator disable_tracer = + ScopeConfigurator::Builder(TracerConfig::Disabled()).Build(); auto tracer = initTracer(std::unique_ptr{exporter}, new AlwaysOnSampler(), new RandomIdGenerator(), disable_tracer); auto span = tracer->StartSpan("span 1"); @@ -522,11 +521,10 @@ TEST(Tracer, StartSpanWithDisabledConfig) TEST(Tracer, StartSpanWithEnabledConfig) { #ifdef OPENTELEMETRY_RTTI_ENABLED - InMemorySpanExporter *exporter = new InMemorySpanExporter(); - std::shared_ptr span_data = exporter->GetData(); - ScopeConfigurator enable_tracer = [](const InstrumentationScope &) { - return TracerConfig::Enabled(); - }; + InMemorySpanExporter *exporter = new InMemorySpanExporter(); + std::shared_ptr span_data = exporter->GetData(); + ScopeConfigurator enable_tracer = + ScopeConfigurator::Builder(TracerConfig::Enabled()).Build(); auto tracer = initTracer(std::unique_ptr{exporter}, new AlwaysOnSampler(), new RandomIdGenerator(), enable_tracer); auto span = tracer->StartSpan("span 1"); @@ -541,13 +539,11 @@ TEST(Tracer, StartSpanWithEnabledConfig) TEST(Tracer, StartSpanWithCustomConfig) { #ifdef OPENTELEMETRY_RTTI_ENABLED - ScopeConfigurator custom_configurator = [](const InstrumentationScope &scope) { - if (scope.GetName() == "" || scope.GetName() == "foo_library") - { - return TracerConfig::Disabled(); - } - return TracerConfig::Enabled(); - }; + ScopeConfigurator custom_configurator = + ScopeConfigurator::Builder(TracerConfig::Enabled()) + .AddConditionNameEquals("foo_library", TracerConfig::Disabled()) + .AddConditionNameEquals("", TracerConfig::Disabled()) + .Build(); const auto tracer_default_scope = initTracer(std::unique_ptr{new InMemorySpanExporter()}, new AlwaysOnSampler(), From c2bf6f9150d18bb8220f7fb73ded591bde6ba1f8 Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Fri, 6 Dec 2024 02:49:07 +0000 Subject: [PATCH 23/32] Fix iwyu warnings --- .../sdk/instrumentationscope/scope_configurator.h | 2 +- sdk/include/opentelemetry/sdk/trace/tracer_config.h | 1 - sdk/src/trace/tracer.cc | 2 +- sdk/src/trace/tracer_config.cc | 1 - sdk/src/trace/tracer_context.cc | 2 +- sdk/src/trace/tracer_context_factory.cc | 2 +- sdk/src/trace/tracer_provider.cc | 1 + sdk/src/trace/tracer_provider_factory.cc | 2 +- sdk/test/trace/tracer_config_test.cc | 3 ++- 9 files changed, 8 insertions(+), 8 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h b/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h index a26bf10282..3b8f611f3c 100644 --- a/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h +++ b/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h @@ -129,7 +129,7 @@ class ScopeConfigurator } private: - // Prevent direct initialization of ScopeConfigurator objects. + // Prevent direct initialization of ScopeConfigurator objects. explicit ScopeConfigurator(std::function configurator) : configurator_(configurator) {} diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_config.h b/sdk/include/opentelemetry/sdk/trace/tracer_config.h index 69125f97d0..708d05b5e8 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_config.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_config.h @@ -3,7 +3,6 @@ #pragma once -#include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" #include "opentelemetry/version.h" OPENTELEMETRY_BEGIN_NAMESPACE diff --git a/sdk/src/trace/tracer.cc b/sdk/src/trace/tracer.cc index 02f32feff9..c166885600 100644 --- a/sdk/src/trace/tracer.cc +++ b/sdk/src/trace/tracer.cc @@ -3,7 +3,6 @@ #include #include -#include #include #include #include @@ -14,6 +13,7 @@ #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/nostd/variant.h" #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" +#include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" #include "opentelemetry/sdk/trace/id_generator.h" #include "opentelemetry/sdk/trace/sampler.h" #include "opentelemetry/sdk/trace/tracer.h" diff --git a/sdk/src/trace/tracer_config.cc b/sdk/src/trace/tracer_config.cc index 2e94f6ea5c..8c9dd93652 100644 --- a/sdk/src/trace/tracer_config.cc +++ b/sdk/src/trace/tracer_config.cc @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 #include "opentelemetry/sdk/trace/tracer_config.h" -#include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk diff --git a/sdk/src/trace/tracer_context.cc b/sdk/src/trace/tracer_context.cc index 37d4d00299..c3cbfb9d76 100644 --- a/sdk/src/trace/tracer_context.cc +++ b/sdk/src/trace/tracer_context.cc @@ -6,7 +6,7 @@ #include #include -#include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" +#include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" #include "opentelemetry/sdk/resource/resource.h" #include "opentelemetry/sdk/trace/id_generator.h" #include "opentelemetry/sdk/trace/multi_span_processor.h" diff --git a/sdk/src/trace/tracer_context_factory.cc b/sdk/src/trace/tracer_context_factory.cc index a2585ce378..9472dc041f 100644 --- a/sdk/src/trace/tracer_context_factory.cc +++ b/sdk/src/trace/tracer_context_factory.cc @@ -5,7 +5,7 @@ #include #include -#include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" +#include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" #include "opentelemetry/sdk/resource/resource.h" #include "opentelemetry/sdk/trace/id_generator.h" #include "opentelemetry/sdk/trace/processor.h" diff --git a/sdk/src/trace/tracer_provider.cc b/sdk/src/trace/tracer_provider.cc index 6d16882511..4006c90007 100644 --- a/sdk/src/trace/tracer_provider.cc +++ b/sdk/src/trace/tracer_provider.cc @@ -12,6 +12,7 @@ #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/sdk/common/global_log_handler.h" #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" +#include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" #include "opentelemetry/sdk/resource/resource.h" #include "opentelemetry/sdk/trace/id_generator.h" #include "opentelemetry/sdk/trace/processor.h" diff --git a/sdk/src/trace/tracer_provider_factory.cc b/sdk/src/trace/tracer_provider_factory.cc index 8219d54866..49d2aac12d 100644 --- a/sdk/src/trace/tracer_provider_factory.cc +++ b/sdk/src/trace/tracer_provider_factory.cc @@ -5,7 +5,7 @@ #include #include -#include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" +#include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" #include "opentelemetry/sdk/resource/resource.h" #include "opentelemetry/sdk/trace/id_generator.h" #include "opentelemetry/sdk/trace/processor.h" diff --git a/sdk/test/trace/tracer_config_test.cc b/sdk/test/trace/tracer_config_test.cc index c5b9fb95c3..057784f395 100644 --- a/sdk/test/trace/tracer_config_test.cc +++ b/sdk/test/trace/tracer_config_test.cc @@ -4,12 +4,13 @@ #include "opentelemetry/sdk/trace/tracer_config.h" #include #include -#include #include #include +#include #include "opentelemetry/common/attribute_value.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/nostd/unique_ptr.h" +#include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" #include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" namespace trace_sdk = opentelemetry::sdk::trace; From 47dd040e1200b51c0be5c55b74ca146f000cbbf0 Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Fri, 6 Dec 2024 03:30:51 +0000 Subject: [PATCH 24/32] Add unit test for condition ordering --- sdk/test/trace/tracer_test.cc | 58 +++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index a4c2c0ffc4..4d2a872f20 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -539,8 +539,12 @@ TEST(Tracer, StartSpanWithEnabledConfig) TEST(Tracer, StartSpanWithCustomConfig) { #ifdef OPENTELEMETRY_RTTI_ENABLED + auto check_if_version_present = [](const InstrumentationScope &scope_info) { + return !scope_info.GetVersion().empty(); + }; ScopeConfigurator custom_configurator = ScopeConfigurator::Builder(TracerConfig::Enabled()) + .AddCondition(check_if_version_present, TracerConfig::Enabled()) .AddConditionNameEquals("foo_library", TracerConfig::Disabled()) .AddConditionNameEquals("", TracerConfig::Disabled()) .Build(); @@ -560,6 +564,14 @@ TEST(Tracer, StartSpanWithCustomConfig) auto &span_foo_scope_ref = *span_foo_scope.get(); EXPECT_EQ(typeid(span_foo_scope_ref), typeid(trace_api::NoopSpan)); + auto foo_scope_with_version = InstrumentationScope::Create("foo_library", "1.0.0"); + const auto tracer_foo_scope_with_version = + initTracer(std::unique_ptr{new InMemorySpanExporter()}, new AlwaysOnSampler(), + new RandomIdGenerator(), custom_configurator, std::move(foo_scope_with_version)); + const auto span_foo_scope_with_version = tracer_foo_scope_with_version->StartSpan("span 1"); + auto &span_foo_scope_with_version_ref = *span_foo_scope_with_version.get(); + EXPECT_EQ(typeid(span_foo_scope_with_version_ref), typeid(opentelemetry::sdk::trace::Span)); + auto bar_scope = InstrumentationScope::Create("bar_library"); auto tracer_bar_scope = initTracer(std::unique_ptr{new InMemorySpanExporter()}, new AlwaysOnSampler(), @@ -572,6 +584,52 @@ TEST(Tracer, StartSpanWithCustomConfig) #endif } +TEST(Tracer, StartSpanWithCustomConfigDifferingConditionOrder) +{ +#ifdef OPENTELEMETRY_RTTI_ENABLED + auto check_if_version_present = [](const InstrumentationScope &scope_info) { + return !scope_info.GetVersion().empty(); + }; + + // 2 configurators with same conditions, but different order + ScopeConfigurator custom_configurator_1 = + ScopeConfigurator::Builder(TracerConfig::Enabled()) + .AddCondition(check_if_version_present, TracerConfig::Enabled()) + .AddConditionNameEquals("foo_library", TracerConfig::Disabled()) + .Build(); + ScopeConfigurator custom_configurator_2 = + ScopeConfigurator::Builder(TracerConfig::Enabled()) + .AddConditionNameEquals("foo_library", TracerConfig::Disabled()) + .AddCondition(check_if_version_present, TracerConfig::Enabled()) + .Build(); + + auto foo_scope_with_version_1 = InstrumentationScope::Create("foo_library", "1.0.0"); + auto foo_scope_with_version_2 = InstrumentationScope::Create("foo_library", "1.0.0"); + + const auto tracer_foo_scope_with_version_1 = initTracer( + std::unique_ptr{new InMemorySpanExporter()}, new AlwaysOnSampler(), + new RandomIdGenerator(), custom_configurator_1, std::move(foo_scope_with_version_1)); + + const auto tracer_foo_scope_with_version_2 = initTracer( + std::unique_ptr{new InMemorySpanExporter()}, new AlwaysOnSampler(), + new RandomIdGenerator(), custom_configurator_2, std::move(foo_scope_with_version_2)); + + // Custom configurator 1 evaluates version first and enables the tracer + const auto span_foo_scope_with_version_1 = tracer_foo_scope_with_version_1->StartSpan("span 1"); + auto &span_foo_scope_with_version_ref_1 = *span_foo_scope_with_version_1.get(); + EXPECT_EQ(typeid(span_foo_scope_with_version_ref_1), typeid(opentelemetry::sdk::trace::Span)); + + // Custom configurator 2 evaluates the name first and therefore disables the tracer without + // evaluating other condition + const auto span_foo_scope_with_version_2 = tracer_foo_scope_with_version_2->StartSpan("span 1"); + auto &span_foo_scope_with_version_ref_2 = *span_foo_scope_with_version_2.get(); + EXPECT_EQ(typeid(span_foo_scope_with_version_ref_2), typeid(trace_api::NoopSpan)); + +#else + GTEST_SKIP() << "cannot use 'typeid' with '-fno-rtti'"; +#endif +} + TEST(Tracer, SpanSetLinks) { InMemorySpanExporter *exporter = new InMemorySpanExporter(); From 98ca2c346c300019224b3e63dcce3fb502065c07 Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Fri, 6 Dec 2024 03:35:59 +0000 Subject: [PATCH 25/32] Remove accidental changes from instrumentation_scope.h --- .../sdk/instrumentationscope/instrumentation_scope.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/include/opentelemetry/sdk/instrumentationscope/instrumentation_scope.h b/sdk/include/opentelemetry/sdk/instrumentationscope/instrumentation_scope.h index 6b565dbc5a..1bf0facaa6 100644 --- a/sdk/include/opentelemetry/sdk/instrumentationscope/instrumentation_scope.h +++ b/sdk/include/opentelemetry/sdk/instrumentationscope/instrumentation_scope.h @@ -3,7 +3,6 @@ #pragma once -#include #include #include @@ -158,6 +157,7 @@ class InstrumentationScope InstrumentationScopeAttributes attributes_; }; + } // namespace instrumentationscope } // namespace sdk From 5efb9ddec6bde199e219e25bbe66d0a291f3d639 Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Thu, 23 Jan 2025 19:31:23 +0000 Subject: [PATCH 26/32] Fix the documentation comment --- sdk/include/opentelemetry/sdk/trace/tracer_provider_factory.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_provider_factory.h b/sdk/include/opentelemetry/sdk/trace/tracer_provider_factory.h index 92140c188c..071a0551ca 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_provider_factory.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_provider_factory.h @@ -55,7 +55,7 @@ class OPENTELEMETRY_EXPORT TracerProviderFactory std::unique_ptr id_generator, std::unique_ptr> tracer_configurator); - /* Series of creator methods with a single processor. */ + /* Series of creator methods with a vector of processors. */ static std::unique_ptr Create( std::vector> &&processors); From 8511e80cd2d367bf96d295e0410459680e180bfc Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Thu, 23 Jan 2025 20:09:31 +0000 Subject: [PATCH 27/32] Address test comment & fix IWYU warnings --- sdk/test/trace/tracer_config_test.cc | 31 +++++++++++++++++----------- sdk/test/trace/tracer_test.cc | 1 + 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/sdk/test/trace/tracer_config_test.cc b/sdk/test/trace/tracer_config_test.cc index 057784f395..b16a3f2dc7 100644 --- a/sdk/test/trace/tracer_config_test.cc +++ b/sdk/test/trace/tracer_config_test.cc @@ -4,7 +4,7 @@ #include "opentelemetry/sdk/trace/tracer_config.h" #include #include -#include +#include #include #include #include "opentelemetry/common/attribute_value.h" @@ -38,33 +38,40 @@ TEST(TracerConfig, CheckDefaultConfigWorksAccToSpec) /** Tests to verify the behavior of trace_sdk::TracerConfig::DefaultConfigurator */ -std::pair attr1 = { +static std::pair attr1 = { "accept_single_attr", true}; -std::pair attr2 = { +static std::pair attr2 = { "accept_second_attr", "some other attr"}; -std::pair attr3 = { +static std::pair attr3 = { "accept_third_attr", 3}; -const std::array instrumentation_scopes = { - std::move(instrumentation_scope::InstrumentationScope::Create("test_scope_1")).get(), - std::move(instrumentation_scope::InstrumentationScope::Create("test_scope_2", "1.0")).get(), +static instrumentation_scope::InstrumentationScope *test_scope_1 = + std::move(instrumentation_scope::InstrumentationScope::Create("test_scope_1")).get(); +static instrumentation_scope::InstrumentationScope *test_scope_2 = + std::move(instrumentation_scope::InstrumentationScope::Create("test_scope_2", "1.0")).get(); +static instrumentation_scope::InstrumentationScope *test_scope_3 = std::move(instrumentation_scope::InstrumentationScope::Create( "test_scope_3", "0", "https://opentelemetry.io/schemas/v1.18.0")) - .get(), + .get(); +static instrumentation_scope::InstrumentationScope *test_scope_4 = std::move(instrumentation_scope::InstrumentationScope::Create( - "test_scope_3", + "test_scope_4", "0", "https://opentelemetry.io/schemas/v1.18.0", {attr1})) - .get(), + .get(); +static instrumentation_scope::InstrumentationScope *test_scope_5 = std::move(instrumentation_scope::InstrumentationScope::Create( - "test_scope_4", + "test_scope_5", "0", "https://opentelemetry.io/schemas/v1.18.0", {attr1, attr2, attr3})) - .get(), + .get(); + +const std::array instrumentation_scopes = { + test_scope_1, test_scope_2, test_scope_3, test_scope_4, test_scope_5, }; // Test fixture for VerifyDefaultConfiguratorBehavior diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index 985fd4f4ec..531eeba1ea 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include From bcbcbe2579b07a02920e2e0a7e60b5fb21b985a3 Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Thu, 23 Jan 2025 21:30:39 +0000 Subject: [PATCH 28/32] Remove the use of RTTI_ENABLED checks --- sdk/test/trace/tracer_test.cc | 75 ++++++++++++++--------------------- 1 file changed, 30 insertions(+), 45 deletions(-) diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index 531eeba1ea..adcfee6ca5 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -498,44 +498,41 @@ TEST(Tracer, SpanSetEvents) TEST(Tracer, StartSpanWithDisabledConfig) { -#ifdef OPENTELEMETRY_RTTI_ENABLED InMemorySpanExporter *exporter = new InMemorySpanExporter(); std::shared_ptr span_data = exporter->GetData(); ScopeConfigurator disable_tracer = ScopeConfigurator::Builder(TracerConfig::Disabled()).Build(); - auto tracer = initTracer(std::unique_ptr{exporter}, new AlwaysOnSampler(), - new RandomIdGenerator(), disable_tracer); - auto span = tracer->StartSpan("span 1"); - auto &span_ref = *span.get(); - - EXPECT_NE(typeid(span_ref), typeid(trace_api::Span)); - EXPECT_EQ(typeid(span_ref), typeid(trace_api::NoopSpan)); -#else - GTEST_SKIP() << "cannot use 'typeid' with '-fno-rtti'"; -#endif + auto tracer = initTracer(std::unique_ptr{exporter}, new AlwaysOnSampler(), + new RandomIdGenerator(), disable_tracer); + auto span = tracer->StartSpan("span 1"); + + std::shared_ptr noop_tracer = + std::make_shared(); + auto noop_span = noop_tracer->StartSpan("noop"); + EXPECT_TRUE(span.get() == noop_span.get()); } TEST(Tracer, StartSpanWithEnabledConfig) { -#ifdef OPENTELEMETRY_RTTI_ENABLED InMemorySpanExporter *exporter = new InMemorySpanExporter(); std::shared_ptr span_data = exporter->GetData(); ScopeConfigurator enable_tracer = ScopeConfigurator::Builder(TracerConfig::Enabled()).Build(); - auto tracer = initTracer(std::unique_ptr{exporter}, new AlwaysOnSampler(), - new RandomIdGenerator(), enable_tracer); - auto span = tracer->StartSpan("span 1"); - auto &span_ref = *span.get(); - - EXPECT_EQ(typeid(span_ref), typeid(Span)); -#else - GTEST_SKIP() << "cannot use 'typeid' with '-fno-rtti'"; -#endif + auto tracer = initTracer(std::unique_ptr{exporter}, new AlwaysOnSampler(), + new RandomIdGenerator(), enable_tracer); + auto span = tracer->StartSpan("span 1"); + + std::shared_ptr noop_tracer = + std::make_shared(); + auto noop_span = noop_tracer->StartSpan("noop"); + EXPECT_FALSE(span.get() == noop_span.get()); } TEST(Tracer, StartSpanWithCustomConfig) { -#ifdef OPENTELEMETRY_RTTI_ENABLED + std::shared_ptr noop_tracer = + std::make_shared(); + auto noop_span = noop_tracer->StartSpan("noop"); auto check_if_version_present = [](const InstrumentationScope &scope_info) { return !scope_info.GetVersion().empty(); }; @@ -549,45 +546,39 @@ TEST(Tracer, StartSpanWithCustomConfig) const auto tracer_default_scope = initTracer(std::unique_ptr{new InMemorySpanExporter()}, new AlwaysOnSampler(), new RandomIdGenerator(), custom_configurator); - const auto span_default_scope = tracer_default_scope->StartSpan("span 1"); - const auto &span_default_scope_ref = *span_default_scope.get(); - EXPECT_EQ(typeid(span_default_scope_ref), typeid(trace_api::NoopSpan)); + const auto span_default_scope = tracer_default_scope->StartSpan("span 1"); + EXPECT_TRUE(span_default_scope == noop_span); auto foo_scope = InstrumentationScope::Create("foo_library"); const auto tracer_foo_scope = initTracer(std::unique_ptr{new InMemorySpanExporter()}, new AlwaysOnSampler(), new RandomIdGenerator(), custom_configurator, std::move(foo_scope)); const auto span_foo_scope = tracer_foo_scope->StartSpan("span 1"); - auto &span_foo_scope_ref = *span_foo_scope.get(); - EXPECT_EQ(typeid(span_foo_scope_ref), typeid(trace_api::NoopSpan)); + EXPECT_TRUE(span_foo_scope == noop_span); auto foo_scope_with_version = InstrumentationScope::Create("foo_library", "1.0.0"); const auto tracer_foo_scope_with_version = initTracer(std::unique_ptr{new InMemorySpanExporter()}, new AlwaysOnSampler(), new RandomIdGenerator(), custom_configurator, std::move(foo_scope_with_version)); const auto span_foo_scope_with_version = tracer_foo_scope_with_version->StartSpan("span 1"); - auto &span_foo_scope_with_version_ref = *span_foo_scope_with_version.get(); - EXPECT_EQ(typeid(span_foo_scope_with_version_ref), typeid(opentelemetry::sdk::trace::Span)); + EXPECT_FALSE(span_foo_scope_with_version == noop_span); auto bar_scope = InstrumentationScope::Create("bar_library"); auto tracer_bar_scope = initTracer(std::unique_ptr{new InMemorySpanExporter()}, new AlwaysOnSampler(), new RandomIdGenerator(), custom_configurator, std::move(bar_scope)); - auto span_bar_scope = tracer_bar_scope->StartSpan("span 1"); - auto &span_bar_scope_ref = *span_bar_scope.get(); - EXPECT_EQ(typeid(span_bar_scope_ref), typeid(opentelemetry::sdk::trace::Span)); -#else - GTEST_SKIP() << "cannot use 'typeid' with '-fno-rtti'"; -#endif + auto span_bar_scope = tracer_bar_scope->StartSpan("span 1"); + EXPECT_FALSE(span_bar_scope == noop_span); } TEST(Tracer, StartSpanWithCustomConfigDifferingConditionOrder) { -#ifdef OPENTELEMETRY_RTTI_ENABLED + std::shared_ptr noop_tracer = + std::make_shared(); + auto noop_span = noop_tracer->StartSpan("noop"); auto check_if_version_present = [](const InstrumentationScope &scope_info) { return !scope_info.GetVersion().empty(); }; - // 2 configurators with same conditions, but different order ScopeConfigurator custom_configurator_1 = ScopeConfigurator::Builder(TracerConfig::Enabled()) @@ -613,18 +604,12 @@ TEST(Tracer, StartSpanWithCustomConfigDifferingConditionOrder) // Custom configurator 1 evaluates version first and enables the tracer const auto span_foo_scope_with_version_1 = tracer_foo_scope_with_version_1->StartSpan("span 1"); - auto &span_foo_scope_with_version_ref_1 = *span_foo_scope_with_version_1.get(); - EXPECT_EQ(typeid(span_foo_scope_with_version_ref_1), typeid(opentelemetry::sdk::trace::Span)); + EXPECT_FALSE(span_foo_scope_with_version_1 == noop_span); // Custom configurator 2 evaluates the name first and therefore disables the tracer without // evaluating other condition const auto span_foo_scope_with_version_2 = tracer_foo_scope_with_version_2->StartSpan("span 1"); - auto &span_foo_scope_with_version_ref_2 = *span_foo_scope_with_version_2.get(); - EXPECT_EQ(typeid(span_foo_scope_with_version_ref_2), typeid(trace_api::NoopSpan)); - -#else - GTEST_SKIP() << "cannot use 'typeid' with '-fno-rtti'"; -#endif + EXPECT_TRUE(span_foo_scope_with_version_2 == noop_span); } TEST(Tracer, SpanSetLinks) From 6bc257ef2399cbcb560e5be06570b896109b2b93 Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Thu, 23 Jan 2025 22:30:42 +0000 Subject: [PATCH 29/32] Fix IWYU warnings --- sdk/test/trace/tracer_test.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index adcfee6ca5..1f31e2336d 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -55,8 +55,6 @@ #include "opentelemetry/trace/trace_state.h" #include "opentelemetry/trace/tracer.h" -#include - using namespace opentelemetry::sdk::trace; using namespace opentelemetry::sdk::resource; using namespace opentelemetry::sdk::instrumentationscope; From 98ec24d6a93df6eef612a3ae5184a92084c38b71 Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Thu, 23 Jan 2025 23:16:18 +0000 Subject: [PATCH 30/32] Fix warnings in test fix dangling pointer warnings & valgrind memcheck warnings --- sdk/test/trace/tracer_config_test.cc | 49 ++++++++++++++-------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/sdk/test/trace/tracer_config_test.cc b/sdk/test/trace/tracer_config_test.cc index b16a3f2dc7..27f845d548 100644 --- a/sdk/test/trace/tracer_config_test.cc +++ b/sdk/test/trace/tracer_config_test.cc @@ -45,33 +45,32 @@ static std::pair attr3 = { "accept_third_attr", 3}; -static instrumentation_scope::InstrumentationScope *test_scope_1 = - std::move(instrumentation_scope::InstrumentationScope::Create("test_scope_1")).get(); -static instrumentation_scope::InstrumentationScope *test_scope_2 = - std::move(instrumentation_scope::InstrumentationScope::Create("test_scope_2", "1.0")).get(); -static instrumentation_scope::InstrumentationScope *test_scope_3 = - std::move(instrumentation_scope::InstrumentationScope::Create( - "test_scope_3", - "0", - "https://opentelemetry.io/schemas/v1.18.0")) - .get(); -static instrumentation_scope::InstrumentationScope *test_scope_4 = - std::move(instrumentation_scope::InstrumentationScope::Create( - "test_scope_4", - "0", - "https://opentelemetry.io/schemas/v1.18.0", - {attr1})) - .get(); -static instrumentation_scope::InstrumentationScope *test_scope_5 = - std::move(instrumentation_scope::InstrumentationScope::Create( - "test_scope_5", - "0", - "https://opentelemetry.io/schemas/v1.18.0", - {attr1, attr2, attr3})) - .get(); +static instrumentation_scope::InstrumentationScope test_scope_1 = + *instrumentation_scope::InstrumentationScope::Create("test_scope_1"); +static instrumentation_scope::InstrumentationScope test_scope_2 = + *instrumentation_scope::InstrumentationScope::Create("test_scope_2", "1.0"); +static instrumentation_scope::InstrumentationScope test_scope_3 = + *instrumentation_scope::InstrumentationScope::Create( + "test_scope_3", + "0", + "https://opentelemetry.io/schemas/v1.18.0"); +static instrumentation_scope::InstrumentationScope test_scope_4 = + *instrumentation_scope::InstrumentationScope::Create("test_scope_4", + "0", + "https://opentelemetry.io/schemas/v1.18.0", + {attr1}); +static instrumentation_scope::InstrumentationScope test_scope_5 = + *instrumentation_scope::InstrumentationScope::Create("test_scope_5", + "0", + "https://opentelemetry.io/schemas/v1.18.0", + {attr1, attr2, attr3}); +// This array could also directly contain the reference types, but that leads to 'uninitialized +// value was created by heap allocation' errors in Valgrind memcheck. This is a bug in Googletest +// library, see https://github.com/google/googletest/issues/3805#issuecomment-1397301790 for more +// details. Using pointers is a workaround to prevent the Valgrind warnings. const std::array instrumentation_scopes = { - test_scope_1, test_scope_2, test_scope_3, test_scope_4, test_scope_5, + &test_scope_1, &test_scope_2, &test_scope_3, &test_scope_4, &test_scope_5, }; // Test fixture for VerifyDefaultConfiguratorBehavior From ad9e1e1bc84a457e79227e5a833d49e0e6d2a94d Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Mon, 27 Jan 2025 17:33:01 +0000 Subject: [PATCH 31/32] Address comments Update string_view -> string for use within the function scope to ensure scope lifetime. Update push_back -> emplace_back to construct directly within the vector container. Added a constructor to the Condition struct to enable emplace_back. --- .../sdk/instrumentationscope/scope_configurator.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h b/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h index 3b8f611f3c..c714d69641 100644 --- a/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h +++ b/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h @@ -48,7 +48,7 @@ class ScopeConfigurator Builder AddCondition(std::function scope_matcher, T scope_config) { - conditions_.push_back(Condition{scope_matcher, scope_config}); + conditions_.emplace_back(scope_matcher, scope_config); return *this; } @@ -63,10 +63,10 @@ class ScopeConfigurator Builder AddConditionNameEquals(nostd::string_view scope_name, T scope_config) { std::function name_equals_matcher = - [scope_name](const InstrumentationScope &scope_info) { + [scope_name = std::string(scope_name)](const InstrumentationScope &scope_info) { return scope_info.GetName() == scope_name; }; - conditions_.push_back(Condition{name_equals_matcher, scope_config}); + conditions_.emplace_back(name_equals_matcher, scope_config); return *this; } @@ -103,13 +103,17 @@ class ScopeConfigurator private: /** * An internal struct to encapsulate 'conditions' that can be applied to a - * ScopeConfiguratorBuilder. The applied conditions influence the behavior of the generatred + * ScopeConfiguratorBuilder. The applied conditions influence the behavior of the generated * ScopeConfigurator. */ struct Condition { std::function scope_matcher; T scope_config; + + Condition(const std::function &matcher, const T &config) + : scope_matcher(matcher), scope_config(config) + {} }; T default_scope_config_; From 784b0d444f7a4deb616d4011493164f8e652dd6f Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Mon, 27 Jan 2025 19:11:22 +0000 Subject: [PATCH 32/32] Make builder return reference type Making builder's methods return reference type prevents unneeded copies of the object. --- .../sdk/instrumentationscope/scope_configurator.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h b/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h index c714d69641..f2d046f53a 100644 --- a/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h +++ b/sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h @@ -45,8 +45,8 @@ class ScopeConfigurator * @param scope_config the scope configuration to return for the matched scope. * @return this */ - Builder AddCondition(std::function scope_matcher, - T scope_config) + Builder &AddCondition(std::function scope_matcher, + T scope_config) { conditions_.emplace_back(scope_matcher, scope_config); return *this; @@ -60,7 +60,7 @@ class ScopeConfigurator * @param scope_config The scope config for the matching scopes. * @return this */ - Builder AddConditionNameEquals(nostd::string_view scope_name, T scope_config) + Builder &AddConditionNameEquals(nostd::string_view scope_name, T scope_config) { std::function name_equals_matcher = [scope_name = std::string(scope_name)](const InstrumentationScope &scope_info) { @@ -75,7 +75,7 @@ class ScopeConfigurator * on the instrumentation scope. * @return a configured scope configurator. */ - ScopeConfigurator Build() + ScopeConfigurator Build() const { if (conditions_.size() == 0) {