Skip to content

feat(configuration): add sampler parsing#6409

Merged
maryliag merged 16 commits intoopen-telemetry:mainfrom
honeycombio:mike/parse-samplers
Mar 13, 2026
Merged

feat(configuration): add sampler parsing#6409
maryliag merged 16 commits intoopen-telemetry:mainfrom
honeycombio:mike/parse-samplers

Conversation

@MikeGoldsmith
Copy link
Copy Markdown
Member

@MikeGoldsmith MikeGoldsmith commented Feb 13, 2026

Which problem is this PR solving?

Fixes #6290

RC3 spec requires sampler configuration via YAML files and environment variables. Currently missing:

  • Type models for probability/development and composite/development samplers
  • YAML sampler parsing in FileConfigFactory
  • Environment variable sampler parsing (OTEL_TRACES_SAMPLER, OTEL_TRACES_SAMPLER_ARG)

Note: jaeger_remote/development is not included as it is an SDK extension plugin (per the spec's isSdkExtensionPlugin: true annotation on the Sampler type) and is expected to be provided via a third-party extension rather than implemented natively. This is consistent with the Java SDK's approach.

Short description of the changes

Type Models (tracerProviderModel.ts):

  • Added ExperimentalProbabilitySampler interface (matching spec name)
  • Added ExperimentalComposableSampler, ExperimentalComposableRuleBasedSampler, ExperimentalComposableRuleBasedSamplerRule, ExperimentalComposableParentThresholdSampler, ExperimentalComposableProbabilitySampler interfaces
  • Extended Sampler union with probability/development and composite/development

YAML Parsing (FileConfigFactory.ts):

  • Added parseSampler() function supporting all sampler types
  • Added parseComposableSampler() helper for ExperimentalComposableSampler types
  • Recursive parsing for nested samplers (parent_based.root, rule-based rules, etc.)
  • Integrated into setTracerProvider()

Environment Variable Parsing (EnvironmentConfigFactory.ts):

  • Added setSampler() function (called from within setTracerProvider()) supporting:
    • always_on, always_off, traceidratio
    • parentbased_always_on, parentbased_always_off, parentbased_traceidratio
  • Parses OTEL_TRACES_SAMPLER_ARG for ratio values

Tests:

  • Env var sampler tests: always_on, always_off, traceidratio, parentbased_always_on, parentbased_always_off, parentbased_traceidratio, unknown sampler warning
  • YAML fixture tests: samplers.yaml (parent_based with probability/development), composite-sampler-array.yaml (rule_based with multiple rules), composite-sampler-rulebased-full.yaml (rule_based with attribute matching)

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

  • Unit tests for env var sampler parsing (always_on, always_off, traceidratio, parentbased variants)
  • Integration tests for YAML sampler parsing (basic, composite rule_based)
  • Existing kitchen-sink.yaml includes complex sampler configurations

Checklist:

  • Followed the style guidelines of this project
  • Unit tests have been added
  • Documentation has been updated (CHANGELOG)

- Add type models: ProbabilitySampler, CompositeSampler, RuleBasedSampler
- Implement YAML sampler parsing in FileConfigFactory.parseSampler()
- Implement env var parsing (OTEL_TRACES_SAMPLER, OTEL_TRACES_SAMPLER_ARG)
- Add tests for always_on, always_off, traceidratio, parentbased samplers
- Support probability/development and composite/development sampler types

Fixes open-telemetry#6290
@MikeGoldsmith MikeGoldsmith requested a review from a team as a code owner February 13, 2026 15:11
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 13, 2026

Codecov Report

❌ Patch coverage is 95.04950% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.74%. Comparing base (2d361ec) to head (a1ba584).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...al/packages/configuration/src/FileConfigFactory.ts 93.05% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6409      +/-   ##
==========================================
- Coverage   95.74%   95.74%   -0.01%     
==========================================
  Files         364      364              
  Lines       11871    11972     +101     
  Branches     2789     2828      +39     
==========================================
+ Hits        11366    11462      +96     
- Misses        505      510       +5     
Files with missing lines Coverage Δ
...ages/configuration/src/EnvironmentConfigFactory.ts 99.56% <100.00%> (+0.02%) ⬆️
...es/configuration/src/models/tracerProviderModel.ts 100.00% <ø> (ø)
...al/packages/configuration/src/FileConfigFactory.ts 97.12% <93.05%> (-0.43%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

MikeGoldsmith and others added 6 commits February 13, 2026 16:33
- Convert null values to undefined for always_on/always_off samplers
- Ensures consistent behavior with existing defaults
- YAML 'always_on:' (no value) → undefined, matching initializeDefaultTracerProviderConfiguration()

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add probability sampler type (non-development)
- Make CompositeSampler.samplers optional
- Expand RuleBasedSamplerRule with full spec fields
- Recursively parse samplers in rule_based rules
- Update kitchen-sink test expectations
- Add unknown sampler type warning test
- Add parentbased_always_off env var test
- Add composite sampler with samplers array test
- Add composite sampler with rule_based full features test
- Coverage: EnvironmentConfigFactory 98.89%, FileConfigFactory 99.82%
Comment thread experimental/packages/configuration/src/models/tracerProviderModel.ts Outdated
Comment thread experimental/packages/configuration/src/models/tracerProviderModel.ts Outdated
Comment thread experimental/packages/configuration/src/models/tracerProviderModel.ts Outdated
Comment thread experimental/packages/configuration/src/models/tracerProviderModel.ts Outdated
Comment thread experimental/packages/configuration/src/EnvironmentConfigFactory.ts Outdated
Comment thread experimental/packages/configuration/src/FileConfigFactory.ts
@MikeGoldsmith
Copy link
Copy Markdown
Member Author

Thank you for the review @maryliag and sorry for the delay getting back to you sooner. I'll work through your feedback and get back to you ASAP.

Copy link
Copy Markdown
Contributor

@maryliag maryliag left a comment

Choose a reason for hiding this comment

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

thank you for the changes, just a small nit

Comment thread experimental/packages/configuration/src/FileConfigFactory.ts
Comment thread experimental/packages/configuration/src/FileConfigFactory.ts
Assisted-by: Claude Sonnet 4.6
Comment thread experimental/CHANGELOG.md Outdated
@trentm trentm added this pull request to the merge queue Mar 10, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Mar 10, 2026
…ILE env var

Upstream main renamed OTEL_EXPERIMENTAL_CONFIG_FILE to OTEL_CONFIG_FILE
in open-telemetry#6486, but the new sampler parsing tests still referenced the old name.

Assisted-by: Claude Sonnet 4.6
@maryliag maryliag added this pull request to the merge queue Mar 13, 2026
Merged via the queue into open-telemetry:main with commit 425bbfe Mar 13, 2026
27 checks passed
@MikeGoldsmith MikeGoldsmith deleted the mike/parse-samplers branch March 16, 2026 18:07
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.

configuration package should parse tracer provider > sampler

3 participants