Skip to content

Commit 1df660b

Browse files
authored
fix(browser): Ensure source is set correctly when updating span name in-place in beforeStartSpan (#17501)
When using `beforeStartSpan` in `browserTracingIntgration`, we document that users should return a new options object rather than mutating the passed in object in-place. However, nothing keeps users from setting the name in-place like so:' ```js browserTracingIntegration({ beforeStartSpan: opts => { opts.name = 'changed'; return opts; }, }), ``` In such a case, we previously didn't catch the mutation and wouldn't adjust the span source attribute to `'custom'` accordingly. This PR ensures we take care of this case, too. (found while writing a test for #17500)
1 parent 5d9e1bb commit 1df660b

File tree

2 files changed

+40
-2
lines changed

2 files changed

+40
-2
lines changed

packages/browser/src/tracing/browserTracingIntegration.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
356356
function _createRouteSpan(client: Client, startSpanOptions: StartSpanOptions, makeActive = true): void {
357357
const isPageloadTransaction = startSpanOptions.op === 'pageload';
358358

359+
const initialSpanName = startSpanOptions.name;
359360
const finalStartSpanOptions: StartSpanOptions = beforeStartSpan
360361
? beforeStartSpan(startSpanOptions)
361362
: startSpanOptions;
@@ -364,7 +365,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
364365

365366
// If `finalStartSpanOptions.name` is different than `startSpanOptions.name`
366367
// it is because `beforeStartSpan` set a custom name. Therefore we set the source to 'custom'.
367-
if (startSpanOptions.name !== finalStartSpanOptions.name) {
368+
if (initialSpanName !== finalStartSpanOptions.name) {
368369
attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] = 'custom';
369370
finalStartSpanOptions.attributes = attributes;
370371
}

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

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,44 @@ describe('browserTracingIntegration', () => {
446446
setCurrentClient(client);
447447
client.init();
448448

449-
startBrowserTracingPageLoadSpan(client, { name: 'test span' });
449+
startBrowserTracingPageLoadSpan(client, {
450+
name: 'test span',
451+
attributes: {
452+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
453+
},
454+
});
455+
456+
const pageloadSpan = getActiveSpan();
457+
458+
expect(spanToJSON(pageloadSpan!).description).toBe('changed');
459+
expect(spanToJSON(pageloadSpan!).data[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]).toBe('custom');
460+
});
461+
462+
it('sets source to "custom" if name is changed in-place in beforeStartSpan', () => {
463+
const client = new BrowserClient(
464+
getDefaultBrowserClientOptions({
465+
tracesSampleRate: 0,
466+
integrations: [
467+
browserTracingIntegration({
468+
instrumentPageLoad: false,
469+
instrumentNavigation: false,
470+
beforeStartSpan: opts => {
471+
opts.name = 'changed';
472+
return opts;
473+
},
474+
}),
475+
],
476+
}),
477+
);
478+
setCurrentClient(client);
479+
client.init();
480+
481+
startBrowserTracingPageLoadSpan(client, {
482+
name: 'test span',
483+
attributes: {
484+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
485+
},
486+
});
450487

451488
const pageloadSpan = getActiveSpan();
452489

0 commit comments

Comments
 (0)