From 1d98867d85d578ce8cb09f14e47b362fff33d033 Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Fri, 10 Jan 2025 10:48:20 +0100 Subject: [PATCH] ref(core): Ensure non-recording root spans have frozen DSC (#14964) Otherwise, parts of the DSC will be missing - we try to make it as complete as we can. Since the span cannot be updated anyhow (e.g. the name cannot be changed), we can safely freeze this at this time. Extracted out of https://github.com/getsentry/sentry-javascript/pull/14955 --- packages/core/src/tracing/idleSpan.ts | 14 +++++++++-- packages/core/src/tracing/trace.ts | 25 +++++++++++++++++-- .../core/test/lib/tracing/idleSpan.test.ts | 9 +++++++ packages/core/test/lib/tracing/trace.test.ts | 22 ++++++++++++++++ 4 files changed, 66 insertions(+), 4 deletions(-) diff --git a/packages/core/src/tracing/idleSpan.ts b/packages/core/src/tracing/idleSpan.ts index c37ef7b388a1..41e336677d2b 100644 --- a/packages/core/src/tracing/idleSpan.ts +++ b/packages/core/src/tracing/idleSpan.ts @@ -1,5 +1,5 @@ import { getClient, getCurrentScope } from '../currentScopes'; -import type { Span, StartSpanOptions } from '../types-hoist'; +import type { DynamicSamplingContext, Span, StartSpanOptions } from '../types-hoist'; import { DEBUG_BUILD } from '../debug-build'; import { SEMANTIC_ATTRIBUTE_SENTRY_IDLE_SPAN_FINISH_REASON } from '../semanticAttributes'; @@ -14,6 +14,7 @@ import { spanTimeInputToSeconds, spanToJSON, } from '../utils/spanUtils'; +import { freezeDscOnSpan, getDynamicSamplingContextFromSpan } from './dynamicSamplingContext'; import { SentryNonRecordingSpan } from './sentryNonRecordingSpan'; import { SPAN_STATUS_ERROR } from './spanstatus'; import { startInactiveSpan } from './trace'; @@ -109,7 +110,16 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti const client = getClient(); if (!client || !hasTracingEnabled()) { - return new SentryNonRecordingSpan(); + const span = new SentryNonRecordingSpan(); + + const dsc = { + sample_rate: '0', + sampled: 'false', + ...getDynamicSamplingContextFromSpan(span), + } satisfies Partial; + freezeDscOnSpan(span, dsc); + + return span; } const scope = getCurrentScope(); diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index 09d27ff43e22..28b9654d5266 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -2,7 +2,14 @@ import type { AsyncContextStrategy } from '../asyncContext/types'; import { getMainCarrier } from '../carrier'; -import type { ClientOptions, SentrySpanArguments, Span, SpanTimeInput, StartSpanOptions } from '../types-hoist'; +import type { + ClientOptions, + DynamicSamplingContext, + SentrySpanArguments, + Span, + SpanTimeInput, + StartSpanOptions, +} from '../types-hoist'; import { getClient, getCurrentScope, getIsolationScope, withScope } from '../currentScopes'; @@ -284,7 +291,21 @@ function createChildOrRootSpan({ scope: Scope; }): Span { if (!hasTracingEnabled()) { - return new SentryNonRecordingSpan(); + const span = new SentryNonRecordingSpan(); + + // If this is a root span, we ensure to freeze a DSC + // So we can have at least partial data here + if (forceTransaction || !parentSpan) { + const dsc = { + sampled: 'false', + sample_rate: '0', + transaction: spanArguments.name, + ...getDynamicSamplingContextFromSpan(span), + } satisfies Partial; + freezeDscOnSpan(span, dsc); + } + + return span; } const isolationScope = getIsolationScope(); diff --git a/packages/core/test/lib/tracing/idleSpan.test.ts b/packages/core/test/lib/tracing/idleSpan.test.ts index 3d720f383173..5280be561067 100644 --- a/packages/core/test/lib/tracing/idleSpan.test.ts +++ b/packages/core/test/lib/tracing/idleSpan.test.ts @@ -7,6 +7,7 @@ import { getActiveSpan, getClient, getCurrentScope, + getDynamicSamplingContextFromSpan, getGlobalScope, getIsolationScope, setCurrentClient, @@ -60,6 +61,14 @@ describe('startIdleSpan', () => { const idleSpan = startIdleSpan({ name: 'foo' }); expect(idleSpan).toBeDefined(); expect(idleSpan).toBeInstanceOf(SentryNonRecordingSpan); + // DSC is still correctly set on the span + expect(getDynamicSamplingContextFromSpan(idleSpan)).toEqual({ + environment: 'production', + public_key: '123', + sample_rate: '0', + sampled: 'false', + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }); // not set as active span, though expect(getActiveSpan()).toBe(undefined); diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index e0d7bb3b555f..e545e814f81d 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -15,6 +15,7 @@ import { getAsyncContextStrategy } from '../../../src/asyncContext'; import { SentrySpan, continueTrace, + getDynamicSamplingContextFromSpan, registerSpanErrorInstrumentation, startInactiveSpan, startSpan, @@ -217,6 +218,13 @@ describe('startSpan', () => { expect(span).toBeDefined(); expect(span).toBeInstanceOf(SentryNonRecordingSpan); + expect(getDynamicSamplingContextFromSpan(span)).toEqual({ + environment: 'production', + sample_rate: '0', + sampled: 'false', + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + transaction: 'GET users/[id]', + }); }); it('creates & finishes span', async () => { @@ -633,6 +641,13 @@ describe('startSpanManual', () => { expect(span).toBeDefined(); expect(span).toBeInstanceOf(SentryNonRecordingSpan); + expect(getDynamicSamplingContextFromSpan(span)).toEqual({ + environment: 'production', + sample_rate: '0', + sampled: 'false', + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + transaction: 'GET users/[id]', + }); }); it('creates & finishes span', async () => { @@ -971,6 +986,13 @@ describe('startInactiveSpan', () => { expect(span).toBeDefined(); expect(span).toBeInstanceOf(SentryNonRecordingSpan); + expect(getDynamicSamplingContextFromSpan(span)).toEqual({ + environment: 'production', + sample_rate: '0', + sampled: 'false', + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + transaction: 'GET users/[id]', + }); }); it('creates & finishes span', async () => {