feat(sampler-composite): implement ProbabilitySampler, deprecate TraceIdRatioBasedSampler by nabeelamjadsheikh · Pull Request #7029 · open-telemetry/opentelemetry-js

nabeelamjadsheikh · 2026-08-24T09:22:20Z

What

Adds createProbabilitySampler(ratio) to @opentelemetry/sampler-composite, implementing the spec's ProbabilitySampler, and marks TraceIdRatioBasedSampler as @deprecated.

import { createProbabilitySampler } from '@opentelemetry/sampler-composite';

// sample 10% of traces, consistently across every SDK that handles the trace
const sampler = createProbabilitySampler(0.1);

Answering the open questions on the issue

@ravitheja4531-cell raised five questions that were never answered. Here's where I landed on each, so they're easy to push back on:

1. Option A (export from sampler-composite) vs Option B (new package). I went with A. @trentm suggested a new experimental sampler-$name package, so this is the one place I've knowingly diverged — happy to move it if you'd rather. My reasoning: sampler-composite is already an experimental package, so it satisfies the "keep it experimental until stable" goal without a new package whose only content depends entirely on sampler-composite. The spec also documents both samplers side by side. The eventual path trentm described — folding into the stable SDK once ProbabilitySampler stabilises — is unaffected either way.

2. Dependency on #6540. Resolved by events: #6540 landed, so createComposableProbabilitySampler already exists under its spec name and this builds directly on it. (The code sketch in that comment referenced createComposableTraceIDRatioBasedSampler/traceidratio.ts, which no longer exist.)

3. toString(). Returns ProbabilitySampler{0.1}, matching the familiar TraceIdRatioBased{RATIO} shape. The spec mandates a description format for TraceIdRatioBased but not for ProbabilitySampler, so this is a convention choice.

4. Compatibility warning — initial or follow-up? Included here, because it's the one thing that makes this more than a wrapper. See below.

5. Deprecate TraceIdRatioBasedSampler now? Yes — annotation only. The spec says implementors "SHALL NOT remove or modify the behavior... until at least January 1, 2027", so behavior is untouched; only the JSDoc changed.

Why this isn't just a one-liner

The spec notes ProbabilitySampler can be implemented as Composite(ComposableProbability(ratio)), and this does build on exactly that. But the composite path does not satisfy one normative requirement:

When a ProbabilitySampler Sampler makes a decision for a non-root Span using TraceID randomness when the Trace random flag was not set, the SDK SHOULD issue a warning statement in its log with a compatibility warning.

So the sampler adds that check. It warns only when all of these hold: the span is non-root, the W3C Trace Context Level 2 random flag is unset, and no explicit rv is present in the OpenTelemetry tracestate.

Two implementation notes worth a reviewer's eye:

It warns at most once per sampler instance. Sampling runs on every span; one warning per span would be unusable. The spec says "a warning", not "a warning per span", but this is a judgement call.
The check short-circuits on the cheap condition first. The random-flag test is a bitwise op, and tracestate is only parsed when the flag is absent — and never again after the warning has fired. So the hot path for an upgraded caller costs one bit test.

TraceFlags in @opentelemetry/api only defines SAMPLED, so the random flag (0x2) is a local constant with a link to the W3C spec rather than an API change.

Testing

18 new tests, all passing; 87 passing in the package overall. probabilitysampler.ts is at 100% statements, branches, functions and lines.

Coverage of the spec's normative requirements:

Requirement	Test
MUST ignore parent `SampledFlag`	unsampled parent still samples at ratio 1; sampled parent still drops at ratio 0
`R >= T` decision	decisions match the equivalent composite sampler across 1000 generated trace IDs
SHOULD write `th:` to tracestate	threshold asserted as `0x80000000000000` at ratio 0.5
SHOULD warn on presumed randomness	warns for non-root; silent for root, for random-flag-set, and for explicit `rv`
—	warns at most once across repeated calls

The equivalence test also asserts the sampled count lands near the expected ratio, so both samplers being broken the same way wouldn't pass silently.

lint and prettier --check are clean for both packages. (sdk-trace reports 6 no-console warnings, but those are pre-existing in benchmark files — identical count on a clean main.)

Not in scope

Wiring ProbabilitySampler into OTEL_TRACES_SAMPLER / declarative config. That's a user-facing configuration surface that seemed worth deciding separately, and the spec's deprecation timeline for TraceIdRatioBased (silent replacement "at that time", January 2027) suggests it wants its own discussion. Glad to follow up if you'd like it here instead.

