diff --git a/dev-packages/e2e-tests/test-applications/supabase-nextjs/package.json b/dev-packages/e2e-tests/test-applications/supabase-nextjs/package.json index c5c86c0d31ae..d5c4c2114b90 100644 --- a/dev-packages/e2e-tests/test-applications/supabase-nextjs/package.json +++ b/dev-packages/e2e-tests/test-applications/supabase-nextjs/package.json @@ -7,7 +7,7 @@ "build": "next build", "start": "next start", "clean": "npx rimraf node_modules pnpm-lock.yaml .next", - "start-local-supabase": "supabase stop --no-backup 2>/dev/null || true && supabase init --force --workdir . && supabase start -o env && supabase db reset", + "start-local-supabase": "supabase stop --no-backup 2>/dev/null || true && supabase start -o env && supabase db reset", "test:prod": "TEST_ENV=production playwright test", "test:build": "pnpm install && pnpm start-local-supabase && pnpm build", "test:assert": "pnpm test:prod" diff --git a/dev-packages/e2e-tests/test-applications/supabase-nextjs/supabase/config.toml b/dev-packages/e2e-tests/test-applications/supabase-nextjs/supabase/config.toml index 35dcff35bec4..c898d44da1bb 100644 --- a/dev-packages/e2e-tests/test-applications/supabase-nextjs/supabase/config.toml +++ b/dev-packages/e2e-tests/test-applications/supabase-nextjs/supabase/config.toml @@ -288,7 +288,7 @@ inspector_port = 8083 # secret_key = "env(SECRET_VALUE)" [analytics] -enabled = true +enabled = false port = 54327 # Configure one of the supported backends: `postgres`, `bigquery`. backend = "postgres" diff --git a/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span-unsampled/test.ts b/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span-unsampled/test.ts index 9fe5f34ef8e5..131c5b208239 100644 --- a/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span-unsampled/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span-unsampled/test.ts @@ -14,6 +14,7 @@ test('envelope header for error event during active unsampled span is correct', sample_rate: '0', sampled: 'false', sample_rand: expect.any(String), + transaction: 'test span', }, }, }) diff --git a/packages/node/src/sdk/index.ts b/packages/node/src/sdk/index.ts index 25577a7df213..bcf8f48bd44b 100644 --- a/packages/node/src/sdk/index.ts +++ b/packages/node/src/sdk/index.ts @@ -15,11 +15,7 @@ import { requestDataIntegration, stackParserFromStackParserOptions, } from '@sentry/core'; -import { - enhanceDscWithOpenTelemetryRootSpanName, - setOpenTelemetryContextAsyncContextStrategy, - setupEventContextTrace, -} from '@sentry/opentelemetry'; +import { setOpenTelemetryContextAsyncContextStrategy, setupEventContextTrace } from '@sentry/opentelemetry'; import { isMainThread, parentPort } from 'node:worker_threads'; import { detectOrchestrionSetup } from '@sentry/server-utils/orchestrion'; import { registerDiagnosticsChannelInjection } from '@sentry/server-utils/orchestrion/register'; @@ -206,7 +202,6 @@ function _init( updateScopeFromEnvVariables(); - enhanceDscWithOpenTelemetryRootSpanName(client); setupEventContextTrace(client); // Ensure we flush events when vercel functions are ended diff --git a/packages/opentelemetry/src/index.ts b/packages/opentelemetry/src/index.ts index 2a06eeee940a..1ae4dff6fa7b 100644 --- a/packages/opentelemetry/src/index.ts +++ b/packages/opentelemetry/src/index.ts @@ -1,7 +1,5 @@ export { getScopesFromContext } from './utils/contextData'; -export { enhanceDscWithOpenTelemetryRootSpanName } from './utils/enhanceDscWithOpenTelemetryRootSpanName'; - export { getTraceContextForScope } from './trace'; export { setupEventContextTrace } from './setupEventContextTrace'; diff --git a/packages/opentelemetry/src/propagator.ts b/packages/opentelemetry/src/propagator.ts index d76418a36ac3..9db3fa6ea995 100644 --- a/packages/opentelemetry/src/propagator.ts +++ b/packages/opentelemetry/src/propagator.ts @@ -21,11 +21,17 @@ import { shouldPropagateTraceForUrl, spanToJSON, } from '@sentry/core'; -import { SENTRY_BAGGAGE_HEADER, SENTRY_TRACE_HEADER, SENTRY_TRACE_STATE_URL } from './constants'; +import { + SENTRY_BAGGAGE_HEADER, + SENTRY_TRACE_HEADER, + SENTRY_TRACE_STATE_DSC, + SENTRY_TRACE_STATE_URL, +} from './constants'; import { DEBUG_BUILD } from './debug-build'; import { getScopesFromContext, setScopesOnContext } from './utils/contextData'; import { getSampledForPropagation, getSamplingDecision } from './utils/getSamplingDecision'; import { makeTraceState } from './utils/makeTraceState'; +import { reconcileDscSampled } from './utils/reconcileDscSampled'; /** * Injects and extracts `sentry-trace` and `baggage` headers from carriers. @@ -149,13 +155,22 @@ export function getInjectionData( // Instead, we use a virtual (generated) spanId for propagation if (span?.spanContext().isRemote) { const spanContext = span.spanContext(); - const dynamicSamplingContext = getDynamicSamplingContextFromSpan(span); + const sampled = getSamplingDecision(spanContext); + const dsc = getDynamicSamplingContextFromSpan(span); + + // When the incoming trace froze its DSC on the trace state, `getDynamicSamplingContextFromSpan` + // returns that DSC verbatim; per the propagation spec it is immutable, so we must not rewrite + // `sampled` or strip `transaction` on it. We only reconcile the DSC that core freshly derives + // from the (binary) span trace flags, which is the sole case that can misrepresent a deferred + // decision as unsampled. + const hasIncomingFrozenDsc = !!spanContext.traceState?.get(SENTRY_TRACE_STATE_DSC); + const dynamicSamplingContext = hasIncomingFrozenDsc ? dsc : reconcileDscSampled(dsc, sampled); return { dynamicSamplingContext, traceId: spanContext.traceId, spanId: undefined, - sampled: getSamplingDecision(spanContext), // TODO: Do we need to change something here? + sampled, }; } diff --git a/packages/opentelemetry/src/trace.ts b/packages/opentelemetry/src/trace.ts index c1b49bb35094..2239127b62fe 100644 --- a/packages/opentelemetry/src/trace.ts +++ b/packages/opentelemetry/src/trace.ts @@ -26,11 +26,13 @@ import { spanToJSON, spanToTraceContext, } from '@sentry/core'; +import { SENTRY_TRACE_STATE_DSC } from './constants'; import { continueTraceAsRemoteSpan } from './propagator'; import type { OpenTelemetrySpanContext } from './types'; import { getContextFromScope } from './utils/contextData'; import { getSamplingDecision } from './utils/getSamplingDecision'; import { makeTraceState } from './utils/makeTraceState'; +import { reconcileDscSampled } from './utils/reconcileDscSampled'; /** * Internal helper for starting spans and manual spans. See {@link startSpan} and {@link startSpanManual} for the public APIs. @@ -257,7 +259,15 @@ function getContext(scope: Scope | undefined, forceTransaction: boolean | undefi // In this case, when we are forcing a transaction, we want to treat this like continuing an incoming trace // so we set the traceState according to the root span const rootSpan = getRootSpan(parentSpan); - const dsc = getDynamicSamplingContextFromSpan(rootSpan); + const rawDsc = getDynamicSamplingContextFromSpan(rootSpan); + + // When the root carried a frozen incoming DSC on its trace state, `getDynamicSamplingContextFromSpan` + // returns it verbatim and it is immutable per the propagation spec. Otherwise core freshly derived the + // DSC from the root's (binary) trace flags, which cannot tell a deferred decision apart from a + // definitive unsampled one — reconcile `sampled` against the authoritative OTel decision so a deferred + // parent (e.g. a `startNewTrace` remote parent with `traceFlags: NONE`) does not bake in `sampled=false`. + const hasIncomingFrozenDsc = !!rootSpan.spanContext().traceState?.get(SENTRY_TRACE_STATE_DSC); + const dsc = hasIncomingFrozenDsc ? rawDsc : reconcileDscSampled(rawDsc, sampled); const traceState = makeTraceState({ dsc, diff --git a/packages/opentelemetry/src/utils/enhanceDscWithOpenTelemetryRootSpanName.ts b/packages/opentelemetry/src/utils/enhanceDscWithOpenTelemetryRootSpanName.ts deleted file mode 100644 index 95b886218e6f..000000000000 --- a/packages/opentelemetry/src/utils/enhanceDscWithOpenTelemetryRootSpanName.ts +++ /dev/null @@ -1,42 +0,0 @@ -import type { Client } from '@sentry/core'; -import { hasSpansEnabled, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, spanToJSON } from '@sentry/core'; -import { getSampledForPropagation } from './getSamplingDecision'; - -/** - * Setup a DSC handler on the passed client, - * ensuring that the transaction name is inferred from the span correctly. - */ -export function enhanceDscWithOpenTelemetryRootSpanName(client: Client): void { - client.on('createDsc', (dsc, rootSpan) => { - if (!rootSpan) { - return; - } - - const jsonSpan = spanToJSON(rootSpan); - const attributes = jsonSpan.data; - const source = attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]; - const description = jsonSpan.description; - - const sampled = getSampledForPropagation(rootSpan, client); - - // We want to overwrite the transaction on the DSC that is created by default in core, so that we - // infer the span name (e.g. "GET /foo" instead of "GET"); `parseSpanDescription` reads the span - // attributes. This mutates the passed-in DSC. - // A negatively sampled trace carries no transaction name in its DSC, matching the OTel SDK whose - // unsampled spans are nameless non-recording spans. Core derives one from the span name, so we - // drop it here for native (SentryTracerProvider) spans that do have a name. - if (sampled === false) { - delete dsc.transaction; - } else if (description) { - if (source !== 'url') { - dsc.transaction = description; - } - } - - // Only write the sampling decision in tracing mode. In TwP mode it is deferred (read from the - // scope/incoming trace state), so we leave any value core already resolved untouched. - if (hasSpansEnabled()) { - dsc.sampled = sampled == undefined ? undefined : String(sampled); - } - }); -} diff --git a/packages/opentelemetry/src/utils/reconcileDscSampled.ts b/packages/opentelemetry/src/utils/reconcileDscSampled.ts new file mode 100644 index 000000000000..724a065f37f2 --- /dev/null +++ b/packages/opentelemetry/src/utils/reconcileDscSampled.ts @@ -0,0 +1,30 @@ +import type { DynamicSamplingContext } from '@sentry/core'; + +/** + * Reconcile a freshly-derived DSC's `sampled` flag with the OTel sampling decision. + * + * Only applies to a DSC that core generated from the span's (binary) trace flags — never to a frozen + * incoming DSC from the trace state, which the caller leaves untouched per the propagation spec. + * Trace flags cannot tell a *deferred* decision (an incoming remote span whose decision lives in the + * trace state) apart from a definitive *unsampled* one — both read as `traceFlags: NONE`. + * `getSamplingDecision` resolves this via the OTel trace state, so we let it win here: drop `sampled` + * when the decision is deferred (`undefined`), and — matching the OTel SDK, whose unsampled spans are + * nameless non-recording spans — drop the transaction name when the trace is definitively unsampled. + */ +export function reconcileDscSampled( + dsc: Partial, + sampled: boolean | undefined, +): Partial { + const reconciled = { ...dsc }; + + if (sampled === undefined) { + delete reconciled.sampled; + } else { + reconciled.sampled = String(sampled); + if (sampled === false) { + delete reconciled.transaction; + } + } + + return reconciled; +} diff --git a/packages/opentelemetry/test/helpers/initOtel.ts b/packages/opentelemetry/test/helpers/initOtel.ts index c5dabe409a10..39a0ee237eb2 100644 --- a/packages/opentelemetry/test/helpers/initOtel.ts +++ b/packages/opentelemetry/test/helpers/initOtel.ts @@ -4,7 +4,6 @@ import { DEBUG_BUILD } from '../../src/debug-build'; import { SentryPropagator } from '../../src/propagator'; import { getSentryResource } from '../../src/resource'; import { setupEventContextTrace } from '../../src/setupEventContextTrace'; -import { enhanceDscWithOpenTelemetryRootSpanName } from '../../src/utils/enhanceDscWithOpenTelemetryRootSpanName'; import type { TestClient } from './TestClient'; import { SentryTracerProvider } from '../../src/tracerProvider'; @@ -38,7 +37,6 @@ export function initOtel(): void { } setupEventContextTrace(client); - enhanceDscWithOpenTelemetryRootSpanName(client); const provider = new SentryTracerProvider({ resource: getSentryResource('node') }); diff --git a/packages/opentelemetry/test/propagator.test.ts b/packages/opentelemetry/test/propagator.test.ts index 3b3ed8c01be0..706396ddea53 100644 --- a/packages/opentelemetry/test/propagator.test.ts +++ b/packages/opentelemetry/test/propagator.test.ts @@ -381,6 +381,48 @@ describe('SentryPropagator', () => { ); }); + it('preserves a frozen incoming DSC on a directly-injected unsampled remote span', () => { + const carrier: Record = {}; + context.with( + trace.setSpanContext(ROOT_CONTEXT, { + traceId: 'd4cda95b652f4a1592b449d5929fda1b', + spanId: '6e0c63257de34c92', + traceFlags: TraceFlags.NONE, + isRemote: true, + // A definitively-unsampled incoming trace that froze its own DSC, including a transaction name. + traceState: makeTraceState({ + sampled: false, + dsc: { + transaction: 'incoming-transaction', + sampled: 'false', + trace_id: 'd4cda95b652f4a1592b449d5929fda1b', + public_key: 'incoming_public_key', + environment: 'incoming_environment', + release: 'incoming_release', + sample_rate: '0.5', + }, + }), + }), + () => { + propagator.inject(context.active(), carrier, defaultTextMapSetter); + + // The frozen incoming DSC is immutable, so its `transaction` must survive even though the + // trace is unsampled — we must not strip it the way we do for a freshly-derived DSC. + expect(baggageToArray(carrier[SENTRY_BAGGAGE_HEADER])).toEqual( + [ + 'sentry-environment=incoming_environment', + 'sentry-release=incoming_release', + 'sentry-public_key=incoming_public_key', + 'sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b', + 'sentry-transaction=incoming-transaction', + 'sentry-sampled=false', + 'sentry-sample_rate=0.5', + ].sort(), + ); + }, + ); + }); + it('uses remote span over propagation context', () => { const carrier: Record = {}; context.with( diff --git a/packages/opentelemetry/test/trace.test.ts b/packages/opentelemetry/test/trace.test.ts index 7073850d7690..16ac9aebc40b 100644 --- a/packages/opentelemetry/test/trace.test.ts +++ b/packages/opentelemetry/test/trace.test.ts @@ -2191,6 +2191,19 @@ describe('startNewTrace', () => { }); }); + it('samples a forced transaction based on tracesSampleRate', () => { + // `startNewTrace` injects a remote parent with `traceFlags: NONE` and no trace state, i.e. a + // *deferred* decision. A forced transaction under it runs through `getContext`'s simulated-root + // branch, which derives a DSC from that parent. Core naively reads `sampled=false` off the binary + // trace flags; without reconciliation that gets baked into the trace state and the transaction + // wrongly inherits a negative decision despite `tracesSampleRate: 1`. + startNewTrace(() => { + const span = startInactiveSpan({ name: 'forced-transaction', forceTransaction: true }); + expect(spanIsSampled(span)).toBe(true); + span.end(); + }); + }); + it('does not leak the new traceId to the outer scope', () => { const outerScope = getCurrentScope(); const outerTraceId = outerScope.getPropagationContext().traceId; diff --git a/packages/vercel-edge/src/sdk.ts b/packages/vercel-edge/src/sdk.ts index 9f8699e208de..903f3492648a 100644 --- a/packages/vercel-edge/src/sdk.ts +++ b/packages/vercel-edge/src/sdk.ts @@ -17,7 +17,6 @@ import { stackParserFromStackParserOptions, } from '@sentry/core'; import { - enhanceDscWithOpenTelemetryRootSpanName, getSentryResource, SentryPropagator, SentryTracerProvider, @@ -104,7 +103,6 @@ export function init(options: VercelEdgeOptions = {}): Client { setupOtel(client); } - enhanceDscWithOpenTelemetryRootSpanName(client); setupEventContextTrace(client); return client;