Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
33485d3
feat(trace): add experimental implementation of draft consistent samp…
anuraaga Aug 8, 2025
0a967ac
Update changelog
anuraaga Aug 8, 2025
1fbc1dc
git add
anuraaga Aug 8, 2025
0f1d33e
Drift
anuraaga Aug 8, 2025
b41131b
Fix folder name
anuraaga Aug 8, 2025
38ecc13
Fix
anuraaga Aug 8, 2025
acc3070
Cleanup
anuraaga Aug 14, 2025
63c6c06
Rename package
anuraaga Aug 14, 2025
3119308
README
anuraaga Aug 14, 2025
a6df902
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
anuraaga Aug 14, 2025
2b5e7c5
Cleanup
anuraaga Aug 21, 2025
7ced0a5
Factory functions
anuraaga Aug 21, 2025
7f7afdc
Doc
anuraaga Aug 21, 2025
47c3fd4
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
anuraaga Aug 21, 2025
61656b5
Hard refresh lock
anuraaga Aug 21, 2025
861ac86
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
anuraaga Aug 28, 2025
6592630
Cleanup
anuraaga Aug 28, 2025
98e5cd8
Update CHANGELOG.md
anuraaga Aug 29, 2025
b7a34e6
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
anuraaga Sep 4, 2025
fd5c24a
Merge branch 'consistent-sampling' of https://github.com/anuraaga/ope…
anuraaga Sep 4, 2025
af4bc3e
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
anuraaga Sep 11, 2025
00a00f9
Force merge package lock
anuraaga Sep 11, 2025
02c8bce
Add precision TODO
anuraaga Sep 18, 2025
04809a3
Finish
anuraaga Sep 19, 2025
ffc9bc4
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
anuraaga Sep 19, 2025
21ad9dc
Update experimental/CHANGELOG.md
trentm Sep 19, 2025
535292e
Merge branch 'main' into consistent-sampling
trentm Sep 19, 2025
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
2 changes: 1 addition & 1 deletion CHANGELOG.md
Copy link
Contributor

Choose a reason for hiding this comment

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

