Skip to content

Commit c273167

Browse files
committed
fix(core): Fix withStreamedSpan typing error add missing exports (#20124)
This PR makes some changes to the span streaming `beforeSendSpan` implementation: Previously `beforeSendSpan` was typed via discriminated union. While I initially thought this would work, it would break type checking for user-provided static `beforeSendSpan` callbacks because TS couldn't infer if the callback received a `SpanJSON` or a `StreamedSpanJSON`. So it defaulted to `unknown`. Now, we "pretend" that every `beforeSendSpan` callback receives and returns a `SpanJSON`. Since we instruct users for span streaming to use the `withStreamedSpan` helper anyway, they will get correct typing within the wrapper function (i.e. a callback receiving and returning `StreamedSpanJSON`. Other changes: - added missing exports for the `withStreamedSpan` helper from every SDK - added integration tests to demonstrate it being used - downleveled the debug log from `warn` to `log` that the browser `spanStreamingIntegration` automatically sets `traceLifecycle: 'stream'`. This is expected behaviour and we'll instruct users to only set `spanStreamingIntegration` to avoid a double-opt-in situation.
1 parent b6f7b86 commit c273167

File tree

36 files changed

+188
-11
lines changed

36 files changed

+188
-11
lines changed
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
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: [Sentry.browserTracingIntegration(), Sentry.spanStreamingIntegration()],
8+
tracesSampleRate: 1,
9+
beforeSendSpan: Sentry.withStreamedSpan(span => {
10+
if (span.attributes['sentry.op'] === 'pageload') {
11+
span.name = 'customPageloadSpanName';
12+
span.links = [
13+
{
14+
context: {
15+
traceId: '123',
16+
spanId: '456',
17+
},
18+
attributes: {
19+
'sentry.link.type': 'custom_link',
20+
},
21+
},
22+
];
23+
span.attributes['sentry.custom_attribute'] = 'customAttributeValue';
24+
span.status = 'something';
25+
}
26+
return span;
27+
}),
28+
});
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 { sentryTest } from '../../../utils/fixtures';
3+
import { shouldSkipTracingTest, testingCdnBundle } from '../../../utils/helpers';
4+
import { getSpanOp, waitForStreamedSpan } from '../../../utils/spanUtils';
5+
6+
sentryTest('beforeSendSpan applies changes to streamed span', async ({ getLocalTestUrl, page }) => {
7+
sentryTest.skip(shouldSkipTracingTest() || testingCdnBundle());
8+
9+
const url = await getLocalTestUrl({ testDir: __dirname });
10+
11+
const pageloadSpanPromise = waitForStreamedSpan(page, span => getSpanOp(span) === 'pageload');
12+
13+
await page.goto(url);
14+
15+
const pageloadSpan = await pageloadSpanPromise;
16+
17+
expect(pageloadSpan.name).toBe('customPageloadSpanName');
18+
expect(pageloadSpan.links).toEqual([
19+
{
20+
context: {
21+
traceId: '123',
22+
spanId: '456',
23+
},
24+
attributes: {
25+
'sentry.link.type': { type: 'string', value: 'custom_link' },
26+
},
27+
},
28+
]);
29+
expect(pageloadSpan.attributes?.['sentry.custom_attribute']).toEqual({
30+
type: 'string',
31+
value: 'customAttributeValue',
32+
});
33+
// we allow overriding any kinds of fields on the span, so we have to expect invalid values
34+
expect(pageloadSpan.status).toBe('something');
35+
});
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import * as Sentry from '@sentry/node-core';
2+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
3+
import { setupOtel } from '../../../utils/setupOtel';
4+
5+
const client = Sentry.init({
6+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
7+
tracesSampleRate: 1.0,
8+
traceLifecycle: 'stream',
9+
transport: loggingTransport,
10+
release: '1.0.0',
11+
beforeSendSpan: Sentry.withStreamedSpan(span => {
12+
if (span.name === 'test-child-span') {
13+
span.name = 'customChildSpanName';
14+
if (!span.attributes) {
15+
span.attributes = {};
16+
}
17+
span.attributes['sentry.custom_attribute'] = 'customAttributeValue';
18+
// @ts-ignore - technically this is something we have to expect, despite types saying it's invalid
19+
span.status = 'something';
20+
span.links = [
21+
{
22+
trace_id: '123',
23+
span_id: '456',
24+
attributes: {
25+
'sentry.link.type': 'custom_link',
26+
},
27+
},
28+
];
29+
}
30+
return span;
31+
}),
32+
});
33+
34+
setupOtel(client);
35+
36+
Sentry.startSpan({ name: 'test-span', op: 'test' }, () => {
37+
Sentry.startSpan({ name: 'test-child-span', op: 'test-child' }, () => {
38+
// noop
39+
});
40+
});
41+
42+
void Sentry.flush();
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import { expect, test } from 'vitest';
2+
import { createRunner } from '../../../utils/runner';
3+
4+
test('beforeSendSpan applies changes to streamed span', async () => {
5+
await createRunner(__dirname, 'scenario.ts')
6+
.expect({
7+
span: container => {
8+
const spans = container.items;
9+
expect(spans.length).toBe(2);
10+
11+
const customChildSpan = spans.find(s => s.name === 'customChildSpanName');
12+
13+
expect(customChildSpan).toBeDefined();
14+
expect(customChildSpan!.attributes?.['sentry.custom_attribute']).toEqual({
15+
type: 'string',
16+
value: 'customAttributeValue',
17+
});
18+
expect(customChildSpan!.status).toBe('something');
19+
expect(customChildSpan!.links).toEqual([
20+
{
21+
trace_id: '123',
22+
span_id: '456',
23+
attributes: {
24+
'sentry.link.type': { type: 'string', value: 'custom_link' },
25+
},
26+
},
27+
]);
28+
},
29+
})
30+
.start()
31+
.completed();
32+
});

