feat(remix): Emit low cardinality function span names

MIGRATION.md

@@ -1001,6 +1001,7 @@ The following span names were adjusted:
 | `http.client`, `http.client.stream`                                      | The request method and sanitized URL                                                                      | `GET https://api.example.com/users/123`                                        | The request method and the domain, or just the method if there is no domain                                                                                                         | `GET api.example.com`, `GET`                           |
 | `router`                                                                 | Framework-specific, sometimes containing the raw URL                                                      | `/users/123`, `SvelteKit Route Change`                                         | The span's `http.route`, or `Router` if the SDK has none                                                                                                                            | `/users/:id`, `Router`                                 |
 | `handler`                                                                | Framework-specific, often carrying the request method                                                     | `GET /users/:id`, `route-handler`, `getUser`                                   | The span's `http.route`, or `Request handler` if the SDK has none                                                                                                                   | `/users/:id`, `Request handler`                        |
+| `function`                                                               | Integration-specific, sometimes the segment span's name                                                   | `serverAction/updateUser`, `LOADER routes/users.$id`                           | The span's `code.function.name`. The previous name is kept as the span description                                                                                                  | `updateUser`, `loader`                                 |
 | `function.gcp`                                                           | The request method and path for HTTP functions, otherwise the trigger's event or trigger type             | `POST /users`, `google.pubsub.topic.publish`, `firebase.function.http.request` | The function name, or `Serverless function execution` if the SDK cannot resolve one                                                                                                 | `myFunction`, `Serverless function execution`          |
 | `function.aws`                                                           | The Lambda function name                                                                                  | `my-function`                                                                  | Unchanged, except that the SDK now falls back to `Serverless function execution` if it cannot resolve the function name                                                             | `my-function`, `Serverless function execution`         |
 | `graphql`                                                                | The graphql phase and, for operations, the operation name                                                 | `query GetUser`, `graphql.parse`, `graphql.resolve user.0.name`                | The operation type, or the processing type where there is none                                                                                                                      | `GraphQL query`, `GraphQL parse`, `GraphQL resolve`    |

dev-packages/e2e-tests/test-applications/create-remix-app-express/tests/server-transactions.test.ts

