From be3a06ad0ba8fbc4ef56072fe58831fc3ec9bed4 Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Mon, 24 Aug 2026 14:13:10 -0400 Subject: [PATCH] feat(core): Emit low cardinality router span names Names `router` spans after the route they dispatch when span streaming is enabled, falling back to `Router` when the SDK has no route, per https://getsentry.github.io/sentry-conventions/names/#routing-router. Covers all seven sites that emit the `router` op: the OTel-derived and orchestrion Express integrations, Koa, Hapi, Angular, Ember and SvelteKit (Svelte 4 and 5). Two of them were leaking high-cardinality values: the orchestrion Express integration named router spans `layer.path`, which Express assigns the *matched* URL segment (verified against express 4.22.1: `app.use('/users/:id', router)` yields `/users/123`), and Angular named them the raw navigation URL, since the parameterized route only resolves at `ResolveEnd` and is applied to the root span rather than to the routing span. Angular, Ember and SvelteKit have no route value at the span, so they take the fallback. Hapi drops its `GET ` prefix because the convention's template has no method component. `traceLifecycle: 'static'` names are byte-identical; the existing static integration and e2e assertions are the regression check. --- MIGRATION.md | 24 ++++-- packages/angular/src/tracing.ts | 6 +- .../src/integrations/express/patch-layer.ts | 12 ++- .../integrations/express/patch-layer.test.ts | 74 +++++++++++++++++++ ...nstrumentEmberAppInstanceForPerformance.ts | 5 +- .../integrations/express/instrumentation.ts | 9 ++- .../src/integrations/hapi-utils.ts | 21 +++++- packages/server-utils/src/integrations/koa.ts | 10 ++- .../test/integrations/hapi-utils.test.ts | 22 +++++- .../src/client/svelte4BrowserTracing.ts | 5 +- .../src/client/svelte5BrowserTracing.ts | 5 +- .../client/browserTracingIntegration.test.ts | 31 ++++++++ 12 files changed, 207 insertions(+), 17 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 6edbb8ad61b1..1315eb8fc6e8 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -613,24 +613,36 @@ These changes are not caught by TypeScript. If you filter, group, or alert on sp ### Span name changes -Affected SDKs: All SDKs running in the browser. +Affected SDKs: All SDKs running in the browser, plus the Express, Koa and Hapi integrations on the server. With [span streaming](#span-streaming-is-now-the-default) enabled(the default), span names are now **low cardinality**, following the [Sentry span name conventions](https://getsentry.github.io/sentry-conventions/names/). -In v11, this only affects `pageload` spans. Further ops will follow in future releases. +In v11, this affects `pageload` and `router` spans. Further ops will follow in future releases. If you [opt out of span streaming](#opting-out-of-span-streaming), span names remain unchanged. The following span names were adjusted: -| Span op | Before | After | -| ---------- | ------------------------------------------------------------------------------------------- | ---------------------------------------------------------- | -| `pageload` | The parameterized route, or the raw URL path if the SDK couldn't resolve one (`/users/123`) | The parameterized route, or `Pageload` if the SDK has none | +| Span op | Before | After | +| ---------- | --------------------------------------------------------------------------------------------- | ---------------------------------------------------------- | +| `pageload` | The parameterized route, or the raw URL path if the SDK couldn't resolve one (`/users/123`) | The parameterized route, or `Pageload` if the SDK has none | +| `router` | Framework-specific, sometimes containing the raw URL (`/users/123`, `SvelteKit Route Change`) | The span's `http.route`, or `Router` if the SDK has none | + +Per framework, `router` spans are named: + +| SDK | Before | After | +| --------- | ----------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------- | +| Express | The router's mount path, or the raw matched URL segment on the orchestrion-based integration (`/users/123`) | The request's `http.route` (`/users/:id/detail`), or `Router` | +| Koa | The layer's path (`/users/:id`) | Unchanged; `Router` when the layer has no path | +| Hapi | `GET /users/{id}` | `/users/{id}` — the method prefix is dropped | +| Angular | The raw navigation URL (`/users/123`) | `Router` — the parameterized route is only resolved after this span starts | +| Ember | `route:index -> route:posts` | `Router` | +| SvelteKit | `SvelteKit Route Change` | `Router` | Some consequences to be aware of: Child spans of a pageload span carry its name in their `sentry.segment.name` attribute, so that changes with it. If you group or filter spans by segment name in dashboards or alerts, update those references. -`ignoreSpans` is evaluated when a span **starts**, at which point a pageload span without a resolved route is already named `'Pageload'`, so filters matching a URL path no longer apply to it. Match on attributes instead: +`ignoreSpans` is evaluated when a span **starts**, at which point a pageload span without a resolved route is already named `'Pageload'` and a router span without one is already named `'Router'`, so filters matching a URL path no longer apply to them. Match on attributes instead: ```js Sentry.init({ diff --git a/packages/angular/src/tracing.ts b/packages/angular/src/tracing.ts index c45c3d7f4185..31afb9515703 100644 --- a/packages/angular/src/tracing.ts +++ b/packages/angular/src/tracing.ts @@ -26,7 +26,9 @@ import { FUNCTION } from '@sentry/conventions/op'; import type { Integration, Span } from '@sentry/core'; import { debug, + hasSpanStreamingEnabled, parseStringToURLObject, + ROUTER_SPAN_NAME_FALLBACK, stripUrlQueryAndFragment, timestampInSeconds, filterCollectedUrl, @@ -136,7 +138,9 @@ export class TraceService implements OnDestroy { this._routingSpan = runOutsideAngular(() => startInactiveSpan({ - name: `${navigationEvent.url}`, + // With span streaming, span names have to be low cardinality. The parameterized route is only + // known at `ResolveEnd`, well after this span starts, so there is nothing but the fallback. + name: hasSpanStreamingEnabled(client) ? ROUTER_SPAN_NAME_FALLBACK : `${navigationEvent.url}`, attributes: { // TODO(conventions): Replace `'router'` with the `router` span op constant once it is released in `@sentry/conventions`. [SENTRY_OP]: 'router', diff --git a/packages/core/src/integrations/express/patch-layer.ts b/packages/core/src/integrations/express/patch-layer.ts index 2d98fd1cb50f..5ccd63ace2fa 100644 --- a/packages/core/src/integrations/express/patch-layer.ts +++ b/packages/core/src/integrations/express/patch-layer.ts @@ -32,6 +32,8 @@ import { MIDDLEWARE } from '@sentry/conventions/op'; import { DEBUG_BUILD } from '../../debug-build'; import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../../semanticAttributes'; import { SPAN_STATUS_ERROR, withActiveSpan } from '../../tracing'; +import { hasSpanStreamingEnabled } from '../../tracing/spans/hasSpanStreamingEnabled'; +import { ROUTER_SPAN_NAME_FALLBACK } from '../../tracing/spans/spanNames'; import { startSpanManual } from '../../tracing/trace'; import { debug } from '../../utils/debug-logger'; import type { SpanAttributes } from '../../types/span'; @@ -56,7 +58,7 @@ import { getLayerMetadata, isLayerIgnored, } from './utils'; -import { getIsolationScope } from '../../currentScopes'; +import { getClient, getIsolationScope } from '../../currentScopes'; import { getDefaultIsolationScope } from '../../defaultScopes'; import { getOriginalFunction, markFunctionWrapped } from '../../utils/object'; import { setSDKProcessingMetadata } from './set-sdk-processing-metadata'; @@ -165,7 +167,13 @@ export function patchLayer( DEBUG_BUILD && debug.warn('Isolation scope is still default isolation scope - skipping setting transactionName'); } - return startSpanManual({ name, attributes }, span => { + const client = getClient(); + // With span streaming, span names have to be low cardinality, so router spans are named after their route. + const isStreamedRouterSpan = type === ExpressLayerType_ROUTER && !!client && hasSpanStreamingEnabled(client); + + const spanName = isStreamedRouterSpan ? actualMatchedRoute || ROUTER_SPAN_NAME_FALLBACK : name; + + return startSpanManual({ name: spanName, attributes }, span => { let spanHasEnded = false; // TODO: Fix router spans (getRouterPath does not work properly) to // have useful names before removing this branch diff --git a/packages/core/test/lib/integrations/express/patch-layer.test.ts b/packages/core/test/lib/integrations/express/patch-layer.test.ts index 4d41dc785a5a..de557a02dac8 100644 --- a/packages/core/test/lib/integrations/express/patch-layer.test.ts +++ b/packages/core/test/lib/integrations/express/patch-layer.test.ts @@ -52,10 +52,15 @@ const defaultIsolationScope = { this._scopeData.sdkProcessingMetadata = data; }, }; +let spanStreamingEnabled = false; +beforeEach(() => (spanStreamingEnabled = false)); vi.mock('../../../../src/currentScopes', () => ({ getIsolationScope() { return inDefaultIsolationScope ? defaultIsolationScope : notDefaultIsolationScope; }, + getClient() { + return { getOptions: () => ({ traceLifecycle: spanStreamingEnabled ? 'stream' : 'static' }) }; + }, })); vi.mock('../../../../src/defaultScopes', () => ({ getDefaultIsolationScope() { @@ -468,6 +473,75 @@ describe('patchLayer', () => { checkSpans([]); }); + it('names router spans after their route when span streaming is enabled', () => { + spanStreamingEnabled = true; + const options: ExpressPatchLayerOptions = {}; + const req = Object.assign(new EventEmitter(), { + originalUrl: '/a/b/c', + }) as unknown as ExpressRequest; + + const layer = { + name: 'router', + handle: vi.fn(), + } as unknown as ExpressLayer; + + const res = Object.assign(new EventEmitter(), {}) as unknown as ExpressResponse; + + storeLayer(req, '/a'); + storeLayer(req, '/b'); + + patchLayer(() => options, layer, '/c'); + layer.handle(req, res); + + checkSpans([ + { + status: { code: 0, message: 'OK' }, + data: { + 'express.name': '/c', + 'express.type': 'router', + 'http.route': '/a/b/c', + 'sentry.op': 'router', + 'sentry.origin': 'auto.http.express', + }, + description: '/a/b/c', + }, + ]); + }); + + it('falls back to a static router span name when the route is unknown', () => { + spanStreamingEnabled = true; + const options: ExpressPatchLayerOptions = {}; + const req = Object.assign(new EventEmitter(), { + originalUrl: '/abcdef', + }) as unknown as ExpressRequest; + + const layer = { + name: 'router', + handle: vi.fn(), + } as unknown as ExpressLayer; + + const res = Object.assign(new EventEmitter(), {}) as unknown as ExpressResponse; + + storeLayer(req, '/a'); + storeLayer(req, '/b'); + + patchLayer(() => options, layer, '/c'); + layer.handle(req, res); + + checkSpans([ + { + status: { code: 0, message: 'OK' }, + data: { + 'express.name': '/c', + 'express.type': 'router', + 'sentry.op': 'router', + 'sentry.origin': 'auto.http.express', + }, + description: 'Router', + }, + ]); + }); + it('handles case when route does not match url', () => { const onRouteResolved = vi.fn(); const options: ExpressPatchLayerOptions = { onRouteResolved }; diff --git a/packages/ember/src/utils/instrumentEmberAppInstanceForPerformance.ts b/packages/ember/src/utils/instrumentEmberAppInstanceForPerformance.ts index 43c9a88cd8d4..7c475095d249 100644 --- a/packages/ember/src/utils/instrumentEmberAppInstanceForPerformance.ts +++ b/packages/ember/src/utils/instrumentEmberAppInstanceForPerformance.ts @@ -18,6 +18,7 @@ import { getCurrentScope, hasSpanStreamingEnabled, PAGELOAD_SPAN_NAME_FALLBACK, + ROUTER_SPAN_NAME_FALLBACK, spanToJSON, type Client, type Span, @@ -148,7 +149,9 @@ export function instrumentEmberAppInstanceForPerformance( [SENTRY_OP]: 'router', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.ember', }, - name: `route:${fromRoute} -> route:${toRoute}`, + // With span streaming, span names have to be low cardinality, and Ember gives us no route + // template for the transition itself, so it's the fallback. + name: hasSpanStreamingEnabled(client) ? ROUTER_SPAN_NAME_FALLBACK : `route:${fromRoute} -> route:${toRoute}`, onlyIfParent: true, }); }); diff --git a/packages/server-utils/src/integrations/express/instrumentation.ts b/packages/server-utils/src/integrations/express/instrumentation.ts index ac1de126015f..417035ab12cc 100644 --- a/packages/server-utils/src/integrations/express/instrumentation.ts +++ b/packages/server-utils/src/integrations/express/instrumentation.ts @@ -5,8 +5,11 @@ import type { Span } from '@sentry/core'; import { debug, getActiveSpan, + getClient, getDefaultIsolationScope, getIsolationScope, + hasSpanStreamingEnabled, + ROUTER_SPAN_NAME_FALLBACK, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startInactiveSpan, stringMatchesSomePattern, @@ -224,8 +227,12 @@ function getSpanForLayer(data: HandleChannelContext, options: ExpressIntegration return undefined; } + const client = getClient(); + // With span streaming, span names have to be low cardinality, so router spans are named after their route. + const isStreamedRouterSpan = type === 'router' && !!client && hasSpanStreamingEnabled(client); + const span = startInactiveSpan({ - name, + name: isStreamedRouterSpan ? matchedRoute || ROUTER_SPAN_NAME_FALLBACK : name, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN, [SENTRY_OP]: EXPRESS_TYPE_TO_SPAN_OP[type], diff --git a/packages/server-utils/src/integrations/hapi-utils.ts b/packages/server-utils/src/integrations/hapi-utils.ts index 58e6655cf000..dd67a6f36e89 100644 --- a/packages/server-utils/src/integrations/hapi-utils.ts +++ b/packages/server-utils/src/integrations/hapi-utils.ts @@ -9,7 +9,14 @@ * is replaced with `getActiveSpan()`. */ -import { getActiveSpan, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/core'; +import { + getActiveSpan, + getClient, + hasSpanStreamingEnabled, + ROUTER_SPAN_NAME_FALLBACK, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + startSpan, +} from '@sentry/core'; import { SENTRY_OP } from '@sentry/conventions/attributes'; import { MIDDLEWARE } from '@sentry/conventions/op'; import type { @@ -74,7 +81,17 @@ export const getRouteMetadata = (route: ServerRoute, pluginName?: string): SpanM attributes[AttributeNames.HAPI_TYPE] = HapiLayerType.ROUTER; } - return { attributes, name: `${route.method.toUpperCase()} ${route.path}` }; + const client = getClient(); + // With span streaming, span names have to be low cardinality, so router spans are named after their + // route alone, without the method prefix. + const isStreamedRouterSpan = !pluginName && !!client && hasSpanStreamingEnabled(client); + + return { + attributes, + name: isStreamedRouterSpan + ? route.path || ROUTER_SPAN_NAME_FALLBACK + : `${route.method.toUpperCase()} ${route.path}`, + }; }; /** Build the span name and attributes for a Hapi server extension. */ diff --git a/packages/server-utils/src/integrations/koa.ts b/packages/server-utils/src/integrations/koa.ts index 7f0519976f01..952229e59c34 100644 --- a/packages/server-utils/src/integrations/koa.ts +++ b/packages/server-utils/src/integrations/koa.ts @@ -4,8 +4,11 @@ import { debug, defineIntegration, getActiveSpan, + getClient, getDefaultIsolationScope, getIsolationScope, + hasSpanStreamingEnabled, + ROUTER_SPAN_NAME_FALLBACK, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan, } from '@sentry/core'; @@ -173,7 +176,12 @@ function patchLayer( const koaName = metadata.attributes[KOA_NAME]; // Somehow, name is sometimes `''` for middleware spans. // See: https://github.com/open-telemetry/opentelemetry-js-contrib/issues/2220 - const name = typeof koaName === 'string' ? koaName || '< unknown >' : metadata.name; + const staticName = typeof koaName === 'string' ? koaName || '< unknown >' : metadata.name; + + const client = getClient(); + // With span streaming, span names have to be low cardinality, so router spans are named after their route. + const isStreamedRouterSpan = layerType === LAYER_TYPE.ROUTER && !!client && hasSpanStreamingEnabled(client); + const name = isStreamedRouterSpan ? metadata.attributes[HTTP_ROUTE] || ROUTER_SPAN_NAME_FALLBACK : staticName; return startSpan( { diff --git a/packages/server-utils/test/integrations/hapi-utils.test.ts b/packages/server-utils/test/integrations/hapi-utils.test.ts index 24fedaf14825..836cb335314b 100644 --- a/packages/server-utils/test/integrations/hapi-utils.test.ts +++ b/packages/server-utils/test/integrations/hapi-utils.test.ts @@ -1,9 +1,15 @@ -import { describe, expect, it } from 'vitest'; +import { setCurrentClient } from '@sentry/core'; +import { afterEach, describe, expect, it } from 'vitest'; import { getExtMetadata, getRouteMetadata } from '../../src/integrations/hapi-utils'; +import { getDefaultTestClientOptions, TestClient } from '../mocks/client'; describe('getRouteMetadata', () => { const route = { path: '/users/{id}', method: 'get' } as any; + afterEach(() => { + setCurrentClient(undefined as unknown as TestClient); + }); + it('describes a directly-registered route as a router layer', () => { expect(getRouteMetadata(route)).toEqual({ name: 'GET /users/{id}', @@ -26,6 +32,20 @@ describe('getRouteMetadata', () => { }, }); }); + + it('drops the method from the router span name when span streaming is enabled', () => { + const client = new TestClient(getDefaultTestClientOptions({ traceLifecycle: 'stream' })); + setCurrentClient(client); + + expect(getRouteMetadata(route).name).toBe('/users/{id}'); + }); + + it('keeps the plugin span name when span streaming is enabled', () => { + const client = new TestClient(getDefaultTestClientOptions({ traceLifecycle: 'stream' })); + setCurrentClient(client); + + expect(getRouteMetadata(route, 'my-plugin').name).toBe('GET /users/{id}'); + }); }); describe('getExtMetadata', () => { diff --git a/packages/sveltekit/src/client/svelte4BrowserTracing.ts b/packages/sveltekit/src/client/svelte4BrowserTracing.ts index 818a8ffe2f61..e49b47736c6a 100644 --- a/packages/sveltekit/src/client/svelte4BrowserTracing.ts +++ b/packages/sveltekit/src/client/svelte4BrowserTracing.ts @@ -2,6 +2,7 @@ import type { Client, Span } from '@sentry/core'; import { hasSpanStreamingEnabled, PAGELOAD_SPAN_NAME_FALLBACK, + ROUTER_SPAN_NAME_FALLBACK, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, } from '@sentry/core'; @@ -132,7 +133,9 @@ function _instrumentNavigations(client: Client, navigatingStore: Readable { expect(routingSpanEndSpy).toHaveBeenCalledTimes(1); }); + it('names the routing span with the low cardinality fallback when span streaming is enabled', async () => { + const streamingClient = { + getOptions: () => ({ traceLifecycle: 'stream' }), + on: () => {}, + addEventProcessor: () => {}, + addIntegration: () => {}, + }; + const integration = browserTracingIntegration({ + instrumentPageLoad: false, + }); + // @ts-expect-error - the fakeClient doesn't satisfy Client but that's fine + integration.afterAllSetup(streamingClient); + await vi.dynamicImportSettled(); + + // TODO(v11): switch to `navigating` from `$app/state` + // @ts-expect-error - navigating is a writable but the types say it's just readable + // eslint-disable-next-line typescript/no-deprecated + navigating.set({ + from: { route: { id: '/users' }, url: { pathname: '/users' } }, + to: { route: { id: '/users/[id]' }, url: { pathname: '/users/7762', href: 'https://sentry-test.io/users/7762' } }, + type: 'link', + }); + + expect(startInactiveSpanSpy).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'Router', + attributes: expect.objectContaining({ [SENTRY_OP]: 'router' }), + }), + ); + }); + describe('handling same origin and destination navigations', () => { it("doesn't start a navigation span if the raw navigation origin and destination are equal", async () => { const integration = browserTracingIntegration({