Skip to content

Commit 2cd5555

Browse files
chargomeclaude
andcommitted
feat(remix): Instrument Remix 3 server requests via fetch-router
Patches `createRouter` through orchestrion to prepend a Sentry middleware to every router. The middleware opens no span: Remix 3 serves over `node:http`, so `httpIntegration` has already opened the `http.server` span. It only enriches it with the route, the response status and a low cardinality name. The matcher has to be supplied by the SDK because the router never exposes one, and the request context carries no pattern at middleware entry. Matching happens after router middleware runs and only writes back `params`, so the route is resolved by re-running the match. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 98ec686 commit 2cd5555

21 files changed

Lines changed: 582 additions & 18 deletions

File tree

‎dev-packages/e2e-tests/test-applications/remix-v3/app/actions/controller.tsx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,5 +30,11 @@ export default createController(routes, {
3030
home(context) {
3131
return context.render(<HomePage />);
3232
},
33+
user(context) {
34+
return Response.json({ id: context.params.id });
35+
},
36+
teapot() {
37+
return new Response("I'm a teapot", { status: 418 });
38+
},
3339
},
3440
});

‎dev-packages/e2e-tests/test-applications/remix-v3/app/router.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,9 @@ export const router = createRouter<AppContext>({
1919
});
2020

2121
router.map(routes, controller);
22+
23+
// Mounted rather than added to the route map, so the tests cover a route whose pattern carries a mount
24+
// prefix.
25+
router.mount('/api', api => {
26+
api.get('/items/:itemId', context => Response.json({ itemId: context.params.itemId }));
27+
});

‎dev-packages/e2e-tests/test-applications/remix-v3/app/routes.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,6 @@ import { get, route } from 'remix/routes';
33
export const routes = route({
44
assets: get('/assets/*path'),
55
home: '/',
6+
user: get('/users/:id'),
7+
teapot: get('/teapot'),
68
});
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import { expect, test } from '@playwright/test';
2+
import type { SerializedStreamedSpan } from '@sentry-internal/test-utils';
3+
import { getSpanOp, waitForStreamedSpan } from '@sentry-internal/test-utils';
4+
5+
const APP_NAME = 'remix-v3';
6+
7+
/**
8+
* Selecting the span by `http.route` rather than by name is what makes the name assertions below mean
9+
* something: a request that never resolved its route would not match, instead of matching and then
10+
* passing a name check against whatever it happened to be called.
11+
*/
12+
function waitForServerSpan(route: string): Promise<SerializedStreamedSpan> {
13+
return waitForStreamedSpan(
14+
APP_NAME,
15+
span =>
16+
getSpanOp(span) === 'http.server' && span.is_segment === true && span.attributes?.['http.route']?.value === route,
17+
);
18+
}
19+
20+
test('names a parameterized route after its pattern', async ({ baseURL }) => {
21+
const spanPromise = waitForServerSpan('/users/:id');
22+
23+
await fetch(`${baseURL}/users/12345`);
24+
25+
const span = await spanPromise;
26+
expect(span.name).toBe('GET /users/:id');
27+
// An id in the name would make every request its own transaction.
28+
expect(span.name).not.toContain('12345');
29+
expect(span.attributes?.['sentry.segment.name.source']?.value).toBe('route');
30+
});
31+
32+
test('includes the mount prefix in the name of a mounted route', async ({ baseURL }) => {
33+
const spanPromise = waitForServerSpan('/api/items/:itemId');
34+
35+
await fetch(`${baseURL}/api/items/abc`);
36+
37+
const span = await spanPromise;
38+
expect(span.name).toBe('GET /api/items/:itemId');
39+
expect(span.name).not.toContain('abc');
40+
});
41+
42+
test('records the response status', async ({ baseURL }) => {
43+
const spanPromise = waitForServerSpan('/teapot');
44+
45+
await fetch(`${baseURL}/teapot`);
46+
47+
const span = await spanPromise;
48+
expect(span.attributes?.['http.response.status_code']?.value).toBe(418);
49+
// OpenTelemetry leaves a 4xx server span unset, so the error status is the SDK's own doing.
50+
expect(span.status).toBe('error');
51+
});
Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,7 @@
11
import { expect, test } from '@playwright/test';
2-
import { waitForStreamedSpan } from '@sentry-internal/test-utils';
32

