feat(remix): Instrument Remix 3 server requests via fetch-router

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

@@ -30,5 +30,11 @@ export default createController(routes, {
     home(context) {
       return context.render(<HomePage />);
     },
+    user(context) {
+      return Response.json({ id: context.params.id });
+    },
+    teapot() {
+      return new Response("I'm a teapot", { status: 418 });
+    },
   },
 });

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

@@ -19,3 +19,9 @@ export const router = createRouter<AppContext>({
 });
 
 router.map(routes, controller);
+
+// Mounted rather than added to the route map, so the tests cover a route whose pattern carries a mount
+// prefix.
+router.mount('/api', api => {
+  api.get('/items/:itemId', context => Response.json({ itemId: context.params.itemId }));
+});

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

@@ -3,4 +3,6 @@ import { get, route } from 'remix/routes';
 export const routes = route({
   assets: get('/assets/*path'),
   home: '/',
+  user: get('/users/:id'),
+  teapot: get('/teapot'),
 });

dev-packages/e2e-tests/test-applications/remix-v3/tests/server-spans.test.ts

@@ -0,0 +1,51 @@
+import { expect, test } from '@playwright/test';
+import type { SerializedStreamedSpan } from '@sentry-internal/test-utils';
+import { getSpanOp, waitForStreamedSpan } from '@sentry-internal/test-utils';
+
+const APP_NAME = 'remix-v3';
+
+/**
+ * Selecting the span by `http.route` rather than by name is what makes the name assertions below mean
+ * something: a request that never resolved its route would not match, instead of matching and then
+ * passing a name check against whatever it happened to be called.
+ */
+function waitForServerSpan(route: string): Promise<SerializedStreamedSpan> {
+  return waitForStreamedSpan(
+    APP_NAME,
+    span =>
+      getSpanOp(span) === 'http.server' && span.is_segment === true && span.attributes?.['http.route']?.value === route,
+  );
+}
+
+test('names a parameterized route after its pattern', async ({ baseURL }) => {
+  const spanPromise = waitForServerSpan('/users/:id');
+
+  await fetch(`${baseURL}/users/12345`);
+
+  const span = await spanPromise;
+  expect(span.name).toBe('GET /users/:id');
+  // An id in the name would make every request its own transaction.
+  expect(span.name).not.toContain('12345');
+  expect(span.attributes?.['sentry.segment.name.source']?.value).toBe('route');
+});
+
+test('includes the mount prefix in the name of a mounted route', async ({ baseURL }) => {
+  const spanPromise = waitForServerSpan('/api/items/:itemId');
+
+  await fetch(`${baseURL}/api/items/abc`);
+
+  const span = await spanPromise;
+  expect(span.name).toBe('GET /api/items/:itemId');
+  expect(span.name).not.toContain('abc');
+});
+
+test('records the response status', async ({ baseURL }) => {
+  const spanPromise = waitForServerSpan('/teapot');
+
+  await fetch(`${baseURL}/teapot`);
+
+  const span = await spanPromise;
+  expect(span.attributes?.['http.response.status_code']?.value).toBe(418);
+  // OpenTelemetry leaves a 4xx server span unset, so the error status is the SDK's own doing.
+  expect(span.status).toBe('error');
+});

dev-packages/e2e-tests/test-applications/remix-v3/tests/smoke.test.ts

@@ -1,20 +1,7 @@
 import { expect, test } from '@playwright/test';
-import { waitForStreamedSpan } from '@sentry-internal/test-utils';
 
-// There is no instrumentation yet. This app exists so later pull requests add instrumentation and its
-// tests together, rather than also introducing a new CI surface.
 test('the app boots under the Sentry --import entry', async ({ page }) => {
   await page.goto('/');
 
   await expect(page.locator('#home')).toBeVisible();
 });
-
-test('Sentry.init from the v3 subpath reports a server span', async ({ baseURL }) => {
-  // Names are still URL based. This only proves the subpath resolves and the SDK is live.
-  const spanPromise = waitForStreamedSpan('remix-v3', span => span.is_segment === true);
-
-  await fetch(`${baseURL}/`);
-
-  const span = await spanPromise;
-  expect(span.attributes?.['http.request.method']?.value).toBe('GET');
-});