Note there is some adjacency with #6907, which also touches TraceIdRatioBasedSampler.ts to add factory functions — a different deprecation (direct construction → factory) from this one (sampler → ProbabilitySampler). Whichever lands second will need a trivial JSDoc merge.

AI assistance disclosure

Claude (Anthropic) was used to assist with implementation, tests, and this description, in line with the project's GenAI policy. All spec requirements were verified against the specification text and all tests were run locally.

Adds `createProbabilitySampler(ratio)`, the non-composable form of probability sampling specified in the trace SDK spec, for direct use as an SDK sampler. The spec notes that the top-level `ProbabilitySampler` can be implemented as `Composite(ComposableProbability(ratio))`, and this builds on those existing pieces. It is not purely a wrapper, though: the spec also requires a compatibility warning when a decision is made for a non-root span that presumes trace ID randomness while the W3C Trace Context Level 2 random flag is unset, which the composite path does not emit. The warning fires at most once per sampler instance, since sampling runs on every span. As required, the parent `SampledFlag` is ignored; `ParentBased` remains the way to respect it. Also marks `TraceIdRatioBasedSampler` as `@deprecated`, pointing at the new sampler. Its behavior is untouched: the spec requires that it is neither removed nor modified before January 1, 2027. Refs: open-telemetry#6541 Signed-off-by: nabeelamjadsheikh <131901574+nabeelamjadsheikh@users.noreply.github.com>

opentelemetry-pr-dashboard · 2026-08-24T09:24:19Z

Pull request dashboard status

Waiting on the author · refreshed 2026-09-04 11:58 UTC

Respond to 2 review items (e.g. link a commit, explain why not, ask a follow-up):

Inline threads: 1, 2

Status above doesn't look right?

Just replied or pushed? Anything around or after the refresh time above may not be picked up yet — give it a few minutes.
Should this be with reviewers? Comment /dashboard route:reviewers to route it to them.
Anything wrong — including the routing? Report it with what you expected; it helps us improve the dashboard.

codecov · 2026-08-24T09:24:42Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.02%. Comparing base (4e853d5) to head (fb53884).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7029      +/-   ##
==========================================
+ Coverage   95.00%   95.02%   +0.01%     
==========================================
  Files         407      408       +1     
  Lines       14342    14378      +36     
  Branches     3289     3295       +6     
==========================================
+ Hits        13625    13662      +37     
+ Misses        717      716       -1

Files with missing lines	Coverage Δ
...mental/packages/sampler-composite/src/composite.ts	`93.18% <100.00%> (+0.32%)`	⬆️
...ntal/packages/sampler-composite/src/probability.ts	`100.00% <100.00%> (+4.54%)`	⬆️
...ckages/sampler-composite/src/probabilitysampler.ts	`100.00% <100.00%> (ø)`
.../sdk-trace/src/sampler/TraceIdRatioBasedSampler.ts	`100.00% <ø> (ø)`

🚀 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.

@JacksonWeber

…-56 ratios Addresses review from @JacksonWeber on open-telemetry#7029. `CompositeSampler.shouldSample()` only called `traceState.set('ot', otts)` when the recomputed `otts` string was non-empty. When a span was dropped and had no random value or other tracestate members to write, `otts` serialized to `''` and the call was skipped entirely -- so a threshold inherited from a parent (e.g. `ot=th:8`) passed straight through unmodified, even though this sampler had dropped the span. A later descendant reading that stale `th` could conclude the span was included in the sampled population when it was not. Now the `ot` member is explicitly cleared via `TraceState#unset()` in that case. Also rejects sampling ratios in `(0, 2^-56)`: the spec's minimum representable nonzero sampling ratio is 2^-56, and smaller values were silently rounding down to the same threshold as `ratio=0` without any indication that the requested ratio was unrepresentable. `ratio=0` itself remains valid and continues to behave as `AlwaysOff`. Both fixes verified with a standalone repro before writing tests: the tracestate leak was reproduced with a parent `ot=th:8` and a `createProbabilitySampler(0)` delegate, confirming `result.traceState` still carried `th:8` on a dropped span pre-fix and `undefined` post-fix. Refs: open-telemetry#6541 Signed-off-by: nabeelamjadsheikh <131901574+nabeelamjadsheikh@users.noreply.github.com>