4-
// There is no instrumentation yet. This app exists so later pull requests add instrumentation and its
5-
// tests together, rather than also introducing a new CI surface.
63
test('the app boots under the Sentry --import entry', async ({ page }) => {
74
await page.goto('/');
85

96
await expect(page.locator('#home')).toBeVisible();
107
});
11-
12-
test('Sentry.init from the v3 subpath reports a server span', async ({ baseURL }) => {
13-
// Names are still URL based. This only proves the subpath resolves and the SDK is live.
14-
const spanPromise = waitForStreamedSpan('remix-v3', span => span.is_segment === true);
15-
16-
await fetch(`${baseURL}/`);
17-
18-
const span = await spanPromise;
19-
expect(span.attributes?.['http.request.method']?.value).toBe('GET');
20-
});

‎packages/remix/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@
8989
"devDependencies": {
9090
"@remix-run/node": "^2.17.5",
9191
"@remix-run/react": "^2.17.5",
92+
"@remix-run/route-pattern": "^0.24.0",
9293
"@remix-run/server-runtime": "^2.17.4",
9394
"@types/express": "^4.17.14",
9495
"react": "^18.3.1",
Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
// Placeholder until the Remix 3 server instrumentation lands. `@sentry/node`'s `init` already emits
2-
// `http.server` spans, so this is useful on its own; route parameterisation and router error capture
3-
// are what is still missing.
41
export * from '@sentry/node';
2+
3+
export { getDefaultIntegrations, init } from './server/sdk';
4+
export { remixV3Integration } from './server/integration';
5+
export { sentryRemixMiddleware } from './server/middleware';
6+
export { instrumentRemixV3 } from './server/instrument';

‎packages/remix/src/v3/node.mjs‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,20 @@
1-
// Replaces `--import remix/node-tsx` rather than adding a second flag. Sentry's module hook is
2-
// registered here once the server instrumentation lands, so for now nothing is instrumented.
1+
// Replaces `--import remix/node-tsx` rather than adding a second flag.
2+
//
3+
// This has to happen here, not in `Sentry.init()`: the module hook must be in place before
4+
// `@remix-run/fetch-router` is imported, and the subscription before `createRouter()` runs, which is
5+
// while the app's own modules are still being imported.
6+
import { registerDiagnosticsChannelInjection } from '@sentry/server-runtime-injection/register';
7+
8+
registerDiagnosticsChannelInjection();
9+
10+
// A failure to load the SDK must not stop the app: this runs before anything of the app has, and an
11+
// uncaught error here means Remix never starts.
12+
try {
13+
const { instrumentRemixV3 } = await import('@sentry/remix/v3');
14+
instrumentRemixV3();
15+
} catch (error) {
16+
// oxlint-disable-next-line no-console
17+
console.warn('[Sentry] Could not load @sentry/remix/v3. The app starts without Sentry instrumentation.', error);
18+
}
19+
320
await import('remix/node-tsx');

‎packages/remix/src/v3/remix.d.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// `remix` is an optional peer, so it is not installed here. Only the one subpath the SDK imports is
2+
// declared, with the shape the SDK relies on. Importing it from `remix` rather than from
3+
// `@remix-run/route-pattern` gives the SDK the copy the app's router uses, and adds no dependency for
4+
// Remix 2 apps.
5+
declare module 'remix/route-pattern/match' {
6+
export function createMultiMatcher(): {
7+
add(pattern: unknown, data: unknown): void;
8+
matchAll(url: string | URL): unknown[];
9+
};
10+
}
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
import * as diagnosticsChannel from 'node:diagnostics_channel';
2+
import { createMultiMatcher } from 'remix/route-pattern/match';
3+
import { remixV3Channels } from '@sentry/server-utils/orchestrion/config';
4+
5+
import type { MatcherLike, RouterOptionsLike } from '../types';
6+
import { sentryRemixMiddleware } from './middleware';
7+
8+
const NOOP = (): void => {};
9+
10+
/** The call's arguments, as orchestrion's transform attaches them to a tracing channel context. */
11+
interface ChannelContext {
12+
arguments: unknown[];
13+
}
14+
15+
// Marks an options object already injected into, so the same mutation cannot be applied twice.
16+
const INJECTED = Symbol.for('SentryRemixV3Injected');
17+
18+
// `subscribe()` takes a fresh object literal and the channel keys handlers by identity, so nothing else
19+
// stops a second call adding a second set. The documented setup calls this twice, from the `--import`
20+
// entry and from `setupOnce()`.
21+
let subscribed = false;
22+
23+
/**
24+
* Prepend the Sentry middleware to every router an app builds.
25+
*
26+
* Injection happens on the channel's `start` event, before `createRouter` reads its options.
27+
* Orchestrion's transform collects the arguments into an array and spreads them back into the call, so
28+
* assigning at an index the caller never passed works. That is what covers `createRouter()` with no
29+
* arguments at all.
30+
*/
31+
export function instrumentRemixV3(): void {
32+
if (subscribed || !diagnosticsChannel.tracingChannel) {
33+
return;
34+
}
35+
subscribed = true;
36+
37+
diagnosticsChannel.tracingChannel<ChannelContext, ChannelContext>(remixV3Channels.REMIX_V3_CREATE_ROUTER).subscribe({
38+
start(data) {
39+
// Node rethrows anything this handler throws as an uncaught exception, which would kill an app
40+
// that runs fine without Sentry. Frozen options, a non-writable property and a `route-pattern`
41+
// copy that cannot build a matcher all reach here, so the router is left uninstrumented instead.
42+
try {
43+
injectRouterMiddleware(ensureOptions(data.arguments));
44+
} catch {
45+
// Ignored on purpose.
46+
}
47+
},
48+
end: NOOP,
49+
asyncStart: NOOP,
50+
asyncEnd: NOOP,
51+
error: NOOP,
52+
});
53+
}
54+
55+
/**
56+
* The options object, created when the caller omitted it. `undefined` when the caller passed something
57+
* that is not an options object, which must not be overwritten.
58+
*/
59+
function ensureOptions(args: unknown[]): Record<string, unknown> | undefined {
60+
const existing = args[0];
61+
62+
if (existing === undefined || existing === null) {
63+
const created: Record<string, unknown> = {};
64+
args[0] = created;
65+
return created;
66+
}
67+
68+
return typeof existing === 'object' ? (existing as Record<string, unknown>) : undefined;
69+
}
70+
71+
function injectRouterMiddleware(raw: Record<string, unknown> | undefined): void {
72+
if (!raw) {
73+
return;
74+
}
75+
76+
const marker = raw as { [INJECTED]?: boolean };
77+
if (marker[INJECTED]) {
78+
return;
79+
}
80+
marker[INJECTED] = true;
81+
82+
const options = raw as RouterOptionsLike;
83+
84+
// The router never exposes its matcher, and resolving the route pattern needs one, so supplying it is
85+
// the only way to hold a reference. An app that supplied its own keeps it.
86+
const matcher: MatcherLike = options.matcher ?? (createMultiMatcher() as MatcherLike);
87+
options.matcher = matcher;
88+
89+
// Prepended rather than appended, so the Sentry middleware wraps the app's own.
90+
options.middleware = [sentryRemixMiddleware(matcher), ...(options.middleware ?? [])];
91+
}

0 commit comments

Comments
 (0)