Skip to content

Commit

Permalink
ref(browser): Improve active span handling for `browserTracingIntegra…
Browse files Browse the repository at this point in the history
…tion` (#14959)

Extracted this out of
#14955

We used to rely on implied stuff here quite a bit, which breaks if we
start returning non recording spans. Honestly this just surfaces that
this is not really ideal as it is 😬 We already pass the client around
there everywhere, so this PR updates this so we keep the active idle
span as non-enumerable prop on the client, ensuring this is consistent
and "pure".
  • Loading branch information
mydea authored Jan 10, 2025
1 parent b98d341 commit 1766d4c
Showing 1 changed file with 25 additions and 19 deletions.
44 changes: 25 additions & 19 deletions packages/browser/src/tracing/browserTracingIntegration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ import {
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
TRACING_DEFAULTS,
addNonEnumerableProperty,
browserPerformanceTimeOrigin,
generateTraceId,
getActiveSpan,
getClient,
getCurrentScope,
getDynamicSamplingContextFromSpan,
Expand Down Expand Up @@ -247,7 +247,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
};

/** Create routing idle transaction. */
function _createRouteSpan(client: Client, startSpanOptions: StartSpanOptions): Span {
function _createRouteSpan(client: Client, startSpanOptions: StartSpanOptions): void {
const isPageloadTransaction = startSpanOptions.op === 'pageload';

const finalStartSpanOptions: StartSpanOptions = beforeStartSpan
Expand Down Expand Up @@ -275,8 +275,10 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
beforeSpanEnd: span => {
_collectWebVitals();
addPerformanceEntries(span, { recordClsOnPageloadSpan: !enableStandaloneClsSpans });
setActiveIdleSpan(client, undefined);
},
});
setActiveIdleSpan(client, idleSpan);

function emitFinish(): void {
if (optionalWindowDocument && ['interactive', 'complete'].includes(optionalWindowDocument.readyState)) {
Expand All @@ -291,17 +293,16 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio

emitFinish();
}

return idleSpan;
}

return {
name: BROWSER_TRACING_INTEGRATION_ID,
afterAllSetup(client) {
let activeSpan: Span | undefined;
let startingUrl: string | undefined = getLocationHref();

function maybeEndActiveSpan(): void {
const activeSpan = getActiveIdleSpan(client);

if (activeSpan && !spanToJSON(activeSpan).timestamp) {
DEBUG_BUILD && logger.log(`[Tracing] Finishing current active span with op: ${spanToJSON(activeSpan).op}`);
// If there's an open active span, we need to finish it before creating an new one.
Expand All @@ -316,7 +317,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio

maybeEndActiveSpan();

activeSpan = _createRouteSpan(client, {
_createRouteSpan(client, {
op: 'navigation',
...startSpanOptions,
});
Expand All @@ -334,7 +335,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
const propagationContext = propagationContextFromHeaders(sentryTrace, baggage);
getCurrentScope().setPropagationContext(propagationContext);

activeSpan = _createRouteSpan(client, {
_createRouteSpan(client, {
op: 'pageload',
...startSpanOptions,
});
Expand Down Expand Up @@ -409,7 +410,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
}

if (enableInteractions) {
registerInteractionListener(idleTimeout, finalTimeout, childSpanTimeout, latestRoute);
registerInteractionListener(client, idleTimeout, finalTimeout, childSpanTimeout, latestRoute);
}

if (enableInp) {
Expand Down Expand Up @@ -441,12 +442,9 @@ export function startBrowserTracingPageLoadSpan(
traceOptions?: { sentryTrace?: string | undefined; baggage?: string | undefined },
): Span | undefined {
client.emit('startPageLoadSpan', spanOptions, traceOptions);

getCurrentScope().setTransactionName(spanOptions.name);

const span = getActiveSpan();
const op = span && spanToJSON(span).op;
return op === 'pageload' ? span : undefined;
return getActiveIdleSpan(client);
}

/**
Expand All @@ -461,9 +459,7 @@ export function startBrowserTracingNavigationSpan(client: Client, spanOptions: S

getCurrentScope().setTransactionName(spanOptions.name);

const span = getActiveSpan();
const op = span && spanToJSON(span).op;
return op === 'navigation' ? span : undefined;
return getActiveIdleSpan(client);
}

/** Returns the value of a meta tag */
Expand All @@ -480,6 +476,7 @@ export function getMetaContent(metaName: string): string | undefined {

/** Start listener for interaction transactions */
function registerInteractionListener(
client: Client,
idleTimeout: BrowserTracingOptions['idleTimeout'],
finalTimeout: BrowserTracingOptions['finalTimeout'],
childSpanTimeout: BrowserTracingOptions['childSpanTimeout'],
Expand All @@ -495,10 +492,9 @@ function registerInteractionListener(
const registerInteractionTransaction = (): void => {
const op = 'ui.action.click';

const activeSpan = getActiveSpan();
const rootSpan = activeSpan && getRootSpan(activeSpan);
if (rootSpan) {
const currentRootSpanOp = spanToJSON(rootSpan).op;
const activeIdleSpan = getActiveIdleSpan(client);
if (activeIdleSpan) {
const currentRootSpanOp = spanToJSON(activeIdleSpan).op;
if (['navigation', 'pageload'].includes(currentRootSpanOp as string)) {
DEBUG_BUILD &&
logger.warn(`[Tracing] Did not create ${op} span because a pageload or navigation span is in progress.`);
Expand Down Expand Up @@ -537,3 +533,13 @@ function registerInteractionListener(
addEventListener('click', registerInteractionTransaction, { once: false, capture: true });
}
}

// We store the active idle span on the client object, so we can access it from exported functions
const ACTIVE_IDLE_SPAN_PROPERTY = '_sentry_idleSpan';
function getActiveIdleSpan(client: Client): Span | undefined {
return (client as { [ACTIVE_IDLE_SPAN_PROPERTY]?: Span })[ACTIVE_IDLE_SPAN_PROPERTY];
}

function setActiveIdleSpan(client: Client, span: Span | undefined): void {
addNonEnumerableProperty(client, ACTIVE_IDLE_SPAN_PROPERTY, span);
}

0 comments on commit 1766d4c

Please sign in to comment.