Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
},
})
Expand Down
7 changes: 1 addition & 6 deletions packages/node/src/sdk/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -206,7 +202,6 @@ function _init(

updateScopeFromEnvVariables();

enhanceDscWithOpenTelemetryRootSpanName(client);
Comment thread
cursor[bot] marked this conversation as resolved.
setupEventContextTrace(client);

// Ensure we flush events when vercel functions are ended
Expand Down
2 changes: 0 additions & 2 deletions packages/opentelemetry/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
export { getScopesFromContext } from './utils/contextData';

export { enhanceDscWithOpenTelemetryRootSpanName } from './utils/enhanceDscWithOpenTelemetryRootSpanName';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Public API removed without migration docs

Medium Severity

enhanceDscWithOpenTelemetryRootSpanName was a public export of @sentry/opentelemetry and is removed here with no deprecation path and no entry under the @sentry/opentelemetry section in MIGRATION.md, where other public removals for that package are already listed. This violates the PR review rule on removal of publicly exported APIs / public API changes without proper deprecation notices. I flagged this because it was mentioned in the rules file.

Additional Locations (2)
Fix in Cursor Fix in Web

Triggered by project rule: PR Review Guidelines for Cursor Bot

Reviewed by Cursor Bugbot for commit baf73c5. Configure here.


export { getTraceContextForScope } from './trace';

export { setupEventContextTrace } from './setupEventContextTrace';
Expand Down
21 changes: 18 additions & 3 deletions packages/opentelemetry/src/propagator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Comment thread
cursor[bot] marked this conversation as resolved.
};
}

Expand Down
12 changes: 11 additions & 1 deletion packages/opentelemetry/src/trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Comment thread
sentry[bot] marked this conversation as resolved.

const traceState = makeTraceState({
dsc,
Expand Down

This file was deleted.

30 changes: 30 additions & 0 deletions packages/opentelemetry/src/utils/reconcileDscSampled.ts
Original file line number Diff line number Diff line change
@@ -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<DynamicSamplingContext>,
sampled: boolean | undefined,
): Partial<DynamicSamplingContext> {
const reconciled = { ...dsc };

if (sampled === undefined) {
delete reconciled.sampled;
} else {
reconciled.sampled = String(sampled);
if (sampled === false) {
delete reconciled.transaction;
}
}

return reconciled;
}
2 changes: 0 additions & 2 deletions packages/opentelemetry/test/helpers/initOtel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -38,7 +37,6 @@ export function initOtel(): void {
}

setupEventContextTrace(client);
enhanceDscWithOpenTelemetryRootSpanName(client);

const provider = new SentryTracerProvider({ resource: getSentryResource('node') });

Expand Down
42 changes: 42 additions & 0 deletions packages/opentelemetry/test/propagator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,48 @@ describe('SentryPropagator', () => {
);
});

it('preserves a frozen incoming DSC on a directly-injected unsampled remote span', () => {
const carrier: Record<string, string> = {};
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<string, string> = {};
context.with(
Expand Down
13 changes: 13 additions & 0 deletions packages/opentelemetry/test/trace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 0 additions & 2 deletions packages/vercel-edge/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
stackParserFromStackParserOptions,
} from '@sentry/core';
import {
enhanceDscWithOpenTelemetryRootSpanName,
getSentryResource,
SentryPropagator,
SentryTracerProvider,
Expand Down Expand Up @@ -104,7 +103,6 @@ export function init(options: VercelEdgeOptions = {}): Client {
setupOtel(client);
}

enhanceDscWithOpenTelemetryRootSpanName(client);
setupEventContextTrace(client);

return client;
Expand Down
Loading