From adf337e295726f09f55e0f1f0cf07e9592c97153 Mon Sep 17 00:00:00 2001 From: Andrei Borza Date: Mon, 31 Aug 2026 13:35:08 +0200 Subject: [PATCH 1/2] ref: Replace `forceTransaction` with `continueTrace` at internal call sites Adds `withSegment`, which reads the trace data of the active span through `getTraceData()` and feeds it back in through `continueTrace()`. A span started inside continues the same trace but has no parent span, so it becomes the segment (root span) of that trace. That is what `forceTransaction: true` does. Applies it to the three sites that need the flag: the MCP server request and notification spans, the Next.js server action span, and the nestjs event span. Experiment. Opened to see whether the composed form behaves the same in CI. Co-Authored-By: Opus 5 --- .../core/src/integrations/mcp-server/spans.ts | 19 ++-- .../src/integrations/mcp-server/transport.ts | 4 +- packages/core/src/tracing/index.ts | 1 + packages/core/src/tracing/trace.ts | 26 ++++++ .../mcp-server/semanticConventions.test.ts | 8 -- .../transportInstrumentation.test.ts | 6 -- packages/nestjs/src/integrations/helpers.ts | 2 - .../nestjs/src/integrations/wrap-handlers.ts | 20 ++-- .../common/withServerActionInstrumentation.ts | 92 ++++++++++--------- 9 files changed, 96 insertions(+), 82 deletions(-) diff --git a/packages/core/src/integrations/mcp-server/spans.ts b/packages/core/src/integrations/mcp-server/spans.ts index 76b3ba4ffa3e..4c2ea70a29f0 100644 --- a/packages/core/src/integrations/mcp-server/spans.ts +++ b/packages/core/src/integrations/mcp-server/spans.ts @@ -15,7 +15,7 @@ import { import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../../semanticAttributes'; import { hasSpanStreamingEnabled } from '../../tracing/spans/hasSpanStreamingEnabled'; import { MCP_NOTIFICATION_SPAN_NAME_FALLBACK, MCP_SERVER_SPAN_NAME_FALLBACK } from '../../tracing/spans/spanNames'; -import { startSpan } from '../../tracing/trace'; +import { startSpan, withSegment } from '../../tracing/trace'; import { buildTransportAttributes, buildTypeSpecificAttributes } from './attributeExtraction'; import { MCP_FUNCTION_ORIGIN_VALUE, @@ -112,13 +112,14 @@ function createMcpSpan(config: McpSpanConfig): unknown { const userInfo = Boolean(client?.getDataCollectionOptions().userInfo); const attributes = filterMcpPiiFromSpanData(rawAttributes, userInfo) as Record; - return startSpan( - { - name: spanName, - forceTransaction: true, - attributes, - }, - callback, + return withSegment(() => + startSpan( + { + name: spanName, + attributes, + }, + callback, + ), ); } @@ -186,7 +187,6 @@ export function buildMcpServerSpanConfig( options?: ResolvedMcpOptions, ): { name: string; - forceTransaction: boolean; attributes: Record; } { const { method } = jsonRpcMessage; @@ -211,7 +211,6 @@ export function buildMcpServerSpanConfig( return { name: spanName, - forceTransaction: true, attributes, }; } diff --git a/packages/core/src/integrations/mcp-server/transport.ts b/packages/core/src/integrations/mcp-server/transport.ts index 0222c820e406..668f9f36259b 100644 --- a/packages/core/src/integrations/mcp-server/transport.ts +++ b/packages/core/src/integrations/mcp-server/transport.ts @@ -7,7 +7,7 @@ import { getIsolationScope, withIsolationScope } from '../../currentScopes'; import { withActiveSpan } from '../../tracing'; -import { startInactiveSpan } from '../../tracing/trace'; +import { startInactiveSpan, withSegment } from '../../tracing/trace'; import { isObjectLike } from '../../utils/is'; import { fill } from '../../utils/object'; import { MCP_PROTOCOL_VERSION_ATTRIBUTE } from './attributes'; @@ -58,7 +58,7 @@ export function wrapTransportOnMessage(transport: MCPTransport, options: Resolve return withIsolationScope(isolationScope, () => { const spanConfig = buildMcpServerSpanConfig(request, transport, extra as ExtraHandlerData, options); - const span = startInactiveSpan(spanConfig); + const span = withSegment(() => startInactiveSpan(spanConfig)); if (request.method === 'initialize' && messageSessionData) { span.setAttributes({ diff --git a/packages/core/src/tracing/index.ts b/packages/core/src/tracing/index.ts index 5ca371eabffa..c01453bf0c51 100644 --- a/packages/core/src/tracing/index.ts +++ b/packages/core/src/tracing/index.ts @@ -19,6 +19,7 @@ export { } from './spanstatus'; export { continueTrace, + withSegment, withActiveSpan, suppressTracing, isTracingSuppressed, diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index b52d62624f23..55c9781be329 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -29,6 +29,7 @@ import { spanTimeInputToSeconds, spanToStaticSpanJSON, } from '../utils/spanUtils'; +import { getTraceData } from '../utils/traceData'; import { propagationContextFromHeaders, shouldContinueTrace } from '../utils/tracing'; import { freezeDscOnSpan, getDynamicSamplingContextFromSpan } from './dynamicSamplingContext'; import { logSpanStart } from './logSpans'; @@ -216,6 +217,31 @@ export const continueTrace = ( }); }; +/** + * Runs the callback in a scope that continues the current trace but carries no parent span, so a + * span started inside becomes the segment (root span) of that trace instead of a child span. + * + * This is the composable equivalent of the `forceTransaction` start-span option: the trace data of + * the active span is read back in through `continueTrace`, which is also how an incoming request + * starts its own segment on a trace that is already in flight. + */ +export function withSegment(callback: () => T): T { + // Without an active span the callback already starts a root span. + if (!getActiveSpan()) { + return callback(); + } + + const { 'sentry-trace': sentryTrace, baggage } = getTraceData(); + + // There is nothing to continue from, so `continueTrace` would start a fresh trace and detach the + // segment from the trace it belongs to. Staying a child span is the lesser evil. + if (!sentryTrace) { + return callback(); + } + + return continueTrace({ sentryTrace, baggage }, callback); +} + /** * Forks the current scope and sets the provided span as active span in the context of the provided callback. Can be * passed `null` to start an entirely new span tree. diff --git a/packages/core/test/lib/integrations/mcp-server/semanticConventions.test.ts b/packages/core/test/lib/integrations/mcp-server/semanticConventions.test.ts index 54c3d630ed6c..f8ee136edfd1 100644 --- a/packages/core/test/lib/integrations/mcp-server/semanticConventions.test.ts +++ b/packages/core/test/lib/integrations/mcp-server/semanticConventions.test.ts @@ -47,7 +47,6 @@ describe('MCP Server Semantic Conventions', () => { expect(startInactiveSpanSpy).toHaveBeenCalledWith({ name: 'tools/call get-weather', - forceTransaction: true, attributes: { 'mcp.method.name': 'tools/call', 'mcp.tool.name': 'get-weather', @@ -80,7 +79,6 @@ describe('MCP Server Semantic Conventions', () => { expect(startInactiveSpanSpy).toHaveBeenCalledWith({ name: 'resources/read file:///docs/api.md', - forceTransaction: true, attributes: { 'mcp.method.name': 'resources/read', 'mcp.resource.uri': 'file:///docs/api.md', @@ -111,7 +109,6 @@ describe('MCP Server Semantic Conventions', () => { expect(startInactiveSpanSpy).toHaveBeenCalledWith({ name: 'prompts/get analyze-code', - forceTransaction: true, attributes: { 'mcp.method.name': 'prompts/get', 'mcp.prompt.name': 'analyze-code', @@ -142,7 +139,6 @@ describe('MCP Server Semantic Conventions', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: 'notifications/tools/list_changed', - forceTransaction: true, attributes: { 'mcp.method.name': 'notifications/tools/list_changed', 'mcp.session.id': 'test-session-123', @@ -179,7 +175,6 @@ describe('MCP Server Semantic Conventions', () => { expect(startInactiveSpanSpy).toHaveBeenCalledWith( expect.objectContaining({ name: 'tools/list', - forceTransaction: true, attributes: expect.objectContaining({ 'mcp.method.name': 'tools/list', 'mcp.request.id': 'req-4', @@ -215,7 +210,6 @@ describe('MCP Server Semantic Conventions', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: 'notifications/message', - forceTransaction: true, attributes: { 'mcp.method.name': 'notifications/message', 'mcp.session.id': 'test-session-123', @@ -416,7 +410,6 @@ describe('MCP Server Semantic Conventions', () => { expect(startInactiveSpanSpy).toHaveBeenCalledWith( expect.objectContaining({ name: 'tools/call weather-lookup', - forceTransaction: true, attributes: expect.objectContaining({ 'mcp.method.name': 'tools/call', 'mcp.tool.name': 'weather-lookup', @@ -489,7 +482,6 @@ describe('MCP Server Semantic Conventions', () => { expect(startInactiveSpanSpy).toHaveBeenCalledWith( expect.objectContaining({ name: 'prompts/get code-review', - forceTransaction: true, attributes: expect.objectContaining({ 'mcp.method.name': 'prompts/get', 'mcp.prompt.name': 'code-review', diff --git a/packages/core/test/lib/integrations/mcp-server/transportInstrumentation.test.ts b/packages/core/test/lib/integrations/mcp-server/transportInstrumentation.test.ts index e97745c57253..7e949cd5af3f 100644 --- a/packages/core/test/lib/integrations/mcp-server/transportInstrumentation.test.ts +++ b/packages/core/test/lib/integrations/mcp-server/transportInstrumentation.test.ts @@ -117,7 +117,6 @@ describe('MCP Server Transport Instrumentation', () => { expect(startInactiveSpanSpy).toHaveBeenCalledWith( expect.objectContaining({ name: 'tools/call get-weather', - forceTransaction: true, }), ); }); @@ -137,7 +136,6 @@ describe('MCP Server Transport Instrumentation', () => { expect(startSpanSpy).toHaveBeenCalledWith( expect.objectContaining({ name: 'notifications/initialized', - forceTransaction: true, }), expect.any(Function), ); @@ -158,7 +156,6 @@ describe('MCP Server Transport Instrumentation', () => { expect(startSpanSpy).toHaveBeenCalledWith( expect.objectContaining({ name: 'notifications/tools/list_changed', - forceTransaction: true, }), expect.any(Function), ); @@ -270,7 +267,6 @@ describe('MCP Server Transport Instrumentation', () => { expect(startInactiveSpanSpy).toHaveBeenCalledWith({ name: 'tools/call process-file', - forceTransaction: true, attributes: { 'mcp.method.name': 'tools/call', 'mcp.tool.name': 'process-file', @@ -419,7 +415,6 @@ describe('MCP Server Transport Instrumentation', () => { expect(config).toEqual({ name: 'tools/call test-tool', - forceTransaction: true, attributes: expect.objectContaining({ 'mcp.method.name': 'tools/call', 'mcp.tool.name': 'test-tool', @@ -871,7 +866,6 @@ describe('MCP Server Transport Instrumentation', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: 'notifications/tools/list_changed', - forceTransaction: true, attributes: { 'mcp.transport': 'StreamableHTTPServerTransport', 'network.transport': 'tcp', diff --git a/packages/nestjs/src/integrations/helpers.ts b/packages/nestjs/src/integrations/helpers.ts index 655f53c6b53b..06e2dac801b3 100644 --- a/packages/nestjs/src/integrations/helpers.ts +++ b/packages/nestjs/src/integrations/helpers.ts @@ -109,7 +109,6 @@ export function getMiddlewareSpanOptions( export function getEventSpanOptions(event: string): { name: string; attributes: Record; - forceTransaction: boolean; } { return { name: `event ${event}`, @@ -117,7 +116,6 @@ export function getEventSpanOptions(event: string): { [SENTRY_OP]: FUNCTION, [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.event.nestjs', }, - forceTransaction: true, }; } diff --git a/packages/nestjs/src/integrations/wrap-handlers.ts b/packages/nestjs/src/integrations/wrap-handlers.ts index 62f677635eeb..cff21c2b0e21 100644 --- a/packages/nestjs/src/integrations/wrap-handlers.ts +++ b/packages/nestjs/src/integrations/wrap-handlers.ts @@ -1,4 +1,4 @@ -import { captureException, isObjectLike, isThenable, startSpan, withIsolationScope } from '@sentry/core'; +import { captureException, isObjectLike, isThenable, startSpan, withIsolationScope, withSegment } from '@sentry/core'; import type { AnyFn, ReflectWithMetadata } from './helpers'; import { getBullMQProcessSpanOptions, getEventSpanOptions, isWrapped, markWrapped } from './helpers'; @@ -99,14 +99,16 @@ export function wrapEventHandler(handler: AnyFn, fallbackEvent: unknown): AnyFn const wrapped = async function (this: unknown, ...args: unknown[]): Promise { const eventName = deriveEventName(wrapped, fallbackEvent); return withIsolationScope(() => - startSpan(getEventSpanOptions(eventName), async () => { - try { - return await handler.apply(this, args); - } catch (error) { - captureHandlerError(error, MECHANISM_EVENT); - throw error; - } - }), + withSegment(() => + startSpan(getEventSpanOptions(eventName), async () => { + try { + return await handler.apply(this, args); + } catch (error) { + captureHandlerError(error, MECHANISM_EVENT); + throw error; + } + }), + ), ); }; return wrapped; diff --git a/packages/nextjs/src/common/withServerActionInstrumentation.ts b/packages/nextjs/src/common/withServerActionInstrumentation.ts index 87c5ab67a65c..b14d66b00e14 100644 --- a/packages/nextjs/src/common/withServerActionInstrumentation.ts +++ b/packages/nextjs/src/common/withServerActionInstrumentation.ts @@ -12,6 +12,7 @@ import { SPAN_STATUS_OK, startSpan, withIsolationScope, + withSegment, } from '@sentry/core'; import { flushSafelyWithTimeout, waitUntil } from '../common/utils/responseEnd'; import { DEBUG_BUILD } from './debug-build'; @@ -111,55 +112,56 @@ async function withServerActionInstrumentationImplementation { try { - return await startSpan( - { - name: `serverAction/${serverActionName}`, - forceTransaction: true, - attributes: { - [SENTRY_KIND]: 'server', - [SENTRY_OP]: FUNCTION, - [SENTRY_SEGMENT_NAME_SOURCE]: 'route', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs.server_action', + return await withSegment(() => + startSpan( + { + name: `serverAction/${serverActionName}`, + attributes: { + [SENTRY_KIND]: 'server', + [SENTRY_OP]: FUNCTION, + [SENTRY_SEGMENT_NAME_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs.server_action', + }, }, - }, - async span => { - // oxlint-disable-next-line typescript/await-thenable -- callback may be async at runtime - const result = await handleCallbackErrors(callback, error => { - if (isNotFoundNavigationError(error)) { - // We don't want to report "not-found"s - span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); - } else if (isRedirectNavigationError(error)) { - // Redirects are normal Next.js control flow, not errors. Mark the span as OK and end it - // early so the surrounding `startSpan` error handler doesn't override the status to - // `internal_error` - span.setStatus({ code: SPAN_STATUS_OK }); - span.end(); - } else { - span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - captureException(error, { - mechanism: { - handled: false, - type: 'auto.function.nextjs.server_action', - }, - }); - } - }); + async span => { + // oxlint-disable-next-line typescript/await-thenable -- callback may be async at runtime + const result = await handleCallbackErrors(callback, error => { + if (isNotFoundNavigationError(error)) { + // We don't want to report "not-found"s + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); + } else if (isRedirectNavigationError(error)) { + // Redirects are normal Next.js control flow, not errors. Mark the span as OK and end it + // early so the surrounding `startSpan` error handler doesn't override the status to + // `internal_error` + span.setStatus({ code: SPAN_STATUS_OK }); + span.end(); + } else { + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + captureException(error, { + mechanism: { + handled: false, + type: 'auto.function.nextjs.server_action', + }, + }); + } + }); - if (options.recordResponse !== undefined ? options.recordResponse : shouldRecordResponse) { - getIsolationScope().setExtra('server_action_result', result); - } + if (options.recordResponse !== undefined ? options.recordResponse : shouldRecordResponse) { + getIsolationScope().setExtra('server_action_result', result); + } - if (options.formData) { - options.formData.forEach((value, key) => { - getIsolationScope().setExtra( - `server_action_form_data.${key}`, - typeof value === 'string' ? value : '[non-string value]', - ); - }); - } + if (options.formData) { + options.formData.forEach((value, key) => { + getIsolationScope().setExtra( + `server_action_form_data.${key}`, + typeof value === 'string' ? value : '[non-string value]', + ); + }); + } - return result; - }, + return result; + }, + ), ); } finally { waitUntil(flushSafelyWithTimeout()); From 4d8bd9750c34d6a1023de1279c1a5fa6c3ea253d Mon Sep 17 00:00:00 2001 From: Andrei Borza Date: Mon, 31 Aug 2026 13:49:16 +0200 Subject: [PATCH 2/2] Set the propagation context directly instead of through continueTrace continueTrace applies the trace continuation policy. With strictTraceContinuation and a frozen DSC that carries no org id, shouldContinueTrace rejects the self-generated baggage and startNewTrace moves the segment onto a new trace, which detaches it from the request it belongs to. withSegment now writes the propagation context from the active span, the way registerPrepareSpanScope does for a remote parent. Adds tests for the segment topology, for equivalence with forceTransaction, and for the strict continuation case. --- packages/core/src/tracing/trace.ts | 35 ++++++---- packages/core/test/lib/tracing/trace.test.ts | 71 ++++++++++++++++++++ 2 files changed, 93 insertions(+), 13 deletions(-) diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index 55c9781be329..d14d3859a008 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -29,7 +29,6 @@ import { spanTimeInputToSeconds, spanToStaticSpanJSON, } from '../utils/spanUtils'; -import { getTraceData } from '../utils/traceData'; import { propagationContextFromHeaders, shouldContinueTrace } from '../utils/tracing'; import { freezeDscOnSpan, getDynamicSamplingContextFromSpan } from './dynamicSamplingContext'; import { logSpanStart } from './logSpans'; @@ -221,25 +220,35 @@ export const continueTrace = ( * Runs the callback in a scope that continues the current trace but carries no parent span, so a * span started inside becomes the segment (root span) of that trace instead of a child span. * - * This is the composable equivalent of the `forceTransaction` start-span option: the trace data of - * the active span is read back in through `continueTrace`, which is also how an incoming request - * starts its own segment on a trace that is already in flight. + * This is the composable equivalent of the `forceTransaction` start-span option. The propagation + * context is written from the active span directly rather than through `continueTrace`, because + * `continueTrace` applies the trace continuation policy (`strictTraceContinuation`, org id + * matching) and would move the segment onto a new trace when a frozen DSC carries no org id. + * + * Mirrors what `registerPrepareSpanScope` in `@sentry/opentelemetry` does for a remote parent. */ export function withSegment(callback: () => T): T { + const parentSpan = getActiveSpan(); + // Without an active span the callback already starts a root span. - if (!getActiveSpan()) { + if (!parentSpan) { return callback(); } - const { 'sentry-trace': sentryTrace, baggage } = getTraceData(); - - // There is nothing to continue from, so `continueTrace` would start a fresh trace and detach the - // segment from the trace it belongs to. Staying a child span is the lesser evil. - if (!sentryTrace) { - return callback(); - } + const { traceId, spanId } = parentSpan.spanContext(); + const dsc = getDynamicSamplingContextFromSpan(parentSpan); + const sampleRand = Number(dsc.sample_rand); - return continueTrace({ sentryTrace, baggage }, callback); + return withScope(scope => { + scope.setPropagationContext({ + traceId, + parentSpanId: spanId, + sampled: spanIsSampled(parentSpan), + dsc, + sampleRand: Number.isNaN(sampleRand) ? safeMathRandom() : sampleRand, + }); + return withActiveSpan(null, callback); + }); } /** diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index e36effb86317..f1168a81ea5f 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -22,6 +22,7 @@ import { SentrySpan, suppressTracing, withActiveSpan, + withSegment, } from '../../../src/tracing'; import { startInactiveSpan, startSpan, startSpanManual } from '../../../src/tracing/trace'; import { SentryNonRecordingSpan } from '../../../src/tracing/sentryNonRecordingSpan'; @@ -2605,6 +2606,76 @@ describe('startNewTrace', () => { }); }); +describe('withSegment', () => { + beforeEach(() => { + resetGlobals(); + setAsyncContextStrategy(undefined); + + const options = getDefaultTestClientOptions({ tracesSampleRate: 1 }); + const client = new TestClient(options); + setCurrentClient(client); + client.init(); + }); + + it('starts a segment on the trace of the active span instead of a child span', () => { + startSpan({ name: 'outer' }, outer => { + const segment = withSegment(() => startInactiveSpan({ name: 'segment' })); + + expect(getRootSpan(segment)).toBe(segment); + expect(spanToJSON(segment).trace_id).toBe(spanToJSON(outer).trace_id); + expect(spanToJSON(segment).parent_span_id).toBe(outer.spanContext().spanId); + expect(getSpanDescendants(outer).map(span => spanToJSON(span).name)).toEqual(['outer']); + }); + }); + + it('matches what `forceTransaction: true` produces', () => { + startSpan({ name: 'outer' }, () => { + const viaHelper = withSegment(() => startInactiveSpan({ name: 'segment' })); + const viaOption = startInactiveSpan({ name: 'segment', forceTransaction: true }); + + const helperJson = spanToJSON(viaHelper); + const optionJson = spanToJSON(viaOption); + + expect(helperJson.trace_id).toBe(optionJson.trace_id); + expect(helperJson.parent_span_id).toBe(optionJson.parent_span_id); + expect(spanIsSampled(viaHelper)).toBe(spanIsSampled(viaOption)); + expect(getDynamicSamplingContextFromSpan(viaHelper)).toEqual(getDynamicSamplingContextFromSpan(viaOption)); + }); + }); + + it('stays on the same trace when `strictTraceContinuation` would reject the frozen DSC', () => { + const options = getDefaultTestClientOptions({ + dsn: 'https://username@domain/123', + tracesSampleRate: 1, + strictTraceContinuation: true, + orgId: '00222111', + }); + const client = new TestClient(options); + setCurrentClient(client); + client.init(); + + // A trace continued without incoming baggage freezes a DSC that carries no `org_id`. + getCurrentScope().setPropagationContext({ + traceId: '12345678901234567890123456789012', + sampleRand: 0.42, + dsc: {}, + }); + + startSpan({ name: 'outer' }, outer => { + const segment = withSegment(() => startInactiveSpan({ name: 'segment' })); + + expect(spanToJSON(segment).trace_id).toBe(spanToJSON(outer).trace_id); + }); + }); + + it('runs the callback unchanged when there is no active span', () => { + const segment = withSegment(() => startInactiveSpan({ name: 'segment' })); + + expect(getRootSpan(segment)).toBe(segment); + expect(spanToJSON(segment).parent_span_id).toBeUndefined(); + }); +}); + describe('ignoreSpans (core path, streaming)', () => { beforeEach(() => { registerSpanErrorInstrumentation();