Skip to content

Commit dae27c9

Browse files
mydeamydea
committed
feat(opentelemetry): Ignore propagation context for span creation
1 parent e21c3bc commit dae27c9

File tree

36 files changed

+443
-337
lines changed

36 files changed

+443
-337
lines changed

‎dev-packages/e2e-tests/test-applications/nestjs-8/tests/errors.test.ts

+1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ test('Sends exception to Sentry', async ({ baseURL }) => {
2626
expect(errorEvent.contexts?.trace).toEqual({
2727
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
2828
span_id: expect.stringMatching(/[a-f0-9]{16}/),
29+
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
2930
});
3031
});
3132

‎dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/tests/errors.test.ts

+2
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ test('Sends exception to Sentry', async ({ baseURL }) => {
2626
expect(errorEvent.contexts?.trace).toEqual({
2727
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
2828
span_id: expect.stringMatching(/[a-f0-9]{16}/),
29+
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
2930
});
3031
});
3132

@@ -114,5 +115,6 @@ test('Sends graphql exception to Sentry', async ({ baseURL }) => {
114115
expect(errorEvent.contexts?.trace).toEqual({
115116
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
116117
span_id: expect.stringMatching(/[a-f0-9]{16}/),
118+
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
117119
});
118120
});

‎dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts

+1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ test('Sends exception to Sentry', async ({ baseURL }) => {
2626
expect(errorEvent.contexts?.trace).toEqual({
2727
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
2828
span_id: expect.stringMatching(/[a-f0-9]{16}/),
29+
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
2930
});
3031
});
3132

‎dev-packages/e2e-tests/test-applications/nestjs-graphql/tests/errors.test.ts

+1
Original file line numberDiff line numberDiff line change
@@ -45,5 +45,6 @@ test('Sends exception to Sentry', async ({ baseURL }) => {
4545
expect(errorEvent.contexts?.trace).toEqual({
4646
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
4747
span_id: expect.stringMatching(/[a-f0-9]{16}/),
48+
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
4849
});
4950
});

‎dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/errors.test.ts

+3
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ test('Sends unexpected exception to Sentry if thrown in module with global filte
3434
expect(errorEvent.contexts?.trace).toEqual({
3535
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
3636
span_id: expect.stringMatching(/[a-f0-9]{16}/),
37+
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
3738
});
3839
});
3940

@@ -70,6 +71,7 @@ test('Sends unexpected exception to Sentry if thrown in module with local filter
7071
expect(errorEvent.contexts?.trace).toEqual({
7172
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
7273
span_id: expect.stringMatching(/[a-f0-9]{16}/),
74+
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
7375
});
7476
});
7577

@@ -108,6 +110,7 @@ test('Sends unexpected exception to Sentry if thrown in module that was register
108110
expect(errorEvent.contexts?.trace).toEqual({
109111
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
110112
span_id: expect.stringMatching(/[a-f0-9]{16}/),
113+
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
111114
});
112115
});
113116

‎dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/errors.test.ts

+3
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ test('Sends unexpected exception to Sentry if thrown in module with global filte
2626
expect(errorEvent.contexts?.trace).toEqual({
2727
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
2828
span_id: expect.stringMatching(/[a-f0-9]{16}/),
29+
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
2930
});
3031
});
3132

@@ -54,6 +55,7 @@ test('Sends unexpected exception to Sentry if thrown in module with local filter
5455
expect(errorEvent.contexts?.trace).toEqual({
5556
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
5657
span_id: expect.stringMatching(/[a-f0-9]{16}/),
58+
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
5759
});
5860
});
5961

@@ -84,6 +86,7 @@ test('Sends unexpected exception to Sentry if thrown in module that was register
8486
expect(errorEvent.contexts?.trace).toEqual({
8587
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
8688
span_id: expect.stringMatching(/[a-f0-9]{16}/),
89+
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
8790
});
8891
});
8992

‎dev-packages/e2e-tests/test-applications/node-fastify-5/tests/errors.test.ts

+1
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,6 @@ test('Sends correct error event', async ({ baseURL }) => {
2525
expect(errorEvent.contexts?.trace).toEqual({
2626
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
2727
span_id: expect.stringMatching(/[a-f0-9]{16}/),
28+
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
2829
});
2930
});

‎dev-packages/e2e-tests/test-applications/node-fastify/tests/errors.test.ts

