From c2cb28d3dd7fc0a6990f59cf991e69861f647336 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 16 Apr 2024 09:15:50 +0200 Subject: [PATCH 1/2] WIP: skip next emitted spans?? --- packages/opentelemetry/src/sampler.ts | 43 ++++++++++++++++++--------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/packages/opentelemetry/src/sampler.ts b/packages/opentelemetry/src/sampler.ts index 024f802c5ae6..774105eb0e05 100644 --- a/packages/opentelemetry/src/sampler.ts +++ b/packages/opentelemetry/src/sampler.ts @@ -52,6 +52,13 @@ export class SentrySampler implements Sampler { return { decision: SamplingDecision.NOT_RECORD, traceState }; } + // If we encounter a span emitted by Next.js, we do not want to sample it + // The reason for this is that the data quality of the spans varies, it is different per version of Next, + // and we need to keep our manual instrumentation around for the edge runtime anyhow. + if (spanAttributes['next.span_type']) { + return { decision: SamplingDecision.NOT_RECORD, traceState: traceState }; + } + // If we have a http.client span that has no local parent, we never want to sample it // but we want to leave downstream sampling decisions up to the server if ( @@ -62,20 +69,7 @@ export class SentrySampler implements Sampler { return { decision: SamplingDecision.NOT_RECORD, traceState }; } - let parentSampled: boolean | undefined = undefined; - - // Only inherit sample rate if `traceId` is the same - // Note for testing: `isSpanContextValid()` checks the format of the traceId/spanId, so we need to pass valid ones - if (parentSpan && parentContext && isSpanContextValid(parentContext) && parentContext.traceId === traceId) { - if (parentContext.isRemote) { - parentSampled = getParentRemoteSampled(parentSpan); - DEBUG_BUILD && - logger.log(`[Tracing] Inheriting remote parent's sampled decision for ${spanName}: ${parentSampled}`); - } else { - parentSampled = getSamplingDecision(parentContext); - DEBUG_BUILD && logger.log(`[Tracing] Inheriting parent's sampled decision for ${spanName}: ${parentSampled}`); - } - } + const parentSampled = parentSpan ? getParentSampled(parentSpan, traceId, spanName) : undefined; const [sampled, sampleRate] = sampleSpan(options, { name: spanName, @@ -129,3 +123,24 @@ function getParentRemoteSampled(parentSpan: Span): boolean | undefined { // Only inherit sampled if `traceId` is the same return traceparentData && traceId === traceparentData.traceId ? traceparentData.sampled : undefined; } + +function getParentSampled(parentSpan: Span, traceId: string, spanName: string): boolean | undefined { + const parentContext = parentSpan.spanContext(); + + // Only inherit sample rate if `traceId` is the same + // Note for testing: `isSpanContextValid()` checks the format of the traceId/spanId, so we need to pass valid ones + if (isSpanContextValid(parentContext) && parentContext.traceId === traceId) { + if (parentContext.isRemote) { + const parentSampled = getParentRemoteSampled(parentSpan); + DEBUG_BUILD && + logger.log(`[Tracing] Inheriting remote parent's sampled decision for ${spanName}: ${parentSampled}`); + return parentSampled; + } + + const parentSampled = getSamplingDecision(parentContext); + DEBUG_BUILD && logger.log(`[Tracing] Inheriting parent's sampled decision for ${spanName}: ${parentSampled}`); + return parentSampled; + } + + return undefined; +} From b078f9b872dc3774ffae52db24df632f330d1802 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 16 Apr 2024 13:16:24 +0200 Subject: [PATCH 2/2] sample next spans after we have non-next instrumentation --- packages/opentelemetry/src/sampler.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/opentelemetry/src/sampler.ts b/packages/opentelemetry/src/sampler.ts index 774105eb0e05..17490686b5bd 100644 --- a/packages/opentelemetry/src/sampler.ts +++ b/packages/opentelemetry/src/sampler.ts @@ -52,13 +52,6 @@ export class SentrySampler implements Sampler { return { decision: SamplingDecision.NOT_RECORD, traceState }; } - // If we encounter a span emitted by Next.js, we do not want to sample it - // The reason for this is that the data quality of the spans varies, it is different per version of Next, - // and we need to keep our manual instrumentation around for the edge runtime anyhow. - if (spanAttributes['next.span_type']) { - return { decision: SamplingDecision.NOT_RECORD, traceState: traceState }; - } - // If we have a http.client span that has no local parent, we never want to sample it // but we want to leave downstream sampling decisions up to the server if ( @@ -71,6 +64,14 @@ export class SentrySampler implements Sampler { const parentSampled = parentSpan ? getParentSampled(parentSpan, traceId, spanName) : undefined; + // If we encounter a span emitted by Next.js, we do not want to sample it + // The reason for this is that the data quality of the spans varies, it is different per version of Next, + // and we need to keep our manual instrumentation around for the edge runtime anyhow. + // BUT we only do this if we don't have a parent span with a sampling decision yet + if (spanAttributes['next.span_type'] && typeof parentSampled !== 'boolean') { + return { decision: SamplingDecision.NOT_RECORD, traceState: traceState }; + } + const [sampled, sampleRate] = sampleSpan(options, { name: spanName, attributes: spanAttributes,