fix(core): Use the final segment span name for streamed spans that end before a rename

dev-packages/node-integration-tests/suites/tracing/segment-name-update/scenario.ts

@@ -0,0 +1,16 @@
+import * as Sentry from '@sentry/node';
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  tracesSampleRate: 1.0,
+  transport: loggingTransport,
+});
+
+Sentry.startSpan({ name: 'GET' }, root => {
+  Sentry.startSpan({ name: 'early child' }, () => {});
+  root.updateName('GET /users/:id');
+  Sentry.startSpan({ name: 'late child' }, () => {});
+});
+
+void Sentry.flush();

dev-packages/node-integration-tests/suites/tracing/segment-name-update/test.ts

@@ -0,0 +1,26 @@
+import { afterAll, expect, test } from 'vitest';
+import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
+
+afterAll(() => {
+  cleanupChildProcesses();
+});
+
+test('sends the final segment span name on child spans that ended before the segment span was renamed', async () => {
+  await createRunner(__dirname, 'scenario.ts')
+    .expect({
+      span: container => {
+        const segmentNames = Object.fromEntries(
+          container.items.map(span => [span.name, span.attributes?.['sentry.segment.name']?.value]),
+        );
+
+        expect(segmentNames).toEqual({
+          'early child': 'GET /users/:id',
+          'late child': 'GET /users/:id',
+          'GET /users/:id': 'GET /users/:id',
+        });
+      },
+    })
+    .unordered()
+    .start()
+    .completed();
+});

packages/core/src/tracing/sentrySpan.ts

@@ -588,7 +588,7 @@ function sendStandaloneSpan(span: SentrySpan, client: Client): void {
     return;
   }
 
-  const { _segmentSpan, ...serializedSpan } = captureSpan(span, client);
+  const { _segmentSpan, _segmentNameAtCapture, ...serializedSpan } = captureSpan(span, client);
   const dsc = getDynamicSamplingContextFromSpan(_segmentSpan);
   // sendEnvelope should not throw
   // eslint-disable-next-line @typescript-eslint/no-floating-promises

packages/core/src/tracing/spans/captureSpan.ts

