Skip to content

Commit

Permalink
ref(core): Ensure non-recording root spans have frozen DSC (#14964)
Browse files Browse the repository at this point in the history
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
#14955
  • Loading branch information
mydea authored Jan 10, 2025
1 parent 3ea500f commit 1d98867
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 4 deletions.
14 changes: 12 additions & 2 deletions packages/core/src/tracing/idleSpan.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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';
Expand Down Expand Up @@ -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<DynamicSamplingContext>;
freezeDscOnSpan(span, dsc);

return span;
}

const scope = getCurrentScope();
Expand Down
25 changes: 23 additions & 2 deletions packages/core/src/tracing/trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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<DynamicSamplingContext>;
freezeDscOnSpan(span, dsc);
}

return span;
}

const isolationScope = getIsolationScope();
Expand Down
9 changes: 9 additions & 0 deletions packages/core/test/lib/tracing/idleSpan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
getActiveSpan,
getClient,
getCurrentScope,
getDynamicSamplingContextFromSpan,
getGlobalScope,
getIsolationScope,
setCurrentClient,
Expand Down Expand Up @@ -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);
Expand Down
22 changes: 22 additions & 0 deletions packages/core/test/lib/tracing/trace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { getAsyncContextStrategy } from '../../../src/asyncContext';
import {
SentrySpan,
continueTrace,
getDynamicSamplingContextFromSpan,
registerSpanErrorInstrumentation,
startInactiveSpan,
startSpan,
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down

0 comments on commit 1d98867

Please sign in to comment.