From a4622ea8dd0e5cd30067f029a2b68d364edfccff 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 | 3 + 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, 192 insertions(+), 11 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index ede48b5f3db8..f2af8d53b6f5 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -626,6 +626,7 @@ 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 | +| `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 | | `graphql` | The graphql phase and, for operations, the operation name (`query GetUser`, `graphql.parse`, `graphql.resolve user.0.name`) | The operation type, or the processing type where there is none (`GraphQL query`, `GraphQL parse`, `GraphQL resolve`) | | `resource.*` | The resource URL, relative to the page origin for same-origin resources (`/assets/app.js`) | The resource domain (`cdn.example.com`), or `Resource` if the SDK has none | @@ -639,6 +640,8 @@ Because a low-cardinality name cannot say which part of request processing a spa For the same reason, `useOperationNameForRootSpan` no longer renames the enclosing root span (`GET /graphql` stays `GET /graphql`, instead of becoming `GET /graphql (query GetUser)`). The operations are still recorded on that span's `sentry.graphql.operation` attribute, as long as the option stays enabled (the default). Disabling it skips both, as before. +Only the Express, Koa and Hapi integrations resolve a route template for `router` spans. Angular, Ember and SvelteKit have none when the span starts, so their router spans are named `Router`. + Child spans of a service or root 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 span might not yet have its final name. For example, an unresolved pageload span name is named `'Pageload'` and might receive its final, resolved route name later. diff --git a/packages/angular/src/tracing.ts b/packages/angular/src/tracing.ts index 07e73f29555f..78f7adf06dbe 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({