Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ For notes on migrating to 3.x see [the 3.x migration guide](doc/3.x/migration-gu

### :books: Documentation

* docs(sdk-trace): deprecate `TraceIdRatioBasedSampler` in favour of `ProbabilitySampler` [#6541](https://github.com/open-telemetry/opentelemetry-js/issues/6541) @nabeelamjadsheikh
* Documentation only. The specification requires that the behavior of this sampler is neither removed nor modified before January 1, 2027, so it is unchanged.

### :house: Internal

* feat(ci): support releasing from maintenance branches [#6767](https://github.com/open-telemetry/opentelemetry-js/issues/6767) @pichlermarc
Expand Down
4 changes: 4 additions & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2

### :rocket: Features

* feat(sampler-composite): add `createProbabilitySampler()` [#6541](https://github.com/open-telemetry/opentelemetry-js/issues/6541) @nabeelamjadsheikh
* Implements the specification's `ProbabilitySampler`, the non-composable form of probability sampling, for direct use as an SDK sampler. Unlike the deprecated `TraceIdRatioBasedSampler`, it samples consistently across SDKs by using the randomness features of W3C Trace Context Level 2.
* Also fixes `CompositeSampler` (used by all samplers in this package) to clear an inherited `ot` tracestate value when a span drops and there is nothing new to write, rejects sampling ratios below the minimum representable nonzero value (`2^-56`) instead of silently treating them the same as `0`, and rejects `NaN` with a clear validation error instead of a confusing `BigInt` conversion failure.

### :bug: Bug Fixes

### :books: Documentation
Expand Down
15 changes: 15 additions & 0 deletions experimental/packages/sampler-composite/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,21 @@ These samplers provide the implementation for (the experimental) [Consistent Pro

To get started you will need to install a compatible OpenTelemetry SDK.

### ProbabilitySampler

[`ProbabilitySampler`](https://opentelemetry.io/docs/specs/otel/trace/sdk/#probabilitysampler) is the
non-composable form of probability sampling, for use directly as an SDK sampler. It replaces the
deprecated `TraceIdRatioBasedSampler` from `@opentelemetry/sdk-trace`, whose algorithm was never
specified and therefore samples a different set of traces than other language SDKs given the same
input.

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

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

### Samplers

This module exports samplers that follow the general behavior of the standard SDK samplers, but ensuring
Expand Down
1 change: 1 addition & 0 deletions experimental/packages/sampler-composite/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
},
"devDependencies": {
"@opentelemetry/api": "1.9.1",
"@opentelemetry/context-async-hooks": "2.10.0",
"@types/mocha": "10.0.10",
"@types/node": "18.19.130",
"mocha": "11.8.0",
Expand Down
5 changes: 5 additions & 0 deletions experimental/packages/sampler-composite/src/composite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ class CompositeSampler implements Sampler {
newTraceState = new CoreTraceState();
}
newTraceState = newTraceState.set('ot', otts);
} else if (newTraceState?.get('ot')) {
// Nothing new to write, but an inherited `ot` value (e.g. a parent's
// `th`) would otherwise pass through unchanged. That would misrepresent
// this sampler's decision, so clear it explicitly.
newTraceState = newTraceState.unset('ot');
}

return {
Expand Down
1 change: 1 addition & 0 deletions experimental/packages/sampler-composite/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ export { createComposableParentThresholdSampler } from './parentthreshold';
export { createComposableAnnotatingSampler } from './annotating';
export { createComposableRuleBasedSampler } from './rulebased';
export { createCompositeSampler } from './composite';
export { createProbabilitySampler } from './probabilitysampler';
export type { ComposableSampler, SamplingIntent } from './types';
16 changes: 15 additions & 1 deletion experimental/packages/sampler-composite/src/probability.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,23 @@ class ComposableProbabilitySampler implements ComposableSampler {
private readonly description: string;

constructor(ratio: number) {
if (ratio < 0 || ratio > 1) {
// `NaN` fails every `<`/`>` comparison, so it would otherwise slip past
// both range checks below and only fail later with a confusing BigInt
// conversion error out of `calculateThreshold()`.
if (Number.isNaN(ratio) || ratio < 0 || ratio > 1) {
throw new Error(
`Invalid sampling probability: ${ratio}. Must be between 0 and 1.`
);
}
// `0` is a valid ratio (sample nothing); anything smaller than the
// minimum representable, nonzero ratio would otherwise be silently
// rounded down to the same behavior, which is more likely to be a
// configuration mistake than intentional.
if (ratio > 0 && ratio < MIN_NONZERO_RATIO) {
throw new Error(
`Invalid sampling probability: ${ratio}. Must be 0 or at least ${MIN_NONZERO_RATIO}.`
);
}
const threshold = calculateThreshold(ratio);
const thresholdStr =
threshold === MAX_THRESHOLD ? 'max' : serializeTh(threshold);
Expand Down Expand Up @@ -56,6 +68,8 @@ export function createComposableProbabilitySampler(
}

const probabilityThresholdScale = Math.pow(2, 56);
// https://opentelemetry.io/docs/specs/otel/trace/sdk/#probabilitysampler-sampler-configuration
const MIN_NONZERO_RATIO = Math.pow(2, -56);

// TODO: Reduce threshold precision following spec recommendation of 4
// to reduce size of serialized tracestate.
Expand Down
122 changes: 122 additions & 0 deletions experimental/packages/sampler-composite/src/probabilitysampler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

import type { Context, SpanKind, Attributes, Link } from '@opentelemetry/api';
import { diag, isSpanContextValid, trace } from '@opentelemetry/api';
import type { Sampler, SamplingResult } from '@opentelemetry/sdk-trace';

import { createCompositeSampler } from './composite';
import { createComposableProbabilitySampler } from './probability';
import { parseOtelTraceState } from './tracestate';
import { isValidRandomValue } from './util';

/**
* The `random` flag from W3C Trace Context Level 2. When set, it confirms that
* the rightmost 56 bits of the trace ID are truly random.
*
* This is deliberately not taken from the API's `TraceFlags` enum, which only
* defines `SAMPLED` so far.
*
* https://www.w3.org/TR/trace-context-2/#random-trace-id-flag
*/
const TRACE_FLAG_RANDOM = 0x2;

const COMPATIBILITY_WARNING =
'The ProbabilitySampler sampler is presuming TraceIDs are random and expects ' +
'the Trace random flag to be set in confirmation. Please upgrade your ' +
'caller(s) to use W3C Trace Context Level 2.';

class ProbabilitySampler implements Sampler {
private readonly _delegate: Sampler;
private readonly _description: string;
/**
* The spec asks for a warning, not for one warning per span. Sampling runs on
* every span, so the warning is emitted at most once per sampler instance.
*/
private _warned = false;

constructor(ratio: number) {
this._delegate = createCompositeSampler(
createComposableProbabilitySampler(ratio)
Comment thread
JacksonWeber marked this conversation as resolved.
);
this._description = `ProbabilitySampler{${ratio}}`;
}

shouldSample(
context: Context,
traceId: string,
spanName: string,
spanKind: SpanKind,
attributes: Attributes,
links: Link[]
): SamplingResult {
this._warnOnPresumedRandomness(context);

return this._delegate.shouldSample(
context,
traceId,
spanName,
spanKind,
attributes,
links
);
}

toString(): string {
return this._description;
}

/**
* Warns when a decision is made for a non-root span using trace ID randomness
* while the trace random flag is unset, as the trace ID may have come from an
* SDK that predates the randomness requirement.
*
* https://opentelemetry.io/docs/specs/otel/trace/sdk/#compatibility-warnings-for-probabilitysampler
*/
private _warnOnPresumedRandomness(context: Context): void {
if (this._warned) {
return;
}

const parentSpanContext = trace.getSpanContext(context);
// A root span carries no risk: nothing upstream could have generated the
// trace ID with an older SDK.
if (!parentSpanContext || !isSpanContextValid(parentSpanContext)) {
return;
}

// The flag confirms the trace ID is random, so there is nothing to presume.
if ((parentSpanContext.traceFlags & TRACE_FLAG_RANDOM) !== 0) {
return;
}

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

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

if (isValidRandomValue(randomValue)) {
return;
}

this._warned = true;
diag.warn(COMPATIBILITY_WARNING);
}
}

/**
* Returns a sampler that samples each span with a fixed ratio, consistently
* across a trace, using the randomness features of
* {@link https://www.w3.org/TR/trace-context-2/ | W3C Trace Context Level 2}.
*
* The parent `SampledFlag` is ignored; wrap this sampler in a `ParentBased`
* sampler to respect it.
*
* This is the non-composable form of probability sampling, for direct use as an
* SDK sampler. Use {@link createComposableProbabilitySampler} to compose it with
* other samplers instead.
*
* @param ratio the fraction of traces to sample, between 0 and 1 inclusive.
*/
export function createProbabilitySampler(ratio: number): Sampler {
Comment thread
JacksonWeber marked this conversation as resolved.
return new ProbabilitySampler(ratio);
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ describe('ComposableProbabilitySampler', () => {
{ ratio: 1.0, thresholdStr: '0' },
{ ratio: 0.5, thresholdStr: '8' },
{ ratio: 0.25, thresholdStr: 'c' },
{ ratio: 1e-300, thresholdStr: 'max' },
{ ratio: 0, thresholdStr: 'max' },
].forEach(({ ratio, thresholdStr }) => {
it(`should have a description for ratio ${ratio}`, () => {
Expand Down Expand Up @@ -73,4 +72,45 @@ describe('ComposableProbabilitySampler', () => {
);
});
});

describe('ratio validation', () => {
[-1, -0.0001, 1.0001, 2, NaN, Infinity, -Infinity].forEach(ratio => {
it(`should reject the out-of-range ratio ${ratio}`, () => {
assert.throws(
() => createComposableProbabilitySampler(ratio),
/Invalid sampling probability/
);
});
});

// The spec's minimum valid nonzero sampling ratio is 2^-56; anything
// smaller cannot be represented and would silently collapse to the same
// behavior as ratio 0.
[Math.pow(2, -57), Math.pow(2, -100), 1e-300].forEach(ratio => {
it(`should reject the unrepresentable nonzero ratio ${ratio}`, () => {
assert.throws(
() => createComposableProbabilitySampler(ratio),
/Invalid sampling probability/
);
});
});

it('should still allow ratio 0', () => {
assert.doesNotThrow(() => createComposableProbabilitySampler(0));
});

it('should still allow the minimum representable nonzero ratio', () => {
assert.doesNotThrow(() =>
createComposableProbabilitySampler(Math.pow(2, -56))
);
});

it('should treat -0 the same as 0', () => {
const sampler = createComposableProbabilitySampler(-0);
assert.strictEqual(
sampler.toString(),
'ComposableProbabilitySampler(threshold=max, ratio=0)'
);
});
});
});
Loading