Skip to content

Commit 6707fd3

Browse files
onurtemizkanonurtemizkan
authored
feat(react-router): Include middleware function names and indices (#19109)
Resolves: #19097 Middleware spans created by the instrumentation API were using the route pattern as the span name (same as loaders and actions), which doesn't help when a route has multiple middlewares. This PR adds function name resolution so middleware spans are named `middleware authMiddleware` instead of just the route pattern. Each span also gets a `react_router.middleware.index` attribute. When names are not available (anonymous arrow functions), it falls back to the route ID. Function names are resolved from the `ServerBuild` object, which is captured through two complementary paths: - The OTEL instrumentation patches createRequestHandler at runtime, capturing the `ServerBuild` from its arguments. This handles both static builds and factory functions (dev mode HMR). - New Vite plugin (makeServerBuildCapturePlugin) that injects into the virtual react-router/server-build module during SSR builds, providing early capture at module initialization. This is needed because virtual modules are not reachable by OTEL's module hooks. The `isInstrumentationApiUsed()` check in the OTEL instrumentation was moved from the per-request handler to `createRequestHandler` itself. The OTEL hook now always needs to run to capture the ServerBuild reference for middleware name lookup; per-request wrapping is gated at the handler level instead. The Node version gating for the OTEL hook was also removed since it needs to run unconditionally now. Limitation: - Anonymous middlewares won't have function names regardless of build mode.
1 parent 0c3b071 commit 6707fd3

File tree

19 files changed

+722
-109
lines changed

19 files changed

+722
-109
lines changed

‎dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ export default [
1010
route('server-loader', 'routes/performance/server-loader.tsx'),
1111
route('server-action', 'routes/performance/server-action.tsx'),
1212
route('with-middleware', 'routes/performance/with-middleware.tsx'),
13+
route('multi-middleware', 'routes/performance/multi-middleware.tsx'),
1314
route('error-loader', 'routes/performance/error-loader.tsx'),
1415
route('error-action', 'routes/performance/error-action.tsx'),
1516
route('error-middleware', 'routes/performance/error-middleware.tsx'),

‎dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/multi-middleware.tsx‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import type { Route } from './+types/multi-middleware';
2+
3+
export const middleware: Route.MiddlewareFunction[] = [
4+
async function multiAuthMiddleware(_args, next) {
5+
return next();
6+
},
7+
async function multiLoggingMiddleware(_args, next) {
8+
return next();
9+
},
10+
async function multiValidationMiddleware(_args, next) {
11+
return next();
12+
},
13+
];
14+
15+
export function loader() {
16+
return { message: 'Multi-middleware route loaded' };
17+
}
18+
19+
export default function MultiMiddlewarePage() {
20+
return (
21+
<div>
22+
<h1 id="multi-middleware-title">Multi Middleware Route</h1>
23+
<p id="multi-middleware-content">This route has 3 middlewares</p>
24+
</div>
25+
);
26+
}

‎dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/middleware.server.test.ts‎

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ import { expect, test } from '@playwright/test';
22
import { waitForTransaction } from '@sentry-internal/test-utils';
33
import { APP_NAME } from '../constants';
44

5-
// Note: React Router middleware instrumentation now works in Framework Mode.
6-
// Previously this was a known limitation (see: https://github.com/remix-run/react-router/discussions/12950)
75
test.describe('server - instrumentation API middleware', () => {
86
test('should instrument server middleware with instrumentation API origin', async ({ page }) => {
97
const txPromise = waitForTransaction(APP_NAME, async transactionEvent => {
@@ -43,20 +41,27 @@ test.describe('server - instrumentation API middleware', () => {
4341
(span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react_router.middleware',
4442
);
4543

44+
expect(middlewareSpan).toBeDefined();
4645
expect(middlewareSpan).toMatchObject({
4746
span_id: expect.any(String),
4847
trace_id: expect.any(String),
49-
data: {
48+
data: expect.objectContaining({
5049
'sentry.origin': 'auto.function.react_router.instrumentation_api',
5150
'sentry.op': 'function.react_router.middleware',
52-
},
53-
description: '/performance/with-middleware',
51+
'react_router.route.id': 'routes/performance/with-middleware',
52+
'http.route': '/performance/with-middleware',
53+
'react_router.middleware.index': 0,
54+
}),
5455
parent_span_id: expect.any(String),
5556
start_timestamp: expect.any(Number),
5657
timestamp: expect.any(Number),
5758
op: 'function.react_router.middleware',
5859
origin: 'auto.function.react_router.instrumentation_api',
5960
});
61+
62+
// Middleware name is available via OTEL patching of createRequestHandler
63+
expect(middlewareSpan!.data?.['react_router.middleware.name']).toBe('authMiddleware');
64+
expect(middlewareSpan!.description).toBe('middleware authMiddleware');
6065
});
6166

6267
test('should have middleware span run before loader span', async ({ page }) => {
@@ -80,6 +85,37 @@ test.describe('server - instrumentation API middleware', () => {
8085
expect(loaderSpan).toBeDefined();
8186

8287
// Middleware should start before loader
83-
expect(middlewareSpan!.start_timestamp).toBeLessThanOrEqual(loaderSpan!.start_timestamp);
88+
expect(middlewareSpan!.start_timestamp).toBeLessThanOrEqual(loaderSpan!.start_timestamp!);
89+
});
90+
91+
test('should track multiple middlewares with correct indices', async ({ page }) => {
92+
const txPromise = waitForTransaction(APP_NAME, async transactionEvent => {
93+
return transactionEvent.transaction === 'GET /performance/multi-middleware';
94+
});
95+
96+
await page.goto(`/performance/multi-middleware`);
97+
98+
const transaction = await txPromise;
99+
100+
await expect(page.locator('#multi-middleware-title')).toBeVisible();
101+
await expect(page.locator('#multi-middleware-content')).toHaveText('This route has 3 middlewares');
102+
103+
const middlewareSpans = transaction?.spans?.filter(
104+
(span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react_router.middleware',
105+
);
106+
107+
expect(middlewareSpans).toHaveLength(3);
108+
109+
const sortedSpans = [...middlewareSpans!].sort(
110+
(a: any, b: any) =>
111+
(a.data?.['react_router.middleware.index'] ?? 0) - (b.data?.['react_router.middleware.index'] ?? 0),
112+
);
113+
114+
expect(sortedSpans.map((s: any) => s.data?.['react_router.middleware.index'])).toEqual([0, 1, 2]);
115+
expect(sortedSpans.map((s: any) => s.data?.['react_router.middleware.name'])).toEqual([
116+
'multiAuthMiddleware',
117+
'multiLoggingMiddleware',
118+
'multiValidationMiddleware',
119+
]);
84120
});
85121
});

‎dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/vite.config.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import { reactRouter } from '@react-router/dev/vite';
2+
import { sentryReactRouter } from '@sentry/react-router';
23
import { defineConfig } from 'vite';
34

4-
export default defineConfig({
5-
plugins: [reactRouter()],
6-
});
5+
export default defineConfig(async config => ({
6+
plugins: [
7+
reactRouter(),
8+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
9+
...((await sentryReactRouter({ sourcemaps: { disable: true } }, config)) as any[]),
10+
],
11+
}));

‎packages/react-router/src/client/createClientInstrumentation.ts‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ const WINDOW = GLOBAL_OBJ as typeof GLOBAL_OBJ & Window;
1919
// Tracks active numeric navigation span to prevent duplicate spans when popstate fires
2020
let currentNumericNavigationSpan: Span | undefined;
2121

22+
// Per-request middleware counters, keyed by Request
23+
const middlewareCountersMap = new WeakMap<object, Record<string, number>>();
24+
2225
const SENTRY_CLIENT_INSTRUMENTATION_FLAG = '__sentryReactRouterClientInstrumentationUsed';
2326
// Intentionally never reset - once set, instrumentation API handles all navigations for the session.
2427
const SENTRY_NAVIGATE_HOOK_INVOKED_FLAG = '__sentryReactRouterNavigateHookInvoked';
@@ -214,6 +217,8 @@ export function createSentryClientInstrumentation(
214217
},
215218

216219
route(route: InstrumentableRoute) {
220+
const routeId = route.id;
221+
217222
route.instrument({
218223
async loader(callLoader, info) {
219224
const urlPath = getPathFromRequest(info.request);
@@ -267,12 +272,24 @@ export function createSentryClientInstrumentation(
267272
const urlPath = getPathFromRequest(info.request);
268273
const routePattern = normalizeRoutePath(getPattern(info)) || urlPath;
269274

275+
let counters = middlewareCountersMap.get(info.request);
276+
if (!counters) {
277+
counters = {};
278+
middlewareCountersMap.set(info.request, counters);
279+
}
280+
281+
const middlewareIndex = counters[routeId] ?? 0;
282+
counters[routeId] = middlewareIndex + 1;
283+
270284
await startSpan(
271285
{
272-
name: routePattern,
286+
name: `middleware ${routeId}`,
273287
attributes: {
274288
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.react_router.client_middleware',
275289
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api',
290+
'react_router.route.id': routeId,
291+
'http.route': routePattern,
292+
'react_router.middleware.index': middlewareIndex,
276293
},
277294
},
278295
async span => {

‎packages/react-router/src/server/createServerInstrumentation.ts‎

Lines changed: 73 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { context } from '@opentelemetry/api';
1+
import { context, createContextKey } from '@opentelemetry/api';
22
import { getRPCMetadata, RPCType } from '@opentelemetry/core';
33
import { ATTR_HTTP_ROUTE } from '@opentelemetry/semantic-conventions';
44
import {
@@ -17,8 +17,11 @@ import {
1717
import { DEBUG_BUILD } from '../common/debug-build';
1818
import type { InstrumentableRequestHandler, InstrumentableRoute, ServerInstrumentation } from '../common/types';
1919
import { captureInstrumentationError, getPathFromRequest, getPattern, normalizeRoutePath } from '../common/utils';
20+
import { getMiddlewareName } from './serverBuild';
2021
import { markInstrumentationApiUsed } from './serverGlobals';
2122

23+
const MIDDLEWARE_COUNTER_KEY = createContextKey('sentry_react_router_middleware_counter');
24+
2225
// Re-export for backward compatibility and external use
2326
export { isInstrumentationApiUsed } from './serverGlobals';
2427

@@ -53,61 +56,68 @@ export function createSentryServerInstrumentation(
5356
const activeSpan = getActiveSpan();
5457
const existingRootSpan = activeSpan ? getRootSpan(activeSpan) : undefined;
5558

56-
if (existingRootSpan) {
57-
updateSpanName(existingRootSpan, `${info.request.method} ${pathname}`);
58-
existingRootSpan.setAttributes({
59-
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server',
60-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react_router.instrumentation_api',
61-
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
62-
});
59+
const counterStore = { counters: {} as Record<string, number> };
60+
const ctx = context.active().setValue(MIDDLEWARE_COUNTER_KEY, counterStore);
6361

64-
try {
65-
const result = await handleRequest();
66-
if (result.status === 'error' && result.error instanceof Error) {
67-
existingRootSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
68-
captureInstrumentationError(result, captureErrors, 'react_router.request_handler', {
69-
'http.method': info.request.method,
70-
'http.url': pathname,
71-
});
62+
await context.with(ctx, async () => {
63+
if (existingRootSpan) {
64+
updateSpanName(existingRootSpan, `${info.request.method} ${pathname}`);
65+
existingRootSpan.setAttributes({
66+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server',
67+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react_router.instrumentation_api',
68+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
69+
});
70+
71+
try {
72+
const result = await handleRequest();
73+
if (result.status === 'error' && result.error instanceof Error) {
74+
existingRootSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
75+
captureInstrumentationError(result, captureErrors, 'react_router.request_handler', {
76+
'http.method': info.request.method,
77+
'http.url': pathname,
78+
});
79+
}
80+
} finally {
81+
await flushIfServerless();
7282
}
73-
} finally {
74-
await flushIfServerless();
75-
}
76-
} else {
77-
await startSpan(
78-
{
79-
name: `${info.request.method} ${pathname}`,
80-
forceTransaction: true,
81-
attributes: {
82-
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server',
83-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react_router.instrumentation_api',
84-
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
85-
'http.request.method': info.request.method,
86-
'url.path': pathname,
87-
'url.full': info.request.url,
83+
} else {
84+
await startSpan(
85+
{
86+
name: `${info.request.method} ${pathname}`,
87+
forceTransaction: true,
88+
attributes: {
89+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server',
90+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react_router.instrumentation_api',
91+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
92+
'http.request.method': info.request.method,
93+
'url.path': pathname,
94+
'url.full': info.request.url,
95+
},
8896
},
89-
},
90-
async span => {
91-
try {
92-
const result = await handleRequest();
93-
if (result.status === 'error' && result.error instanceof Error) {
94-
span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
95-
captureInstrumentationError(result, captureErrors, 'react_router.request_handler', {
96-
'http.method': info.request.method,
97-
'http.url': pathname,
98-
});
97+
async span => {
98+
try {
99+
const result = await handleRequest();
100+
if (result.status === 'error' && result.error instanceof Error) {
101+
span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
102+
captureInstrumentationError(result, captureErrors, 'react_router.request_handler', {
103+
'http.method': info.request.method,
104+
'http.url': pathname,
105+
});
106+
}
107+
} finally {
108+
await flushIfServerless();
99109
}
100-
} finally {
101-
await flushIfServerless();
102-
}
103-
},
104-
);
105-
}
110+
},
111+
);
112+
}
113+
});
106114
},
107115
});
108116
},
109117

110118
route(route: InstrumentableRoute) {
119+
const routeId = route.id;
120+
111121
route.instrument({
112122
async loader(callLoader, info) {
113123
const urlPath = getPathFromRequest(info.request);
@@ -168,15 +178,29 @@ export function createSentryServerInstrumentation(
168178
const pattern = getPattern(info);
169179
const routePattern = normalizeRoutePath(pattern) || urlPath;
170180

171-
// Update root span with parameterized route (same as loader/action)
172181
updateRootSpanWithRoute(info.request.method, pattern, urlPath);
173182

183+
const counterStore = context.active().getValue(MIDDLEWARE_COUNTER_KEY) as
184+
| { counters: Record<string, number> }
185+
| undefined;
186+
let middlewareIndex = 0;
187+
if (counterStore) {
188+
middlewareIndex = counterStore.counters[routeId] ?? 0;
189+
counterStore.counters[routeId] = middlewareIndex + 1;
190+
}
191+
192+
const middlewareName = getMiddlewareName(routeId, middlewareIndex);
193+
174194
await startSpan(
175195
{
176-
name: routePattern,
196+
name: `middleware ${middlewareName || routeId}`,
177197
attributes: {
178198
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.react_router.middleware',
179199
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api',
200+
'react_router.route.id': routeId,
201+
[ATTR_HTTP_ROUTE]: routePattern,
202+
...(middlewareName && { 'react_router.middleware.name': middlewareName }),
203+
'react_router.middleware.index': middlewareIndex,
180204
},
181205
},
182206
async span => {

‎packages/react-router/src/server/instrumentation/reactRouter.ts‎

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ import {
1515
} from '@sentry/core';
1616
import type * as reactRouter from 'react-router';
1717
import { DEBUG_BUILD } from '../../common/debug-build';
18-
import { isInstrumentationApiUsed } from '../serverGlobals';
18+
import { isServerBuildLike, setServerBuild } from '../serverBuild';
19+
import { isInstrumentationApiUsed, isOtelDataLoaderSpanCreationEnabled } from '../serverGlobals';
1920
import { getOpName, getSpanName, isDataRequest } from './util';
2021

2122
type ReactRouterModuleExports = typeof reactRouter;
@@ -62,9 +63,31 @@ export class ReactRouterInstrumentation extends InstrumentationBase<Instrumentat
6263
if (prop === 'createRequestHandler') {
6364
const original = target[prop];
6465
return function sentryWrappedCreateRequestHandler(this: unknown, ...args: unknown[]) {
66+
// Capture the ServerBuild reference for middleware name lookup
67+
const build = args[0];
68+
if (isServerBuildLike(build)) {
69+
setServerBuild(build);
70+
} else if (typeof build === 'function') {
71+
// Build arg can be a factory function (dev mode HMR). Wrap to capture resolved build.
72+
const originalBuildFn = build as () => unknown;
73+
args[0] = async function sentryWrappedBuildFn() {
74+
const resolvedBuild = await originalBuildFn();
75+
if (isServerBuildLike(resolvedBuild)) {
76+
setServerBuild(resolvedBuild);
77+
}
78+
return resolvedBuild;
79+
};
80+
}
81+
6582
const originalRequestHandler = original.apply(this, args);
6683

6784
return async function sentryWrappedRequestHandler(request: Request, initialContext?: unknown) {
85+
// Skip OTEL span creation when instrumentation API is active or when span creation is not enabled.
86+
// Checked per-request (not at handler-creation time) because in dev, createRequestHandler runs before entry.server.tsx.
87+
if (isInstrumentationApiUsed() || !isOtelDataLoaderSpanCreationEnabled()) {
88+
return originalRequestHandler(request, initialContext);
89+
}
90+
6891
let url: URL;
6992
try {
7093
url = new URL(request.url);
@@ -77,13 +100,6 @@ export class ReactRouterInstrumentation extends InstrumentationBase<Instrumentat
77100
return originalRequestHandler(request, initialContext);
78101
}
79102

80-
// Skip OTEL instrumentation if instrumentation API is being used
81-
// as it handles loader/action spans itself
82-
if (isInstrumentationApiUsed()) {
83-
DEBUG_BUILD && debug.log('Skipping OTEL loader/action instrumentation - using instrumentation API');
84-
return originalRequestHandler(request, initialContext);
85-
}
86-
87103
const activeSpan = getActiveSpan();
88104
const rootSpan = activeSpan && getRootSpan(activeSpan);
89105

0 commit comments

Comments
 (0)