Skip to content

Commit 5e627d9

Browse files
committed
fix(node): UnifygetDynamicSamplingContextFromSpan
We used to have a different implementation, that worked slightly different. This had two problems: 1. We did not look up the root span in the OTEL implementation, but relied on already passing in the root span (in core, we convert it to root span) 2. We did not use frozen DSC properly Now, the core implementation just works for both OTEL and core spans.
1 parent 1b6c22e commit 5e627d9

File tree

9 files changed

+58
-94
lines changed

9 files changed

+58
-94
lines changed

packages/core/src/tracing/dynamicSamplingContext.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { Client, DynamicSamplingContext, Span } from '@sentry/types';
22
import {
33
addNonEnumerableProperty,
4+
baggageHeaderToDynamicSamplingContext,
45
dropUndefinedKeys,
56
dynamicSamplingContextToSentryBaggageHeader,
67
} from '@sentry/utils';
@@ -66,15 +67,25 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly<Partial<
6667
const dsc = getDynamicSamplingContextFromClient(spanToJSON(span).trace_id || '', client);
6768

6869
const rootSpan = getRootSpan(span);
69-
if (!rootSpan) {
70-
return dsc;
71-
}
7270

71+
// For core implementation, we freeze the DSC onto the span as a non-enumerable property
7372
const frozenDsc = (rootSpan as SpanWithMaybeDsc)[FROZEN_DSC_FIELD];
7473
if (frozenDsc) {
7574
return frozenDsc;
7675
}
7776

77+
// For OpenTelemetry, we freeze the DSC on the trace state
78+
const traceState = rootSpan.spanContext().traceState;
79+
const traceStateDsc = traceState && traceState.get('sentry.dsc');
80+
81+
// If the span has a DSC, we want it to take precedence
82+
const dscOnTraceState = traceStateDsc && baggageHeaderToDynamicSamplingContext(traceStateDsc);
83+
84+
if (dscOnTraceState) {
85+
return dscOnTraceState;
86+
}
87+
88+
// Else, we generate it from the span
7889
const jsonSpan = spanToJSON(rootSpan);
7990
const attributes = jsonSpan.data || {};
8091
const maybeSampleRate = attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE];
@@ -87,8 +98,9 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly<Partial<
8798
const source = attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE];
8899