@@ -32,6 +32,7 @@ import {
 
 export type SerializedStreamedSpanWithSegmentSpan = SerializedStreamedSpan & {
   _segmentSpan: Span;
+  _segmentNameAtCapture?: string;
 };
 
 /**
@@ -41,7 +42,7 @@ export type SerializedStreamedSpanWithSegmentSpan = SerializedStreamedSpan & {
  * to an already-ended OTel span instance (which is blocked by the OTel Span class).
  *
  * @returns the final serialized span with a reference to its segment span. This reference
- * is needed later on to compute the DSC for the span envelope.
+ * is needed later on to compute the DSC and the final segment name for the span envelope.
  */
 export function captureSpan(span: Span, client: Client): SerializedStreamedSpanWithSegmentSpan {
   // Convert to JSON FIRST - we cannot write to an already-ended span
@@ -88,6 +89,7 @@ export function captureSpan(span: Span, client: Client): SerializedStreamedSpanW
   return {
     ...streamedSpanJsonToSerializedSpan(processedSpan),
     _segmentSpan: segmentSpan,
+    _segmentNameAtCapture: serializedSegmentSpan.name,
   };
 }
 

packages/core/src/tracing/spans/spanBuffer.ts

@@ -1,12 +1,14 @@
 import type { Client } from '../../client';
 import { DEBUG_BUILD } from '../../debug-build';
-import type { SerializedStreamedSpan } from '../../types/span';
+import type { SerializedStreamedSpan, Span } from '../../types/span';
 import { debug } from '../../utils/debug-logger';
+import { spanToJSON } from '../../utils/spanUtils';
 import { safeUnref } from '../../utils/timer';
 import { getDynamicSamplingContextFromSpan } from '../dynamicSamplingContext';
 import type { SerializedStreamedSpanWithSegmentSpan } from './captureSpan';
 import { createStreamedSpanEnvelope } from './envelope';
 import { estimateSerializedSpanSizeInBytes } from './estimateSize';
+import { SENTRY_SEGMENT_NAME } from '@sentry/conventions/attributes';
 
 /**
  * We must not send more than 1000 spans in one envelope.
@@ -168,9 +170,19 @@ export class SpanBuffer {
 
     const dsc = getDynamicSamplingContextFromSpan(segmentSpan);
 
+    const currentSegmentNames = new Map<Span, string>();
+    const getCurrentSegmentName = (span: Span): string => {
+      let name = currentSegmentNames.get(span);
+      if (name === undefined) {
+        name = spanToJSON(span).name;
+        currentSegmentNames.set(span, name);
+      }
+      return name;
+    };
+
     const cleanedSpans: SerializedStreamedSpan[] = spans.map(spanJSON => {
-      // eslint-disable-next-line @typescript-eslint/no-unused-vars
-      const { _segmentSpan, ...cleanSpanJSON } = spanJSON;
+      const { _segmentSpan, _segmentNameAtCapture, ...cleanSpanJSON } = spanJSON;
+      refreshSegmentName(cleanSpanJSON, _segmentNameAtCapture, () => getCurrentSegmentName(_segmentSpan));
       return cleanSpanJSON;
     });
 
@@ -193,3 +205,24 @@ export class SpanBuffer {
     this._traceBuckets.delete(traceId);
   }
 }
+
+/**
+ * Spans are captured when they end, but the segment span can still be renamed afterwards
+ * (e.g. once the route is known). Only replace the value we set at capture time, so that
+ * values set by users or `beforeSendSpan` are kept.
+ */
+function refreshSegmentName(
+  spanJSON: SerializedStreamedSpan,
+  segmentNameAtCapture: string | undefined,
+  getCurrentSegmentName: () => string,
+): void {
+  const segmentNameAttribute = spanJSON.attributes[SENTRY_SEGMENT_NAME];
+  if (segmentNameAtCapture === undefined || segmentNameAttribute?.value !== segmentNameAtCapture) {
+    return;
+  }
+
+  const currentSegmentName = getCurrentSegmentName();
+  if (currentSegmentName !== segmentNameAtCapture) {
+    spanJSON.attributes[SENTRY_SEGMENT_NAME] = { ...segmentNameAttribute, value: currentSegmentName };
+  }
+}

packages/core/test/lib/tracing/spans/captureSpan.test.ts

@@ -127,6 +127,7 @@ describe('captureSpan', () => {
         },
       },
       _segmentSpan: span,
+      _segmentNameAtCapture: 'my-span',
     });
   });
 
@@ -235,6 +236,7 @@ describe('captureSpan', () => {
         },
       },
       _segmentSpan: span,
+      _segmentNameAtCapture: 'my-span',
     });
   });
 
@@ -307,6 +309,7 @@ describe('captureSpan', () => {
         },
       },
       _segmentSpan: span,
+      _segmentNameAtCapture: 'my-span',
     });
   });
 
@@ -366,6 +369,7 @@ describe('captureSpan', () => {
         },
       },
       _segmentSpan: span,
+      _segmentNameAtCapture: 'my-span',
     });
   });
 

packages/core/test/lib/tracing/spans/spanBuffer.test.ts

@@ -1,9 +1,17 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 import type { Client, StreamedSpanEnvelope } from '../../../../src';
-import { SentrySpan, setCurrentClient, SpanBuffer } from '../../../../src';
+import {
+  SentrySpan,
+  setCurrentClient,
+  SpanBuffer,
+  startInactiveSpan,
+  startSpan,
+  withStreamedSpan,
+} from '../../../../src';
 import type { SerializedStreamedSpanWithSegmentSpan } from '../../../../src/tracing/spans/captureSpan';
+import { captureSpan } from '../../../../src/tracing/spans/captureSpan';
 import { getDefaultTestClientOptions, TestClient } from '../../../mocks/client';
-import { HTTP_METHOD, URL_FULL } from '@sentry/conventions/attributes';
+import { HTTP_METHOD, SENTRY_SEGMENT_NAME, URL_FULL } from '@sentry/conventions/attributes';
 
 describe('SpanBuffer', () => {
   let client: TestClient;
@@ -370,4 +378,123 @@ describe('SpanBuffer', () => {
       expect(sendEnvelopeSpy).toHaveBeenCalledTimes(1);
     });
   });
