Skip to content

Commit bee0677

Browse files
authored
fix(browser): Set custom sentry source correctly (#11735)
The primary change is to make `finalStartSpanOptions.name !== finalStartSpanOptions.name` -> `startSpanOptions.name !== finalStartSpanOptions.name` in `packages/browser/src/tracing/browserTracingIntegration.ts`. While I was here though I cleaned up some other code to improve bundle size. Let me know if you want me to extract the cleanup changes, I figured it's all easier to review here with the same context.
1 parent d874b3c commit bee0677

File tree

1 file changed

+89
-79
lines changed

1 file changed

+89
-79
lines changed

packages/browser/src/tracing/browserTracingIntegration.ts

Lines changed: 89 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -171,21 +171,36 @@ const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = {
171171
export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptions> = {}) => {
172172
registerSpanErrorInstrumentation();
173173

174-
const options = {
174+
const {
175+
enableInp,
176+
enableLongTask,
177+
_experiments: { enableInteractions },
178+
beforeStartSpan,
179+
idleTimeout,
180+
finalTimeout,
181+
childSpanTimeout,
182+
markBackgroundSpan,
183+
traceFetch,
184+
traceXHR,
185+
shouldCreateSpanForRequest,
186+
enableHTTPTimings,
187+
instrumentPageLoad,
188+
instrumentNavigation,
189+
} = {
175190
...DEFAULT_BROWSER_TRACING_OPTIONS,
176191
..._options,
177192
};
178193

179194
const _collectWebVitals = startTrackingWebVitals();
180195

181-
if (options.enableInp) {
196+
if (enableInp) {
182197
startTrackingINP();
183198
}
184199

185-
if (options.enableLongTask) {
200+
if (enableLongTask) {
186201
startTrackingLongTasks();
187202
}
188-
if (options._experiments.enableInteractions) {
203+
if (enableInteractions) {
189204
startTrackingInteractions();
190205
}
191206

@@ -196,18 +211,17 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
196211

197212
/** Create routing idle transaction. */
198213
function _createRouteSpan(client: Client, startSpanOptions: StartSpanOptions): Span {
199-
const { beforeStartSpan, idleTimeout, finalTimeout, childSpanTimeout } = options;
200-
201214
const isPageloadTransaction = startSpanOptions.op === 'pageload';
202215

203216
const finalStartSpanOptions: StartSpanOptions = beforeStartSpan
204217
? beforeStartSpan(startSpanOptions)
205218
: startSpanOptions;
206219

207-
// If `beforeStartSpan` set a custom name, record that fact
208220
const attributes = finalStartSpanOptions.attributes || {};
209221

210-
if (finalStartSpanOptions.name !== finalStartSpanOptions.name) {
222+
// If `finalStartSpanOptions.name` is different than `startSpanOptions.name`
223+
// it is because `beforeStartSpan` set a custom name. Therefore we set the source to 'custom'.
224+
if (startSpanOptions.name !== finalStartSpanOptions.name) {
211225
attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] = 'custom';
212226
finalStartSpanOptions.attributes = attributes;
213227
}
@@ -227,16 +241,18 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
227241
},
228242
});
229243

244+
function emitFinish(): void {
245+
if (['interactive', 'complete'].includes(WINDOW.document.readyState)) {
246+
client.emit('idleSpanEnableAutoFinish', idleSpan);
247+
}
248+
}
249+
230250
if (isPageloadTransaction && WINDOW.document) {
231251
WINDOW.document.addEventListener('readystatechange', () => {
232-
if (['interactive', 'complete'].includes(WINDOW.document.readyState)) {
233-
client.emit('idleSpanEnableAutoFinish', idleSpan);
234-
}
252+
emitFinish();
235253
});
236254

237-
if (['interactive', 'complete'].includes(WINDOW.document.readyState)) {
238-
client.emit('idleSpanEnableAutoFinish', idleSpan);
239-
}
255+
emitFinish();
240256
}
241257

242258
return idleSpan;
@@ -245,9 +261,6 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
245261
return {
246262
name: BROWSER_TRACING_INTEGRATION_ID,
247263
afterAllSetup(client) {
248-
const { markBackgroundSpan, traceFetch, traceXHR, shouldCreateSpanForRequest, enableHTTPTimings, _experiments } =
249-
options;
250-
251264
let activeSpan: Span | undefined;
252265
let startingUrl: string | undefined = WINDOW.location && WINDOW.location.href;
253266

@@ -304,65 +317,62 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
304317
const scope = getCurrentScope();
305318
const oldPropagationContext = scope.getPropagationContext();
306319

307-
const newPropagationContext = {
320+
scope.setPropagationContext({
308321
...oldPropagationContext,
309322
sampled: oldPropagationContext.sampled !== undefined ? oldPropagationContext.sampled : spanIsSampled(span),
310323
dsc: oldPropagationContext.dsc || getDynamicSamplingContextFromSpan(span),
311-
};
312-
313-
scope.setPropagationContext(newPropagationContext);
324+
});
314325
});
315326

316-
if (options.instrumentPageLoad && WINDOW.location) {
317-
const startSpanOptions: StartSpanOptions = {
318-
name: WINDOW.location.pathname,
319-
// pageload should always start at timeOrigin (and needs to be in s, not ms)
320-
startTime: browserPerformanceTimeOrigin ? browserPerformanceTimeOrigin / 1000 : undefined,
321-
attributes: {
322-
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
323-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser',
324-
},
325-
};
326-
startBrowserTracingPageLoadSpan(client, startSpanOptions);
327-
}
327+
if (WINDOW.location) {
328+
if (instrumentPageLoad) {
329+
startBrowserTracingPageLoadSpan(client, {
330+
name: WINDOW.location.pathname,
331+
// pageload should always start at timeOrigin (and needs to be in s, not ms)
332+
startTime: browserPerformanceTimeOrigin ? browserPerformanceTimeOrigin / 1000 : undefined,
333+
attributes: {
334+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
335+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser',
336+
},
337+
});
338+
}
328339

329-
if (options.instrumentNavigation && WINDOW.location) {
330-
addHistoryInstrumentationHandler(({ to, from }) => {
331-
/**
332-
* This early return is there to account for some cases where a navigation transaction starts right after
333-
* long-running pageload. We make sure that if `from` is undefined and a valid `startingURL` exists, we don't
334-
* create an uneccessary navigation transaction.
335-
*
336-
* This was hard to duplicate, but this behavior stopped as soon as this fix was applied. This issue might also
337-
* only be caused in certain development environments where the usage of a hot module reloader is causing
338-
* errors.
339-
*/
340-
if (from === undefined && startingUrl && startingUrl.indexOf(to) !== -1) {
341-
startingUrl = undefined;
342-
return;
343-
}
344-
345-
if (from !== to) {
346-
startingUrl = undefined;
347-
const startSpanOptions: StartSpanOptions = {
348-
name: WINDOW.location.pathname,
349-
attributes: {
350-
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
351-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser',
352-
},
353-
};
354-
355-
startBrowserTracingNavigationSpan(client, startSpanOptions);
356-
}
357-
});
340+
if (instrumentNavigation) {
341+
addHistoryInstrumentationHandler(({ to, from }) => {
342+
/**
343+
* This early return is there to account for some cases where a navigation transaction starts right after
344+
* long-running pageload. We make sure that if `from` is undefined and a valid `startingURL` exists, we don't
345+
* create an uneccessary navigation transaction.
346+
*
347+
* This was hard to duplicate, but this behavior stopped as soon as this fix was applied. This issue might also
348+
* only be caused in certain development environments where the usage of a hot module reloader is causing
349+
* errors.
350+
*/
351+
if (from === undefined && startingUrl && startingUrl.indexOf(to) !== -1) {
352+
startingUrl = undefined;
353+
return;
354+
}
355+
356+
if (from !== to) {
357+
startingUrl = undefined;
358+
startBrowserTracingNavigationSpan(client, {
359+
name: WINDOW.location.pathname,
360+
attributes: {
361+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
362+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser',
363+
},
364+
});
365+
}
366+
});
367+
}
358368
}
359369

360370
if (markBackgroundSpan) {
361371
registerBackgroundTabDetection();
362372
}
363373

364-
if (_experiments.enableInteractions) {
365-
registerInteractionListener(options, latestRoute);
374+
if (enableInteractions) {
375+
registerInteractionListener(idleTimeout, finalTimeout, childSpanTimeout, latestRoute);
366376
}
367377

368378
instrumentOutgoingRequests({
@@ -402,14 +412,8 @@ export function startBrowserTracingPageLoadSpan(
402412
* This will only do something if a browser tracing integration has been setup.
403413
*/
404414
export function startBrowserTracingNavigationSpan(client: Client, spanOptions: StartSpanOptions): Span | undefined {
405-
getCurrentScope().setPropagationContext({
406-
traceId: uuid4(),
407-
spanId: uuid4().substring(16),
408-
});
409-
getIsolationScope().setPropagationContext({
410-
traceId: uuid4(),
411-
spanId: uuid4().substring(16),
412-
});
415+
getCurrentScope().setPropagationContext(generatePropagationContext());
416+
getIsolationScope().setPropagationContext(generatePropagationContext());
413417

414418
client.emit('startNavigationSpan', spanOptions);
415419

@@ -432,12 +436,13 @@ export function getMetaContent(metaName: string): string | undefined {
432436

433437
/** Start listener for interaction transactions */
434438
function registerInteractionListener(
435-
options: BrowserTracingOptions,
439+
idleTimeout: BrowserTracingOptions['idleTimeout'],
440+
finalTimeout: BrowserTracingOptions['finalTimeout'],
441+
childSpanTimeout: BrowserTracingOptions['childSpanTimeout'],
436442
latestRoute: { name: string | undefined; source: TransactionSource | undefined },
437443
): void {
438444
let inflightInteractionSpan: Span | undefined;
439445
const registerInteractionTransaction = (): void => {
440-
const { idleTimeout, finalTimeout, childSpanTimeout } = options;
441446
const op = 'ui.action.click';
442447

443448
const activeSpan = getActiveSpan();
@@ -478,9 +483,14 @@ function registerInteractionListener(
478483
);
479484
};
480485

481-
['click'].forEach(type => {
482-
if (WINDOW.document) {
483-
addEventListener(type, registerInteractionTransaction, { once: false, capture: true });
484-
}
485-
});
486+
if (WINDOW.document) {
487+
addEventListener('click', registerInteractionTransaction, { once: false, capture: true });
488+
}
489+
}
490+
491+
function generatePropagationContext(): { traceId: string; spanId: string } {
492+
return {
493+
traceId: uuid4(),
494+
spanId: uuid4().substring(16),
495+
};
486496
}

0 commit comments

Comments
 (0)