Skip to content

Commit e8bed82

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 bff1fba commit e8bed82

19 files changed

Lines changed: 559 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
@@ -74,6 +74,7 @@
7474
"access": "public"
7575
},
7676
"dependencies": {
77+
"@remix-run/route-pattern": "^0.24.0",
7778
"@remix-run/router": "^1.23.4",
7879
"@sentry/browser": "11.1.0",
7980
"@sentry/bundler-plugins": "11.1.0",
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: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
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+
const { instrumentRemixV3 } = await import('@sentry/remix/v3');
11+
instrumentRemixV3();
12+
313
await import('remix/node-tsx');
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-run/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+
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import { defineIntegration, type IntegrationFn } from '@sentry/core';
2+
3+
import { instrumentRemixV3 } from './instrument';
4+
5+
const INTEGRATION_NAME = 'RemixV3' as const;
6+
7+
const _remixV3Integration = (() => {
8+
return {
9+
name: INTEGRATION_NAME,
10+
setupOnce() {
11+
// Usually a no-op: `createRouter()` runs while the app's modules are imported, before any
12+
// `init()`, so `--import @sentry/remix/v3/node` has already subscribed. This covers setups that
13+
// register the module hook from `init()` instead.
14+
instrumentRemixV3();
15+
},
16+
};
17+
}) satisfies IntegrationFn;
18+
19+
/**
20+
* Names the `http.server` spans `@sentry/node` opens after the matched Remix 3 route.
21+
*/
22+
export const remixV3Integration = defineIntegration(_remixV3Integration);

0 commit comments

Comments
 (0)