Skip to content

ref(tracing): Implement changes to new browserTracingIntegration #10393

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
Jan 29, 2024
Merged
Show file tree
Hide file tree
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
2 changes: 0 additions & 2 deletions packages/browser/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,6 @@ export {
browserTracingIntegration,
startBrowserTracingNavigationSpan,
startBrowserTracingPageLoadSpan,
disableDefaultBrowserTracingNavigationSpan,
disableDefaultBrowserTracingPageLoadSpan,
} from '@sentry-internal/tracing';
export type { RequestInstrumentationOptions } from '@sentry-internal/tracing';
export {
Expand Down
58 changes: 11 additions & 47 deletions packages/tracing-internal/src/browser/browserTracingIntegration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,14 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions {

/**
* If a span should be created on page load.
* If this is set to `false`, this integration will not start the default page load span.
* Default: true
*/
instrumentPageLoad: boolean;

/**
* If a span should be created on navigation (history change).
* If this is set to `false`, this integration will not start the default navigation spans.
* Default: true
*/
instrumentNavigation: boolean;
Expand Down Expand Up @@ -144,9 +146,6 @@ const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = {
...defaultRequestInstrumentationOptions,
};

let shouldUseDefaultPageLoadSpan = true;
let shouldUseDefaultNavigationSpan = true;

/**
* The Browser Tracing integration automatically instruments browser pageload/navigation
* actions as transactions, and captures requests, metrics and errors as spans.
Expand Down Expand Up @@ -323,13 +322,6 @@ export const _browserTracingIntegration = ((_options: Partial<BrowserTracingOpti

if (client.on) {
client.on('startNavigationSpan', (context: StartSpanOptions) => {
// We check this inside of the hook handler, so that if a custom instrumentation triggers this,
// we don't need to check this option in the instrumentation, but can simply invoke it
// without needing to know the options of this integration
if (!options.instrumentNavigation) {
return;
}

if (activeSpan) {
DEBUG_BUILD && logger.log(`[Tracing] Finishing current transaction with op: ${spanToJSON(activeSpan).op}`);
// If there's an open transaction on the scope, we need to finish it before creating an new one.
Expand All @@ -339,13 +331,6 @@ export const _browserTracingIntegration = ((_options: Partial<BrowserTracingOpti
});

client.on('startPageLoadSpan', (context: StartSpanOptions) => {
// We check this inside of the hook handler, so that if a custom instrumentation triggers this,
// we don't need to check this option in the instrumentation, but can simply invoke it
// without needing to know the options of this integration
if (!options.instrumentPageLoad) {
return;
}

if (activeSpan) {
DEBUG_BUILD && logger.log(`[Tracing] Finishing current transaction with op: ${spanToJSON(activeSpan).op}`);
// If there's an open transaction on the scope, we need to finish it before creating an new one.
Expand All @@ -355,7 +340,7 @@ export const _browserTracingIntegration = ((_options: Partial<BrowserTracingOpti
});
}

if (options.instrumentPageLoad && client.emit && shouldUseDefaultPageLoadSpan) {
if (options.instrumentPageLoad && client.emit) {
const context: StartSpanOptions = {
name: WINDOW.location.pathname,
// pageload should always start at timeOrigin (and needs to be in s, not ms)
Expand Down Expand Up @@ -385,17 +370,14 @@ export const _browserTracingIntegration = ((_options: Partial<BrowserTracingOpti

if (from !== to) {
startingUrl = undefined;
// We check this in here again, as a custom instrumentation may have been triggered in the meanwhile
if (shouldUseDefaultNavigationSpan) {
const context: StartSpanOptions = {
name: WINDOW.location.pathname,
op: 'navigation',
origin: 'auto.navigation.browser',
metadata: { source: 'url' },
};

startBrowserTracingNavigationSpan(context);
}
const context: StartSpanOptions = {
name: WINDOW.location.pathname,
op: 'navigation',
origin: 'auto.navigation.browser',
metadata: { source: 'url' },
};

startBrowserTracingNavigationSpan(context);
}
});
}
Expand Down Expand Up @@ -435,7 +417,6 @@ export function startBrowserTracingPageLoadSpan(spanOptions: StartSpanOptions):
}

client.emit('startPageLoadSpan', spanOptions);
shouldUseDefaultPageLoadSpan = false;
}

/**
Expand All @@ -449,23 +430,6 @@ export function startBrowserTracingNavigationSpan(spanOptions: StartSpanOptions)
}

client.emit('startNavigationSpan', spanOptions);
shouldUseDefaultNavigationSpan = false;
}

/**
* Use this method if you want to disable the default navigation span.
* This is useful if you want to add custom routing instrumentation.
*/
export function disableDefaultBrowserTracingNavigationSpan(disable = true): void {
shouldUseDefaultNavigationSpan = !disable;
}

/**
* Use this method if you want to disable the default page load span.
* This is useful if you want to add custom routing instrumentation.
*/
export function disableDefaultBrowserTracingPageLoadSpan(disable = true): void {
shouldUseDefaultPageLoadSpan = !disable;
}

/** Returns the value of a meta tag */
Expand Down
2 changes: 0 additions & 2 deletions packages/tracing-internal/src/browser/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ export {
browserTracingIntegration,
startBrowserTracingNavigationSpan,
startBrowserTracingPageLoadSpan,
disableDefaultBrowserTracingNavigationSpan,
disableDefaultBrowserTracingPageLoadSpan,
} from './browserTracingIntegration';

export { instrumentOutgoingRequests, defaultRequestInstrumentationOptions } from './request';
Expand Down
2 changes: 0 additions & 2 deletions packages/tracing-internal/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ export {
browserTracingIntegration,
startBrowserTracingNavigationSpan,
startBrowserTracingPageLoadSpan,
disableDefaultBrowserTracingNavigationSpan,
disableDefaultBrowserTracingPageLoadSpan,
BROWSER_TRACING_INTEGRATION_ID,
instrumentOutgoingRequests,
defaultRequestInstrumentationOptions,
Expand Down