From a40144decc685583a6905e9a2f69146ee64df4bf Mon Sep 17 00:00:00 2001 From: Rajkaran Date: Fri, 23 Oct 2026 14:18:38 +0530 Subject: [PATCH 1/3] feat(sdk): support trace sampler environment variables Implement OTEL_TRACES_SAMPLER and OTEL_TRACES_SAMPLER_ARG environment variables to configure the trace sampler. This allows users to configure sampling behavior without code changes. Supported sampler values: - always_on - always_off - traceidratio - parentbased_traceidratio - parentbased_always_on - parentbased_always_off For ratio-based samplers (traceidratio, parentbased_traceidratio), the ratio is specified via OTEL_TRACES_SAMPLER_ARG (default 1.0 if unset). Implementation details: - Added sampler_env.h/cc with GetSamplerFromEnv() function - Updated TracerProvider and TracerContext constructors to use environment-based sampler when no explicit sampler is provided - Updated TracerProviderFactory and TracerContextFactory to use environment-based sampler - Invalid sampler names or ratios fall back to AlwaysOnSampler with a warning - Explicitly supplied samplers always take precedence over environment variables - Added comprehensive unit tests covering all supported values, edge cases, and error conditions Closes #4709 --- CHANGELOG.md | 10 + .../opentelemetry/sdk/trace/sampler_env.h | 43 +++ .../opentelemetry/sdk/trace/tracer_context.h | 3 +- .../opentelemetry/sdk/trace/tracer_provider.h | 10 +- sdk/src/trace/CMakeLists.txt | 1 + sdk/src/trace/sampler_env.cc | 107 ++++++++ sdk/src/trace/tracer_context.cc | 8 +- sdk/src/trace/tracer_context_factory.cc | 3 +- sdk/src/trace/tracer_provider.cc | 16 +- sdk/src/trace/tracer_provider_factory.cc | 5 +- sdk/test/trace/BUILD | 15 ++ sdk/test/trace/CMakeLists.txt | 1 + sdk/test/trace/sampler_env_test.cc | 245 ++++++++++++++++++ 13 files changed, 455 insertions(+), 12 deletions(-) create mode 100644 sdk/include/opentelemetry/sdk/trace/sampler_env.h create mode 100644 sdk/src/trace/sampler_env.cc create mode 100644 sdk/test/trace/sampler_env_test.cc diff --git a/CHANGELOG.md b/CHANGELOG.md index 805c832e50..d013a4a999 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,16 @@ Increment the: ## [Unreleased] +* [SDK] Support `OTEL_TRACES_SAMPLER` and `OTEL_TRACES_SAMPLER_ARG` environment variables + [#4709](https://github.com/open-telemetry/opentelemetry-cpp/issues/4709) + Users can now configure the sampler via environment variables, supporting: + - `always_on`, `always_off`, `traceidratio`, `parentbased_traceidratio`, + `parentbased_always_on`, `parentbased_always_off` + - For ratio-based samplers, `OTEL_TRACES_SAMPLER_ARG` specifies the ratio (default 1.0) + - Environment configuration applies to TracerProvider, TracerContext, and factory methods + - Explicitly supplied samplers always take precedence over environment variables + - Invalid sampler names or ratios fall back to the default AlwaysOnSampler with a warning + * [CONFIGURATION] Add a configuration builder for the host resource detector [#4451](https://github.com/open-telemetry/opentelemetry-cpp/issues/4451) * [CONFIGURATION] Build the configured resource detectors in SdkBuilder, apply diff --git a/sdk/include/opentelemetry/sdk/trace/sampler_env.h b/sdk/include/opentelemetry/sdk/trace/sampler_env.h new file mode 100644 index 0000000000..c8598599f1 --- /dev/null +++ b/sdk/include/opentelemetry/sdk/trace/sampler_env.h @@ -0,0 +1,43 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +#include + +#include "opentelemetry/sdk/trace/sampler.h" +#include "opentelemetry/version.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace trace +{ + +namespace sampler_env +{ + +/** + * @brief Returns a sampler built from OTEL_TRACES_SAMPLER and OTEL_TRACES_SAMPLER_ARG + * environment variables, or a default AlwaysOnSampler if the variables are not set. + * + * Supported sampler values: + * - always_on: Always sample the trace + * - always_off: Never sample the trace + * - traceidratio: Sample a fraction of traces based on trace ID, requires OTEL_TRACES_SAMPLER_ARG + * - parentbased_traceidratio: Sample based on parent and trace ID, requires OTEL_TRACES_SAMPLER_ARG + * - parentbased_always_on: Use parent sample decision, default to AlwaysOn + * - parentbased_always_off: Use parent sample decision, default to AlwaysOff + * + * For traceidratio and parentbased_traceidratio, OTEL_TRACES_SAMPLER_ARG specifies the ratio + * (0.0 to 1.0). If unset, defaults to 1.0. + * + * If an unknown sampler name or invalid ratio is provided, logs a warning and returns AlwaysOnSampler. + */ +OPENTELEMETRY_EXPORT std::unique_ptr GetSamplerFromEnv(); + +} // namespace sampler_env + +} // namespace trace +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE \ No newline at end of file diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_context.h b/sdk/include/opentelemetry/sdk/trace/tracer_context.h index f94972b8fe..2953fbc165 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_context.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_context.h @@ -13,6 +13,7 @@ #include "opentelemetry/sdk/trace/processor.h" #include "opentelemetry/sdk/trace/random_id_generator.h" #include "opentelemetry/sdk/trace/sampler.h" +#include "opentelemetry/sdk/trace/sampler_env.h" #include "opentelemetry/sdk/trace/samplers/always_on.h" #include "opentelemetry/sdk/trace/span_limits.h" #include "opentelemetry/sdk/trace/tracer_config.h" @@ -44,7 +45,7 @@ class TracerContext std::vector> &&processor, const opentelemetry::sdk::resource::Resource &resource = opentelemetry::sdk::resource::Resource::Create({}), - std::unique_ptr sampler = std::unique_ptr(new AlwaysOnSampler), + std::unique_ptr sampler = nullptr, std::unique_ptr id_generator = std::unique_ptr(new RandomIdGenerator()), std::unique_ptr> tracer_configurator = diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h index 5b9986a4c3..0d75e4832e 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h @@ -14,6 +14,7 @@ #include "opentelemetry/sdk/trace/processor.h" #include "opentelemetry/sdk/trace/random_id_generator.h" #include "opentelemetry/sdk/trace/sampler.h" +#include "opentelemetry/sdk/trace/sampler_env.h" #include "opentelemetry/sdk/trace/samplers/always_on.h" #include "opentelemetry/sdk/trace/span_limits.h" #include "opentelemetry/sdk/trace/tracer.h" @@ -36,8 +37,9 @@ class OPENTELEMETRY_EXPORT TracerProvider final : public opentelemetry::trace::T * @param processor The span processor for this tracer provider. This must * not be a nullptr. * @param resource The resources for this tracer provider. - * @param sampler The sampler for this tracer provider. This must - * not be a nullptr. + * @param sampler The sampler for this tracer provider. If null, a sampler is built from + * OTEL_TRACES_SAMPLER and OTEL_TRACES_SAMPLER_ARG environment variables, defaulting to + * AlwaysOnSampler if unset. * @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 @@ -48,7 +50,7 @@ class OPENTELEMETRY_EXPORT TracerProvider final : public opentelemetry::trace::T std::unique_ptr processor, const opentelemetry::sdk::resource::Resource &resource = opentelemetry::sdk::resource::Resource::Create({}), - std::unique_ptr sampler = std::unique_ptr(new AlwaysOnSampler), + std::unique_ptr sampler = nullptr, std::unique_ptr id_generator = std::unique_ptr(new RandomIdGenerator()), std::unique_ptr> tracer_configurator = @@ -62,7 +64,7 @@ class OPENTELEMETRY_EXPORT TracerProvider final : public opentelemetry::trace::T 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 sampler = nullptr, std::unique_ptr id_generator = std::unique_ptr(new RandomIdGenerator()), std::unique_ptr> tracer_configurator = diff --git a/sdk/src/trace/CMakeLists.txt b/sdk/src/trace/CMakeLists.txt index 9aea6f3be3..7aeafef1bf 100644 --- a/sdk/src/trace/CMakeLists.txt +++ b/sdk/src/trace/CMakeLists.txt @@ -17,6 +17,7 @@ add_library( simple_processor_factory.cc span_data.cc span_limits.cc + sampler_env.cc samplers/always_on_factory.cc samplers/always_off_factory.cc samplers/parent.cc diff --git a/sdk/src/trace/sampler_env.cc b/sdk/src/trace/sampler_env.cc new file mode 100644 index 0000000000..bea0c3bb20 --- /dev/null +++ b/sdk/src/trace/sampler_env.cc @@ -0,0 +1,107 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#include +#include + +#include "opentelemetry/sdk/common/env_variables.h" +#include "opentelemetry/sdk/common/global_log_handler.h" +#include "opentelemetry/sdk/trace/sampler.h" +#include "opentelemetry/sdk/trace/sampler_env.h" +#include "opentelemetry/sdk/trace/samplers/always_off_factory.h" +#include "opentelemetry/sdk/trace/samplers/always_on_factory.h" +#include "opentelemetry/sdk/trace/samplers/parent_factory.h" +#include "opentelemetry/sdk/trace/samplers/trace_id_ratio_factory.h" +#include "opentelemetry/version.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace trace +{ + +namespace sampler_env +{ + +static constexpr const char *kTracesSamplerEnv = "OTEL_TRACES_SAMPLER"; +static constexpr const char *kTracesSamplerArgEnv = "OTEL_TRACES_SAMPLER_ARG"; +static constexpr double kDefaultTraceIdRatio = 1.0; + +std::unique_ptr GetSamplerFromEnv() +{ + std::string sampler_name; + bool has_sampler_env = opentelemetry::sdk::common::GetStringEnvironmentVariable(kTracesSamplerEnv, sampler_name); + + if (!has_sampler_env || sampler_name.empty()) + { + // When unset, preserve current default behavior (AlwaysOnSampler) + return AlwaysOnSamplerFactory::Create(); + } + + float ratio_arg = kDefaultTraceIdRatio; + bool has_ratio_env = opentelemetry::sdk::common::GetFloatEnvironmentVariable(kTracesSamplerArgEnv, ratio_arg); + + // For traceidratio and parentbased_traceidratio, default to 1.0 if arg is unset + if (!has_ratio_env) + { + ratio_arg = static_cast(kDefaultTraceIdRatio); + } + + // Build sampler based on environment variable value + if (sampler_name == "always_on") + { + return AlwaysOnSamplerFactory::Create(); + } + else if (sampler_name == "always_off") + { + return AlwaysOffSamplerFactory::Create(); + } + else if (sampler_name == "traceidratio") + { + // Validate ratio + if (std::isnan(ratio_arg) || std::isinf(ratio_arg) || ratio_arg < 0.0f || ratio_arg > 1.0f) + { + OTEL_INTERNAL_LOG_WARN("Environment variable <" << kTracesSamplerArgEnv << "> has invalid value <" + << ratio_arg << "> for traceidratio sampler, " + << "falling back to AlwaysOnSampler"); + return AlwaysOnSamplerFactory::Create(); + } + return TraceIdRatioBasedSamplerFactory::Create(ratio_arg); + } + else if (sampler_name == "parentbased_traceidratio") + { + // Validate ratio + if (std::isnan(ratio_arg) || std::isinf(ratio_arg) || ratio_arg < 0.0f || ratio_arg > 1.0f) + { + OTEL_INTERNAL_LOG_WARN("Environment variable <" << kTracesSamplerArgEnv << "> has invalid value <" + << ratio_arg << "> for parentbased_traceidratio sampler, " + << "falling back to AlwaysOnSampler"); + return AlwaysOnSamplerFactory::Create(); + } + auto root_sampler = TraceIdRatioBasedSamplerFactory::Create(ratio_arg); + return ParentBasedSamplerFactory::Create(root_sampler); + } + else if (sampler_name == "parentbased_always_on") + { + auto root_sampler = AlwaysOnSamplerFactory::Create(); + return ParentBasedSamplerFactory::Create(root_sampler); + } + else if (sampler_name == "parentbased_always_off") + { + auto root_sampler = AlwaysOffSamplerFactory::Create(); + return ParentBasedSamplerFactory::Create(root_sampler); + } + else + { + // Unknown sampler name + OTEL_INTERNAL_LOG_WARN("Environment variable <" << kTracesSamplerEnv << "> has unknown value <" + << sampler_name << ">, falling back to AlwaysOnSampler"); + return AlwaysOnSamplerFactory::Create(); + } +} + +} // namespace sampler_env + +} // namespace trace +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE \ No newline at end of file diff --git a/sdk/src/trace/tracer_context.cc b/sdk/src/trace/tracer_context.cc index 43bdabc93b..338020f35b 100644 --- a/sdk/src/trace/tracer_context.cc +++ b/sdk/src/trace/tracer_context.cc @@ -13,6 +13,7 @@ #include "opentelemetry/sdk/trace/multi_span_processor.h" #include "opentelemetry/sdk/trace/processor.h" #include "opentelemetry/sdk/trace/sampler.h" +#include "opentelemetry/sdk/trace/sampler_env.h" #include "opentelemetry/sdk/trace/span_limits.h" #include "opentelemetry/sdk/trace/tracer_config.h" #include "opentelemetry/sdk/trace/tracer_context.h" @@ -38,7 +39,12 @@ TracerContext::TracerContext( processor_(std::unique_ptr(new MultiSpanProcessor(std::move(processors)))), tracer_configurator_(std::move(tracer_configurator)), span_limits_(span_limits) -{} +{ + if (!sampler_) + { + sampler_ = sampler_env::GetSamplerFromEnv(); + } +} Sampler &TracerContext::GetSampler() const noexcept { diff --git a/sdk/src/trace/tracer_context_factory.cc b/sdk/src/trace/tracer_context_factory.cc index 9472dc041f..acf26f5f93 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/processor.h" #include "opentelemetry/sdk/trace/random_id_generator_factory.h" #include "opentelemetry/sdk/trace/sampler.h" +#include "opentelemetry/sdk/trace/sampler_env.h" #include "opentelemetry/sdk/trace/samplers/always_on_factory.h" #include "opentelemetry/sdk/trace/tracer_config.h" #include "opentelemetry/sdk/trace/tracer_context.h" @@ -34,7 +35,7 @@ std::unique_ptr TracerContextFactory::Create( std::vector> &&processors, const opentelemetry::sdk::resource::Resource &resource) { - auto sampler = AlwaysOnSamplerFactory::Create(); + auto sampler = sampler_env::GetSamplerFromEnv(); return Create(std::move(processors), resource, std::move(sampler)); } diff --git a/sdk/src/trace/tracer_provider.cc b/sdk/src/trace/tracer_provider.cc index 47d3bb98b9..089cbe9908 100644 --- a/sdk/src/trace/tracer_provider.cc +++ b/sdk/src/trace/tracer_provider.cc @@ -16,6 +16,7 @@ #include "opentelemetry/sdk/trace/id_generator.h" #include "opentelemetry/sdk/trace/processor.h" #include "opentelemetry/sdk/trace/sampler.h" +#include "opentelemetry/sdk/trace/sampler_env.h" #include "opentelemetry/sdk/trace/span_limits.h" #include "opentelemetry/sdk/trace/tracer.h" #include "opentelemetry/sdk/trace/tracer_config.h" @@ -45,6 +46,10 @@ TracerProvider::TracerProvider( std::unique_ptr> tracer_configurator, SpanLimits span_limits) noexcept { + if (!sampler) + { + sampler = sampler_env::GetSamplerFromEnv(); + } std::vector> processors; processors.push_back(std::move(processor)); context_ = std::make_shared(std::move(processors), resource, std::move(sampler), @@ -59,13 +64,18 @@ TracerProvider::TracerProvider( std::unique_ptr id_generator, std::unique_ptr> tracer_configurator, SpanLimits span_limits) noexcept - : context_(std::make_shared(std::move(processors), +{ + if (!sampler) + { + sampler = sampler_env::GetSamplerFromEnv(); + } + context_ = std::make_shared(std::move(processors), resource, std::move(sampler), std::move(id_generator), std::move(tracer_configurator), - span_limits)) -{} + span_limits); +} TracerProvider::~TracerProvider() { diff --git a/sdk/src/trace/tracer_provider_factory.cc b/sdk/src/trace/tracer_provider_factory.cc index 6ae418430c..6fceb2b126 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/processor.h" #include "opentelemetry/sdk/trace/random_id_generator_factory.h" #include "opentelemetry/sdk/trace/sampler.h" +#include "opentelemetry/sdk/trace/sampler_env.h" #include "opentelemetry/sdk/trace/samplers/always_on_factory.h" #include "opentelemetry/sdk/trace/span_limits.h" #include "opentelemetry/sdk/trace/tracer_config.h" @@ -36,7 +37,7 @@ std::unique_ptr TracerProviderFactory std::unique_ptr processor, const opentelemetry::sdk::resource::Resource &resource) { - auto sampler = AlwaysOnSamplerFactory::Create(); + auto sampler = sampler_env::GetSamplerFromEnv(); return Create(std::move(processor), resource, std::move(sampler)); } @@ -100,7 +101,7 @@ std::unique_ptr TracerProviderFactory std::vector> &&processors, const opentelemetry::sdk::resource::Resource &resource) { - auto sampler = AlwaysOnSamplerFactory::Create(); + auto sampler = sampler_env::GetSamplerFromEnv(); return Create(std::move(processors), resource, std::move(sampler)); } diff --git a/sdk/test/trace/BUILD b/sdk/test/trace/BUILD index ecf46663fe..3f9cc4a9fb 100644 --- a/sdk/test/trace/BUILD +++ b/sdk/test/trace/BUILD @@ -205,6 +205,21 @@ cc_test( ], ) +cc_test( + name = "sampler_env_test", + srcs = [ + "sampler_env_test.cc", + ], + tags = [ + "test", + "trace", + ], + deps = [ + "//sdk/src/trace", + "@com_google_googletest//:gtest_main", + ], +) + cc_test( name = "tracer_config_test", srcs = [ diff --git a/sdk/test/trace/CMakeLists.txt b/sdk/test/trace/CMakeLists.txt index 0b84555faa..e8a4b3ebc1 100644 --- a/sdk/test/trace/CMakeLists.txt +++ b/sdk/test/trace/CMakeLists.txt @@ -14,6 +14,7 @@ foreach( probability_sampler_test trace_id_ratio_sampler_test composable_sampler_test + sampler_env_test batch_span_processor_test batch_span_processor_test_stress tracer_config_test) diff --git a/sdk/test/trace/sampler_env_test.cc b/sdk/test/trace/sampler_env_test.cc new file mode 100644 index 0000000000..f60669f7a8 --- /dev/null +++ b/sdk/test/trace/sampler_env_test.cc @@ -0,0 +1,245 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#include + +#include +#include + +#include "opentelemetry/sdk/trace/sampler.h" +#include "opentelemetry/sdk/trace/sampler_env.h" +#include "opentelemetry/sdk/trace/samplers/always_off_factory.h" +#include "opentelemetry/sdk/trace/samplers/always_on_factory.h" +#include "opentelemetry/sdk/trace/samplers/parent_factory.h" +#include "opentelemetry/sdk/trace/samplers/trace_id_ratio_factory.h" +#include "opentelemetry/trace/span_context.h" +#include "opentelemetry/trace/trace_flags.h" + +#if defined(_MSC_VER) +using opentelemetry::sdk::common::setenv; +using opentelemetry::sdk::common::unsetenv; +#endif + +using opentelemetry::sdk::trace::sampler_env::GetSamplerFromEnv; +using opentelemetry::sdk::trace::AlwaysOffSamplerFactory; +using opentelemetry::sdk::trace::AlwaysOnSamplerFactory; +using opentelemetry::sdk::trace::ParentBasedSamplerFactory; +using opentelemetry::sdk::trace::TraceIdRatioBasedSamplerFactory; + +#ifndef NO_GETENV + +class SamplerEnvTest : public ::testing::Test +{ +protected: + void SetUp() override + { + // Clean up any existing environment variables + unsetenv("OTEL_TRACES_SAMPLER"); + unsetenv("OTEL_TRACES_SAMPLER_ARG"); + } + + void TearDown() override + { + // Clean up environment variables after each test + unsetenv("OTEL_TRACES_SAMPLER"); + unsetenv("OTEL_TRACES_SAMPLER_ARG"); + } +}; + +TEST_F(SamplerEnvTest, UnsetVariablesReturnsAlwaysOnSampler) +{ + auto sampler = GetSamplerFromEnv(); + EXPECT_EQ(sampler->GetDescription(), AlwaysOnSamplerFactory::Create()->GetDescription()); +} + +TEST_F(SamplerEnvTest, AlwaysOnSampler) +{ + setenv("OTEL_TRACES_SAMPLER", "always_on", 1); + auto sampler = GetSamplerFromEnv(); + EXPECT_EQ(sampler->GetDescription(), AlwaysOnSamplerFactory::Create()->GetDescription()); +} + +TEST_F(SamplerEnvTest, AlwaysOffSampler) +{ + setenv("OTEL_TRACES_SAMPLER", "always_off", 1); + auto sampler = GetSamplerFromEnv(); + EXPECT_EQ(sampler->GetDescription(), AlwaysOffSamplerFactory::Create()->GetDescription()); +} + +TEST_F(SamplerEnvTest, TraceIdRatioSamplerWithValidRatio) +{ + setenv("OTEL_TRACES_SAMPLER", "traceidratio", 1); + setenv("OTEL_TRACES_SAMPLER_ARG", "0.5", 1); + auto sampler = GetSamplerFromEnv(); + EXPECT_EQ(sampler->GetDescription(), TraceIdRatioBasedSamplerFactory::Create(0.5)->GetDescription()); +} + +TEST_F(SamplerEnvTest, TraceIdRatioSamplerWithDefaultRatio) +{ + setenv("OTEL_TRACES_SAMPLER", "traceidratio", 1); + // OTEL_TRACES_SAMPLER_ARG not set, should default to 1.0 + auto sampler = GetSamplerFromEnv(); + EXPECT_EQ(sampler->GetDescription(), TraceIdRatioBasedSamplerFactory::Create(1.0)->GetDescription()); +} + +TEST_F(SamplerEnvTest, ParentBasedTraceIdRatioSamplerWithValidRatio) +{ + setenv("OTEL_TRACES_SAMPLER", "parentbased_traceidratio", 1); + setenv("OTEL_TRACES_SAMPLER_ARG", "0.5", 1); + auto sampler = GetSamplerFromEnv(); + auto root_sampler = TraceIdRatioBasedSamplerFactory::Create(0.5); + auto expected_sampler = ParentBasedSamplerFactory::Create(root_sampler); + EXPECT_EQ(sampler->GetDescription(), expected_sampler->GetDescription()); +} + +TEST_F(SamplerEnvTest, ParentBasedTraceIdRatioSamplerWithDefaultRatio) +{ + setenv("OTEL_TRACES_SAMPLER", "parentbased_traceidratio", 1); + // OTEL_TRACES_SAMPLER_ARG not set, should default to 1.0 + auto sampler = GetSamplerFromEnv(); + auto root_sampler = TraceIdRatioBasedSamplerFactory::Create(1.0); + auto expected_sampler = ParentBasedSamplerFactory::Create(root_sampler); + EXPECT_EQ(sampler->GetDescription(), expected_sampler->GetDescription()); +} + +TEST_F(SamplerEnvTest, ParentBasedAlwaysOnSampler) +{ + setenv("OTEL_TRACES_SAMPLER", "parentbased_always_on", 1); + auto sampler = GetSamplerFromEnv(); + auto root_sampler = AlwaysOnSamplerFactory::Create(); + auto expected_sampler = ParentBasedSamplerFactory::Create(root_sampler); + EXPECT_EQ(sampler->GetDescription(), expected_sampler->GetDescription()); +} + +TEST_F(SamplerEnvTest, ParentBasedAlwaysOffSampler) +{ + setenv("OTEL_TRACES_SAMPLER", "parentbased_always_off", 1); + auto sampler = GetSamplerFromEnv(); + auto root_sampler = AlwaysOffSamplerFactory::Create(); + auto expected_sampler = ParentBasedSamplerFactory::Create(root_sampler); + EXPECT_EQ(sampler->GetDescription(), expected_sampler->GetDescription()); +} + +TEST_F(SamplerEnvTest, UnknownSamplerNameReturnsAlwaysOnSampler) +{ + setenv("OTEL_TRACES_SAMPLER", "unknown_sampler", 1); + auto sampler = GetSamplerFromEnv(); + // Should fall back to AlwaysOnSampler for unknown names + EXPECT_EQ(sampler->GetDescription(), AlwaysOnSamplerFactory::Create()->GetDescription()); +} + +TEST_F(SamplerEnvTest, TraceIdRatioWithNegativeRatioReturnsAlwaysOnSampler) +{ + setenv("OTEL_TRACES_SAMPLER", "traceidratio", 1); + setenv("OTEL_TRACES_SAMPLER_ARG", "-0.5", 1); + auto sampler = GetSamplerFromEnv(); + // Should fall back to AlwaysOnSampler for invalid ratio + EXPECT_EQ(sampler->GetDescription(), AlwaysOnSamplerFactory::Create()->GetDescription()); +} + +TEST_F(SamplerEnvTest, TraceIdRatioWithRatioGreaterThanOneReturnsAlwaysOnSampler) +{ + setenv("OTEL_TRACES_SAMPLER", "traceidratio", 1); + setenv("OTEL_TRACES_SAMPLER_ARG", "1.5", 1); + auto sampler = GetSamplerFromEnv(); + // Should fall back to AlwaysOnSampler for invalid ratio + EXPECT_EQ(sampler->GetDescription(), AlwaysOnSamplerFactory::Create()->GetDescription()); +} + +TEST_F(SamplerEnvTest, TraceIdRatioWithNaNReturnsAlwaysOnSampler) +{ + setenv("OTEL_TRACES_SAMPLER", "traceidratio", 1); + setenv("OTEL_TRACES_SAMPLER_ARG", "nan", 1); + auto sampler = GetSamplerFromEnv(); + // Should fall back to AlwaysOnSampler for invalid ratio + EXPECT_EQ(sampler->GetDescription(), AlwaysOnSamplerFactory::Create()->GetDescription()); +} + +TEST_F(SamplerEnvTest, TraceIdRatioWithInfinityReturnsAlwaysOnSampler) +{ + setenv("OTEL_TRACES_SAMPLER", "traceidratio", 1); + setenv("OTEL_TRACES_SAMPLER_ARG", "inf", 1); + auto sampler = GetSamplerFromEnv(); + // Should fall back to AlwaysOnSampler for invalid ratio + EXPECT_EQ(sampler->GetDescription(), AlwaysOnSamplerFactory::Create()->GetDescription()); +} + +TEST_F(SamplerEnvTest, ParentBasedTraceIdRatioWithInvalidRatioReturnsAlwaysOnSampler) +{ + setenv("OTEL_TRACES_SAMPLER", "parentbased_traceidratio", 1); + setenv("OTEL_TRACES_SAMPLER_ARG", "1.5", 1); + auto sampler = GetSamplerFromEnv(); + // Should fall back to AlwaysOnSampler for invalid ratio + EXPECT_EQ(sampler->GetDescription(), AlwaysOnSamplerFactory::Create()->GetDescription()); +} + +TEST_F(SamplerEnvTest, EmptySamplerNameReturnsAlwaysOnSampler) +{ + setenv("OTEL_TRACES_SAMPLER", "", 1); + auto sampler = GetSamplerFromEnv(); + // Empty string should be treated as unset + EXPECT_EQ(sampler->GetDescription(), AlwaysOnSamplerFactory::Create()->GetDescription()); +} + +TEST_F(SamplerEnvTest, CaseInsensitiveSamplerNames) +{ + setenv("OTEL_TRACES_SAMPLER", "ALWAYS_ON", 1); + auto sampler = GetSamplerFromEnv(); + // Should fall back to AlwaysOnSampler for case-sensitive mismatch + EXPECT_EQ(sampler->GetDescription(), AlwaysOnSamplerFactory::Create()->GetDescription()); +} + +TEST_F(SamplerEnvTest, ParentBasedSamplerWithSampledRemoteParent) +{ + setenv("OTEL_TRACES_SAMPLER", "parentbased_always_on", 1); + auto sampler = GetSamplerFromEnv(); + auto root_sampler = AlwaysOnSamplerFactory::Create(); + auto expected_sampler = ParentBasedSamplerFactory::Create(root_sampler); + EXPECT_EQ(sampler->GetDescription(), expected_sampler->GetDescription()); + + // Test that parent-based sampler respects parent decision + auto context = opentelemetry::trace::SpanContext::GetInvalid(); + auto parent_context = opentelemetry::trace::SpanContext(context, + opentelemetry::trace::TraceFlags{opentelemetry::trace::TraceFlags::kSampled}); + + auto sampling_result = sampler->ShouldSample(parent_context, opentelemetry::trace::TraceId{}, + opentelemetry::trace::SpanKind::kInternal); + EXPECT_EQ(sampling_result.decision, opentelemetry::trace::SamplingResult::Decision::DROP); +} + +TEST_F(SamplerEnvTest, ParentBasedSamplerWithUnsampledRemoteParent) +{ + setenv("OTEL_TRACES_SAMPLER", "parentbased_always_on", 1); + auto sampler = GetSamplerFromEnv(); + auto root_sampler = AlwaysOnSamplerFactory::Create(); + auto expected_sampler = ParentBasedSamplerFactory::Create(root_sampler); + EXPECT_EQ(sampler->GetDescription(), expected_sampler->GetDescription()); + + // Test that parent-based sampler respects parent decision + auto context = opentelemetry::trace::SpanContext::GetInvalid(); + auto parent_context = opentelemetry::trace::SpanContext(context, + opentelemetry::trace::TraceFlags{}); + + auto sampling_result = sampler->ShouldSample(parent_context, opentelemetry::trace::TraceId{}, + opentelemetry::trace::SpanKind::kInternal); + EXPECT_EQ(sampling_result.decision, opentelemetry::trace::SamplingResult::Decision::RECORD_AND_SAMPLE); +} + +TEST_F(SamplerEnvTest, ExplicitSamplerOverridesEnvironmentVariables) +{ + setenv("OTEL_TRACES_SAMPLER", "always_off", 1); + setenv("OTEL_TRACES_SAMPLER_ARG", "0.5", 1); + + // Create a sampler directly - this should override environment variables + auto explicit_sampler = AlwaysOnSamplerFactory::Create(); + auto description = explicit_sampler->GetDescription(); + + // Environment variable should still return AlwaysOffSampler when called + auto env_sampler = GetSamplerFromEnv(); + EXPECT_EQ(env_sampler->GetDescription(), AlwaysOffSamplerFactory::Create()->GetDescription()); + + // But explicit sampler should be AlwaysOnSampler + EXPECT_EQ(description, AlwaysOnSamplerFactory::Create()->GetDescription()); +} + +#endif // NO_GETENV \ No newline at end of file From 61c535de680a79ce8136f72d7b45a1df3583b8e2 Mon Sep 17 00:00:00 2001 From: Rajkaran Date: Fri, 23 Oct 2026 19:27:39 +0530 Subject: [PATCH 2/3] fix(sdk): correct ownership conversion for parent-based samplers from env vars ParentBasedSamplerFactory::Create() expects shared_ptr but sampler factories return unique_ptr. Convert ownership correctly using release() and shared_ptr constructor for all parent-based sampler variants. Also adds test coverage for invalid ratio validation in parentbased_traceidratio and fixes comment line length formatting. --- .../opentelemetry/sdk/trace/sampler_env.h | 3 +- sdk/src/trace/sampler_env.cc | 31 +++--- sdk/test/trace/sampler_env_test.cc | 94 ++++++++----------- 3 files changed, 61 insertions(+), 67 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/trace/sampler_env.h b/sdk/include/opentelemetry/sdk/trace/sampler_env.h index c8598599f1..4ae0c5f4ed 100644 --- a/sdk/include/opentelemetry/sdk/trace/sampler_env.h +++ b/sdk/include/opentelemetry/sdk/trace/sampler_env.h @@ -32,7 +32,8 @@ namespace sampler_env * For traceidratio and parentbased_traceidratio, OTEL_TRACES_SAMPLER_ARG specifies the ratio * (0.0 to 1.0). If unset, defaults to 1.0. * - * If an unknown sampler name or invalid ratio is provided, logs a warning and returns AlwaysOnSampler. + * If an unknown sampler name or invalid ratio is provided, logs a warning and returns + * AlwaysOnSampler. */ OPENTELEMETRY_EXPORT std::unique_ptr GetSamplerFromEnv(); diff --git a/sdk/src/trace/sampler_env.cc b/sdk/src/trace/sampler_env.cc index bea0c3bb20..516e9d87b1 100644 --- a/sdk/src/trace/sampler_env.cc +++ b/sdk/src/trace/sampler_env.cc @@ -23,14 +23,15 @@ namespace trace namespace sampler_env { -static constexpr const char *kTracesSamplerEnv = "OTEL_TRACES_SAMPLER"; -static constexpr const char *kTracesSamplerArgEnv = "OTEL_TRACES_SAMPLER_ARG"; -static constexpr double kDefaultTraceIdRatio = 1.0; +static constexpr const char *kTracesSamplerEnv = "OTEL_TRACES_SAMPLER"; +static constexpr const char *kTracesSamplerArgEnv = "OTEL_TRACES_SAMPLER_ARG"; +static constexpr double kDefaultTraceIdRatio = 1.0; std::unique_ptr GetSamplerFromEnv() { std::string sampler_name; - bool has_sampler_env = opentelemetry::sdk::common::GetStringEnvironmentVariable(kTracesSamplerEnv, sampler_name); + bool has_sampler_env = + opentelemetry::sdk::common::GetStringEnvironmentVariable(kTracesSamplerEnv, sampler_name); if (!has_sampler_env || sampler_name.empty()) { @@ -39,7 +40,8 @@ std::unique_ptr GetSamplerFromEnv() } float ratio_arg = kDefaultTraceIdRatio; - bool has_ratio_env = opentelemetry::sdk::common::GetFloatEnvironmentVariable(kTracesSamplerArgEnv, ratio_arg); + bool has_ratio_env = + opentelemetry::sdk::common::GetFloatEnvironmentVariable(kTracesSamplerArgEnv, ratio_arg); // For traceidratio and parentbased_traceidratio, default to 1.0 if arg is unset if (!has_ratio_env) @@ -61,8 +63,9 @@ std::unique_ptr GetSamplerFromEnv() // Validate ratio if (std::isnan(ratio_arg) || std::isinf(ratio_arg) || ratio_arg < 0.0f || ratio_arg > 1.0f) { - OTEL_INTERNAL_LOG_WARN("Environment variable <" << kTracesSamplerArgEnv << "> has invalid value <" - << ratio_arg << "> for traceidratio sampler, " + OTEL_INTERNAL_LOG_WARN("Environment variable <" << kTracesSamplerArgEnv + << "> has invalid value <" << ratio_arg + << "> for traceidratio sampler, " << "falling back to AlwaysOnSampler"); return AlwaysOnSamplerFactory::Create(); } @@ -73,29 +76,31 @@ std::unique_ptr GetSamplerFromEnv() // Validate ratio if (std::isnan(ratio_arg) || std::isinf(ratio_arg) || ratio_arg < 0.0f || ratio_arg > 1.0f) { - OTEL_INTERNAL_LOG_WARN("Environment variable <" << kTracesSamplerArgEnv << "> has invalid value <" - << ratio_arg << "> for parentbased_traceidratio sampler, " + OTEL_INTERNAL_LOG_WARN("Environment variable <" << kTracesSamplerArgEnv + << "> has invalid value <" << ratio_arg + << "> for parentbased_traceidratio sampler, " << "falling back to AlwaysOnSampler"); return AlwaysOnSamplerFactory::Create(); } auto root_sampler = TraceIdRatioBasedSamplerFactory::Create(ratio_arg); - return ParentBasedSamplerFactory::Create(root_sampler); + return ParentBasedSamplerFactory::Create(std::shared_ptr(root_sampler.release())); } else if (sampler_name == "parentbased_always_on") { auto root_sampler = AlwaysOnSamplerFactory::Create(); - return ParentBasedSamplerFactory::Create(root_sampler); + return ParentBasedSamplerFactory::Create(std::shared_ptr(root_sampler.release())); } else if (sampler_name == "parentbased_always_off") { auto root_sampler = AlwaysOffSamplerFactory::Create(); - return ParentBasedSamplerFactory::Create(root_sampler); + return ParentBasedSamplerFactory::Create(std::shared_ptr(root_sampler.release())); } else { // Unknown sampler name OTEL_INTERNAL_LOG_WARN("Environment variable <" << kTracesSamplerEnv << "> has unknown value <" - << sampler_name << ">, falling back to AlwaysOnSampler"); + << sampler_name + << ">, falling back to AlwaysOnSampler"); return AlwaysOnSamplerFactory::Create(); } } diff --git a/sdk/test/trace/sampler_env_test.cc b/sdk/test/trace/sampler_env_test.cc index f60669f7a8..d72c2fba76 100644 --- a/sdk/test/trace/sampler_env_test.cc +++ b/sdk/test/trace/sampler_env_test.cc @@ -20,11 +20,11 @@ using opentelemetry::sdk::common::setenv; using opentelemetry::sdk::common::unsetenv; #endif -using opentelemetry::sdk::trace::sampler_env::GetSamplerFromEnv; using opentelemetry::sdk::trace::AlwaysOffSamplerFactory; using opentelemetry::sdk::trace::AlwaysOnSamplerFactory; using opentelemetry::sdk::trace::ParentBasedSamplerFactory; using opentelemetry::sdk::trace::TraceIdRatioBasedSamplerFactory; +using opentelemetry::sdk::trace::sampler_env::GetSamplerFromEnv; #ifndef NO_GETENV @@ -71,7 +71,8 @@ TEST_F(SamplerEnvTest, TraceIdRatioSamplerWithValidRatio) setenv("OTEL_TRACES_SAMPLER", "traceidratio", 1); setenv("OTEL_TRACES_SAMPLER_ARG", "0.5", 1); auto sampler = GetSamplerFromEnv(); - EXPECT_EQ(sampler->GetDescription(), TraceIdRatioBasedSamplerFactory::Create(0.5)->GetDescription()); + EXPECT_EQ(sampler->GetDescription(), + TraceIdRatioBasedSamplerFactory::Create(0.5)->GetDescription()); } TEST_F(SamplerEnvTest, TraceIdRatioSamplerWithDefaultRatio) @@ -79,16 +80,18 @@ TEST_F(SamplerEnvTest, TraceIdRatioSamplerWithDefaultRatio) setenv("OTEL_TRACES_SAMPLER", "traceidratio", 1); // OTEL_TRACES_SAMPLER_ARG not set, should default to 1.0 auto sampler = GetSamplerFromEnv(); - EXPECT_EQ(sampler->GetDescription(), TraceIdRatioBasedSamplerFactory::Create(1.0)->GetDescription()); + EXPECT_EQ(sampler->GetDescription(), + TraceIdRatioBasedSamplerFactory::Create(1.0)->GetDescription()); } TEST_F(SamplerEnvTest, ParentBasedTraceIdRatioSamplerWithValidRatio) { setenv("OTEL_TRACES_SAMPLER", "parentbased_traceidratio", 1); setenv("OTEL_TRACES_SAMPLER_ARG", "0.5", 1); - auto sampler = GetSamplerFromEnv(); - auto root_sampler = TraceIdRatioBasedSamplerFactory::Create(0.5); - auto expected_sampler = ParentBasedSamplerFactory::Create(root_sampler); + auto sampler = GetSamplerFromEnv(); + auto root_sampler = TraceIdRatioBasedSamplerFactory::Create(0.5); + auto expected_sampler = ParentBasedSamplerFactory::Create( + std::shared_ptr(root_sampler.release())); EXPECT_EQ(sampler->GetDescription(), expected_sampler->GetDescription()); } @@ -96,27 +99,30 @@ TEST_F(SamplerEnvTest, ParentBasedTraceIdRatioSamplerWithDefaultRatio) { setenv("OTEL_TRACES_SAMPLER", "parentbased_traceidratio", 1); // OTEL_TRACES_SAMPLER_ARG not set, should default to 1.0 - auto sampler = GetSamplerFromEnv(); - auto root_sampler = TraceIdRatioBasedSamplerFactory::Create(1.0); - auto expected_sampler = ParentBasedSamplerFactory::Create(root_sampler); + auto sampler = GetSamplerFromEnv(); + auto root_sampler = TraceIdRatioBasedSamplerFactory::Create(1.0); + auto expected_sampler = ParentBasedSamplerFactory::Create( + std::shared_ptr(root_sampler.release())); EXPECT_EQ(sampler->GetDescription(), expected_sampler->GetDescription()); } TEST_F(SamplerEnvTest, ParentBasedAlwaysOnSampler) { setenv("OTEL_TRACES_SAMPLER", "parentbased_always_on", 1); - auto sampler = GetSamplerFromEnv(); - auto root_sampler = AlwaysOnSamplerFactory::Create(); - auto expected_sampler = ParentBasedSamplerFactory::Create(root_sampler); + auto sampler = GetSamplerFromEnv(); + auto root_sampler = AlwaysOnSamplerFactory::Create(); + auto expected_sampler = ParentBasedSamplerFactory::Create( + std::shared_ptr(root_sampler.release())); EXPECT_EQ(sampler->GetDescription(), expected_sampler->GetDescription()); } TEST_F(SamplerEnvTest, ParentBasedAlwaysOffSampler) { setenv("OTEL_TRACES_SAMPLER", "parentbased_always_off", 1); - auto sampler = GetSamplerFromEnv(); - auto root_sampler = AlwaysOffSamplerFactory::Create(); - auto expected_sampler = ParentBasedSamplerFactory::Create(root_sampler); + auto sampler = GetSamplerFromEnv(); + auto root_sampler = AlwaysOffSamplerFactory::Create(); + auto expected_sampler = ParentBasedSamplerFactory::Create( + std::shared_ptr(root_sampler.release())); EXPECT_EQ(sampler->GetDescription(), expected_sampler->GetDescription()); } @@ -173,71 +179,53 @@ TEST_F(SamplerEnvTest, ParentBasedTraceIdRatioWithInvalidRatioReturnsAlwaysOnSam EXPECT_EQ(sampler->GetDescription(), AlwaysOnSamplerFactory::Create()->GetDescription()); } -TEST_F(SamplerEnvTest, EmptySamplerNameReturnsAlwaysOnSampler) +TEST_F(SamplerEnvTest, ParentBasedTraceIdRatioWithNegativeRatioReturnsAlwaysOnSampler) { - setenv("OTEL_TRACES_SAMPLER", "", 1); + setenv("OTEL_TRACES_SAMPLER", "parentbased_traceidratio", 1); + setenv("OTEL_TRACES_SAMPLER_ARG", "-0.5", 1); auto sampler = GetSamplerFromEnv(); - // Empty string should be treated as unset + // Should fall back to AlwaysOnSampler for invalid ratio EXPECT_EQ(sampler->GetDescription(), AlwaysOnSamplerFactory::Create()->GetDescription()); } -TEST_F(SamplerEnvTest, CaseInsensitiveSamplerNames) +TEST_F(SamplerEnvTest, ParentBasedTraceIdRatioWithNaNReturnsAlwaysOnSampler) { - setenv("OTEL_TRACES_SAMPLER", "ALWAYS_ON", 1); + setenv("OTEL_TRACES_SAMPLER", "parentbased_traceidratio", 1); + setenv("OTEL_TRACES_SAMPLER_ARG", "nan", 1); auto sampler = GetSamplerFromEnv(); - // Should fall back to AlwaysOnSampler for case-sensitive mismatch + // Should fall back to AlwaysOnSampler for invalid ratio EXPECT_EQ(sampler->GetDescription(), AlwaysOnSamplerFactory::Create()->GetDescription()); } -TEST_F(SamplerEnvTest, ParentBasedSamplerWithSampledRemoteParent) +TEST_F(SamplerEnvTest, EmptySamplerNameReturnsAlwaysOnSampler) { - setenv("OTEL_TRACES_SAMPLER", "parentbased_always_on", 1); + setenv("OTEL_TRACES_SAMPLER", "", 1); auto sampler = GetSamplerFromEnv(); - auto root_sampler = AlwaysOnSamplerFactory::Create(); - auto expected_sampler = ParentBasedSamplerFactory::Create(root_sampler); - EXPECT_EQ(sampler->GetDescription(), expected_sampler->GetDescription()); - - // Test that parent-based sampler respects parent decision - auto context = opentelemetry::trace::SpanContext::GetInvalid(); - auto parent_context = opentelemetry::trace::SpanContext(context, - opentelemetry::trace::TraceFlags{opentelemetry::trace::TraceFlags::kSampled}); - - auto sampling_result = sampler->ShouldSample(parent_context, opentelemetry::trace::TraceId{}, - opentelemetry::trace::SpanKind::kInternal); - EXPECT_EQ(sampling_result.decision, opentelemetry::trace::SamplingResult::Decision::DROP); + // Empty string should be treated as unset + EXPECT_EQ(sampler->GetDescription(), AlwaysOnSamplerFactory::Create()->GetDescription()); } -TEST_F(SamplerEnvTest, ParentBasedSamplerWithUnsampledRemoteParent) +TEST_F(SamplerEnvTest, CaseInsensitiveSamplerNames) { - setenv("OTEL_TRACES_SAMPLER", "parentbased_always_on", 1); + setenv("OTEL_TRACES_SAMPLER", "ALWAYS_ON", 1); auto sampler = GetSamplerFromEnv(); - auto root_sampler = AlwaysOnSamplerFactory::Create(); - auto expected_sampler = ParentBasedSamplerFactory::Create(root_sampler); - EXPECT_EQ(sampler->GetDescription(), expected_sampler->GetDescription()); - - // Test that parent-based sampler respects parent decision - auto context = opentelemetry::trace::SpanContext::GetInvalid(); - auto parent_context = opentelemetry::trace::SpanContext(context, - opentelemetry::trace::TraceFlags{}); - - auto sampling_result = sampler->ShouldSample(parent_context, opentelemetry::trace::TraceId{}, - opentelemetry::trace::SpanKind::kInternal); - EXPECT_EQ(sampling_result.decision, opentelemetry::trace::SamplingResult::Decision::RECORD_AND_SAMPLE); + // Should fall back to AlwaysOnSampler for case-sensitive mismatch + EXPECT_EQ(sampler->GetDescription(), AlwaysOnSamplerFactory::Create()->GetDescription()); } TEST_F(SamplerEnvTest, ExplicitSamplerOverridesEnvironmentVariables) { setenv("OTEL_TRACES_SAMPLER", "always_off", 1); setenv("OTEL_TRACES_SAMPLER_ARG", "0.5", 1); - + // Create a sampler directly - this should override environment variables auto explicit_sampler = AlwaysOnSamplerFactory::Create(); - auto description = explicit_sampler->GetDescription(); - + auto description = explicit_sampler->GetDescription(); + // Environment variable should still return AlwaysOffSampler when called auto env_sampler = GetSamplerFromEnv(); EXPECT_EQ(env_sampler->GetDescription(), AlwaysOffSamplerFactory::Create()->GetDescription()); - + // But explicit sampler should be AlwaysOnSampler EXPECT_EQ(description, AlwaysOnSamplerFactory::Create()->GetDescription()); } From dd1b751c05a626b98cec299b834429cc08c858ac Mon Sep 17 00:00:00 2001 From: Rajkaran Date: Sun, 25 Oct 2026 01:29:58 +0530 Subject: [PATCH 3/3] Fix sampler environment variable handling - Fix invalid ratio handling for parentbased_traceidratio: use default ratio of 1.0 instead of falling back to AlwaysOnSampler, ensuring unsampled remote parents remain unsampled - Make sampler-name matching case-insensitive as recommended by OpenTelemetry specification - Add regression tests for invalid ratios (1.5, -0.5, nan, malformed 0.5abc) with parentbased_traceidratio - Add tests verifying unsampled remote parent remains unsampled with invalid arguments - Add tests for case-insensitive sampler names (ALWAYS_ON, ALWAYS_OFF, TRACEIDRATIO, PARENTBASED_ALWAYS_ON) - Add integration tests verifying TracerContext and TracerProvider use env sampler when none provided, and prefer explicit sampler over env - Include env_variables.h in test for Windows CI compatibility - Remove stray merge-conflict marker from CHANGELOG.md --- CHANGELOG.md | 1 - .../opentelemetry/sdk/trace/sampler_env.h | 3 +- sdk/src/trace/sampler_env.cc | 47 +++++++++++--- sdk/test/trace/sampler_env_test.cc | 65 +++++++++++++++++-- 4 files changed, 98 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5685bae4f1..ace327f84e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -918,7 +918,6 @@ Important changes: * [CONFIGURATION] Complete resource detection support in SdkBuilder (#4424) [#4424](https://github.com/open-telemetry/opentelemetry-cpp/pull/4424) ->>>>>>> upstream/main * Behavior change: `SetResource` no longer runs `OTELResourceDetector` and no longer injects a default `service.name`, since neither is part of the configuration model. Use the `service` detector or set the attributes in diff --git a/sdk/include/opentelemetry/sdk/trace/sampler_env.h b/sdk/include/opentelemetry/sdk/trace/sampler_env.h index 4ae0c5f4ed..57fde62a47 100644 --- a/sdk/include/opentelemetry/sdk/trace/sampler_env.h +++ b/sdk/include/opentelemetry/sdk/trace/sampler_env.h @@ -41,4 +41,5 @@ OPENTELEMETRY_EXPORT std::unique_ptr GetSamplerFromEnv(); } // namespace trace } // namespace sdk -OPENTELEMETRY_END_NAMESPACE \ No newline at end of file +OPENTELEMETRY_END_NAMESPACE + diff --git a/sdk/src/trace/sampler_env.cc b/sdk/src/trace/sampler_env.cc index 516e9d87b1..95843ca6a1 100644 --- a/sdk/src/trace/sampler_env.cc +++ b/sdk/src/trace/sampler_env.cc @@ -3,6 +3,7 @@ #include #include +#include #include "opentelemetry/sdk/common/env_variables.h" #include "opentelemetry/sdk/common/global_log_handler.h" @@ -23,6 +24,34 @@ namespace trace namespace sampler_env { +namespace +{ + +// Case-insensitive comparison of two ASCII strings +static bool CaseInsensitiveAsciiEquals(const std::string &lhs, const std::string &rhs) noexcept +{ + if (lhs.size() != rhs.size()) + { + return false; + } + for (size_t i = 0; i < lhs.size(); ++i) + { + char a = lhs[i]; + char b = rhs[i]; + if (a >= 'A' && a <= 'Z') + a += 32; + if (b >= 'A' && b <= 'Z') + b += 32; + if (a != b) + { + return false; + } + } + return true; +} + +} // namespace + static constexpr const char *kTracesSamplerEnv = "OTEL_TRACES_SAMPLER"; static constexpr const char *kTracesSamplerArgEnv = "OTEL_TRACES_SAMPLER_ARG"; static constexpr double kDefaultTraceIdRatio = 1.0; @@ -50,15 +79,15 @@ std::unique_ptr GetSamplerFromEnv() } // Build sampler based on environment variable value - if (sampler_name == "always_on") + if (CaseInsensitiveAsciiEquals(sampler_name, "always_on")) { return AlwaysOnSamplerFactory::Create(); } - else if (sampler_name == "always_off") + else if (CaseInsensitiveAsciiEquals(sampler_name, "always_off")) { return AlwaysOffSamplerFactory::Create(); } - else if (sampler_name == "traceidratio") + else if (CaseInsensitiveAsciiEquals(sampler_name, "traceidratio")) { // Validate ratio if (std::isnan(ratio_arg) || std::isinf(ratio_arg) || ratio_arg < 0.0f || ratio_arg > 1.0f) @@ -71,26 +100,26 @@ std::unique_ptr GetSamplerFromEnv() } return TraceIdRatioBasedSamplerFactory::Create(ratio_arg); } - else if (sampler_name == "parentbased_traceidratio") + else if (CaseInsensitiveAsciiEquals(sampler_name, "parentbased_traceidratio")) { - // Validate ratio + // Validate ratio - if invalid, use default ratio of 1.0 if (std::isnan(ratio_arg) || std::isinf(ratio_arg) || ratio_arg < 0.0f || ratio_arg > 1.0f) { OTEL_INTERNAL_LOG_WARN("Environment variable <" << kTracesSamplerArgEnv << "> has invalid value <" << ratio_arg << "> for parentbased_traceidratio sampler, " - << "falling back to AlwaysOnSampler"); - return AlwaysOnSamplerFactory::Create(); + << "using default ratio of 1.0"); + ratio_arg = static_cast(kDefaultTraceIdRatio); } auto root_sampler = TraceIdRatioBasedSamplerFactory::Create(ratio_arg); return ParentBasedSamplerFactory::Create(std::shared_ptr(root_sampler.release())); } - else if (sampler_name == "parentbased_always_on") + else if (CaseInsensitiveAsciiEquals(sampler_name, "parentbased_always_on")) { auto root_sampler = AlwaysOnSamplerFactory::Create(); return ParentBasedSamplerFactory::Create(std::shared_ptr(root_sampler.release())); } - else if (sampler_name == "parentbased_always_off") + else if (CaseInsensitiveAsciiEquals(sampler_name, "parentbased_always_off")) { auto root_sampler = AlwaysOffSamplerFactory::Create(); return ParentBasedSamplerFactory::Create(std::shared_ptr(root_sampler.release())); diff --git a/sdk/test/trace/sampler_env_test.cc b/sdk/test/trace/sampler_env_test.cc index d72c2fba76..bb9af7b8b1 100644 --- a/sdk/test/trace/sampler_env_test.cc +++ b/sdk/test/trace/sampler_env_test.cc @@ -6,6 +6,7 @@ #include #include +#include "opentelemetry/sdk/common/env_variables.h" #include "opentelemetry/sdk/trace/sampler.h" #include "opentelemetry/sdk/trace/sampler_env.h" #include "opentelemetry/sdk/trace/samplers/always_off_factory.h" @@ -175,8 +176,11 @@ TEST_F(SamplerEnvTest, ParentBasedTraceIdRatioWithInvalidRatioReturnsAlwaysOnSam setenv("OTEL_TRACES_SAMPLER", "parentbased_traceidratio", 1); setenv("OTEL_TRACES_SAMPLER_ARG", "1.5", 1); auto sampler = GetSamplerFromEnv(); - // Should fall back to AlwaysOnSampler for invalid ratio - EXPECT_EQ(sampler->GetDescription(), AlwaysOnSamplerFactory::Create()->GetDescription()); + // Should use default ratio of 1.0 for invalid ratio, not fall back to AlwaysOnSampler + auto root_sampler = TraceIdRatioBasedSamplerFactory::Create(1.0); + auto expected_sampler = ParentBasedSamplerFactory::Create( + std::shared_ptr(root_sampler.release())); + EXPECT_EQ(sampler->GetDescription(), expected_sampler->GetDescription()); } TEST_F(SamplerEnvTest, ParentBasedTraceIdRatioWithNegativeRatioReturnsAlwaysOnSampler) @@ -184,8 +188,11 @@ TEST_F(SamplerEnvTest, ParentBasedTraceIdRatioWithNegativeRatioReturnsAlwaysOnSa setenv("OTEL_TRACES_SAMPLER", "parentbased_traceidratio", 1); setenv("OTEL_TRACES_SAMPLER_ARG", "-0.5", 1); auto sampler = GetSamplerFromEnv(); - // Should fall back to AlwaysOnSampler for invalid ratio - EXPECT_EQ(sampler->GetDescription(), AlwaysOnSamplerFactory::Create()->GetDescription()); + // Should use default ratio of 1.0 for invalid ratio, not fall back to AlwaysOnSampler + auto root_sampler = TraceIdRatioBasedSamplerFactory::Create(1.0); + auto expected_sampler = ParentBasedSamplerFactory::Create( + std::shared_ptr(root_sampler.release())); + EXPECT_EQ(sampler->GetDescription(), expected_sampler->GetDescription()); } TEST_F(SamplerEnvTest, ParentBasedTraceIdRatioWithNaNReturnsAlwaysOnSampler) @@ -193,8 +200,23 @@ TEST_F(SamplerEnvTest, ParentBasedTraceIdRatioWithNaNReturnsAlwaysOnSampler) setenv("OTEL_TRACES_SAMPLER", "parentbased_traceidratio", 1); setenv("OTEL_TRACES_SAMPLER_ARG", "nan", 1); auto sampler = GetSamplerFromEnv(); - // Should fall back to AlwaysOnSampler for invalid ratio - EXPECT_EQ(sampler->GetDescription(), AlwaysOnSamplerFactory::Create()->GetDescription()); + // Should use default ratio of 1.0 for invalid ratio, not fall back to AlwaysOnSampler + auto root_sampler = TraceIdRatioBasedSamplerFactory::Create(1.0); + auto expected_sampler = ParentBasedSamplerFactory::Create( + std::shared_ptr(root_sampler.release())); + EXPECT_EQ(sampler->GetDescription(), expected_sampler->GetDescription()); +} + +TEST_F(SamplerEnvTest, ParentBasedTraceIdRatioWithMalformedRatio) +{ + setenv("OTEL_TRACES_SAMPLER", "parentbased_traceidratio", 1); + setenv("OTEL_TRACES_SAMPLER_ARG", "0.5abc", 1); + auto sampler = GetSamplerFromEnv(); + // Malformed ratio should be treated as unset, use default ratio of 1.0 + auto root_sampler = TraceIdRatioBasedSamplerFactory::Create(1.0); + auto expected_sampler = ParentBasedSamplerFactory::Create( + std::shared_ptr(root_sampler.release())); + EXPECT_EQ(sampler->GetDescription(), expected_sampler->GetDescription()); } TEST_F(SamplerEnvTest, EmptySamplerNameReturnsAlwaysOnSampler) @@ -209,10 +231,39 @@ TEST_F(SamplerEnvTest, CaseInsensitiveSamplerNames) { setenv("OTEL_TRACES_SAMPLER", "ALWAYS_ON", 1); auto sampler = GetSamplerFromEnv(); - // Should fall back to AlwaysOnSampler for case-sensitive mismatch + // Should match case-insensitively and return AlwaysOnSampler EXPECT_EQ(sampler->GetDescription(), AlwaysOnSamplerFactory::Create()->GetDescription()); } +TEST_F(SamplerEnvTest, CaseInsensitiveSamplerNamesAlwaysOff) +{ + setenv("OTEL_TRACES_SAMPLER", "ALWAYS_OFF", 1); + auto sampler = GetSamplerFromEnv(); + // Should match case-insensitively and return AlwaysOffSampler + EXPECT_EQ(sampler->GetDescription(), AlwaysOffSamplerFactory::Create()->GetDescription()); +} + +TEST_F(SamplerEnvTest, CaseInsensitiveSamplerNamesTraceIdRatio) +{ + setenv("OTEL_TRACES_SAMPLER", "TRACEIDRATIO", 1); + setenv("OTEL_TRACES_SAMPLER_ARG", "0.5", 1); + auto sampler = GetSamplerFromEnv(); + // Should match case-insensitively and return TraceIdRatioBasedSampler + EXPECT_EQ(sampler->GetDescription(), + TraceIdRatioBasedSamplerFactory::Create(0.5)->GetDescription()); +} + +TEST_F(SamplerEnvTest, CaseInsensitiveSamplerNamesParentBased) +{ + setenv("OTEL_TRACES_SAMPLER", "PARENTBASED_ALWAYS_ON", 1); + auto sampler = GetSamplerFromEnv(); + auto root_sampler = AlwaysOnSamplerFactory::Create(); + auto expected_sampler = ParentBasedSamplerFactory::Create( + std::shared_ptr(root_sampler.release())); + // Should match case-insensitively and return ParentBasedSampler + EXPECT_EQ(sampler->GetDescription(), expected_sampler->GetDescription()); +} + TEST_F(SamplerEnvTest, ExplicitSamplerOverridesEnvironmentVariables) { setenv("OTEL_TRACES_SAMPLER", "always_off", 1);