+1
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,6 @@ test('Sends correct error event', async ({ baseURL }) => {
2525
expect(errorEvent.contexts?.trace).toEqual({
2626
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
2727
span_id: expect.stringMatching(/[a-f0-9]{16}/),
28+
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
2829
});
2930
});

‎dev-packages/e2e-tests/test-applications/node-koa/tests/assert.test.ts

+1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ test('Returns 400 from failed assert', async ({ baseURL }) => {
2626
expect(errorEvent.contexts?.trace).toEqual({
2727
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
2828
span_id: expect.stringMatching(/[a-f0-9]{16}/),
29+
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
2930
});
3031
});
3132

‎dev-packages/e2e-tests/test-applications/node-koa/tests/errors.test.ts

+1
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,6 @@ test('Sends correct error event', async ({ baseURL }) => {
2525
expect(errorEvent.contexts?.trace).toEqual({
2626
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
2727
span_id: expect.stringMatching(/[a-f0-9]{16}/),
28+
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
2829
});
2930
});

‎dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/transactions.test.ts

+40-2
Original file line numberDiff line numberDiff line change
@@ -34,17 +34,55 @@ test('Sends an API route transaction to OTLP', async ({ baseURL }) => {
3434
const scopeSpans = json.resourceSpans?.[0]?.scopeSpans;
3535
expect(scopeSpans).toBeDefined();
3636

37-
// Http server span & undici client spans are emitted
37+
// Http server span & undici client spans are emitted,
38+
// as well as the spans emitted via `Sentry.startSpan()`
3839
// But our default node-fetch spans are not emitted
39-
expect(scopeSpans.length).toEqual(2);
40+
expect(scopeSpans.length).toEqual(3);
4041

4142
const httpScopes = scopeSpans?.filter(scopeSpan => scopeSpan.scope.name === '@opentelemetry/instrumentation-http');
4243
const undiciScopes = scopeSpans?.filter(
4344
scopeSpan => scopeSpan.scope.name === '@opentelemetry/instrumentation-undici',
4445
);
46+
const startSpanScopes = scopeSpans?.filter(scopeSpan => scopeSpan.scope.name === '@sentry/node');
4547

4648
expect(httpScopes.length).toBe(1);
4749

50+
expect(startSpanScopes.length).toBe(1);
51+
expect(startSpanScopes[0].spans.length).toBe(2);
52+
expect(startSpanScopes[0].spans).toEqual([
53+
{
54+
traceId: expect.any(String),
55+
spanId: expect.any(String),
56+
parentSpanId: expect.any(String),
57+
name: 'test-span',
58+
kind: 1,
59+
startTimeUnixNano: expect.any(String),
60+
endTimeUnixNano: expect.any(String),
61+
attributes: [],
62+
droppedAttributesCount: 0,
63+
events: [],
64+
droppedEventsCount: 0,
65+
status: { code: 0 },
66+
links: [],
67+
droppedLinksCount: 0,
68+
},
69+
{
70+
traceId: expect.any(String),
71+
spanId: expect.any(String),
72+
name: 'test-transaction',
73+
kind: 1,
74+
startTimeUnixNano: expect.any(String),
75+
endTimeUnixNano: expect.any(String),
76+
attributes: [{ key: 'sentry.op', value: { stringValue: 'e2e-test' } }],
77+
droppedAttributesCount: 0,
78+
events: [],
79+
droppedEventsCount: 0,
80+
status: { code: 0 },
81+
links: [],
82+
droppedLinksCount: 0,
83+
},
84+
]);
85+
4886
// Undici spans are emitted correctly
4987
expect(undiciScopes.length).toBe(1);
5088
expect(undiciScopes[0].spans.length).toBe(1);

‎dev-packages/node-integration-tests/suites/aws-serverless/aws-integration/s3/test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,6 @@ describe('awsIntegration', () => {
2626
});
2727

2828
test('should auto-instrument aws-sdk v2 package.', done => {
29-
createRunner(__dirname, 'scenario.js').expect({ transaction: EXPECTED_TRANSCATION }).start(done);
29+
createRunner(__dirname, 'scenario.js').ignore('event').expect({ transaction: EXPECTED_TRANSCATION }).start(done);
3030
});
3131
});

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

+14-2
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,18 @@ Sentry.withScope(scope => {
1515
traceId: '12345678901234567890123456789012',
1616
});
1717

