diff --git a/packages/react-router/src/server/createServerInstrumentation.ts b/packages/react-router/src/server/createServerInstrumentation.ts index 98d8c98ee46f..47c701ba377f 100644 --- a/packages/react-router/src/server/createServerInstrumentation.ts +++ b/packages/react-router/src/server/createServerInstrumentation.ts @@ -1,4 +1,3 @@ -import { context, createContextKey } from '@opentelemetry/api'; import { HTTP_REQUEST_METHOD, HTTP_ROUTE, URL_FULL, URL_PATH } from '@sentry/conventions/attributes'; import { debug, @@ -19,7 +18,11 @@ import { captureInstrumentationError, getPathFromRequest, getPattern, normalizeR import { getMiddlewareName } from './serverBuild'; import { markInstrumentationApiUsed } from './serverGlobals'; -const MIDDLEWARE_COUNTER_KEY = createContextKey('sentry_react_router_middleware_counter'); +// Per-request middleware counters, keyed by the request's root span (the one transaction all of a +// request's middlewares run under). The root span is the same instance across those middleware hooks +// whether or not a Sentry OpenTelemetry tracer provider is set up, unlike the OTel context the counter +// used to live on (which does not propagate without a provider). +const middlewareCountersByRootSpan = new WeakMap>(); // Re-export for backward compatibility and external use export { isInstrumentationApiUsed } from './serverGlobals'; @@ -55,63 +58,58 @@ export function createSentryServerInstrumentation( const activeSpan = getActiveSpan(); const existingRootSpan = activeSpan ? getRootSpan(activeSpan) : undefined; - const counterStore = { counters: {} as Record }; - const ctx = context.active().setValue(MIDDLEWARE_COUNTER_KEY, counterStore); + if (existingRootSpan) { + updateSpanName(existingRootSpan, `${info.request.method} ${pathname}`); + existingRootSpan.setAttributes({ + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react_router.instrumentation_api', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [URL_FULL]: info.request.url, + [URL_PATH]: pathname, + }); - await context.with(ctx, async () => { - if (existingRootSpan) { - updateSpanName(existingRootSpan, `${info.request.method} ${pathname}`); - existingRootSpan.setAttributes({ - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react_router.instrumentation_api', - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', - [URL_FULL]: info.request.url, - [URL_PATH]: pathname, - }); - - try { - const result = await handleRequest(); - if (result.status === 'error' && result.error instanceof Error) { - existingRootSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - captureInstrumentationError(result, captureErrors, 'react_router.request_handler', { - 'http.method': info.request.method, - [URL_FULL]: pathname, - }); - } - } finally { - await flushIfServerless(); + try { + const result = await handleRequest(); + if (result.status === 'error' && result.error instanceof Error) { + existingRootSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + captureInstrumentationError(result, captureErrors, 'react_router.request_handler', { + 'http.method': info.request.method, + [URL_FULL]: pathname, + }); } - } else { - await startSpan( - { - name: `${info.request.method} ${pathname}`, - forceTransaction: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react_router.instrumentation_api', - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', - [HTTP_REQUEST_METHOD]: info.request.method, - [URL_PATH]: pathname, - [URL_FULL]: info.request.url, - }, + } finally { + await flushIfServerless(); + } + } else { + await startSpan( + { + name: `${info.request.method} ${pathname}`, + forceTransaction: true, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react_router.instrumentation_api', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [HTTP_REQUEST_METHOD]: info.request.method, + [URL_PATH]: pathname, + [URL_FULL]: info.request.url, }, - async span => { - try { - const result = await handleRequest(); - if (result.status === 'error' && result.error instanceof Error) { - span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - captureInstrumentationError(result, captureErrors, 'react_router.request_handler', { - 'http.method': info.request.method, - [URL_FULL]: pathname, - }); - } - } finally { - await flushIfServerless(); + }, + async span => { + try { + const result = await handleRequest(); + if (result.status === 'error' && result.error instanceof Error) { + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + captureInstrumentationError(result, captureErrors, 'react_router.request_handler', { + 'http.method': info.request.method, + [URL_FULL]: pathname, + }); } - }, - ); - } - }); + } finally { + await flushIfServerless(); + } + }, + ); + } }, }); }, @@ -183,13 +181,17 @@ export function createSentryServerInstrumentation( updateRootSpanWithRoute(info.request.method, pattern, urlPath); - const counterStore = context.active().getValue(MIDDLEWARE_COUNTER_KEY) as - | { counters: Record } - | undefined; + const activeSpan = getActiveSpan(); + const rootSpan = activeSpan ? getRootSpan(activeSpan) : undefined; let middlewareIndex = 0; - if (counterStore) { - middlewareIndex = counterStore.counters[routeId] ?? 0; - counterStore.counters[routeId] = middlewareIndex + 1; + if (rootSpan) { + let counters = middlewareCountersByRootSpan.get(rootSpan); + if (!counters) { + counters = {}; + middlewareCountersByRootSpan.set(rootSpan, counters); + } + middlewareIndex = counters[routeId] ?? 0; + counters[routeId] = middlewareIndex + 1; } const middlewareName = getMiddlewareName(routeId, middlewareIndex); diff --git a/packages/react-router/test/server/createServerInstrumentation.test.ts b/packages/react-router/test/server/createServerInstrumentation.test.ts index ad2219deb18b..12d01c86d15c 100644 --- a/packages/react-router/test/server/createServerInstrumentation.test.ts +++ b/packages/react-router/test/server/createServerInstrumentation.test.ts @@ -1,4 +1,3 @@ -import * as otelApi from '@opentelemetry/api'; import { URL_FULL, URL_PATH } from '@sentry/conventions/attributes'; import * as core from '@sentry/core'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -29,21 +28,6 @@ vi.mock('../../src/server/serverBuild', () => ({ getMiddlewareName: vi.fn(), })); -vi.mock('@opentelemetry/api', async () => { - const actual = await vi.importActual('@opentelemetry/api'); - return { - ...actual, - context: { - active: vi.fn(() => ({ - getValue: vi.fn(), - setValue: vi.fn(), - })), - with: vi.fn((ctx, fn) => fn()), - }, - createContextKey: actual.createContextKey, - }; -}); - describe('createSentryServerInstrumentation', () => { beforeEach(() => { vi.clearAllMocks(); @@ -425,19 +409,8 @@ describe('createSentryServerInstrumentation', () => { it('should increment middleware index for multiple middleware calls on same route', async () => { const mockCallMiddleware = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); const mockInstrument = vi.fn(); - const mockSetAttributes = vi.fn(); - const mockRootSpan = { setAttributes: mockSetAttributes }; const routeId = 'routes/multi-middleware'; - // Simulate counter store that would be created by handler and stored in OTel context - const counterStore = { counters: {} as Record }; - - // eslint-disable-next-line @typescript-eslint/unbound-method - vi.mocked(otelApi.context.active).mockReturnValue({ - getValue: vi.fn(() => counterStore), - setValue: vi.fn(), - } as any); - vi.mocked(serverBuildModule.getMiddlewareName).mockReturnValue(undefined); const startSpanCalls: any[] = []; @@ -445,7 +418,9 @@ describe('createSentryServerInstrumentation', () => { startSpanCalls.push(opts); return fn(); }); - (core.getActiveSpan as any).mockReturnValue({}); + // The per-request middleware counter is keyed by the (stable) root span, so the 3 calls increment. + const mockRootSpan = { setAttributes: vi.fn() }; + (core.getActiveSpan as any).mockReturnValue(mockRootSpan); (core.getRootSpan as any).mockReturnValue(mockRootSpan); const instrumentation = createSentryServerInstrumentation(); @@ -464,7 +439,6 @@ describe('createSentryServerInstrumentation', () => { context: undefined, }; - // Call middleware 3 times (simulating 3 middlewares on same route) await hooks.middleware(mockCallMiddleware, requestInfo); await hooks.middleware(mockCallMiddleware, requestInfo); await hooks.middleware(mockCallMiddleware, requestInfo);