Skip to content

Commit 379a9e5

Browse files
authored
fix(node): Ensure DSC is correctly set in envelope headers (#11628)
This was not correctly set before when we had an active span - the `error-active-span` & `error-active-span-unsampled` scenarios failed before and had no `trace` set on the envelope headers. The fix was to actually fix our `setupEventContextTrace` utility for OTEL, where we now just set the dsc on the event sdkProcessingMetadata, which is picked up when event processing. I also moved this to `preprocessEvent` to ensure this runs before other stuff. This should fix https://github.com/getsentry/sentry-javascript-examples/pull/7/files#r1561220216 which @s1gr1d found in her example app. 🚀
1 parent d5ac938 commit 379a9e5

File tree

14 files changed

+289
-6
lines changed

14 files changed

+289
-6
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
2+
import * as Sentry from '@sentry/node';
3+
4+
Sentry.init({
5+
dsn: 'https://[email protected]/1337',
6+
release: '1.0',
7+
tracePropagationTargets: [/\/v0/, 'v1'],
8+
tracesSampleRate: 0,
9+
integrations: [],
10+
transport: loggingTransport,
11+
});
12+
13+
Sentry.startSpan({ name: 'test span' }, () => {
14+
Sentry.captureException(new Error('foo'));
15+
});
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { createRunner } from '../../../../utils/runner';
2+
3+
test('envelope header for error event during active unsampled span is correct', done => {
4+
createRunner(__dirname, 'scenario.ts')
5+
.ignore('session', 'sessions', 'transaction')
6+
.expectHeader({
7+
event: {
8+
trace: {
9+
trace_id: expect.any(String),
10+
public_key: 'public',
11+
environment: 'production',
12+
release: '1.0',
13+
sampled: 'false',
14+
},
15+
},
16+
})
17+
.start(done);
18+
});
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
2+
import * as Sentry from '@sentry/node';
3+
4+
Sentry.init({
5+
dsn: 'https://[email protected]/1337',
6+
release: '1.0',
7+
tracePropagationTargets: [/\/v0/, 'v1'],
8+
tracesSampleRate: 1,
9+
integrations: [],
10+
transport: loggingTransport,
11+
});
12+
13+
Sentry.startSpan({ name: 'test span' }, () => {
14+
Sentry.captureException(new Error('foo'));
15+
});
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { createRunner } from '../../../../utils/runner';
2+
3+
test('envelope header for error event during active span is correct', done => {
4+
createRunner(__dirname, 'scenario.ts')
5+
.ignore('session', 'sessions', 'transaction')
6+
.expectHeader({
7+
event: {
8+
trace: {
9+
trace_id: expect.any(String),
10+
public_key: 'public',
11+
environment: 'production',
12+
release: '1.0',
13+
sample_rate: '1',
14+
sampled: 'true',
15+
transaction: 'test span',
16+
},
17+
},
18+
})
19+
.start(done);
20+
});
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
2+
import * as Sentry from '@sentry/node';
3+
4+
Sentry.init({
5+
dsn: 'https://[email protected]/1337',
6+
release: '1.0',
7+
tracePropagationTargets: [/\/v0/, 'v1'],
8+
tracesSampleRate: 0,
9+
integrations: [],
10+
transport: loggingTransport,
11+
});
12+
13+
Sentry.captureException(new Error('foo'));
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import { createRunner } from '../../../../utils/runner';
2+
3+
test('envelope header for error events is correct', done => {
4+
createRunner(__dirname, 'scenario.ts')
5+
.ignore('session', 'sessions')
6+
.expectHeader({
7+
event: {
8+
trace: {
9+
trace_id: expect.any(String),
10+
environment: 'production',
11+
public_key: 'public',
12+
release: '1.0',
13+
},
14+
},
15+
})
16+
.start(done);
17+
});
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
2+
import * as Sentry from '@sentry/node';
3+
4+
Sentry.init({
5+
dsn: 'https://[email protected]/1337',
6+
release: '1.0',
7+
tracePropagationTargets: [/\/v0/, 'v1'],
8+
tracesSampleRate: 1,
9+
integrations: [],
10+
transport: loggingTransport,
11+
});
12+
13+
Sentry.startSpan(
14+
{
15+
name: 'GET /route',
16+
attributes: {
17+
'http.method': 'GET',
18+
'http.route': '/route',
19+
[Sentry.SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server',
20+
[Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
21+
},
22+
},
23+
() => {
24+
// noop
25+
},
26+
);
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { createRunner } from '../../../../utils/runner';
2+
3+
test('envelope header for transaction event of route correct', done => {
4+
createRunner(__dirname, 'scenario.ts')
5+
.ignore('session', 'sessions')
6+
.expectHeader({
7+
transaction: {
8+
trace: {
9+
trace_id: expect.any(String),
10+
public_key: 'public',
11+
transaction: 'GET /route',
12+
environment: 'production',
13+
release: '1.0',
14+
sample_rate: '1',
15+
sampled: 'true',
16+
},
17+
},
18+
})
19+
.start(done);
20+
});
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
2+
import * as Sentry from '@sentry/node';
3+
4+
Sentry.init({
5+
dsn: 'https://[email protected]/1337',
6+
release: '1.0',
7+
tracePropagationTargets: [/\/v0/, 'v1'],
8+
tracesSampleRate: 1,
9+
integrations: [],
10+
transport: loggingTransport,
11+
});
12+
13+
Sentry.startSpan(
14+
{
15+
name: 'GET /route/1',
16+
attributes: {
17+
'http.method': 'GET',
18+
'http.route': '/route',
19+
[Sentry.SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server',
20+
[Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
21+
},
22+
},
23+
() => {
24+
// noop
25+
},
26+
);
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import { createRunner } from '../../../../utils/runner';
2+
3+
test('envelope header for transaction event with source=url correct', done => {
4+
createRunner(__dirname, 'scenario.ts')
5+
.ignore('session', 'sessions')
6+
.expectHeader({
7+
transaction: {
8+
trace: {
9+
trace_id: expect.any(String),
10+
public_key: 'public',
11+
environment: 'production',
12+
release: '1.0',
13+
sample_rate: '1',
14+
sampled: 'true',
15+
},
16+
},
17+
})
18+
.start(done);
19+
});
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
2+
import * as Sentry from '@sentry/node';
3+
4+
Sentry.init({
5+
dsn: 'https://[email protected]/1337',
6+
release: '1.0',
7+
tracePropagationTargets: [/\/v0/, 'v1'],
8+
tracesSampleRate: 1,
9+
integrations: [],
10+
transport: loggingTransport,
11+
});
12+
13+
Sentry.startSpan({ name: 'test span' }, () => {
14+
// noop
15+
});
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { createRunner } from '../../../../utils/runner';
2+
3+
test('envelope header for transaction event is correct', done => {
4+
createRunner(__dirname, 'scenario.ts')
5+
.ignore('session', 'sessions')
6+
.expectHeader({
7+
transaction: {
8+
trace: {
9+
trace_id: expect.any(String),
10+
public_key: 'public',
11+
environment: 'production',
12+
release: '1.0',
13+
sample_rate: '1',
14+
sampled: 'true',
15+
transaction: 'test span',
16+
},
17+
},
18+
})
19+
.start(done);
20+
});

dev-packages/node-integration-tests/utils/runner.ts

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,15 @@
1+
/* eslint-disable max-lines */
12
import { spawn, spawnSync } from 'child_process';
23
import { join } from 'path';
3-
import type { Envelope, EnvelopeItemType, Event, SerializedSession, SessionAggregates } from '@sentry/types';
4+
import { SDK_VERSION } from '@sentry/node';
5+
import type {
6+
Envelope,
7+
EnvelopeItemType,
8+
Event,
9+
EventEnvelope,
10+
SerializedSession,
11+
SessionAggregates,
12+
} from '@sentry/types';
413
import axios from 'axios';
514
import { createBasicSentryServer } from './server';
615

@@ -29,6 +38,18 @@ export function assertSentryTransaction(actual: Event, expected: Partial<Event>)
2938
});
3039
}
3140

41+
export function assertEnvelopeHeader(actual: Envelope[0], expected: Partial<Envelope[0]>): void {
42+
expect(actual).toEqual({
43+
event_id: expect.any(String),
44+
sent_at: expect.any(String),
45+
sdk: {
46+
name: 'sentry.javascript.node',
47+
version: SDK_VERSION,
48+
},
49+
...expected,
50+
});
51+
}
52+
3253
const CLEANUP_STEPS = new Set<VoidFunction>();
3354

3455
export function cleanupChildProcesses(): void {
@@ -118,12 +139,19 @@ type Expected =
118139
sessions: Partial<SessionAggregates> | ((event: SessionAggregates) => void);
119140
};
120141

142+
type ExpectedEnvelopeHeader =
143+
| { event: Partial<EventEnvelope[0]> }
144+
| { transaction: Partial<Envelope[0]> }
145+
| { session: Partial<Envelope[0]> }
146+
| { sessions: Partial<Envelope[0]> };
147+
121148
/** Creates a test runner */
122149
// eslint-disable-next-line @typescript-eslint/explicit-function-return-type
123150
export function createRunner(...paths: string[]) {
124151
const testPath = join(...paths);
125152

126153
const expectedEnvelopes: Expected[] = [];
154+
let expectedEnvelopeHeaders: ExpectedEnvelopeHeader[] | undefined = undefined;
127155
const flags: string[] = [];
128156
const ignored: EnvelopeItemType[] = [];
129157
let withEnv: Record<string, string> = {};
@@ -141,6 +169,14 @@ export function createRunner(...paths: string[]) {
141169
expectedEnvelopes.push(expected);
142170
return this;
143171
},
172+
expectHeader: function (expected: ExpectedEnvelopeHeader) {
173+
if (!expectedEnvelopeHeaders) {
174+
expectedEnvelopeHeaders = [];
175+
}
176+
177+
expectedEnvelopeHeaders.push(expected);
178+
return this;
179+
},
144180
expectError: function () {
145181
expectError = true;
146182
return this;
@@ -170,7 +206,7 @@ export function createRunner(...paths: string[]) {
170206
return this;
171207
},
172208
start: function (done?: (e?: unknown) => void) {
173-
const expectedEnvelopeCount = expectedEnvelopes.length;
209+
const expectedEnvelopeCount = Math.max(expectedEnvelopes.length, (expectedEnvelopeHeaders || []).length);
174210

175211
let envelopeCount = 0;
176212
let scenarioServerPort: number | undefined;
@@ -198,6 +234,25 @@ export function createRunner(...paths: string[]) {
198234
continue;
199235
}
200236

237+
if (expectedEnvelopeHeaders) {
238+
const header = envelope[0];
239+
const expected = expectedEnvelopeHeaders.shift()?.[envelopeItemType as keyof ExpectedEnvelopeHeader];
240+
241+
try {
242+
if (!expected) {
243+
throw new Error(`No more expected envelope items but we received ${JSON.stringify(header)}`);
244+
}
245+
246+
assertEnvelopeHeader(header, expected);
247+
248+
expectCallbackCalled();
249+
} catch (e) {
250+
complete(e as Error);
251+
}
252+
253+
return;
254+
}
255+
201256
const expected = expectedEnvelopes.shift();
202257

203258
// Catch any error or failed assertions and pass them to done to end the test quickly

packages/opentelemetry/src/setupEventContextTrace.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,19 @@
11
import { getRootSpan } from '@sentry/core';
22
import type { Client } from '@sentry/types';
33
import { dropUndefinedKeys } from '@sentry/utils';
4+
import { getDynamicSamplingContextFromSpan } from './utils/dynamicSamplingContext';
45

56
import { getActiveSpan } from './utils/getActiveSpan';
67
import { spanHasName, spanHasParentId } from './utils/spanTypes';
78

89
/** Ensure the `trace` context is set on all events. */
910
export function setupEventContextTrace(client: Client): void {
10-
client.addEventProcessor(event => {
11+
client.on('preprocessEvent', event => {
1112
const span = getActiveSpan();
1213
// For transaction events, this is handled separately
1314
// Because the active span may not be the span that is actually the transaction event
1415
if (!span || event.type === 'transaction') {
15-
return event;
16+
return;
1617
}
1718

1819
const spanContext = span.spanContext();
@@ -27,12 +28,15 @@ export function setupEventContextTrace(client: Client): void {
2728
...event.contexts,
2829
};
2930

31+
event.sdkProcessingMetadata = {
32+
dynamicSamplingContext: getDynamicSamplingContextFromSpan(span),
33+
...event.sdkProcessingMetadata,
34+
};
35+
3036
const rootSpan = getRootSpan(span);
3137
const transactionName = spanHasName(rootSpan) ? rootSpan.name : undefined;
3238
if (transactionName && !event.transaction) {
3339
event.transaction = transactionName;
3440
}
35-
36-
return event;
3741
});
3842
}

0 commit comments

Comments
 (0)