+
+  describe('segment name', () => {
+    function getSentSegmentNames(): Record<string, unknown> {
+      const items = sentEnvelopes[0]?.[1]?.[0]?.[1]?.items ?? [];
+      return Object.fromEntries(items.map(item => [item.name, item.attributes[SENTRY_SEGMENT_NAME]?.value]));
+    }
+
+    it('uses the final segment span name for spans that ended before the segment span was renamed', () => {
+      const buffer = new SpanBuffer(client);
+
+      startSpan({ name: 'GET' }, root => {
+        startSpan({ name: 'early child' }, span => {
+          span.end();
+          buffer.add(captureSpan(span, client));
+        });
+
+        root.updateName('GET /users/:id');
+
+        startSpan({ name: 'late child' }, span => {
+          span.end();
+          buffer.add(captureSpan(span, client));
+        });
+
+        root.end();
+        buffer.add(captureSpan(root, client));
+      });
+
+      buffer.drain();
+
+      expect(getSentSegmentNames()).toEqual({
+        'early child': 'GET /users/:id',
+        'late child': 'GET /users/:id',
+        'GET /users/:id': 'GET /users/:id',
+      });
+    });
+
+    it('uses the segment span name at flush time if the segment span has not ended yet', () => {
+      const buffer = new SpanBuffer(client);
+
+      const root = startInactiveSpan({ name: 'GET' });
+      const child = startInactiveSpan({ name: 'child', parentSpan: root });
+      child.end();
+      buffer.add(captureSpan(child, client));
+
+      root.updateName('GET /users/:id');
+      buffer.drain();
+
+      expect(getSentSegmentNames()).toEqual({ child: 'GET /users/:id' });
+    });
+
+    it('keeps a segment name that differs from the segment span name at capture time', () => {
+      const buffer = new SpanBuffer(client);
+
+      const root = startInactiveSpan({ name: 'GET' });
+      const child = startInactiveSpan({
+        name: 'child',
+        parentSpan: root,
+        attributes: { [SENTRY_SEGMENT_NAME]: 'custom segment name' },
+      });
+      child.end();
+      buffer.add(captureSpan(child, client));
+
+      root.updateName('GET /users/:id');
+      buffer.drain();
+
+      expect(getSentSegmentNames()).toEqual({ child: 'custom segment name' });
+    });
+
+    it('keeps a segment name set by beforeSendSpan', () => {
+      client.getOptions().traceLifecycle = 'stream';
+      client.getOptions().beforeSendSpan = withStreamedSpan(span => {
+        if (span.name === 'child') {
+          span.attributes = { ...span.attributes, [SENTRY_SEGMENT_NAME]: 'from beforeSendSpan' };
+        }
+        return span;
+      });
+      const buffer = new SpanBuffer(client);
+
+      const root = startInactiveSpan({ name: 'GET' });
+      const child = startInactiveSpan({ name: 'child', parentSpan: root });
+      child.end();
+      buffer.add(captureSpan(child, client));
+
+      root.updateName('GET /users/:id');
+      buffer.drain();
+
+      expect(getSentSegmentNames()).toEqual({ child: 'from beforeSendSpan' });
+    });
+
+    it("uses the name of each span's own segment span when a trace has several segment spans", () => {
+      const buffer = new SpanBuffer(client);
+
+      const root1 = startInactiveSpan({ name: 'root 1', forceTransaction: true });
+      const root2 = startInactiveSpan({ name: 'root 2', parentSpan: root1, forceTransaction: true });
+      const child1 = startInactiveSpan({ name: 'child 1', parentSpan: root1 });
+      const child2 = startInactiveSpan({ name: 'child 2', parentSpan: root2 });
+      child1.end();
+      child2.end();
+      buffer.add(captureSpan(child1, client));
+      buffer.add(captureSpan(child2, client));
+
+      root1.updateName('root 1 renamed');
+      root2.updateName('root 2 renamed');
+      buffer.drain();
+
+      expect(getSentSegmentNames()).toEqual({ 'child 1': 'root 1 renamed', 'child 2': 'root 2 renamed' });
+    });
+
+    it('does not send the internal segment name field', () => {
+      const buffer = new SpanBuffer(client);
+
+      const root = startInactiveSpan({ name: 'GET' });
+      root.end();
+      buffer.add(captureSpan(root, client));
+      buffer.drain();
+
+      expect(sentEnvelopes[0]?.[1]?.[0]?.[1]?.items[0]).not.toHaveProperty('_segmentNameAtCapture');
+    });
+  });
 });