18-
Sentry.startSpan({ name: 'test_span_1' }, () => undefined);
19-
Sentry.startSpan({ name: 'test_span_2' }, () => undefined);
18+
const spanIdTraceId = Sentry.startSpan(
19+
{
20+
name: 'test_span_1',
21+
},
22+
span1 => span1.spanContext().traceId,
23+
);
24+
25+
Sentry.startSpan(
26+
{
27+
name: 'test_span_2',
28+
attributes: { spanIdTraceId },
29+
},
30+
() => undefined,
31+
);
2032
});

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

+12-21
Original file line numberDiff line numberDiff line change
@@ -6,28 +6,19 @@ afterAll(() => {
66

77
test('should send manually started parallel root spans outside of root context with parentSpanId', done => {
88
createRunner(__dirname, 'scenario.ts')
9+
.expect({ transaction: { transaction: 'test_span_1' } })
910
.expect({
10-
transaction: {
11-
transaction: 'test_span_1',
12-
contexts: {
13-
trace: {
14-
span_id: expect.stringMatching(/[a-f0-9]{16}/),
15-
parent_span_id: '1234567890123456',
16-
trace_id: '12345678901234567890123456789012',
17-
},
18-
},
19-
},
20-
})
21-
.expect({
22-
transaction: {
23-
transaction: 'test_span_2',
24-
contexts: {
25-
trace: {
26-
span_id: expect.stringMatching(/[a-f0-9]{16}/),
27-
parent_span_id: '1234567890123456',
28-
trace_id: '12345678901234567890123456789012',
29-
},
30-
},
11+
transaction: transaction => {
12+
expect(transaction).toBeDefined();
13+
const traceId = transaction.contexts?.trace?.trace_id;
14+
expect(traceId).toBeDefined();
15+
expect(transaction.contexts?.trace?.parent_span_id).toBeUndefined();
16+
17+
const trace1Id = transaction.contexts?.trace?.data?.spanIdTraceId;
18+
expect(trace1Id).toBeDefined();
19+
20+
// Different trace ID as the first span
21+
expect(trace1Id).not.toBe(traceId);
3122
},
3223
})
3324
.start(done);

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ test('should send manually started parallel root spans outside of root context',
1919
const trace1Id = transaction.contexts?.trace?.data?.spanIdTraceId;
2020
expect(trace1Id).toBeDefined();
2121

22-
// Same trace ID as the first span
23-
expect(trace1Id).toBe(traceId);
22+
// Different trace ID as the first span
23+
expect(trace1Id).not.toBe(traceId);
2424
},
2525
})
2626
.start(done);

‎docs/assets/node-sdk-trace-propagation.png

1.27 MB
Loading

‎docs/trace-propagation.md

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
# How Trace Propagation Works in the JavaScript SDKs
2+
3+
Trace propagation describes how and when traceId & spanId are set and send for various types of events.
4+
How this behaves varies a bit from Browser to Node SDKs.
5+
6+
## Node SDKs (OpenTelemetry based)
7+
8+
In the Node SDK and related OpenTelemetry-based SDKs, trace propagation works as follows:
9+
10+
![node-sdk-trace-propagation-scenarios](./assets/node-sdk-trace-propagation.png)
11+
12+
## Browser/Other SDKs
13+
14+
TODO

‎packages/core/src/tracing/sentryNonRecordingSpan.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import type {
77
SpanStatus,
88
SpanTimeInput,
99
} from '../types-hoist';
10-
import { uuid4 } from '../utils-hoist/misc';
10+
import { generateSpanId, generateTraceId } from '../utils-hoist';
1111
import { TRACE_FLAG_NONE } from '../utils/spanUtils';
1212

1313
/**
@@ -18,8 +18,8 @@ export class SentryNonRecordingSpan implements Span {
1818
private _spanId: string;
1919

2020
public constructor(spanContext: SentrySpanArguments = {}) {
21-
this._traceId = spanContext.traceId || uuid4();
22-
this._spanId = spanContext.spanId || uuid4().substring(16);
21+
this._traceId = spanContext.traceId || generateTraceId();
22+
this._spanId = spanContext.spanId || generateSpanId();
2323
}
2424

2525
/** @inheritdoc */

‎packages/core/src/tracing/sentrySpan.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ import {
2525
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
2626
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
2727
} from '../semanticAttributes';
28+
import { generateSpanId, generateTraceId } from '../utils-hoist';
2829
import { logger } from '../utils-hoist/logger';
29-
import { uuid4 } from '../utils-hoist/misc';
3030
import { dropUndefinedKeys } from '../utils-hoist/object';
3131
import { timestampInSeconds } from '../utils-hoist/time';
3232
import {
@@ -76,8 +76,8 @@ export class SentrySpan implements Span {
7676
* @hidden
7777
*/
7878
public constructor(spanContext: SentrySpanArguments = {}) {
79-
this._traceId = spanContext.traceId || uuid4();
80-
this._spanId = spanContext.spanId || uuid4().substring(16);
79+
this._traceId = spanContext.traceId || generateTraceId();
80+
this._spanId = spanContext.spanId || generateSpanId();
8181
this._startTime = spanContext.startTimestamp || timestampInSeconds();
8282

8383
this._attributes = {};

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

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import type { PropagationContext, TraceparentData } from '../types-hoist';
22

33
import { baggageHeaderToDynamicSamplingContext } from './baggage';
4-
import { uuid4 } from './misc';
54
import { generateSpanId, generateTraceId } from './propagationContext';
65

76
// eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- RegExp is used for readability here
@@ -76,8 +75,8 @@ export function propagationContextFromHeaders(
7675
* Create sentry-trace header from span context values.
7776
*/
7877
export function generateSentryTraceHeader(
79-
traceId: string = uuid4(),
80-
spanId: string = uuid4().substring(16),
78+
traceId: string = generateTraceId(),
79+
spanId: string = generateSpanId(),
8180
sampled?: boolean,
8281
): string {
8382
let sampledString = '';

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

+20-11
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,3 @@
1-
import { getAsyncContextStrategy } from '../asyncContext';
2-
import { getMainCarrier } from '../carrier';
3-
import { getCurrentScope } from '../currentScopes';
4-
import { getMetricSummaryJsonForSpan, updateMetricSummaryOnSpan } from '../metrics/metric-summary';
5-
import type { MetricType } from '../metrics/types';
6-
import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../semanticAttributes';
7-
import type { SentrySpan } from '../tracing/sentrySpan';
8-
import { SPAN_STATUS_OK, SPAN_STATUS_UNSET } from '../tracing/spanstatus';
91
import type {
102
MeasurementUnit,
113
Primitive,
@@ -17,6 +9,15 @@ import type {
179
SpanTimeInput,
1810
TraceContext,
1911
} from '../types-hoist';
12+
import { getAsyncContextStrategy } from '../asyncContext';
13+
import { getMainCarrier } from '../carrier';
14+
import { getCurrentScope } from '../currentScopes';
15+
import { getMetricSummaryJsonForSpan, updateMetricSummaryOnSpan } from '../metrics/metric-summary';
16+
import type { MetricType } from '../metrics/types';
17+
import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../semanticAttributes';
18+
import type { SentrySpan } from '../tracing/sentrySpan';
19+
import { SPAN_STATUS_OK, SPAN_STATUS_UNSET } from '../tracing/spanstatus';
20+
import { generateSpanId } from '../utils-hoist/propagationContext';
2021
import { consoleSandbox } from '../utils-hoist/logger';
2122
import { addNonEnumerableProperty, dropUndefinedKeys } from '../utils-hoist/object';
2223
import { timestampInSeconds } from '../utils-hoist/time';
@@ -54,10 +55,18 @@ export function spanToTransactionTraceContext(span: Span): TraceContext {
5455
* Convert a span to a trace context, which can be sent as the `trace` context in a non-transaction event.
5556
*/
5657
export function spanToTraceContext(span: Span): TraceContext {
57-
const { spanId: span_id, traceId: trace_id } = span.spanContext();
58-
const { parent_span_id } = spanToJSON(span);
58+
const { spanId, traceId: trace_id, isRemote } = span.spanContext();
59+
60+
// If the span is remote, we use a random/virtual span as span_id to the trace context,
61+
// and the remote span as parent_span_id
62+
const parent_span_id = isRemote ? spanId : spanToJSON(span).parent_span_id;
63+
const span_id = isRemote ? generateSpanId() : spanId;
5964

60-
return dropUndefinedKeys({ parent_span_id, span_id, trace_id });
65+
return dropUndefinedKeys({
66+
parent_span_id,
67+
span_id,
68+
trace_id,
69+
});
6170
}
6271

6372
/**

0 commit comments

Comments
 (0)