…/Infinity/tracestate preservation Auditing edge cases turned up one real gap: NaN bypassed both range checks (all comparisons against NaN are false), reaching calculateThreshold() and throwing a confusing BigInt conversion error instead of the intended validation message. Also adds explicit regression coverage for cases that were already correct but untested: -0 behaves like 0, +/-Infinity are rejected by the existing range check, and unrelated tracestate members survive the ot-clearing fix in composite.ts. Signed-off-by: nabeelamjadsheikh <131901574+nabeelamjadsheikh@users.noreply.github.com>

…eal TracerProvider The existing compatibility-warning tests hand-build a SpanContext to reach every branch of _warnOnPresumedRandomness() directly. That misses a real constraint: Tracer.startSpan() always rebuilds a new span's own traceFlags down to just SAMPLED/NONE, so a locally created span can never itself carry the W3C random flag -- only a remote parent extracted from real W3C Trace Context headers can. Adds an end-to-end suite that goes through W3CTraceContextPropagator extraction and a real TracerProvider/Tracer.startSpan() for the four cases that matter: local root (no warning), remote parent with the random flag set (no warning), remote parent with an explicit rv (no warning), and an old-style remote parent with neither (warns). Signed-off-by: nabeelamjadsheikh <131901574+nabeelamjadsheikh@users.noreply.github.com>

… local chains, and multi-hop propagation Extends the real-TracerProvider compatibility-warning suite with four more scenarios verified against actual propagator/TracerProvider behavior, not synthetic contexts: - a malformed rv (invalid hex) still falls back to unconfirmed and warns - an rv buried among unrelated tracestate vendors is still found - a local child of a local root re-warns, since the root's trace-ID fallback carries no rv for the child to inherit - a documented, spec-compliant limitation: confirmed randomness from an upstream random flag does not survive a second hop, because Tracer.startSpan() always rebuilds a span's own traceFlags down to SAMPLED/NONE and the spec only permits (does not require) a root sampler to write an explicit rv to compensate. This is captured as a test documenting current behavior, not a claim that it's ideal. Signed-off-by: nabeelamjadsheikh <131901574+nabeelamjadsheikh@users.noreply.github.com>

…e, and ParentBasedSampler composition Three more scenarios verified against real code before being written as tests: - warning state is scoped per ProbabilitySampler instance, not global -- a second, independent sampler instance still warns on its own unconfirmed parent even after a first instance already warned once - Tracer.startSpan()'s `root: true` option strips the active parent before invoking the sampler, so a forced-root span never warns even with an unconfirmed parent active - composing ProbabilitySampler as the `root` of a real ParentBasedSampler (the pattern this package's README recommends for respecting the parent SampledFlag) samples at the correct rate for genuine root spans, and is never invoked at all for a remote parent with an existing sampling decision -- confirming the ParentBasedSampler delegation description'\''s literal claim, not just in comments Signed-off-by: nabeelamjadsheikh <131901574+nabeelamjadsheikh@users.noreply.github.com>

trentm · 2026-08-26T22:14:46Z

+    }
+
+    // An explicit `rv` means the decision does not rest on trace ID randomness.
+    const { randomValue } = parseOtelTraceState(parentSpanContext.traceState);


I suppose this *re-*run of parseOtelTraceState() could be avoided by adding this diag.warn(...) logic directly inside CompositeSampler.shouldSample() inside this else block:

opentelemetry-js/experimental/packages/sampler-composite/src/composite.ts

Lines 60 to 63 in 59dac70

} else {

// Use last 56 bits of trace_id as randomness.

randomness = BigInt(`0x${traceId.slice(-14)}`);

}

But then we'd want to expose an option to CompositeSampler on whether to do this warning logic, which we'd set only when ProbabilitySampler is being used.

So I think what you have here is reasonable. It adds some overhead, but likely fine.
Optimization could be done separately and later if someone demonstrates this is a hot point.

I like this note, also from the spec, at https://opentelemetry.io/docs/specs/otel/trace/tracestate-probability-sampling/#migration-to-consistent-probability-samplers
that implies (to me) that we can eventually consider removing this warning logic:

To avoid inconsistency during this transition, users SHOULD follow this guidance until all Trace SDKs in a system have been upgraded to modern Trace randomness requirements based on W3C Trace Context Level 2.

trentm · 2026-08-26T22:45:29Z

+As the specification requires, this sampler ignores the parent `SampledFlag`. To respect it, use the
+sampler as a delegate of `ParentBasedSampler`, or use `createComposableParentThresholdSampler` as
+shown below.
+


nit: I'd be fine dropping this paragraph. It is more general advice, rather than anything specific to the ProbabilitySampler.

trentm · 2026-08-26T22:50:08Z

