Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support TraceState in SamplingResult #3530

Merged
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
177931d
feat(sampling): add traceState field to SamplingResult
raphael-theriault-swi Jan 11, 2023
c709487
feat(tracer): use SamplingResult's traceState
raphael-theriault-swi Jan 11, 2023
af52040
feat(sampling): forward traceState in builtin samplers
raphael-theriault-swi Jan 11, 2023
467b282
test(sampling): update existing sampler tests with traceState
raphael-theriault-swi Jan 12, 2023
e008229
test(sampling): builtin samplers should forward traceState
raphael-theriault-swi Jan 12, 2023
a886d08
test(tracer): spans should use traceState from sampling result
raphael-theriault-swi Jan 12, 2023
861adce
chore: update changelog
raphael-theriault-swi Jan 12, 2023
11e8265
Merge branch 'main' into samplingresult-tracestate
dyladan Jan 13, 2023
c4f174b
fix(opentelemetry-core): traceState in sampler tests
raphael-theriault-swi Jan 13, 2023
5527f6d
fix(sampling): better specify the behaviour of traceState in Sampling…
raphael-theriault-swi Jan 13, 2023
ac2bc02
fix(sampling-result): clarify where the tracestate should originate from
raphael-theriault-swi Jan 18, 2023
3aae7ee
Merge branch 'main' into samplingresult-tracestate
Flarna Jan 30, 2023
b45c670
Revert "feat(sampling): forward traceState in builtin samplers"
raphael-theriault-swi Feb 2, 2023
3099265
Revert "test(sampling): builtin samplers should forward traceState"
raphael-theriault-swi Feb 2, 2023
b51fdea
Revert "test(sampling): update existing sampler tests with traceState"
raphael-theriault-swi Feb 2, 2023
a27dbac
Revert "fix(opentelemetry-core): traceState in sampler tests"
raphael-theriault-swi Feb 2, 2023
6573804
merge main into samplingresult-tracestate
raphael-theriault-swi Feb 2, 2023
c5526fa
Merge branch 'main' into samplingresult-tracestate
legendecas Feb 9, 2023
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: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/

### :rocket: (Enhancement)