dev-packages/node-integration-tests/suites/tracing/ignoreSpans-streamed/children/instrument.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,6 @@ Sentry.init({
77
tracesSampleRate: 1.0,
88
transport: loggingTransport,
99
traceLifecycle: 'stream',
10-
ignoreSpans: ['middleware - expressInit', /custom-to-drop/, { op: 'ignored-op' }],
10+
ignoreSpans: ['expressInit', /custom-to-drop/, { op: 'ignored-op' }],
1111
clientReportFlushInterval: 1_000,
1212
});

dev-packages/node-integration-tests/suites/tracing/ignoreSpans-streamed/children/server.mjs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@ app.get('/test/express', (_req, res) => {
3838
() => {},
3939
);
4040
res.send({ response: 'response 1' });
41+
42+
setTimeout(() => {
43+
// flush to avoid waiting for the span buffer timeout to send spans
44+
// but defer it to the next tick to let the SDK finish the http.server span first.
45+
Sentry.flush();
46+
});
4147
});
4248

4349
Sentry.setupExpressErrorHandler(app);

dev-packages/node-integration-tests/suites/tracing/ignoreSpans-streamed/segments/server.mjs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@ app.get('/health', (_req, res) => {
1313

1414
app.get('/ok', (_req, res) => {
1515
res.send({ status: 'ok' });
16+
setTimeout(() => {
17+
// flush to avoid waiting for the span buffer timeout to send spans
18+
// but defer it to the next tick to let the SDK finish the http.server span first.
19+
Sentry.flush();
20+
});
1621
});
1722

1823
Sentry.setupExpressErrorHandler(app);

packages/astro/src/index.server.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ export {
174174
unleashIntegration,
175175
growthbookIntegration,
176176
spanStreamingIntegration,
177+
withStreamedSpan,
177178
metrics,
178179
} from '@sentry/node';
179180

packages/astro/src/index.types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ export declare function init(options: Options | clientSdk.BrowserOptions | NodeO
2121
export declare const linkedErrorsIntegration: typeof clientSdk.linkedErrorsIntegration;
2222
export declare const contextLinesIntegration: typeof clientSdk.contextLinesIntegration;
2323
export declare const spanStreamingIntegration: typeof clientSdk.spanStreamingIntegration;
24+
export declare const withStreamedSpan: typeof clientSdk.withStreamedSpan;
2425

2526
export declare const getDefaultIntegrations: (options: Options) => Integration[];
2627
export declare const defaultStackParser: StackParser;

packages/aws-serverless/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ export {
161161
growthbookIntegration,
162162
metrics,
163163
spanStreamingIntegration,
164+
withStreamedSpan,
164165
} from '@sentry/node';
165166

166167
export {

0 commit comments

Comments
 (0)