Skip to content

feat(sdk): support trace sampler environment variables - #4729

Open
Rajkaran-122 wants to merge 4 commits into
open-telemetry:mainfrom
Rajkaran-122:feat/sdk-trace-sampler-env-vars
Open

Rajkaran-122 wants to merge 4 commits into
open-telemetry:mainfrom
Rajkaran-122:feat/sdk-trace-sampler-env-vars

Conversation

@Rajkaran-122

@Rajkaran-122 Rajkaran-122 commented Oct 23, 2026 •

Copy link
Copy Markdown

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

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 open-telemetry#4709
@codecov

codecov Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.23810% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.74%. Comparing base (ac7b87a) to head (dd1b751).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
sdk/src/trace/sampler_env.cc 95.92% 2 Missing ⚠️
sdk/src/trace/tracer_provider.cc 88.89% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4729      +/-   ##
==========================================
+ Coverage   86.69%   86.74%   +0.05%     
==========================================
  Files         527      528       +1     
  Lines       20902    20958      +56     
==========================================
+ Hits        18119    18178      +59     
+ Misses       2783     2780       -3     
Files with missing lines Coverage Δ
...k/include/opentelemetry/sdk/trace/tracer_context.h 100.00% <ø> (ø)
sdk/src/trace/tracer_context.cc 83.34% <100.00%> (+0.99%) ⬆️
sdk/src/trace/tracer_provider_factory.cc 100.00% <100.00%> (ø)
sdk/src/trace/tracer_provider.cc 89.25% <88.89%> (-0.52%) ⬇️
sdk/src/trace/sampler_env.cc 95.92% <95.92%> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

… 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.
@Rajkaran-122
Rajkaran-122 force-pushed the feat/sdk-trace-sampler-env-vars branch from 42e557f to 61c535d Compare October 23, 2026 14:03
Resolves conflicts in CHANGELOG.md and tracer_provider.cc by keeping
sampler environment variable integration.

@mateenali66 mateenali66 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ran it at ad74150f. unset matches main on all three paths, and the six values reach the constructors and both factories. a few things:

  1. parentbased_traceidratio with an out of range arg (1.5, -0.5, nan) returns plain AlwaysOnSampler, so a child of an unsampled remote parent gets sampled. the spec says an invalid arg MUST be ignored as if unset, which is ratio 1.0 with the wrapper kept. 0.5abc already ends up there, so the two invalid paths disagree.
  2. names are matched case sensitively, so ALWAYS_OFF samples everything. the spec says enum values SHOULD be case-insensitive, and meter_context.cc already does it for the exemplar filter. CaseInsensitiveSamplerNames passes either way.
  3. no test sets the variable and then builds a provider or context.
  4. CHANGELOG.md:921 has a leftover >>>>>>> upstream/main, and the Windows jobs need env_variables.h in the test for setenv.

@Rajkaran-122
Rajkaran-122 force-pushed the feat/sdk-trace-sampler-env-vars branch from 7bb4b61 to 3c9d85e Compare October 24, 2026 20:14
- 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
@Rajkaran-122
Rajkaran-122 force-pushed the feat/sdk-trace-sampler-env-vars branch from 3c9d85e to dd1b751 Compare October 24, 2026 22:04
@mateenali66

Copy link
Copy Markdown
Member

re-ran at dd1b751. points 1, 2 and 4 are fixed. PARENTBASED_TRACEIDRATIO with 1.5 now gives ParentBased{TraceIdRatioBasedSampler{1.000000}} from the provider constructor, the factory and the context factory.

still open:

  1. CI is red on this PR's own changes. sampler_env.cc has no newline at the end, which fails the 8 clang builds under -Wnewline-eof. format wants sampler_env.h and tracer_provider.cc, markdown-lint wants * bullets and 80 columns in the CHANGELOG entry, iwyu wants always_on_factory.h out of tracer_context_factory.cc, and clang-tidy wants SamplerEnvTest in an anonymous namespace.
  2. still no test that sets the variable and then builds a TracerProvider or TracerContext.
  3. the last CHANGELOG bullet and the three ParentBasedTraceIdRatioWith...ReturnsAlwaysOnSampler test names still describe the old fallback.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[SDK] Support OTEL_TRACES_SAMPLER and OTEL_TRACES_SAMPLER_ARG environment variables

2 participants