@@ -50,6 +50,7 @@ test('Sends form data with the action span', async ({ page }) => {
 
   expect(actionSpan).toBeDefined();
   expect(getSpanOp(actionSpan!)).toBe('function');
+  expect(actionSpan!.name).toBe('action');
   expect(actionSpan!.attributes).toMatchObject({
     'remix.action_form_data.text': { value: 'test', type: 'string' },
     'remix.action_form_data.file': { value: 'file.txt', type: 'string' },
@@ -71,6 +72,32 @@ test('Sends a loader span to Sentry', async ({ page }) => {
 
   expect(loaderSpan).toBeDefined();
   expect(getSpanOp(loaderSpan!)).toBe('function');
+  expect(loaderSpan!.name).toBe('loader');
+  // The route id left the span name, so it has to stay reachable as an attribute.
+  expect(loaderSpan!.attributes['match.route.id']?.value).toEqual(expect.any(String));
+});
+
+test('Sends a low cardinality documentRequest span to Sentry', async ({ page }) => {
+  // Other tests hit `/user/:id` too, and `collectStreamedSpans` evaluates one trace at a time, so a
+  // unique path is what keeps this assertion on the request under test.
+  const path = `/user/${crypto.randomUUID()}`;
+  const spansPromise = collectStreamedSpansUntilSegment(
+    APP_NAME,
+    span => getSpanOp(span) === 'http.server' && span.attributes['url.path']?.value === path,
+  );
+
+  await page.goto(path);
+
+  const spans = await spansPromise;
+  const segment = spans.find(span => span.is_segment)!;
+  const documentRequestSpan = spans.find(span => span.attributes['code.function.name']?.value === 'documentRequest');
+
+  expect(documentRequestSpan).toBeDefined();
+  expect(documentRequestSpan!.name).toBe('documentRequest');
+  expect(getSpanOp(documentRequestSpan!)).toBe('function');
+  expect(documentRequestSpan!.attributes['sentry.origin']?.value).toBe('auto.function.remix');
+  // The span name is low cardinality now, so the description carries the name it used to have.
+  expect(documentRequestSpan!.attributes['sentry.description']?.value).toBe(segment.name);
 });
 
 test('Propagates the trace when the ErrorBoundary is triggered', async ({ page }) => {

dev-packages/e2e-tests/test-applications/create-remix-app-v2-static/tests/server-transactions.test.ts

@@ -56,3 +56,17 @@ test('Sends two linked transactions (server & client) to Sentry', async ({ page
   expect(pageLoadParentSpanId).toEqual(loaderSpanId);
   expect(pageLoadSpanId).not.toEqual(httpServerSpanId);
 });
+
+test('Keeps the documentRequest span name unchanged with static spans', async ({ page }) => {
+  const transactionPromise = waitForTransaction('create-remix-app-v2-static', transactionEvent => {
+    return transactionEvent.contexts?.trace?.op === 'http.server' && transactionEvent.transaction === 'GET user/:id';
+  });
+
+  await page.goto('/user/123');
+
+  const transaction = await transactionPromise;
+  const documentRequestSpan = transaction.spans?.find(span => span.data?.['code.function.name'] === 'documentRequest');
+
+  expect(documentRequestSpan).toBeDefined();
+  expect(documentRequestSpan?.description).toBe('GET user/:id');
+});

packages/remix/src/server/instrumentServer.ts

@@ -41,8 +41,9 @@ import { extractData, isResponse, json } from '../utils/vendor/response';
 import { captureRemixServerException, errorHandleDataFunction } from './errors';
 import { generateSentryServerTimingHeader, injectServerTimingHeaderValue } from './serverTimingTracePropagation';
 import {
-  SENTRY_SEGMENT_NAME_SOURCE,
   CODE_FUNCTION_NAME,
+  SENTRY_DESCRIPTION,
+  SENTRY_SEGMENT_NAME_SOURCE,
   HTTP_ROUTE,
   SENTRY_OP,
   URL_FULL,
@@ -131,19 +132,22 @@ function makeWrappedDocumentRequestFunction(instrumentTracing?: boolean) {
       if (instrumentTracing) {
         const activeSpan = getActiveSpan();
         const rootSpan = activeSpan && getRootSpan(activeSpan);
-        const name = rootSpan ? spanToJSON(rootSpan).name : undefined;
+        const client = getClient();
+
+        const description = (rootSpan ? spanToJSON(rootSpan).name : undefined) || '<unknown>';
+        const name = client && hasSpanStreamingEnabled(client) ? 'documentRequest' : description;
 
         response = await startSpan(
           {
-            // If we don't have a root span, `onlyIfParent` will lead to the span not being created anyhow
-            // So we don't need to care too much about the fallback name, it's just for typing purposes....
-            name: name || '<unknown>',
+            name,
             onlyIfParent: true,
             attributes: {
               method: request.method,
               [URL_FULL]: filterCollectedUrl(request.url),
               [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.remix',
               [SENTRY_OP]: FUNCTION,
+              [SENTRY_DESCRIPTION]: description,
+              [CODE_FUNCTION_NAME]: 'documentRequest',
             },
           },
           () => {
@@ -218,14 +222,21 @@ function makeWrappedDataFunction(
         updateSpanWithRoute(args, build);
       }
 
+      const client = getClient();
+
       res = await startSpan(
         {
-          name: id,
+          // With span streaming, a `function` span is named after the function it wraps. The route
+          // module id stays on `router.navigation.route.id`.
+          name: client && hasSpanStreamingEnabled(client) ? name : id,
           attributes: {
             [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.remix',
             [SENTRY_OP]: FUNCTION,
+            [SENTRY_DESCRIPTION]: id,
             [CODE_FUNCTION_NAME]: name,
-            name,
+            // TODO: use conventions constant once `router.navigation.route.id` is in
+            // `@sentry/conventions`.
+            'router.navigation.route.id': id,
           },
         },
         (span: Span) => {

packages/remix/src/server/integrations/tracing-channel.ts

@@ -20,6 +20,7 @@ import {
   SENTRY_SEGMENT_NAME_SOURCE,
   CODE_FUNCTION_NAME,
   HTTP_ROUTE,
+  SENTRY_DESCRIPTION,
   URL_FULL,
   URL_PATH,
   SENTRY_KIND,
@@ -191,11 +192,16 @@ function subscribeCallRouteLoader(): void {
     diagnosticsChannel.tracingChannel(remixChannels.REMIX_CALL_ROUTE_LOADER),
     data => {
       const params = (data.arguments[0] ?? {}) as RouteCallParams;
+      const client = getClient();
+      const description = `LOADER ${params.routeId}`;
       return startInactiveSpan({
-        name: `LOADER ${params.routeId}`,
+        // With span streaming, a `function` span is named after the function it wraps. The route id
+        // stays on `match.route.id`.
+        name: client && hasSpanStreamingEnabled(client) ? 'loader' : description,
         attributes: {
           [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN,
           [SENTRY_OP]: FUNCTION,
+          [SENTRY_DESCRIPTION]: description,
           [CODE_FUNCTION_NAME]: 'loader',
           ...getRequestAttributes(params.request),
           ...getMatchAttributes(params),
@@ -225,11 +231,16 @@ function subscribeCallRouteAction(formDataCapture: FormDataCapture | undefined):
         formData.catch(() => undefined);
         data._sentryFormData = formData;
       }
+      const client = getClient();
+      const description = `ACTION ${params.routeId}`;
       return startInactiveSpan({
-        name: `ACTION ${params.routeId}`,
+        // With span streaming, a `function` span is named after the function it wraps. The route id
+        // stays on `match.route.id`.
+        name: client && hasSpanStreamingEnabled(client) ? 'action' : description,
         attributes: {
           [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN,
           [SENTRY_OP]: FUNCTION,
+          [SENTRY_DESCRIPTION]: description,
           [CODE_FUNCTION_NAME]: 'action',
           ...getRequestAttributes(params.request),
           ...getMatchAttributes(params),

packages/remix/test/server/instrumentServer.test.ts

@@ -1,5 +1,6 @@
 import type { LoaderFunctionArgs, ServerBuild } from '@remix-run/server-runtime';
-import { HTTP_ROUTE } from '@sentry/conventions/attributes';
+import { CODE_FUNCTION_NAME, HTTP_ROUTE, SENTRY_DESCRIPTION, SENTRY_OP } from '@sentry/conventions/attributes';
+import { FUNCTION } from '@sentry/conventions/op';
 import type { Span } from '@sentry/core';
 import * as SentryCore from '@sentry/core';
 import { afterEach, describe, expect, it, vi } from 'vitest';
@@ -43,4 +44,97 @@ describe('instrumentBuild', () => {
 
     expect(rootSpan.setAttribute).toHaveBeenCalledWith(HTTP_ROUTE, '/users/:id');
   });
+
+  describe('documentRequest span', () => {
+    function instrumentDocumentRequest(): (request: Request) => Promise<Response> {
+      const rootSpan = { setAttribute: vi.fn(), updateName: vi.fn() } as unknown as Span;
+      vi.spyOn(SentryCore, 'getActiveSpan').mockReturnValue(rootSpan);
+      vi.spyOn(SentryCore, 'getRootSpan').mockReturnValue(rootSpan);
+      vi.spyOn(SentryCore, 'spanToJSON').mockReturnValue({ name: 'GET /users/:id' });
+
+      const build = {
+        entry: { module: { default: vi.fn(async () => new Response('ok')) } },
+        routes: {},
+      } as unknown as ServerBuild;
+
+      return instrumentBuild(build, { instrumentTracing: true }).entry.module.default as never;
+    }
+
+    it.each([
+      { lifecycle: 'streaming', streamed: true, expectedName: 'documentRequest' },
+      { lifecycle: 'static', streamed: false, expectedName: 'GET /users/:id' },
+    ])('names the span $expectedName with $lifecycle spans', async ({ streamed, expectedName }) => {
+      vi.spyOn(SentryCore, 'getClient').mockReturnValue({} as never);
+      vi.spyOn(SentryCore, 'hasSpanStreamingEnabled').mockReturnValue(streamed);
+      const startSpan = vi
+        .spyOn(SentryCore, 'startSpan')
+        .mockImplementation((_options, callback) => callback(undefined as unknown as Span));
+
+      await instrumentDocumentRequest()(new Request('https://example.com/users/42'));
+
+      expect(startSpan).toHaveBeenCalledWith(
+        expect.objectContaining({
+          name: expectedName,
+          attributes: expect.objectContaining({
+            [SENTRY_OP]: FUNCTION,
+            // The description keeps the name the span had before it went low cardinality.
+            [SENTRY_DESCRIPTION]: 'GET /users/:id',
+            [CODE_FUNCTION_NAME]: 'documentRequest',
+          }),
+        }),
+        expect.any(Function),
+      );
+    });
+  });
+
+  describe('loader and action spans', () => {
+    function instrumentRouteLoader(): (args: LoaderFunctionArgs) => Promise<unknown> {
+      const build = {
+        entry: { module: {} },
+        routes: {
+          root: { id: 'root', module: {} },
+          'routes/users.$id': {
+            id: 'routes/users.$id',
+            parentId: 'root',
+            path: 'users/:id',
+            module: { loader: vi.fn(async () => ({})) },
+          },
+        },
+      } as unknown as ServerBuild;
+
+      return instrumentBuild(build, { instrumentTracing: true }).routes['routes/users.$id']!.module.loader as never;
+    }
+
+    it.each([
+      { lifecycle: 'streaming', streamed: true, expectedName: 'loader' },
+      { lifecycle: 'static', streamed: false, expectedName: 'routes/users.$id' },
+    ])('names the span $expectedName with $lifecycle spans', async ({ streamed, expectedName }) => {
+      vi.spyOn(SentryCore, 'getClient').mockReturnValue({} as never);
+      vi.spyOn(SentryCore, 'hasSpanStreamingEnabled').mockReturnValue(streamed);
+      const startSpan = vi
+        .spyOn(SentryCore, 'startSpan')
+        .mockImplementation((_options, callback) => callback(undefined as unknown as Span));
+
+      await instrumentRouteLoader()({
+        context: {},
+        params: { id: '42' },
+        request: new Request('https://example.com/users/42'),
+      } as LoaderFunctionArgs);
+
+      expect(startSpan).toHaveBeenCalledWith(
+        expect.objectContaining({
+          name: expectedName,
+          attributes: expect.objectContaining({
+            [SENTRY_OP]: FUNCTION,
+            // The description keeps the name the span had before it went low cardinality.
+            [SENTRY_DESCRIPTION]: 'routes/users.$id',
+            [CODE_FUNCTION_NAME]: 'loader',
+            // The route id left the span name, so it has to stay reachable as an attribute.
+            'router.navigation.route.id': 'routes/users.$id',
+          }),
+        }),
+        expect.any(Function),
+      );
+    });
+  });
 });

packages/remix/test/server/tracing-channel.test.ts

@@ -113,6 +113,7 @@ describe('remixIntegration (Orchestrion-based)', () => {
       expect.objectContaining({
         name: 'LOADER routes/users.$userId',
         attributes: expect.objectContaining({
+          'sentry.description': 'LOADER routes/users.$userId',
           'sentry.origin': 'auto.http.remix',
           'sentry.op': 'function',
           'code.function.name': 'loader',
@@ -153,6 +154,7 @@ describe('remixIntegration (Orchestrion-based)', () => {
       expect.objectContaining({
         name: 'ACTION routes/submit',
         attributes: expect.objectContaining({
+          'sentry.description': 'ACTION routes/submit',
           'sentry.op': 'function',
           'code.function.name': 'action',
           'http.request.method': 'POST',
@@ -164,4 +166,77 @@ describe('remixIntegration (Orchestrion-based)', () => {
     expect(span.setAttribute).toHaveBeenCalledWith('http.response.status_code', 201);
     expect(span.setAttribute).toHaveBeenCalledWith('remix.action_form_data.actionType', 'create');
   });
+
+  describe('with span streaming', () => {
+    // The shared harness mocks `@sentry/node`'s `getClient`, but the instrumentation reads
+    // `@sentry/core`'s, so the lifecycle has to be stubbed here to reach the streamed branch.
+    let clientSpy: MockInstance;
+    let streamingSpy: MockInstance;
+
+    beforeEach(() => {
+      clientSpy = vi.spyOn(SentryCore, 'getClient').mockReturnValue({
+        getOptions: () => ({}),
+        getDataCollectionOptions: () => ({ httpBodies: [] }),
+      } as never);
+      streamingSpy = vi.spyOn(SentryCore, 'hasSpanStreamingEnabled').mockReturnValue(true);
+    });
+
+    afterEach(() => {
+      clientSpy.mockRestore();
+      streamingSpy.mockRestore();
+    });
+
+    it('callRouteLoader: names the span after the function and keeps the route id on an attribute', async () => {
+      const ctx = {
+        arguments: [
+          {
+            routeId: 'routes/users.$userId',
+            request: makeRequest({ method: 'GET', url: 'http://localhost/users/123' }),
+            params: { userId: '123' },
+          },
+        ],
+      };
+
+      await tracingChannel(remixChannels.REMIX_CALL_ROUTE_LOADER).tracePromise(async () => ({ status: 200 }), ctx);
+
+      expect(startInactiveSpanSpy).toHaveBeenCalledWith(
+        expect.objectContaining({
+          name: 'loader',
+          attributes: expect.objectContaining({
+            'sentry.op': 'function',
+            'sentry.description': 'LOADER routes/users.$userId',
+            'code.function.name': 'loader',
+            'match.route.id': 'routes/users.$userId',
+            'match.params.userId': '123',
+          }),
+        }),
+      );
+    });
+
+    it('callRouteAction: names the span after the function and keeps the route id on an attribute', async () => {
+      const ctx = {
+        arguments: [
+          {
+            routeId: 'routes/submit',
+            request: makeRequest({ method: 'POST', url: 'http://localhost/submit' }),
+            params: {},
+          },
+        ],
+      };
+
+      await tracingChannel(remixChannels.REMIX_CALL_ROUTE_ACTION).tracePromise(async () => ({ status: 201 }), ctx);
+
+      expect(startInactiveSpanSpy).toHaveBeenCalledWith(
+        expect.objectContaining({
+          name: 'action',
+          attributes: expect.objectContaining({
+            'sentry.op': 'function',
+            'sentry.description': 'ACTION routes/submit',
+            'code.function.name': 'action',
+            'match.route.id': 'routes/submit',
+          }),
+        }),
+      );
+    });
+  });
 });