diff --git a/dev-packages/e2e-tests/test-applications/ember-classic/tests/performance.test.ts b/dev-packages/e2e-tests/test-applications/ember-classic/tests/performance.test.ts index e6aa4747bf92..4aa00f61b599 100644 --- a/dev-packages/e2e-tests/test-applications/ember-classic/tests/performance.test.ts +++ b/dev-packages/e2e-tests/test-applications/ember-classic/tests/performance.test.ts @@ -15,6 +15,13 @@ test('sends a pageload transaction with a parameterized URL', async ({ page }) = trace: { op: 'pageload', origin: 'auto.pageload.ember', + data: { + 'sentry.origin': 'auto.pageload.ember', + 'sentry.source': 'route', + 'url.template': '/', + 'url.path': '/', + 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/$/), + }, }, }, transaction: 'route:index', @@ -43,6 +50,13 @@ test('sends a navigation transaction with a parameterized URL', async ({ page }) trace: { op: 'navigation', origin: 'auto.navigation.ember', + data: { + 'sentry.origin': 'auto.navigation.ember', + 'sentry.source': 'route', + 'url.template': '/tracing', + 'url.path': '/tracing', + 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/tracing$/), + }, }, }, transaction: 'route:tracing', @@ -75,6 +89,13 @@ test('sends a navigation transaction even if the pageload span is still active', trace: { op: 'pageload', origin: 'auto.pageload.ember', + data: { + 'sentry.origin': 'auto.pageload.ember', + 'sentry.source': 'route', + 'url.template': '/', + 'url.path': '/', + 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/$/), + }, }, }, transaction: 'route:index', @@ -88,6 +109,13 @@ test('sends a navigation transaction even if the pageload span is still active', trace: { op: 'navigation', origin: 'auto.navigation.ember', + data: { + 'sentry.origin': 'auto.navigation.ember', + 'sentry.source': 'route', + 'url.template': '/tracing', + 'url.path': '/tracing', + 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/tracing$/), + }, }, }, transaction: 'route:tracing', @@ -124,6 +152,13 @@ test('captures correct spans for navigation', async ({ page }) => { trace: { op: 'navigation', origin: 'auto.navigation.ember', + data: { + 'sentry.origin': 'auto.navigation.ember', + 'sentry.source': 'route', + 'url.template': '/slow-loading-route', + 'url.path': '/slow-loading-route', + 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/slow-loading-route$/), + }, }, }, transaction: 'route:slow-loading-route.index', diff --git a/dev-packages/e2e-tests/test-applications/ember-embroider/tests/performance.test.ts b/dev-packages/e2e-tests/test-applications/ember-embroider/tests/performance.test.ts index a82964b46a92..1545409ad3c5 100644 --- a/dev-packages/e2e-tests/test-applications/ember-embroider/tests/performance.test.ts +++ b/dev-packages/e2e-tests/test-applications/ember-embroider/tests/performance.test.ts @@ -15,6 +15,13 @@ test('sends a pageload transaction with a parameterized URL', async ({ page }) = trace: { op: 'pageload', origin: 'auto.pageload.ember', + data: { + 'sentry.origin': 'auto.pageload.ember', + 'sentry.source': 'route', + 'url.template': '/', + 'url.path': '/', + 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/$/), + }, }, }, transaction: 'route:index', @@ -43,6 +50,13 @@ test('sends a navigation transaction with a parameterized URL', async ({ page }) trace: { op: 'navigation', origin: 'auto.navigation.ember', + data: { + 'sentry.origin': 'auto.navigation.ember', + 'sentry.source': 'route', + 'url.template': '/tracing', + 'url.path': '/tracing', + 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/tracing$/), + }, }, }, transaction: 'route:tracing', @@ -75,6 +89,13 @@ test('sends a navigation transaction even if the pageload span is still active', trace: { op: 'pageload', origin: 'auto.pageload.ember', + data: { + 'sentry.origin': 'auto.pageload.ember', + 'sentry.source': 'route', + 'url.template': '/', + 'url.path': '/', + 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/$/), + }, }, }, transaction: 'route:index', @@ -88,6 +109,13 @@ test('sends a navigation transaction even if the pageload span is still active', trace: { op: 'navigation', origin: 'auto.navigation.ember', + data: { + 'sentry.origin': 'auto.navigation.ember', + 'sentry.source': 'route', + 'url.template': '/tracing', + 'url.path': '/tracing', + 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/tracing$/), + }, }, }, transaction: 'route:tracing', @@ -124,6 +152,13 @@ test('captures correct spans for navigation', async ({ page }) => { trace: { op: 'navigation', origin: 'auto.navigation.ember', + data: { + 'sentry.origin': 'auto.navigation.ember', + 'sentry.source': 'route', + 'url.template': '/slow-loading-route', + 'url.path': '/slow-loading-route', + 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/slow-loading-route$/), + }, }, }, transaction: 'route:slow-loading-route.index', diff --git a/packages/ember/addon/utils/instrumentEmberAppInstanceForPerformance.ts b/packages/ember/addon/utils/instrumentEmberAppInstanceForPerformance.ts index c10e3913b4ef..750f0115ee1a 100644 --- a/packages/ember/addon/utils/instrumentEmberAppInstanceForPerformance.ts +++ b/packages/ember/addon/utils/instrumentEmberAppInstanceForPerformance.ts @@ -5,11 +5,22 @@ import type { startBrowserTracingNavigationSpan as startBrowserTracingNavigationSpanType, startBrowserTracingPageLoadSpan as startBrowserTracingPageLoadSpanType, } from '@sentry/browser'; -import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, startInactiveSpan } from '@sentry/browser'; +import { + getAbsoluteUrl, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, + startInactiveSpan, +} from '@sentry/browser'; import type { Client, Span } from '@sentry/core'; import type { EmberRouterMain } from '../types'; import { getBackburner } from './performance'; +const URL_FULL = 'url.full'; +const URL_PATH = 'url.path'; +const URL_TEMPLATE = 'url.template'; + +type TransitionWithIntent = Transition & { intent?: { url?: string } }; + export function instrumentEmberAppInstanceForPerformance( client: Client, appInstance: ApplicationInstance, @@ -22,6 +33,7 @@ export function instrumentEmberAppInstanceForPerformance( let routerService = appInstance.lookup('service:router') as RouterService & { externalRouter?: RouterService; _hasMountedSentryPerformanceRouting?: boolean; + currentURL?: string; }; if (routerService.externalRouter) { @@ -60,6 +72,52 @@ function getTransitionInformation( }; } +function getUrlPathFromEmberLocation(url: string): string { + if (!url) { + return '/'; + } + + const withoutQuery = url.split('?')[0] ?? url; + + if (withoutQuery.includes('#')) { + const hashPart = withoutQuery.substring(withoutQuery.indexOf('#') + 1); + return hashPart.startsWith('/') ? hashPart : `/${hashPart}`; + } + + return withoutQuery.startsWith('/') ? withoutQuery : `/${withoutQuery}`; +} + +function buildUrlTemplate(path: string, params: Record = {}): string { + let template = path; + + const paramEntries = Object.entries(params) + .filter((entry): entry is [string, string] => typeof entry[1] === 'string' && entry[1].length > 0) + .sort(([, a], [, b]) => b.length - a.length); + + for (const [key, value] of paramEntries) { + template = template.replace(`/${value}`, `/:${key}`); + } + + return template; +} + +// Only exported for testing +export function _getRouteUrlAttributes( + url: string, + params: Record = {}, + fullUrl: string = url, +): Record { + const path = getUrlPathFromEmberLocation(url); + + // `url.full` is derived from the unstripped URL so that hash-location apps keep their `#/...` + // fragment (e.g. `https://host/#/tracing`), which would otherwise be lost by `getUrlPathFromEmberLocation`. + return { + [URL_PATH]: path, + [URL_FULL]: getAbsoluteUrl(fullUrl), + [URL_TEMPLATE]: buildUrlTemplate(path, params), + }; +} + // Only exported for testing export function _getLocationURL(location: EmberRouterMain['location']): string { if (!location?.getURL || !location?.formatURL) { @@ -76,7 +134,7 @@ export function _getLocationURL(location: EmberRouterMain['location']): string { function _instrumentEmberRouter( client: Client, - routerService: RouterService, + routerService: RouterService & { currentURL?: string }, routerMain: EmberRouterMain, config: { disableRunloopPerformance?: boolean; instrumentPageLoad?: boolean; instrumentNavigation?: boolean }, startBrowserTracingPageLoadSpan: typeof startBrowserTracingPageLoadSpanType, @@ -96,6 +154,7 @@ function _instrumentEmberRouter( attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.ember', + ..._getRouteUrlAttributes(url, routeInfo.params), url, toRoute: routeInfo.name, }, @@ -124,11 +183,19 @@ function _instrumentEmberRouter( activeRootSpan?.end(); + // Only `intent.url` reliably reflects the *destination* URL at `routeWillChange` time. The + // router's location still points at the current (pre-transition) route here, so falling back to + // it would tag the navigation span with the previous route's `url.*` attributes. When we don't + // have a trustworthy target URL, we omit them and let `routeDidChange` set them from `currentURL`. + const targetUrl = (transition as TransitionWithIntent).intent?.url; + const urlAttributes = targetUrl ? _getRouteUrlAttributes(targetUrl, transition.to?.params) : {}; + activeRootSpan = startBrowserTracingNavigationSpan(client, { name: `route:${toRoute}`, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.ember', + ...urlAttributes, fromRoute, toRoute, }, @@ -150,6 +217,15 @@ function _instrumentEmberRouter( } transitionSpan.end(); + const url = routerService.currentURL ?? _getLocationURL(location); + if (url) { + const routeInfo = routerService.recognize(url); + // `currentURL` is the normalized route path and never includes the hash fragment, so we source + // `url.full` from the location URL (which preserves `#/...` for hash-location apps) when available. + const fullUrl = _getLocationURL(location) || url; + activeRootSpan.setAttributes(_getRouteUrlAttributes(url, routeInfo.params ?? transition.to?.params, fullUrl)); + } + if (disableRunloopPerformance) { activeRootSpan.end(); return; diff --git a/packages/ember/tests/unit/instrument-router-location-test.ts b/packages/ember/tests/unit/instrument-router-location-test.ts index a225089319af..3c513330e522 100644 --- a/packages/ember/tests/unit/instrument-router-location-test.ts +++ b/packages/ember/tests/unit/instrument-router-location-test.ts @@ -1,5 +1,5 @@ import type { EmberRouterMain } from '@sentry/ember/addon/types'; -import { _getLocationURL } from '@sentry/ember/utils/instrumentEmberAppInstanceForPerformance'; +import { _getLocationURL, _getRouteUrlAttributes } from '@sentry/ember/utils/instrumentEmberAppInstanceForPerformance'; import { setupTest } from 'ember-qunit'; import { module, test } from 'qunit'; import type { SentryTestContext } from '../helpers/setup-sentry'; @@ -97,4 +97,37 @@ module('Unit | Utility | instrument-router-location', function (hooks) { const result = _getLocationURL(mockLocation); assert.strictEqual(result, '', 'Should return empty string when formatURL is not available'); }); + + test('_getRouteUrlAttributes handles history location paths', function (this: SentryTestContext, assert) { + const result = _getRouteUrlAttributes('/tracing'); + + assert.strictEqual(result['url.path'], '/tracing'); + assert.strictEqual(result['url.template'], '/tracing'); + assert.ok(result['url.full']?.includes('/tracing'), 'url.full includes the path'); + }); + + test('_getRouteUrlAttributes parameterizes dynamic segments', function (this: SentryTestContext, assert) { + const result = _getRouteUrlAttributes('/users/123', { user_id: '123' }); + + assert.strictEqual(result['url.path'], '/users/123'); + assert.strictEqual(result['url.template'], '/users/:user_id'); + assert.ok(result['url.full']?.includes('/users/123'), 'url.full includes the path'); + }); + + test('_getRouteUrlAttributes handles hash location paths', function (this: SentryTestContext, assert) { + const result = _getRouteUrlAttributes('/#/tracing'); + + assert.strictEqual(result['url.path'], '/tracing'); + assert.strictEqual(result['url.template'], '/tracing'); + assert.ok(result['url.full']?.includes('/#/tracing'), 'url.full preserves the hash fragment'); + }); + + test('_getRouteUrlAttributes sources url.full from an explicit fullUrl', function (this: SentryTestContext, assert) { + // Mirrors `routeDidChange`, where the route path (`currentURL`) has no hash but the location URL does. + const result = _getRouteUrlAttributes('/tracing', {}, '/#/tracing'); + + assert.strictEqual(result['url.path'], '/tracing'); + assert.strictEqual(result['url.template'], '/tracing'); + assert.ok(result['url.full']?.includes('/#/tracing'), 'url.full preserves the hash fragment from fullUrl'); + }); });