diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 4b3796c295f3..59afda8dc43b 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -414,7 +414,7 @@ export abstract class BaseClient implements Client { public on(hook: 'beforeAddBreadcrumb', callback: (breadcrumb: Breadcrumb, hint?: BreadcrumbHint) => void): void; /** @inheritdoc */ - public on(hook: 'createDsc', callback: (dsc: DynamicSamplingContext) => void): void; + public on(hook: 'createDsc', callback: (dsc: DynamicSamplingContext, rootSpan?: Span) => void): void; /** @inheritdoc */ public on( @@ -499,7 +499,7 @@ export abstract class BaseClient implements Client { public emit(hook: 'beforeAddBreadcrumb', breadcrumb: Breadcrumb, hint?: BreadcrumbHint): void; /** @inheritdoc */ - public emit(hook: 'createDsc', dsc: DynamicSamplingContext): void; + public emit(hook: 'createDsc', dsc: DynamicSamplingContext, rootSpan?: Span): void; /** @inheritdoc */ public emit(hook: 'beforeSendFeedback', feedback: FeedbackEvent, options?: { includeReplay: boolean }): void; diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index 15486950649c..d47dfd7ff317 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -1,6 +1,7 @@ import type { Client, DynamicSamplingContext, Span } from '@sentry/types'; import { addNonEnumerableProperty, + baggageHeaderToDynamicSamplingContext, dropUndefinedKeys, dynamicSamplingContextToSentryBaggageHeader, } from '@sentry/utils'; @@ -66,15 +67,25 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly { expect(dynamicSamplingContext).toStrictEqual({ environment: 'myEnv' }); }); + test('uses frozen DSC from traceState', () => { + const rootSpan = { + spanContext() { + return { + traceId: '1234', + spanId: '12345', + traceFlags: 0, + traceState: { + get() { + return 'sentry-environment=myEnv2'; + }, + } as unknown as SpanContextData['traceState'], + }; + }, + } as Span; + + const dynamicSamplingContext = getDynamicSamplingContextFromSpan(rootSpan); + + expect(dynamicSamplingContext).toStrictEqual({ environment: 'myEnv2' }); + }); + test('returns a new DSC, if no DSC was provided during rootSpan creation (via attributes)', () => { const rootSpan = startInactiveSpan({ name: 'tx' }); diff --git a/packages/node/src/sdk/index.ts b/packages/node/src/sdk/index.ts index f149a44c06a0..490826c3953b 100644 --- a/packages/node/src/sdk/index.ts +++ b/packages/node/src/sdk/index.ts @@ -12,6 +12,7 @@ import { startSession, } from '@sentry/core'; import { + enhanceDscWithOpenTelemetryRootSpanName, openTelemetrySetupCheck, setOpenTelemetryContextAsyncContextStrategy, setupEventContextTrace, @@ -175,6 +176,7 @@ function _init( validateOpenTelemetrySetup(); } + enhanceDscWithOpenTelemetryRootSpanName(client); setupEventContextTrace(client); } diff --git a/packages/opentelemetry/src/index.ts b/packages/opentelemetry/src/index.ts index 97ed2f2e4764..5e99f7eb8c33 100644 --- a/packages/opentelemetry/src/index.ts +++ b/packages/opentelemetry/src/index.ts @@ -16,10 +16,13 @@ export { spanHasStatus, } from './utils/spanTypes'; -export { getDynamicSamplingContextFromSpan } from './utils/dynamicSamplingContext'; +// Re-export this for backwards compatibility (this used to be a different implementation) +export { getDynamicSamplingContextFromSpan } from '@sentry/core'; export { isSentryRequestSpan } from './utils/isSentryRequest'; +export { enhanceDscWithOpenTelemetryRootSpanName } from './utils/enhanceDscWithOpenTelemetryRootSpanName'; + export { getActiveSpan } from './utils/getActiveSpan'; export { startSpan, startSpanManual, startInactiveSpan, withActiveSpan, continueTrace } from './trace'; diff --git a/packages/opentelemetry/src/propagator.ts b/packages/opentelemetry/src/propagator.ts index 05b108583f98..ac8340bdc66a 100644 --- a/packages/opentelemetry/src/propagator.ts +++ b/packages/opentelemetry/src/propagator.ts @@ -8,7 +8,13 @@ import type { continueTrace } from '@sentry/core'; import { hasTracingEnabled } from '@sentry/core'; import { getRootSpan } from '@sentry/core'; import { spanToJSON } from '@sentry/core'; -import { getClient, getCurrentScope, getDynamicSamplingContextFromClient, getIsolationScope } from '@sentry/core'; +import { + getClient, + getCurrentScope, + getDynamicSamplingContextFromClient, + getDynamicSamplingContextFromSpan, + getIsolationScope, +} from '@sentry/core'; import type { DynamicSamplingContext, Options, PropagationContext } from '@sentry/types'; import { LRUMap, @@ -32,7 +38,6 @@ import { } from './constants'; import { DEBUG_BUILD } from './debug-build'; import { getScopesFromContext, setScopesOnContext } from './utils/contextData'; -import { getDynamicSamplingContextFromSpan } from './utils/dynamicSamplingContext'; import { getSamplingDecision } from './utils/getSamplingDecision'; import { setIsSetup } from './utils/setupCheck'; diff --git a/packages/opentelemetry/src/setupEventContextTrace.ts b/packages/opentelemetry/src/setupEventContextTrace.ts index fc3441b06792..f2e1454a02df 100644 --- a/packages/opentelemetry/src/setupEventContextTrace.ts +++ b/packages/opentelemetry/src/setupEventContextTrace.ts @@ -1,8 +1,6 @@ +import { getDynamicSamplingContextFromSpan, getRootSpan } from '@sentry/core'; import type { Client } from '@sentry/types'; import { dropUndefinedKeys } from '@sentry/utils'; -import { getDynamicSamplingContextFromSpan } from './utils/dynamicSamplingContext'; - -import { getRootSpan } from '@sentry/core'; import { SENTRY_TRACE_STATE_PARENT_SPAN_ID } from './constants'; import { getActiveSpan } from './utils/getActiveSpan'; import { spanHasParentId } from './utils/spanTypes'; diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts index ebd1536ff9b4..94e12bc019a8 100644 --- a/packages/opentelemetry/src/spanExporter.ts +++ b/packages/opentelemetry/src/spanExporter.ts @@ -5,6 +5,7 @@ import { SEMATTRS_HTTP_STATUS_CODE } from '@opentelemetry/semantic-conventions'; import { captureEvent, getCapturedScopesOnSpan, + getDynamicSamplingContextFromSpan, getMetricSummaryJsonForSpan, timedEventsToMeasurements, } from '@sentry/core'; @@ -22,7 +23,6 @@ import { SENTRY_TRACE_STATE_PARENT_SPAN_ID } from './constants'; import { DEBUG_BUILD } from './debug-build'; import { SEMANTIC_ATTRIBUTE_SENTRY_PARENT_IS_REMOTE } from './semanticAttributes'; import { convertOtelTimeToSeconds } from './utils/convertOtelTimeToSeconds'; -import { getDynamicSamplingContextFromSpan } from './utils/dynamicSamplingContext'; import { getRequestSpanData } from './utils/getRequestSpanData'; import type { SpanNode } from './utils/groupSpansWithParents'; import { getLocalParentId } from './utils/groupSpansWithParents'; @@ -242,7 +242,7 @@ function createTransactionForOtelSpan(span: ReadableSpan): TransactionEvent { capturedSpanScope: capturedSpanScopes.scope, capturedSpanIsolationScope: capturedSpanScopes.isolationScope, sampleRate, - dynamicSamplingContext: getDynamicSamplingContextFromSpan(span), + dynamicSamplingContext: getDynamicSamplingContextFromSpan(span as unknown as Span), }), }, ...(source && { diff --git a/packages/opentelemetry/src/trace.ts b/packages/opentelemetry/src/trace.ts index d55a7ba04a9f..b7d60ab117ad 100644 --- a/packages/opentelemetry/src/trace.ts +++ b/packages/opentelemetry/src/trace.ts @@ -7,6 +7,7 @@ import { continueTrace as baseContinueTrace, getClient, getCurrentScope, + getDynamicSamplingContextFromSpan, getRootSpan, handleCallbackErrors, spanToJSON, @@ -16,7 +17,6 @@ import { continueTraceAsRemoteSpan, makeTraceState } from './propagator'; import type { OpenTelemetryClient, OpenTelemetrySpanContext } from './types'; import { getContextFromScope, getScopesFromContext } from './utils/contextData'; -import { getDynamicSamplingContextFromSpan } from './utils/dynamicSamplingContext'; import { getSamplingDecision } from './utils/getSamplingDecision'; /** diff --git a/packages/opentelemetry/src/utils/dynamicSamplingContext.ts b/packages/opentelemetry/src/utils/dynamicSamplingContext.ts deleted file mode 100644 index 3e4f9f67ae84..000000000000 --- a/packages/opentelemetry/src/utils/dynamicSamplingContext.ts +++ /dev/null @@ -1,65 +0,0 @@ -import { - SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, - SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, - getClient, - getDynamicSamplingContextFromClient, -} from '@sentry/core'; -import type { DynamicSamplingContext } from '@sentry/types'; -import { baggageHeaderToDynamicSamplingContext } from '@sentry/utils'; -import { SENTRY_TRACE_STATE_DSC } from '../constants'; -import type { AbstractSpan } from '../types'; -import { getSamplingDecision } from './getSamplingDecision'; -import { parseSpanDescription } from './parseSpanDescription'; -import { spanHasAttributes, spanHasName } from './spanTypes'; - -/** - * Creates a dynamic sampling context from a span (and client and scope) - * - * @param span the span from which a few values like the root span name and sample rate are extracted. - * - * @returns a dynamic sampling context - */ -export function getDynamicSamplingContextFromSpan(span: AbstractSpan): Readonly> { - const client = getClient(); - if (!client) { - return {}; - } - - const traceState = span.spanContext().traceState; - const traceStateDsc = traceState?.get(SENTRY_TRACE_STATE_DSC); - - // If the span has a DSC, we want it to take precedence - const dscOnSpan = traceStateDsc ? baggageHeaderToDynamicSamplingContext(traceStateDsc) : undefined; - - if (dscOnSpan) { - return dscOnSpan; - } - - const dsc = getDynamicSamplingContextFromClient(span.spanContext().traceId, client); - - const attributes = spanHasAttributes(span) ? span.attributes : {}; - - const sampleRate = attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]; - if (sampleRate != null) { - dsc.sample_rate = `${sampleRate}`; - } - - // We don't want to have a transaction name in the DSC if the source is "url" because URLs might contain PII - const source = attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]; - - // If the span has no name, we assume it is non-recording and want to opt out of using any description - const { description } = spanHasName(span) ? parseSpanDescription(span) : { description: '' }; - - if (source !== 'url' && description) { - dsc.transaction = description; - } - - const sampled = getSamplingDecision(span.spanContext()); - if (sampled != null) { - dsc.sampled = String(sampled); - } - - client.emit('createDsc', dsc); - - return dsc; -} diff --git a/packages/opentelemetry/src/utils/enhanceDscWithOpenTelemetryRootSpanName.ts b/packages/opentelemetry/src/utils/enhanceDscWithOpenTelemetryRootSpanName.ts new file mode 100644 index 000000000000..af086451ac1b --- /dev/null +++ b/packages/opentelemetry/src/utils/enhanceDscWithOpenTelemetryRootSpanName.ts @@ -0,0 +1,28 @@ +import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, spanToJSON } from '@sentry/core'; +import type { Client } from '@sentry/types'; +import { parseSpanDescription } from './parseSpanDescription'; +import { spanHasName } from './spanTypes'; + +/** + * 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) => { + // We want to overwrite the transaction on the DSC that is created by default in core + // The reason for this is that we want to infer the span name, not use the initial one + // Otherwise, we'll get names like "GET" instead of e.g. "GET /foo" + // `parseSpanDescription` takes the attributes of the span into account for the name + // This mutates the passed-in DSC + if (rootSpan) { + const jsonSpan = spanToJSON(rootSpan); + const attributes = jsonSpan.data || {}; + const source = attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]; + + const { description } = spanHasName(rootSpan) ? parseSpanDescription(rootSpan) : { description: undefined }; + if (source !== 'url' && description) { + dsc.transaction = description; + } + } + }); +} diff --git a/packages/opentelemetry/test/trace.test.ts b/packages/opentelemetry/test/trace.test.ts index 289c494c7a75..2da8c7a8b511 100644 --- a/packages/opentelemetry/test/trace.test.ts +++ b/packages/opentelemetry/test/trace.test.ts @@ -12,6 +12,7 @@ import { getClient, getCurrentScope, getDynamicSamplingContextFromClient, + getDynamicSamplingContextFromSpan, getRootSpan, spanIsSampled, spanToJSON, @@ -24,7 +25,6 @@ import { makeTraceState } from '../src/propagator'; import { SEMATTRS_HTTP_METHOD } from '@opentelemetry/semantic-conventions'; import { continueTrace, startInactiveSpan, startSpan, startSpanManual } from '../src/trace'; import type { AbstractSpan } from '../src/types'; -import { getDynamicSamplingContextFromSpan } from '../src/utils/dynamicSamplingContext'; import { getActiveSpan } from '../src/utils/getActiveSpan'; import { getSamplingDecision } from '../src/utils/getSamplingDecision'; import { getSpanKind } from '../src/utils/getSpanKind'; @@ -983,24 +983,16 @@ describe('trace', () => { withScope(scope => { const propagationContext = scope.getPropagationContext(); - const ctx = trace.setSpanContext(ROOT_CONTEXT, { - traceId: '12312012123120121231201212312012', - spanId: '1121201211212012', - isRemote: false, - traceFlags: TraceFlags.SAMPLED, - traceState: undefined, - }); - - context.with(ctx, () => { + startSpan({ name: 'parent span' }, parentSpan => { const span = startInactiveSpan({ name: 'test span' }); expect(span).toBeDefined(); - expect(spanToJSON(span).trace_id).toEqual('12312012123120121231201212312012'); - expect(spanToJSON(span).parent_span_id).toEqual('1121201211212012'); + expect(spanToJSON(span).trace_id).toEqual(parentSpan.spanContext().traceId); + expect(spanToJSON(span).parent_span_id).toEqual(parentSpan.spanContext().spanId); expect(getDynamicSamplingContextFromSpan(span)).toEqual({ ...getDynamicSamplingContextFromClient(propagationContext.traceId, getClient()!), - trace_id: '12312012123120121231201212312012', - transaction: 'test span', + trace_id: parentSpan.spanContext().traceId, + transaction: 'parent span', sampled: 'true', sample_rate: '1', }); diff --git a/packages/types/src/client.ts b/packages/types/src/client.ts index b4609ec0568c..bb0ec4646211 100644 --- a/packages/types/src/client.ts +++ b/packages/types/src/client.ts @@ -244,7 +244,7 @@ export interface Client { /** * Register a callback when a DSC (Dynamic Sampling Context) is created. */ - on(hook: 'createDsc', callback: (dsc: DynamicSamplingContext) => void): void; + on(hook: 'createDsc', callback: (dsc: DynamicSamplingContext, rootSpan?: Span) => void): void; /** * Register a callback when a Feedback event has been prepared. @@ -338,7 +338,7 @@ export interface Client { /** * Fire a hook for when a DSC (Dynamic Sampling Context) is created. Expects the DSC as second argument. */ - emit(hook: 'createDsc', dsc: DynamicSamplingContext): void; + emit(hook: 'createDsc', dsc: DynamicSamplingContext, rootSpan?: Span): void; /** * Fire a hook event for after preparing a feedback event. Events to be given