fix(ember): Add route IDs to pageload and navigation spans

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': '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': 'tracing',
+        },
       },
     },
     transaction: 'route:tracing',

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('adds the route ID to 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: 'index' });
+  expect(pageloadSpan.name).toBe('route:index');
+});
+
+test('adds the route ID to 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: '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');
 

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,
@@ -68,13 +69,16 @@ export function instrumentEmberAppInstanceForPerformance(
     activeRootSpan = startBrowserTracingPageLoadSpan(client, {
       // With span streaming, span names have to be low cardinality, so we can't fall back to the URL.
       name: routeInfo
-        ? `route:${routeInfo.name}`
+        ? routeInfo.name
+          ? `route:${routeInfo.name}`
+          : 'route'
         : hasSpanStreamingEnabled(client)
           ? PAGELOAD_SPAN_NAME_FALLBACK
           : url || WINDOW.location.pathname,
       attributes: {
         [SENTRY_SEGMENT_NAME_SOURCE]: routeInfo ? 'route' : 'url',
         [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.ember',
+        ...(routeInfo?.name && { [ROUTER_NAVIGATION_ROUTE_ID]: routeInfo.name }),
         ...(url ? _getRouteUrlAttributes(client, url, routeInfo?.params) : {}),
         toRoute: routeInfo?.name,
       },
@@ -91,9 +95,10 @@ export function instrumentEmberAppInstanceForPerformance(
 
   routerService.on('routeWillChange', (transition: Transition) => {
     const { fromRoute, toRoute } = getTransitionInformation(transition, routerService);
+    const transactionName = toRoute ? `route:${toRoute}` : 'route';
 
     // Store this here to be used, even if the active span has ended
-    getCurrentScope().setTransactionName(`route:${toRoute}`);
+    getCurrentScope().setTransactionName(transactionName);
 
     // We want to ignore loading && error routes
     if (transitionIsIntermediate(transition)) {
@@ -113,10 +118,11 @@ export function instrumentEmberAppInstanceForPerformance(
         const urlAttributes = targetUrl ? _getRouteUrlAttributes(client, targetUrl, transition.to?.params) : {};
 
         activeRootSpan = startBrowserTracingNavigationSpan(client, {
-          name: `route:${toRoute}`,
+          name: transactionName,
           attributes: {
             [SENTRY_SEGMENT_NAME_SOURCE]: 'route',
             [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.ember',
+            ...(toRoute && { [ROUTER_NAVIGATION_ROUTE_ID]: toRoute }),
             ...urlAttributes,
             fromRoute,
             toRoute,
@@ -130,9 +136,10 @@ export function instrumentEmberAppInstanceForPerformance(
       const url = _getLocationURL(location);
       if (url) {
         const routeInfo = _recognizeURL(routerService, url);
-        activeRootSpan.updateName(`route:${toRoute}`);
+        activeRootSpan.updateName(transactionName);
         activeRootSpan.setAttributes({
           [SENTRY_SEGMENT_NAME_SOURCE]: 'route',
+          ...(toRoute && { [ROUTER_NAVIGATION_ROUTE_ID]: toRoute }),
           ..._getRouteUrlAttributes(client, url, routeInfo?.params),
           toRoute: toRoute,
         });

packages/ember/tests/instrument-ember-app-instance-performance.test.ts

@@ -0,0 +1,236 @@
+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(traceLifecycle: 'stream' | 'static' = 'stream') {
+  const handlers = new Map<string, (transition: Transition) => void>();
+  const router = {
+    recognize: vi.fn<() => { name: string; params: Record<string, string> } | 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 }),
+    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<typeof instrumentEmberAppInstanceForPerformance>[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': 'index',
+        'url.path': '/',
+        'url.full': 'https://ember.example.com/',
+        'url.template': '/',
+        toRoute: 'index',
+      },
+    });
+  });
+
+  it.each(['stream', 'static'] as const)('falls back for an empty %s pageload name', traceLifecycle => {
+    const fixture = createRouterFixture(traceLifecycle);
+    fixture.router.recognize.mockReturnValue({ name: '', params: {} });
+
+    fixture.instrument();
+
+    expect(fixture.startPageloadSpan).toHaveBeenCalledTimes(1);
+    expect(spanToJSON(fixture.pageloadSpan).name).toBe('route');
+    expect(spanToJSON(fixture.pageloadSpan).attributes).not.toHaveProperty('router.navigation.route.id');
+  });
+
+  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(getCurrentScope().getScopeData().transactionName).toBe('route:index');
+    expect(spanToJSON(fixture.pageloadSpan).attributes['router.navigation.route.id']).toBe('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(getCurrentScope().getScopeData().transactionName).toBe('route: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': 'tracing',
+        fromRoute: 'index',
+        toRoute: 'tracing',
+      },
+    });
+  });
+
+  it('uses the current route fallback when the transition has no destination', () => {
+    const fixture = createRouterFixture();
+    fixture.router.currentRouteName = 'tracing';
+    fixture.instrument();
+
+    fixture.routeWillChange({ from: { name: 'index' } });
+
+    expect(spanToJSON(fixture.navigationSpan).attributes['router.navigation.route.id']).toBe('tracing');
+  });
+
+  it.each([
+    ['stream', undefined],
+    ['stream', ''],
+    ['static', undefined],
+    ['static', ''],
+  ] as const)('uses a navigation fallback in %s mode when the route name is %j', (traceLifecycle, currentRouteName) => {
+    const fixture = createRouterFixture(traceLifecycle);
+    fixture.router.currentRouteName = currentRouteName;
+    fixture.instrument();
+
+    fixture.routeWillChange({ from: { name: 'index' } });
+
+    expect(fixture.startNavigationSpan).toHaveBeenCalledExactlyOnceWith(fixture.client, {
+      name: 'route',
+      attributes: {
+        [SENTRY_SEGMENT_NAME_SOURCE]: 'route',
+        'sentry.origin': 'auto.navigation.ember',
+        fromRoute: 'index',
+        toRoute: currentRouteName,
+      },
+    });
+    expect(getCurrentScope().getScopeData().transactionName).toBe('route');
+  });
+
+  it.each([
+    ['stream', undefined],
+    ['stream', ''],
+    ['static', undefined],
+    ['static', ''],
+  ] as const)('preserves caller route ID in %s mode for %j', (traceLifecycle, currentRouteName) => {
+    const fixture = createRouterFixture(traceLifecycle);
+    fixture.router.currentRouteName = currentRouteName;
+    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');
+    expect(spanToJSON(fixture.pageloadSpan).name).toBe('route');
+    expect(getCurrentScope().getScopeData().transactionName).toBe('route');
+    expect(setAttributes).toHaveBeenCalledExactlyOnceWith({
+      [SENTRY_SEGMENT_NAME_SOURCE]: 'route',
+      'url.path': '/',
+      'url.full': 'https://ember.example.com/',
+      'url.template': '/',
+      toRoute: currentRouteName,
+    });
+    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();
+  });
+});