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({