feat(react-router): Clean up bogus * http.route attribute on segment spans (#20471)

In span-streaming mode, the existing `processEvent` cleanup of bogus `*`
`http.route` attributes doesn't run, so segment spans end up with the
garbage route. Ports the same logic to the `processSegmentSpan` hook.

Added unit tests for this (also for the event path), since the
alternative would have been to add a separate streaming e2e test for
react-router, which seems overkill to me.

Closes #20361

Author: nicohrubec

packages/react-router/src/server/integration/reactRouterServer.ts

@@ -1,5 +1,5 @@
 import { ATTR_HTTP_ROUTE } from '@opentelemetry/semantic-conventions';
-import { defineIntegration } from '@sentry/core';
+import { defineIntegration, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';
 import { generateInstrumentOnce, NODE_VERSION } from '@sentry/node';
 import { ReactRouterInstrumentation } from '../instrumentation/reactRouter';
 import { registerServerBuildGlobal } from '../serverBuild';
@@ -60,5 +60,23 @@ export const reactRouterServerIntegration = defineIntegration(() => {
 
       return event;
     },
+    processSegmentSpan(span) {
+      // Express generates bogus `*` routes for data loaders, which we want to remove here
+      // we cannot do this earlier because some OTEL instrumentation adds this at some unexpected point
+      const attributes = span.attributes;
+      if (attributes?.[ATTR_HTTP_ROUTE] !== '*') {
+        return;
+      }
+
+      const origin = attributes[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN];
+      const isInstrumentationApiOrigin = typeof origin === 'string' && origin.includes('instrumentation_api');
+
+      // For instrumentation_api, always clean up bogus `*` route since we set better names
+      // For legacy, only clean up if the name has been adjusted (not METHOD *)
+      if (isInstrumentationApiOrigin || !span.name?.endsWith(' *')) {
+        // eslint-disable-next-line @typescript-eslint/no-dynamic-delete
+        delete attributes[ATTR_HTTP_ROUTE];
+      }
+    },
   };
 });

packages/react-router/test/server/integration/reactRouterServer.test.ts

@@ -1,3 +1,5 @@
+import { ATTR_HTTP_ROUTE } from '@opentelemetry/semantic-conventions';
+import type { Client, Event, EventType, StreamedSpanJSON } from '@sentry/core';
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 import { ReactRouterInstrumentation } from '../../../src/server/instrumentation/reactRouter';
 import { reactRouterServerIntegration } from '../../../src/server/integration/reactRouterServer';
@@ -98,4 +100,133 @@ describe('reactRouterServerIntegration', () => {
     expect(ReactRouterInstrumentation).toHaveBeenCalledTimes(1);
     expect(registerServerBuildGlobalSpy).toHaveBeenCalledTimes(1);
   });
+
+  describe('processEvent', () => {
+    const client = {} as Client;
+    const hint = {};
+
+    it('preserves http.route when it is not "*"', () => {
+      const integration = reactRouterServerIntegration();
+      const event = {
+        type: 'transaction' as EventType,
+        transaction: 'GET /users/:id',
+        contexts: {
+          trace: {
+            data: { [ATTR_HTTP_ROUTE]: '/users/:id' },
+            origin: 'auto.http.otel.http',
+          },
+        },
+      } as unknown as Event;
+
+      integration.processEvent!(event, hint, client);
+
+      expect(event.contexts?.trace?.data?.[ATTR_HTTP_ROUTE]).toBe('/users/:id');
+    });
+
+    it('deletes bogus "*" route when origin is instrumentation_api', () => {
+      const integration = reactRouterServerIntegration();
+      const event = {
+        type: 'transaction' as EventType,
+        transaction: 'GET *',
+        contexts: {
+          trace: {
+            data: { [ATTR_HTTP_ROUTE]: '*' },
+            origin: 'auto.http.otel.instrumentation_api',
+          },
+        },
+      } as unknown as Event;
+
+      integration.processEvent!(event, hint, client);
+
+      expect(event.contexts?.trace?.data?.[ATTR_HTTP_ROUTE]).toBeUndefined();
+    });
+
+    it('deletes bogus "*" route when legacy origin and transaction name was renamed', () => {
+      const integration = reactRouterServerIntegration();
+      const event = {
+        type: 'transaction' as EventType,
+        transaction: 'GET /api/users',
+        contexts: {
+          trace: {
+            data: { [ATTR_HTTP_ROUTE]: '*' },
+            origin: 'auto.http.otel.http',
+          },
+        },
+      } as unknown as Event;
+
+      integration.processEvent!(event, hint, client);
+
+      expect(event.contexts?.trace?.data?.[ATTR_HTTP_ROUTE]).toBeUndefined();
+    });
+
+    it('keeps "*" when legacy origin and transaction name still ends with " *"', () => {
+      const integration = reactRouterServerIntegration();
+      const event = {
+        type: 'transaction' as EventType,
+        transaction: 'GET *',
+        contexts: {
+          trace: {
+            data: { [ATTR_HTTP_ROUTE]: '*' },
+            origin: 'auto.http.otel.http',
+          },
+        },
+      } as unknown as Event;
+
+      integration.processEvent!(event, hint, client);
+
+      expect(event.contexts?.trace?.data?.[ATTR_HTTP_ROUTE]).toBe('*');
+    });
+  });
+
+  describe('processSegmentSpan', () => {
+    const client = {} as Client;
+
+    it('preserves http.route when it is not "*"', () => {
+      const integration = reactRouterServerIntegration();
+      const span = {
+        name: 'GET /users/:id',
+        attributes: { [ATTR_HTTP_ROUTE]: '/users/:id', 'sentry.origin': 'auto.http.otel.http' },
+      } as unknown as StreamedSpanJSON;
+
+      integration.processSegmentSpan!(span, client);
+
+      expect(span.attributes?.[ATTR_HTTP_ROUTE]).toBe('/users/:id');
+    });
+
+    it('deletes bogus "*" route when origin is instrumentation_api', () => {
+      const integration = reactRouterServerIntegration();
+      const span = {
+        name: 'GET *',
+        attributes: { [ATTR_HTTP_ROUTE]: '*', 'sentry.origin': 'auto.http.otel.instrumentation_api' },
+      } as unknown as StreamedSpanJSON;
+
+      integration.processSegmentSpan!(span, client);
+
+      expect(span.attributes?.[ATTR_HTTP_ROUTE]).toBeUndefined();
+    });
+
+    it('deletes bogus "*" route when legacy origin and span name was renamed', () => {
+      const integration = reactRouterServerIntegration();
+      const span = {
+        name: 'GET /api/users',
+        attributes: { [ATTR_HTTP_ROUTE]: '*', 'sentry.origin': 'auto.http.otel.http' },
+      } as unknown as StreamedSpanJSON;
+
+      integration.processSegmentSpan!(span, client);
+
+      expect(span.attributes?.[ATTR_HTTP_ROUTE]).toBeUndefined();
+    });
+
+    it('keeps "*" when legacy origin and span name still ends with " *"', () => {
+      const integration = reactRouterServerIntegration();
+      const span = {
+        name: 'GET *',
+        attributes: { [ATTR_HTTP_ROUTE]: '*', 'sentry.origin': 'auto.http.otel.http' },
+      } as unknown as StreamedSpanJSON;
+
+      integration.processSegmentSpan!(span, client);
+
+      expect(span.attributes?.[ATTR_HTTP_ROUTE]).toBe('*');
+    });
+  });
 });