[This is a general question. I'm commenting here for lack of a good place to mention it.]

My head is spinning from the inconsistent (har har) use of Composite or Composable or Consistent usage in class and interface names in the spec, OTEP, and implementations. And also, a little less so, in some class name changes, e.g. ConsistentFixedThreshold in the OTEP vs. ComposableTraceIDRatioBased in the spec. I haven't read through this PR yet, but I'm guessing other API diffs between OTEP and spec will show up, e.g. threshold_reliable in the spec vs. IsAdjustedCountReliable in the OTEP.

While I don't know that the names used in the spec need to be the same names as in the implementation APIs, approaching the same names is likely desired. Are we implementing the spec here, or the Java consistent56 implementation, which seems to follow the OTEP naming (which makes sense as it was probably developed before the spec renamings)?

I'm happy to go asking about this in the OTel channels.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks - I will read the spec and align with it as much as possible, sorry for missing it

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious what Java (and the Rust/Go implementations that are linked to from open-telemetry/opentelemetry-specification#4466) will do. I.e. will they change their implementations to use the class names in the spec? Or change the spec to use Consistent-prefix?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at the rust and go implementations (esp. the go implementation README: https://github.com/jmacd/go-sampler) I'm pretty certain CompositeSampler and Composable*Sampler naming is the intent.

Copy link
Contributor

Choose a reason for hiding this comment

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

I asked for clarification in #otel-sampling: https://cloud-native.slack.com/archives/C027DS6GZD3/p1755299016130699

Copy link
Contributor

Choose a reason for hiding this comment

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

Issue/answer on the naming: open-telemetry/opentelemetry-specification#4640

Basically: "So the short answer is that the Specification names should be used.", but "do not strongly believe that all SDKs need the same exact names".
So, whatever names are used by a particular SDK is fine.

(Note that I would expect the OTel schema for its configuration data model would use the spec names for these samplers, when these new samplers are added to the schema. At that point there is some value to users and maintainers if the configuration names match the implementation names.)

Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2
## Unreleased

* feat(instrumentation-http): Added support for redacting specific url query string values and url credentials in instrumentations [#5743](https://github.com/open-telemetry/opentelemetry-js/pull/5743) @rads-1996
* feat(sampler-consistent): Added experimental implementations of draft consistent sampling spec [#5839](https://github.com/open-telemetry/opentelemetry-js/pull/5839) @anuraaga
* feat(sampler-composite): Added experimental implementations of draft composite sampling spec [#5839](https://github.com/open-telemetry/opentelemetry-js/pull/5839) @anuraaga

### :boom: Breaking Changes

Expand Down
18 changes: 9 additions & 9 deletions experimental/packages/sampler-composite/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,20 @@ in exported spans to reconstruct population metrics.

```typescript
import {
CompositeSampler,
ComposableAlwaysOffSampler,
ComposableAlwaysOnSampler,
ComposableParentThresholdSampler,
ComposableTraceIDRatioBasedSampler,
createCompositeSampler,
createComposableAlwaysOffSampler,
createComposableAlwaysOnSampler,
createComposableParentThresholdSampler,
createComposableTraceIDRatioBasedSampler,
} from '@opentelemetry/sampler-composite';

// never sample
const sampler = new CompositeSampler(new ComposableAlwaysOffSampler());
const sampler = createCompositeSampler(createComposableAlwaysOffSampler());
// always sample
const sampler = new CompositeSampler(new ComposableAlwaysOnSampler());
const sampler = createCompositeSampler(createComposableAlwaysOnSampler());
// follow the parent, or otherwise sample with a probability if root
const sampler = new CompositeSampler(
new ComposableParentThresholdSampler(new ComposableTraceIDRatioBasedSampler(0.3)));
const sampler = createCompositeSampler(
createComposableParentThresholdSampler(createComposableTraceIDRatioBasedSampler(0.3)));
```

## Useful links
Expand Down
4 changes: 1 addition & 3 deletions experimental/packages/sampler-composite/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"lint:fix": "eslint . --ext .ts --fix",
"version": "node ../../../scripts/version-update.js",
"watch": "tsc --build --watch",
"precompile": "cross-var lerna run version --scope $npm_package_name --include-dependencies",
"precompile": "lerna run version --scope @opentelemetry/sampler-composite --include-dependencies",
"prewatch": "npm run precompile",
"peer-api-check": "node ../../../scripts/peer-api-check.js",
"align-api-deps": "node ../../../scripts/align-api-deps.js"
Expand Down Expand Up @@ -57,10 +57,8 @@
"@opentelemetry/api": "1.9.0",
"@types/mocha": "10.0.10",
"@types/node": "18.6.5",
"cross-var": "1.1.0",
"lerna": "6.6.2",
"mocha": "11.1.0",
"nock": "13.5.6",
"nyc": "17.1.0"
},
"dependencies": {
Expand Down
2 changes: 1 addition & 1 deletion experimental/packages/sampler-composite/src/alwaysoff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,6 @@ const _sampler = new ComposableAlwaysOffSampler();
/**
* Returns a composable sampler that does not sample any span.
*/
export function composable_always_off_sampler(): ComposableSampler {
export function createComposableAlwaysOffSampler(): ComposableSampler {
return _sampler;
}
2 changes: 1 addition & 1 deletion experimental/packages/sampler-composite/src/alwayson.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,6 @@ const _sampler = new ComposableAlwaysOnSampler();
/**
* Returns a composable sampler that samples all span.
*/
export function composable_always_on_sampler(): ComposableSampler {
export function createComposableAlwaysOnSampler(): ComposableSampler {
return _sampler;
}
18 changes: 14 additions & 4 deletions experimental/packages/sampler-composite/src/composite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class CompositeSampler implements Sampler {
const spanContext = trace.getSpanContext(context);

const traceState = spanContext?.traceState;
const otTraceState = parseOtelTraceState(traceState);
let otTraceState = parseOtelTraceState(traceState);

const intent = this.delegate.getSamplingIntent(
context,
Expand Down Expand Up @@ -78,9 +78,15 @@ class CompositeSampler implements Sampler {
? SamplingDecision.RECORD_AND_SAMPLED
: SamplingDecision.NOT_RECORD;
if (sampled && adjustedCountCorrect) {
otTraceState.threshold = intent.threshold;
otTraceState = {
...otTraceState,
threshold: intent.threshold,
};
} else {
otTraceState.threshold = INVALID_THRESHOLD;
otTraceState = {
...otTraceState,
threshold: INVALID_THRESHOLD,
};
}

const otts = serializeTraceState(otTraceState);
Expand All @@ -105,12 +111,16 @@ class CompositeSampler implements Sampler {
traceState: newTraceState,
};
}

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

/**
* Returns a composite sampler that uses a composable sampler to make its
* sampling decisions while handling tracestate.
*/
export function composite_sampler(delegate: ComposableSampler): Sampler {
export function createCompositeSampler(delegate: ComposableSampler): Sampler {
return new CompositeSampler(delegate);
}
10 changes: 5 additions & 5 deletions experimental/packages/sampler-composite/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
* limitations under the License.
*/

export { composable_always_off_sampler } from './alwaysoff';
export { composable_always_on_sampler } from './alwayson';
export { composable_trace_id_ratio_based_sampler } from './traceidratio';
export { composable_parent_threshold_sampler } from './parentthreshold';
export { composite_sampler } from './composite';
export { createComposableAlwaysOffSampler } from './alwaysoff';
export { createComposableAlwaysOnSampler } from './alwayson';
export { createComposableTraceIDRatioBasedSampler } from './traceidratio';
export { createComposableParentThresholdSampler } from './parentthreshold';
export { createCompositeSampler } from './composite';
export type { ComposableSampler, SamplingIntent } from './types';
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class ComposableParentThresholdSampler implements ComposableSampler {
private readonly description: string;

constructor(private readonly rootSampler: ComposableSampler) {
this.description = `ComposableParentThresholdSampler(root_sampler=${rootSampler})`;
this.description = `ComposableParentThresholdSampler(rootSampler=${rootSampler})`;
}

getSamplingIntent(
Expand Down Expand Up @@ -82,7 +82,7 @@ class ComposableParentThresholdSampler implements ComposableSampler {
* Returns a composable sampler that respects the sampling decision of the
* parent span or falls back to the given sampler if it is a root span.
*/
export function composable_parent_threshold_sampler(
export function createComposableParentThresholdSampler(
rootSampler: ComposableSampler
): ComposableSampler {
return new ComposableParentThresholdSampler(rootSampler);
Expand Down
6 changes: 4 additions & 2 deletions experimental/packages/sampler-composite/src/traceidratio.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ class ComposableTraceIDRatioBasedSampler implements ComposableSampler {
thresholdReliable: true,
};
} else {
// Same as AlwaysOff, notably the threshold is not considered reliable
// Same as AlwaysOff, notably the threshold is not considered reliable. The spec mentions
// returning an instance of ComposableAlwaysOffSampler in this case but it seems clearer
// if the description of the sampler matches the user's request.
this.intent = {
threshold: INVALID_THRESHOLD,
thresholdReliable: false,
Expand All @@ -58,7 +60,7 @@ class ComposableTraceIDRatioBasedSampler implements ComposableSampler {
/**
* Returns a composable sampler that samples each span with a fixed ratio.
*/
export function composable_trace_id_ratio_based_sampler(
export function createComposableTraceIDRatioBasedSampler(
ratio: number
): ComposableSampler {
return new ComposableTraceIDRatioBasedSampler(ratio);
Expand Down
19 changes: 7 additions & 12 deletions experimental/packages/sampler-composite/src/tracestate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,10 @@ export type OtelTraceState = {
rest?: string[];
};

export function invalidTraceState(): OtelTraceState {
return {
randomValue: INVALID_RANDOM_VALUE,
threshold: INVALID_THRESHOLD,
};
}
export const INVALID_TRACE_STATE: OtelTraceState = {
randomValue: INVALID_RANDOM_VALUE,
threshold: INVALID_THRESHOLD,
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export const INVALID_TRACE_STATE: OtelTraceState = {
randomValue: INVALID_RANDOM_VALUE,
threshold: INVALID_THRESHOLD,
};
export const INVALID_TRACE_STATE: OtelTraceState = Object.freeze({
randomValue: INVALID_RANDOM_VALUE,
threshold: INVALID_THRESHOLD,
});

IIUC, using Object.freeze here should help prevent accidental mutation of this re-used object.

There is one current usage in opentelemetry-js.git:

const DEFAULT_AGGREGATION = Object.freeze({
type: AggregationType.DEFAULT,
});

I only went looking because I don't have a lot of personal experience using Object.freeze.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, makes sense!


const TRACE_STATE_SIZE_LIMIT = 256;
const MAX_VALUE_LENGTH = 14; // 56 bits, 4 bits per hex digit
Expand All @@ -47,12 +45,13 @@ export function parseOtelTraceState(
): OtelTraceState {
const ot = traceState?.get('ot');
if (!ot || ot.length > TRACE_STATE_SIZE_LIMIT) {
return invalidTraceState();
return INVALID_TRACE_STATE;
}

let threshold = INVALID_THRESHOLD;
let randomValue = INVALID_RANDOM_VALUE;

// Parse based on https://opentelemetry.io/docs/specs/otel/trace/tracestate-handling/
const members = ot.split(';');
let rest: string[] | undefined;
for (const member of members) {
Expand Down Expand Up @@ -115,15 +114,11 @@ function parseTh(value: string, defaultValue: bigint): bigint {
return defaultValue;
}

let parsed: bigint;
try {
parsed = BigInt(`0x${value}`);
return BigInt('0x' + value.padEnd(MAX_VALUE_LENGTH, '0'));
} catch {
return defaultValue;
}

const trailingZeros = MAX_VALUE_LENGTH - value.length;
return parsed << BigInt(trailingZeros * 4);
}

function parseRv(value: string, defaultValue: bigint): bigint {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@ import * as assert from 'assert';
import { context, SpanKind } from '@opentelemetry/api';
import { SamplingDecision } from '@opentelemetry/sdk-trace-base';

import { composite_sampler, composable_always_off_sampler } from '../src';
import {
createCompositeSampler,
createComposableAlwaysOffSampler,
} from '../src';
import { traceIdGenerator } from './util';

describe('ComposableAlwaysOffSampler', () => {
const composableSampler = composable_always_off_sampler();
const composableSampler = createComposableAlwaysOffSampler();

it('should have a description', () => {
assert.strictEqual(
Expand All @@ -47,7 +50,7 @@ describe('ComposableAlwaysOffSampler', () => {
});

it('should never sample', () => {
const sampler = composite_sampler(composableSampler);
const sampler = createCompositeSampler(composableSampler);
const generator = traceIdGenerator();
let numSampled = 0;
for (let i = 0; i < 10000; i++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@ import * as assert from 'assert';
import { context, SpanKind } from '@opentelemetry/api';
import { SamplingDecision } from '@opentelemetry/sdk-trace-base';

import { composite_sampler, composable_always_on_sampler } from '../src';
import {
createCompositeSampler,
createComposableAlwaysOnSampler,
} from '../src';
import { traceIdGenerator } from './util';

describe('ComposableAlwaysOnSampler', () => {
const composableSampler = composable_always_on_sampler();
const composableSampler = createComposableAlwaysOnSampler();

it('should have a description', () => {
assert.strictEqual(
Expand All @@ -47,7 +50,7 @@ describe('ComposableAlwaysOnSampler', () => {
});

it('should always sample', () => {
const sampler = composite_sampler(composableSampler);
const sampler = createCompositeSampler(composableSampler);
const generator = traceIdGenerator();
let numSampled = 0;
for (let i = 0; i < 10000; i++) {
Expand Down
Loading
Loading