From ebcf7bfe6f83ba3ecf0bbdc8ae572c97c53cdd25 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 9 Jan 2025 13:25:29 +0000 Subject: [PATCH 01/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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 631ed1b89d5260805263d8e26028d0e64572c514 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 15 Jan 2025 10:14:27 +0000 Subject: [PATCH 15/17] 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 8afa5b301e9af1d8e22698b91886e4d344b05428 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 16 Jan 2025 09:35:53 +0000 Subject: [PATCH 16/17] Review feedback --- .../suites/sample-rand-propagation/test.ts | 2 ++ packages/core/src/tracing/sampling.ts | 4 ++-- packages/core/src/utils-hoist/tracing.ts | 6 ++++-- 3 files changed, 8 insertions(+), 4 deletions(-) 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 0253bfddd843..ed8c0a6afa49 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 @@ -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).toBeLessThan(1); }); 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/packages/core/src/tracing/sampling.ts b/packages/core/src/tracing/sampling.ts index 2eb8e2fbbd7c..82a0ce91d9dd 100644 --- a/packages/core/src/tracing/sampling.ts +++ b/packages/core/src/tracing/sampling.ts @@ -57,8 +57,8 @@ export function sampleSpan( return [false, parsedSampleRate]; } - // 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. + // We always compare the sample rand for the current execution context against the chosen sample rate. + // Read more: https://develop.sentry.dev/sdk/telemetry/traces/#propagated-random-value const shouldSample = sampleRand < parsedSampleRate; // if we're not going to keep it, we're done diff --git a/packages/core/src/utils-hoist/tracing.ts b/packages/core/src/utils-hoist/tracing.ts index 3fa4d9677c02..35b71fda472a 100644 --- a/packages/core/src/utils-hoist/tracing.ts +++ b/packages/core/src/utils-hoist/tracing.ts @@ -76,7 +76,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: sampleRand, + sampleRand, }; } @@ -97,6 +97,8 @@ export function generateSentryTraceHeader( /** * Given any combination of an incoming trace, generate a sample rand based on its defined semantics. + * + * Read more: https://develop.sentry.dev/sdk/telemetry/traces/#propagated-random-value */ function getSampleRandFromTraceparentAndDsc( traceparentData: TraceparentData | undefined, @@ -114,7 +116,7 @@ function getSampleRandFromTraceparentAndDsc( 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] + : // Returns a sample rand with negative sampling decision [sampleRate, 1) parsedSampleRate + Math.random() * (1 - parsedSampleRate); } else { // If nothing applies, return a random sample rand. From 9543126f70554ecda761053e8f5039e81eae5d6c Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 16 Jan 2025 10:38:35 +0000 Subject: [PATCH 17/17] . --- .../suites/sample-rand-propagation/test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 ed8c0a6afa49..42e6a0a5e555 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 @@ -58,7 +58,7 @@ describe('sample_rand propagation', () => { expect(sampleRand).toStrictEqual(expect.any(Number)); expect(sampleRand).not.toBeNaN(); expect(sampleRand).toBeLessThan(0.25); - expect(sampleRand).toBeLessThan(1); + 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 () => {