From b99c2a3d05e227fe708c2cc3a0ac87455a3afe78 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 27 Jun 2024 13:24:12 +0200 Subject: [PATCH 1/3] feat(opentelemetry): Expose sampling helpers When users want to use a custom sampler, they can use these new helpers to still have sentry working nicely with whatever they decide to do in there. For e.g. trace propagation etc. to work correctly with Sentry, we need to attach some things to trace state etc. These helpers encapsulate this for the user, while still allowing them to decide however they want if the span should be sampled or not. --- packages/opentelemetry/src/index.ts | 7 ++- packages/opentelemetry/src/sampler.ts | 79 ++++++++++++++++++++------- 2 files changed, 66 insertions(+), 20 deletions(-) diff --git a/packages/opentelemetry/src/index.ts b/packages/opentelemetry/src/index.ts index 5e99f7eb8c33..439461f056fc 100644 --- a/packages/opentelemetry/src/index.ts +++ b/packages/opentelemetry/src/index.ts @@ -36,7 +36,12 @@ export { setOpenTelemetryContextAsyncContextStrategy } from './asyncContextStrat export { wrapContextManagerClass } from './contextManager'; export { SentryPropagator } from './propagator'; export { SentrySpanProcessor } from './spanProcessor'; -export { SentrySampler } from './sampler'; +export { + SentrySampler, + sentrySamplerNoDecision, + sentrySamplerNotSampled, + sentrySamplerSampled, +} from './sampler'; export { openTelemetrySetupCheck } from './utils/setupCheck'; diff --git a/packages/opentelemetry/src/sampler.ts b/packages/opentelemetry/src/sampler.ts index 96342503e72a..249ae17bb4b9 100644 --- a/packages/opentelemetry/src/sampler.ts +++ b/packages/opentelemetry/src/sampler.ts @@ -1,4 +1,4 @@ -import type { Attributes, Context, Span } from '@opentelemetry/api'; +import type { Attributes, Context, Span , TraceState as TraceStateInterface} from '@opentelemetry/api'; import { SpanKind } from '@opentelemetry/api'; import { isSpanContextValid, trace } from '@opentelemetry/api'; import { TraceState } from '@opentelemetry/core'; @@ -40,16 +40,8 @@ export class SentrySampler implements Sampler { const parentSpan = trace.getSpan(context); const parentContext = parentSpan?.spanContext(); - let traceState = parentContext?.traceState || new TraceState(); - - // We always keep the URL on the trace state, so we can access it in the propagator - const url = spanAttributes[SEMATTRS_HTTP_URL]; - if (url && typeof url === 'string') { - traceState = traceState.set(SENTRY_TRACE_STATE_URL, url); - } - if (!hasTracingEnabled(options)) { - return { decision: SamplingDecision.NOT_RECORD, traceState }; + return sentrySamplerNoDecision({ context, spanAttributes }); } // If we have a http.client span that has no local parent, we never want to sample it @@ -59,7 +51,7 @@ export class SentrySampler implements Sampler { spanAttributes[SEMATTRS_HTTP_METHOD] && (!parentSpan || parentContext?.isRemote) ) { - return { decision: SamplingDecision.NOT_RECORD, traceState }; + return sentrySamplerNoDecision({ context, spanAttributes }); } const parentSampled = parentSpan ? getParentSampled(parentSpan, traceId, spanName) : undefined; @@ -76,7 +68,7 @@ export class SentrySampler implements Sampler { mutableSamplingDecision, ); if (!mutableSamplingDecision.decision) { - return { decision: SamplingDecision.NOT_RECORD, traceState: traceState }; + return sentrySamplerNoDecision({ context, spanAttributes }); } const [sampled, sampleRate] = sampleSpan(options, { @@ -96,25 +88,22 @@ export class SentrySampler implements Sampler { const method = `${spanAttributes[SEMATTRS_HTTP_METHOD]}`.toUpperCase(); if (method === 'OPTIONS' || method === 'HEAD') { DEBUG_BUILD && logger.log(`[Tracing] Not sampling span because HTTP method is '${method}' for ${spanName}`); + return { - decision: SamplingDecision.NOT_RECORD, + ...sentrySamplerNotSampled({ context, spanAttributes }), attributes, - traceState: traceState.set(SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING, '1'), }; } if (!sampled) { return { - decision: SamplingDecision.NOT_RECORD, + ...sentrySamplerNotSampled({ context, spanAttributes }), attributes, - traceState: traceState.set(SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING, '1'), }; } - return { - decision: SamplingDecision.RECORD_AND_SAMPLED, + ...sentrySamplerSampled({ context, spanAttributes }), attributes, - traceState, }; } @@ -152,3 +141,55 @@ function getParentSampled(parentSpan: Span, traceId: string, spanName: string): return undefined; } + +/** + * Returns a SamplingResult that indicates that a span was not sampled, but no definite decision was made yet. + * This indicates to downstream SDKs that they may make their own decision. + */ +export function sentrySamplerNoDecision({ + context, + spanAttributes, +}: { context: Context; spanAttributes: SpanAttributes }): SamplingResult { + const traceState = getBaseTraceState(context, spanAttributes); + + return { decision: SamplingDecision.NOT_RECORD, traceState }; +} + +/** + * Returns a SamplingResult that indicates that a span was not sampled. + */ +export function sentrySamplerNotSampled({ + context, + spanAttributes, +}: { context: Context; spanAttributes: SpanAttributes }): SamplingResult { + const traceState = getBaseTraceState(context, spanAttributes).set(SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING, '1'); + + return { decision: SamplingDecision.NOT_RECORD, traceState }; +} + +/** + * Returns a SamplingResult that indicates that a span was sampled. + */ +export function sentrySamplerSampled({ + context, + spanAttributes, +}: { context: Context; spanAttributes: SpanAttributes }): SamplingResult { + const traceState = getBaseTraceState(context, spanAttributes); + + return { decision: SamplingDecision.RECORD_AND_SAMPLED, traceState }; +} + +function getBaseTraceState(context: Context, spanAttributes: SpanAttributes): TraceStateInterface { + const parentSpan = trace.getSpan(context); + const parentContext = parentSpan?.spanContext(); + + let traceState = parentContext?.traceState || new TraceState(); + + // We always keep the URL on the trace state, so we can access it in the propagator + const url = spanAttributes[SEMATTRS_HTTP_URL]; + if (url && typeof url === 'string') { + traceState = traceState.set(SENTRY_TRACE_STATE_URL, url); + } + + return traceState; +} From 46bb0c7465340574458b1b73659e7c2187492396 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 27 Jun 2024 14:17:17 +0200 Subject: [PATCH 2/3] fix prettier --- packages/opentelemetry/src/sampler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry/src/sampler.ts b/packages/opentelemetry/src/sampler.ts index 249ae17bb4b9..bf8240d37476 100644 --- a/packages/opentelemetry/src/sampler.ts +++ b/packages/opentelemetry/src/sampler.ts @@ -1,4 +1,4 @@ -import type { Attributes, Context, Span , TraceState as TraceStateInterface} from '@opentelemetry/api'; +import type { Attributes, Context, Span, TraceState as TraceStateInterface } from '@opentelemetry/api'; import { SpanKind } from '@opentelemetry/api'; import { isSpanContextValid, trace } from '@opentelemetry/api'; import { TraceState } from '@opentelemetry/core'; From 235620f16d9439a1828d9d90c13218f72ea1e48a Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 2 Jul 2024 16:43:23 +0200 Subject: [PATCH 3/3] ref: rewrite to `wrapSamplingDecision` --- packages/opentelemetry/src/index.ts | 4 +- packages/opentelemetry/src/sampler.ts | 54 +++++++++++---------------- 2 files changed, 22 insertions(+), 36 deletions(-) diff --git a/packages/opentelemetry/src/index.ts b/packages/opentelemetry/src/index.ts index 439461f056fc..e262247bce1e 100644 --- a/packages/opentelemetry/src/index.ts +++ b/packages/opentelemetry/src/index.ts @@ -38,9 +38,7 @@ export { SentryPropagator } from './propagator'; export { SentrySpanProcessor } from './spanProcessor'; export { SentrySampler, - sentrySamplerNoDecision, - sentrySamplerNotSampled, - sentrySamplerSampled, + wrapSamplingDecision, } from './sampler'; export { openTelemetrySetupCheck } from './utils/setupCheck'; diff --git a/packages/opentelemetry/src/sampler.ts b/packages/opentelemetry/src/sampler.ts index bf8240d37476..0a7d2f9af972 100644 --- a/packages/opentelemetry/src/sampler.ts +++ b/packages/opentelemetry/src/sampler.ts @@ -41,7 +41,7 @@ export class SentrySampler implements Sampler { const parentContext = parentSpan?.spanContext(); if (!hasTracingEnabled(options)) { - return sentrySamplerNoDecision({ context, spanAttributes }); + return wrapSamplingDecision({ decision: undefined, context, spanAttributes }); } // If we have a http.client span that has no local parent, we never want to sample it @@ -51,7 +51,7 @@ export class SentrySampler implements Sampler { spanAttributes[SEMATTRS_HTTP_METHOD] && (!parentSpan || parentContext?.isRemote) ) { - return sentrySamplerNoDecision({ context, spanAttributes }); + return wrapSamplingDecision({ decision: undefined, context, spanAttributes }); } const parentSampled = parentSpan ? getParentSampled(parentSpan, traceId, spanName) : undefined; @@ -68,7 +68,7 @@ export class SentrySampler implements Sampler { mutableSamplingDecision, ); if (!mutableSamplingDecision.decision) { - return sentrySamplerNoDecision({ context, spanAttributes }); + return wrapSamplingDecision({ decision: undefined, context, spanAttributes }); } const [sampled, sampleRate] = sampleSpan(options, { @@ -90,19 +90,19 @@ export class SentrySampler implements Sampler { DEBUG_BUILD && logger.log(`[Tracing] Not sampling span because HTTP method is '${method}' for ${spanName}`); return { - ...sentrySamplerNotSampled({ context, spanAttributes }), + ...wrapSamplingDecision({ decision: SamplingDecision.NOT_RECORD, context, spanAttributes }), attributes, }; } if (!sampled) { return { - ...sentrySamplerNotSampled({ context, spanAttributes }), + ...wrapSamplingDecision({ decision: SamplingDecision.NOT_RECORD, context, spanAttributes }), attributes, }; } return { - ...sentrySamplerSampled({ context, spanAttributes }), + ...wrapSamplingDecision({ decision: SamplingDecision.RECORD_AND_SAMPLED, context, spanAttributes }), attributes, }; } @@ -143,40 +143,28 @@ function getParentSampled(parentSpan: Span, traceId: string, spanName: string): } /** - * Returns a SamplingResult that indicates that a span was not sampled, but no definite decision was made yet. - * This indicates to downstream SDKs that they may make their own decision. + * Wrap a sampling decision with data that Sentry needs to work properly with it. + * If you pass `decision: undefined`, it will be treated as `NOT_RECORDING`, but in contrast to passing `NOT_RECORDING` + * it will not propagate this decision to downstream Sentry SDKs. */ -export function sentrySamplerNoDecision({ +export function wrapSamplingDecision({ + decision, context, spanAttributes, -}: { context: Context; spanAttributes: SpanAttributes }): SamplingResult { +}: { decision: SamplingDecision | undefined; context: Context; spanAttributes: SpanAttributes }): SamplingResult { const traceState = getBaseTraceState(context, spanAttributes); - return { decision: SamplingDecision.NOT_RECORD, traceState }; -} - -/** - * Returns a SamplingResult that indicates that a span was not sampled. - */ -export function sentrySamplerNotSampled({ - context, - spanAttributes, -}: { context: Context; spanAttributes: SpanAttributes }): SamplingResult { - const traceState = getBaseTraceState(context, spanAttributes).set(SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING, '1'); - - return { decision: SamplingDecision.NOT_RECORD, traceState }; -} + // If the decision is undefined, we treat it as NOT_RECORDING, but we don't propagate this decision to downstream SDKs + // Which is done by not setting `SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING` traceState + if (decision == undefined) { + return { decision: SamplingDecision.NOT_RECORD, traceState }; + } -/** - * Returns a SamplingResult that indicates that a span was sampled. - */ -export function sentrySamplerSampled({ - context, - spanAttributes, -}: { context: Context; spanAttributes: SpanAttributes }): SamplingResult { - const traceState = getBaseTraceState(context, spanAttributes); + if (decision === SamplingDecision.NOT_RECORD) { + return { decision, traceState: traceState.set(SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING, '1') }; + } - return { decision: SamplingDecision.RECORD_AND_SAMPLED, traceState }; + return { decision, traceState }; } function getBaseTraceState(context: Context, spanAttributes: SpanAttributes): TraceStateInterface {