+    );
+  });
+
+  it('should decide identically to the equivalent composite sampler', () => {


nit: I'm not sure I get the point of this test. We know that ProbabilitySampler is implemented with CompositeProbabilitySampler, so what is the point of comparing outputs of the two?

trentm · 2026-08-26T22:52:02Z

+      assert.deepStrictEqual(presumed(), []);
+      // The new span's own flags never carry the random bit, confirming it
+      // can only ever be observed by way of an extracted remote parent.
+      assert.strictEqual(span.spanContext().traceFlags & 0x2, 0);


nit: unless I'm missing a reason to use the literal 0x2.

Suggested change

assert.strictEqual(span.spanContext().traceFlags & 0x2, 0);

assert.strictEqual(span.spanContext().traceFlags & TRACE_FLAG_RANDOM, 0);

trentm · 2026-08-26T23:16:37Z

+    it('known limitation: confirmed randomness does not survive a second hop', () => {
+      // Service A receives a remote parent with the random flag confirmed.
+      // Tracer.startSpan() always rebuilds a new span's own traceFlags down to
+      // SAMPLED/NONE (see Tracer.ts), and the spec only permits -- it does not
+      // require -- a *root* sampler to write an explicit `rv` into tracestate
+      // (https://opentelemetry.io/docs/specs/otel/trace/sdk/#probabilitysampler-sampler-configuration).
+      // ProbabilitySampler doesn't do this, so once the flag is gone, a second
+      // hop (service B, receiving A's outgoing header) has no way to confirm
+      // randomness and warns again, even though the trace genuinely started
+      // with confirmed randomness. This test documents that current, spec-
+      // compliant behavior rather than asserting it's ideal.


Thanks a lot for finding this limitation.

So, ideally, we'd update the api and sdk-trace packages to support TraceFlags.RANDOM so that we didn't have this limitation.

- drop the README paragraph on ignoring the parent SampledFlag; it's general ParentBased advice, not specific to ProbabilitySampler, and the same guidance is already in createProbabilitySampler()'s own doc comment - remove 'should decide identically to the equivalent composite sampler' -- ProbabilitySampler is literally implemented by wrapping createCompositeSampler(createComposableProbabilitySampler(ratio)), so the comparison was closer to tautological than a real regression guard; the ratio/threshold behavior it exercised is already covered by the dedicated ratio-validation and threshold tests - use the TRACE_FLAG_RANDOM constant instead of a literal 0x2 in the local-root traceFlags assertion Signed-off-by: nabeelamjadsheikh <131901574+nabeelamjadsheikh@users.noreply.github.com>

nabeelamjadsheikh · 2026-08-27T07:17:18Z

@trentm Thanks! Applied the nits in 808f088: dropped the README paragraph, removed the redundant "identical to composite sampler" test, swapped literal 0x2 for TRACE_FLAG_RANDOM. Agreed on leaving the parseOtelTraceState re-run as-is, and on TraceFlags.RANDOM being the real long-term fix out of scope here, happy to file a follow-up.

… merge The prior 'Update branch' merge (cb5f567) mechanically merged CHANGELOG.md/experimental/CHANGELOG.md without moving this PR's entries into the new 'Unreleased' sections that main's 2.11.0/0.222.0 release cut created, leaving them stuck under the already-released version headers instead. That merge also didn't resolve why every CI job was failing: 'npm ci' rejected package-lock.json because it no longer matched the merged package.json tree (missing @opentelemetry/context-async-hooks). Regenerated via npm install; verified npm ci succeeds and sampler-composite's test suite (113 passing) and lint are unaffected. Signed-off-by: nabeelamjadsheikh <131901574+nabeelamjadsheikh@users.noreply.github.com>

opentelemetry-pr-dashboard · 2026-09-03T01:41:04Z

Hi @nabeelamjadsheikh — just a friendly reminder that this pull request is waiting on you. The dashboard status comment has the open items and is kept current.

Replying is enough to hand it off — answer, explain why no change is needed, or ask a follow-up. The dashboard routes it onward once nothing on the list is waiting on you.
To hand it back for any other reason, including the dashboard getting this wrong, comment /dashboard route:reviewers.

nabeelamjadsheikh · 2026-09-03T14:18:01Z

Hi @trentm @JacksonWeber, just following up on this PR. I’ve addressed the review nits, added the requested regression coverage, and the latest CI checks are passing (30/30).

Please let me know if there are any remaining concerns or changes needed from my side. Thanks!

linux-foundation-easycla · 2026-09-04T11:45:14Z

The committers listed above are authorized under a signed CLA.

✅ login: nabeelamjadsheikh / name: nabeelamjadsheikh (0cc93c1, 3464af6, 53e9cfc, 69cd1fb, 808f088, 829563f, b43fd1f, c4917a6, cb5f567, fb53884)

Resolves modify/delete conflicts on sampler-composite's tsconfig.esm.json/tsconfig.esnext.json, which main removed as part of the tsc -> tsdown build migration (open-telemetry#6293). Adopts main's deletion and relies on the tsdown.config.ts / package.json exports map main already added for this package, matching every other package in the migration. Verified npm ci succeeds and the sampler-composite test suite (113 tests) and lint pass after the merge. Signed-off-by: nabeelamjadsheikh <131901574+nabeelamjadsheikh@users.noreply.github.com>

nabeelamjadsheikh requested a review from a team as a code owner August 24, 2026 09:22

opentelemetry-pr-dashboard Bot mentioned this pull request Aug 24, 2026

Pull Request Dashboard #6856

Open

JacksonWeber reviewed Aug 24, 2026

View reviewed changes

Comment thread experimental/packages/sampler-composite/src/probabilitysampler.ts

JacksonWeber reviewed Aug 24, 2026

View reviewed changes

Comment thread experimental/packages/sampler-composite/src/probabilitysampler.ts

nabeelamjadsheikh added 2 commits August 25, 2026 12:54

JacksonWeber reviewed Aug 25, 2026

View reviewed changes

Comment thread experimental/packages/sampler-composite/test/probabilitysampler.test.ts

nabeelamjadsheikh added 3 commits August 26, 2026 14:16

trentm reviewed Aug 26, 2026

View reviewed changes

nabeelamjadsheikh requested review from JacksonWeber and trentm August 28, 2026 19:17

nabeelamjadsheikh added 2 commits September 1, 2026 11:51

Merge branch 'main' into feat/6541-probability-sampler

cb5f567

nabeelamjadsheikh force-pushed the feat/6541-probability-sampler branch from 1ac356e to fb53884 Compare September 4, 2026 11:48

Navigation Menu

Uh oh!

feat(sampler-composite): implement ProbabilitySampler, deprecate TraceIdRatioBasedSampler - #7029

feat(sampler-composite): implement ProbabilitySampler, deprecate TraceIdRatioBasedSampler#7029
nabeelamjadsheikh wants to merge 10 commits into
open-telemetry:mainfrom
nabeelamjadsheikh:feat/6541-probability-sampler

nabeelamjadsheikh commented Aug 24, 2026

Uh oh!

opentelemetry-pr-dashboard Bot commented Aug 24, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Aug 24, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

trentm Aug 26, 2026

Uh oh!

trentm Aug 26, 2026

Uh oh!

trentm Aug 26, 2026

Uh oh!

trentm Aug 26, 2026

Uh oh!

trentm Aug 26, 2026

Uh oh!

nabeelamjadsheikh commented Aug 27, 2026

Uh oh!

opentelemetry-pr-dashboard Bot commented Sep 3, 2026

Uh oh!

nabeelamjadsheikh commented Sep 3, 2026

Uh oh!

linux-foundation-easycla Bot commented Sep 4, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	} else {
	// Use last 56 bits of trace_id as randomness.
	randomness = BigInt(`0x${traceId.slice(-14)}`);
	}

	assert.strictEqual(span.spanContext().traceFlags & 0x2, 0);
	assert.strictEqual(span.spanContext().traceFlags & TRACE_FLAG_RANDOM, 0);

Uh oh!

Conversation

nabeelamjadsheikh commented Aug 24, 2026

What

Answering the open questions on the issue

Why this isn't just a one-liner

Testing

Not in scope

AI assistance disclosure

Uh oh!

opentelemetry-pr-dashboard Bot commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request dashboard status

Uh oh!

codecov Bot commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

trentm Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

trentm Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

trentm Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

trentm Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

trentm Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

nabeelamjadsheikh commented Aug 27, 2026

Uh oh!

opentelemetry-pr-dashboard Bot commented Sep 3, 2026

Uh oh!

nabeelamjadsheikh commented Sep 3, 2026

Uh oh!

linux-foundation-easycla Bot commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

opentelemetry-pr-dashboard Bot commented Aug 24, 2026 •

edited

Loading

codecov Bot commented Aug 24, 2026 •

edited

Loading

linux-foundation-easycla Bot commented Sep 4, 2026 •

edited

Loading