Skip to content

Commit d5af638

Browse files
mydealforst
mydea
and
lforst
authored
feat!: Remove spanId from propagation context (#14733)
Closes #12385 This also deprecates `getPropagationContextFromSpan` as it is no longer used/needed. We may think about removing this in v9, but IMHO we can also just leave this for v9, it does not hurt too much to have it in there... --------- Co-authored-by: Luca Forstner <luca.forstner@sentry.io>
1 parent d4e94fe commit d5af638

File tree

38 files changed

+190
-401
lines changed

38 files changed

+190
-401
lines changed

‎dev-packages/browser-integration-tests/suites/public-api/startSpan/parallel-root-spans-with-parentSpanId/subject.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
Sentry.getCurrentScope().setPropagationContext({
22
parentSpanId: '1234567890123456',
3-
spanId: '123456789012345x',
43
traceId: '12345678901234567890123456789012',
54
});
65

‎dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/twp-errors-meta/init.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
5+
Sentry.init({
6+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
7+
integrations: integrations => {
8+
integrations.push(Sentry.browserTracingIntegration());
9+
return integrations.filter(i => i.name !== 'BrowserSession');
10+
},
11+
tracesSampleRate: 0,
12+
});

‎dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/twp-errors-meta/subject.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Sentry.captureException(new Error('test error'));
2+
Sentry.captureException(new Error('test error 2'));

‎dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/twp-errors-meta/template.html

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8" />
5+
<meta name="sentry-trace" content="12312012123120121231201212312012-1121201211212012" />
6+
<meta
7+
name="baggage"
8+
content="sentry-release=2.1.12,sentry-public_key=public,sentry-trace_id=123"
9+
/>
10+
</head>
11+
</html>

‎dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/twp-errors-meta/test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import { expect } from '@playwright/test';
2+
import type { Event } from '@sentry/browser';
3+
import { sentryTest } from '../../../../utils/fixtures';
4+
import { getMultipleSentryEnvelopeRequests, shouldSkipTracingTest } from '../../../../utils/helpers';
5+
6+
sentryTest('errors in TwP mode have same trace ID & span IDs', async ({ getLocalTestUrl, page }) => {
7+
if (shouldSkipTracingTest()) {
8+
sentryTest.skip();
9+
}
10+
11+
const traceId = '12312012123120121231201212312012';
12+
const spanId = '1121201211212012';
13+
14+
const url = await getLocalTestUrl({ testDir: __dirname });
15+
const [event1, event2] = await getMultipleSentryEnvelopeRequests<Event>(page, 2, { url });
16+
17+
// Ensure these are the actual errors we care about
18+
expect(event1.exception?.values?.[0].value).toContain('test error');
19+
expect(event2.exception?.values?.[0].value).toContain('test error');
20+
21+
const contexts1 = event1.contexts;
22+
const { trace_id: traceId1, span_id: spanId1 } = contexts1?.trace || {};
23+
expect(traceId1).toEqual(traceId);
24+
25+
// Span ID is a virtual span, not the propagated one
26+
expect(spanId1).not.toEqual(spanId);
27+
expect(spanId1).toMatch(/^[a-f0-9]{16}$/);
28+
29+
const contexts2 = event2.contexts;
30+
const { trace_id: traceId2, span_id: spanId2 } = contexts2?.trace || {};
31+
expect(traceId2).toEqual(traceId);
32+
expect(spanId2).toMatch(/^[a-f0-9]{16}$/);
33+
34+
expect(spanId2).toEqual(spanId1);
35+
});

‎dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/twp-errors/init.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
5+
Sentry.init({
6+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
7+
integrations: integrations => {
8+
integrations.push(Sentry.browserTracingIntegration());
9+
return integrations.filter(i => i.name !== 'BrowserSession');
10+
},
11+
tracesSampleRate: 0,
12+
});

‎dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/twp-errors/subject.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Sentry.captureException(new Error('test error'));
2+
Sentry.captureException(new Error('test error 2'));

‎dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/twp-errors/test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import { expect } from '@playwright/test';
2+
import type { Event } from '@sentry/browser';
3+
import { sentryTest } from '../../../../utils/fixtures';
4+
import { getMultipleSentryEnvelopeRequests, shouldSkipTracingTest } from '../../../../utils/helpers';
5+
6+
sentryTest('errors in TwP mode have same trace ID & span IDs', async ({ getLocalTestUrl, page }) => {
7+
if (shouldSkipTracingTest()) {
8+
sentryTest.skip();
9+
}
10+
11+
const url = await getLocalTestUrl({ testDir: __dirname });
12+
const [event1, event2] = await getMultipleSentryEnvelopeRequests<Event>(page, 2, { url });
13+
14+
// Ensure these are the actual errors we care about
15+
expect(event1.exception?.values?.[0].value).toContain('test error');
16+
expect(event2.exception?.values?.[0].value).toContain('test error');
17+
18+
const contexts1 = event1.contexts;
19+
const { trace_id: traceId1, span_id: spanId1 } = contexts1?.trace || {};
20+
expect(traceId1).toMatch(/^[a-f0-9]{32}$/);
21+
expect(spanId1).toMatch(/^[a-f0-9]{16}$/);
22+
23+
const contexts2 = event2.contexts;
24+
const { trace_id: traceId2, span_id: spanId2 } = contexts2?.trace || {};
25+
expect(traceId2).toMatch(/^[a-f0-9]{32}$/);
26+
expect(spanId2).toMatch(/^[a-f0-9]{16}$/);
27+
28+
expect(traceId2).toEqual(traceId1);
29+
expect(spanId2).toEqual(spanId1);
30+
});

‎dev-packages/node-integration-tests/suites/public-api/startSpan/parallel-root-spans/scenario.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ Sentry.init({
1010

1111
Sentry.getCurrentScope().setPropagationContext({
1212
parentSpanId: '1234567890123456',
13-
spanId: '123456789012345x',
1413
traceId: '12345678901234567890123456789012',
1514
});
1615

‎dev-packages/node-integration-tests/suites/public-api/startSpan/parallel-spans-in-scope-with-parentSpanId/scenario.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ Sentry.init({
1111
Sentry.withScope(scope => {
1212
scope.setPropagationContext({
1313
parentSpanId: '1234567890123456',
14-
spanId: '123456789012345x',
1514
traceId: '12345678901234567890123456789012',
1615
});
1716

‎dev-packages/node-integration-tests/suites/tracing/meta-tags-twp-errors/test.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ describe('errors in TwP mode have same trace in trace context and getTraceData()
55
cleanupChildProcesses();
66
});
77

8+
// In a request handler, the spanId is consistent inside of the request
89
test('in incoming request', done => {
910
createRunner(__dirname, 'server.js')
1011
.expect({
@@ -30,6 +31,7 @@ describe('errors in TwP mode have same trace in trace context and getTraceData()
3031
.makeRequest('get', '/test');
3132
});
3233

34+
// Outside of a request handler, the spanId is random
3335
test('outside of a request handler', done => {
3436
createRunner(__dirname, 'no-server.js')
3537
.expect({
@@ -41,11 +43,18 @@ describe('errors in TwP mode have same trace in trace context and getTraceData()
4143

4244
const traceData = contexts?.traceData || {};
4345

44-
expect(traceData['sentry-trace']).toEqual(`${trace_id}-${span_id}`);
46+
expect(traceData['sentry-trace']).toMatch(/^[a-f0-9]{32}-[a-f0-9]{16}$/);
47+
expect(traceData['sentry-trace']).toContain(`${trace_id}-`);
48+
// span_id is a random span ID
49+
expect(traceData['sentry-trace']).not.toContain(span_id);
50+
4551
expect(traceData.baggage).toContain(`sentry-trace_id=${trace_id}`);
4652
expect(traceData.baggage).not.toContain('sentry-sampled=');
4753

48-
expect(traceData.metaTags).toContain(`<meta name="sentry-trace" content="${trace_id}-${span_id}"/>`);
54+
expect(traceData.metaTags).toMatch(/<meta name="sentry-trace" content="[a-f0-9]{32}-[a-f0-9]{16}"\/>/);
55+
expect(traceData.metaTags).toContain(`<meta name="sentry-trace" content="${trace_id}-`);
56+
// span_id is a random span ID
57+
expect(traceData.metaTags).not.toContain(span_id);
4958
expect(traceData.metaTags).toContain(`sentry-trace_id=${trace_id}`);
5059
expect(traceData.metaTags).not.toContain('sentry-sampled=');
5160
},

‎packages/browser/test/tracing/browserTracingIntegration.test.ts

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -643,19 +643,15 @@ describe('browserTracingIntegration', () => {
643643
const newCurrentScopePropCtx = getCurrentScope().getPropagationContext();
644644

645645
expect(oldCurrentScopePropCtx).toEqual({
646-
spanId: expect.stringMatching(/[a-f0-9]{16}/),
647646
traceId: expect.stringMatching(/[a-f0-9]{32}/),
648647
});
649648
expect(oldIsolationScopePropCtx).toEqual({
650-
spanId: expect.stringMatching(/[a-f0-9]{16}/),
651649
traceId: expect.stringMatching(/[a-f0-9]{32}/),
652650
});
653651
expect(newCurrentScopePropCtx).toEqual({
654-
spanId: expect.stringMatching(/[a-f0-9]{16}/),
655652
traceId: expect.stringMatching(/[a-f0-9]{32}/),
656653
});
657654
expect(newIsolationScopePropCtx).toEqual({
658-
spanId: expect.stringMatching(/[a-f0-9]{16}/),
659655
traceId: expect.stringMatching(/[a-f0-9]{32}/),
660656
});
661657

@@ -680,16 +676,13 @@ describe('browserTracingIntegration', () => {
680676

681677
const propCtxBeforeEnd = getCurrentScope().getPropagationContext();
682678
expect(propCtxBeforeEnd).toStrictEqual({
683-
spanId: expect.stringMatching(/[a-f0-9]{16}/),
684679
traceId: expect.stringMatching(/[a-f0-9]{32}/),
685680
});
686681

687682
navigationSpan!.end();
688683

689684
const propCtxAfterEnd = getCurrentScope().getPropagationContext();
690685
expect(propCtxAfterEnd).toStrictEqual({
691-
// eslint-disable-next-line deprecation/deprecation
692-
spanId: propCtxBeforeEnd.spanId,
693686
traceId: propCtxBeforeEnd.traceId,
694687
sampled: true,
695688
dsc: {
@@ -720,16 +713,13 @@ describe('browserTracingIntegration', () => {
720713

721714
const propCtxBeforeEnd = getCurrentScope().getPropagationContext();
722715
expect(propCtxBeforeEnd).toStrictEqual({
723-
spanId: expect.stringMatching(/[a-f0-9]{16}/),
724716
traceId: expect.stringMatching(/[a-f0-9]{32}/),
725717
});
726718

727719
navigationSpan!.end();
728720

729721
const propCtxAfterEnd = getCurrentScope().getPropagationContext();
730722
expect(propCtxAfterEnd).toStrictEqual({
731-
// eslint-disable-next-line deprecation/deprecation
732-
spanId: propCtxBeforeEnd.spanId,
733723
traceId: propCtxBeforeEnd.traceId,
734724
sampled: false,
735725
dsc: {

‎packages/core/src/currentScopes.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { getGlobalSingleton, getMainCarrier } from './carrier';
33
import type { Client } from './client';
44
import { Scope } from './scope';
55
import type { TraceContext } from './types-hoist';
6+
import { generateSpanId } from './utils-hoist';
67
import { dropUndefinedKeys } from './utils-hoist/object';
78

89
/**
@@ -127,13 +128,11 @@ export function getClient<C extends Client>(): C | undefined {
127128
export function getTraceContextFromScope(scope: Scope): TraceContext {
128129
const propagationContext = scope.getPropagationContext();
129130

130-
// TODO(v9): Use generateSpanId() instead of spanId
131-
// eslint-disable-next-line deprecation/deprecation
132-
const { traceId, spanId, parentSpanId } = propagationContext;
131+
const { traceId, parentSpanId, propagationSpanId } = propagationContext;
133132

134133
const traceContext: TraceContext = dropUndefinedKeys({
135134
trace_id: traceId,
136-
span_id: spanId,
135+
span_id: propagationSpanId || generateSpanId(),
137136
parent_span_id: parentSpanId,
138137
});
139138

‎packages/core/src/scope.ts

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import type {
2424
import { isPlainObject } from './utils-hoist/is';
2525
import { logger } from './utils-hoist/logger';
2626
import { uuid4 } from './utils-hoist/misc';
27-
import { generateSpanId, generateTraceId } from './utils-hoist/propagationContext';
27+
import { generateTraceId } from './utils-hoist/propagationContext';
2828
import { dateTimestampInSeconds } from './utils-hoist/time';
2929
import { merge } from './utils/merge';
3030
import { _getSpanForScope, _setSpanForScope } from './utils/spanOnScope';
@@ -166,7 +166,6 @@ export class Scope {
166166
this._sdkProcessingMetadata = {};
167167
this._propagationContext = {
168168
traceId: generateTraceId(),
169-
spanId: generateSpanId(),
170169
};
171170
}
172171

@@ -555,14 +554,8 @@ export class Scope {
555554
/**
556555
* Add propagation context to the scope, used for distributed tracing
557556
*/
558-
public setPropagationContext(
559-
context: Omit<PropagationContext, 'spanId'> & Partial<Pick<PropagationContext, 'spanId'>>,
560-
): this {
561-
this._propagationContext = {
562-
// eslint-disable-next-line deprecation/deprecation
563-
spanId: generateSpanId(),
564-
...context,
565-
};
557+
public setPropagationContext(context: PropagationContext): this {
558+
this._propagationContext = context;
566559
return this;
567560
}
568561

‎packages/core/src/types-hoist/tracing.ts

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,28 +15,29 @@ export interface PropagationContext {
1515
* Either represents the incoming `traceId` or the `traceId` generated by the current SDK, if there was no incoming trace.
1616
*/
1717
traceId: string;
18-
/**
19-
* Represents the execution context of the current SDK. This acts as a fallback value to associate events with a
20-
* particular execution context when performance monitoring is disabled.
21-
*
22-
* The ID of a current span (if one exists) should have precedence over this value when propagating trace data.
23-
*
24-
* @deprecated This value will not be used anymore in the future, and should not be set or read anymore.
25-
*/
26-
spanId: string;
18+
2719
/**
2820
* Represents the sampling decision of the incoming trace.
2921
*
3022
* The current SDK should not modify this value!
3123
*/
3224
sampled?: boolean;
25+
3326
/**
3427
* The `parentSpanId` denotes the ID of the incoming client span. If there is no `parentSpanId` on the propagation
3528
* context, it means that the the incoming trace didn't come from a span.
3629
*
3730
* The current SDK should not modify this value!
3831
*/
3932
parentSpanId?: string;
33+
34+
/**
35+
* A span ID that should be used for the `trace` context of various event types, and for propagation of a `parentSpanId` to downstream services, when performance is disabled or when there is no active span.
36+
* This value should be set by the SDK in an informed way when the same span ID should be used for one unit of execution (e.g. a request, usually tied to the isolation scope).
37+
* If this value is undefined on the propagation context, the SDK will generate a random span ID for `trace` contexts and trace propagation.
38+
*/
39+
propagationSpanId?: string;
40+
4041
/**
4142
* An undefined dsc in the propagation context means that the current SDK invocation is the head of trace and still free to modify and set the DSC for outgoing requests.
4243
*

‎packages/core/src/utils-hoist/propagationContext.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import { uuid4 } from './misc';
99
export function generatePropagationContext(): PropagationContext {
1010
return {
1111
traceId: generateTraceId(),
12-
spanId: generateSpanId(),
1312
};
1413
}
1514

‎packages/core/src/utils-hoist/tracing.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,17 +55,14 @@ export function propagationContextFromHeaders(
5555
const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(baggage);
5656

5757
if (!traceparentData || !traceparentData.traceId) {
58-
return { traceId: generateTraceId(), spanId: generateSpanId() };
58+
return { traceId: generateTraceId() };
5959
}
6060

6161
const { traceId, parentSpanId, parentSampled } = traceparentData;
6262

63-
const virtualSpanId = generateSpanId();
64-
6563
return {
6664
traceId,
6765
parentSpanId,
68-
spanId: virtualSpanId,
6966
sampled: parentSampled,
7067
dsc: dynamicSamplingContext || {}, // If we have traceparent data but no DSC it means we are not head of trace and we must freeze it
7168
};
@@ -75,8 +72,8 @@ export function propagationContextFromHeaders(
7572
* Create sentry-trace header from span context values.
7673
*/
7774
export function generateSentryTraceHeader(
78-
traceId: string = generateTraceId(),
79-
spanId: string = generateSpanId(),
75+
traceId: string | undefined = generateTraceId(),
76+
spanId: string | undefined = generateSpanId(),
8077
sampled?: boolean,
8178
): string {
8279
let sampledString = '';

‎packages/core/src/utils/spanUtils.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
} from '../semanticAttributes';
1010
import type { SentrySpan } from '../tracing/sentrySpan';
1111
import { SPAN_STATUS_OK, SPAN_STATUS_UNSET } from '../tracing/spanstatus';
12+
import { getCapturedScopesOnSpan } from '../tracing/utils';
1213
import type {
1314
Span,
1415
SpanAttributes,
@@ -61,7 +62,9 @@ export function spanToTraceContext(span: Span): TraceContext {
6162
// If the span is remote, we use a random/virtual span as span_id to the trace context,
6263
// and the remote span as parent_span_id
6364
const parent_span_id = isRemote ? spanId : spanToJSON(span).parent_span_id;
64-
const span_id = isRemote ? generateSpanId() : spanId;
65+
const scope = getCapturedScopesOnSpan(span).scope;
66+
67+
const span_id = isRemote ? scope?.getPropagationContext().propagationSpanId || generateSpanId() : spanId;
6568

6669
return dropUndefinedKeys({
6770
parent_span_id,

‎packages/core/src/utils/traceData.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,6 @@ export function getTraceData(options: { span?: Span } = {}): SerializedTraceData
5555
* Get a sentry-trace header value for the given scope.
5656
*/
5757
function scopeToTraceHeader(scope: Scope): string {
58-
// TODO(v9): Use generateSpanId() instead of spanId
59-
// eslint-disable-next-line deprecation/deprecation
60-
const { traceId, sampled, spanId } = scope.getPropagationContext();
61-
return generateSentryTraceHeader(traceId, spanId, sampled);
58+
const { traceId, sampled, propagationSpanId } = scope.getPropagationContext();
59+
return generateSentryTraceHeader(traceId, propagationSpanId, sampled);
6260
}

0 commit comments

Comments
 (0)