From ebcf7bfe6f83ba3ecf0bbdc8ae572c97c53cdd25 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 9 Jan 2025 13:25:29 +0000 Subject: [PATCH 01/45] Remove unused function --- docs/migration/v8-to-v9.md | 6 ++++ packages/opentelemetry/src/index.ts | 2 -- packages/opentelemetry/src/propagator.ts | 39 ++---------------------- 3 files changed, 8 insertions(+), 39 deletions(-) diff --git a/docs/migration/v8-to-v9.md b/docs/migration/v8-to-v9.md index 8d82fea6f0ad..74603b3f377d 100644 --- a/docs/migration/v8-to-v9.md +++ b/docs/migration/v8-to-v9.md @@ -217,6 +217,12 @@ The following changes are unlikely to affect users of the SDK. They are listed h }); ``` +### `@sentry/opentelemetry` + +- Removed `getPropagationContextFromSpan`. + This function was primarily internally used. + It's functionality was misleading and should not be used. + ## 5. Build Changes Previously the CJS versions of the SDK code (wrongfully) contained compatibility statements for default exports in ESM: diff --git a/packages/opentelemetry/src/index.ts b/packages/opentelemetry/src/index.ts index 262d356d04c4..572b154f6add 100644 --- a/packages/opentelemetry/src/index.ts +++ b/packages/opentelemetry/src/index.ts @@ -47,8 +47,6 @@ export { setOpenTelemetryContextAsyncContextStrategy } from './asyncContextStrat export { wrapContextManagerClass } from './contextManager'; export { SentryPropagator, - // eslint-disable-next-line deprecation/deprecation - getPropagationContextFromSpan, shouldPropagateTraceForUrl, } from './propagator'; export { SentrySpanProcessor } from './spanProcessor'; diff --git a/packages/opentelemetry/src/propagator.ts b/packages/opentelemetry/src/propagator.ts index b73a37b25a67..c0c48840c043 100644 --- a/packages/opentelemetry/src/propagator.ts +++ b/packages/opentelemetry/src/propagator.ts @@ -2,63 +2,28 @@ import type { Baggage, Context, Span, SpanContext, TextMapGetter, TextMapSetter import { INVALID_TRACEID, TraceFlags, context, propagation, trace } from '@opentelemetry/api'; import { W3CBaggagePropagator, isTracingSuppressed } from '@opentelemetry/core'; import { ATTR_URL_FULL, SEMATTRS_HTTP_URL } from '@opentelemetry/semantic-conventions'; -import type { DynamicSamplingContext, Options, PropagationContext, continueTrace } from '@sentry/core'; +import type { DynamicSamplingContext, Options, continueTrace } from '@sentry/core'; import { LRUMap, SENTRY_BAGGAGE_KEY_PREFIX, - baggageHeaderToDynamicSamplingContext, generateSentryTraceHeader, getClient, getCurrentScope, getDynamicSamplingContextFromScope, getDynamicSamplingContextFromSpan, getIsolationScope, - getRootSpan, logger, parseBaggageHeader, propagationContextFromHeaders, spanToJSON, stringMatchesSomePattern, } from '@sentry/core'; -import { - SENTRY_BAGGAGE_HEADER, - SENTRY_TRACE_HEADER, - SENTRY_TRACE_STATE_DSC, - SENTRY_TRACE_STATE_URL, -} from './constants'; +import { SENTRY_BAGGAGE_HEADER, SENTRY_TRACE_HEADER, SENTRY_TRACE_STATE_URL } from './constants'; import { DEBUG_BUILD } from './debug-build'; import { getScopesFromContext, setScopesOnContext } from './utils/contextData'; import { getSamplingDecision } from './utils/getSamplingDecision'; import { makeTraceState } from './utils/makeTraceState'; import { setIsSetup } from './utils/setupCheck'; -import { spanHasParentId } from './utils/spanTypes'; - -/** - * Get the Sentry propagation context from a span context. - * @deprecated This method is not used anymore and may be removed in a future major. - */ -export function getPropagationContextFromSpan(span: Span): PropagationContext { - const spanContext = span.spanContext(); - const { traceId, traceState } = spanContext; - - // When we have a dsc trace state, it means this came from the incoming trace - // Then this takes presedence over the root span - const dscString = traceState ? traceState.get(SENTRY_TRACE_STATE_DSC) : undefined; - const traceStateDsc = dscString ? baggageHeaderToDynamicSamplingContext(dscString) : undefined; - - const parentSpanId = spanHasParentId(span) ? span.parentSpanId : undefined; - const sampled = getSamplingDecision(spanContext); - - // No trace state? --> Take DSC from root span - const dsc = traceStateDsc || getDynamicSamplingContextFromSpan(getRootSpan(span)); - - return { - traceId, - sampled, - parentSpanId, - dsc, - }; -} /** * Injects and extracts `sentry-trace` and `baggage` headers from carriers. From 1646f0b52e6297cd5476db4c1213477121a12e07 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 13 Jan 2025 13:17:34 +0000 Subject: [PATCH 02/45] Write and use sample rand on propagation context --- docs/migration/v8-to-v9.md | 1 + .../src/tracing/browserTracingIntegration.ts | 4 ++-- packages/core/src/scope.ts | 3 ++- .../src/tracing/dynamicSamplingContext.ts | 1 + packages/core/src/tracing/sampling.ts | 17 ++++---------- packages/core/src/tracing/trace.ts | 23 +++++++++++++------ packages/core/src/types-hoist/envelope.ts | 1 + .../core/src/types-hoist/samplingcontext.ts | 5 ++++ packages/core/src/types-hoist/tracing.ts | 5 ++++ .../src/utils-hoist/propagationContext.ts | 1 + packages/core/src/utils-hoist/tracing.ts | 21 ++++++++++++++++- .../wrapGenerationFunctionWithSentry.ts | 12 +++++----- .../common/wrapServerComponentWithSentry.ts | 10 ++++---- packages/opentelemetry/src/sampler.ts | 23 ++++++++++++++----- 14 files changed, 87 insertions(+), 40 deletions(-) diff --git a/docs/migration/v8-to-v9.md b/docs/migration/v8-to-v9.md index e65db79afb0e..be7d4b3a2206 100644 --- a/docs/migration/v8-to-v9.md +++ b/docs/migration/v8-to-v9.md @@ -186,6 +186,7 @@ Sentry.init({ - The `addRequestDataToEvent` method has been removed. Use `httpRequestToRequestData` instead and put the resulting object directly on `event.request`. - The `extractPathForTransaction` method has been removed. There is no replacement. - The `addNormalizedRequestDataToEvent` method has been removed. Use `httpRequestToRequestData` instead and put the resulting object directly on `event.request`. +- A `sampleRand` field on `PropagationContext` is now required. This is relevant if you used `scope.setPropagationContext(...)` #### Other/Internal Changes diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index f4ab605be9c2..6dc7b44f57b3 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -452,8 +452,8 @@ export function startBrowserTracingPageLoadSpan( * This will only do something if a browser tracing integration has been setup. */ export function startBrowserTracingNavigationSpan(client: Client, spanOptions: StartSpanOptions): Span | undefined { - getIsolationScope().setPropagationContext({ traceId: generateTraceId() }); - getCurrentScope().setPropagationContext({ traceId: generateTraceId() }); + getIsolationScope().setPropagationContext({ traceId: generateTraceId(), sampleRand: Math.random() }); + getCurrentScope().setPropagationContext({ traceId: generateTraceId(), sampleRand: Math.random() }); client.emit('startNavigationSpan', spanOptions); diff --git a/packages/core/src/scope.ts b/packages/core/src/scope.ts index 995be01bb202..53593314be57 100644 --- a/packages/core/src/scope.ts +++ b/packages/core/src/scope.ts @@ -164,6 +164,7 @@ export class Scope { this._sdkProcessingMetadata = {}; this._propagationContext = { traceId: generateTraceId(), + sampleRand: Math.random(), }; } @@ -456,7 +457,7 @@ export class Scope { this._session = undefined; _setSpanForScope(this, undefined); this._attachments = []; - this.setPropagationContext({ traceId: generateTraceId() }); + this.setPropagationContext({ traceId: generateTraceId(), sampleRand: Math.random() }); this._notifyScopeListeners(); return this; diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index a4e0aa1d3222..23f47bcaaa4c 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -116,6 +116,7 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly, samplingContext: SamplingContext, + sampleRand: number, ): [sampled: boolean, sampleRate?: number] { // nothing to do if tracing is not enabled if (!hasTracingEnabled(options)) { return [false]; } - const normalizedRequest = getIsolationScope().getScopeData().sdkProcessingMetadata.normalizedRequest; - - const enhancedSamplingContext = { - ...samplingContext, - normalizedRequest: samplingContext.normalizedRequest || normalizedRequest, - }; - // we would have bailed already if neither `tracesSampler` nor `tracesSampleRate` nor `enableTracing` were defined, so one of these should // work; prefer the hook if so let sampleRate; if (typeof options.tracesSampler === 'function') { - sampleRate = options.tracesSampler(enhancedSamplingContext); - } else if (enhancedSamplingContext.parentSampled !== undefined) { - sampleRate = enhancedSamplingContext.parentSampled; + sampleRate = options.tracesSampler(samplingContext); + } else if (samplingContext.parentSampled !== undefined) { + sampleRate = samplingContext.parentSampled; } else if (typeof options.tracesSampleRate !== 'undefined') { sampleRate = options.tracesSampleRate; } else { @@ -66,7 +59,7 @@ export function sampleSpan( // Now we roll the dice. Math.random is inclusive of 0, but not of 1, so strict < is safe here. In case sampleRate is // a boolean, the < comparison will cause it to be automatically cast to 1 if it's true and 0 if it's false. - const shouldSample = Math.random() < parsedSampleRate; + const shouldSample = sampleRand < parsedSampleRate; // if we're not going to keep it, we're done if (!shouldSample) { diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index b8bd419bf713..fc6fd222c962 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -279,7 +279,10 @@ export function suppressTracing(callback: () => T): T { */ export function startNewTrace(callback: () => T): T { return withScope(scope => { - scope.setPropagationContext({ traceId: generateTraceId() }); + scope.setPropagationContext({ + traceId: generateTraceId(), + sampleRand: Math.random(), + }); DEBUG_BUILD && logger.info(`Starting a new trace with id ${scope.getPropagationContext().traceId}`); return withActiveSpan(null, callback); }); @@ -402,17 +405,23 @@ function _startRootSpan(spanArguments: SentrySpanArguments, scope: Scope, parent const options: Partial = client?.getOptions() || {}; const { name = '', attributes } = spanArguments; + const sampleRand = scope.getPropagationContext().sampleRand; const [sampled, sampleRate] = scope.getScopeData().sdkProcessingMetadata[SUPPRESS_TRACING_KEY] ? [false] - : sampleSpan(options, { - name, - parentSampled, - attributes, - transactionContext: { + : sampleSpan( + options, + { name, parentSampled, + attributes, + transactionContext: { + name, + parentSampled, + }, + // parentSampleRate: 'TODO', }, - }); + sampleRand, + ); const rootSpan = new SentrySpan({ ...spanArguments, diff --git a/packages/core/src/types-hoist/envelope.ts b/packages/core/src/types-hoist/envelope.ts index ab7af66f3a01..5a54ffc7b8c2 100644 --- a/packages/core/src/types-hoist/envelope.ts +++ b/packages/core/src/types-hoist/envelope.ts @@ -23,6 +23,7 @@ export type DynamicSamplingContext = { transaction?: string; replay_id?: string; sampled?: string; + sample_rand?: string; }; // https://github.com/getsentry/relay/blob/311b237cd4471042352fa45e7a0824b8995f216f/relay-server/src/envelope.rs#L154 diff --git a/packages/core/src/types-hoist/samplingcontext.ts b/packages/core/src/types-hoist/samplingcontext.ts index 0c73ba0968c2..2cc68c191b69 100644 --- a/packages/core/src/types-hoist/samplingcontext.ts +++ b/packages/core/src/types-hoist/samplingcontext.ts @@ -29,6 +29,11 @@ export interface SamplingContext extends CustomSamplingContext { */ parentSampled?: boolean; + /** + * Sample rate that is coming from an incoming trace (if there is one). + */ + parentSampleRate?: number; + /** * Object representing the URL of the current page or worker script. Passed by default when using the `BrowserTracing` * integration. diff --git a/packages/core/src/types-hoist/tracing.ts b/packages/core/src/types-hoist/tracing.ts index a1c57e29b3bb..518744e56aa2 100644 --- a/packages/core/src/types-hoist/tracing.ts +++ b/packages/core/src/types-hoist/tracing.ts @@ -16,6 +16,11 @@ export interface PropagationContext { */ traceId: string; + /** + * TODO + */ + sampleRand: number; + /** * Represents the sampling decision of the incoming trace. * diff --git a/packages/core/src/utils-hoist/propagationContext.ts b/packages/core/src/utils-hoist/propagationContext.ts index 5fb7a30db0bd..d3d291075658 100644 --- a/packages/core/src/utils-hoist/propagationContext.ts +++ b/packages/core/src/utils-hoist/propagationContext.ts @@ -9,6 +9,7 @@ import { uuid4 } from './misc'; export function generatePropagationContext(): PropagationContext { return { traceId: generateTraceId(), + sampleRand: Math.random(), }; } diff --git a/packages/core/src/utils-hoist/tracing.ts b/packages/core/src/utils-hoist/tracing.ts index 59359310f548..080db562da40 100644 --- a/packages/core/src/utils-hoist/tracing.ts +++ b/packages/core/src/utils-hoist/tracing.ts @@ -55,7 +55,10 @@ export function propagationContextFromHeaders( const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(baggage); if (!traceparentData?.traceId) { - return { traceId: generateTraceId() }; + return { + traceId: generateTraceId(), + sampleRand: Math.random(), + }; } const { traceId, parentSpanId, parentSampled } = traceparentData; @@ -65,6 +68,7 @@ export function propagationContextFromHeaders( parentSpanId, sampled: parentSampled, dsc: dynamicSamplingContext || {}, // If we have traceparent data but no DSC it means we are not head of trace and we must freeze it + sampleRand: parseSampleRandFromDsc(dynamicSamplingContext?.sample_rand), }; } @@ -82,3 +86,18 @@ export function generateSentryTraceHeader( } return `${traceId}-${spanId}${sampledString}`; } + +/** + * TODO + */ +export function parseSampleRandFromDsc(sampleRand: string | undefined): number { + if (!sampleRand) { + return Math.random(); + } + + try { + return Number(sampleRand); + } catch { + return Math.random(); + } +} diff --git a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts index c9ba434e949e..e3252600cc79 100644 --- a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts +++ b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts @@ -6,7 +6,6 @@ import { SPAN_STATUS_OK, Scope, captureException, - generateTraceId, getActiveSpan, getCapturedScopesOnSpan, getClient, @@ -85,12 +84,13 @@ export function wrapGenerationFunctionWithSentry a const propagationContext = commonObjectToPropagationContext( headers, - headersDict?.['sentry-trace'] - ? propagationContextFromHeaders(headersDict['sentry-trace'], headersDict['baggage']) - : { - traceId: requestTraceId || generateTraceId(), - }, + propagationContextFromHeaders(headersDict?.['sentry-trace'], headersDict?.['baggage']), ); + + if (requestTraceId) { + propagationContext.traceId = requestTraceId; + } + scope.setPropagationContext(propagationContext); scope.setExtra('route_data', data); diff --git a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts index 68d8b2df5c44..06f78417ffa0 100644 --- a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts +++ b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts @@ -64,13 +64,13 @@ export function wrapServerComponentWithSentry any> if (process.env.NEXT_RUNTIME === 'edge') { const propagationContext = commonObjectToPropagationContext( context.headers, - headersDict?.['sentry-trace'] - ? propagationContextFromHeaders(headersDict['sentry-trace'], headersDict['baggage']) - : { - traceId: requestTraceId || generateTraceId(), - }, + propagationContextFromHeaders(headersDict?.['sentry-trace'], headersDict?.['baggage']), ); + if (requestTraceId) { + propagationContext.traceId = requestTraceId; + } + scope.setPropagationContext(propagationContext); } diff --git a/packages/opentelemetry/src/sampler.ts b/packages/opentelemetry/src/sampler.ts index 8f90bc2e9ec6..aafacfaaea7f 100644 --- a/packages/opentelemetry/src/sampler.ts +++ b/packages/opentelemetry/src/sampler.ts @@ -1,3 +1,4 @@ +/* eslint-disable complexity */ import type { Attributes, Context, Span, TraceState as TraceStateInterface } from '@opentelemetry/api'; import { SpanKind, isSpanContextValid, trace } from '@opentelemetry/api'; import { TraceState } from '@opentelemetry/core'; @@ -19,6 +20,7 @@ import { } from '@sentry/core'; import { SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING, SENTRY_TRACE_STATE_URL } from './constants'; import { DEBUG_BUILD } from './debug-build'; +import { getScopesFromContext } from './utils/contextData'; import { getSamplingDecision } from './utils/getSamplingDecision'; import { inferSpanData } from './utils/parseSpanDescription'; import { setIsSetup } from './utils/setupCheck'; @@ -97,18 +99,27 @@ export class SentrySampler implements Sampler { const isRootSpan = !parentSpan || parentContext?.isRemote; + const { isolationScope, scope } = getScopesFromContext(context) ?? {}; + // We only sample based on parameters (like tracesSampleRate or tracesSampler) for root spans (which is done in sampleSpan). // Non-root-spans simply inherit the sampling decision from their parent. if (isRootSpan) { - const [sampled, sampleRate] = sampleSpan(options, { - name: inferredSpanName, - attributes: mergedAttributes, - transactionContext: { + const sampleRand = scope?.getPropagationContext().sampleRand ?? Math.random(); + const [sampled, sampleRate] = sampleSpan( + options, + { name: inferredSpanName, + attributes: mergedAttributes, + transactionContext: { + name: inferredSpanName, + parentSampled, + }, + // parentSampleRate: 'TODO', + normalizedRequest: isolationScope?.getScopeData().sdkProcessingMetadata.normalizedRequest, parentSampled, }, - parentSampled, - }); + sampleRand, + ); const attributes: Attributes = { [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: sampleRate, From 7fbba525d4e833c3a37d7802976018b264dc2b4c Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 13 Jan 2025 13:42:52 +0000 Subject: [PATCH 03/45] Implement propagating sampling decision properly --- packages/core/src/utils-hoist/tracing.ts | 39 ++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/packages/core/src/utils-hoist/tracing.ts b/packages/core/src/utils-hoist/tracing.ts index 080db562da40..e0f8d576101b 100644 --- a/packages/core/src/utils-hoist/tracing.ts +++ b/packages/core/src/utils-hoist/tracing.ts @@ -1,4 +1,4 @@ -import type { PropagationContext, TraceparentData } from '../types-hoist'; +import type { DynamicSamplingContext, PropagationContext, TraceparentData } from '../types-hoist'; import { baggageHeaderToDynamicSamplingContext } from './baggage'; import { generateSpanId, generateTraceId } from './propagationContext'; @@ -68,7 +68,7 @@ export function propagationContextFromHeaders( parentSpanId, sampled: parentSampled, dsc: dynamicSamplingContext || {}, // If we have traceparent data but no DSC it means we are not head of trace and we must freeze it - sampleRand: parseSampleRandFromDsc(dynamicSamplingContext?.sample_rand), + sampleRand: getSampleRandFromTraceparentAndDsc(traceparentData, dynamicSamplingContext), }; } @@ -101,3 +101,38 @@ export function parseSampleRandFromDsc(sampleRand: string | undefined): number { return Math.random(); } } + +function getSampleRandFromTraceparentAndDsc( + traceparentData: TraceparentData | undefined, + dsc: Partial | undefined, +): number { + const parsedSampleRand = parseSamplingDscNumber(dsc?.sample_rand); + if (parsedSampleRand !== undefined) { + return parsedSampleRand; + } + + const parsedSampleRate = parseSamplingDscNumber(dsc?.sample_rate); + if (parsedSampleRate && traceparentData?.parentSampled !== undefined) { + return traceparentData.parentSampled + ? // Returns a sample rand with positive sampling decision [0, sampleRate) + Math.random() * parsedSampleRate + : // Returns a sample rand with negative sampling decision [sampleRate, 1] + parsedSampleRate + Math.random() * (1 - parsedSampleRate); + } else { + return Math.random(); + } +} + +function parseSamplingDscNumber(sampleRand: string | undefined): number | undefined { + try { + const parsed = Number(sampleRand); // Number(undefined) will return NaN and fail the next check + if (isNaN(parsed) || parsed < 0 || parsed > 1) { + // This is probably an invariant but returning undefined seems sensible. + return undefined; + } else { + return parsed; + } + } catch { + return undefined; + } +} From 0b1261521f817bb078ef24e58c1b717070fb85b9 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 13 Jan 2025 13:53:35 +0000 Subject: [PATCH 04/45] Inject sample rand --- packages/core/src/tracing/dynamicSamplingContext.ts | 3 ++- packages/opentelemetry/src/propagator.ts | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index 23f47bcaaa4c..ba362c49795a 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -11,6 +11,7 @@ import { import { addNonEnumerableProperty, dropUndefinedKeys } from '../utils-hoist/object'; import { hasTracingEnabled } from '../utils/hasTracingEnabled'; import { getRootSpan, spanIsSampled, spanToJSON } from '../utils/spanUtils'; +import { getCapturedScopesOnSpan } from './utils'; /** * If you change this value, also update the terser plugin config to @@ -116,7 +117,7 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly Date: Mon, 13 Jan 2025 15:02:46 +0000 Subject: [PATCH 05/45] format --- packages/nextjs/src/common/wrapServerComponentWithSentry.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts index 06f78417ffa0..d4dce97979f9 100644 --- a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts +++ b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts @@ -6,7 +6,6 @@ import { SPAN_STATUS_OK, Scope, captureException, - generateTraceId, getActiveSpan, getCapturedScopesOnSpan, getRootSpan, From 4fb937feb5d17718f78ec2f2e34d05bfc239058a Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 14 Jan 2025 10:22:13 +0000 Subject: [PATCH 06/45] put onto initial dsc --- packages/core/src/utils-hoist/tracing.ts | 29 ++++++++++++------------ 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/packages/core/src/utils-hoist/tracing.ts b/packages/core/src/utils-hoist/tracing.ts index e0f8d576101b..98175f427762 100644 --- a/packages/core/src/utils-hoist/tracing.ts +++ b/packages/core/src/utils-hoist/tracing.ts @@ -61,6 +61,13 @@ export function propagationContextFromHeaders( }; } + const sampleRand = getSampleRandFromTraceparentAndDsc(traceparentData, dynamicSamplingContext); + + // The sample_rand on the DSC needs to be generated based on traceparent + baggage. + if (dynamicSamplingContext) { + dynamicSamplingContext.sample_rand = sampleRand.toString(); + } + const { traceId, parentSpanId, parentSampled } = traceparentData; return { @@ -68,7 +75,7 @@ export function propagationContextFromHeaders( parentSpanId, sampled: parentSampled, dsc: dynamicSamplingContext || {}, // If we have traceparent data but no DSC it means we are not head of trace and we must freeze it - sampleRand: getSampleRandFromTraceparentAndDsc(traceparentData, dynamicSamplingContext), + sampleRand: sampleRand, }; } @@ -88,29 +95,19 @@ export function generateSentryTraceHeader( } /** - * TODO + * Given any combination of an incoming trace, generate a sample rand based on its defined semantics. */ -export function parseSampleRandFromDsc(sampleRand: string | undefined): number { - if (!sampleRand) { - return Math.random(); - } - - try { - return Number(sampleRand); - } catch { - return Math.random(); - } -} - function getSampleRandFromTraceparentAndDsc( traceparentData: TraceparentData | undefined, dsc: Partial | undefined, ): number { + // When there is an incoming sample rand use it. const parsedSampleRand = parseSamplingDscNumber(dsc?.sample_rand); if (parsedSampleRand !== undefined) { return parsedSampleRand; } + // Otherwise, if there is an incoming sampling decision + sample rate, generate a sample rand that would lead to the same sampling decision. const parsedSampleRate = parseSamplingDscNumber(dsc?.sample_rate); if (parsedSampleRate && traceparentData?.parentSampled !== undefined) { return traceparentData.parentSampled @@ -119,10 +116,14 @@ function getSampleRandFromTraceparentAndDsc( : // Returns a sample rand with negative sampling decision [sampleRate, 1] parsedSampleRate + Math.random() * (1 - parsedSampleRate); } else { + // If nothing applies, return a random sample rand. return Math.random(); } } +/** + * Given a string form of a numeric-like on the DSC, safely convert it to a number. + */ function parseSamplingDscNumber(sampleRand: string | undefined): number | undefined { try { const parsed = Number(sampleRand); // Number(undefined) will return NaN and fail the next check From 6c2793159cb5d42f86de0a483ec73d41858b3fb8 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 14 Jan 2025 10:26:33 +0000 Subject: [PATCH 07/45] Add tests for `tracesSampleRate` behavior --- .../node-integration-tests/src/index.ts | 11 +++ .../tracesSampleRate/server.js | 40 ++++++++ .../tracesSampleRate/test.ts | 91 +++++++++++++++++++ 3 files changed, 142 insertions(+) create mode 100644 dev-packages/node-integration-tests/suites/sample-rand-propagation/tracesSampleRate/server.js create mode 100644 dev-packages/node-integration-tests/suites/sample-rand-propagation/tracesSampleRate/test.ts diff --git a/dev-packages/node-integration-tests/src/index.ts b/dev-packages/node-integration-tests/src/index.ts index 18c443203926..9c8971ea329d 100644 --- a/dev-packages/node-integration-tests/src/index.ts +++ b/dev-packages/node-integration-tests/src/index.ts @@ -29,6 +29,9 @@ export function startExpressServerAndSendPortToRunner(app: Express, port: number const server = app.listen(port || 0, () => { const address = server.address() as AddressInfo; + // @ts-expect-error If we write the port to the app we can read it within route handlers in tests + app.port = port || address.port; + // eslint-disable-next-line no-console console.log(`{"port":${port || address.port}}`); }); @@ -41,3 +44,11 @@ export function sendPortToRunner(port: number): void { // eslint-disable-next-line no-console console.log(`{"port":${port}}`); } + +/** + * Can be used to get the port of a running app, so requests can be sent to a server from within the server. + */ +export function getPortAppIsRunningOn(app: Express): number | undefined { + // @ts-expect-error It's not defined in the types but we'd like to read it. + return app.port; +} diff --git a/dev-packages/node-integration-tests/suites/sample-rand-propagation/tracesSampleRate/server.js b/dev-packages/node-integration-tests/suites/sample-rand-propagation/tracesSampleRate/server.js new file mode 100644 index 000000000000..89ad5ef12f21 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/sample-rand-propagation/tracesSampleRate/server.js @@ -0,0 +1,40 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + transport: loggingTransport, + tracesSampleRate: 1, +}); + +// express must be required after Sentry is initialized +const express = require('express'); +const cors = require('cors'); +const { + startExpressServerAndSendPortToRunner, + getPortAppIsRunningOn, +} = require('@sentry-internal/node-integration-tests'); + +const app = express(); + +app.use(cors()); + +app.get('/check', (req, res) => { + const appPort = getPortAppIsRunningOn(app); + // eslint-disable-next-line no-undef + fetch(`http://localhost:${appPort}/bounce`) + .then(r => r.json()) + .then(bounceRes => { + res.json({ propagatedData: bounceRes }); + }); +}); + +app.get('/bounce', (req, res) => { + res.json({ + baggage: req.headers['baggage'], + }); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/sample-rand-propagation/tracesSampleRate/test.ts b/dev-packages/node-integration-tests/suites/sample-rand-propagation/tracesSampleRate/test.ts new file mode 100644 index 000000000000..4a382f5a20d0 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/sample-rand-propagation/tracesSampleRate/test.ts @@ -0,0 +1,91 @@ +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; + +describe('sample_rand propagation (default)', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + test('propagates a sample rand when there are no incoming trace headers', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + const response = await runner.makeRequest('get', '/check'); + expect(response).toEqual({ + propagatedData: { + baggage: expect.stringMatching(/sentry-sample_rand=0\.[0-9]+/), + }, + }); + }); + + test('propagates a sample rand when there is a sentry-trace header and incoming sentry baggage', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + const response = await runner.makeRequest('get', '/check', { + headers: { + 'sentry-trace': '530699e319cc067ce440315d74acb312-414dc2a08d5d1dac-1', + baggage: 'sentry-release=foo,sentry-sample_rand=0.424242', + }, + }); + expect(response).toEqual({ + propagatedData: { + baggage: expect.stringMatching(/sentry-sample_rand=0\.424242/), + }, + }); + }); + + test('propagates a sample rand when there is an incoming sentry-trace header but no baggage header', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + const response = await runner.makeRequest('get', '/check', { + headers: { + 'sentry-trace': '530699e319cc067ce440315d74acb312-414dc2a08d5d1dac-1', + }, + }); + expect(response).toEqual({ + propagatedData: { + baggage: expect.stringMatching(/sentry-sample_rand=0\.[0-9]+/), + }, + }); + }); + + test('propagates a sample_rand that would lead to a positive sampling decision when there is an incoming positive sampling decision but no sample_rand in the baggage header', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + const response = await runner.makeRequest('get', '/check', { + headers: { + 'sentry-trace': '530699e319cc067ce440315d74acb312-414dc2a08d5d1dac-1', + baggage: 'sentry-sample_rate=0.25', + }, + }); + + const sampleRand = Number((response as any).propagatedData.baggage.match(/sentry-sample_rand=(0\.[0-9]+)/)[1]); + + expect(sampleRand).toStrictEqual(expect.any(Number)); + expect(sampleRand).not.toBeNaN(); + expect(sampleRand).toBeLessThan(0.25); + }); + + test('propagates a sample_rand that would lead to a negative sampling decision when there is an incoming negative sampling decision but no sample_rand in the baggage header', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + const response = await runner.makeRequest('get', '/check', { + headers: { + 'sentry-trace': '530699e319cc067ce440315d74acb312-414dc2a08d5d1dac-0', + baggage: 'sentry-sample_rate=0.75', + }, + }); + + const sampleRand = Number((response as any).propagatedData.baggage.match(/sentry-sample_rand=(0\.[0-9]+)/)[1]); + + expect(sampleRand).toStrictEqual(expect.any(Number)); + expect(sampleRand).not.toBeNaN(); + expect(sampleRand).toBeGreaterThanOrEqual(0.75); + }); + + test('a new sample_rand when there is no sentry-trace header but a baggage header with sample_rand', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + const response = await runner.makeRequest('get', '/check', { + headers: { + baggage: 'sentry-sample_rate=0.75,sentry-sample_rand=0.5', + }, + }); + + expect((response as any).propagatedData.baggage).toMatch(/sentry-sample_rand=0\.[0-9]+/); + const sampleRandStr = (response as any).propagatedData.baggage.match(/sentry-sample_rand=(0\.[0-9]+)/)[1]; + expect(sampleRandStr).not.toBe('0.5'); + }); +}); From 8d6984f0c15d41bc187dc938d94ace17724a2efa Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 14 Jan 2025 10:29:23 +0000 Subject: [PATCH 08/45] mv --- .../sample-rand-propagation/{tracesSampleRate => }/server.js | 0 .../sample-rand-propagation/{tracesSampleRate => }/test.ts | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename dev-packages/node-integration-tests/suites/sample-rand-propagation/{tracesSampleRate => }/server.js (100%) rename dev-packages/node-integration-tests/suites/sample-rand-propagation/{tracesSampleRate => }/test.ts (98%) diff --git a/dev-packages/node-integration-tests/suites/sample-rand-propagation/tracesSampleRate/server.js b/dev-packages/node-integration-tests/suites/sample-rand-propagation/server.js similarity index 100% rename from dev-packages/node-integration-tests/suites/sample-rand-propagation/tracesSampleRate/server.js rename to dev-packages/node-integration-tests/suites/sample-rand-propagation/server.js diff --git a/dev-packages/node-integration-tests/suites/sample-rand-propagation/tracesSampleRate/test.ts b/dev-packages/node-integration-tests/suites/sample-rand-propagation/test.ts similarity index 98% rename from dev-packages/node-integration-tests/suites/sample-rand-propagation/tracesSampleRate/test.ts rename to dev-packages/node-integration-tests/suites/sample-rand-propagation/test.ts index 4a382f5a20d0..6a55d0a67c36 100644 --- a/dev-packages/node-integration-tests/suites/sample-rand-propagation/tracesSampleRate/test.ts +++ b/dev-packages/node-integration-tests/suites/sample-rand-propagation/test.ts @@ -1,6 +1,6 @@ import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; -describe('sample_rand propagation (default)', () => { +describe('sample_rand propagation', () => { afterAll(() => { cleanupChildProcesses(); }); From 9b394c359afaf1ebf1eebdcd904ac16367bccc7c Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 14 Jan 2025 11:13:41 +0000 Subject: [PATCH 09/45] todo comment --- packages/core/src/tracing/trace.ts | 2 +- packages/opentelemetry/src/sampler.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index 9cd665d204c6..a7841ae631d4 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -414,7 +414,7 @@ function _startRootSpan(spanArguments: SentrySpanArguments, scope: Scope, parent name, parentSampled, attributes, - // parentSampleRate: 'TODO', + // TODO(v9): provide a parentSampleRate here }, sampleRand, ); diff --git a/packages/opentelemetry/src/sampler.ts b/packages/opentelemetry/src/sampler.ts index dc4e7f9d2b38..b71dd7422d7e 100644 --- a/packages/opentelemetry/src/sampler.ts +++ b/packages/opentelemetry/src/sampler.ts @@ -110,9 +110,9 @@ export class SentrySampler implements Sampler { { name: inferredSpanName, attributes: mergedAttributes, - // parentSampleRate: 'TODO', normalizedRequest: isolationScope?.getScopeData().sdkProcessingMetadata.normalizedRequest, parentSampled, + // TODO(v9): provide a parentSampleRate here }, sampleRand, ); From 1d4a6410026bdbbdae80b5cd010a10e33f6ba502 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 14 Jan 2025 12:55:18 +0000 Subject: [PATCH 10/45] unit tests --- .../tracing/browserTracingIntegration.test.ts | 23 ++++++++--- packages/core/src/types-hoist/tracing.ts | 3 +- packages/core/test/lib/feedback.test.ts | 2 + packages/core/test/lib/prepareEvent.test.ts | 3 +- packages/core/test/lib/scope.test.ts | 11 ++++- .../tracing/dynamicSamplingContext.test.ts | 3 ++ packages/core/test/lib/tracing/trace.test.ts | 15 +++++++ .../lib/utils/applyScopeDataToEvent.test.ts | 20 +++++----- .../core/test/lib/utils/traceData.test.ts | 5 ++- .../utils-hoist/proagationContext.test.ts | 10 ----- .../core/test/utils-hoist/tracing.test.ts | 11 +++-- .../feedback/src/core/sendFeedback.test.ts | 1 + .../test/integration/transactions.test.ts | 1 + .../test/integration/transactions.test.ts | 1 + .../opentelemetry/test/propagator.test.ts | 40 ++++++++++++------- packages/opentelemetry/test/trace.test.ts | 10 +++++ .../test/utils/getTraceData.test.ts | 1 + packages/sveltekit/test/server/handle.test.ts | 4 +- 18 files changed, 117 insertions(+), 47 deletions(-) delete mode 100644 packages/core/test/utils-hoist/proagationContext.test.ts diff --git a/packages/browser/test/tracing/browserTracingIntegration.test.ts b/packages/browser/test/tracing/browserTracingIntegration.test.ts index 3e9d2354a532..0b659332df99 100644 --- a/packages/browser/test/tracing/browserTracingIntegration.test.ts +++ b/packages/browser/test/tracing/browserTracingIntegration.test.ts @@ -644,15 +644,19 @@ describe('browserTracingIntegration', () => { expect(oldCurrentScopePropCtx).toEqual({ traceId: expect.stringMatching(/[a-f0-9]{32}/), + sampleRand: expect.any(Number), }); expect(oldIsolationScopePropCtx).toEqual({ traceId: expect.stringMatching(/[a-f0-9]{32}/), + sampleRand: expect.any(Number), }); expect(newCurrentScopePropCtx).toEqual({ traceId: expect.stringMatching(/[a-f0-9]{32}/), + sampleRand: expect.any(Number), }); expect(newIsolationScopePropCtx).toEqual({ traceId: expect.stringMatching(/[a-f0-9]{32}/), + sampleRand: expect.any(Number), }); expect(newIsolationScopePropCtx.traceId).not.toEqual(oldIsolationScopePropCtx.traceId); @@ -676,6 +680,7 @@ describe('browserTracingIntegration', () => { const propCtxBeforeEnd = getCurrentScope().getPropagationContext(); expect(propCtxBeforeEnd).toStrictEqual({ + sampleRand: expect.any(Number), traceId: expect.stringMatching(/[a-f0-9]{32}/), }); @@ -685,6 +690,7 @@ describe('browserTracingIntegration', () => { expect(propCtxAfterEnd).toStrictEqual({ traceId: propCtxBeforeEnd.traceId, sampled: true, + sampleRand: expect.any(Number), dsc: { environment: 'production', public_key: 'examplePublicKey', @@ -692,6 +698,7 @@ describe('browserTracingIntegration', () => { sampled: 'true', transaction: 'mySpan', trace_id: propCtxBeforeEnd.traceId, + sample_rand: expect.any(String), }, }); }); @@ -714,6 +721,7 @@ describe('browserTracingIntegration', () => { const propCtxBeforeEnd = getCurrentScope().getPropagationContext(); expect(propCtxBeforeEnd).toStrictEqual({ traceId: expect.stringMatching(/[a-f0-9]{32}/), + sampleRand: expect.any(Number), }); navigationSpan!.end(); @@ -722,6 +730,7 @@ describe('browserTracingIntegration', () => { expect(propCtxAfterEnd).toStrictEqual({ traceId: propCtxBeforeEnd.traceId, sampled: false, + sampleRand: expect.any(Number), dsc: { environment: 'production', public_key: 'examplePublicKey', @@ -729,6 +738,7 @@ describe('browserTracingIntegration', () => { sampled: 'false', transaction: 'mySpan', trace_id: propCtxBeforeEnd.traceId, + sample_rand: expect.any(String), }, }); }); @@ -739,7 +749,7 @@ describe('browserTracingIntegration', () => { // make sampled false here, so we can see that it's being used rather than the tracesSampleRate-dictated one document.head.innerHTML = '' + - ''; + ''; const client = new BrowserClient( getDefaultBrowserClientOptions({ @@ -765,11 +775,12 @@ describe('browserTracingIntegration', () => { expect(spanIsSampled(idleSpan)).toBe(false); expect(dynamicSamplingContext).toBeDefined(); - expect(dynamicSamplingContext).toStrictEqual({ release: '2.1.14' }); + expect(dynamicSamplingContext).toStrictEqual({ release: '2.1.14', sample_rand: '0.123' }); // Propagation context keeps the meta tag trace data for later events on the same route to add them to the trace expect(propagationContext.traceId).toEqual('12312012123120121231201212312012'); expect(propagationContext.parentSpanId).toEqual('1121201211212012'); + expect(propagationContext.sampleRand).toBe(0.123); }); it('puts frozen Dynamic Sampling Context on pageload span if sentry-trace data and only 3rd party baggage is present', () => { @@ -849,6 +860,7 @@ describe('browserTracingIntegration', () => { public_key: 'examplePublicKey', sample_rate: '1', sampled: 'true', + sample_rand: expect.any(String), trace_id: expect.not.stringContaining('12312012123120121231201212312012'), }); @@ -861,7 +873,7 @@ describe('browserTracingIntegration', () => { // make sampled false here, so we can see that it's being used rather than the tracesSampleRate-dictated one document.head.innerHTML = '' + - ''; + ''; const client = new BrowserClient( getDefaultBrowserClientOptions({ @@ -881,7 +893,7 @@ describe('browserTracingIntegration', () => { }, { sentryTrace: '12312012123120121231201212312011-1121201211212011-1', - baggage: 'sentry-release=2.2.14,foo=bar', + baggage: 'sentry-release=2.2.14,foo=bar,sentry-sample_rand=0.123', }, ); @@ -898,11 +910,12 @@ describe('browserTracingIntegration', () => { expect(spanIsSampled(idleSpan)).toBe(true); expect(dynamicSamplingContext).toBeDefined(); - expect(dynamicSamplingContext).toStrictEqual({ release: '2.2.14' }); + expect(dynamicSamplingContext).toStrictEqual({ release: '2.2.14', sample_rand: '0.123' }); // Propagation context keeps the custom trace data for later events on the same route to add them to the trace expect(propagationContext.traceId).toEqual('12312012123120121231201212312011'); expect(propagationContext.parentSpanId).toEqual('1121201211212011'); + expect(propagationContext.sampleRand).toEqual(0.123); }); }); diff --git a/packages/core/src/types-hoist/tracing.ts b/packages/core/src/types-hoist/tracing.ts index 518744e56aa2..e1dcfef96c6a 100644 --- a/packages/core/src/types-hoist/tracing.ts +++ b/packages/core/src/types-hoist/tracing.ts @@ -17,7 +17,8 @@ export interface PropagationContext { traceId: string; /** - * TODO + * A random between 0 an 1 (including 0, excluding 1) used for sampling in the current execution context. + * This should be newly generated when a new trace is started. */ sampleRand: number; diff --git a/packages/core/test/lib/feedback.test.ts b/packages/core/test/lib/feedback.test.ts index 6980a87df74d..74bfeec1f236 100644 --- a/packages/core/test/lib/feedback.test.ts +++ b/packages/core/test/lib/feedback.test.ts @@ -262,6 +262,7 @@ describe('captureFeedback', () => { traceId, parentSpanId: spanId, dsc, + sampleRand: 0.42, }); const eventId = captureFeedback({ @@ -351,6 +352,7 @@ describe('captureFeedback', () => { sampled: 'true', sample_rate: '1', transaction: 'test-span', + sample_rand: expect.any(String), }, }, [ diff --git a/packages/core/test/lib/prepareEvent.test.ts b/packages/core/test/lib/prepareEvent.test.ts index 4a1951f00d08..9806bd06bd14 100644 --- a/packages/core/test/lib/prepareEvent.test.ts +++ b/packages/core/test/lib/prepareEvent.test.ts @@ -238,6 +238,7 @@ describe('parseEventHintOrCaptureContext', () => { fingerprint: ['xx', 'yy'], propagationContext: { traceId: 'xxx', + sampleRand: Math.random(), }, }; @@ -328,7 +329,7 @@ describe('prepareEvent', () => { tags: { tag1: 'aa', tag2: 'aa' }, extra: { extra1: 'aa', extra2: 'aa' }, contexts: { os: { name: 'os1' }, culture: { display_name: 'name1' } }, - propagationContext: { traceId: '1' }, + propagationContext: { traceId: '1', sampleRand: 0.42 }, fingerprint: ['aa'], }); scope.addBreadcrumb(breadcrumb1); diff --git a/packages/core/test/lib/scope.test.ts b/packages/core/test/lib/scope.test.ts index 58a8aefae28e..026d33fd54e5 100644 --- a/packages/core/test/lib/scope.test.ts +++ b/packages/core/test/lib/scope.test.ts @@ -32,6 +32,7 @@ describe('Scope', () => { eventProcessors: [], propagationContext: { traceId: expect.any(String), + sampleRand: expect.any(Number), }, sdkProcessingMetadata: {}, }); @@ -57,6 +58,7 @@ describe('Scope', () => { eventProcessors: [], propagationContext: { traceId: expect.any(String), + sampleRand: expect.any(Number), }, sdkProcessingMetadata: {}, }); @@ -90,6 +92,7 @@ describe('Scope', () => { eventProcessors: [], propagationContext: { traceId: expect.any(String), + sampleRand: expect.any(Number), }, sdkProcessingMetadata: {}, }); @@ -101,6 +104,7 @@ describe('Scope', () => { expect(scope.getScopeData().propagationContext).toEqual({ traceId: expect.any(String), + sampleRand: expect.any(Number), sampled: undefined, dsc: undefined, parentSpanId: undefined, @@ -228,12 +232,14 @@ describe('Scope', () => { const oldPropagationContext = scope.getPropagationContext(); scope.setPropagationContext({ traceId: '86f39e84263a4de99c326acab3bfe3bd', + sampleRand: 0.42, sampled: true, }); expect(scope.getPropagationContext()).not.toEqual(oldPropagationContext); expect(scope.getPropagationContext()).toEqual({ traceId: '86f39e84263a4de99c326acab3bfe3bd', sampled: true, + sampleRand: 0.42, }); }); @@ -293,6 +299,7 @@ describe('Scope', () => { expect(scope['_propagationContext']).toEqual({ traceId: expect.any(String), sampled: undefined, + sampleRand: expect.any(Number), }); expect(scope['_propagationContext']).not.toEqual(oldPropagationContext); }); @@ -420,6 +427,7 @@ describe('Scope', () => { propagationContext: { traceId: '8949daf83f4a4a70bee4c1eb9ab242ed', sampled: true, + sampleRand: 0.42, }, }; @@ -446,6 +454,7 @@ describe('Scope', () => { expect(updatedScope._propagationContext).toEqual({ traceId: '8949daf83f4a4a70bee4c1eb9ab242ed', sampled: true, + sampleRand: 0.42, }); }); }); @@ -493,7 +502,7 @@ describe('Scope', () => { tags: { tag1: 'aa', tag2: 'aa' }, extra: { extra1: 'aa', extra2: 'aa' }, contexts: { os: { name: 'os1' }, culture: { display_name: 'name1' } }, - propagationContext: { traceId: '1' }, + propagationContext: { traceId: '1', sampleRand: 0.42 }, fingerprint: ['aa'], }); scope.addBreadcrumb(breadcrumb1); diff --git a/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts b/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts index 579f90fbad56..3856b9d35d13 100644 --- a/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts +++ b/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts @@ -71,6 +71,7 @@ describe('getDynamicSamplingContextFromSpan', () => { sample_rate: '0.56', trace_id: expect.stringMatching(/^[a-f0-9]{32}$/), transaction: 'tx', + sample_rand: expect.any(String), }); }); @@ -88,6 +89,7 @@ describe('getDynamicSamplingContextFromSpan', () => { sample_rate: '1', trace_id: expect.stringMatching(/^[a-f0-9]{32}$/), transaction: 'tx', + sample_rand: expect.any(String), }); }); @@ -110,6 +112,7 @@ describe('getDynamicSamplingContextFromSpan', () => { sample_rate: '0.56', trace_id: expect.stringMatching(/^[a-f0-9]{32}$/), transaction: 'tx', + sample_rand: undefined, // this is a bit funky admittedly }); }); diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index 83b875edb59b..8eb7d054c048 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -473,6 +473,7 @@ describe('startSpan', () => { sample_rate: '1', transaction: 'outer transaction', sampled: 'true', + sample_rand: expect.any(String), }, }); @@ -498,6 +499,7 @@ describe('startSpan', () => { sample_rate: '1', transaction: 'outer transaction', sampled: 'true', + sample_rand: expect.any(String), }, }); }); @@ -507,6 +509,7 @@ describe('startSpan', () => { withScope(scope => { scope.setPropagationContext({ traceId: '99999999999999999999999999999999', + sampleRand: Math.random(), dsc: {}, parentSpanId: '4242424242424242', }); @@ -902,6 +905,7 @@ describe('startSpanManual', () => { sample_rate: '1', transaction: 'outer transaction', sampled: 'true', + sample_rand: expect.any(String), }, }); @@ -927,6 +931,7 @@ describe('startSpanManual', () => { sample_rate: '1', transaction: 'outer transaction', sampled: 'true', + sample_rand: expect.any(String), }, }); }); @@ -945,6 +950,7 @@ describe('startSpanManual', () => { withScope(scope => { scope.setPropagationContext({ traceId: '99999999999999999999999999999991', + sampleRand: Math.random(), dsc: {}, parentSpanId: '4242424242424242', }); @@ -1230,6 +1236,7 @@ describe('startInactiveSpan', () => { sample_rate: '1', transaction: 'outer transaction', sampled: 'true', + sample_rand: expect.any(String), }, }); @@ -1255,6 +1262,7 @@ describe('startInactiveSpan', () => { sample_rate: '1', transaction: 'outer transaction', sampled: 'true', + sample_rand: expect.any(String), }, }); }); @@ -1269,6 +1277,7 @@ describe('startInactiveSpan', () => { withScope(scope => { scope.setPropagationContext({ traceId: '99999999999999999999999999999991', + sampleRand: Math.random(), dsc: {}, parentSpanId: '4242424242424242', }); @@ -1451,6 +1460,7 @@ describe('continueTrace', () => { expect(scope.getPropagationContext()).toEqual({ sampled: undefined, traceId: expect.any(String), + sampleRand: expect.any(Number), }); expect(scope.getScopeData().sdkProcessingMetadata).toEqual({}); @@ -1472,6 +1482,7 @@ describe('continueTrace', () => { sampled: false, parentSpanId: '1121201211212012', traceId: '12312012123120121231201212312012', + sampleRand: expect.any(Number), }); expect(scope.getScopeData().sdkProcessingMetadata).toEqual({}); @@ -1492,10 +1503,12 @@ describe('continueTrace', () => { dsc: { environment: 'production', version: '1.0', + sample_rand: expect.any(String), }, sampled: true, parentSpanId: '1121201211212012', traceId: '12312012123120121231201212312012', + sampleRand: expect.any(Number), }); expect(scope.getScopeData().sdkProcessingMetadata).toEqual({}); @@ -1516,10 +1529,12 @@ describe('continueTrace', () => { dsc: { environment: 'production', version: '1.0', + sample_rand: expect.any(String), }, sampled: true, parentSpanId: '1121201211212012', traceId: '12312012123120121231201212312012', + sampleRand: expect.any(Number), }); expect(scope.getScopeData().sdkProcessingMetadata).toEqual({}); diff --git a/packages/core/test/lib/utils/applyScopeDataToEvent.test.ts b/packages/core/test/lib/utils/applyScopeDataToEvent.test.ts index 553dd4cc6aa5..077190670aba 100644 --- a/packages/core/test/lib/utils/applyScopeDataToEvent.test.ts +++ b/packages/core/test/lib/utils/applyScopeDataToEvent.test.ts @@ -82,7 +82,7 @@ describe('mergeScopeData', () => { extra: {}, contexts: {}, attachments: [], - propagationContext: { traceId: '1' }, + propagationContext: { traceId: '1', sampleRand: 0.42 }, sdkProcessingMetadata: {}, fingerprint: [], }; @@ -94,7 +94,7 @@ describe('mergeScopeData', () => { extra: {}, contexts: {}, attachments: [], - propagationContext: { traceId: '1' }, + propagationContext: { traceId: '1', sampleRand: 0.42 }, sdkProcessingMetadata: {}, fingerprint: [], }; @@ -107,7 +107,7 @@ describe('mergeScopeData', () => { extra: {}, contexts: {}, attachments: [], - propagationContext: { traceId: '1' }, + propagationContext: { traceId: '1', sampleRand: 0.42 }, sdkProcessingMetadata: {}, fingerprint: [], }); @@ -134,7 +134,7 @@ describe('mergeScopeData', () => { extra: { extra1: 'aa', extra2: 'aa' }, contexts: { os: { name: 'os1' }, culture: { display_name: 'name1' } }, attachments: [attachment1], - propagationContext: { traceId: '1' }, + propagationContext: { traceId: '1', sampleRand: 0.42 }, sdkProcessingMetadata: { aa: 'aa', bb: 'aa', @@ -154,7 +154,7 @@ describe('mergeScopeData', () => { extra: { extra2: 'bb', extra3: 'bb' }, contexts: { os: { name: 'os2' } }, attachments: [attachment2, attachment3], - propagationContext: { traceId: '2' }, + propagationContext: { traceId: '2', sampleRand: 0.42 }, sdkProcessingMetadata: { bb: 'bb', cc: 'bb', @@ -175,7 +175,7 @@ describe('mergeScopeData', () => { extra: { extra1: 'aa', extra2: 'bb', extra3: 'bb' }, contexts: { os: { name: 'os2' }, culture: { display_name: 'name1' } }, attachments: [attachment1, attachment2, attachment3], - propagationContext: { traceId: '2' }, + propagationContext: { traceId: '2', sampleRand: 0.42 }, sdkProcessingMetadata: { aa: 'aa', bb: 'bb', @@ -202,7 +202,7 @@ describe('applyScopeDataToEvent', () => { extra: {}, contexts: {}, attachments: [], - propagationContext: { traceId: '1' }, + propagationContext: { traceId: '1', sampleRand: 0.42 }, sdkProcessingMetadata: {}, fingerprint: [], transactionName: 'foo', @@ -223,7 +223,7 @@ describe('applyScopeDataToEvent', () => { extra: {}, contexts: {}, attachments: [], - propagationContext: { traceId: '1' }, + propagationContext: { traceId: '1', sampleRand: 0.42 }, sdkProcessingMetadata: {}, fingerprint: [], transactionName: 'foo', @@ -254,7 +254,7 @@ describe('applyScopeDataToEvent', () => { extra: {}, contexts: {}, attachments: [], - propagationContext: { traceId: '1' }, + propagationContext: { traceId: '1', sampleRand: 0.42 }, sdkProcessingMetadata: {}, fingerprint: [], transactionName: '/users/:id', @@ -278,7 +278,7 @@ describe('applyScopeDataToEvent', () => { extra: {}, contexts: {}, attachments: [], - propagationContext: { traceId: '1' }, + propagationContext: { traceId: '1', sampleRand: 0.42 }, sdkProcessingMetadata: {}, fingerprint: [], transactionName: 'foo', diff --git a/packages/core/test/lib/utils/traceData.test.ts b/packages/core/test/lib/utils/traceData.test.ts index f7978b24f41d..78c8d806be2a 100644 --- a/packages/core/test/lib/utils/traceData.test.ts +++ b/packages/core/test/lib/utils/traceData.test.ts @@ -23,6 +23,7 @@ const SCOPE_TRACE_ID = '12345678901234567890123456789012'; function setupClient(opts?: Partial): Client { getCurrentScope().setPropagationContext({ traceId: SCOPE_TRACE_ID, + sampleRand: Math.random(), }); const options = getDefaultTestClientOptions({ @@ -163,10 +164,12 @@ describe('getTraceData', () => { traceId: '12345678901234567890123456789012', sampled: true, parentSpanId: '1234567890123456', + sampleRand: 0.42, dsc: { environment: 'staging', public_key: 'key', trace_id: '12345678901234567890123456789012', + sample_rand: '0.42', }, }); @@ -174,7 +177,7 @@ describe('getTraceData', () => { expect(traceData['sentry-trace']).toMatch(/^12345678901234567890123456789012-[a-f0-9]{16}-1$/); expect(traceData.baggage).toEqual( - 'sentry-environment=staging,sentry-public_key=key,sentry-trace_id=12345678901234567890123456789012', + 'sentry-environment=staging,sentry-public_key=key,sentry-trace_id=12345678901234567890123456789012,sentry-sample_rand=0.42', ); }); diff --git a/packages/core/test/utils-hoist/proagationContext.test.ts b/packages/core/test/utils-hoist/proagationContext.test.ts deleted file mode 100644 index 3c8812e688f3..000000000000 --- a/packages/core/test/utils-hoist/proagationContext.test.ts +++ /dev/null @@ -1,10 +0,0 @@ -import { generatePropagationContext } from '../../src/utils-hoist/propagationContext'; - -describe('generatePropagationContext', () => { - it('generates a new minimal propagation context', () => { - // eslint-disable-next-line deprecation/deprecation - expect(generatePropagationContext()).toEqual({ - traceId: expect.stringMatching(/^[0-9a-f]{32}$/), - }); - }); -}); diff --git a/packages/core/test/utils-hoist/tracing.test.ts b/packages/core/test/utils-hoist/tracing.test.ts index c2137871aa40..75b39a573437 100644 --- a/packages/core/test/utils-hoist/tracing.test.ts +++ b/packages/core/test/utils-hoist/tracing.test.ts @@ -1,30 +1,33 @@ import { extractTraceparentData, propagationContextFromHeaders } from '../../src/utils-hoist/tracing'; const EXAMPLE_SENTRY_TRACE = '12312012123120121231201212312012-1121201211212012-1'; -const EXAMPLE_BAGGAGE = 'sentry-release=1.2.3,sentry-foo=bar,other=baz'; +const EXAMPLE_BAGGAGE = 'sentry-release=1.2.3,sentry-foo=bar,other=baz,sentry-sample_rand=0.42'; describe('propagationContextFromHeaders()', () => { it('returns a completely new propagation context when no sentry-trace data is given but baggage data is given', () => { const result = propagationContextFromHeaders(undefined, undefined); expect(result).toEqual({ traceId: expect.any(String), + sampleRand: expect.any(Number), }); }); it('returns a completely new propagation context when no sentry-trace data is given', () => { const result = propagationContextFromHeaders(undefined, EXAMPLE_BAGGAGE); - expect(result).toEqual({ + expect(result).toStrictEqual({ traceId: expect.any(String), + sampleRand: expect.any(Number), }); }); it('returns the correct traceparent data within the propagation context when sentry trace data is given', () => { const result = propagationContextFromHeaders(EXAMPLE_SENTRY_TRACE, undefined); - expect(result).toEqual( + expect(result).toStrictEqual( expect.objectContaining({ traceId: '12312012123120121231201212312012', parentSpanId: '1121201211212012', sampled: true, + sampleRand: expect.any(Number), }), ); }); @@ -44,9 +47,11 @@ describe('propagationContextFromHeaders()', () => { traceId: '12312012123120121231201212312012', parentSpanId: '1121201211212012', sampled: true, + sampleRand: 0.42, dsc: { release: '1.2.3', foo: 'bar', + sample_rand: '0.42', }, }); }); diff --git a/packages/feedback/src/core/sendFeedback.test.ts b/packages/feedback/src/core/sendFeedback.test.ts index 826dd3f05daf..cf6c09f2e6f5 100644 --- a/packages/feedback/src/core/sendFeedback.test.ts +++ b/packages/feedback/src/core/sendFeedback.test.ts @@ -163,6 +163,7 @@ describe('sendFeedback', () => { sample_rate: '1', sampled: 'true', transaction: 'test span', + sample_rand: expect.any(String), }, }, [ diff --git a/packages/node/test/integration/transactions.test.ts b/packages/node/test/integration/transactions.test.ts index 525b2978fc2e..8b481c2bf1a4 100644 --- a/packages/node/test/integration/transactions.test.ts +++ b/packages/node/test/integration/transactions.test.ts @@ -109,6 +109,7 @@ describe('Integration | Transactions', () => { release: '8.0.0', trace_id: expect.stringMatching(/[a-f0-9]{32}/), transaction: 'test name', + sample_rand: expect.any(String), }); expect(transaction.environment).toEqual('production'); diff --git a/packages/opentelemetry/test/integration/transactions.test.ts b/packages/opentelemetry/test/integration/transactions.test.ts index 3e299574d51b..8000c078e3bf 100644 --- a/packages/opentelemetry/test/integration/transactions.test.ts +++ b/packages/opentelemetry/test/integration/transactions.test.ts @@ -124,6 +124,7 @@ describe('Integration | Transactions', () => { trace_id: expect.stringMatching(/[a-f0-9]{32}/), transaction: 'test name', release: '8.0.0', + sample_rand: expect.any(String), }); expect(transaction.environment).toEqual('production'); diff --git a/packages/opentelemetry/test/propagator.test.ts b/packages/opentelemetry/test/propagator.test.ts index 13d90e963f8a..408a151e95ef 100644 --- a/packages/opentelemetry/test/propagator.test.ts +++ b/packages/opentelemetry/test/propagator.test.ts @@ -46,6 +46,7 @@ describe('SentryPropagator', () => { traceId: 'd4cda95b652f4a1592b449d5929fda1b', parentSpanId: '6e0c63257de34c93', sampled: true, + sampleRand: Math.random(), }); propagator.inject(context.active(), carrier, defaultTextMapSetter); @@ -68,6 +69,7 @@ describe('SentryPropagator', () => { traceId: 'd4cda95b652f4a1592b449d5929fda1b', parentSpanId: '6e0c63257de34c93', sampled: true, + sampleRand: Math.random(), dsc: { transaction: 'sampled-transaction', sampled: 'false', @@ -133,6 +135,7 @@ describe('SentryPropagator', () => { 'sentry-sampled=true', 'sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b', 'sentry-transaction=test', + expect.stringMatching(/sentry-sample_rand=0\.[0-9]+/), ], 'd4cda95b652f4a1592b449d5929fda1b-{{spanId}}-1', true, @@ -186,6 +189,7 @@ describe('SentryPropagator', () => { 'sentry-sampled=true', 'sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b', 'sentry-transaction=test', + expect.stringMatching(/sentry-sample_rand=0\.[0-9]+/), ], 'd4cda95b652f4a1592b449d5929fda1b-{{spanId}}-1', undefined, @@ -299,8 +303,9 @@ describe('SentryPropagator', () => { context.with(trace.setSpanContext(ROOT_CONTEXT, spanContext), () => { trace.getTracer('test').startActiveSpan('test', span => { propagator.inject(context.active(), carrier, defaultTextMapSetter); - - expect(baggageToArray(carrier[SENTRY_BAGGAGE_HEADER])).toEqual(baggage.sort()); + baggage.forEach(baggageItem => { + expect(baggageToArray(carrier[SENTRY_BAGGAGE_HEADER])).toContainEqual(baggageItem); + }); expect(carrier[SENTRY_TRACE_HEADER]).toBe(sentryTrace.replace('{{spanId}}', span.spanContext().spanId)); }); }); @@ -321,21 +326,23 @@ describe('SentryPropagator', () => { traceId: 'TRACE_ID', parentSpanId: 'PARENT_SPAN_ID', sampled: true, + sampleRand: Math.random(), }); propagator.inject(context.active(), carrier, defaultTextMapSetter); - expect(baggageToArray(carrier[SENTRY_BAGGAGE_HEADER])).toEqual( - [ - 'sentry-environment=production', - 'sentry-release=1.0.0', - 'sentry-public_key=abc', - 'sentry-sample_rate=1', - 'sentry-sampled=true', - 'sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b', - 'sentry-transaction=test', - ].sort(), - ); + [ + 'sentry-environment=production', + 'sentry-release=1.0.0', + 'sentry-public_key=abc', + 'sentry-sample_rate=1', + 'sentry-sampled=true', + 'sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b', + 'sentry-transaction=test', + expect.stringMatching(/sentry-sample_rand=0\.[0-9]+/), + ].forEach(item => { + expect(baggageToArray(carrier[SENTRY_BAGGAGE_HEADER])).toContainEqual(item); + }); expect(carrier[SENTRY_TRACE_HEADER]).toBe( `d4cda95b652f4a1592b449d5929fda1b-${span.spanContext().spanId}-1`, ); @@ -360,6 +367,7 @@ describe('SentryPropagator', () => { traceId: 'TRACE_ID', parentSpanId: 'PARENT_SPAN_ID', sampled: true, + sampleRand: Math.random(), }); propagator.inject(context.active(), carrier, defaultTextMapSetter); @@ -396,6 +404,7 @@ describe('SentryPropagator', () => { traceId: 'TRACE_ID', parentSpanId: 'PARENT_SPAN_ID', sampled: true, + sampleRand: Math.random(), }); propagator.inject(context.active(), carrier, defaultTextMapSetter); @@ -597,13 +606,14 @@ describe('SentryPropagator', () => { expect(trace.getSpanContext(context)).toEqual(undefined); expect(getCurrentScope().getPropagationContext()).toEqual({ traceId: expect.stringMatching(/[a-f0-9]{32}/), + sampleRand: expect.any(Number), }); }); it('sets data from baggage header on span context', () => { const sentryTraceHeader = 'd4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-1'; const baggage = - 'sentry-environment=production,sentry-release=1.0.0,sentry-public_key=abc,sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b,sentry-transaction=dsc-transaction'; + 'sentry-environment=production,sentry-release=1.0.0,sentry-public_key=abc,sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b,sentry-transaction=dsc-transaction,sentry-sample_rand=0.123'; carrier[SENTRY_TRACE_HEADER] = sentryTraceHeader; carrier[SENTRY_BAGGAGE_HEADER] = baggage; const context = propagator.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter); @@ -619,6 +629,7 @@ describe('SentryPropagator', () => { public_key: 'abc', trace_id: 'd4cda95b652f4a1592b449d5929fda1b', transaction: 'dsc-transaction', + sample_rand: '0.123', }, }), }); @@ -647,6 +658,7 @@ describe('SentryPropagator', () => { expect(trace.getSpanContext(context)).toEqual(undefined); expect(getCurrentScope().getPropagationContext()).toEqual({ traceId: expect.stringMatching(/[a-f0-9]{32}/), + sampleRand: expect.any(Number), }); }); }); diff --git a/packages/opentelemetry/test/trace.test.ts b/packages/opentelemetry/test/trace.test.ts index 8639ee354e2d..2c677ded4516 100644 --- a/packages/opentelemetry/test/trace.test.ts +++ b/packages/opentelemetry/test/trace.test.ts @@ -426,6 +426,7 @@ describe('trace', () => { sample_rate: '1', transaction: 'outer transaction', sampled: 'true', + sample_rand: expect.any(String), }, }); @@ -451,6 +452,7 @@ describe('trace', () => { sample_rate: '1', transaction: 'outer transaction', sampled: 'true', + sample_rand: expect.any(String), }, }); }); @@ -683,6 +685,7 @@ describe('trace', () => { sample_rate: '1', transaction: 'outer transaction', sampled: 'true', + sample_rand: expect.any(String), }, }); @@ -708,6 +711,7 @@ describe('trace', () => { sample_rate: '1', transaction: 'outer transaction', sampled: 'true', + sample_rand: expect.any(String), }, }); }); @@ -978,6 +982,7 @@ describe('trace', () => { sample_rate: '1', transaction: 'outer transaction', sampled: 'true', + sample_rand: expect.any(String), }, }); @@ -1003,6 +1008,7 @@ describe('trace', () => { sample_rate: '1', transaction: 'outer transaction', sampled: 'true', + sample_rand: expect.any(String), }, }); }); @@ -1049,6 +1055,7 @@ describe('trace', () => { sample_rate: '1', sampled: 'true', transaction: 'test span', + sample_rand: expect.any(String), }); }); }); @@ -1073,6 +1080,7 @@ describe('trace', () => { sample_rate: '1', sampled: 'true', transaction: 'test span', + sample_rand: expect.any(String), }); }); }); @@ -1093,6 +1101,7 @@ describe('trace', () => { transaction: 'parent span', sampled: 'true', sample_rate: '1', + sample_rand: expect.any(String), }); }); }); @@ -1582,6 +1591,7 @@ describe('continueTrace', () => { expect(scope.getPropagationContext()).toEqual({ traceId: expect.any(String), + sampleRand: expect.any(Number), }); expect(scope.getScopeData().sdkProcessingMetadata).toEqual({}); diff --git a/packages/opentelemetry/test/utils/getTraceData.test.ts b/packages/opentelemetry/test/utils/getTraceData.test.ts index f18f1c307639..cde519143e26 100644 --- a/packages/opentelemetry/test/utils/getTraceData.test.ts +++ b/packages/opentelemetry/test/utils/getTraceData.test.ts @@ -54,6 +54,7 @@ describe('getTraceData', () => { it('returns propagationContext DSC data if no span is available', () => { getCurrentScope().setPropagationContext({ traceId: '12345678901234567890123456789012', + sampleRand: Math.random(), sampled: true, dsc: { environment: 'staging', diff --git a/packages/sveltekit/test/server/handle.test.ts b/packages/sveltekit/test/server/handle.test.ts index c5225124ace3..7097c46dd4cd 100644 --- a/packages/sveltekit/test/server/handle.test.ts +++ b/packages/sveltekit/test/server/handle.test.ts @@ -296,7 +296,8 @@ describe('sentryHandle', () => { return ( 'sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,' + 'sentry-public_key=dogsarebadatkeepingsecrets,' + - 'sentry-trace_id=1234567890abcdef1234567890abcdef,sentry-sample_rate=1' + 'sentry-trace_id=1234567890abcdef1234567890abcdef,sentry-sample_rate=1,' + + 'sentry-sample_rand=0.42' ); } @@ -332,6 +333,7 @@ describe('sentryHandle', () => { sample_rate: '1', trace_id: '1234567890abcdef1234567890abcdef', transaction: 'dogpark', + sample_rand: '0.42', }); }); From 2d35988184f1f5442a1dbc8364a96052452bcf4a Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 14 Jan 2025 14:23:12 +0000 Subject: [PATCH 11/45] node integration tests --- .../express/sentry-trace/baggage-header-assign/test.ts | 4 ++-- .../express/sentry-trace/baggage-header-out/test.ts | 9 ++++++--- .../baggage-other-vendors-with-sentry-entries/test.ts | 2 ++ .../express/sentry-trace/baggage-other-vendors/test.ts | 4 ++-- .../suites/sample-rand-propagation/test.ts | 2 +- .../suites/tracing/dsc-txn-name-update/test.ts | 7 +++++++ .../tracing/envelope-header/error-active-span/test.ts | 1 + .../envelope-header/sampleRate-propagation/test.ts | 3 ++- .../tracing/envelope-header/transaction-route/test.ts | 1 + .../tracing/envelope-header/transaction-url/test.ts | 1 + .../suites/tracing/envelope-header/transaction/test.ts | 1 + .../suites/tracing/meta-tags/test.ts | 4 ++-- 12 files changed, 28 insertions(+), 11 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts index 0ee5ca2204f5..b873e4a5c0aa 100644 --- a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts +++ b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts @@ -29,7 +29,7 @@ test('Should propagate sentry trace baggage data from an incoming to an outgoing const response = await runner.makeRequest('get', '/test/express', { headers: { 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1', - baggage: 'sentry-release=2.0.0,sentry-environment=myEnv,dogs=great', + baggage: 'sentry-release=2.0.0,sentry-environment=myEnv,dogs=great,sentry-sample_rand=0.42', }, }); @@ -37,7 +37,7 @@ test('Should propagate sentry trace baggage data from an incoming to an outgoing expect(response).toMatchObject({ test_data: { host: 'somewhere.not.sentry', - baggage: 'sentry-release=2.0.0,sentry-environment=myEnv', + baggage: 'sentry-release=2.0.0,sentry-environment=myEnv,sentry-sample_rand=0.42', }, }); }); diff --git a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts index 5a052a454b56..72b6a7139f35 100644 --- a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts +++ b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts @@ -12,9 +12,9 @@ test('should attach a baggage header to an outgoing request.', async () => { expect(response).toBeDefined(); - const baggage = response?.test_data.baggage?.split(',').sort(); + const baggage = response?.test_data.baggage?.split(','); - expect(baggage).toEqual([ + [ 'sentry-environment=prod', 'sentry-public_key=public', 'sentry-release=1.0', @@ -22,7 +22,10 @@ test('should attach a baggage header to an outgoing request.', async () => { 'sentry-sampled=true', 'sentry-trace_id=__SENTRY_TRACE_ID__', 'sentry-transaction=GET%20%2Ftest%2Fexpress', - ]); + expect.stringMatching(/sentry-sample_rand=0\.[0-9]+/), + ].forEach(item => { + expect(baggage).toContainEqual(item); + }); expect(response).toMatchObject({ test_data: { diff --git a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts index 0e083f5c2dc6..ebf2a15bedf4 100644 --- a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts +++ b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts @@ -31,6 +31,7 @@ test('should ignore sentry-values in `baggage` header of a third party vendor an 'other=vendor', 'sentry-environment=myEnv', 'sentry-release=2.1.0', + expect.stringMatching(/sentry-sample_rand=[0-9]+/), 'sentry-sample_rate=0.54', 'third=party', ]); @@ -58,6 +59,7 @@ test('should ignore sentry-values in `baggage` header of a third party vendor an 'sentry-environment=prod', 'sentry-public_key=public', 'sentry-release=1.0', + expect.stringMatching(/sentry-sample_rand=[0-9]+/), 'sentry-sample_rate=1', 'sentry-sampled=true', expect.stringMatching(/sentry-trace_id=[0-9a-f]{32}/), diff --git a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors/test.ts b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors/test.ts index 2403da850d9d..0beecb54a905 100644 --- a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors/test.ts +++ b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors/test.ts @@ -11,7 +11,7 @@ test('should merge `baggage` header of a third party vendor with the Sentry DSC const response = await runner.makeRequest('get', '/test/express', { headers: { 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1', - baggage: 'sentry-release=2.0.0,sentry-environment=myEnv', + baggage: 'sentry-release=2.0.0,sentry-environment=myEnv,sentry-sample_rand=0.42', }, }); @@ -19,7 +19,7 @@ test('should merge `baggage` header of a third party vendor with the Sentry DSC expect(response).toMatchObject({ test_data: { host: 'somewhere.not.sentry', - baggage: 'other=vendor,foo=bar,third=party,sentry-release=2.0.0,sentry-environment=myEnv', + baggage: 'other=vendor,foo=bar,third=party,sentry-release=2.0.0,sentry-environment=myEnv,sentry-sample_rand=0.42', }, }); }); diff --git a/dev-packages/node-integration-tests/suites/sample-rand-propagation/test.ts b/dev-packages/node-integration-tests/suites/sample-rand-propagation/test.ts index 6a55d0a67c36..0253bfddd843 100644 --- a/dev-packages/node-integration-tests/suites/sample-rand-propagation/test.ts +++ b/dev-packages/node-integration-tests/suites/sample-rand-propagation/test.ts @@ -1,4 +1,4 @@ -import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; +import { cleanupChildProcesses, createRunner } from '../../utils/runner'; describe('sample_rand propagation', () => { afterAll(() => { diff --git a/dev-packages/node-integration-tests/suites/tracing/dsc-txn-name-update/test.ts b/dev-packages/node-integration-tests/suites/tracing/dsc-txn-name-update/test.ts index 82f86baa835f..f669f50f5d7b 100644 --- a/dev-packages/node-integration-tests/suites/tracing/dsc-txn-name-update/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/dsc-txn-name-update/test.ts @@ -17,6 +17,7 @@ test('adds current transaction name to baggage when the txn name is high-quality 'sentry-environment=production', 'sentry-public_key=public', 'sentry-release=1.0', + expect.stringMatching(/sentry-sample_rand=0\.[0-9]+/), 'sentry-sample_rate=1', 'sentry-sampled=true', `sentry-trace_id=${traceId}`, @@ -27,6 +28,7 @@ test('adds current transaction name to baggage when the txn name is high-quality 'sentry-environment=production', 'sentry-public_key=public', 'sentry-release=1.0', + expect.stringMatching(/sentry-sample_rand=0\.[0-9]+/), 'sentry-sample_rate=1', 'sentry-sampled=true', `sentry-trace_id=${traceId}`, @@ -38,6 +40,7 @@ test('adds current transaction name to baggage when the txn name is high-quality 'sentry-environment=production', 'sentry-public_key=public', 'sentry-release=1.0', + expect.stringMatching(/sentry-sample_rand=0\.[0-9]+/), 'sentry-sample_rate=1', 'sentry-sampled=true', `sentry-trace_id=${traceId}`, @@ -68,6 +71,7 @@ test('adds current transaction name to trace envelope header when the txn name i sample_rate: '1', sampled: 'true', trace_id: expect.stringMatching(/[a-f0-9]{32}/), + sample_rand: expect.any(String), }, }, }) @@ -81,6 +85,7 @@ test('adds current transaction name to trace envelope header when the txn name i sampled: 'true', trace_id: expect.stringMatching(/[a-f0-9]{32}/), transaction: 'updated-name-1', + sample_rand: expect.any(String), }, }, }) @@ -94,6 +99,7 @@ test('adds current transaction name to trace envelope header when the txn name i sampled: 'true', trace_id: expect.stringMatching(/[a-f0-9]{32}/), transaction: 'updated-name-2', + sample_rand: expect.any(String), }, }, }) @@ -107,6 +113,7 @@ test('adds current transaction name to trace envelope header when the txn name i sampled: 'true', trace_id: expect.stringMatching(/[a-f0-9]{32}/), transaction: 'updated-name-2', + sample_rand: expect.any(String), }, }, }) diff --git a/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span/test.ts b/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span/test.ts index 6749f275035b..51d62deb75af 100644 --- a/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span/test.ts @@ -13,6 +13,7 @@ test('envelope header for error event during active span is correct', done => { sample_rate: '1', sampled: 'true', transaction: 'test span', + sample_rand: expect.any(String), }, }, }) diff --git a/dev-packages/node-integration-tests/suites/tracing/envelope-header/sampleRate-propagation/test.ts b/dev-packages/node-integration-tests/suites/tracing/envelope-header/sampleRate-propagation/test.ts index c7d7b6a4e433..55223beff4f6 100644 --- a/dev-packages/node-integration-tests/suites/tracing/envelope-header/sampleRate-propagation/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/envelope-header/sampleRate-propagation/test.ts @@ -16,6 +16,7 @@ describe('tracesSampleRate propagation', () => { sampled: 'true', trace_id: traceId, transaction: 'myTransaction', + sample_rand: '0.42', }, }, }) @@ -23,7 +24,7 @@ describe('tracesSampleRate propagation', () => { .makeRequest('get', '/test', { headers: { 'sentry-trace': `${traceId}-1234567812345678-1`, - baggage: `sentry-sample_rate=0.05,sentry-trace_id=${traceId},sentry-sampled=true,sentry-transaction=myTransaction`, + baggage: `sentry-sample_rate=0.05,sentry-trace_id=${traceId},sentry-sampled=true,sentry-transaction=myTransaction,sentry-sample_rand=0.42`, }, }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-route/test.ts b/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-route/test.ts index 592d75f30ae6..15088157994d 100644 --- a/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-route/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-route/test.ts @@ -12,6 +12,7 @@ test('envelope header for transaction event of route correct', done => { release: '1.0', sample_rate: '1', sampled: 'true', + sample_rand: expect.any(String), }, }, }) diff --git a/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-url/test.ts b/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-url/test.ts index a7de2f95c965..8b5eb84392c9 100644 --- a/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-url/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-url/test.ts @@ -11,6 +11,7 @@ test('envelope header for transaction event with source=url correct', done => { release: '1.0', sample_rate: '1', sampled: 'true', + sample_rand: expect.any(String), }, }, }) diff --git a/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction/test.ts b/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction/test.ts index 3d4ff2d8d96a..1f26a45ffcac 100644 --- a/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction/test.ts @@ -12,6 +12,7 @@ test('envelope header for transaction event is correct', done => { sample_rate: '1', sampled: 'true', transaction: 'test span', + sample_rand: expect.any(String), }, }, }) diff --git a/dev-packages/node-integration-tests/suites/tracing/meta-tags/test.ts b/dev-packages/node-integration-tests/suites/tracing/meta-tags/test.ts index 7c94d30b686a..0d61adf18913 100644 --- a/dev-packages/node-integration-tests/suites/tracing/meta-tags/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/meta-tags/test.ts @@ -14,7 +14,7 @@ describe('getTraceMetaTags', () => { const response = await runner.makeRequest('get', '/test', { headers: { 'sentry-trace': `${traceId}-${parentSpanId}-1`, - baggage: 'sentry-environment=production', + baggage: 'sentry-environment=production,sentry-sample_rand=0.42', }, }); @@ -22,7 +22,7 @@ describe('getTraceMetaTags', () => { const html = response?.response as unknown as string; expect(html).toMatch(//); - expect(html).toContain(''); + expect(html).toContain(''); }); test('injects tags with new trace if no incoming headers', async () => { From 957bc091ab31ad8617c955987b8a3e0c7841bf54 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 14 Jan 2025 15:44:02 +0000 Subject: [PATCH 12/45] browser integration tests --- .../standalone-mixed-transaction/test.ts | 2 ++ .../public-api/startSpan/standalone/test.ts | 1 + .../suites/replay/dsc/test.ts | 6 ++++++ .../browserTracingIntegration/meta/template.html | 2 +- .../browserTracingIntegration/meta/test.ts | 1 + .../suites/tracing/dsc-txn-name-update/test.ts | 8 ++++++++ .../envelope-header-transaction-name/test.ts | 1 + .../suites/tracing/envelope-header/test.ts | 1 + .../web-vitals-cls-standalone-spans/test.ts | 4 ++++ .../tracing/metrics/web-vitals-inp-late/test.ts | 1 + .../web-vitals-inp-parametrized-late/test.ts | 1 + .../metrics/web-vitals-inp-parametrized/test.ts | 1 + .../tracing/metrics/web-vitals-inp/test.ts | 1 + .../tracing/trace-lifetime/navigation/test.ts | 15 ++++++++++++--- .../trace-lifetime/pageload-meta/template.html | 2 +- .../tracing/trace-lifetime/pageload-meta/test.ts | 11 ++++++++++- .../tracing/trace-lifetime/pageload/test.ts | 16 ++++++++++++---- .../tracing/trace-lifetime/startNewTrace/test.ts | 3 +++ .../tracing-without-performance/template.html | 2 +- .../tracing-without-performance/test.ts | 4 +++- 20 files changed, 71 insertions(+), 12 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/test.ts index af87e11df37e..0663d16b6995 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/test.ts @@ -47,6 +47,7 @@ sentryTest( sampled: 'true', trace_id: traceId, transaction: 'outer', + sample_rand: expect.any(String), }, }); @@ -64,6 +65,7 @@ sentryTest( sampled: 'true', trace_id: traceId, transaction: 'outer', + sample_rand: expect.any(String), }, }); diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/test.ts index a37a50f28e04..ef1019d02b1d 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/test.ts @@ -33,6 +33,7 @@ sentryTest('sends a segment span envelope', async ({ getLocalTestUrl, page }) => sampled: 'true', trace_id: traceId, transaction: 'standalone_segment_span', + sample_rand: expect.any(String), }, }); diff --git a/dev-packages/browser-integration-tests/suites/replay/dsc/test.ts b/dev-packages/browser-integration-tests/suites/replay/dsc/test.ts index 384ee0071f64..a0deed767979 100644 --- a/dev-packages/browser-integration-tests/suites/replay/dsc/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/dsc/test.ts @@ -62,6 +62,7 @@ sentryTest( public_key: 'public', replay_id: replay.session?.id, sampled: 'true', + sample_rand: expect.any(String), }); }, ); @@ -108,6 +109,7 @@ sentryTest( trace_id: expect.stringMatching(/[a-f0-9]{32}/), public_key: 'public', sampled: 'true', + sample_rand: expect.any(String), }); }, ); @@ -161,6 +163,7 @@ sentryTest( public_key: 'public', replay_id: replay.session?.id, sampled: 'true', + sample_rand: expect.any(String), }); }, ); @@ -202,6 +205,7 @@ sentryTest( trace_id: expect.stringMatching(/[a-f0-9]{32}/), public_key: 'public', sampled: 'true', + sample_rand: expect.any(String), }); }, ); @@ -247,6 +251,7 @@ sentryTest('should add replay_id to error DSC while replay is active', async ({ ? { sample_rate: '1', sampled: 'true', + sample_rand: expect.any(String), } : {}), }); @@ -267,6 +272,7 @@ sentryTest('should add replay_id to error DSC while replay is active', async ({ ? { sample_rate: '1', sampled: 'true', + sample_rand: expect.any(String), } : {}), }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/meta/template.html b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/meta/template.html index 09984cb0c488..7f7b0b159fee 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/meta/template.html +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/meta/template.html @@ -5,7 +5,7 @@ diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/meta/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/meta/test.ts index 39cad4b8f7d0..1d6e7044b007 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/meta/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/meta/test.ts @@ -43,6 +43,7 @@ sentryTest( sample_rate: '0.3232', trace_id: '123', public_key: 'public', + sample_rand: '0.42', }); }, ); diff --git a/dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/test.ts b/dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/test.ts index bb644d9a7de7..84e4210b68ac 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/test.ts @@ -48,6 +48,7 @@ sentryTest('updates the DSC when the txn name is updated and high-quality', asyn 'sentry-environment=production', 'sentry-public_key=public', 'sentry-release=1.1.1', + expect.stringMatching(/sentry-sample_rand=0\.[0-9]+/), 'sentry-sample_rate=1', 'sentry-sampled=true', `sentry-trace_id=${traceId}`, @@ -62,6 +63,7 @@ sentryTest('updates the DSC when the txn name is updated and high-quality', asyn sample_rate: '1', sampled: 'true', trace_id: traceId, + sample_rand: expect.any(String), }); // 4 @@ -73,6 +75,7 @@ sentryTest('updates the DSC when the txn name is updated and high-quality', asyn 'sentry-environment=production', 'sentry-public_key=public', 'sentry-release=1.1.1', + expect.stringMatching(/sentry-sample_rand=0\.[0-9]+/), 'sentry-sample_rate=1', 'sentry-sampled=true', `sentry-trace_id=${traceId}`, @@ -89,6 +92,7 @@ sentryTest('updates the DSC when the txn name is updated and high-quality', asyn sampled: 'true', trace_id: traceId, transaction: 'updated-root-span-1', + sample_rand: expect.any(String), }); // 7 @@ -100,6 +104,7 @@ sentryTest('updates the DSC when the txn name is updated and high-quality', asyn 'sentry-environment=production', 'sentry-public_key=public', 'sentry-release=1.1.1', + expect.stringMatching(/sentry-sample_rand=0\.[0-9]+/), 'sentry-sample_rate=1', 'sentry-sampled=true', `sentry-trace_id=${traceId}`, @@ -116,6 +121,7 @@ sentryTest('updates the DSC when the txn name is updated and high-quality', asyn sampled: 'true', trace_id: traceId, transaction: 'updated-root-span-2', + sample_rand: expect.any(String), }); // 10 @@ -137,6 +143,7 @@ sentryTest('updates the DSC when the txn name is updated and high-quality', asyn sampled: 'true', trace_id: traceId, transaction: 'updated-root-span-2', + sample_rand: expect.any(String), }); expect(txnEvent.transaction).toEqual('updated-root-span-2'); @@ -157,6 +164,7 @@ sentryTest('updates the DSC when the txn name is updated and high-quality', asyn public_key: 'public', release: '1.1.1', trace_id: traceId, + sample_rand: expect.any(String), }); }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/envelope-header-transaction-name/test.ts b/dev-packages/browser-integration-tests/suites/tracing/envelope-header-transaction-name/test.ts index db658c53d973..ff1c42f7c851 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/envelope-header-transaction-name/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/envelope-header-transaction-name/test.ts @@ -27,6 +27,7 @@ sentryTest( trace_id: expect.stringMatching(/[a-f0-9]{32}/), public_key: 'public', sampled: 'true', + sample_rand: expect.any(String), }); }, ); diff --git a/dev-packages/browser-integration-tests/suites/tracing/envelope-header/test.ts b/dev-packages/browser-integration-tests/suites/tracing/envelope-header/test.ts index 0f2c08ff1781..8f245c89ad38 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/envelope-header/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/envelope-header/test.ts @@ -30,6 +30,7 @@ sentryTest( trace_id: expect.stringMatching(/[a-f0-9]{32}/), public_key: 'public', sampled: 'true', + sample_rand: expect.any(String), }); }, ); diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts index d227c0aaf575..02431dae3b79 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts @@ -100,6 +100,7 @@ sentryTest('captures a "GOOD" CLS vital with its source as a standalone span', a sample_rate: '1', sampled: 'true', trace_id: spanEnvelopeItem.trace_id, + sample_rand: expect.any(String), // no transaction, because span source is URL }, }); @@ -167,6 +168,7 @@ sentryTest('captures a "MEH" CLS vital with its source as a standalone span', as sample_rate: '1', sampled: 'true', trace_id: spanEnvelopeItem.trace_id, + sample_rand: expect.any(String), // no transaction, because span source is URL }, }); @@ -232,6 +234,7 @@ sentryTest('captures a "POOR" CLS vital with its source as a standalone span.', sample_rate: '1', sampled: 'true', trace_id: spanEnvelopeItem.trace_id, + sample_rand: expect.any(String), // no transaction, because span source is URL }, }); @@ -294,6 +297,7 @@ sentryTest( sample_rate: '1', sampled: 'true', trace_id: spanEnvelopeItem.trace_id, + sample_rand: expect.any(String), // no transaction, because span source is URL }, }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/test.ts index 61962bd40593..fffa85b89ae2 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/test.ts @@ -55,6 +55,7 @@ sentryTest('should capture an INP click event span after pageload', async ({ bro sample_rate: '1', sampled: 'true', trace_id: traceId, + sample_rand: expect.any(String), }, }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/test.ts index 788c168067bd..65852c734c98 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/test.ts @@ -58,6 +58,7 @@ sentryTest( sampled: 'true', trace_id: traceId, transaction: 'test-route', + sample_rand: expect.any(String), }, }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/test.ts index 5da11a8caae3..5705fe6863b5 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/test.ts @@ -56,6 +56,7 @@ sentryTest( sampled: 'true', trace_id: traceId, transaction: 'test-route', + sample_rand: expect.any(String), }, }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp/test.ts index 0a159db4dc31..b3435a49b002 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp/test.ts @@ -54,6 +54,7 @@ sentryTest('should capture an INP click event span during pageload', async ({ br sample_rate: '1', sampled: 'true', trace_id: traceId, + sample_rand: expect.any(String), // no transaction, because span source is URL }, }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts index cd1e79c211aa..a123099107d2 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts @@ -10,7 +10,7 @@ import { shouldSkipTracingTest, } from '../../../../utils/helpers'; -sentryTest('creates a new trace on each navigation', async ({ getLocalTestUrl, page }) => { +sentryTest('creates a new trace and sample_rand on each navigation', async ({ getLocalTestUrl, page }) => { if (shouldSkipTracingTest()) { sentryTest.skip(); } @@ -49,6 +49,7 @@ sentryTest('creates a new trace on each navigation', async ({ getLocalTestUrl, p sample_rate: '1', sampled: 'true', trace_id: navigation1TraceContext?.trace_id, + sample_rand: expect.any(String), }); expect(navigation2TraceContext).toMatchObject({ @@ -64,9 +65,11 @@ sentryTest('creates a new trace on each navigation', async ({ getLocalTestUrl, p sample_rate: '1', sampled: 'true', trace_id: navigation2TraceContext?.trace_id, + sample_rand: expect.any(String), }); expect(navigation1TraceContext?.trace_id).not.toEqual(navigation2TraceContext?.trace_id); + expect(navigation1TraceHeader?.sample_rand).not.toEqual(navigation2TraceHeader?.sample_rand); }); sentryTest('error after navigation has navigation traceId', async ({ getLocalTestUrl, page }) => { @@ -101,6 +104,7 @@ sentryTest('error after navigation has navigation traceId', async ({ getLocalTes sample_rate: '1', sampled: 'true', trace_id: navigationTraceContext?.trace_id, + sample_rand: expect.any(String), }); const errorEventPromise = getFirstSentryEnvelopeRequest( @@ -124,6 +128,7 @@ sentryTest('error after navigation has navigation traceId', async ({ getLocalTes sample_rate: '1', sampled: 'true', trace_id: navigationTraceContext?.trace_id, + sample_rand: expect.any(String), }); }); @@ -168,6 +173,7 @@ sentryTest('error during navigation has new navigation traceId', async ({ getLoc sample_rate: '1', sampled: 'true', trace_id: navigationTraceContext?.trace_id, + sample_rand: expect.any(String), }); const errorTraceContext = errorEvent?.contexts?.trace; @@ -182,6 +188,7 @@ sentryTest('error during navigation has new navigation traceId', async ({ getLoc sample_rate: '1', sampled: 'true', trace_id: navigationTraceContext?.trace_id, + sample_rand: expect.any(String), }); }); @@ -234,6 +241,7 @@ sentryTest( sample_rate: '1', sampled: 'true', trace_id: navigationTraceContext?.trace_id, + sample_rand: expect.any(String), }); const headers = request.headers(); @@ -242,7 +250,7 @@ sentryTest( const navigationTraceId = navigationTraceContext?.trace_id; expect(headers['sentry-trace']).toMatch(new RegExp(`^${navigationTraceId}-[0-9a-f]{16}-1$`)); expect(headers['baggage']).toEqual( - `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId},sentry-sample_rate=1,sentry-sampled=true`, + `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId},sentry-sample_rate=1,sentry-sampled=true,sentry-sample_rand=${navigationTraceHeader?.sample_rand}`, ); }, ); @@ -296,6 +304,7 @@ sentryTest( sample_rate: '1', sampled: 'true', trace_id: navigationTraceContext?.trace_id, + sample_rand: expect.any(String), }); const headers = request.headers(); @@ -304,7 +313,7 @@ sentryTest( const navigationTraceId = navigationTraceContext?.trace_id; expect(headers['sentry-trace']).toMatch(new RegExp(`^${navigationTraceId}-[0-9a-f]{16}-1$`)); expect(headers['baggage']).toEqual( - `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId},sentry-sample_rate=1,sentry-sampled=true`, + `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId},sentry-sample_rate=1,sentry-sampled=true,sentry-sample_rand=${navigationTraceHeader?.sample_rand}`, ); }, ); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/template.html b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/template.html index 0dee204aef16..64b3a29fac28 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/template.html +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/template.html @@ -4,7 +4,7 @@ + content="sentry-trace_id=12345678901234567890123456789012,sentry-sample_rate=0.2,sentry-sampled=true,sentry-transaction=my-transaction,sentry-public_key=public,sentry-release=1.0.0,sentry-environment=prod,sentry-sample_rand=0.42"/> diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts index d102ad4c128e..d087dd0b32af 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts @@ -13,7 +13,7 @@ import { const META_TAG_TRACE_ID = '12345678901234567890123456789012'; const META_TAG_PARENT_SPAN_ID = '1234567890123456'; const META_TAG_BAGGAGE = - 'sentry-trace_id=12345678901234567890123456789012,sentry-sample_rate=0.2,sentry-sampled=true,sentry-transaction=my-transaction,sentry-public_key=public,sentry-release=1.0.0,sentry-environment=prod'; + 'sentry-trace_id=12345678901234567890123456789012,sentry-sample_rate=0.2,sentry-sampled=true,sentry-transaction=my-transaction,sentry-public_key=public,sentry-release=1.0.0,sentry-environment=prod,sentry-sample_rand=0.42'; sentryTest( 'create a new trace for a navigation after the tag pageload trace', @@ -54,6 +54,7 @@ sentryTest( transaction: 'my-transaction', public_key: 'public', trace_id: META_TAG_TRACE_ID, + sample_rand: '0.42', }); expect(navigationEvent.type).toEqual('transaction'); @@ -71,9 +72,11 @@ sentryTest( sample_rate: '1', sampled: 'true', trace_id: navigationTraceContext?.trace_id, + sample_rand: expect.any(String), }); expect(pageloadTraceContext?.trace_id).not.toEqual(navigationTraceContext?.trace_id); + expect(pageloadTraceHeader?.sample_rand).not.toEqual(navigationTraceHeader?.sample_rand); }, ); @@ -105,6 +108,7 @@ sentryTest('error after tag pageload has pageload traceId', async ({ getL transaction: 'my-transaction', public_key: 'public', trace_id: META_TAG_TRACE_ID, + sample_rand: '0.42', }); const errorEventPromise = getFirstSentryEnvelopeRequest( @@ -130,6 +134,7 @@ sentryTest('error after tag pageload has pageload traceId', async ({ getL transaction: 'my-transaction', public_key: 'public', trace_id: META_TAG_TRACE_ID, + sample_rand: '0.42', }); }); @@ -171,6 +176,7 @@ sentryTest('error during tag pageload has pageload traceId', async ({ get transaction: 'my-transaction', public_key: 'public', trace_id: META_TAG_TRACE_ID, + sample_rand: '0.42', }); expect(errorEvent.type).toEqual(undefined); @@ -188,6 +194,7 @@ sentryTest('error during tag pageload has pageload traceId', async ({ get transaction: 'my-transaction', public_key: 'public', trace_id: META_TAG_TRACE_ID, + sample_rand: '0.42', }); }); @@ -234,6 +241,7 @@ sentryTest( transaction: 'my-transaction', public_key: 'public', trace_id: META_TAG_TRACE_ID, + sample_rand: '0.42', }); const headers = request.headers(); @@ -287,6 +295,7 @@ sentryTest( transaction: 'my-transaction', public_key: 'public', trace_id: META_TAG_TRACE_ID, + sample_rand: '0.42', }); const headers = request.headers(); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts index 00bbb26740de..a4439840da7d 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts @@ -49,6 +49,7 @@ sentryTest( sample_rate: '1', sampled: 'true', trace_id: pageloadTraceContext?.trace_id, + sample_rand: expect.any(String), }); expect(navigationTraceContext).toMatchObject({ @@ -64,6 +65,7 @@ sentryTest( sample_rate: '1', sampled: 'true', trace_id: navigationTraceContext?.trace_id, + sample_rand: expect.any(String), }); expect(pageloadTraceContext?.span_id).not.toEqual(navigationTraceContext?.span_id); @@ -98,6 +100,7 @@ sentryTest('error after pageload has pageload traceId', async ({ getLocalTestUrl sample_rate: '1', sampled: 'true', trace_id: pageloadTraceContext?.trace_id, + sample_rand: expect.any(String), }); const errorEventPromise = getFirstSentryEnvelopeRequest( @@ -122,6 +125,7 @@ sentryTest('error after pageload has pageload traceId', async ({ getLocalTestUrl sample_rate: '1', sampled: 'true', trace_id: pageloadTraceContext?.trace_id, + sample_rand: expect.any(String), }); }); @@ -163,6 +167,7 @@ sentryTest('error during pageload has pageload traceId', async ({ getLocalTestUr sample_rate: '1', sampled: 'true', trace_id: pageloadTraceContext?.trace_id, + sample_rand: expect.any(String), }); const errorTraceContext = errorEvent?.contexts?.trace; @@ -179,6 +184,7 @@ sentryTest('error during pageload has pageload traceId', async ({ getLocalTestUr sample_rate: '1', sampled: 'true', trace_id: pageloadTraceContext?.trace_id, + sample_rand: expect.any(String), }); }); @@ -226,14 +232,15 @@ sentryTest( sample_rate: '1', sampled: 'true', trace_id: pageloadTraceId, + sample_rand: expect.any(String), }); const headers = request.headers(); // sampling decision is propagated from active span sampling decision expect(headers['sentry-trace']).toMatch(new RegExp(`^${pageloadTraceId}-[0-9a-f]{16}-1$`)); - expect(headers['baggage']).toEqual( - `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId},sentry-sample_rate=1,sentry-sampled=true`, + expect(headers['baggage']).toBe( + `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId},sentry-sample_rate=1,sentry-sampled=true,sentry-sample_rand=${pageloadTraceHeader?.sample_rand}`, ); }, ); @@ -282,14 +289,15 @@ sentryTest( sample_rate: '1', sampled: 'true', trace_id: pageloadTraceId, + sample_rand: expect.any(String), }); const headers = request.headers(); // sampling decision is propagated from active span sampling decision expect(headers['sentry-trace']).toMatch(new RegExp(`^${pageloadTraceId}-[0-9a-f]{16}-1$`)); - expect(headers['baggage']).toEqual( - `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId},sentry-sample_rate=1,sentry-sampled=true`, + expect(headers['baggage']).toBe( + `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId},sentry-sample_rate=1,sentry-sampled=true,sentry-sample_rand=${pageloadTraceHeader?.sample_rand}`, ); }, ); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/startNewTrace/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/startNewTrace/test.ts index 3ddca4787aee..a785327a0031 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/startNewTrace/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/startNewTrace/test.ts @@ -48,6 +48,7 @@ sentryTest( sample_rate: '1', sampled: 'true', trace_id: pageloadTraceContext?.trace_id, + sample_rand: expect.any(String), }); const transactionPromises = getMultipleSentryEnvelopeRequests( @@ -81,6 +82,7 @@ sentryTest( sampled: 'true', trace_id: newTraceTransactionTraceContext?.trace_id, transaction: 'new-trace', + sample_rand: expect.any(String), }); const oldTraceTransactionEventTraceContext = oldTraceTransactionEvent.contexts?.trace; @@ -96,6 +98,7 @@ sentryTest( sample_rate: '1', sampled: 'true', trace_id: oldTraceTransactionTraceHeaders?.trace_id, + sample_rand: expect.any(String), // transaction: 'old-trace', <-- this is not in the DSC because the DSC is continued from the pageload transaction // which does not have a `transaction` field because its source is URL. }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/tracing-without-performance/template.html b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/tracing-without-performance/template.html index 7cf101e4cf9e..d32f02cb6413 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/tracing-without-performance/template.html +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/tracing-without-performance/template.html @@ -5,7 +5,7 @@ + content="sentry-trace_id=12345678901234567890123456789012,sentry-public_key=public,sentry-release=1.0.0,sentry-environment=prod,sentry-sample_rand=0.42"/> diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/tracing-without-performance/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/tracing-without-performance/test.ts index 8fceec718447..bea3c10cbde5 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/tracing-without-performance/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/tracing-without-performance/test.ts @@ -10,7 +10,7 @@ import { const META_TAG_TRACE_ID = '12345678901234567890123456789012'; const META_TAG_PARENT_SPAN_ID = '1234567890123456'; const META_TAG_BAGGAGE = - 'sentry-trace_id=12345678901234567890123456789012,sentry-public_key=public,sentry-release=1.0.0,sentry-environment=prod'; + 'sentry-trace_id=12345678901234567890123456789012,sentry-public_key=public,sentry-release=1.0.0,sentry-environment=prod,sentry-sample_rand=0.42'; sentryTest('error on initial page has traceId from meta tag', async ({ getLocalTestUrl, page }) => { if (shouldSkipTracingTest()) { @@ -41,6 +41,7 @@ sentryTest('error on initial page has traceId from meta tag', async ({ getLocalT public_key: 'public', release: '1.0.0', trace_id: META_TAG_TRACE_ID, + sample_rand: '0.42', }); }); @@ -72,6 +73,7 @@ sentryTest('error has new traceId after navigation', async ({ getLocalTestUrl, p public_key: 'public', release: '1.0.0', trace_id: META_TAG_TRACE_ID, + sample_rand: expect.any(String), }); const errorEventPromise2 = getFirstSentryEnvelopeRequest( From e1c531f23652b0ebaa4252d4af3c77989dcbfe2d Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 14 Jan 2025 20:08:32 +0000 Subject: [PATCH 13/45] Fix test --- .../suites/tracing/dsc-txn-name-update/test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/test.ts b/dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/test.ts index 84e4210b68ac..829d75924ac8 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/test.ts @@ -164,7 +164,6 @@ sentryTest('updates the DSC when the txn name is updated and high-quality', asyn public_key: 'public', release: '1.1.1', trace_id: traceId, - sample_rand: expect.any(String), }); }); From a1d5dd3f67e80122a08645c331e42cc6ff02407f Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 15 Jan 2025 09:37:32 +0000 Subject: [PATCH 14/45] test fix --- .../startSpan/parallel-root-spans-with-parentSpanId/subject.js | 1 + .../suites/public-api/startSpan/parallel-root-spans/scenario.ts | 1 + .../parallel-spans-in-scope-with-parentSpanId/scenario.ts | 1 + 3 files changed, 3 insertions(+) diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/parallel-root-spans-with-parentSpanId/subject.js b/dev-packages/browser-integration-tests/suites/public-api/startSpan/parallel-root-spans-with-parentSpanId/subject.js index 85a9847e1c3f..8509c200c15d 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/parallel-root-spans-with-parentSpanId/subject.js +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/parallel-root-spans-with-parentSpanId/subject.js @@ -1,6 +1,7 @@ Sentry.getCurrentScope().setPropagationContext({ parentSpanId: '1234567890123456', traceId: '12345678901234567890123456789012', + sampleRand: Math.random(), }); Sentry.startSpan({ name: 'test_span_1' }, () => undefined); diff --git a/dev-packages/node-integration-tests/suites/public-api/startSpan/parallel-root-spans/scenario.ts b/dev-packages/node-integration-tests/suites/public-api/startSpan/parallel-root-spans/scenario.ts index 9275f9fe4505..fada0ea3aad4 100644 --- a/dev-packages/node-integration-tests/suites/public-api/startSpan/parallel-root-spans/scenario.ts +++ b/dev-packages/node-integration-tests/suites/public-api/startSpan/parallel-root-spans/scenario.ts @@ -11,6 +11,7 @@ Sentry.init({ Sentry.getCurrentScope().setPropagationContext({ parentSpanId: '1234567890123456', traceId: '12345678901234567890123456789012', + sampleRand: Math.random(), }); const spanIdTraceId = Sentry.startSpan( diff --git a/dev-packages/node-integration-tests/suites/public-api/startSpan/parallel-spans-in-scope-with-parentSpanId/scenario.ts b/dev-packages/node-integration-tests/suites/public-api/startSpan/parallel-spans-in-scope-with-parentSpanId/scenario.ts index cbd2dd023f37..ca0431f2318f 100644 --- a/dev-packages/node-integration-tests/suites/public-api/startSpan/parallel-spans-in-scope-with-parentSpanId/scenario.ts +++ b/dev-packages/node-integration-tests/suites/public-api/startSpan/parallel-spans-in-scope-with-parentSpanId/scenario.ts @@ -12,6 +12,7 @@ Sentry.withScope(scope => { scope.setPropagationContext({ parentSpanId: '1234567890123456', traceId: '12345678901234567890123456789012', + sampleRand: Math.random(), }); const spanIdTraceId = Sentry.startSpan( From 83ac88569c76bc8270d2c8ad63f22d41b2e2d84c Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 15 Jan 2025 10:01:25 +0000 Subject: [PATCH 15/45] feat: Pass `parentSampleRate` to `tracesSampler` --- packages/core/src/tracing/trace.ts | 7 ++++--- packages/opentelemetry/src/sampler.ts | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index a7841ae631d4..6cb159ee96f3 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -22,6 +22,7 @@ import { generateTraceId } from '../utils-hoist/propagationContext'; import { propagationContextFromHeaders } from '../utils-hoist/tracing'; import { handleCallbackErrors } from '../utils/handleCallbackErrors'; import { hasTracingEnabled } from '../utils/hasTracingEnabled'; +import { parseSampleRate } from '../utils/parseSampleRate'; import { _getSpanForScope, _setSpanForScope } from '../utils/spanOnScope'; import { addChildSpanToSpan, getRootSpan, spanIsSampled, spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils'; import { freezeDscOnSpan, getDynamicSamplingContextFromSpan } from './dynamicSamplingContext'; @@ -405,7 +406,7 @@ function _startRootSpan(spanArguments: SentrySpanArguments, scope: Scope, parent const options: Partial = client?.getOptions() || {}; const { name = '', attributes } = spanArguments; - const sampleRand = scope.getPropagationContext().sampleRand; + const currentPropagationContext = scope.getPropagationContext(); const [sampled, sampleRate] = scope.getScopeData().sdkProcessingMetadata[SUPPRESS_TRACING_KEY] ? [false] : sampleSpan( @@ -414,9 +415,9 @@ function _startRootSpan(spanArguments: SentrySpanArguments, scope: Scope, parent name, parentSampled, attributes, - // TODO(v9): provide a parentSampleRate here + parentSampleRate: parseSampleRate(currentPropagationContext.dsc?.sample_rate), }, - sampleRand, + currentPropagationContext.sampleRand, ); const rootSpan = new SentrySpan({ diff --git a/packages/opentelemetry/src/sampler.ts b/packages/opentelemetry/src/sampler.ts index b71dd7422d7e..bbf4ff17eb72 100644 --- a/packages/opentelemetry/src/sampler.ts +++ b/packages/opentelemetry/src/sampler.ts @@ -16,6 +16,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, hasTracingEnabled, logger, + parseSampleRate, sampleSpan, } from '@sentry/core'; import { SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING, SENTRY_TRACE_STATE_URL } from './constants'; @@ -104,7 +105,7 @@ export class SentrySampler implements Sampler { // We only sample based on parameters (like tracesSampleRate or tracesSampler) for root spans (which is done in sampleSpan). // Non-root-spans simply inherit the sampling decision from their parent. if (isRootSpan) { - const sampleRand = scope?.getPropagationContext().sampleRand ?? Math.random(); + const currentPropagationContext = scope?.getPropagationContext(); const [sampled, sampleRate] = sampleSpan( options, { @@ -112,9 +113,9 @@ export class SentrySampler implements Sampler { attributes: mergedAttributes, normalizedRequest: isolationScope?.getScopeData().sdkProcessingMetadata.normalizedRequest, parentSampled, - // TODO(v9): provide a parentSampleRate here + parentSampleRate: parseSampleRate(currentPropagationContext?.dsc?.sample_rate), }, - sampleRand, + currentPropagationContext?.sampleRand ?? Math.random(), ); const attributes: Attributes = { From 045b41f2eb3b4fb70892b9ff4a961f188a321d3a Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 15 Jan 2025 10:10:06 +0000 Subject: [PATCH 16/45] always apply root span sample rate to dsc --- .../src/tracing/dynamicSamplingContext.ts | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index ba362c49795a..39d781c47cfb 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -75,11 +75,21 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly): Partial { + if (maybeSampleRate != null) { + dsc.sample_rate = `${maybeSampleRate}`; + } + return dsc; + } // For core implementation, we freeze the DSC onto the span as a non-enumerable property const frozenDsc = (rootSpan as SpanWithMaybeDsc)[FROZEN_DSC_FIELD]; if (frozenDsc) { - return frozenDsc; + return applyRootSpanSampleRateToDsc(frozenDsc); } // For OpenTelemetry, we freeze the DSC on the trace state @@ -90,24 +100,17 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly Date: Wed, 15 Jan 2025 10:12:02 +0000 Subject: [PATCH 17/45] e l a b o r a t e --- packages/core/src/tracing/dynamicSamplingContext.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index 39d781c47cfb..fa65cce218cc 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -79,6 +79,8 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly): Partial { if (maybeSampleRate != null) { dsc.sample_rate = `${maybeSampleRate}`; From 631ed1b89d5260805263d8e26028d0e64572c514 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 15 Jan 2025 10:14:27 +0000 Subject: [PATCH 18/45] Dont reinvent util we already have --- packages/core/src/utils-hoist/tracing.ts | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/packages/core/src/utils-hoist/tracing.ts b/packages/core/src/utils-hoist/tracing.ts index 98175f427762..3fa4d9677c02 100644 --- a/packages/core/src/utils-hoist/tracing.ts +++ b/packages/core/src/utils-hoist/tracing.ts @@ -1,4 +1,5 @@ import type { DynamicSamplingContext, PropagationContext, TraceparentData } from '../types-hoist'; +import { parseSampleRate } from '../utils/parseSampleRate'; import { baggageHeaderToDynamicSamplingContext } from './baggage'; import { generateSpanId, generateTraceId } from './propagationContext'; @@ -102,13 +103,13 @@ function getSampleRandFromTraceparentAndDsc( dsc: Partial | undefined, ): number { // When there is an incoming sample rand use it. - const parsedSampleRand = parseSamplingDscNumber(dsc?.sample_rand); + const parsedSampleRand = parseSampleRate(dsc?.sample_rand); if (parsedSampleRand !== undefined) { return parsedSampleRand; } // Otherwise, if there is an incoming sampling decision + sample rate, generate a sample rand that would lead to the same sampling decision. - const parsedSampleRate = parseSamplingDscNumber(dsc?.sample_rate); + const parsedSampleRate = parseSampleRate(dsc?.sample_rate); if (parsedSampleRate && traceparentData?.parentSampled !== undefined) { return traceparentData.parentSampled ? // Returns a sample rand with positive sampling decision [0, sampleRate) @@ -120,20 +121,3 @@ function getSampleRandFromTraceparentAndDsc( return Math.random(); } } - -/** - * Given a string form of a numeric-like on the DSC, safely convert it to a number. - */ -function parseSamplingDscNumber(sampleRand: string | undefined): number | undefined { - try { - const parsed = Number(sampleRand); // Number(undefined) will return NaN and fail the next check - if (isNaN(parsed) || parsed < 0 || parsed > 1) { - // This is probably an invariant but returning undefined seems sensible. - return undefined; - } else { - return parsed; - } - } catch { - return undefined; - } -} From 37b6ca2beb9e6e4b7665acd8411a69aa59a3d503 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 15 Jan 2025 11:36:30 +0000 Subject: [PATCH 19/45] beep boop actually do what I am supposed to beep boop --- packages/core/src/semanticAttributes.ts | 5 +++++ .../core/src/tracing/dynamicSamplingContext.ts | 15 +++++++++++++-- packages/core/src/tracing/sampling.ts | 13 +++++++++---- packages/core/src/tracing/trace.ts | 11 +++++++++-- packages/opentelemetry/src/sampler.ts | 4 +++- 5 files changed, 39 insertions(+), 9 deletions(-) diff --git a/packages/core/src/semanticAttributes.ts b/packages/core/src/semanticAttributes.ts index a0b1921ec8f3..aca14792f4de 100644 --- a/packages/core/src/semanticAttributes.ts +++ b/packages/core/src/semanticAttributes.ts @@ -12,6 +12,11 @@ export const SEMANTIC_ATTRIBUTE_SENTRY_SOURCE = 'sentry.source'; */ export const SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE = 'sentry.sample_rate'; +/** + * Use this attribute on a root span to propagate the spans sample rate downstream as parent sample rate in the DSC, overriding anything that was previously set on the DSC. + */ +export const SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE = 'sentry.override_trace_sample_rate'; + /** * Use this attribute to represent the operation of a span. */ diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index fa65cce218cc..519481a0fb36 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -2,7 +2,11 @@ import type { Client } from '../client'; import { DEFAULT_ENVIRONMENT } from '../constants'; import { getClient } from '../currentScopes'; import type { Scope } from '../scope'; -import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, + SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, +} from '../semanticAttributes'; import type { DynamicSamplingContext, Span } from '../types-hoist'; import { baggageHeaderToDynamicSamplingContext, @@ -77,12 +81,14 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly): Partial { - if (maybeSampleRate != null) { + // Note: This is a `!= null` check meaning "nullish" + if (shouldOverrideDscSampleRate && maybeSampleRate != null) { dsc.sample_rate = `${maybeSampleRate}`; } return dsc; @@ -108,6 +114,11 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly, samplingContext: SamplingContext, sampleRand: number, -): [sampled: boolean, sampleRate?: number] { +): [sampled: boolean, sampleRate?: number, shouldUpdateSampleRateOnDsc?: boolean] { // nothing to do if tracing is not enabled if (!hasTracingEnabled(options)) { return [false]; } + let shouldUpdateSampleRateOnDsc = false; + // we would have bailed already if neither `tracesSampler` nor `tracesSampleRate` nor `enableTracing` were defined, so one of these should // work; prefer the hook if so let sampleRate; if (typeof options.tracesSampler === 'function') { sampleRate = options.tracesSampler(samplingContext); + shouldUpdateSampleRateOnDsc = true; } else if (samplingContext.parentSampled !== undefined) { sampleRate = samplingContext.parentSampled; } else if (typeof options.tracesSampleRate !== 'undefined') { sampleRate = options.tracesSampleRate; + shouldUpdateSampleRateOnDsc = true; } else { // When `enableTracing === true`, we use a sample rate of 100% sampleRate = 1; + shouldUpdateSampleRateOnDsc = true; } // Since this is coming from the user (or from a function provided by the user), who knows what we might get. @@ -54,7 +59,7 @@ export function sampleSpan( : 'a negative sampling decision was inherited or tracesSampleRate is set to 0' }`, ); - return [false, parsedSampleRate]; + return [false, parsedSampleRate, shouldUpdateSampleRateOnDsc]; } // Now we roll the dice. Math.random is inclusive of 0, but not of 1, so strict < is safe here. In case sampleRate is @@ -69,8 +74,8 @@ export function sampleSpan( sampleRate, )})`, ); - return [false, parsedSampleRate]; + return [false, parsedSampleRate, shouldUpdateSampleRateOnDsc]; } - return [true, parsedSampleRate]; + return [true, parsedSampleRate, shouldUpdateSampleRateOnDsc]; } diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index 6cb159ee96f3..7f06e4d0e759 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -16,7 +16,11 @@ import { getClient, getCurrentScope, getIsolationScope, withScope } from '../cur import { getAsyncContextStrategy } from '../asyncContext'; import { DEBUG_BUILD } from '../debug-build'; import type { Scope } from '../scope'; -import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, + SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, +} from '../semanticAttributes'; import { logger } from '../utils-hoist/logger'; import { generateTraceId } from '../utils-hoist/propagationContext'; import { propagationContextFromHeaders } from '../utils-hoist/tracing'; @@ -407,7 +411,9 @@ function _startRootSpan(spanArguments: SentrySpanArguments, scope: Scope, parent const { name = '', attributes } = spanArguments; const currentPropagationContext = scope.getPropagationContext(); - const [sampled, sampleRate] = scope.getScopeData().sdkProcessingMetadata[SUPPRESS_TRACING_KEY] + const [sampled, sampleRate, shouldUpdateSampleRateOnDsc] = scope.getScopeData().sdkProcessingMetadata[ + SUPPRESS_TRACING_KEY + ] ? [false] : sampleSpan( options, @@ -436,6 +442,7 @@ function _startRootSpan(spanArguments: SentrySpanArguments, scope: Scope, parent if (sampleRate !== undefined) { rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, sampleRate); + rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, shouldUpdateSampleRateOnDsc); } if (client) { diff --git a/packages/opentelemetry/src/sampler.ts b/packages/opentelemetry/src/sampler.ts index bbf4ff17eb72..6797c107517f 100644 --- a/packages/opentelemetry/src/sampler.ts +++ b/packages/opentelemetry/src/sampler.ts @@ -13,6 +13,7 @@ import { import type { Client, SpanAttributes } from '@sentry/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, hasTracingEnabled, logger, @@ -106,7 +107,7 @@ export class SentrySampler implements Sampler { // Non-root-spans simply inherit the sampling decision from their parent. if (isRootSpan) { const currentPropagationContext = scope?.getPropagationContext(); - const [sampled, sampleRate] = sampleSpan( + const [sampled, sampleRate, shouldUpdateSampleRateOnDsc] = sampleSpan( options, { name: inferredSpanName, @@ -120,6 +121,7 @@ export class SentrySampler implements Sampler { const attributes: Attributes = { [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: sampleRate, + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: shouldUpdateSampleRateOnDsc, }; const method = `${maybeSpanHttpMethod}`.toUpperCase(); From 5e3f7066575c96532525033f76d95fd7819abe19 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 15 Jan 2025 11:49:39 +0000 Subject: [PATCH 20/45] don't leak attribute into actual data --- packages/core/src/tracing/sentrySpan.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/core/src/tracing/sentrySpan.ts b/packages/core/src/tracing/sentrySpan.ts index acc14d37bc31..13bc2a0811fe 100644 --- a/packages/core/src/tracing/sentrySpan.ts +++ b/packages/core/src/tracing/sentrySpan.ts @@ -7,6 +7,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, } from '../semanticAttributes'; import type { @@ -348,6 +349,7 @@ export class SentrySpan implements Span { // remove internal root span attributes we don't need to send. /* eslint-disable @typescript-eslint/no-dynamic-delete */ delete this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME]; + delete this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]; spans.forEach(span => { span.data && delete span.data[SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME]; }); From c4282b8fff86db661a4d3637126aa47a959e64ff Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 15 Jan 2025 12:06:22 +0000 Subject: [PATCH 21/45] . --- packages/opentelemetry/src/spanExporter.ts | 2 ++ packages/opentelemetry/test/sampler.test.ts | 4 ++-- packages/opentelemetry/test/trace.test.ts | 8 ++++++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts index c6a838a5574f..e72e1831b46f 100644 --- a/packages/opentelemetry/src/spanExporter.ts +++ b/packages/opentelemetry/src/spanExporter.ts @@ -15,6 +15,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, captureEvent, @@ -391,6 +392,7 @@ function removeSentryAttributes(data: Record): Record { const actual = sampler.shouldSample(ctx, traceId, spanName, spanKind, spanAttributes, links); expect(actual).toEqual({ decision: SamplingDecision.NOT_RECORD, - attributes: { 'sentry.sample_rate': 0 }, + attributes: { 'sentry.sample_rate': 0, 'sentry.override_trace_sample_rate': true }, traceState: new TraceState().set('sentry.sampled_not_recording', '1'), }); expect(spyOnDroppedEvent).toHaveBeenCalledTimes(1); @@ -80,7 +80,7 @@ describe('SentrySampler', () => { const actual = sampler.shouldSample(ctx, traceId, spanName, spanKind, spanAttributes, links); expect(actual).toEqual({ decision: SamplingDecision.RECORD_AND_SAMPLED, - attributes: { 'sentry.sample_rate': 1 }, + attributes: { 'sentry.sample_rate': 1, 'sentry.override_trace_sample_rate': true }, traceState: new TraceState(), }); expect(spyOnDroppedEvent).toHaveBeenCalledTimes(0); diff --git a/packages/opentelemetry/test/trace.test.ts b/packages/opentelemetry/test/trace.test.ts index 2c677ded4516..410dd7cfbe70 100644 --- a/packages/opentelemetry/test/trace.test.ts +++ b/packages/opentelemetry/test/trace.test.ts @@ -8,6 +8,7 @@ import { Span as SpanClass } from '@opentelemetry/sdk-trace-base'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, getClient, @@ -223,6 +224,7 @@ describe('trace', () => { expect(span).toBeDefined(); expect(getSpanAttributes(span)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: true, }); }, ); @@ -243,6 +245,7 @@ describe('trace', () => { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.test.origin', [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'my-op', [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: true, }); }, ); @@ -267,6 +270,7 @@ describe('trace', () => { expect(getSpanStartTime(span)).toEqual(date); expect(getSpanAttributes(span)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: true, test1: 'test 1', test2: 2, }); @@ -524,6 +528,7 @@ describe('trace', () => { expect(span).toBeDefined(); expect(getSpanAttributes(span)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: true, }); const span2 = startInactiveSpan({ @@ -538,6 +543,7 @@ describe('trace', () => { expect(span2).toBeDefined(); expect(getSpanAttributes(span2)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: true, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'task', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.test.origin', [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'my-op', @@ -562,6 +568,7 @@ describe('trace', () => { expect(getSpanStartTime(span)).toEqual(date); expect(getSpanAttributes(span)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: true, test1: 'test 1', test2: 2, }); @@ -834,6 +841,7 @@ describe('trace', () => { expect(getSpanStartTime(span)).toEqual(date); expect(getSpanAttributes(span)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: true, test1: 'test 1', test2: 2, }); From 4a2a7ae548ee8903690968d3eebea624537a623d Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 15 Jan 2025 12:39:11 +0000 Subject: [PATCH 22/45] . --- .../suites/public-api/startSpan/standalone/test.ts | 1 + .../browser/test/tracing/browserTracingIntegration.test.ts | 5 +++++ packages/core/src/envelope.ts | 3 +++ packages/core/src/tracing/sampling.ts | 2 +- packages/core/src/tracing/sentrySpan.ts | 3 ++- 5 files changed, 12 insertions(+), 2 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/test.ts index ef1019d02b1d..06999162e560 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/test.ts @@ -45,6 +45,7 @@ sentryTest('sends a segment span envelope', async ({ getLocalTestUrl, page }) => data: { 'sentry.origin': 'manual', 'sentry.sample_rate': 1, + 'sentry.override_trace_sample_rate': true, // This should probably not be here but for now it will be. 'sentry.source': 'custom', }, description: 'standalone_segment_span', diff --git a/packages/browser/test/tracing/browserTracingIntegration.test.ts b/packages/browser/test/tracing/browserTracingIntegration.test.ts index 0b659332df99..73820b565c70 100644 --- a/packages/browser/test/tracing/browserTracingIntegration.test.ts +++ b/packages/browser/test/tracing/browserTracingIntegration.test.ts @@ -18,6 +18,7 @@ global.window.TextDecoder = TextDecoder; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, TRACING_DEFAULTS, @@ -103,6 +104,7 @@ describe('browserTracingIntegration', () => { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser', [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: true, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', }, span_id: expect.stringMatching(/[a-f0-9]{16}/), @@ -174,6 +176,7 @@ describe('browserTracingIntegration', () => { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser', [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: true, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', }, span_id: expect.stringMatching(/[a-f0-9]{16}/), @@ -288,6 +291,7 @@ describe('browserTracingIntegration', () => { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'manual', [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: true, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom', }, span_id: expect.stringMatching(/[a-f0-9]{16}/), @@ -323,6 +327,7 @@ describe('browserTracingIntegration', () => { data: { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.test', + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: true, [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom', testy: 'yes', diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index 89e231c305fc..238601402178 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -1,4 +1,5 @@ import type { Client } from './client'; +import { SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE } from './semanticAttributes'; import { getDynamicSamplingContextFromSpan } from './tracing/dynamicSamplingContext'; import type { SentrySpan } from './tracing/sentrySpan'; import type { @@ -141,6 +142,8 @@ export function createSpanEnvelope(spans: [SentrySpan, ...SentrySpan[]], client? const items: SpanItem[] = []; for (const span of spans) { const spanJson = convertToSpanJSON(span); + // eslint-disable-next-line @typescript-eslint/no-dynamic-delete + delete spanJson.data[SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]; if (spanJson) { items.push(createSpanEnvelopeItem(spanJson)); } diff --git a/packages/core/src/tracing/sampling.ts b/packages/core/src/tracing/sampling.ts index fd673d76039f..d554417fe026 100644 --- a/packages/core/src/tracing/sampling.ts +++ b/packages/core/src/tracing/sampling.ts @@ -21,7 +21,7 @@ export function sampleSpan( return [false]; } - let shouldUpdateSampleRateOnDsc = false; + let shouldUpdateSampleRateOnDsc = undefined; // we would have bailed already if neither `tracesSampler` nor `tracesSampleRate` nor `enableTracing` were defined, so one of these should // work; prefer the hook if so diff --git a/packages/core/src/tracing/sentrySpan.ts b/packages/core/src/tracing/sentrySpan.ts index 13bc2a0811fe..e22c1278950f 100644 --- a/packages/core/src/tracing/sentrySpan.ts +++ b/packages/core/src/tracing/sentrySpan.ts @@ -351,7 +351,8 @@ export class SentrySpan implements Span { delete this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME]; delete this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]; spans.forEach(span => { - span.data && delete span.data[SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME]; + delete span.data[SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME]; + delete span.data[SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]; }); // eslint-enabled-next-line @typescript-eslint/no-dynamic-delete From 5b81700687a5093c8e4c1e70b6712cf7af64e44e Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 15 Jan 2025 13:19:48 +0000 Subject: [PATCH 23/45] . --- .../suites/public-api/startSpan/standalone/test.ts | 1 - packages/core/src/envelope.ts | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/test.ts index 06999162e560..ef1019d02b1d 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/test.ts @@ -45,7 +45,6 @@ sentryTest('sends a segment span envelope', async ({ getLocalTestUrl, page }) => data: { 'sentry.origin': 'manual', 'sentry.sample_rate': 1, - 'sentry.override_trace_sample_rate': true, // This should probably not be here but for now it will be. 'sentry.source': 'custom', }, description: 'standalone_segment_span', diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index 238601402178..007e6380038e 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -142,8 +142,11 @@ export function createSpanEnvelope(spans: [SentrySpan, ...SentrySpan[]], client? const items: SpanItem[] = []; for (const span of spans) { const spanJson = convertToSpanJSON(span); + + // This attribute is important for internal logic but should not leak into actual data // eslint-disable-next-line @typescript-eslint/no-dynamic-delete delete spanJson.data[SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]; + if (spanJson) { items.push(createSpanEnvelopeItem(spanJson)); } From 9b2afffa81f57aa0fb3d83c8670ef4498e210582 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 16 Jan 2025 12:57:10 +0000 Subject: [PATCH 24/45] tests --- .../sample-rand-propagation/server.js | 0 .../sample-rand-propagation/test.ts | 2 +- .../no-tracing-enabled/server.js | 39 ++++++++++++ .../no-tracing-enabled/test.ts | 25 ++++++++ .../tracesSampleRate/server.js | 40 ++++++++++++ .../tracesSampleRate/test.ts | 61 +++++++++++++++++++ .../tracesSampler/server.js | 46 ++++++++++++++ .../tracesSampler/test.ts | 37 +++++++++++ 8 files changed, 249 insertions(+), 1 deletion(-) rename dev-packages/node-integration-tests/suites/{ => tracing}/sample-rand-propagation/server.js (100%) rename dev-packages/node-integration-tests/suites/{ => tracing}/sample-rand-propagation/test.ts (97%) create mode 100644 dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/no-tracing-enabled/server.js create mode 100644 dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/no-tracing-enabled/test.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampleRate/server.js create mode 100644 dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampleRate/test.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/server.js create mode 100644 dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/test.ts diff --git a/dev-packages/node-integration-tests/suites/sample-rand-propagation/server.js b/dev-packages/node-integration-tests/suites/tracing/sample-rand-propagation/server.js similarity index 100% rename from dev-packages/node-integration-tests/suites/sample-rand-propagation/server.js rename to dev-packages/node-integration-tests/suites/tracing/sample-rand-propagation/server.js diff --git a/dev-packages/node-integration-tests/suites/sample-rand-propagation/test.ts b/dev-packages/node-integration-tests/suites/tracing/sample-rand-propagation/test.ts similarity index 97% rename from dev-packages/node-integration-tests/suites/sample-rand-propagation/test.ts rename to dev-packages/node-integration-tests/suites/tracing/sample-rand-propagation/test.ts index 0253bfddd843..6a55d0a67c36 100644 --- a/dev-packages/node-integration-tests/suites/sample-rand-propagation/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/sample-rand-propagation/test.ts @@ -1,4 +1,4 @@ -import { cleanupChildProcesses, createRunner } from '../../utils/runner'; +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; describe('sample_rand propagation', () => { afterAll(() => { diff --git a/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/no-tracing-enabled/server.js b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/no-tracing-enabled/server.js new file mode 100644 index 000000000000..dbc6b9009c49 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/no-tracing-enabled/server.js @@ -0,0 +1,39 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + transport: loggingTransport, +}); + +// express must be required after Sentry is initialized +const express = require('express'); +const cors = require('cors'); +const { + startExpressServerAndSendPortToRunner, + getPortAppIsRunningOn, +} = require('@sentry-internal/node-integration-tests'); + +const app = express(); + +app.use(cors()); + +app.get('/check', (req, res) => { + const appPort = getPortAppIsRunningOn(app); + // eslint-disable-next-line no-undef + fetch(`http://localhost:${appPort}/bounce`) + .then(r => r.json()) + .then(bounceRes => { + res.json({ propagatedData: bounceRes }); + }); +}); + +app.get('/bounce', (req, res) => { + res.json({ + baggage: req.headers['baggage'], + }); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/no-tracing-enabled/test.ts b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/no-tracing-enabled/test.ts new file mode 100644 index 000000000000..c6800055c84b --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/no-tracing-enabled/test.ts @@ -0,0 +1,25 @@ +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; + +describe('parentSampleRate propagation with no tracing enabled', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + test('should propagate an incoming sample rate', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + const response = await runner.makeRequest('get', '/check', { + headers: { + 'sentry-trace': '530699e319cc067ce440315d74acb312-414dc2a08d5d1dac-1', + baggage: 'sentry-sample_rate=0.1337', + }, + }); + + expect((response as any).propagatedData.baggage).toMatch(/sentry-sample_rate=0\.1337/); + }); + + test('should not propagate a sample rate for root traces', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + const response = await runner.makeRequest('get', '/check'); + expect((response as any).propagatedData.baggage).not.toMatch(/sentry-sample_rate/); + }); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampleRate/server.js b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampleRate/server.js new file mode 100644 index 000000000000..512681043d4d --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampleRate/server.js @@ -0,0 +1,40 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + transport: loggingTransport, + tracesSampleRate: 0.69, +}); + +// express must be required after Sentry is initialized +const express = require('express'); +const cors = require('cors'); +const { + startExpressServerAndSendPortToRunner, + getPortAppIsRunningOn, +} = require('@sentry-internal/node-integration-tests'); + +const app = express(); + +app.use(cors()); + +app.get('/check', (req, res) => { + const appPort = getPortAppIsRunningOn(app); + // eslint-disable-next-line no-undef + fetch(`http://localhost:${appPort}/bounce`) + .then(r => r.json()) + .then(bounceRes => { + res.json({ propagatedData: bounceRes }); + }); +}); + +app.get('/bounce', (req, res) => { + res.json({ + baggage: req.headers['baggage'], + }); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampleRate/test.ts b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampleRate/test.ts new file mode 100644 index 000000000000..27afa03e9045 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampleRate/test.ts @@ -0,0 +1,61 @@ +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; + +describe('parentSampleRate propagation with tracesSampleRate', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + test('should propagate incoming sample rate when inheriting a positive sampling decision', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + const response = await runner.makeRequest('get', '/check', { + headers: { + 'sentry-trace': '530699e319cc067ce440315d74acb312-414dc2a08d5d1dac-1', + baggage: 'sentry-sample_rate=0.1337', + }, + }); + + expect((response as any).propagatedData.baggage).toMatch(/sentry-sample_rate=0\.1337/); + }); + + test('should propagate incoming sample rate when inheriting a negative sampling decision', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + const response = await runner.makeRequest('get', '/check', { + headers: { + 'sentry-trace': '530699e319cc067ce440315d74acb312-414dc2a08d5d1dac-0', + baggage: 'sentry-sample_rate=0.1337', + }, + }); + + expect((response as any).propagatedData.baggage).toMatch(/sentry-sample_rate=0\.1337/); + }); + + test('should propagate configured sample rate when receiving a trace without sampling decision and sample rate', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + const response = await runner.makeRequest('get', '/check', { + headers: { + 'sentry-trace': '530699e319cc067ce440315d74acb312-414dc2a08d5d1dac', + baggage: '', + }, + }); + + expect((response as any).propagatedData.baggage).toMatch(/sentry-sample_rate=0\.69/); + }); + + test('should propagate configured sample rate when receiving a trace without sampling decision, but with sample rate', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + const response = await runner.makeRequest('get', '/check', { + headers: { + 'sentry-trace': '530699e319cc067ce440315d74acb312-414dc2a08d5d1dac', + baggage: 'sentry-sample_rate=0.1337', + }, + }); + + expect((response as any).propagatedData.baggage).toMatch(/sentry-sample_rate=0\.69/); + }); + + test('should propagate configured sample rate when there is no incoming trace', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + const response = await runner.makeRequest('get', '/check'); + expect((response as any).propagatedData.baggage).toMatch(/sentry-sample_rate=0\.69/); + }); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/server.js b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/server.js new file mode 100644 index 000000000000..5dc1d17588e5 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/server.js @@ -0,0 +1,46 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + transport: loggingTransport, + tracesSampler: ({ parentSampleRate }) => { + if (parentSampleRate) { + return parentSampleRate; + } + + return 0.69; + }, +}); + +// express must be required after Sentry is initialized +const express = require('express'); +const cors = require('cors'); +const { + startExpressServerAndSendPortToRunner, + getPortAppIsRunningOn, +} = require('@sentry-internal/node-integration-tests'); + +const app = express(); + +app.use(cors()); + +app.get('/check', (req, res) => { + const appPort = getPortAppIsRunningOn(app); + // eslint-disable-next-line no-undef + fetch(`http://localhost:${appPort}/bounce`) + .then(r => r.json()) + .then(bounceRes => { + res.json({ propagatedData: bounceRes }); + }); +}); + +app.get('/bounce', (req, res) => { + res.json({ + baggage: req.headers['baggage'], + }); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/test.ts b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/test.ts new file mode 100644 index 000000000000..304725268f03 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/test.ts @@ -0,0 +1,37 @@ +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; + +describe('parentSampleRate propagation with tracesSampler', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + test('should propagate sample_rate equivalent to sample rate returned by tracesSampler when there is no incoming trace', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + const response = await runner.makeRequest('get', '/check'); + expect((response as any).propagatedData.baggage).toMatch(/sentry-sample_rate=0\.69/); + }); + + test('should propagate sample_rate equivalent to sample rate returned by tracesSampler when there is no incoming sample rate', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + const response = await runner.makeRequest('get', '/check', { + headers: { + 'sentry-trace': '530699e319cc067ce440315d74acb312-414dc2a08d5d1dac-1', + baggage: '', + }, + }); + + expect((response as any).propagatedData.baggage).toMatch(/sentry-sample_rate=0\.69/); + }); + + test('should propagate sample_rate equivalent to incoming sample_rate (because tracesSampler is configured that way)', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + const response = await runner.makeRequest('get', '/check', { + headers: { + 'sentry-trace': '530699e319cc067ce440315d74acb312-414dc2a08d5d1dac-1', + baggage: 'sentry-sample_rate=0.1337', + }, + }); + + expect((response as any).propagatedData.baggage).toMatch(/sentry-sample_rate=0\.1337/); + }); +}); From 6c6febf7eb4de849722b8ec2a05a326c95e069b7 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 16 Jan 2025 13:42:23 +0000 Subject: [PATCH 25/45] something's off --- .../tracing/browserTracingIntegration.test.ts | 5 ----- packages/core/src/envelope.ts | 6 ----- packages/core/src/semanticAttributes.ts | 5 ----- .../src/tracing/dynamicSamplingContext.ts | 22 ++++++++----------- packages/core/src/tracing/sampling.ts | 5 ++--- packages/core/src/tracing/sentrySpan.ts | 5 +---- packages/core/src/tracing/trace.ts | 10 ++++----- packages/core/src/types-hoist/tracing.ts | 10 +++++++++ packages/opentelemetry/src/sampler.ts | 6 +++-- packages/opentelemetry/src/spanExporter.ts | 2 -- packages/opentelemetry/test/sampler.test.ts | 4 ++-- packages/opentelemetry/test/trace.test.ts | 8 ------- 12 files changed, 32 insertions(+), 56 deletions(-) diff --git a/packages/browser/test/tracing/browserTracingIntegration.test.ts b/packages/browser/test/tracing/browserTracingIntegration.test.ts index 73820b565c70..0b659332df99 100644 --- a/packages/browser/test/tracing/browserTracingIntegration.test.ts +++ b/packages/browser/test/tracing/browserTracingIntegration.test.ts @@ -18,7 +18,6 @@ global.window.TextDecoder = TextDecoder; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, TRACING_DEFAULTS, @@ -104,7 +103,6 @@ describe('browserTracingIntegration', () => { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser', [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: true, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', }, span_id: expect.stringMatching(/[a-f0-9]{16}/), @@ -176,7 +174,6 @@ describe('browserTracingIntegration', () => { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser', [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: true, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', }, span_id: expect.stringMatching(/[a-f0-9]{16}/), @@ -291,7 +288,6 @@ describe('browserTracingIntegration', () => { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'manual', [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: true, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom', }, span_id: expect.stringMatching(/[a-f0-9]{16}/), @@ -327,7 +323,6 @@ describe('browserTracingIntegration', () => { data: { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.test', - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: true, [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom', testy: 'yes', diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index 007e6380038e..89e231c305fc 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -1,5 +1,4 @@ import type { Client } from './client'; -import { SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE } from './semanticAttributes'; import { getDynamicSamplingContextFromSpan } from './tracing/dynamicSamplingContext'; import type { SentrySpan } from './tracing/sentrySpan'; import type { @@ -142,11 +141,6 @@ export function createSpanEnvelope(spans: [SentrySpan, ...SentrySpan[]], client? const items: SpanItem[] = []; for (const span of spans) { const spanJson = convertToSpanJSON(span); - - // This attribute is important for internal logic but should not leak into actual data - // eslint-disable-next-line @typescript-eslint/no-dynamic-delete - delete spanJson.data[SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]; - if (spanJson) { items.push(createSpanEnvelopeItem(spanJson)); } diff --git a/packages/core/src/semanticAttributes.ts b/packages/core/src/semanticAttributes.ts index aca14792f4de..a0b1921ec8f3 100644 --- a/packages/core/src/semanticAttributes.ts +++ b/packages/core/src/semanticAttributes.ts @@ -12,11 +12,6 @@ export const SEMANTIC_ATTRIBUTE_SENTRY_SOURCE = 'sentry.source'; */ export const SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE = 'sentry.sample_rate'; -/** - * Use this attribute on a root span to propagate the spans sample rate downstream as parent sample rate in the DSC, overriding anything that was previously set on the DSC. - */ -export const SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE = 'sentry.override_trace_sample_rate'; - /** * Use this attribute to represent the operation of a span. */ diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index 519481a0fb36..88eccc2bdf60 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -2,11 +2,7 @@ import type { Client } from '../client'; import { DEFAULT_ENVIRONMENT } from '../constants'; import { getClient } from '../currentScopes'; import type { Scope } from '../scope'; -import { - SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, - SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, - SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, -} from '../semanticAttributes'; +import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes'; import type { DynamicSamplingContext, Span } from '../types-hoist'; import { baggageHeaderToDynamicSamplingContext, @@ -79,17 +75,13 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly): Partial { - // Note: This is a `!= null` check meaning "nullish" - if (shouldOverrideDscSampleRate && maybeSampleRate != null) { - dsc.sample_rate = `${maybeSampleRate}`; + if (rootSpanPropagationContext?.sampleRateOverride !== undefined) { + dsc.sample_rate = `${rootSpanPropagationContext.sampleRateOverride}`; } return dsc; } @@ -114,6 +106,10 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly { - delete span.data[SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME]; - delete span.data[SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]; + span.data && delete span.data[SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME]; }); // eslint-enabled-next-line @typescript-eslint/no-dynamic-delete diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index 7f06e4d0e759..d8ed5a0a4995 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -16,11 +16,7 @@ import { getClient, getCurrentScope, getIsolationScope, withScope } from '../cur import { getAsyncContextStrategy } from '../asyncContext'; import { DEBUG_BUILD } from '../debug-build'; import type { Scope } from '../scope'; -import { - SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, - SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, - SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, -} from '../semanticAttributes'; +import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes'; import { logger } from '../utils-hoist/logger'; import { generateTraceId } from '../utils-hoist/propagationContext'; import { propagationContextFromHeaders } from '../utils-hoist/tracing'; @@ -442,7 +438,9 @@ function _startRootSpan(spanArguments: SentrySpanArguments, scope: Scope, parent if (sampleRate !== undefined) { rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, sampleRate); - rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, shouldUpdateSampleRateOnDsc); + if (shouldUpdateSampleRateOnDsc) { + currentPropagationContext.sampleRateOverride = sampleRate; + } } if (client) { diff --git a/packages/core/src/types-hoist/tracing.ts b/packages/core/src/types-hoist/tracing.ts index e1dcfef96c6a..c3aab6a34ef0 100644 --- a/packages/core/src/types-hoist/tracing.ts +++ b/packages/core/src/types-hoist/tracing.ts @@ -50,6 +50,16 @@ export interface PropagationContext { * The current SDK should not modify this value! */ dsc?: Partial; + + /** + * This is set when the currently incoming sample rate should be overridden for further propagation. + * + * This is the case whenever + * - the tracesSampleRate is applied + * - the tracesSampler is invoked + */ + // TODO: Link to docs when this is properly specified. For now: https://github.com/getsentry/team-sdks/issues/117 + sampleRateOverride?: number; } /** diff --git a/packages/opentelemetry/src/sampler.ts b/packages/opentelemetry/src/sampler.ts index 6797c107517f..6e5955844132 100644 --- a/packages/opentelemetry/src/sampler.ts +++ b/packages/opentelemetry/src/sampler.ts @@ -13,7 +13,6 @@ import { import type { Client, SpanAttributes } from '@sentry/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, - SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, hasTracingEnabled, logger, @@ -121,9 +120,12 @@ export class SentrySampler implements Sampler { const attributes: Attributes = { [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: sampleRate, - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: shouldUpdateSampleRateOnDsc, }; + if (currentPropagationContext && sampleRate && shouldUpdateSampleRateOnDsc) { + currentPropagationContext.sampleRateOverride = sampleRate; + } + const method = `${maybeSpanHttpMethod}`.toUpperCase(); if (method === 'OPTIONS' || method === 'HEAD') { DEBUG_BUILD && logger.log(`[Tracing] Not sampling span because HTTP method is '${method}' for ${spanName}`); diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts index e72e1831b46f..c6a838a5574f 100644 --- a/packages/opentelemetry/src/spanExporter.ts +++ b/packages/opentelemetry/src/spanExporter.ts @@ -15,7 +15,6 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, captureEvent, @@ -392,7 +391,6 @@ function removeSentryAttributes(data: Record): Record { const actual = sampler.shouldSample(ctx, traceId, spanName, spanKind, spanAttributes, links); expect(actual).toEqual({ decision: SamplingDecision.NOT_RECORD, - attributes: { 'sentry.sample_rate': 0, 'sentry.override_trace_sample_rate': true }, + attributes: { 'sentry.sample_rate': 0 }, traceState: new TraceState().set('sentry.sampled_not_recording', '1'), }); expect(spyOnDroppedEvent).toHaveBeenCalledTimes(1); @@ -80,7 +80,7 @@ describe('SentrySampler', () => { const actual = sampler.shouldSample(ctx, traceId, spanName, spanKind, spanAttributes, links); expect(actual).toEqual({ decision: SamplingDecision.RECORD_AND_SAMPLED, - attributes: { 'sentry.sample_rate': 1, 'sentry.override_trace_sample_rate': true }, + attributes: { 'sentry.sample_rate': 1 }, traceState: new TraceState(), }); expect(spyOnDroppedEvent).toHaveBeenCalledTimes(0); diff --git a/packages/opentelemetry/test/trace.test.ts b/packages/opentelemetry/test/trace.test.ts index 410dd7cfbe70..2c677ded4516 100644 --- a/packages/opentelemetry/test/trace.test.ts +++ b/packages/opentelemetry/test/trace.test.ts @@ -8,7 +8,6 @@ import { Span as SpanClass } from '@opentelemetry/sdk-trace-base'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, getClient, @@ -224,7 +223,6 @@ describe('trace', () => { expect(span).toBeDefined(); expect(getSpanAttributes(span)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: true, }); }, ); @@ -245,7 +243,6 @@ describe('trace', () => { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.test.origin', [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'my-op', [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: true, }); }, ); @@ -270,7 +267,6 @@ describe('trace', () => { expect(getSpanStartTime(span)).toEqual(date); expect(getSpanAttributes(span)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: true, test1: 'test 1', test2: 2, }); @@ -528,7 +524,6 @@ describe('trace', () => { expect(span).toBeDefined(); expect(getSpanAttributes(span)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: true, }); const span2 = startInactiveSpan({ @@ -543,7 +538,6 @@ describe('trace', () => { expect(span2).toBeDefined(); expect(getSpanAttributes(span2)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: true, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'task', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.test.origin', [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'my-op', @@ -568,7 +562,6 @@ describe('trace', () => { expect(getSpanStartTime(span)).toEqual(date); expect(getSpanAttributes(span)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: true, test1: 'test 1', test2: 2, }); @@ -841,7 +834,6 @@ describe('trace', () => { expect(getSpanStartTime(span)).toEqual(date); expect(getSpanAttributes(span)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: true, test1: 'test 1', test2: 2, }); From 4092e0924f7b340a56e1ac5d93b15e6006e5d341 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 16 Jan 2025 14:32:26 +0000 Subject: [PATCH 26/45] pain --- .../src/tracing/dynamicSamplingContext.ts | 30 ++++++------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index 88eccc2bdf60..ba362c49795a 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -75,21 +75,11 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly): Partial { - if (rootSpanPropagationContext?.sampleRateOverride !== undefined) { - dsc.sample_rate = `${rootSpanPropagationContext.sampleRateOverride}`; - } - return dsc; - } // For core implementation, we freeze the DSC onto the span as a non-enumerable property const frozenDsc = (rootSpan as SpanWithMaybeDsc)[FROZEN_DSC_FIELD]; if (frozenDsc) { - return applyRootSpanSampleRateToDsc(frozenDsc); + return frozenDsc; } // For OpenTelemetry, we freeze the DSC on the trace state @@ -100,26 +90,24 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly Date: Mon, 20 Jan 2025 08:46:23 +0000 Subject: [PATCH 27/45] otel side of things --- packages/core/src/tracing/dynamicSamplingContext.ts | 8 ++++++-- packages/opentelemetry/src/constants.ts | 1 + packages/opentelemetry/src/sampler.ts | 11 +++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index 5da5883f95ce..8c12cb8d5452 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -97,9 +97,13 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly Date: Mon, 20 Jan 2025 09:40:17 +0000 Subject: [PATCH 28/45] help --- .../suites/sample-rand-propagation/server.js | 40 -------- .../suites/sample-rand-propagation/test.ts | 93 ------------------- .../tracing/sample-rand-propagation/server.js | 2 +- .../tracing/sample-rand-propagation/test.ts | 2 + .../tracesSampler/server.js | 2 + .../tracesSampler/test.ts | 4 +- .../tracing/browserTracingIntegration.test.ts | 5 + packages/core/src/envelope.ts | 6 ++ packages/core/src/semanticAttributes.ts | 5 + .../src/tracing/dynamicSamplingContext.ts | 37 +++++--- packages/core/src/tracing/sampling.ts | 15 +-- packages/core/src/tracing/sentrySpan.ts | 5 +- packages/core/src/tracing/trace.ts | 18 ++-- packages/core/src/types-hoist/tracing.ts | 10 -- packages/opentelemetry/src/sampler.ts | 12 +-- packages/opentelemetry/src/spanExporter.ts | 2 + packages/opentelemetry/test/sampler.test.ts | 4 +- packages/opentelemetry/test/trace.test.ts | 8 ++ 18 files changed, 91 insertions(+), 179 deletions(-) delete mode 100644 dev-packages/node-integration-tests/suites/sample-rand-propagation/server.js delete mode 100644 dev-packages/node-integration-tests/suites/sample-rand-propagation/test.ts diff --git a/dev-packages/node-integration-tests/suites/sample-rand-propagation/server.js b/dev-packages/node-integration-tests/suites/sample-rand-propagation/server.js deleted file mode 100644 index d7b54cb25c4f..000000000000 --- a/dev-packages/node-integration-tests/suites/sample-rand-propagation/server.js +++ /dev/null @@ -1,40 +0,0 @@ -const { loggingTransport } = require('@sentry-internal/node-integration-tests'); -const Sentry = require('@sentry/node'); - -Sentry.init({ - dsn: 'https://public@dsn.ingest.sentry.io/1337', - transport: loggingTransport, - tracesSampleRate: 0.00000001, // It's important that this is not 1, so that we also check logic for NonRecordingSpans, which is usually the edge-case -}); - -// express must be required after Sentry is initialized -const express = require('express'); -const cors = require('cors'); -const { - startExpressServerAndSendPortToRunner, - getPortAppIsRunningOn, -} = require('@sentry-internal/node-integration-tests'); - -const app = express(); - -app.use(cors()); - -app.get('/check', (req, res) => { - const appPort = getPortAppIsRunningOn(app); - // eslint-disable-next-line no-undef - fetch(`http://localhost:${appPort}/bounce`) - .then(r => r.json()) - .then(bounceRes => { - res.json({ propagatedData: bounceRes }); - }); -}); - -app.get('/bounce', (req, res) => { - res.json({ - baggage: req.headers['baggage'], - }); -}); - -Sentry.setupExpressErrorHandler(app); - -startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/sample-rand-propagation/test.ts b/dev-packages/node-integration-tests/suites/sample-rand-propagation/test.ts deleted file mode 100644 index 42e6a0a5e555..000000000000 --- a/dev-packages/node-integration-tests/suites/sample-rand-propagation/test.ts +++ /dev/null @@ -1,93 +0,0 @@ -import { cleanupChildProcesses, createRunner } from '../../utils/runner'; - -describe('sample_rand propagation', () => { - afterAll(() => { - cleanupChildProcesses(); - }); - - test('propagates a sample rand when there are no incoming trace headers', async () => { - const runner = createRunner(__dirname, 'server.js').start(); - const response = await runner.makeRequest('get', '/check'); - expect(response).toEqual({ - propagatedData: { - baggage: expect.stringMatching(/sentry-sample_rand=0\.[0-9]+/), - }, - }); - }); - - test('propagates a sample rand when there is a sentry-trace header and incoming sentry baggage', async () => { - const runner = createRunner(__dirname, 'server.js').start(); - const response = await runner.makeRequest('get', '/check', { - headers: { - 'sentry-trace': '530699e319cc067ce440315d74acb312-414dc2a08d5d1dac-1', - baggage: 'sentry-release=foo,sentry-sample_rand=0.424242', - }, - }); - expect(response).toEqual({ - propagatedData: { - baggage: expect.stringMatching(/sentry-sample_rand=0\.424242/), - }, - }); - }); - - test('propagates a sample rand when there is an incoming sentry-trace header but no baggage header', async () => { - const runner = createRunner(__dirname, 'server.js').start(); - const response = await runner.makeRequest('get', '/check', { - headers: { - 'sentry-trace': '530699e319cc067ce440315d74acb312-414dc2a08d5d1dac-1', - }, - }); - expect(response).toEqual({ - propagatedData: { - baggage: expect.stringMatching(/sentry-sample_rand=0\.[0-9]+/), - }, - }); - }); - - test('propagates a sample_rand that would lead to a positive sampling decision when there is an incoming positive sampling decision but no sample_rand in the baggage header', async () => { - const runner = createRunner(__dirname, 'server.js').start(); - const response = await runner.makeRequest('get', '/check', { - headers: { - 'sentry-trace': '530699e319cc067ce440315d74acb312-414dc2a08d5d1dac-1', - baggage: 'sentry-sample_rate=0.25', - }, - }); - - const sampleRand = Number((response as any).propagatedData.baggage.match(/sentry-sample_rand=(0\.[0-9]+)/)[1]); - - expect(sampleRand).toStrictEqual(expect.any(Number)); - expect(sampleRand).not.toBeNaN(); - expect(sampleRand).toBeLessThan(0.25); - expect(sampleRand).toBeGreaterThanOrEqual(0); - }); - - test('propagates a sample_rand that would lead to a negative sampling decision when there is an incoming negative sampling decision but no sample_rand in the baggage header', async () => { - const runner = createRunner(__dirname, 'server.js').start(); - const response = await runner.makeRequest('get', '/check', { - headers: { - 'sentry-trace': '530699e319cc067ce440315d74acb312-414dc2a08d5d1dac-0', - baggage: 'sentry-sample_rate=0.75', - }, - }); - - const sampleRand = Number((response as any).propagatedData.baggage.match(/sentry-sample_rand=(0\.[0-9]+)/)[1]); - - expect(sampleRand).toStrictEqual(expect.any(Number)); - expect(sampleRand).not.toBeNaN(); - expect(sampleRand).toBeGreaterThanOrEqual(0.75); - expect(sampleRand).toBeLessThan(1); - }); - - test('a new sample_rand when there is no sentry-trace header but a baggage header with sample_rand', async () => { - const runner = createRunner(__dirname, 'server.js').start(); - const response = await runner.makeRequest('get', '/check', { - headers: { - baggage: 'sentry-sample_rate=0.75,sentry-sample_rand=0.5', - }, - }); - - expect((response as any).propagatedData.baggage).toMatch(/sentry-sample_rand=0\.[0-9]+/); - const sampleRandStr = (response as any).propagatedData.baggage.match(/sentry-sample_rand=(0\.[0-9]+)/)[1]; - expect(sampleRandStr).not.toBe('0.5'); - }); -}); diff --git a/dev-packages/node-integration-tests/suites/tracing/sample-rand-propagation/server.js b/dev-packages/node-integration-tests/suites/tracing/sample-rand-propagation/server.js index 89ad5ef12f21..d7b54cb25c4f 100644 --- a/dev-packages/node-integration-tests/suites/tracing/sample-rand-propagation/server.js +++ b/dev-packages/node-integration-tests/suites/tracing/sample-rand-propagation/server.js @@ -4,7 +4,7 @@ const Sentry = require('@sentry/node'); Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', transport: loggingTransport, - tracesSampleRate: 1, + tracesSampleRate: 0.00000001, // It's important that this is not 1, so that we also check logic for NonRecordingSpans, which is usually the edge-case }); // express must be required after Sentry is initialized diff --git a/dev-packages/node-integration-tests/suites/tracing/sample-rand-propagation/test.ts b/dev-packages/node-integration-tests/suites/tracing/sample-rand-propagation/test.ts index 6a55d0a67c36..7c566c1d8eeb 100644 --- a/dev-packages/node-integration-tests/suites/tracing/sample-rand-propagation/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/sample-rand-propagation/test.ts @@ -58,6 +58,7 @@ describe('sample_rand propagation', () => { expect(sampleRand).toStrictEqual(expect.any(Number)); expect(sampleRand).not.toBeNaN(); expect(sampleRand).toBeLessThan(0.25); + expect(sampleRand).toBeGreaterThanOrEqual(0); }); test('propagates a sample_rand that would lead to a negative sampling decision when there is an incoming negative sampling decision but no sample_rand in the baggage header', async () => { @@ -74,6 +75,7 @@ describe('sample_rand propagation', () => { expect(sampleRand).toStrictEqual(expect.any(Number)); expect(sampleRand).not.toBeNaN(); expect(sampleRand).toBeGreaterThanOrEqual(0.75); + expect(sampleRand).toBeLessThan(1); }); test('a new sample_rand when there is no sentry-trace header but a baggage header with sample_rand', async () => { diff --git a/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/server.js b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/server.js index 5dc1d17588e5..09d5a6ac9c72 100644 --- a/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/server.js +++ b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/server.js @@ -5,6 +5,8 @@ Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', transport: loggingTransport, tracesSampler: ({ parentSampleRate }) => { + console.log({ parentSampleRate }); + if (parentSampleRate) { return parentSampleRate; } diff --git a/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/test.ts b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/test.ts index 304725268f03..ac320b9cebc4 100644 --- a/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/test.ts @@ -23,12 +23,12 @@ describe('parentSampleRate propagation with tracesSampler', () => { expect((response as any).propagatedData.baggage).toMatch(/sentry-sample_rate=0\.69/); }); - test('should propagate sample_rate equivalent to incoming sample_rate (because tracesSampler is configured that way)', async () => { + test.only('should propagate sample_rate equivalent to incoming sample_rate (because tracesSampler is configured that way)', async () => { const runner = createRunner(__dirname, 'server.js').start(); const response = await runner.makeRequest('get', '/check', { headers: { 'sentry-trace': '530699e319cc067ce440315d74acb312-414dc2a08d5d1dac-1', - baggage: 'sentry-sample_rate=0.1337', + baggage: 'sentry-sample_rate=0.1337,sentry-sample_rand=0.444', }, }); diff --git a/packages/browser/test/tracing/browserTracingIntegration.test.ts b/packages/browser/test/tracing/browserTracingIntegration.test.ts index 0b659332df99..3c94e17298c0 100644 --- a/packages/browser/test/tracing/browserTracingIntegration.test.ts +++ b/packages/browser/test/tracing/browserTracingIntegration.test.ts @@ -18,6 +18,7 @@ global.window.TextDecoder = TextDecoder; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, TRACING_DEFAULTS, @@ -103,6 +104,7 @@ describe('browserTracingIntegration', () => { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser', [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', }, span_id: expect.stringMatching(/[a-f0-9]{16}/), @@ -174,6 +176,7 @@ describe('browserTracingIntegration', () => { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser', [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', }, span_id: expect.stringMatching(/[a-f0-9]{16}/), @@ -288,6 +291,7 @@ describe('browserTracingIntegration', () => { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'manual', [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom', }, span_id: expect.stringMatching(/[a-f0-9]{16}/), @@ -323,6 +327,7 @@ describe('browserTracingIntegration', () => { data: { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.test', + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom', testy: 'yes', diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index 89e231c305fc..007e6380038e 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -1,4 +1,5 @@ import type { Client } from './client'; +import { SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE } from './semanticAttributes'; import { getDynamicSamplingContextFromSpan } from './tracing/dynamicSamplingContext'; import type { SentrySpan } from './tracing/sentrySpan'; import type { @@ -141,6 +142,11 @@ export function createSpanEnvelope(spans: [SentrySpan, ...SentrySpan[]], client? const items: SpanItem[] = []; for (const span of spans) { const spanJson = convertToSpanJSON(span); + + // This attribute is important for internal logic but should not leak into actual data + // eslint-disable-next-line @typescript-eslint/no-dynamic-delete + delete spanJson.data[SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]; + if (spanJson) { items.push(createSpanEnvelopeItem(spanJson)); } diff --git a/packages/core/src/semanticAttributes.ts b/packages/core/src/semanticAttributes.ts index a0b1921ec8f3..aca14792f4de 100644 --- a/packages/core/src/semanticAttributes.ts +++ b/packages/core/src/semanticAttributes.ts @@ -12,6 +12,11 @@ export const SEMANTIC_ATTRIBUTE_SENTRY_SOURCE = 'sentry.source'; */ export const SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE = 'sentry.sample_rate'; +/** + * Use this attribute on a root span to propagate the spans sample rate downstream as parent sample rate in the DSC, overriding anything that was previously set on the DSC. + */ +export const SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE = 'sentry.override_trace_sample_rate'; + /** * Use this attribute to represent the operation of a span. */ diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index 8c12cb8d5452..8cdedb81f5eb 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -2,7 +2,11 @@ import type { Client } from '../client'; import { DEFAULT_ENVIRONMENT } from '../constants'; import { getClient } from '../currentScopes'; import type { Scope } from '../scope'; -import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, + SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, +} from '../semanticAttributes'; import type { DynamicSamplingContext, Span } from '../types-hoist'; import { baggageHeaderToDynamicSamplingContext, @@ -75,31 +79,42 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly): Partial { + if (typeof traceSampleRateOverride === 'number' || typeof traceSampleRateOverride === 'string') { + dsc.sample_rate = `${traceSampleRateOverride}`; + } + return dsc; + } // For core implementation, we freeze the DSC onto the span as a non-enumerable property const frozenDsc = (rootSpan as SpanWithMaybeDsc)[FROZEN_DSC_FIELD]; if (frozenDsc) { - return frozenDsc; + return applyTraceSampleRateOverrideToDsc(frozenDsc); } // For OpenTelemetry, we freeze the DSC on the trace state - const traceState = rootSpan.spanContext().traceState; const traceStateDsc = traceState?.get('sentry.dsc'); // If the span has a DSC, we want it to take precedence const dscOnTraceState = traceStateDsc && baggageHeaderToDynamicSamplingContext(traceStateDsc); if (dscOnTraceState) { - return dscOnTraceState; + return applyTraceSampleRateOverrideToDsc(dscOnTraceState); } // Else, we generate it from the span const dsc = getDynamicSamplingContextFromClient(span.spanContext().traceId, client); - const jsonSpan = spanToJSON(rootSpan); - const attributes = jsonSpan.data; - const maybeSampleRate = - traceState?.get('sentry.sample_rate_override') ?? attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]; + const maybeSampleRate = rootSpanAttributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]; if ( // Note: This check is `!=`, so we effectively check for "nullish" maybeSampleRate != null @@ -108,10 +123,10 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly, samplingContext: SamplingContext, sampleRand: number, -): [sampled: boolean, sampleRate?: number, shouldUpdateSampleRateOnDsc?: boolean] { +): [sampled: boolean, sampleRate?: number, shouldUpdateSampleRateOnDownstreamTrace?: boolean] { // nothing to do if tracing is not enabled if (!hasTracingEnabled(options)) { return [false]; } - let shouldUpdateSampleRateOnDsc: undefined | true = undefined; + let shouldUpdateSampleRateOnDownstreamTrace = undefined; // we would have bailed already if neither `tracesSampler` nor `tracesSampleRate` nor `enableTracing` were defined, so one of these should // work; prefer the hook if so let sampleRate; if (typeof options.tracesSampler === 'function') { sampleRate = options.tracesSampler(samplingContext); - shouldUpdateSampleRateOnDsc = true; + shouldUpdateSampleRateOnDownstreamTrace = true; } else if (samplingContext.parentSampled !== undefined) { sampleRate = samplingContext.parentSampled; } else if (typeof options.tracesSampleRate !== 'undefined') { sampleRate = options.tracesSampleRate; - shouldUpdateSampleRateOnDsc = true; + shouldUpdateSampleRateOnDownstreamTrace = true; } else { // When `enableTracing === true`, we use a sample rate of 100% sampleRate = 1; - shouldUpdateSampleRateOnDsc = true; + shouldUpdateSampleRateOnDownstreamTrace = true; } // Since this is coming from the user (or from a function provided by the user), who knows what we might get. @@ -59,7 +59,7 @@ export function sampleSpan( : 'a negative sampling decision was inherited or tracesSampleRate is set to 0' }`, ); - return [false, parsedSampleRate, shouldUpdateSampleRateOnDsc]; + return [false, parsedSampleRate, shouldUpdateSampleRateOnDownstreamTrace]; } // We always compare the sample rand for the current execution context against the chosen sample rate. @@ -74,7 +74,8 @@ export function sampleSpan( sampleRate, )})`, ); + return [false, parsedSampleRate, shouldUpdateSampleRateOnDownstreamTrace]; } - return [shouldSample, parsedSampleRate, shouldUpdateSampleRateOnDsc]; + return [true, parsedSampleRate, shouldUpdateSampleRateOnDownstreamTrace]; } diff --git a/packages/core/src/tracing/sentrySpan.ts b/packages/core/src/tracing/sentrySpan.ts index acc14d37bc31..e22c1278950f 100644 --- a/packages/core/src/tracing/sentrySpan.ts +++ b/packages/core/src/tracing/sentrySpan.ts @@ -7,6 +7,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, } from '../semanticAttributes'; import type { @@ -348,8 +349,10 @@ export class SentrySpan implements Span { // remove internal root span attributes we don't need to send. /* eslint-disable @typescript-eslint/no-dynamic-delete */ delete this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME]; + delete this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]; spans.forEach(span => { - span.data && delete span.data[SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME]; + delete span.data[SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME]; + delete span.data[SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]; }); // eslint-enabled-next-line @typescript-eslint/no-dynamic-delete diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index a60190f0ea17..9b5cdfd2eb57 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -16,7 +16,11 @@ import { getClient, getCurrentScope, getIsolationScope, withScope } from '../cur import { getAsyncContextStrategy } from '../asyncContext'; import { DEBUG_BUILD } from '../debug-build'; import type { Scope } from '../scope'; -import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, + SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, +} from '../semanticAttributes'; import { logger } from '../utils-hoist/logger'; import { generateTraceId } from '../utils-hoist/propagationContext'; import { propagationContextFromHeaders } from '../utils-hoist/tracing'; @@ -409,7 +413,7 @@ function _startRootSpan(spanArguments: SentrySpanArguments, scope: Scope, parent const { name = '', attributes } = spanArguments; const currentPropagationContext = scope.getPropagationContext(); - const [sampled, sampleRate, shouldUpdateSampleRateOnDsc] = scope.getScopeData().sdkProcessingMetadata[ + const [sampled, sampleRate, shouldUpdateSampleRateOnDownstreamTrace] = scope.getScopeData().sdkProcessingMetadata[ SUPPRESS_TRACING_KEY ] ? [false] @@ -439,10 +443,12 @@ function _startRootSpan(spanArguments: SentrySpanArguments, scope: Scope, parent } if (sampleRate !== undefined) { - rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, sampleRate); - if (shouldUpdateSampleRateOnDsc) { - currentPropagationContext.sampleRateOverride = sampleRate; - } + rootSpan.setAttributes({ + [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: sampleRate, + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: shouldUpdateSampleRateOnDownstreamTrace + ? sampleRate + : undefined, + }); } if (client) { diff --git a/packages/core/src/types-hoist/tracing.ts b/packages/core/src/types-hoist/tracing.ts index c3aab6a34ef0..e1dcfef96c6a 100644 --- a/packages/core/src/types-hoist/tracing.ts +++ b/packages/core/src/types-hoist/tracing.ts @@ -50,16 +50,6 @@ export interface PropagationContext { * The current SDK should not modify this value! */ dsc?: Partial; - - /** - * This is set when the currently incoming sample rate should be overridden for further propagation. - * - * This is the case whenever - * - the tracesSampleRate is applied - * - the tracesSampler is invoked - */ - // TODO: Link to docs when this is properly specified. For now: https://github.com/getsentry/team-sdks/issues/117 - sampleRateOverride?: number; } /** diff --git a/packages/opentelemetry/src/sampler.ts b/packages/opentelemetry/src/sampler.ts index fbf3e6b0d4ee..a45555ea8f55 100644 --- a/packages/opentelemetry/src/sampler.ts +++ b/packages/opentelemetry/src/sampler.ts @@ -13,6 +13,7 @@ import { import type { Client, SpanAttributes } from '@sentry/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, hasTracingEnabled, logger, @@ -111,7 +112,7 @@ export class SentrySampler implements Sampler { const { isolationScope, scope } = getScopesFromContext(context) ?? {}; const currentPropagationContext = scope?.getPropagationContext(); const sampleRand = currentPropagationContext?.sampleRand ?? Math.random(); - const [sampled, sampleRate, shouldUpdateSampleRateOnDsc] = sampleSpan( + const [sampled, sampleRate, shouldUpdateSampleRateOnDownstreamTrace] = sampleSpan( options, { name: inferredSpanName, @@ -125,12 +126,11 @@ export class SentrySampler implements Sampler { const attributes: Attributes = { [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: sampleRate, + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: shouldUpdateSampleRateOnDownstreamTrace + ? sampleRate + : undefined, }; - if (currentPropagationContext && sampleRate && shouldUpdateSampleRateOnDsc) { - currentPropagationContext.sampleRateOverride = sampleRate; - } - const method = `${maybeSpanHttpMethod}`.toUpperCase(); if (method === 'OPTIONS' || method === 'HEAD') { DEBUG_BUILD && logger.log(`[Tracing] Not sampling span because HTTP method is '${method}' for ${spanName}`); @@ -227,7 +227,7 @@ export function wrapSamplingDecision({ // - the tracesSampler is invoked // Since unsampled OTEL spans (NonRecordingSpans) cannot hold attributes we need to store this on the (trace)context. if (sampleRateOverride) { - traceState = traceState.set(SENTRY_TRACE_STATE_SAMPLE_RATE_OVERRIDE, `${sampleRand}`); + traceState = traceState.set(SENTRY_TRACE_STATE_SAMPLE_RATE_OVERRIDE, `${sampleRateOverride}`); } // If the decision is undefined, we treat it as NOT_RECORDING, but we don't propagate this decision to downstream SDKs diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts index c6a838a5574f..e72e1831b46f 100644 --- a/packages/opentelemetry/src/spanExporter.ts +++ b/packages/opentelemetry/src/spanExporter.ts @@ -15,6 +15,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, captureEvent, @@ -391,6 +392,7 @@ function removeSentryAttributes(data: Record): Record { expect(actual).toEqual( expect.objectContaining({ decision: SamplingDecision.NOT_RECORD, - attributes: { 'sentry.sample_rate': 0 }, + attributes: { 'sentry.sample_rate': 0, 'sentry.override_trace_sample_rate': 0 }, }), ); expect(actual.traceState?.get('sentry.sampled_not_recording')).toBe('1'); @@ -83,7 +83,7 @@ describe('SentrySampler', () => { const actual = sampler.shouldSample(ctx, traceId, spanName, spanKind, spanAttributes, links); expect(actual).toEqual({ decision: SamplingDecision.RECORD_AND_SAMPLED, - attributes: { 'sentry.sample_rate': 1 }, + attributes: { 'sentry.sample_rate': 1, 'sentry.override_trace_sample_rate': 1 }, traceState: expect.any(TraceState), }); expect(spyOnDroppedEvent).toHaveBeenCalledTimes(0); diff --git a/packages/opentelemetry/test/trace.test.ts b/packages/opentelemetry/test/trace.test.ts index 2c677ded4516..761283630f6d 100644 --- a/packages/opentelemetry/test/trace.test.ts +++ b/packages/opentelemetry/test/trace.test.ts @@ -8,6 +8,7 @@ import { Span as SpanClass } from '@opentelemetry/sdk-trace-base'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, getClient, @@ -223,6 +224,7 @@ describe('trace', () => { expect(span).toBeDefined(); expect(getSpanAttributes(span)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: 1, }); }, ); @@ -243,6 +245,7 @@ describe('trace', () => { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.test.origin', [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'my-op', [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: 1, }); }, ); @@ -267,6 +270,7 @@ describe('trace', () => { expect(getSpanStartTime(span)).toEqual(date); expect(getSpanAttributes(span)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: 1, test1: 'test 1', test2: 2, }); @@ -524,6 +528,7 @@ describe('trace', () => { expect(span).toBeDefined(); expect(getSpanAttributes(span)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: 1, }); const span2 = startInactiveSpan({ @@ -538,6 +543,7 @@ describe('trace', () => { expect(span2).toBeDefined(); expect(getSpanAttributes(span2)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'task', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.test.origin', [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'my-op', @@ -562,6 +568,7 @@ describe('trace', () => { expect(getSpanStartTime(span)).toEqual(date); expect(getSpanAttributes(span)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: 1, test1: 'test 1', test2: 2, }); @@ -834,6 +841,7 @@ describe('trace', () => { expect(getSpanStartTime(span)).toEqual(date); expect(getSpanAttributes(span)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: 1, test1: 'test 1', test2: 2, }); From fdf0fa7c84d75a74662f8ec2a66bc0fdbf0744e2 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 20 Jan 2025 09:43:13 +0000 Subject: [PATCH 29/45] . --- packages/opentelemetry/src/sampler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry/src/sampler.ts b/packages/opentelemetry/src/sampler.ts index a45555ea8f55..d32058daf24d 100644 --- a/packages/opentelemetry/src/sampler.ts +++ b/packages/opentelemetry/src/sampler.ts @@ -121,7 +121,7 @@ export class SentrySampler implements Sampler { parentSampled, parentSampleRate: parseSampleRate(currentPropagationContext?.dsc?.sample_rate), }, - currentPropagationContext?.sampleRand ?? Math.random(), + sampleRand, ); const attributes: Attributes = { From e9471ebb8eba19aa290a964b602a6c7d2da122b1 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 20 Jan 2025 10:15:02 +0000 Subject: [PATCH 30/45] bless --- .../tracesSampler/test.ts | 4 ++-- packages/opentelemetry/src/sampler.ts | 19 +++++++++++-------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/test.ts b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/test.ts index ac320b9cebc4..304725268f03 100644 --- a/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/test.ts @@ -23,12 +23,12 @@ describe('parentSampleRate propagation with tracesSampler', () => { expect((response as any).propagatedData.baggage).toMatch(/sentry-sample_rate=0\.69/); }); - test.only('should propagate sample_rate equivalent to incoming sample_rate (because tracesSampler is configured that way)', async () => { + test('should propagate sample_rate equivalent to incoming sample_rate (because tracesSampler is configured that way)', async () => { const runner = createRunner(__dirname, 'server.js').start(); const response = await runner.makeRequest('get', '/check', { headers: { 'sentry-trace': '530699e319cc067ce440315d74acb312-414dc2a08d5d1dac-1', - baggage: 'sentry-sample_rate=0.1337,sentry-sample_rand=0.444', + baggage: 'sentry-sample_rate=0.1337', }, }); diff --git a/packages/opentelemetry/src/sampler.ts b/packages/opentelemetry/src/sampler.ts index d32058daf24d..8738e485dc44 100644 --- a/packages/opentelemetry/src/sampler.ts +++ b/packages/opentelemetry/src/sampler.ts @@ -11,9 +11,9 @@ import { SEMATTRS_HTTP_URL, } from '@opentelemetry/semantic-conventions'; import type { Client, SpanAttributes } from '@sentry/core'; +import { baggageHeaderToDynamicSamplingContext } from '@sentry/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, - SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, hasTracingEnabled, logger, @@ -21,6 +21,7 @@ import { sampleSpan, } from '@sentry/core'; import { + SENTRY_TRACE_STATE_DSC, SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING, SENTRY_TRACE_STATE_SAMPLE_RAND, SENTRY_TRACE_STATE_SAMPLE_RATE_OVERRIDE, @@ -109,9 +110,13 @@ export class SentrySampler implements Sampler { // We only sample based on parameters (like tracesSampleRate or tracesSampler) for root spans (which is done in sampleSpan). // Non-root-spans simply inherit the sampling decision from their parent. if (isRootSpan) { - const { isolationScope, scope } = getScopesFromContext(context) ?? {}; - const currentPropagationContext = scope?.getPropagationContext(); - const sampleRand = currentPropagationContext?.sampleRand ?? Math.random(); + const { isolationScope } = getScopesFromContext(context) ?? {}; + + const dscString = parentContext?.traceState ? parentContext.traceState.get(SENTRY_TRACE_STATE_DSC) : undefined; + const dsc = dscString ? baggageHeaderToDynamicSamplingContext(dscString) : undefined; + + const sampleRand = parseSampleRate(dsc?.sample_rand) ?? Math.random(); + const [sampled, sampleRate, shouldUpdateSampleRateOnDownstreamTrace] = sampleSpan( options, { @@ -119,16 +124,13 @@ export class SentrySampler implements Sampler { attributes: mergedAttributes, normalizedRequest: isolationScope?.getScopeData().sdkProcessingMetadata.normalizedRequest, parentSampled, - parentSampleRate: parseSampleRate(currentPropagationContext?.dsc?.sample_rate), + parentSampleRate: parseSampleRate(dsc?.sample_rate), }, sampleRand, ); const attributes: Attributes = { [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: sampleRate, - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: shouldUpdateSampleRateOnDownstreamTrace - ? sampleRate - : undefined, }; const method = `${maybeSpanHttpMethod}`.toUpperCase(); @@ -156,6 +158,7 @@ export class SentrySampler implements Sampler { context, spanAttributes, sampleRand, + sampleRateOverride: shouldUpdateSampleRateOnDownstreamTrace ? sampleRate : undefined, }), attributes, }; From 2b491f2c792fab5e6a0fbd90abb6673f7274b4c2 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 20 Jan 2025 10:26:38 +0000 Subject: [PATCH 31/45] beep boop --- .../sample-rate-propagation/tracesSampler/server.js | 2 -- .../test/lib/tracing/dynamicSamplingContext.test.ts | 10 +++++++--- packages/opentelemetry/src/sampler.ts | 8 +++++++- packages/opentelemetry/src/spanExporter.ts | 2 -- packages/opentelemetry/test/sampler.test.ts | 4 ++-- packages/opentelemetry/test/trace.test.ts | 7 ------- 6 files changed, 16 insertions(+), 17 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/server.js b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/server.js index 09d5a6ac9c72..5dc1d17588e5 100644 --- a/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/server.js +++ b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampler/server.js @@ -5,8 +5,6 @@ Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', transport: loggingTransport, tracesSampler: ({ parentSampleRate }) => { - console.log({ parentSampleRate }); - if (parentSampleRate) { return parentSampleRate; } diff --git a/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts b/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts index 3856b9d35d13..e103e9623149 100644 --- a/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts +++ b/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts @@ -42,8 +42,12 @@ describe('getDynamicSamplingContextFromSpan', () => { spanId: '12345', traceFlags: 0, traceState: { - get() { - return 'sentry-environment=myEnv2'; + get(key: string) { + if (key === 'sentry.dsc') { + return 'sentry-environment=myEnv2'; + } else { + return undefined; + } }, } as unknown as SpanContextData['traceState'], }; @@ -68,7 +72,7 @@ describe('getDynamicSamplingContextFromSpan', () => { release: '1.0.1', environment: 'production', sampled: 'true', - sample_rate: '0.56', + sample_rate: '1', trace_id: expect.stringMatching(/^[a-f0-9]{32}$/), transaction: 'tx', sample_rand: expect.any(String), diff --git a/packages/opentelemetry/src/sampler.ts b/packages/opentelemetry/src/sampler.ts index 8738e485dc44..8362a47188cf 100644 --- a/packages/opentelemetry/src/sampler.ts +++ b/packages/opentelemetry/src/sampler.ts @@ -138,7 +138,13 @@ export class SentrySampler implements Sampler { DEBUG_BUILD && logger.log(`[Tracing] Not sampling span because HTTP method is '${method}' for ${spanName}`); return { - ...wrapSamplingDecision({ decision: SamplingDecision.NOT_RECORD, context, spanAttributes, sampleRand }), + ...wrapSamplingDecision({ + decision: SamplingDecision.NOT_RECORD, + context, + spanAttributes, + sampleRand, + sampleRateOverride: 0, // we don't want to sample anything in the downstream trace either + }), attributes, }; } diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts index e72e1831b46f..c6a838a5574f 100644 --- a/packages/opentelemetry/src/spanExporter.ts +++ b/packages/opentelemetry/src/spanExporter.ts @@ -15,7 +15,6 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, captureEvent, @@ -392,7 +391,6 @@ function removeSentryAttributes(data: Record): Record { expect(actual).toEqual( expect.objectContaining({ decision: SamplingDecision.NOT_RECORD, - attributes: { 'sentry.sample_rate': 0, 'sentry.override_trace_sample_rate': 0 }, + attributes: { 'sentry.sample_rate': 0 }, }), ); expect(actual.traceState?.get('sentry.sampled_not_recording')).toBe('1'); @@ -83,7 +83,7 @@ describe('SentrySampler', () => { const actual = sampler.shouldSample(ctx, traceId, spanName, spanKind, spanAttributes, links); expect(actual).toEqual({ decision: SamplingDecision.RECORD_AND_SAMPLED, - attributes: { 'sentry.sample_rate': 1, 'sentry.override_trace_sample_rate': 1 }, + attributes: { 'sentry.sample_rate': 1 }, traceState: expect.any(TraceState), }); expect(spyOnDroppedEvent).toHaveBeenCalledTimes(0); diff --git a/packages/opentelemetry/test/trace.test.ts b/packages/opentelemetry/test/trace.test.ts index 761283630f6d..d8d5af12cbcc 100644 --- a/packages/opentelemetry/test/trace.test.ts +++ b/packages/opentelemetry/test/trace.test.ts @@ -224,7 +224,6 @@ describe('trace', () => { expect(span).toBeDefined(); expect(getSpanAttributes(span)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: 1, }); }, ); @@ -245,7 +244,6 @@ describe('trace', () => { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.test.origin', [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'my-op', [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: 1, }); }, ); @@ -270,7 +268,6 @@ describe('trace', () => { expect(getSpanStartTime(span)).toEqual(date); expect(getSpanAttributes(span)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: 1, test1: 'test 1', test2: 2, }); @@ -528,7 +525,6 @@ describe('trace', () => { expect(span).toBeDefined(); expect(getSpanAttributes(span)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: 1, }); const span2 = startInactiveSpan({ @@ -543,7 +539,6 @@ describe('trace', () => { expect(span2).toBeDefined(); expect(getSpanAttributes(span2)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'task', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.test.origin', [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'my-op', @@ -568,7 +563,6 @@ describe('trace', () => { expect(getSpanStartTime(span)).toEqual(date); expect(getSpanAttributes(span)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: 1, test1: 'test 1', test2: 2, }); @@ -841,7 +835,6 @@ describe('trace', () => { expect(getSpanStartTime(span)).toEqual(date); expect(getSpanAttributes(span)).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: 1, test1: 'test 1', test2: 2, }); From a96e9c66abb9568d47c1b8de0c8e6ef15ff8e401 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 20 Jan 2025 11:57:06 +0000 Subject: [PATCH 32/45] lint --- packages/opentelemetry/test/trace.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/opentelemetry/test/trace.test.ts b/packages/opentelemetry/test/trace.test.ts index d8d5af12cbcc..2c677ded4516 100644 --- a/packages/opentelemetry/test/trace.test.ts +++ b/packages/opentelemetry/test/trace.test.ts @@ -8,7 +8,6 @@ import { Span as SpanClass } from '@opentelemetry/sdk-trace-base'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, getClient, From 063984bc8814733cd576f9e5e5616b99831d3fa6 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 20 Jan 2025 13:12:16 +0000 Subject: [PATCH 33/45] fix and test 0% sample rate --- .../tracesSampleRate-0/server.js | 40 ++++++++++++ .../tracesSampleRate-0/test.ts | 61 +++++++++++++++++++ packages/opentelemetry/src/sampler.ts | 12 ++-- 3 files changed, 107 insertions(+), 6 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampleRate-0/server.js create mode 100644 dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampleRate-0/test.ts diff --git a/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampleRate-0/server.js b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampleRate-0/server.js new file mode 100644 index 000000000000..dc2f49b081cf --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampleRate-0/server.js @@ -0,0 +1,40 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + transport: loggingTransport, + tracesSampleRate: 0, +}); + +// express must be required after Sentry is initialized +const express = require('express'); +const cors = require('cors'); +const { + startExpressServerAndSendPortToRunner, + getPortAppIsRunningOn, +} = require('@sentry-internal/node-integration-tests'); + +const app = express(); + +app.use(cors()); + +app.get('/check', (req, res) => { + const appPort = getPortAppIsRunningOn(app); + // eslint-disable-next-line no-undef + fetch(`http://localhost:${appPort}/bounce`) + .then(r => r.json()) + .then(bounceRes => { + res.json({ propagatedData: bounceRes }); + }); +}); + +app.get('/bounce', (req, res) => { + res.json({ + baggage: req.headers['baggage'], + }); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampleRate-0/test.ts b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampleRate-0/test.ts new file mode 100644 index 000000000000..c12d2920dd9f --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/sample-rate-propagation/tracesSampleRate-0/test.ts @@ -0,0 +1,61 @@ +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; + +describe('parentSampleRate propagation with tracesSampleRate=0', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + test('should propagate incoming sample rate when inheriting a positive sampling decision', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + const response = await runner.makeRequest('get', '/check', { + headers: { + 'sentry-trace': '530699e319cc067ce440315d74acb312-414dc2a08d5d1dac-1', + baggage: 'sentry-sample_rate=0.1337', + }, + }); + + expect((response as any).propagatedData.baggage).toMatch(/sentry-sample_rate=0\.1337/); + }); + + test('should propagate incoming sample rate when inheriting a negative sampling decision', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + const response = await runner.makeRequest('get', '/check', { + headers: { + 'sentry-trace': '530699e319cc067ce440315d74acb312-414dc2a08d5d1dac-0', + baggage: 'sentry-sample_rate=0.1337', + }, + }); + + expect((response as any).propagatedData.baggage).toMatch(/sentry-sample_rate=0\.1337/); + }); + + test('should propagate configured sample rate when receiving a trace without sampling decision and sample rate', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + const response = await runner.makeRequest('get', '/check', { + headers: { + 'sentry-trace': '530699e319cc067ce440315d74acb312-414dc2a08d5d1dac', + baggage: '', + }, + }); + + expect((response as any).propagatedData.baggage).toMatch(/sentry-sample_rate=0/); + }); + + test('should propagate configured sample rate when receiving a trace without sampling decision, but with sample rate', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + const response = await runner.makeRequest('get', '/check', { + headers: { + 'sentry-trace': '530699e319cc067ce440315d74acb312-414dc2a08d5d1dac', + baggage: 'sentry-sample_rate=0.1337', + }, + }); + + expect((response as any).propagatedData.baggage).toMatch(/sentry-sample_rate=0/); + }); + + test('should propagate configured sample rate when there is no incoming trace', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + const response = await runner.makeRequest('get', '/check'); + expect((response as any).propagatedData.baggage).toMatch(/sentry-sample_rate=0/); + }); +}); diff --git a/packages/opentelemetry/src/sampler.ts b/packages/opentelemetry/src/sampler.ts index 8362a47188cf..69146c7bd672 100644 --- a/packages/opentelemetry/src/sampler.ts +++ b/packages/opentelemetry/src/sampler.ts @@ -143,7 +143,7 @@ export class SentrySampler implements Sampler { context, spanAttributes, sampleRand, - sampleRateOverride: 0, // we don't want to sample anything in the downstream trace either + shouldUpdateSampleRateOnDownstreamTrace: 0, // we don't want to sample anything in the downstream trace either }), attributes, }; @@ -164,7 +164,7 @@ export class SentrySampler implements Sampler { context, spanAttributes, sampleRand, - sampleRateOverride: shouldUpdateSampleRateOnDownstreamTrace ? sampleRate : undefined, + shouldUpdateSampleRateOnDownstreamTrace: shouldUpdateSampleRateOnDownstreamTrace ? sampleRate : undefined, }), attributes, }; @@ -217,13 +217,13 @@ export function wrapSamplingDecision({ context, spanAttributes, sampleRand, - sampleRateOverride, + shouldUpdateSampleRateOnDownstreamTrace, }: { decision: SamplingDecision | undefined; context: Context; spanAttributes: SpanAttributes; sampleRand?: number; - sampleRateOverride?: number; + shouldUpdateSampleRateOnDownstreamTrace?: number; }): SamplingResult { let traceState = getBaseTraceState(context, spanAttributes); @@ -235,8 +235,8 @@ export function wrapSamplingDecision({ // - the tracesSampleRate is applied // - the tracesSampler is invoked // Since unsampled OTEL spans (NonRecordingSpans) cannot hold attributes we need to store this on the (trace)context. - if (sampleRateOverride) { - traceState = traceState.set(SENTRY_TRACE_STATE_SAMPLE_RATE_OVERRIDE, `${sampleRateOverride}`); + if (shouldUpdateSampleRateOnDownstreamTrace !== undefined) { + traceState = traceState.set(SENTRY_TRACE_STATE_SAMPLE_RATE_OVERRIDE, `${shouldUpdateSampleRateOnDownstreamTrace}`); } // If the decision is undefined, we treat it as NOT_RECORDING, but we don't propagate this decision to downstream SDKs From 8607cc8ff9d4747ba6759380bac63c320c84752c Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 20 Jan 2025 13:13:54 +0000 Subject: [PATCH 34/45] apply before emit --- packages/core/src/tracing/dynamicSamplingContext.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index 8cdedb81f5eb..bd02a0e6d968 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -146,9 +146,11 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly Date: Mon, 20 Jan 2025 15:45:33 +0000 Subject: [PATCH 35/45] fix merge mistake --- packages/core/src/tracing/sampling.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/core/src/tracing/sampling.ts b/packages/core/src/tracing/sampling.ts index 9a9e8fe33073..24567ab2ef3f 100644 --- a/packages/core/src/tracing/sampling.ts +++ b/packages/core/src/tracing/sampling.ts @@ -21,6 +21,8 @@ export function sampleSpan( return [false]; } + let shouldUpdateSampleRateOnDownstreamTrace = undefined; + // we would have bailed already if neither `tracesSampler` nor `tracesSampleRate` were defined, so one of these should // work; prefer the hook if so let sampleRate; From bceb9f3596f71d423297449b90300e2b2b59a208 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 21 Jan 2025 10:10:13 +0000 Subject: [PATCH 36/45] Refactor again --- .../tracing/browserTracingIntegration.test.ts | 5 -- packages/core/src/envelope.ts | 6 --- packages/core/src/semanticAttributes.ts | 8 +--- .../src/tracing/dynamicSamplingContext.ts | 33 ++++--------- packages/core/src/tracing/sampling.ts | 13 +++-- packages/core/src/tracing/sentrySpan.ts | 3 -- packages/core/src/tracing/trace.ts | 17 ++----- .../tracing/dynamicSamplingContext.test.ts | 2 +- packages/core/test/lib/tracing/trace.test.ts | 3 -- packages/opentelemetry/src/constants.ts | 2 +- packages/opentelemetry/src/sampler.ts | 48 ++++++++----------- .../test/integration/transactions.test.ts | 1 - .../opentelemetry/test/propagator.test.ts | 3 -- packages/opentelemetry/test/sampler.test.ts | 1 - packages/opentelemetry/test/trace.test.ts | 3 -- 15 files changed, 43 insertions(+), 105 deletions(-) diff --git a/packages/browser/test/tracing/browserTracingIntegration.test.ts b/packages/browser/test/tracing/browserTracingIntegration.test.ts index 3c94e17298c0..0b659332df99 100644 --- a/packages/browser/test/tracing/browserTracingIntegration.test.ts +++ b/packages/browser/test/tracing/browserTracingIntegration.test.ts @@ -18,7 +18,6 @@ global.window.TextDecoder = TextDecoder; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, TRACING_DEFAULTS, @@ -104,7 +103,6 @@ describe('browserTracingIntegration', () => { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser', [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', }, span_id: expect.stringMatching(/[a-f0-9]{16}/), @@ -176,7 +174,6 @@ describe('browserTracingIntegration', () => { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser', [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', }, span_id: expect.stringMatching(/[a-f0-9]{16}/), @@ -291,7 +288,6 @@ describe('browserTracingIntegration', () => { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'manual', [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom', }, span_id: expect.stringMatching(/[a-f0-9]{16}/), @@ -327,7 +323,6 @@ describe('browserTracingIntegration', () => { data: { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.test', - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom', testy: 'yes', diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index 007e6380038e..89e231c305fc 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -1,5 +1,4 @@ import type { Client } from './client'; -import { SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE } from './semanticAttributes'; import { getDynamicSamplingContextFromSpan } from './tracing/dynamicSamplingContext'; import type { SentrySpan } from './tracing/sentrySpan'; import type { @@ -142,11 +141,6 @@ export function createSpanEnvelope(spans: [SentrySpan, ...SentrySpan[]], client? const items: SpanItem[] = []; for (const span of spans) { const spanJson = convertToSpanJSON(span); - - // This attribute is important for internal logic but should not leak into actual data - // eslint-disable-next-line @typescript-eslint/no-dynamic-delete - delete spanJson.data[SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]; - if (spanJson) { items.push(createSpanEnvelopeItem(spanJson)); } diff --git a/packages/core/src/semanticAttributes.ts b/packages/core/src/semanticAttributes.ts index aca14792f4de..dea57836d3bc 100644 --- a/packages/core/src/semanticAttributes.ts +++ b/packages/core/src/semanticAttributes.ts @@ -6,17 +6,13 @@ export const SEMANTIC_ATTRIBUTE_SENTRY_SOURCE = 'sentry.source'; /** - * Use this attribute to represent the sample rate used for a span. + * Attributes that holds the sample rate that was locally applied to a span. + * If this attribute is not defined, it means that the span inherited a sampling decision. * * NOTE: Is only defined on root spans. */ export const SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE = 'sentry.sample_rate'; -/** - * Use this attribute on a root span to propagate the spans sample rate downstream as parent sample rate in the DSC, overriding anything that was previously set on the DSC. - */ -export const SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE = 'sentry.override_trace_sample_rate'; - /** * Use this attribute to represent the operation of a span. */ diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index bd02a0e6d968..e6f3ceae79f8 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -2,11 +2,7 @@ import type { Client } from '../client'; import { DEFAULT_ENVIRONMENT } from '../constants'; import { getClient } from '../currentScopes'; import type { Scope } from '../scope'; -import { - SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, - SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, - SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, -} from '../semanticAttributes'; +import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes'; import type { DynamicSamplingContext, Span } from '../types-hoist'; import { baggageHeaderToDynamicSamplingContext, @@ -83,14 +79,13 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly): Partial { - if (typeof traceSampleRateOverride === 'number' || typeof traceSampleRateOverride === 'string') { - dsc.sample_rate = `${traceSampleRateOverride}`; + const rootSpanSampleRate = + traceState?.get('sentry.sample_rate') ?? rootSpanAttributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]; + function applyLocalSampleRateToDsc(dsc: Partial): Partial { + if (typeof rootSpanSampleRate === 'number' || typeof rootSpanSampleRate === 'string') { + dsc.sample_rate = `${rootSpanSampleRate}`; } return dsc; } @@ -98,7 +93,7 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly, samplingContext: SamplingContext, sampleRand: number, -): [sampled: boolean, sampleRate?: number, shouldUpdateSampleRateOnDownstreamTrace?: boolean] { +): [sampled: boolean, sampleRate?: number, localSampleRateWasApplied?: boolean] { // nothing to do if tracing is not enabled if (!hasTracingEnabled(options)) { return [false]; } - let shouldUpdateSampleRateOnDownstreamTrace = undefined; + let localSampleRateWasApplied = undefined; // we would have bailed already if neither `tracesSampler` nor `tracesSampleRate` were defined, so one of these should // work; prefer the hook if so let sampleRate; if (typeof options.tracesSampler === 'function') { sampleRate = options.tracesSampler(samplingContext); - shouldUpdateSampleRateOnDownstreamTrace = true; + localSampleRateWasApplied = true; } else if (samplingContext.parentSampled !== undefined) { sampleRate = samplingContext.parentSampled; } else if (typeof options.tracesSampleRate !== 'undefined') { sampleRate = options.tracesSampleRate; - shouldUpdateSampleRateOnDownstreamTrace = true; + localSampleRateWasApplied = true; } // Since this is coming from the user (or from a function provided by the user), who knows what we might get. @@ -55,7 +55,7 @@ export function sampleSpan( : 'a negative sampling decision was inherited or tracesSampleRate is set to 0' }`, ); - return [false, parsedSampleRate, shouldUpdateSampleRateOnDownstreamTrace]; + return [false, parsedSampleRate, localSampleRateWasApplied]; } // We always compare the sample rand for the current execution context against the chosen sample rate. @@ -70,8 +70,7 @@ export function sampleSpan( sampleRate, )})`, ); - return [false, parsedSampleRate, shouldUpdateSampleRateOnDownstreamTrace]; } - return [true, parsedSampleRate, shouldUpdateSampleRateOnDownstreamTrace]; + return [shouldSample, parsedSampleRate, localSampleRateWasApplied]; } diff --git a/packages/core/src/tracing/sentrySpan.ts b/packages/core/src/tracing/sentrySpan.ts index e22c1278950f..74478f79903f 100644 --- a/packages/core/src/tracing/sentrySpan.ts +++ b/packages/core/src/tracing/sentrySpan.ts @@ -7,7 +7,6 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, } from '../semanticAttributes'; import type { @@ -349,10 +348,8 @@ export class SentrySpan implements Span { // remove internal root span attributes we don't need to send. /* eslint-disable @typescript-eslint/no-dynamic-delete */ delete this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME]; - delete this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]; spans.forEach(span => { delete span.data[SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME]; - delete span.data[SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]; }); // eslint-enabled-next-line @typescript-eslint/no-dynamic-delete diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index 9b5cdfd2eb57..aae7ba0ca9b1 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -16,11 +16,7 @@ import { getClient, getCurrentScope, getIsolationScope, withScope } from '../cur import { getAsyncContextStrategy } from '../asyncContext'; import { DEBUG_BUILD } from '../debug-build'; import type { Scope } from '../scope'; -import { - SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE, - SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, - SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, -} from '../semanticAttributes'; +import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes'; import { logger } from '../utils-hoist/logger'; import { generateTraceId } from '../utils-hoist/propagationContext'; import { propagationContextFromHeaders } from '../utils-hoist/tracing'; @@ -413,7 +409,7 @@ function _startRootSpan(spanArguments: SentrySpanArguments, scope: Scope, parent const { name = '', attributes } = spanArguments; const currentPropagationContext = scope.getPropagationContext(); - const [sampled, sampleRate, shouldUpdateSampleRateOnDownstreamTrace] = scope.getScopeData().sdkProcessingMetadata[ + const [sampled, sampleRate, localSampleRateWasApplied] = scope.getScopeData().sdkProcessingMetadata[ SUPPRESS_TRACING_KEY ] ? [false] @@ -442,13 +438,8 @@ function _startRootSpan(spanArguments: SentrySpanArguments, scope: Scope, parent client.recordDroppedEvent('sample_rate', 'transaction'); } - if (sampleRate !== undefined) { - rootSpan.setAttributes({ - [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: sampleRate, - [SEMANTIC_ATTRIBUTE_SENTRY_OVERRIDE_TRACE_SAMPLE_RATE]: shouldUpdateSampleRateOnDownstreamTrace - ? sampleRate - : undefined, - }); + if (sampleRate !== undefined && localSampleRateWasApplied) { + rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, sampleRate); } if (client) { diff --git a/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts b/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts index e103e9623149..63a6910de06b 100644 --- a/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts +++ b/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts @@ -72,7 +72,7 @@ describe('getDynamicSamplingContextFromSpan', () => { release: '1.0.1', environment: 'production', sampled: 'true', - sample_rate: '1', + sample_rate: '0.56', trace_id: expect.stringMatching(/^[a-f0-9]{32}$/), transaction: 'tx', sample_rand: expect.any(String), diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index cf2db6cb316a..0eee7338a93d 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -481,7 +481,6 @@ describe('startSpan', () => { trace: { data: { 'sentry.source': 'custom', - 'sentry.sample_rate': 1, 'sentry.origin': 'manual', }, parent_span_id: innerParentSpanId, @@ -986,7 +985,6 @@ describe('startSpanManual', () => { trace: { data: { 'sentry.source': 'custom', - 'sentry.sample_rate': 1, 'sentry.origin': 'manual', }, parent_span_id: innerParentSpanId, @@ -1317,7 +1315,6 @@ describe('startInactiveSpan', () => { trace: { data: { 'sentry.source': 'custom', - 'sentry.sample_rate': 1, 'sentry.origin': 'manual', }, parent_span_id: innerParentSpanId, diff --git a/packages/opentelemetry/src/constants.ts b/packages/opentelemetry/src/constants.ts index ad9a3911495c..375e42dfdd00 100644 --- a/packages/opentelemetry/src/constants.ts +++ b/packages/opentelemetry/src/constants.ts @@ -7,7 +7,7 @@ export const SENTRY_TRACE_STATE_DSC = 'sentry.dsc'; export const SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING = 'sentry.sampled_not_recording'; export const SENTRY_TRACE_STATE_URL = 'sentry.url'; export const SENTRY_TRACE_STATE_SAMPLE_RAND = 'sentry.sample_rand'; -export const SENTRY_TRACE_STATE_SAMPLE_RATE_OVERRIDE = 'sentry.sample_rate_override'; +export const SENTRY_TRACE_STATE_SAMPLE_RATE = 'sentry.sample_rate'; export const SENTRY_SCOPES_CONTEXT_KEY = createContextKey('sentry_scopes'); diff --git a/packages/opentelemetry/src/sampler.ts b/packages/opentelemetry/src/sampler.ts index 69146c7bd672..91bb72a12967 100644 --- a/packages/opentelemetry/src/sampler.ts +++ b/packages/opentelemetry/src/sampler.ts @@ -1,5 +1,5 @@ /* eslint-disable complexity */ -import type { Attributes, Context, Span, TraceState as TraceStateInterface } from '@opentelemetry/api'; +import type { Context, Span, TraceState as TraceStateInterface } from '@opentelemetry/api'; import { SpanKind, isSpanContextValid, trace } from '@opentelemetry/api'; import { TraceState } from '@opentelemetry/core'; import type { Sampler, SamplingResult } from '@opentelemetry/sdk-trace-base'; @@ -10,21 +10,14 @@ import { SEMATTRS_HTTP_METHOD, SEMATTRS_HTTP_URL, } from '@opentelemetry/semantic-conventions'; -import type { Client, SpanAttributes } from '@sentry/core'; +import { Client, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SpanAttributes } from '@sentry/core'; import { baggageHeaderToDynamicSamplingContext } from '@sentry/core'; -import { - SEMANTIC_ATTRIBUTE_SENTRY_OP, - SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, - hasTracingEnabled, - logger, - parseSampleRate, - sampleSpan, -} from '@sentry/core'; +import { SEMANTIC_ATTRIBUTE_SENTRY_OP, hasTracingEnabled, logger, parseSampleRate, sampleSpan } from '@sentry/core'; import { SENTRY_TRACE_STATE_DSC, SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING, SENTRY_TRACE_STATE_SAMPLE_RAND, - SENTRY_TRACE_STATE_SAMPLE_RATE_OVERRIDE, + SENTRY_TRACE_STATE_SAMPLE_RATE, SENTRY_TRACE_STATE_URL, } from './constants'; import { DEBUG_BUILD } from './debug-build'; @@ -117,7 +110,7 @@ export class SentrySampler implements Sampler { const sampleRand = parseSampleRate(dsc?.sample_rand) ?? Math.random(); - const [sampled, sampleRate, shouldUpdateSampleRateOnDownstreamTrace] = sampleSpan( + const [sampled, sampleRate, localSampleRateWasApplied] = sampleSpan( options, { name: inferredSpanName, @@ -129,10 +122,6 @@ export class SentrySampler implements Sampler { sampleRand, ); - const attributes: Attributes = { - [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: sampleRate, - }; - const method = `${maybeSpanHttpMethod}`.toUpperCase(); if (method === 'OPTIONS' || method === 'HEAD') { DEBUG_BUILD && logger.log(`[Tracing] Not sampling span because HTTP method is '${method}' for ${spanName}`); @@ -143,9 +132,8 @@ export class SentrySampler implements Sampler { context, spanAttributes, sampleRand, - shouldUpdateSampleRateOnDownstreamTrace: 0, // we don't want to sample anything in the downstream trace either + downstreamTraceSampleRate: 0, // we don't want to sample anything in the downstream trace either }), - attributes, }; } @@ -164,9 +152,12 @@ export class SentrySampler implements Sampler { context, spanAttributes, sampleRand, - shouldUpdateSampleRateOnDownstreamTrace: shouldUpdateSampleRateOnDownstreamTrace ? sampleRate : undefined, + downstreamTraceSampleRate: localSampleRateWasApplied ? sampleRate : undefined, }), - attributes, + attributes: { + // We set the sample rate on the span when a local sample rate was applied to better understand how traces were sampled in Sentry + [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: localSampleRateWasApplied ? sampleRate : undefined, + }, }; } else { return { @@ -175,7 +166,6 @@ export class SentrySampler implements Sampler { context, spanAttributes, }), - attributes: {}, }; } } @@ -217,26 +207,26 @@ export function wrapSamplingDecision({ context, spanAttributes, sampleRand, - shouldUpdateSampleRateOnDownstreamTrace, + downstreamTraceSampleRate, }: { decision: SamplingDecision | undefined; context: Context; spanAttributes: SpanAttributes; sampleRand?: number; - shouldUpdateSampleRateOnDownstreamTrace?: number; + downstreamTraceSampleRate?: number; }): SamplingResult { let traceState = getBaseTraceState(context, spanAttributes); - if (sampleRand !== undefined) { - traceState = traceState.set(SENTRY_TRACE_STATE_SAMPLE_RAND, `${sampleRand}`); - } - // We will override the propagated sample rate downstream when // - the tracesSampleRate is applied // - the tracesSampler is invoked // Since unsampled OTEL spans (NonRecordingSpans) cannot hold attributes we need to store this on the (trace)context. - if (shouldUpdateSampleRateOnDownstreamTrace !== undefined) { - traceState = traceState.set(SENTRY_TRACE_STATE_SAMPLE_RATE_OVERRIDE, `${shouldUpdateSampleRateOnDownstreamTrace}`); + if (downstreamTraceSampleRate !== undefined) { + traceState = traceState.set(SENTRY_TRACE_STATE_SAMPLE_RATE, `${downstreamTraceSampleRate}`); + } + + if (sampleRand !== undefined) { + traceState = traceState.set(SENTRY_TRACE_STATE_SAMPLE_RAND, `${sampleRand}`); } // If the decision is undefined, we treat it as NOT_RECORDING, but we don't propagate this decision to downstream SDKs diff --git a/packages/opentelemetry/test/integration/transactions.test.ts b/packages/opentelemetry/test/integration/transactions.test.ts index 8000c078e3bf..fa2705dec961 100644 --- a/packages/opentelemetry/test/integration/transactions.test.ts +++ b/packages/opentelemetry/test/integration/transactions.test.ts @@ -374,7 +374,6 @@ describe('Integration | Transactions', () => { 'sentry.op': 'test op', 'sentry.origin': 'auto.test', 'sentry.source': 'task', - 'sentry.sample_rate': 1, }, op: 'test op', span_id: expect.stringMatching(/[a-f0-9]{16}/), diff --git a/packages/opentelemetry/test/propagator.test.ts b/packages/opentelemetry/test/propagator.test.ts index 408a151e95ef..48b6372e65a0 100644 --- a/packages/opentelemetry/test/propagator.test.ts +++ b/packages/opentelemetry/test/propagator.test.ts @@ -131,7 +131,6 @@ describe('SentryPropagator', () => { 'sentry-environment=production', 'sentry-release=1.0.0', 'sentry-public_key=abc', - 'sentry-sample_rate=1', 'sentry-sampled=true', 'sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b', 'sentry-transaction=test', @@ -185,7 +184,6 @@ describe('SentryPropagator', () => { 'sentry-environment=production', 'sentry-release=1.0.0', 'sentry-public_key=abc', - 'sentry-sample_rate=1', 'sentry-sampled=true', 'sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b', 'sentry-transaction=test', @@ -335,7 +333,6 @@ describe('SentryPropagator', () => { 'sentry-environment=production', 'sentry-release=1.0.0', 'sentry-public_key=abc', - 'sentry-sample_rate=1', 'sentry-sampled=true', 'sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b', 'sentry-transaction=test', diff --git a/packages/opentelemetry/test/sampler.test.ts b/packages/opentelemetry/test/sampler.test.ts index fc38037a7ff2..585e3271a93a 100644 --- a/packages/opentelemetry/test/sampler.test.ts +++ b/packages/opentelemetry/test/sampler.test.ts @@ -60,7 +60,6 @@ describe('SentrySampler', () => { const actual = sampler.shouldSample(ctx, traceId, spanName, spanKind, spanAttributes, links); expect(actual).toEqual({ decision: SamplingDecision.NOT_RECORD, - attributes: {}, traceState: new TraceState().set(SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING, '1'), }); expect(spyOnDroppedEvent).toHaveBeenCalledTimes(0); diff --git a/packages/opentelemetry/test/trace.test.ts b/packages/opentelemetry/test/trace.test.ts index 2c677ded4516..a841bec6ffb6 100644 --- a/packages/opentelemetry/test/trace.test.ts +++ b/packages/opentelemetry/test/trace.test.ts @@ -434,7 +434,6 @@ describe('trace', () => { data: { 'sentry.source': 'custom', 'sentry.origin': 'manual', - 'sentry.sample_rate': 1, }, parent_span_id: innerParentSpanId, span_id: expect.stringMatching(/[a-f0-9]{16}/), @@ -693,7 +692,6 @@ describe('trace', () => { data: { 'sentry.source': 'custom', 'sentry.origin': 'manual', - 'sentry.sample_rate': 1, }, parent_span_id: innerParentSpanId, span_id: expect.stringMatching(/[a-f0-9]{16}/), @@ -990,7 +988,6 @@ describe('trace', () => { data: { 'sentry.source': 'custom', 'sentry.origin': 'manual', - 'sentry.sample_rate': 1, }, parent_span_id: innerParentSpanId, span_id: expect.stringMatching(/[a-f0-9]{16}/), From e2ffc52ee1fa231dc194affbf7835c1670b52d40 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 21 Jan 2025 10:22:15 +0000 Subject: [PATCH 37/45] lint --- packages/opentelemetry/src/sampler.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/opentelemetry/src/sampler.ts b/packages/opentelemetry/src/sampler.ts index 91bb72a12967..9e2f00d565de 100644 --- a/packages/opentelemetry/src/sampler.ts +++ b/packages/opentelemetry/src/sampler.ts @@ -10,7 +10,8 @@ import { SEMATTRS_HTTP_METHOD, SEMATTRS_HTTP_URL, } from '@opentelemetry/semantic-conventions'; -import { Client, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SpanAttributes } from '@sentry/core'; +import type { Client, SpanAttributes } from '@sentry/core'; +import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE } from '@sentry/core'; import { baggageHeaderToDynamicSamplingContext } from '@sentry/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, hasTracingEnabled, logger, parseSampleRate, sampleSpan } from '@sentry/core'; import { From feb43ee8e778038de1d11edbd9e4980db1e08e0a Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 21 Jan 2025 10:29:08 +0000 Subject: [PATCH 38/45] update tests --- .../suites/tracing/trace-lifetime/navigation/test.ts | 4 ++-- .../suites/tracing/trace-lifetime/pageload/test.ts | 4 ++-- packages/node/test/integration/transactions.test.ts | 1 - 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts index a123099107d2..ca6e9723ce65 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts @@ -250,7 +250,7 @@ sentryTest( const navigationTraceId = navigationTraceContext?.trace_id; expect(headers['sentry-trace']).toMatch(new RegExp(`^${navigationTraceId}-[0-9a-f]{16}-1$`)); expect(headers['baggage']).toEqual( - `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId},sentry-sample_rate=1,sentry-sampled=true,sentry-sample_rand=${navigationTraceHeader?.sample_rand}`, + `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId},sentry-sampled=true,sentry-sample_rand=${navigationTraceHeader?.sample_rand},sentry-sample_rate=1`, ); }, ); @@ -313,7 +313,7 @@ sentryTest( const navigationTraceId = navigationTraceContext?.trace_id; expect(headers['sentry-trace']).toMatch(new RegExp(`^${navigationTraceId}-[0-9a-f]{16}-1$`)); expect(headers['baggage']).toEqual( - `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId},sentry-sample_rate=1,sentry-sampled=true,sentry-sample_rand=${navigationTraceHeader?.sample_rand}`, + `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId},sentry-sampled=true,sentry-sample_rand=${navigationTraceHeader?.sample_rand},sentry-sample_rate=1`, ); }, ); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts index a4439840da7d..4af462e26aca 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts @@ -240,7 +240,7 @@ sentryTest( // sampling decision is propagated from active span sampling decision expect(headers['sentry-trace']).toMatch(new RegExp(`^${pageloadTraceId}-[0-9a-f]{16}-1$`)); expect(headers['baggage']).toBe( - `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId},sentry-sample_rate=1,sentry-sampled=true,sentry-sample_rand=${pageloadTraceHeader?.sample_rand}`, + `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId},sentry-sampled=true,sentry-sample_rand=${pageloadTraceHeader?.sample_rand},sentry-sample_rate=1`, ); }, ); @@ -297,7 +297,7 @@ sentryTest( // sampling decision is propagated from active span sampling decision expect(headers['sentry-trace']).toMatch(new RegExp(`^${pageloadTraceId}-[0-9a-f]{16}-1$`)); expect(headers['baggage']).toBe( - `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId},sentry-sample_rate=1,sentry-sampled=true,sentry-sample_rand=${pageloadTraceHeader?.sample_rand}`, + `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId},sentry-sampled=true,sentry-sample_rand=${pageloadTraceHeader?.sample_rand},sentry-sample_rate=1`, ); }, ); diff --git a/packages/node/test/integration/transactions.test.ts b/packages/node/test/integration/transactions.test.ts index 4768f3176859..1f396b11c3af 100644 --- a/packages/node/test/integration/transactions.test.ts +++ b/packages/node/test/integration/transactions.test.ts @@ -492,7 +492,6 @@ describe('Integration | Transactions', () => { 'sentry.op': 'test op', 'sentry.origin': 'auto.test', 'sentry.source': 'task', - 'sentry.sample_rate': 1, }, op: 'test op', span_id: expect.stringMatching(/[a-f0-9]{16}/), From 2a127b2146fdd1f037d3ebd14355ef81cea47dfa Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 21 Jan 2025 10:43:56 +0000 Subject: [PATCH 39/45] . --- packages/core/src/tracing/trace.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index aae7ba0ca9b1..64a4c8ba5a4c 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -428,6 +428,8 @@ function _startRootSpan(spanArguments: SentrySpanArguments, scope: Scope, parent ...spanArguments, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom', + [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: + sampleRate !== undefined && localSampleRateWasApplied ? sampleRate : undefined, ...spanArguments.attributes, }, sampled, @@ -438,10 +440,6 @@ function _startRootSpan(spanArguments: SentrySpanArguments, scope: Scope, parent client.recordDroppedEvent('sample_rate', 'transaction'); } - if (sampleRate !== undefined && localSampleRateWasApplied) { - rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, sampleRate); - } - if (client) { client.emit('spanStart', rootSpan); } From 70c3fd919314871c0f006835259c020f16d13338 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 21 Jan 2025 12:39:39 +0000 Subject: [PATCH 40/45] fix crap --- packages/browser/src/tracing/browserTracingIntegration.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index 5bbda349b25d..b32cab7db3f5 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -317,6 +317,9 @@ export const browserTracingIntegration = ((_options: Partial Date: Tue, 21 Jan 2025 12:39:44 +0000 Subject: [PATCH 41/45] update tests --- .../suites/tracing/metrics/web-vitals-inp-late/test.ts | 1 - .../tracing/metrics/web-vitals-inp-parametrized-late/test.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/test.ts index d151772439a1..00fa6042934c 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/test.ts @@ -67,7 +67,6 @@ sentryTest('should capture an INP click event span after pageload', async ({ bro 'sentry.exclusive_time': inpValue, 'sentry.op': 'ui.interaction.click', 'sentry.origin': 'auto.http.browser.inp', - 'sentry.sample_rate': 1, 'sentry.source': 'custom', transaction: 'test-url', 'user_agent.original': expect.stringContaining('Chrome'), diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/test.ts index 961f98518049..ece59d8fd281 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/test.ts @@ -70,7 +70,6 @@ sentryTest( 'sentry.exclusive_time': inpValue, 'sentry.op': 'ui.interaction.click', 'sentry.origin': 'auto.http.browser.inp', - 'sentry.sample_rate': 1, 'sentry.source': 'custom', transaction: 'test-route', 'user_agent.original': expect.stringContaining('Chrome'), From 812e7500b9984545223073a5c18fa8867676777f Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 21 Jan 2025 13:12:51 +0000 Subject: [PATCH 42/45] more tests --- .../baggage-header-assign/test.ts | 44 ++++++++++++------- .../error-active-span-unsampled/test.ts | 1 + 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts index b873e4a5c0aa..513cf6146d0f 100644 --- a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts +++ b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts @@ -1,3 +1,4 @@ +import { parseBaggageHeader } from '@sentry/core'; import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; import type { TestAPIResponse } from '../server'; @@ -102,14 +103,20 @@ test('Should populate and propagate sentry baggage if sentry-trace header does n const response = await runner.makeRequest('get', '/test/express'); expect(response).toBeDefined(); - expect(response).toMatchObject({ - test_data: { - host: 'somewhere.not.sentry', - // TraceId changes, hence we only expect that the string contains the traceid key - baggage: expect.stringMatching( - /sentry-environment=prod,sentry-release=1.0,sentry-public_key=public,sentry-trace_id=[\S]*,sentry-sample_rate=1,sentry-transaction=GET%20%2Ftest%2Fexpress/, - ), - }, + + const parsedBaggage = parseBaggageHeader(response?.test_data.baggage); + + expect(response?.test_data.host).toBe('somewhere.not.sentry'); + expect(parsedBaggage).toStrictEqual({ + 'sentry-environment': 'prod', + 'sentry-release': '1.0', + 'sentry-public_key': 'public', + // TraceId changes, hence we only expect that the string contains the traceid key + 'sentry-trace_id': expect.stringMatching(/[\S]*/), + 'sentry-sample_rand': expect.stringMatching(/[\S]*/), + 'sentry-sample_rate': '1', + 'sentry-sampled': 'true', + 'sentry-transaction': 'GET /test/express', }); }); @@ -123,13 +130,18 @@ test('Should populate Sentry and ignore 3rd party content if sentry-trace header }); expect(response).toBeDefined(); - expect(response).toMatchObject({ - test_data: { - host: 'somewhere.not.sentry', - // TraceId changes, hence we only expect that the string contains the traceid key - baggage: expect.stringMatching( - /sentry-environment=prod,sentry-release=1.0,sentry-public_key=public,sentry-trace_id=[\S]*,sentry-sample_rate=1,sentry-transaction=GET%20%2Ftest%2Fexpress/, - ), - }, + expect(response?.test_data.host).toBe('somewhere.not.sentry'); + + const parsedBaggage = parseBaggageHeader(response?.test_data.baggage); + expect(parsedBaggage).toStrictEqual({ + 'sentry-environment': 'prod', + 'sentry-release': '1.0', + 'sentry-public_key': 'public', + // TraceId changes, hence we only expect that the string contains the traceid key + 'sentry-trace_id': expect.stringMatching(/[\S]*/), + 'sentry-sample_rand': expect.stringMatching(/[\S]*/), + 'sentry-sample_rate': '1', + 'sentry-sampled': 'true', + 'sentry-transaction': 'GET /test/express', }); }); 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 6ba4aaa74b32..105722a43239 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 @@ -10,6 +10,7 @@ test('envelope header for error event during active unsampled span is correct', public_key: 'public', environment: 'production', release: '1.0', + sample_rate: '0', sampled: 'false', sample_rand: expect.any(String), }, From 2882ca4de6882b44a31128d550c33ac75b282f1c Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 21 Jan 2025 13:18:14 +0000 Subject: [PATCH 43/45] e2e tests --- .../test-applications/astro-4/tests/tracing.dynamic.test.ts | 1 - .../test-applications/astro-4/tests/tracing.static.test.ts | 1 - .../test-applications/astro-5/tests/tracing.dynamic.test.ts | 1 - .../astro-5/tests/tracing.serverIslands.test.ts | 2 -- .../test-applications/astro-5/tests/tracing.static.test.ts | 1 - .../nestjs-distributed-tracing/tests/propagation.test.ts | 3 --- .../nextjs-app-dir/tests/transactions.test.ts | 1 - .../test-applications/node-fastify-5/tests/propagation.test.ts | 2 -- .../test-applications/node-fastify/tests/propagation.test.ts | 2 -- .../test-applications/node-koa/tests/propagation.test.ts | 2 -- 10 files changed, 16 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.dynamic.test.ts b/dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.dynamic.test.ts index 9a295f677d96..644afc377545 100644 --- a/dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.dynamic.test.ts +++ b/dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.dynamic.test.ts @@ -31,7 +31,6 @@ test.describe('tracing in dynamically rendered (ssr) routes', () => { data: expect.objectContaining({ 'sentry.op': 'pageload', 'sentry.origin': 'auto.pageload.browser', - 'sentry.sample_rate': 1, 'sentry.source': 'url', }), op: 'pageload', diff --git a/dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.static.test.ts b/dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.static.test.ts index 8817b2b22aa7..c04bbb568f2e 100644 --- a/dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.static.test.ts +++ b/dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.static.test.ts @@ -36,7 +36,6 @@ test.describe('tracing in static/pre-rendered routes', () => { data: expect.objectContaining({ 'sentry.op': 'pageload', 'sentry.origin': 'auto.pageload.browser', - 'sentry.sample_rate': 1, 'sentry.source': 'url', }), op: 'pageload', diff --git a/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.dynamic.test.ts b/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.dynamic.test.ts index 8c0e2c0c8850..2bcf6cbf2362 100644 --- a/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.dynamic.test.ts +++ b/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.dynamic.test.ts @@ -31,7 +31,6 @@ test.describe('tracing in dynamically rendered (ssr) routes', () => { data: expect.objectContaining({ 'sentry.op': 'pageload', 'sentry.origin': 'auto.pageload.browser', - 'sentry.sample_rate': 1, 'sentry.source': 'url', }), op: 'pageload', diff --git a/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.serverIslands.test.ts b/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.serverIslands.test.ts index a6b288f4de71..fc396999d76e 100644 --- a/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.serverIslands.test.ts +++ b/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.serverIslands.test.ts @@ -33,7 +33,6 @@ test.describe('tracing in static routes with server islands', () => { data: expect.objectContaining({ 'sentry.op': 'pageload', 'sentry.origin': 'auto.pageload.browser', - 'sentry.sample_rate': 1, 'sentry.source': 'url', }), op: 'pageload', @@ -75,7 +74,6 @@ test.describe('tracing in static routes with server islands', () => { data: expect.objectContaining({ 'sentry.op': 'http.server', 'sentry.origin': 'auto.http.astro', - 'sentry.sample_rate': 1, 'sentry.source': 'route', }), op: 'http.server', diff --git a/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.static.test.ts b/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.static.test.ts index 9c202da53542..9db35c72a47d 100644 --- a/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.static.test.ts +++ b/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.static.test.ts @@ -36,7 +36,6 @@ test.describe('tracing in static/pre-rendered routes', () => { data: expect.objectContaining({ 'sentry.op': 'pageload', 'sentry.origin': 'auto.pageload.browser', - 'sentry.sample_rate': 1, 'sentry.source': 'url', }), op: 'pageload', diff --git a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/propagation.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/propagation.test.ts index ecc67e26ab16..d80c3bebdf79 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/propagation.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/propagation.test.ts @@ -89,7 +89,6 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => { 'sentry.source': 'route', 'sentry.origin': 'auto.http.otel.http', 'sentry.op': 'http.server', - 'sentry.sample_rate': 1, url: `http://localhost:3030/test-inbound-headers/${id}`, 'otel.kind': 'SERVER', 'http.response.status_code': 200, @@ -171,7 +170,6 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { 'sentry.source': 'route', 'sentry.origin': 'auto.http.otel.http', 'sentry.op': 'http.server', - 'sentry.sample_rate': 1, url: `http://localhost:3030/test-outgoing-fetch/${id}`, 'otel.kind': 'SERVER', 'http.response.status_code': 200, @@ -204,7 +202,6 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { 'sentry.source': 'route', 'sentry.origin': 'auto.http.otel.http', 'sentry.op': 'http.server', - 'sentry.sample_rate': 1, url: `http://localhost:3030/test-inbound-headers/${id}`, 'otel.kind': 'SERVER', 'http.response.status_code': 200, diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts index fa5cd062d862..5656ce0e5e57 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts @@ -36,7 +36,6 @@ test('Sends a pageload transaction', async ({ page }) => { data: expect.objectContaining({ 'sentry.op': 'pageload', 'sentry.origin': 'auto.pageload.nextjs.app_router_instrumentation', - 'sentry.sample_rate': 1, 'sentry.source': 'url', }), }, diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-5/tests/propagation.test.ts b/dev-packages/e2e-tests/test-applications/node-fastify-5/tests/propagation.test.ts index 7e059b99354b..f7fb1cdcdb59 100644 --- a/dev-packages/e2e-tests/test-applications/node-fastify-5/tests/propagation.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-fastify-5/tests/propagation.test.ts @@ -89,7 +89,6 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => { 'sentry.source': 'route', 'sentry.origin': 'auto.http.otel.http', 'sentry.op': 'http.server', - 'sentry.sample_rate': 1, url: `http://localhost:3030/test-inbound-headers/${id}`, 'otel.kind': 'SERVER', 'http.response.status_code': 200, @@ -171,7 +170,6 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { 'sentry.source': 'route', 'sentry.origin': 'auto.http.otel.http', 'sentry.op': 'http.server', - 'sentry.sample_rate': 1, url: `http://localhost:3030/test-outgoing-fetch/${id}`, 'otel.kind': 'SERVER', 'http.response.status_code': 200, diff --git a/dev-packages/e2e-tests/test-applications/node-fastify/tests/propagation.test.ts b/dev-packages/e2e-tests/test-applications/node-fastify/tests/propagation.test.ts index 7e4aeee0f220..e84fe2db06d4 100644 --- a/dev-packages/e2e-tests/test-applications/node-fastify/tests/propagation.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-fastify/tests/propagation.test.ts @@ -89,7 +89,6 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => { 'sentry.source': 'route', 'sentry.origin': 'auto.http.otel.http', 'sentry.op': 'http.server', - 'sentry.sample_rate': 1, url: `http://localhost:3030/test-inbound-headers/${id}`, 'otel.kind': 'SERVER', 'http.response.status_code': 200, @@ -171,7 +170,6 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { 'sentry.source': 'route', 'sentry.origin': 'auto.http.otel.http', 'sentry.op': 'http.server', - 'sentry.sample_rate': 1, url: `http://localhost:3030/test-outgoing-fetch/${id}`, 'otel.kind': 'SERVER', 'http.response.status_code': 200, diff --git a/dev-packages/e2e-tests/test-applications/node-koa/tests/propagation.test.ts b/dev-packages/e2e-tests/test-applications/node-koa/tests/propagation.test.ts index 0ff946d07d3d..00fa3ecfca9d 100644 --- a/dev-packages/e2e-tests/test-applications/node-koa/tests/propagation.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-koa/tests/propagation.test.ts @@ -89,7 +89,6 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => { 'sentry.source': 'route', 'sentry.origin': 'auto.http.otel.http', 'sentry.op': 'http.server', - 'sentry.sample_rate': 1, url: `http://localhost:3030/test-inbound-headers/${id}`, 'otel.kind': 'SERVER', 'http.response.status_code': 200, @@ -204,7 +203,6 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { 'sentry.source': 'route', 'sentry.origin': 'auto.http.otel.http', 'sentry.op': 'http.server', - 'sentry.sample_rate': 1, url: `http://localhost:3030/test-inbound-headers/${id}`, 'otel.kind': 'SERVER', 'http.response.status_code': 200, From f5a8220d35c0a78b49e26bec4ecae9e06169e9c5 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 21 Jan 2025 13:35:23 +0000 Subject: [PATCH 44/45] . --- .../nestjs-distributed-tracing/tests/events.test.ts | 2 +- .../nestjs-distributed-tracing/tests/propagation.test.ts | 2 ++ .../test-applications/node-fastify-5/tests/propagation.test.ts | 2 +- .../test-applications/node-fastify/tests/propagation.test.ts | 2 +- 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/events.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/events.test.ts index 227851d458cf..d196c79766be 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/events.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/events.test.ts @@ -32,8 +32,8 @@ test('Event emitter', async () => { trace_id: expect.stringMatching(/[a-f0-9]{32}/), data: { 'sentry.source': 'custom', - 'sentry.sample_rate': 1, 'sentry.op': 'event.nestjs', + 'sentry.sample_rate': 1, 'sentry.origin': 'auto.event.nestjs', }, origin: 'auto.event.nestjs', diff --git a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/propagation.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/propagation.test.ts index d80c3bebdf79..176891b95cca 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/propagation.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/propagation.test.ts @@ -88,6 +88,7 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => { data: { 'sentry.source': 'route', 'sentry.origin': 'auto.http.otel.http', + 'sentry.sample_rate': 1, 'sentry.op': 'http.server', url: `http://localhost:3030/test-inbound-headers/${id}`, 'otel.kind': 'SERVER', @@ -170,6 +171,7 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { 'sentry.source': 'route', 'sentry.origin': 'auto.http.otel.http', 'sentry.op': 'http.server', + 'sentry.sample_rate': 1, url: `http://localhost:3030/test-outgoing-fetch/${id}`, 'otel.kind': 'SERVER', 'http.response.status_code': 200, diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-5/tests/propagation.test.ts b/dev-packages/e2e-tests/test-applications/node-fastify-5/tests/propagation.test.ts index f7fb1cdcdb59..1b4e0f92a97a 100644 --- a/dev-packages/e2e-tests/test-applications/node-fastify-5/tests/propagation.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-fastify-5/tests/propagation.test.ts @@ -170,6 +170,7 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { 'sentry.source': 'route', 'sentry.origin': 'auto.http.otel.http', 'sentry.op': 'http.server', + 'sentry.sample_rate': 1, url: `http://localhost:3030/test-outgoing-fetch/${id}`, 'otel.kind': 'SERVER', 'http.response.status_code': 200, @@ -202,7 +203,6 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { 'sentry.source': 'route', 'sentry.origin': 'auto.http.otel.http', 'sentry.op': 'http.server', - 'sentry.sample_rate': 1, url: `http://localhost:3030/test-inbound-headers/${id}`, 'otel.kind': 'SERVER', 'http.response.status_code': 200, diff --git a/dev-packages/e2e-tests/test-applications/node-fastify/tests/propagation.test.ts b/dev-packages/e2e-tests/test-applications/node-fastify/tests/propagation.test.ts index e84fe2db06d4..af2cfddded9a 100644 --- a/dev-packages/e2e-tests/test-applications/node-fastify/tests/propagation.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-fastify/tests/propagation.test.ts @@ -170,6 +170,7 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { 'sentry.source': 'route', 'sentry.origin': 'auto.http.otel.http', 'sentry.op': 'http.server', + 'sentry.sample_rate': 1, url: `http://localhost:3030/test-outgoing-fetch/${id}`, 'otel.kind': 'SERVER', 'http.response.status_code': 200, @@ -202,7 +203,6 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { 'sentry.source': 'route', 'sentry.origin': 'auto.http.otel.http', 'sentry.op': 'http.server', - 'sentry.sample_rate': 1, url: `http://localhost:3030/test-inbound-headers/${id}`, 'otel.kind': 'SERVER', 'http.response.status_code': 200, From f683884123cafc7d28158531191a21fe4fa345b0 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 21 Jan 2025 13:46:54 +0000 Subject: [PATCH 45/45] . --- .../nestjs-distributed-tracing/tests/events.test.ts | 1 - .../nestjs-distributed-tracing/tests/propagation.test.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/events.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/events.test.ts index d196c79766be..ed4a36303efa 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/events.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/events.test.ts @@ -33,7 +33,6 @@ test('Event emitter', async () => { data: { 'sentry.source': 'custom', 'sentry.op': 'event.nestjs', - 'sentry.sample_rate': 1, 'sentry.origin': 'auto.event.nestjs', }, origin: 'auto.event.nestjs', diff --git a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/propagation.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/propagation.test.ts index 176891b95cca..040d5d326809 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/propagation.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-distributed-tracing/tests/propagation.test.ts @@ -88,7 +88,6 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => { data: { 'sentry.source': 'route', 'sentry.origin': 'auto.http.otel.http', - 'sentry.sample_rate': 1, 'sentry.op': 'http.server', url: `http://localhost:3030/test-inbound-headers/${id}`, 'otel.kind': 'SERVER',