Skip to content

fix(browser): Set custom sentry source correctly #11735

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 23, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
168 changes: 89 additions & 79 deletions packages/browser/src/tracing/browserTracingIntegration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,21 +171,36 @@ const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = {
export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptions> = {}) => {
registerSpanErrorInstrumentation();

const options = {
const {
enableInp,
enableLongTask,
_experiments: { enableInteractions },
beforeStartSpan,
idleTimeout,
finalTimeout,
childSpanTimeout,
markBackgroundSpan,
traceFetch,
traceXHR,
shouldCreateSpanForRequest,
enableHTTPTimings,
instrumentPageLoad,
instrumentNavigation,
} = {
...DEFAULT_BROWSER_TRACING_OPTIONS,
..._options,
};

const _collectWebVitals = startTrackingWebVitals();

if (options.enableInp) {
if (enableInp) {
startTrackingINP();
}

if (options.enableLongTask) {
if (enableLongTask) {
startTrackingLongTasks();
}
if (options._experiments.enableInteractions) {
if (enableInteractions) {
startTrackingInteractions();
}

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

/** Create routing idle transaction. */
function _createRouteSpan(client: Client, startSpanOptions: StartSpanOptions): Span {
const { beforeStartSpan, idleTimeout, finalTimeout, childSpanTimeout } = options;

const isPageloadTransaction = startSpanOptions.op === 'pageload';

const finalStartSpanOptions: StartSpanOptions = beforeStartSpan
? beforeStartSpan(startSpanOptions)
: startSpanOptions;

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

if (finalStartSpanOptions.name !== finalStartSpanOptions.name) {
// If `finalStartSpanOptions.name` is different than `startSpanOptions.name`
// it is because `beforeStartSpan` set a custom name. Therefore we set the source to 'custom'.
if (startSpanOptions.name !== finalStartSpanOptions.name) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the change. I'm not sure how the tests were passing 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OMG 🤯

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we never tested starting a pageload/navigation txn with beforeStartSpan 🤔

attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] = 'custom';
finalStartSpanOptions.attributes = attributes;
}
Expand All @@ -227,16 +241,18 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
},
});

function emitFinish(): void {
if (['interactive', 'complete'].includes(WINDOW.document.readyState)) {
client.emit('idleSpanEnableAutoFinish', idleSpan);
}
}

if (isPageloadTransaction && WINDOW.document) {
WINDOW.document.addEventListener('readystatechange', () => {
if (['interactive', 'complete'].includes(WINDOW.document.readyState)) {
client.emit('idleSpanEnableAutoFinish', idleSpan);
}
emitFinish();
});

if (['interactive', 'complete'].includes(WINDOW.document.readyState)) {
client.emit('idleSpanEnableAutoFinish', idleSpan);
}
emitFinish();
}

return idleSpan;
Expand All @@ -245,9 +261,6 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
return {
name: BROWSER_TRACING_INTEGRATION_ID,
afterAllSetup(client) {
const { markBackgroundSpan, traceFetch, traceXHR, shouldCreateSpanForRequest, enableHTTPTimings, _experiments } =
options;

let activeSpan: Span | undefined;
let startingUrl: string | undefined = WINDOW.location && WINDOW.location.href;

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

const newPropagationContext = {
scope.setPropagationContext({
...oldPropagationContext,
sampled: oldPropagationContext.sampled !== undefined ? oldPropagationContext.sampled : spanIsSampled(span),
dsc: oldPropagationContext.dsc || getDynamicSamplingContextFromSpan(span),
};

scope.setPropagationContext(newPropagationContext);
});
});

if (options.instrumentPageLoad && WINDOW.location) {
const startSpanOptions: StartSpanOptions = {
name: WINDOW.location.pathname,
// pageload should always start at timeOrigin (and needs to be in s, not ms)
startTime: browserPerformanceTimeOrigin ? browserPerformanceTimeOrigin / 1000 : undefined,
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser',
},
};
startBrowserTracingPageLoadSpan(client, startSpanOptions);
}
if (WINDOW.location) {
if (instrumentPageLoad) {
startBrowserTracingPageLoadSpan(client, {
name: WINDOW.location.pathname,
// pageload should always start at timeOrigin (and needs to be in s, not ms)
startTime: browserPerformanceTimeOrigin ? browserPerformanceTimeOrigin / 1000 : undefined,
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser',
},
});
}

if (options.instrumentNavigation && WINDOW.location) {
addHistoryInstrumentationHandler(({ to, from }) => {
/**
* This early return is there to account for some cases where a navigation transaction starts right after
* long-running pageload. We make sure that if `from` is undefined and a valid `startingURL` exists, we don't
* create an uneccessary navigation transaction.
*
* This was hard to duplicate, but this behavior stopped as soon as this fix was applied. This issue might also
* only be caused in certain development environments where the usage of a hot module reloader is causing
* errors.
*/
if (from === undefined && startingUrl && startingUrl.indexOf(to) !== -1) {
startingUrl = undefined;
return;
}

if (from !== to) {
startingUrl = undefined;
const startSpanOptions: StartSpanOptions = {
name: WINDOW.location.pathname,
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser',
},
};

startBrowserTracingNavigationSpan(client, startSpanOptions);
}
});
if (instrumentNavigation) {
addHistoryInstrumentationHandler(({ to, from }) => {
/**
* This early return is there to account for some cases where a navigation transaction starts right after
* long-running pageload. We make sure that if `from` is undefined and a valid `startingURL` exists, we don't
* create an uneccessary navigation transaction.
*
* This was hard to duplicate, but this behavior stopped as soon as this fix was applied. This issue might also
* only be caused in certain development environments where the usage of a hot module reloader is causing
* errors.
*/
if (from === undefined && startingUrl && startingUrl.indexOf(to) !== -1) {
startingUrl = undefined;
return;
}

if (from !== to) {
startingUrl = undefined;
startBrowserTracingNavigationSpan(client, {
name: WINDOW.location.pathname,
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser',
},
});
}
});
}
}

