Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(browser): Set url.path and url.full on pageload and navigation spans#21952
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
c0799db53eb086ea9ba1c5eec1a46c5126b975a966File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -24,6 +24,7 @@ import { | ||
| GLOBAL_OBJ, | ||
| hasSpansEnabled, | ||
| hasSpanStreamingEnabled, | ||
| isURLObjectRelative, | ||
| parseStringToURLObject, | ||
| propagationContextFromHeaders, | ||
| registerSpanErrorInstrumentation, | ||
| @@ -51,6 +52,7 @@ import { WEB_VITALS_INTEGRATION_NAME, webVitalsIntegration } from '../integratio | ||
| import { registerBackgroundTabDetection } from './backgroundtab'; | ||
| import { linkTraces } from './linkedTraces'; | ||
| import { defaultRequestInstrumentationOptions, instrumentOutgoingRequests } from './request'; | ||
| import { URL_FULL, URL_PATH } from '@sentry/conventions/attributes'; | ||
| export const BROWSER_TRACING_INTEGRATION_ID = 'BrowserTracing'; | ||
| @@ -416,23 +418,32 @@ export const browserTracingIntegration = ((options: Partial<BrowserTracingOption | ||
| let _pageloadSpan: Span | undefined; | ||
| /** Create routing idle transaction. */ | ||
| function _createRouteSpan(client: Client, startSpanOptions: StartSpanOptions, makeActive = true): void { | ||
| function _createRouteSpan(client: Client, startSpanOptions: StartSpanOptions, makeActive = true, url?: string): void { | ||
| const isPageloadSpan = startSpanOptions.op === 'pageload'; | ||
| const initialSpanName = startSpanOptions.name; | ||
| const finalStartSpanOptions: StartSpanOptions = beforeStartSpan | ||
| ? beforeStartSpan(startSpanOptions) | ||
| : startSpanOptions; | ||
| const attributes = finalStartSpanOptions.attributes || {}; | ||
| // For navigations, `url` is the destination URL, so we use it to reflect the post-navigation location. | ||
| // For pageloads (and manual navigation spans without a URL) we fall back to the current location. | ||
| const urlObject = parseStringToURLObject(url || getLocationHref()); | ||
| const attributes = { | ||
| ...(urlObject?.pathname && { [URL_PATH]: urlObject.pathname }), | ||
| ...(urlObject && !isURLObjectRelative(urlObject) && { [URL_FULL]: urlObject.href }), | ||
| ...finalStartSpanOptions.attributes, | ||
| }; | ||
Lms24 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| // If `finalStartSpanOptions.name` is different than `startSpanOptions.name` | ||
| // it is because `beforeStartSpan` set a custom name. Therefore we set the source to 'custom'. | ||
| if (initialSpanName !== finalStartSpanOptions.name) { | ||
| attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] = 'custom'; | ||
| finalStartSpanOptions.attributes = attributes; | ||
| } | ||
| finalStartSpanOptions.attributes = attributes; | ||
| if (!makeActive) { | ||
| // We want to ensure this has 0s duration | ||
| const now = dateTimestampInSeconds(); | ||
| @@ -563,6 +574,7 @@ export const browserTracingIntegration = ((options: Partial<BrowserTracingOption | ||
| ...startSpanOptions, | ||
| }, | ||
| false, | ||
| navigationOptions.url, | ||
| ); | ||
| return; | ||
| } | ||
| @@ -593,13 +605,18 @@ export const browserTracingIntegration = ((options: Partial<BrowserTracingOption | ||
| normalizedRequest: undefined, | ||
| }); | ||
| _createRouteSpan(client, { | ||
| op: 'navigation', | ||
| ...startSpanOptions, | ||
| // Navigation starts a new trace and is NOT parented under any active interaction (e.g. ui.action.click) | ||
| parentSpan: null, | ||
| forceTransaction: true, | ||
| }); | ||
| _createRouteSpan( | ||
| client, | ||
| { | ||
| op: 'navigation', | ||
| ...startSpanOptions, | ||
| // Navigation starts a new trace and is NOT parented under any active interaction (e.g. ui.action.click) | ||
| parentSpan: null, | ||
| forceTransaction: true, | ||
| }, | ||
| true, | ||
| navigationOptions?.url, | ||
| ); | ||
| }); | ||
| client.on('startPageLoadSpan', (startSpanOptions, traceOptions = {}) => { | ||
| @@ -779,8 +796,8 @@ export function startBrowserTracingNavigationSpan( | ||
| options?: { url?: string; isRedirect?: boolean }, | ||
| ): Span | undefined { | ||
| const { url, isRedirect } = options || {}; | ||
| client.emit('beforeStartNavigationSpan', spanOptions, { isRedirect }); | ||
| client.emit('startNavigationSpan', spanOptions, { isRedirect }); | ||
| client.emit('beforeStartNavigationSpan', spanOptions, { isRedirect, url }); | ||
| client.emit('startNavigationSpan', spanOptions, { isRedirect, url }); | ||
| const scope = getCurrentScope(); | ||
| scope.setTransactionName(spanOptions.name); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -33,6 +33,7 @@ import { | ||
| } from '../../src/tracing/browserTracingIntegration'; | ||
| import { PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE } from '../../src/tracing/linkedTraces'; | ||
| import { getDefaultBrowserClientOptions } from '../helper/browser-client-options'; | ||
| import { URL_FULL, URL_PATH } from '@sentry/conventions/attributes'; | ||
| const oldTextEncoder = global.window.TextEncoder; | ||
| const oldTextDecoder = global.window.TextDecoder; | ||
| @@ -65,11 +66,17 @@ describe('browserTracingIntegration', () => { | ||
| getCurrentScope().clear(); | ||
| getIsolationScope().clear(); | ||
| getCurrentScope().setClient(undefined); | ||
| document.head.innerHTML = ''; | ||
| // Reset document and location to a fresh JSDOM for every test. `getLocationHref()` reads | ||
| // `WINDOW.document.location.href`, so leaving `document` bound to a shared instance leaks URL state | ||
| // (e.g. `/test`) from `pushState` in earlier tests into later ones. `history` must stay bound to the | ||
| // shared instance because the history instrumentation patches it once globally on `client.init()`. | ||
| const dom = new JSDOM(undefined, { url: 'https://example.com/' }); | ||
| Object.defineProperty(global, 'document', { value: dom.window.document, writable: true }); | ||
| Object.defineProperty(global, 'location', { value: dom.window.document.location, writable: true }); | ||
| document.head.innerHTML = ''; | ||
| // We want to suppress the "Multiple browserTracingIntegration instances are not supported." warnings | ||
| vi.spyOn(console, 'warn').mockImplementation(() => {}); | ||
| }); | ||
| @@ -174,6 +181,8 @@ describe('browserTracingIntegration', () => { | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser', | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', | ||
| [URL_FULL]: 'https://example.com/', | ||
| [URL_PATH]: '/', | ||
| }, | ||
| span_id: expect.stringMatching(/[a-f0-9]{16}/), | ||
| start_timestamp: expect.any(Number), | ||
| @@ -258,6 +267,8 @@ describe('browserTracingIntegration', () => { | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser', | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', | ||
| [URL_FULL]: 'https://example.com/', | ||
| [URL_PATH]: '/', | ||
| }, | ||
| span_id: expect.stringMatching(/[a-f0-9]{16}/), | ||
| start_timestamp: expect.any(Number), | ||
| @@ -286,6 +297,8 @@ describe('browserTracingIntegration', () => { | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser', | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', | ||
| [URL_FULL]: 'https://example.com/test', | ||
| [URL_PATH]: '/test', | ||
| [PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE]: `${span?.spanContext().traceId}-${span?.spanContext().spanId}-1`, | ||
| }, | ||
| links: [ | ||
| @@ -325,6 +338,8 @@ describe('browserTracingIntegration', () => { | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser', | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', | ||
| [URL_FULL]: 'https://example.com/test2', | ||
| [URL_PATH]: '/test2', | ||
| [PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE]: `${span2?.spanContext().traceId}-${span2?.spanContext().spanId}-1`, | ||
| }, | ||
| links: [ | ||
| @@ -366,6 +381,8 @@ describe('browserTracingIntegration', () => { | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser', | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', | ||
| [URL_FULL]: 'https://example.com/', | ||
| [URL_PATH]: '/', | ||
| }, | ||
| span_id: expect.stringMatching(/[a-f0-9]{16}/), | ||
| start_timestamp: expect.any(Number), | ||
| @@ -394,6 +411,8 @@ describe('browserTracingIntegration', () => { | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation.redirect', | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser', | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', | ||
| [URL_FULL]: 'https://example.com/test', | ||
| [URL_PATH]: '/test', | ||
| }, | ||
| description: '/test', | ||
| op: 'navigation.redirect', | ||
| @@ -456,6 +475,8 @@ describe('browserTracingIntegration', () => { | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'manual', | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom', | ||
| [URL_FULL]: 'https://example.com/', | ||
| [URL_PATH]: '/', | ||
| }, | ||
| span_id: expect.stringMatching(/[a-f0-9]{16}/), | ||
| start_timestamp: expect.any(Number), | ||
| @@ -492,6 +513,8 @@ describe('browserTracingIntegration', () => { | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.test', | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom', | ||
| [URL_FULL]: 'https://example.com/', | ||
| [URL_PATH]: '/', | ||
| testy: 'yes', | ||
| }, | ||
| span_id: expect.stringMatching(/[a-f0-9]{16}/), | ||
| @@ -733,6 +756,8 @@ describe('browserTracingIntegration', () => { | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom', | ||
| [PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE]: expect.stringMatching(/[a-f0-9]{32}-[a-f0-9]{16}-1/), | ||
| [URL_FULL]: 'https://example.com/', | ||
| [URL_PATH]: '/', | ||
| }, | ||
| links: [ | ||
| { | ||
| @@ -785,6 +810,8 @@ describe('browserTracingIntegration', () => { | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.test', | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom', | ||
| [URL_FULL]: 'https://example.com/', | ||
| [URL_PATH]: '/', | ||
cursor[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| testy: 'yes', | ||
| }, | ||
| span_id: expect.stringMatching(/[a-f0-9]{16}/), | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.