From 5c106e58e1f82e1f2fdfc1465ffde324b24b4230 Mon Sep 17 00:00:00 2001 From: Maharshi Mishra Date: Mon, 14 Sep 2026 16:53:36 +0530 Subject: [PATCH] fix(ember): Add route IDs to pageload and navigation spans Fixes #24284. Co-Authored-By: GPT-6 --- .../tests/sentry-performance.test.ts | 6 + .../tests/streamed-performance.test.ts | 46 ++++ ...nstrumentEmberAppInstanceForPerformance.ts | 4 + ...ent-ember-app-instance-performance.test.ts | 211 ++++++++++++++++++ 4 files changed, 267 insertions(+) create mode 100644 packages/ember/tests/instrument-ember-app-instance-performance.test.ts diff --git a/dev-packages/e2e-tests/test-applications/ember-strict-resolver/tests/sentry-performance.test.ts b/dev-packages/e2e-tests/test-applications/ember-strict-resolver/tests/sentry-performance.test.ts index 57d9a6c26972..8c7d1340ef12 100644 --- a/dev-packages/e2e-tests/test-applications/ember-strict-resolver/tests/sentry-performance.test.ts +++ b/dev-packages/e2e-tests/test-applications/ember-strict-resolver/tests/sentry-performance.test.ts @@ -19,6 +19,9 @@ test('sends a pageload transaction with a parameterized URL', async ({ page }) = trace: { op: 'pageload', origin: 'auto.pageload.ember', + data: { + 'router.navigation.route.id': 'route:index', + }, }, }, transaction: 'route:index', @@ -47,6 +50,9 @@ test('sends a navigation transaction with a parameterized URL', async ({ page }) trace: { op: 'navigation', origin: 'auto.navigation.ember', + data: { + 'router.navigation.route.id': 'route:tracing', + }, }, }, transaction: 'route:tracing', diff --git a/dev-packages/e2e-tests/test-applications/ember-strict-resolver/tests/streamed-performance.test.ts b/dev-packages/e2e-tests/test-applications/ember-strict-resolver/tests/streamed-performance.test.ts index bd6405989474..5c9a6ef26c31 100644 --- a/dev-packages/e2e-tests/test-applications/ember-strict-resolver/tests/streamed-performance.test.ts +++ b/dev-packages/e2e-tests/test-applications/ember-strict-resolver/tests/streamed-performance.test.ts @@ -4,6 +4,52 @@ import { getSpanOp, waitForStreamedSpan } from '@sentry-internal/test-utils'; // Only the `ember-strict-resolver (streamed)` variant builds the app with `traceLifecycle: 'stream'`. test.skip(process.env.E2E_TEST_TRACE_LIFECYCLE !== 'stream', 'requires the app built with span streaming'); +test('preserves the route name on a streamed pageload', async ({ page }) => { + const pageloadSpanPromise = waitForStreamedSpan('ember-strict-resolver', span => { + return ( + span.is_segment && + getSpanOp(span) === 'pageload' && + span.attributes['sentry.origin']?.value === 'auto.pageload.ember' + ); + }); + + await page.goto('/'); + + const pageloadSpan = await pageloadSpanPromise; + + expect(pageloadSpan.attributes['router.navigation.route.id']).toEqual({ type: 'string', value: 'route:index' }); + expect(pageloadSpan.name).toBe('route:index'); +}); + +test('preserves the route name on a streamed navigation', async ({ page }) => { + const pageloadSpanPromise = waitForStreamedSpan('ember-strict-resolver', span => { + return ( + span.is_segment && + getSpanOp(span) === 'pageload' && + span.attributes['sentry.origin']?.value === 'auto.pageload.ember' + ); + }); + + await page.goto('/'); + await pageloadSpanPromise; + + const navigationSpanPromise = waitForStreamedSpan('ember-strict-resolver', span => { + return ( + span.is_segment && + getSpanOp(span) === 'navigation' && + span.attributes['sentry.origin']?.value === 'auto.navigation.ember' + ); + }); + + await page.getByText('Tracing').click(); + await expect(page).toHaveURL(/\/tracing$/); + + const navigationSpan = await navigationSpanPromise; + + expect(navigationSpan.attributes['router.navigation.route.id']).toEqual({ type: 'string', value: 'route:tracing' }); + expect(navigationSpan.name).toBe('route:tracing'); +}); + test('names the transition span with the low cardinality fallback', async ({ page }) => { const transitionSpanPromise = waitForStreamedSpan('ember-strict-resolver', span => getSpanOp(span) === 'router'); diff --git a/packages/ember/src/utils/instrumentEmberAppInstanceForPerformance.ts b/packages/ember/src/utils/instrumentEmberAppInstanceForPerformance.ts index 3abd359bad21..cbf5e56be7ef 100644 --- a/packages/ember/src/utils/instrumentEmberAppInstanceForPerformance.ts +++ b/packages/ember/src/utils/instrumentEmberAppInstanceForPerformance.ts @@ -7,6 +7,7 @@ import type { } from '@sentry/browser'; import { getAbsoluteUrl, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startInactiveSpan, WINDOW } from '@sentry/browser'; import { + ROUTER_NAVIGATION_ROUTE_ID, SENTRY_SEGMENT_NAME_SOURCE, SENTRY_OP, URL_FULL, @@ -75,6 +76,7 @@ export function instrumentEmberAppInstanceForPerformance( attributes: { [SENTRY_SEGMENT_NAME_SOURCE]: routeInfo ? 'route' : 'url', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.ember', + ...(routeInfo ? { [ROUTER_NAVIGATION_ROUTE_ID]: `route:${routeInfo.name}` } : {}), ...(url ? _getRouteUrlAttributes(client, url, routeInfo?.params) : {}), toRoute: routeInfo?.name, }, @@ -117,6 +119,7 @@ export function instrumentEmberAppInstanceForPerformance( attributes: { [SENTRY_SEGMENT_NAME_SOURCE]: 'route', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.ember', + ...(toRoute !== undefined ? { [ROUTER_NAVIGATION_ROUTE_ID]: `route:${toRoute}` } : {}), ...urlAttributes, fromRoute, toRoute, @@ -133,6 +136,7 @@ export function instrumentEmberAppInstanceForPerformance( activeRootSpan.updateName(`route:${toRoute}`); activeRootSpan.setAttributes({ [SENTRY_SEGMENT_NAME_SOURCE]: 'route', + ...(toRoute !== undefined ? { [ROUTER_NAVIGATION_ROUTE_ID]: `route:${toRoute}` } : {}), ..._getRouteUrlAttributes(client, url, routeInfo?.params), toRoute: toRoute, }); diff --git a/packages/ember/tests/instrument-ember-app-instance-performance.test.ts b/packages/ember/tests/instrument-ember-app-instance-performance.test.ts new file mode 100644 index 000000000000..b7bae342d9c9 --- /dev/null +++ b/packages/ember/tests/instrument-ember-app-instance-performance.test.ts @@ -0,0 +1,211 @@ +import type ApplicationInstance from '@ember/application/instance'; +import type Transition from '@ember/routing/transition'; +import { SENTRY_SEGMENT_NAME_SOURCE } from '@sentry/conventions/attributes'; +import { getCurrentScope, SentrySpan, spanToJSON, type Client, type StartSpanOptions } from '@sentry/core'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { instrumentEmberAppInstanceForPerformance } from '../src/utils/instrumentEmberAppInstanceForPerformance.ts'; + +function createRouterFixture() { + const handlers = new Map void>(); + const router = { + recognize: vi.fn<() => { name: string; params: Record } | undefined>().mockReturnValue({ + name: 'index', + params: {}, + }), + currentRouteName: undefined as string | undefined, + currentURL: '/', + on: (event: string, callback: (transition: Transition) => void) => handlers.set(event, callback), + }; + const location = { + rootURL: '/', + getURL: () => '/', + formatURL: (url: string) => url, + }; + const appInstance = { + lookup: (name: string) => (name === 'service:router' ? router : { location }), + } as unknown as ApplicationInstance; + const client = { + getOptions: () => ({ traceLifecycle: 'stream' }), + getDataCollectionOptions: () => ({ urlQueryParams: true }), + } as unknown as Client; + const pageloadSpan = new SentrySpan({ name: 'Pageload' }); + const navigationSpan = new SentrySpan({ name: 'Navigation' }); + const startPageloadSpan = vi.fn((_client: Client, options: StartSpanOptions) => { + pageloadSpan.updateName(options.name); + pageloadSpan.setAttributes(options.attributes ?? {}); + return pageloadSpan; + }); + const startNavigationSpan = vi.fn((_client: Client, options: StartSpanOptions) => { + navigationSpan.updateName(options.name); + navigationSpan.setAttributes(options.attributes ?? {}); + return navigationSpan; + }); + + return { + client, + router, + pageloadSpan, + navigationSpan, + startPageloadSpan, + startNavigationSpan, + instrument: (config: Parameters[2] = {}) => + instrumentEmberAppInstanceForPerformance(client, appInstance, config, startPageloadSpan, startNavigationSpan), + routeWillChange: (transition: { from?: { name: string }; to?: { name?: string; localName?: string } }) => { + const handler = handlers.get('routeWillChange'); + if (!handler) { + throw new Error('routeWillChange was not registered'); + } + handler(transition as Transition); + }, + }; +} + +describe('instrumentEmberAppInstanceForPerformance', () => { + let previousTransactionName: string | undefined; + + beforeEach(() => { + previousTransactionName = getCurrentScope().getScopeData().transactionName; + getCurrentScope().setTransactionName(undefined); + vi.stubGlobal('location', { origin: 'https://ember.example.com', pathname: '/' }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllGlobals(); + getCurrentScope().setTransactionName(previousTransactionName); + }); + + it('adds the recognized route ID to the pageload', () => { + const fixture = createRouterFixture(); + + fixture.instrument(); + + expect(fixture.startPageloadSpan).toHaveBeenCalledExactlyOnceWith(fixture.client, { + name: 'route:index', + attributes: { + [SENTRY_SEGMENT_NAME_SOURCE]: 'route', + 'sentry.origin': 'auto.pageload.ember', + 'router.navigation.route.id': 'route:index', + 'url.path': '/', + 'url.full': 'https://ember.example.com/', + 'url.template': '/', + toRoute: 'index', + }, + }); + }); + + it.each([true, false])('updates the initial pageload when navigation instrumentation is %s', instrumentNavigation => { + const fixture = createRouterFixture(); + fixture.router.recognize.mockReturnValue(undefined); + fixture.instrument({ instrumentNavigation }); + expect(spanToJSON(fixture.pageloadSpan).name).toBe('Pageload'); + expect(spanToJSON(fixture.pageloadSpan).attributes).not.toHaveProperty('router.navigation.route.id'); + + fixture.routeWillChange({ to: { name: 'index' } }); + + expect(fixture.startPageloadSpan).toHaveBeenCalledTimes(1); + expect(fixture.startNavigationSpan).not.toHaveBeenCalled(); + expect(spanToJSON(fixture.pageloadSpan).name).toBe('route:index'); + expect(spanToJSON(fixture.pageloadSpan).attributes['router.navigation.route.id']).toBe('route:index'); + expect(spanToJSON(fixture.pageloadSpan).attributes[SENTRY_SEGMENT_NAME_SOURCE]).toBe('route'); + }); + + it('adds the destination route ID without a destination URL', () => { + const fixture = createRouterFixture(); + fixture.instrument(); + + fixture.routeWillChange({ from: { name: 'index' }, to: { name: 'tracing' } }); + + expect(fixture.startNavigationSpan).toHaveBeenCalledExactlyOnceWith(fixture.client, { + name: 'route:tracing', + attributes: { + [SENTRY_SEGMENT_NAME_SOURCE]: 'route', + 'sentry.origin': 'auto.navigation.ember', + 'router.navigation.route.id': 'route:tracing', + fromRoute: 'index', + toRoute: 'tracing', + }, + }); + }); + + it.each([ + ['tracing', 'route:tracing'], + ['', 'route:'], + ])('uses the current route fallback %j when the transition has no destination', (currentRouteName, expectedId) => { + const fixture = createRouterFixture(); + fixture.router.currentRouteName = currentRouteName; + fixture.instrument(); + + fixture.routeWillChange({ from: { name: 'index' } }); + + expect(spanToJSON(fixture.navigationSpan).attributes['router.navigation.route.id']).toBe(expectedId); + }); + + it('omits the navigation route ID when neither route name is available', () => { + const fixture = createRouterFixture(); + fixture.instrument(); + + fixture.routeWillChange({ from: { name: 'index' } }); + + expect(fixture.startNavigationSpan).toHaveBeenCalledExactlyOnceWith(fixture.client, { + name: 'route:undefined', + attributes: { + [SENTRY_SEGMENT_NAME_SOURCE]: 'route', + 'sentry.origin': 'auto.navigation.ember', + fromRoute: 'index', + toRoute: undefined, + }, + }); + }); + + it('preserves a caller route ID when the delayed pageload route is unknown', () => { + const fixture = createRouterFixture(); + fixture.router.recognize.mockReturnValue(undefined); + fixture.instrument(); + fixture.pageloadSpan.setAttribute('router.navigation.route.id', 'caller-route'); + const updateName = vi.spyOn(fixture.pageloadSpan, 'updateName'); + const setAttributes = vi.spyOn(fixture.pageloadSpan, 'setAttributes'); + + fixture.routeWillChange({}); + + expect(updateName).toHaveBeenCalledExactlyOnceWith('route:undefined'); + expect(setAttributes).toHaveBeenCalledExactlyOnceWith({ + [SENTRY_SEGMENT_NAME_SOURCE]: 'route', + 'url.path': '/', + 'url.full': 'https://ember.example.com/', + 'url.template': '/', + toRoute: undefined, + }); + expect(spanToJSON(fixture.pageloadSpan).attributes['router.navigation.route.id']).toBe('caller-route'); + }); + + it('does not create a pageload when pageload instrumentation is disabled', () => { + const fixture = createRouterFixture(); + + fixture.instrument({ instrumentPageLoad: false }); + fixture.routeWillChange({ to: { name: 'index' } }); + + expect(fixture.startPageloadSpan).not.toHaveBeenCalled(); + expect(fixture.startNavigationSpan).not.toHaveBeenCalled(); + }); + + it('does not create a navigation when navigation instrumentation is disabled', () => { + const fixture = createRouterFixture(); + fixture.instrument({ instrumentNavigation: false }); + + fixture.routeWillChange({ from: { name: 'index' }, to: { name: 'tracing' } }); + + expect(fixture.startNavigationSpan).not.toHaveBeenCalled(); + }); + + it.each(['loading', 'error'])('does not create a navigation for an intermediate %s route', localName => { + const fixture = createRouterFixture(); + fixture.instrument(); + const endPageload = vi.spyOn(fixture.pageloadSpan, 'end'); + + fixture.routeWillChange({ from: { name: 'index' }, to: { name: `tracing.${localName}`, localName } }); + + expect(endPageload).not.toHaveBeenCalled(); + expect(fixture.startNavigationSpan).not.toHaveBeenCalled(); + }); +});