From e1eae84d5fb7f0f395ca5a1fdcafb0d63e04e9a9 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Fri, 6 Nov 2026 13:02:11 +0100 Subject: [PATCH] fix(core): Use the final segment span name for streamed spans that end before a rename Spans are captured when they end, so a child that ended before the segment span was renamed kept the old sentry.segment.name. The span buffer now refreshes it from the segment span when the trace is sent, unless the value was changed by the user or beforeSendSpan. Fixes #26080 Co-Authored-By: Claude Opus 5.5 --- .../tracing/segment-name-update/scenario.ts | 16 +++ .../tracing/segment-name-update/test.ts | 26 ++++ packages/core/src/tracing/sentrySpan.ts | 2 +- .../core/src/tracing/spans/captureSpan.ts | 4 +- packages/core/src/tracing/spans/spanBuffer.ts | 39 +++++- .../lib/tracing/spans/captureSpan.test.ts | 4 + .../test/lib/tracing/spans/spanBuffer.test.ts | 131 +++++++++++++++++- 7 files changed, 215 insertions(+), 7 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/tracing/segment-name-update/scenario.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/segment-name-update/test.ts diff --git a/dev-packages/node-integration-tests/suites/tracing/segment-name-update/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/segment-name-update/scenario.ts new file mode 100644 index 000000000000..8fb0ac0784b4 --- /dev/null +++ b/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(); diff --git a/dev-packages/node-integration-tests/suites/tracing/segment-name-update/test.ts b/dev-packages/node-integration-tests/suites/tracing/segment-name-update/test.ts new file mode 100644 index 000000000000..f8d520e6b21e --- /dev/null +++ b/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(); +}); diff --git a/packages/core/src/tracing/sentrySpan.ts b/packages/core/src/tracing/sentrySpan.ts index 41c3a0257de3..4e209c562723 100644 --- a/packages/core/src/tracing/sentrySpan.ts +++ b/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 diff --git a/packages/core/src/tracing/spans/captureSpan.ts b/packages/core/src/tracing/spans/captureSpan.ts index 886f0d95cc0e..6a5bcc8b719e 100644 --- a/packages/core/src/tracing/spans/captureSpan.ts +++ b/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, }; } diff --git a/packages/core/src/tracing/spans/spanBuffer.ts b/packages/core/src/tracing/spans/spanBuffer.ts index 3066ae3b2790..fb1385799757 100644 --- a/packages/core/src/tracing/spans/spanBuffer.ts +++ b/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(); + 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 }; + } +} diff --git a/packages/core/test/lib/tracing/spans/captureSpan.test.ts b/packages/core/test/lib/tracing/spans/captureSpan.test.ts index 23cdaba20cf6..e7f2bc39a90d 100644 --- a/packages/core/test/lib/tracing/spans/captureSpan.test.ts +++ b/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', }); }); diff --git a/packages/core/test/lib/tracing/spans/spanBuffer.test.ts b/packages/core/test/lib/tracing/spans/spanBuffer.test.ts index 224ab4504dd2..281a09485a00 100644 --- a/packages/core/test/lib/tracing/spans/spanBuffer.test.ts +++ b/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 { + 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'); + }); + }); });