packages/remix/package.json

@@ -74,6 +74,7 @@
     "access": "public"
   },
   "dependencies": {
+    "@remix-run/route-pattern": "^0.24.0",
     "@remix-run/router": "^1.23.4",
     "@sentry/browser": "11.0.0",
     "@sentry/bundler-plugins": "11.0.0",

packages/remix/src/v3/index.server.ts

@@ -1,4 +1,6 @@
-// Placeholder until the Remix 3 server instrumentation lands. `@sentry/node`'s `init` already emits
-// `http.server` spans, so this is useful on its own; route parameterisation and router error capture
-// are what is still missing.
 export * from '@sentry/node';
+
+export { getDefaultIntegrations, init } from './server/sdk';
+export { remixV3Integration } from './server/integration';
+export { sentryRemixMiddleware } from './server/middleware';
+export { instrumentRemixV3 } from './server/instrument';

packages/remix/src/v3/node.mjs

@@ -1,3 +1,13 @@
-// Replaces `--import remix/node-tsx` rather than adding a second flag. Sentry's module hook is
-// registered here once the server instrumentation lands, so for now nothing is instrumented.
+// Replaces `--import remix/node-tsx` rather than adding a second flag.
+//
+// This has to happen here, not in `Sentry.init()`: the module hook must be in place before
+// `@remix-run/fetch-router` is imported, and the subscription before `createRouter()` runs, which is
+// while the app's own modules are still being imported.
+import { registerDiagnosticsChannelInjection } from '@sentry/server-runtime-injection/register';
+
+registerDiagnosticsChannelInjection();
+
+const { instrumentRemixV3 } = await import('@sentry/remix/v3');
+instrumentRemixV3();
+
 await import('remix/node-tsx');

packages/remix/src/v3/server/instrument.ts

@@ -0,0 +1,91 @@
+import * as diagnosticsChannel from 'node:diagnostics_channel';
+import { createMultiMatcher } from '@remix-run/route-pattern/match';
+import { remixV3Channels } from '@sentry/server-utils/orchestrion/config';
+
+import type { MatcherLike, RouterOptionsLike } from '../types';
+import { sentryRemixMiddleware } from './middleware';
+
+const NOOP = (): void => {};
+
+/** The call's arguments, as orchestrion's transform attaches them to a tracing channel context. */
+interface ChannelContext {
+  arguments: unknown[];
+}
+
+// Marks an options object already injected into, so the same mutation cannot be applied twice.
+const INJECTED = Symbol.for('SentryRemixV3Injected');
+
+// `subscribe()` takes a fresh object literal and the channel keys handlers by identity, so nothing else
+// stops a second call adding a second set. The documented setup calls this twice, from the `--import`
+// entry and from `setupOnce()`.
+let subscribed = false;
+
+/**
+ * Prepend the Sentry middleware to every router an app builds.
+ *
+ * Injection happens on the channel's `start` event, before `createRouter` reads its options.
+ * Orchestrion's transform collects the arguments into an array and spreads them back into the call, so
+ * assigning at an index the caller never passed works. That is what covers `createRouter()` with no
+ * arguments at all.
+ */
+export function instrumentRemixV3(): void {
+  if (subscribed || !diagnosticsChannel.tracingChannel) {
+    return;
+  }
+  subscribed = true;
+
+  diagnosticsChannel.tracingChannel<ChannelContext, ChannelContext>(remixV3Channels.REMIX_V3_CREATE_ROUTER).subscribe({
+    start(data) {
+      // Node rethrows anything this handler throws as an uncaught exception, which would kill an app
+      // that runs fine without Sentry. Frozen options, a non-writable property and a `route-pattern`
+      // copy that cannot build a matcher all reach here, so the router is left uninstrumented instead.
+      try {
+        injectRouterMiddleware(ensureOptions(data.arguments));
+      } catch {
+        // Ignored on purpose.
+      }
+    },
+    end: NOOP,
+    asyncStart: NOOP,
+    asyncEnd: NOOP,
+    error: NOOP,
+  });
+}
+
+/**
+ * The options object, created when the caller omitted it. `undefined` when the caller passed something
+ * that is not an options object, which must not be overwritten.
+ */
+function ensureOptions(args: unknown[]): Record<string, unknown> | undefined {
+  const existing = args[0];
+
+  if (existing === undefined || existing === null) {
+    const created: Record<string, unknown> = {};
+    args[0] = created;
+    return created;
+  }
+
+  return typeof existing === 'object' ? (existing as Record<string, unknown>) : undefined;
+}
+
+function injectRouterMiddleware(raw: Record<string, unknown> | undefined): void {
+  if (!raw) {
+    return;
+  }
+
+  const marker = raw as { [INJECTED]?: boolean };
+  if (marker[INJECTED]) {
+    return;
+  }
+  marker[INJECTED] = true;
+
+  const options = raw as RouterOptionsLike;
+
+  // The router never exposes its matcher, and resolving the route pattern needs one, so supplying it is
+  // the only way to hold a reference. An app that supplied its own keeps it.
+  const matcher: MatcherLike = options.matcher ?? (createMultiMatcher() as MatcherLike);
+  options.matcher = matcher;
+
+  // Prepended rather than appended, so the Sentry middleware wraps the app's own.
+  options.middleware = [sentryRemixMiddleware(matcher), ...(options.middleware ?? [])];
+}

packages/remix/src/v3/server/integration.ts

@@ -0,0 +1,22 @@
+import { defineIntegration, type IntegrationFn } from '@sentry/core';
+
+import { instrumentRemixV3 } from './instrument';
+
+const INTEGRATION_NAME = 'RemixV3' as const;
+
+const _remixV3Integration = (() => {
+  return {
+    name: INTEGRATION_NAME,
+    setupOnce() {
+      // Usually a no-op: `createRouter()` runs while the app's modules are imported, before any
+      // `init()`, so `--import @sentry/remix/v3/node` has already subscribed. This covers setups that
+      // register the module hook from `init()` instead.
+      instrumentRemixV3();
+    },
+  };
+}) satisfies IntegrationFn;
+
+/**
+ * Names the `http.server` spans `@sentry/node` opens after the matched Remix 3 route.
+ */
+export const remixV3Integration = defineIntegration(_remixV3Integration);

packages/remix/src/v3/server/middleware.ts

@@ -0,0 +1,69 @@
+import { HTTP_RESPONSE_STATUS_CODE, HTTP_ROUTE } from '@sentry/conventions/attributes';
+import {
+  getActiveSpan,
+  getIsolationScope,
+  getRootSpan,
+  getSpanStatusFromHttpCode,
+  INTERNAL_setSegmentNameSourceIfSegment,
+  type Scope,
+  updateSpanName,
+  winterCGRequestToRequestData,
+} from '@sentry/core';
+
+import type { MatcherLike, MiddlewareLike, NextFunctionLike, RequestContextLike } from '../types';
+import { resolveRoutePattern } from './route';
+
+/**
+ * The middleware orchestrion prepends to every router.
+ *
+ * It opens no span. Remix 3 serves over `node:http`, so `httpIntegration` has already opened the
+ * `http.server` span and forked an isolation scope by the time this runs. Opening another would double
+ * count every request, so this only enriches what exists, as `@sentry/hono` does.
+ */
+export function sentryRemixMiddleware(matcher: MatcherLike): MiddlewareLike {
+  return async function sentryMiddleware(context: RequestContextLike, next: NextFunctionLike): Promise<Response> {
+    const isolationScope = getIsolationScope();
+    isolationScope.setSDKProcessingMetadata({ normalizedRequest: winterCGRequestToRequestData(context.request) });
+
+    // Applied before `next()` so anything captured while the handler runs already carries the route.
+    applyRoute(isolationScope, matcher, context);
+
+    const response = await next();
+    setResponseStatus(response);
+
+    return response;
+  };
+}
+
+function applyRoute(isolationScope: Scope, matcher: MatcherLike, context: RequestContextLike): void {
+  const route = resolveRoutePattern(matcher, context);
+  if (!route) {
+    // Nothing matched, so the router falls through to its 404 handler. Leaving the name alone keeps the
+    // raw URL out of it, which span streaming requires.
+    return;
+  }
+
+  const name = `${context.method} ${route}`;
+  isolationScope.setTransactionName(name);
+
+  const activeSpan = getActiveSpan();
+  if (!activeSpan) {
+    return;
+  }
+
+  const rootSpan = getRootSpan(activeSpan);
+  updateSpanName(rootSpan, name);
+  INTERNAL_setSegmentNameSourceIfSegment(rootSpan, 'route');
+  rootSpan.setAttribute(HTTP_ROUTE, route);
+}
+
+function setResponseStatus(response: Response): void {
+  const activeSpan = getActiveSpan();
+  if (!activeSpan || typeof response?.status !== 'number') {
+    return;
+  }
+
+  const rootSpan = getRootSpan(activeSpan);
+  rootSpan.setAttribute(HTTP_RESPONSE_STATUS_CODE, response.status);
+  rootSpan.setStatus(getSpanStatusFromHttpCode(response.status));
+}

packages/remix/src/v3/server/route.ts

@@ -0,0 +1,37 @@
+import type { MatcherLike, RequestContextLike } from '../types';
+
+/**
+ * Resolve the low cardinality route pattern for a request.
+ *
+ * The request context never carries the matched pattern: the router matches after its middleware is
+ * entered, and only writes `params` back. So this re-runs the match against the router's own matcher,
+ * applying the router's own selection rules.
+ */
+export function resolveRoutePattern(matcher: MatcherLike, context: RequestContextLike): string | undefined {
+  let matches;
+  try {
+    matches = matcher.matchAll(context.url);
+  } catch {
+    // A matcher from a mismatched `@remix-run/route-pattern` copy can throw on a shape it does not
+    // recognise. Losing the route name is not worth failing the request over.
+    return undefined;
+  }
+
+  let headFallback: string | undefined;
+
+  for (const match of matches) {
+    const method = match.data?.method;
+
+    if (method === context.method || method === 'ANY') {
+      return match.data?.pattern?.source;
+    }
+
+    // A GET route also serves HEAD. The router compares specificity to pick between several; for a
+    // span name either pattern is equally correct, so the first one wins here.
+    if (context.method === 'HEAD' && method === 'GET') {
+      headFallback ??= match.data?.pattern?.source;
+    }
+  }
+
+  return headFallback;
+}

packages/remix/src/v3/server/sdk.ts

@@ -0,0 +1,32 @@
+import { applySdkMetadata, type Integration } from '@sentry/core';
+import {
+  getDefaultIntegrations as getNodeDefaultIntegrations,
+  init as nodeInit,
+  type NodeClient,
+  type NodeOptions,
+} from '@sentry/node';
+
+import { remixV3Integration } from './integration';
+
+/** Default integrations for the Remix 3 server SDK. */
+export function getDefaultIntegrations(options: NodeOptions): Integration[] {
+  return [...getNodeDefaultIntegrations(options), remixV3Integration()];
+}
+
+/**
+ * Initialize the Sentry Remix 3 SDK on the server.
+ *
+ * Start the app with `--import @sentry/remix/v3/node` so the module hook is registered before
+ * `@remix-run/fetch-router` is imported. Remix 3 has no build step, so no bundler plugin can apply the
+ * transform, and imports are hoisted above any `init()` call in the server entry.
+ */
+export function init(options: NodeOptions): NodeClient | undefined {
+  const opts = {
+    ...options,
+    defaultIntegrations: options.defaultIntegrations ?? getDefaultIntegrations(options),
+  };
+
+  applySdkMetadata(opts, 'remix', ['remix', 'node']);
+
+  return nodeInit(opts);
+}