89100
// after JSON conversion, txn.name becomes jsonSpan.description
90-
if (source && source !== 'url') {
91-
dsc.transaction = jsonSpan.description;
101+
const name = jsonSpan.description;
102+
if (source !== 'url' && name) {
103+
dsc.transaction = name;
92104
}
93105

94106
dsc.sampled = String(spanIsSampled(rootSpan));

packages/core/test/lib/tracing/dynamicSamplingContext.test.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { TransactionSource } from '@sentry/types';
1+
import type { Span, SpanContextData, TransactionSource } from '@sentry/types';
22
import {
33
SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE,
44
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
@@ -33,6 +33,27 @@ describe('getDynamicSamplingContextFromSpan', () => {
3333
expect(dynamicSamplingContext).toStrictEqual({ environment: 'myEnv' });
3434
});
3535

36+
test('uses frozen DSC from traceState', () => {
37+
const rootSpan = {
38+
spanContext() {
39+
return {
40+
traceId: '1234',
41+
spanId: '12345',
42+
traceFlags: 0,
43+
traceState: {
44+
get() {
45+
return 'sentry-environment=myEnv2';
46+
},
47+
} as unknown as SpanContextData['traceState'],
48+
};
49+
},
50+
} as Span;
51+
52+
const dynamicSamplingContext = getDynamicSamplingContextFromSpan(rootSpan);
53+
54+
expect(dynamicSamplingContext).toStrictEqual({ environment: 'myEnv2' });
55+
});
56+
3657
test('returns a new DSC, if no DSC was provided during rootSpan creation (via attributes)', () => {
3758
const rootSpan = startInactiveSpan({ name: 'tx' });
3859

packages/opentelemetry/src/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ export {
1616
spanHasStatus,
1717
} from './utils/spanTypes';
1818

19-
export { getDynamicSamplingContextFromSpan } from './utils/dynamicSamplingContext';
19+
// Re-export this for backwards compatibility (this used to be a different implementation)
20+
export { getDynamicSamplingContextFromSpan } from '@sentry/core';
2021

2122
export { isSentryRequestSpan } from './utils/isSentryRequest';
2223

packages/opentelemetry/src/propagator.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,13 @@ import type { continueTrace } from '@sentry/core';
88
import { hasTracingEnabled } from '@sentry/core';
99
import { getRootSpan } from '@sentry/core';
1010
import { spanToJSON } from '@sentry/core';
11-
import { getClient, getCurrentScope, getDynamicSamplingContextFromClient, getIsolationScope } from '@sentry/core';
11+
import {
12+
getClient,
13+
getCurrentScope,
14+
getDynamicSamplingContextFromClient,
15+
getDynamicSamplingContextFromSpan,
16+
getIsolationScope,
17+
} from '@sentry/core';
1218
import type { DynamicSamplingContext, Options, PropagationContext } from '@sentry/types';
1319
import {
1420
LRUMap,
@@ -32,7 +38,6 @@ import {
3238
} from './constants';
3339
import { DEBUG_BUILD } from './debug-build';
3440
import { getScopesFromContext, setScopesOnContext } from './utils/contextData';
35-
import { getDynamicSamplingContextFromSpan } from './utils/dynamicSamplingContext';
3641
import { getSamplingDecision } from './utils/getSamplingDecision';
3742
import { setIsSetup } from './utils/setupCheck';
3843

packages/opentelemetry/src/setupEventContextTrace.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
1+
import { getDynamicSamplingContextFromSpan, getRootSpan } from '@sentry/core';
12
import type { Client } from '@sentry/types';
23
import { dropUndefinedKeys } from '@sentry/utils';
3-
import { getDynamicSamplingContextFromSpan } from './utils/dynamicSamplingContext';
4-
5-
import { getRootSpan } from '@sentry/core';
64
import { SENTRY_TRACE_STATE_PARENT_SPAN_ID } from './constants';
75
import { getActiveSpan } from './utils/getActiveSpan';
86
import { spanHasParentId } from './utils/spanTypes';

packages/opentelemetry/src/spanExporter.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { SEMATTRS_HTTP_STATUS_CODE } from '@opentelemetry/semantic-conventions';
55
import {
66
captureEvent,
77
getCapturedScopesOnSpan,
8+
getDynamicSamplingContextFromSpan,
89
getMetricSummaryJsonForSpan,
910
timedEventsToMeasurements,
1011
} from '@sentry/core';
@@ -22,7 +23,6 @@ import { SENTRY_TRACE_STATE_PARENT_SPAN_ID } from './constants';
2223
import { DEBUG_BUILD } from './debug-build';
2324
import { SEMANTIC_ATTRIBUTE_SENTRY_PARENT_IS_REMOTE } from './semanticAttributes';
2425
import { convertOtelTimeToSeconds } from './utils/convertOtelTimeToSeconds';
25-
import { getDynamicSamplingContextFromSpan } from './utils/dynamicSamplingContext';
2626
import { getRequestSpanData } from './utils/getRequestSpanData';
2727
import type { SpanNode } from './utils/groupSpansWithParents';
2828
import { getLocalParentId } from './utils/groupSpansWithParents';
@@ -245,7 +245,7 @@ function createTransactionForOtelSpan(span: ReadableSpan): TransactionEvent {
245245
capturedSpanScope: capturedSpanScopes.scope,
246246
capturedSpanIsolationScope: capturedSpanScopes.isolationScope,
247247
sampleRate,
248-
dynamicSamplingContext: getDynamicSamplingContextFromSpan(span),
248+
dynamicSamplingContext: getDynamicSamplingContextFromSpan(span as unknown as Span),
249249
}),
250250
},
251251
...(source && {

packages/opentelemetry/src/trace.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
continueTrace as baseContinueTrace,
88
getClient,
99
getCurrentScope,
10+
getDynamicSamplingContextFromSpan,
1011
getRootSpan,
1112
handleCallbackErrors,
1213
spanToJSON,
@@ -16,7 +17,6 @@ import { continueTraceAsRemoteSpan, makeTraceState } from './propagator';
1617

1718
import type { OpenTelemetryClient, OpenTelemetrySpanContext } from './types';
1819
import { getContextFromScope, getScopesFromContext } from './utils/contextData';
19-
import { getDynamicSamplingContextFromSpan } from './utils/dynamicSamplingContext';
2020
import { getSamplingDecision } from './utils/getSamplingDecision';
2121

2222
/**

packages/opentelemetry/src/utils/dynamicSamplingContext.ts

Lines changed: 0 additions & 65 deletions
This file was deleted.

packages/opentelemetry/test/trace.test.ts

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
getClient,
1313
getCurrentScope,
1414
getDynamicSamplingContextFromClient,
15+
getDynamicSamplingContextFromSpan,
1516
getRootSpan,
1617
spanIsSampled,
1718
spanToJSON,
@@ -24,7 +25,6 @@ import { makeTraceState } from '../src/propagator';
2425
import { SEMATTRS_HTTP_METHOD } from '@opentelemetry/semantic-conventions';
2526
import { continueTrace, startInactiveSpan, startSpan, startSpanManual } from '../src/trace';
2627
import type { AbstractSpan } from '../src/types';
27-
import { getDynamicSamplingContextFromSpan } from '../src/utils/dynamicSamplingContext';
2828
import { getActiveSpan } from '../src/utils/getActiveSpan';
2929
import { getSamplingDecision } from '../src/utils/getSamplingDecision';
3030
import { getSpanKind } from '../src/utils/getSpanKind';
@@ -983,24 +983,16 @@ describe('trace', () => {
983983
withScope(scope => {
984984
const propagationContext = scope.getPropagationContext();
985985

986-
const ctx = trace.setSpanContext(ROOT_CONTEXT, {
987-
traceId: '12312012123120121231201212312012',
988-
spanId: '1121201211212012',
989-
isRemote: false,
990-
traceFlags: TraceFlags.SAMPLED,
991-
traceState: undefined,
992-
});
993-
994-
context.with(ctx, () => {
986+
startSpan({ name: 'parent span' }, parentSpan => {
995987
const span = startInactiveSpan({ name: 'test span' });
996988

997989
expect(span).toBeDefined();
998-
expect(spanToJSON(span).trace_id).toEqual('12312012123120121231201212312012');
999-
expect(spanToJSON(span).parent_span_id).toEqual('1121201211212012');
990+
expect(spanToJSON(span).trace_id).toEqual(parentSpan.spanContext().traceId);
991+
expect(spanToJSON(span).parent_span_id).toEqual(parentSpan.spanContext().spanId);
1000992
expect(getDynamicSamplingContextFromSpan(span)).toEqual({
1001993
...getDynamicSamplingContextFromClient(propagationContext.traceId, getClient()!),
1002-
trace_id: '12312012123120121231201212312012',
1003-
transaction: 'test span',
994+
trace_id: parentSpan.spanContext().traceId,
995+
transaction: 'parent span',
1004996
sampled: 'true',
1005997
sample_rate: '1',
1006998
});

0 commit comments

Comments
 (0)