if (markBackgroundSpan) {
registerBackgroundTabDetection();
}

if (_experiments.enableInteractions) {
registerInteractionListener(options, latestRoute);
if (enableInteractions) {
registerInteractionListener(idleTimeout, finalTimeout, childSpanTimeout, latestRoute);
}

instrumentOutgoingRequests({
Expand Down Expand Up @@ -402,14 +412,8 @@ export function startBrowserTracingPageLoadSpan(
* This will only do something if a browser tracing integration has been setup.
*/
export function startBrowserTracingNavigationSpan(client: Client, spanOptions: StartSpanOptions): Span | undefined {
getCurrentScope().setPropagationContext({
traceId: uuid4(),
spanId: uuid4().substring(16),
});
getIsolationScope().setPropagationContext({
traceId: uuid4(),
spanId: uuid4().substring(16),
});
getCurrentScope().setPropagationContext(generatePropagationContext());
getIsolationScope().setPropagationContext(generatePropagationContext());

client.emit('startNavigationSpan', spanOptions);

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

/** Start listener for interaction transactions */
function registerInteractionListener(
options: BrowserTracingOptions,
idleTimeout: BrowserTracingOptions['idleTimeout'],
finalTimeout: BrowserTracingOptions['finalTimeout'],
childSpanTimeout: BrowserTracingOptions['childSpanTimeout'],
latestRoute: { name: string | undefined; source: TransactionSource | undefined },
): void {
let inflightInteractionSpan: Span | undefined;
const registerInteractionTransaction = (): void => {
const { idleTimeout, finalTimeout, childSpanTimeout } = options;
const op = 'ui.action.click';

const activeSpan = getActiveSpan();
Expand Down Expand Up @@ -478,9 +483,14 @@ function registerInteractionListener(
);
};

['click'].forEach(type => {
if (WINDOW.document) {
addEventListener(type, registerInteractionTransaction, { once: false, capture: true });
}
});
if (WINDOW.document) {
addEventListener('click', registerInteractionTransaction, { once: false, capture: true });
}
}

function generatePropagationContext(): { traceId: string; spanId: string } {
return {
traceId: uuid4(),
spanId: uuid4().substring(16),
};
}
Loading