* feat: support TraceState in SamplingResult [#3530](https://github.com/open-telemetry/opentelemetry-js/pull/3530) @raphael-theriault-swi

### :bug: (Bug Fix)

### :books: (Refine Doc)
Expand Down
8 changes: 8 additions & 0 deletions api/src/trace/SamplingResult.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

import { SpanAttributes } from './attributes';
import { TraceState } from './trace_state';

/**
* @deprecated use the one declared in @opentelemetry/sdk-trace-base instead.
Expand Down Expand Up @@ -55,4 +56,11 @@ export interface SamplingResult {
* can safely cache the returned value.
*/
attributes?: Readonly<SpanAttributes>;
/**
* A {@link TraceState} that will be associated with the {@link Span} through
* the new {@link SpanContext}. Samplers SHOULD return the TraceState from
* the passed-in {@link Context} if they do not intend to change it. Leaving
* the value undefined will also leave the TraceState unchanged.
*/
traceState?: TraceState;
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,23 @@
* limitations under the License.
*/

import { Sampler, SamplingDecision, SamplingResult } from '@opentelemetry/api';
import {
Context,
Sampler,
SamplingDecision,
SamplingResult,
trace,
} from '@opentelemetry/api';

/**
* @deprecated Use the one defined in @opentelemetry/sdk-trace-base instead.
* Sampler that samples no traces.
*/
export class AlwaysOffSampler implements Sampler {
shouldSample(): SamplingResult {
shouldSample(context: Context): SamplingResult {
return {
decision: SamplingDecision.NOT_RECORD,
traceState: trace.getSpanContext(context)?.traceState,
};
}

Expand Down
11 changes: 9 additions & 2 deletions packages/opentelemetry-core/src/trace/sampler/AlwaysOnSampler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,23 @@
* limitations under the License.
*/

import { Sampler, SamplingDecision, SamplingResult } from '@opentelemetry/api';
import {
Context,
Sampler,
SamplingDecision,
SamplingResult,
trace,
} from '@opentelemetry/api';

/**
* @deprecated Use the one defined in @opentelemetry/sdk-trace-base instead.
* Sampler that samples all traces.
*/
export class AlwaysOnSampler implements Sampler {
shouldSample(): SamplingResult {
shouldSample(context: Context): SamplingResult {
return {
decision: SamplingDecision.RECORD_AND_SAMPLED,
traceState: trace.getSpanContext(context)?.traceState,
legendecas marked this conversation as resolved.
Show resolved Hide resolved
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import {
SamplingDecision,
SamplingResult,
isValidTraceId,
Context,
trace,
} from '@opentelemetry/api';

/**
Expand All @@ -33,12 +35,13 @@ export class TraceIdRatioBasedSampler implements Sampler {
this._upperBound = Math.floor(this._ratio * 0xffffffff);
}

shouldSample(context: unknown, traceId: string): SamplingResult {
shouldSample(context: Context, traceId: string): SamplingResult {
return {
decision:
isValidTraceId(traceId) && this._accumulate(traceId) < this._upperBound
? SamplingDecision.RECORD_AND_SAMPLED
: SamplingDecision.NOT_RECORD,
traceState: trace.getSpanContext(context)?.traceState,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ describe('AlwaysOffSampler', () => {

it('should return decision: api.SamplingDecision.NOT_RECORD for AlwaysOffSampler', () => {
const sampler = new AlwaysOffSampler();
assert.deepStrictEqual(sampler.shouldSample(), {
assert.deepStrictEqual(sampler.shouldSample(api.ROOT_CONTEXT), {
decision: api.SamplingDecision.NOT_RECORD,
traceState: undefined,
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ describe('AlwaysOnSampler', () => {

it('should return api.SamplingDecision.RECORD_AND_SAMPLED for AlwaysOnSampler', () => {
const sampler = new AlwaysOnSampler();
assert.deepStrictEqual(sampler.shouldSample(), {
assert.deepStrictEqual(sampler.shouldSample(api.ROOT_CONTEXT), {
decision: api.SamplingDecision.RECORD_AND_SAMPLED,
traceState: undefined,
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ describe('ParentBasedSampler', () => {
),
{
decision: api.SamplingDecision.NOT_RECORD,
traceState: undefined,
}
);
});
Expand All @@ -85,6 +86,7 @@ describe('ParentBasedSampler', () => {
),
{
decision: api.SamplingDecision.RECORD_AND_SAMPLED,
traceState: undefined,
}
);
});
Expand All @@ -103,6 +105,7 @@ describe('ParentBasedSampler', () => {
),
{
decision: api.SamplingDecision.RECORD_AND_SAMPLED,
traceState: undefined,
}
);
});
Expand All @@ -126,6 +129,7 @@ describe('ParentBasedSampler', () => {
),
{
decision: api.SamplingDecision.RECORD_AND_SAMPLED,
traceState: undefined,
}
);
});
Expand All @@ -144,6 +148,7 @@ describe('ParentBasedSampler', () => {
),
{
decision: api.SamplingDecision.NOT_RECORD,
traceState: undefined,
}
);
});
Expand All @@ -162,6 +167,7 @@ describe('ParentBasedSampler', () => {
),
{
decision: api.SamplingDecision.NOT_RECORD,
traceState: undefined,
}
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@ import * as assert from 'assert';
import * as api from '@opentelemetry/api';
import { TraceIdRatioBasedSampler } from '../../src/trace/sampler/TraceIdRatioBasedSampler';

const spanContext = (traceId = '1') => ({
traceId,
spanId: '1.1',
traceFlags: api.TraceFlags.NONE,
});
const spanContext = (traceId = '1') =>
api.trace.setSpanContext(api.ROOT_CONTEXT, {
traceId,
spanId: '1.1',
traceFlags: api.TraceFlags.NONE,
});

const traceId = (part: string) => ('0'.repeat(32) + part).slice(-32);

Expand Down Expand Up @@ -59,6 +60,7 @@ describe('TraceIdRatioBasedSampler', () => {
sampler.shouldSample(spanContext(traceId('1')), traceId('1')),
{
decision: api.SamplingDecision.RECORD_AND_SAMPLED,
traceState: undefined,
}
);
});
Expand All @@ -69,6 +71,7 @@ describe('TraceIdRatioBasedSampler', () => {
sampler.shouldSample(spanContext(traceId('1')), traceId('1')),
{
decision: api.SamplingDecision.RECORD_AND_SAMPLED,
traceState: undefined,
}
);
});
Expand All @@ -79,6 +82,7 @@ describe('TraceIdRatioBasedSampler', () => {
sampler.shouldSample(spanContext(traceId('1')), traceId('1')),
{
decision: api.SamplingDecision.NOT_RECORD,
traceState: undefined,
}
);
});
Expand All @@ -89,6 +93,7 @@ describe('TraceIdRatioBasedSampler', () => {
sampler.shouldSample(spanContext(traceId('1')), traceId('1')),
{
decision: api.SamplingDecision.NOT_RECORD,
traceState: undefined,
}
);
});
Expand All @@ -100,6 +105,7 @@ describe('TraceIdRatioBasedSampler', () => {
sampler.shouldSample(spanContext(traceId('1')), traceId('1')),
{
decision: api.SamplingDecision.NOT_RECORD,
traceState: undefined,
}
);
});
Expand All @@ -111,6 +117,7 @@ describe('TraceIdRatioBasedSampler', () => {
sampler.shouldSample(spanContext(traceId('1')), traceId('1')),
{
decision: api.SamplingDecision.NOT_RECORD,
traceState: undefined,
}
);
});
Expand All @@ -122,6 +129,7 @@ describe('TraceIdRatioBasedSampler', () => {
sampler.shouldSample(spanContext(traceId('1')), traceId('1')),
{
decision: api.SamplingDecision.NOT_RECORD,
traceState: undefined,
}
);
});
Expand All @@ -132,6 +140,7 @@ describe('TraceIdRatioBasedSampler', () => {
sampler.shouldSample(spanContext(traceId('1')), traceId('1')),
{
decision: api.SamplingDecision.RECORD_AND_SAMPLED,
traceState: undefined,
}
);

Expand All @@ -142,6 +151,7 @@ describe('TraceIdRatioBasedSampler', () => {
),
{
decision: api.SamplingDecision.NOT_RECORD,
traceState: undefined,
}
);
});
Expand All @@ -150,12 +160,14 @@ describe('TraceIdRatioBasedSampler', () => {
const sampler = new TraceIdRatioBasedSampler(1);
assert.deepStrictEqual(sampler.shouldSample(spanContext(''), ''), {
decision: api.SamplingDecision.NOT_RECORD,
traceState: undefined,
});

assert.deepStrictEqual(
sampler.shouldSample(spanContext(traceId('g')), traceId('g')),
{
decision: api.SamplingDecision.NOT_RECORD,
traceState: undefined,
}
);
});
Expand All @@ -167,30 +179,36 @@ describe('TraceIdRatioBasedSampler', () => {
const id1 = traceId((Math.floor(0xffffffff * 0.1) - 1).toString(16));
assert.deepStrictEqual(sampler10.shouldSample(spanContext(id1), id1), {
decision: api.SamplingDecision.RECORD_AND_SAMPLED,
traceState: undefined,
});
assert.deepStrictEqual(sampler20.shouldSample(spanContext(id1), id1), {
decision: api.SamplingDecision.RECORD_AND_SAMPLED,
traceState: undefined,
});

const id2 = traceId((Math.floor(0xffffffff * 0.2) - 1).toString(16));
assert.deepStrictEqual(sampler10.shouldSample(spanContext(id2), id2), {
decision: api.SamplingDecision.NOT_RECORD,
traceState: undefined,
});
assert.deepStrictEqual(sampler20.shouldSample(spanContext(id2), id2), {
decision: api.SamplingDecision.RECORD_AND_SAMPLED,
traceState: undefined,
});

const id2delta = traceId(Math.floor(0xffffffff * 0.2).toString(16));
assert.deepStrictEqual(
sampler10.shouldSample(spanContext(id2delta), id2delta),
{
decision: api.SamplingDecision.NOT_RECORD,
traceState: undefined,
}
);
assert.deepStrictEqual(
sampler20.shouldSample(spanContext(id2delta), id2delta),
{
decision: api.SamplingDecision.NOT_RECORD,
traceState: undefined,
}
);
});
Expand Down
15 changes: 14 additions & 1 deletion packages/opentelemetry-sdk-trace-base/src/Sampler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,13 @@
* limitations under the License.
*/

import { Context, Link, SpanAttributes, SpanKind } from '@opentelemetry/api';
import {
Context,
Link,
SpanAttributes,
SpanKind,
TraceState,
} from '@opentelemetry/api';

/**
* A sampling decision that determines how a {@link Span} will be recorded
Expand Down Expand Up @@ -53,6 +59,13 @@ export interface SamplingResult {
* can safely cache the returned value.
*/
attributes?: Readonly<SpanAttributes>;
/**
* A {@link TraceState} that will be associated with the {@link Span} through
* the new {@link SpanContext}. Samplers SHOULD return the TraceState from
* the passed-in {@link Context} if they do not intend to change it. Leaving
* the value undefined will also leave the TraceState unchanged.
*/
traceState?: TraceState;
}

/**
Expand Down
2 changes: 2 additions & 0 deletions packages/opentelemetry-sdk-trace-base/src/Tracer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ export class Tracer implements api.Tracer {
links
);

traceState = samplingResult.traceState ?? traceState;

const traceFlags =
samplingResult.decision === api.SamplingDecision.RECORD_AND_SAMPLED
? api.TraceFlags.SAMPLED
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@
* limitations under the License.
*/

import { Context, trace } from '@opentelemetry/api';
import { Sampler, SamplingDecision, SamplingResult } from '../Sampler';

/** Sampler that samples no traces. */
export class AlwaysOffSampler implements Sampler {
shouldSample(): SamplingResult {
shouldSample(context: Context): SamplingResult {
return {
decision: SamplingDecision.NOT_RECORD,
traceState: trace.getSpanContext(context)?.traceState,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@
* limitations under the License.
*/

import { Context, trace } from '@opentelemetry/api';
import { Sampler, SamplingDecision, SamplingResult } from '../Sampler';

/** Sampler that samples all traces. */
export class AlwaysOnSampler implements Sampler {
shouldSample(): SamplingResult {
shouldSample(context: Context): SamplingResult {
return {
decision: SamplingDecision.RECORD_AND_SAMPLED,
traceState: trace.getSpanContext(context)?.traceState,
};
}

Expand Down
Loading