From 33385ff511508dbf742160a0c85269393b0e2990 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Fri, 16 May 2025 11:20:21 +0200 Subject: [PATCH 1/4] feat(core): Implement `strictTraceContinuation` --- packages/core/src/tracing/trace.ts | 43 ++++++ packages/core/src/types-hoist/options.ts | 12 ++ packages/core/test/lib/tracing/trace.test.ts | 146 +++++++++++++++++++ 3 files changed, 201 insertions(+) diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index a96159692ac3..41f3fae23a12 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -16,6 +16,8 @@ import { hasSpansEnabled } from '../utils/hasSpansEnabled'; import { parseSampleRate } from '../utils/parseSampleRate'; import { _getSpanForScope, _setSpanForScope } from '../utils/spanOnScope'; import { addChildSpanToSpan, getRootSpan, spanIsSampled, spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils'; +import { baggageHeaderToDynamicSamplingContext } from '../utils-hoist/baggage'; +import { extractOrgIdFromDsnHost } from '../utils-hoist/dsn'; import { logger } from '../utils-hoist/logger'; import { generateTraceId } from '../utils-hoist/propagationContext'; import { propagationContextFromHeaders } from '../utils-hoist/tracing'; @@ -215,6 +217,47 @@ export const continueTrace = ( } const { sentryTrace, baggage } = options; + const client = getClient(); + + const clientOptions = client?.getOptions(); + const strictTraceContinuation = clientOptions?.strictTraceContinuation || false; // default for `strictTraceContinuation` is `false` todo(v10): set default to `true` + + const incomingDsc = baggageHeaderToDynamicSamplingContext(baggage); + const baggageOrgId = incomingDsc?.org_id; + + let sdkOrgId: string | undefined; + const dsn = client?.getDsn(); + if (dsn?.host) { + sdkOrgId = extractOrgIdFromDsnHost(dsn.host); + } + + const shouldStartNewTrace = (): boolean => { + // Case: baggage org ID and SDK org ID don't match - always start new trace + if (baggageOrgId && sdkOrgId && baggageOrgId !== sdkOrgId) { + DEBUG_BUILD && + logger.info(`Starting a new trace because org IDs don't match (incoming: ${baggageOrgId}, sdk: ${sdkOrgId})`); + return true; + } + + if (strictTraceContinuation) { + // With strict continuation enabled, start new trace if: + // - Baggage has org ID but SDK doesn't have one + // - SDK has org ID but baggage doesn't have one + if ((baggageOrgId && !sdkOrgId) || (!baggageOrgId && sdkOrgId)) { + DEBUG_BUILD && + logger.info( + `Starting a new trace because strict trace continuation is enabled and one org ID is missing (incoming: ${baggageOrgId}, sdk: ${sdkOrgId})`, + ); + return true; + } + } + + return false; + }; + + if (shouldStartNewTrace()) { + return startNewTrace(callback); + } return withScope(scope => { const propagationContext = propagationContextFromHeaders(sentryTrace, baggage); diff --git a/packages/core/src/types-hoist/options.ts b/packages/core/src/types-hoist/options.ts index 09dab550be4c..a308405a7174 100644 --- a/packages/core/src/types-hoist/options.ts +++ b/packages/core/src/types-hoist/options.ts @@ -320,6 +320,18 @@ export interface ClientOptions { expect(result).toEqual('aha'); }); + + describe('strictTraceContinuation', () => { + const creatOrgIdInDsn = (orgId: number) => { + vi.spyOn(client, 'getDsn').mockReturnValue({ + host: `o${orgId}.ingest.sentry.io`, + protocol: 'https', + projectId: 'projId', + }); + }; + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('continues trace when org IDs match', () => { + creatOrgIdInDsn(123); + + const scope = continueTrace( + { + sentryTrace: '12312012123120121231201212312012-1121201211212012-1', + baggage: 'sentry-org_id=123', + }, + () => { + return getCurrentScope(); + }, + ); + + expect(scope.getPropagationContext().traceId).toBe('12312012123120121231201212312012'); + expect(scope.getPropagationContext().parentSpanId).toBe('1121201211212012'); + }); + + it('starts new trace when both SDK and baggage org IDs are set and do not match', () => { + creatOrgIdInDsn(123); + + const scope = continueTrace( + { + sentryTrace: '12312012123120121231201212312012-1121201211212012-1', + baggage: 'sentry-org_id=456', + }, + () => { + return getCurrentScope(); + }, + ); + + // Should start a new trace with a different trace ID + expect(scope.getPropagationContext().traceId).not.toBe('12312012123120121231201212312012'); + expect(scope.getPropagationContext().parentSpanId).toBeUndefined(); + }); + + describe('when strictTraceContinuation is true', () => { + it('starts new trace when baggage org ID is missing', () => { + client.getOptions().strictTraceContinuation = true; + + creatOrgIdInDsn(123); + + const scope = continueTrace( + { + sentryTrace: '12312012123120121231201212312012-1121201211212012-1', + baggage: 'sentry-environment=production', + }, + () => { + return getCurrentScope(); + }, + ); + + // Should start a new trace with a different trace ID + expect(scope.getPropagationContext().traceId).not.toBe('12312012123120121231201212312012'); + expect(scope.getPropagationContext().parentSpanId).toBeUndefined(); + }); + + it('starts new trace when SDK org ID is missing', () => { + client.getOptions().strictTraceContinuation = true; + + const scope = continueTrace( + { + sentryTrace: '12312012123120121231201212312012-1121201211212012-1', + baggage: 'sentry-org_id=123', + }, + () => { + return getCurrentScope(); + }, + ); + + // Should start a new trace with a different trace ID + expect(scope.getPropagationContext().traceId).not.toBe('12312012123120121231201212312012'); + expect(scope.getPropagationContext().parentSpanId).toBeUndefined(); + }); + + it('continues trace when both org IDs are missing', () => { + client.getOptions().strictTraceContinuation = true; + + const scope = continueTrace( + { + sentryTrace: '12312012123120121231201212312012-1121201211212012-1', + baggage: 'sentry-environment=production', + }, + () => { + return getCurrentScope(); + }, + ); + + // Should continue the trace + expect(scope.getPropagationContext().traceId).toBe('12312012123120121231201212312012'); + expect(scope.getPropagationContext().parentSpanId).toBe('1121201211212012'); + }); + }); + + describe('when strictTraceContinuation is false', () => { + it('continues trace when baggage org ID is missing', () => { + client.getOptions().strictTraceContinuation = false; + + creatOrgIdInDsn(123); + + const scope = continueTrace( + { + sentryTrace: '12312012123120121231201212312012-1121201211212012-1', + baggage: 'sentry-environment=production', + }, + () => { + return getCurrentScope(); + }, + ); + + expect(scope.getPropagationContext().traceId).toBe('12312012123120121231201212312012'); + expect(scope.getPropagationContext().parentSpanId).toBe('1121201211212012'); + }); + + it('SDK org ID is missing', () => { + client.getOptions().strictTraceContinuation = false; + + const scope = continueTrace( + { + sentryTrace: '12312012123120121231201212312012-1121201211212012-1', + baggage: 'sentry-org_id=123', + }, + () => { + return getCurrentScope(); + }, + ); + + expect(scope.getPropagationContext().traceId).toBe('12312012123120121231201212312012'); + expect(scope.getPropagationContext().parentSpanId).toBe('1121201211212012'); + }); + }); + }); }); describe('getActiveSpan', () => { From e2dd6a693172cd82cb4085116da8ac08ae775e18 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Fri, 16 May 2025 14:24:53 +0200 Subject: [PATCH 2/4] create utility function for getting org id --- .../src/tracing/dynamicSamplingContext.ts | 14 ++--- packages/core/src/tracing/trace.ts | 9 +--- packages/core/src/tracing/utils.ts | 23 ++++++++ packages/core/test/lib/tracing/utils.test.ts | 53 +++++++++++++++++++ packages/core/tsconfig.test.json | 1 + 5 files changed, 82 insertions(+), 18 deletions(-) create mode 100644 packages/core/test/lib/tracing/utils.test.ts diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index 5f10f11db19c..e5858fdd135e 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -15,9 +15,8 @@ import { baggageHeaderToDynamicSamplingContext, dynamicSamplingContextToSentryBaggageHeader, } from '../utils-hoist/baggage'; -import { extractOrgIdFromDsnHost } from '../utils-hoist/dsn'; import { addNonEnumerableProperty } from '../utils-hoist/object'; -import { getCapturedScopesOnSpan } from './utils'; +import { deriveOrgIdFromClient, getCapturedScopesOnSpan } from './utils'; /** * If you change this value, also update the terser plugin config to @@ -45,14 +44,7 @@ export function freezeDscOnSpan(span: Span, dsc: Partial export function getDynamicSamplingContextFromClient(trace_id: string, client: Client): DynamicSamplingContext { const options = client.getOptions(); - const { publicKey: public_key, host } = client.getDsn() || {}; - - let org_id: string | undefined; - if (options.orgId) { - org_id = String(options.orgId); - } else if (host) { - org_id = extractOrgIdFromDsnHost(host); - } + const { publicKey: public_key } = client.getDsn() || {}; // Instead of conditionally adding non-undefined values, we add them and then remove them if needed // otherwise, the order of baggage entries changes, which "breaks" a bunch of tests etc. @@ -61,7 +53,7 @@ export function getDynamicSamplingContextFromClient(trace_id: string, client: Cl release: options.release, public_key, trace_id, - org_id, + org_id: deriveOrgIdFromClient(client), }; client.emit('createDsc', dsc); diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index 41f3fae23a12..6a84f4db31da 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -17,7 +17,6 @@ import { parseSampleRate } from '../utils/parseSampleRate'; import { _getSpanForScope, _setSpanForScope } from '../utils/spanOnScope'; import { addChildSpanToSpan, getRootSpan, spanIsSampled, spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils'; import { baggageHeaderToDynamicSamplingContext } from '../utils-hoist/baggage'; -import { extractOrgIdFromDsnHost } from '../utils-hoist/dsn'; import { logger } from '../utils-hoist/logger'; import { generateTraceId } from '../utils-hoist/propagationContext'; import { propagationContextFromHeaders } from '../utils-hoist/tracing'; @@ -27,7 +26,7 @@ import { sampleSpan } from './sampling'; import { SentryNonRecordingSpan } from './sentryNonRecordingSpan'; import { SentrySpan } from './sentrySpan'; import { SPAN_STATUS_ERROR } from './spanstatus'; -import { setCapturedScopesOnSpan } from './utils'; +import { deriveOrgIdFromClient, setCapturedScopesOnSpan } from './utils'; const SUPPRESS_TRACING_KEY = '__SENTRY_SUPPRESS_TRACING__'; @@ -225,11 +224,7 @@ export const continueTrace = ( const incomingDsc = baggageHeaderToDynamicSamplingContext(baggage); const baggageOrgId = incomingDsc?.org_id; - let sdkOrgId: string | undefined; - const dsn = client?.getDsn(); - if (dsn?.host) { - sdkOrgId = extractOrgIdFromDsnHost(dsn.host); - } + const sdkOrgId = deriveOrgIdFromClient(client); const shouldStartNewTrace = (): boolean => { // Case: baggage org ID and SDK org ID don't match - always start new trace diff --git a/packages/core/src/tracing/utils.ts b/packages/core/src/tracing/utils.ts index f7e3d4924b35..4ffae4ad9113 100644 --- a/packages/core/src/tracing/utils.ts +++ b/packages/core/src/tracing/utils.ts @@ -1,5 +1,7 @@ +import type { Client } from '../client'; import type { Scope } from '../scope'; import type { Span } from '../types-hoist/span'; +import { extractOrgIdFromDsnHost } from '../utils-hoist/dsn'; import { addNonEnumerableProperty } from '../utils-hoist/object'; const SCOPE_ON_START_SPAN_FIELD = '_sentryScope'; @@ -27,3 +29,24 @@ export function getCapturedScopesOnSpan(span: Span): { scope?: Scope; isolationS isolationScope: (span as SpanWithScopes)[ISOLATION_SCOPE_ON_START_SPAN_FIELD], }; } + +/** + * Returns the organization ID of the client. + * + * The organization ID is extracted from the DSN. If the client options include a `orgId`, this will always take precedence. + */ +export function deriveOrgIdFromClient(client: Client | undefined): string | undefined { + const options = client?.getOptions(); + + const { host } = client?.getDsn() || {}; + + let org_id: string | undefined; + + if (options?.orgId) { + org_id = String(options.orgId); + } else if (host) { + org_id = extractOrgIdFromDsnHost(host); + } + + return org_id; +} diff --git a/packages/core/test/lib/tracing/utils.test.ts b/packages/core/test/lib/tracing/utils.test.ts new file mode 100644 index 000000000000..7fbb624644a8 --- /dev/null +++ b/packages/core/test/lib/tracing/utils.test.ts @@ -0,0 +1,53 @@ +import { beforeEach, describe, expect, test, vi } from 'vitest'; +import { deriveOrgIdFromClient } from '../../../src/tracing/utils'; +import { getDefaultTestClientOptions, TestClient } from '../../mocks/client'; + +describe('deriveOrgIdFromClient', () => { + let client: TestClient; + + beforeEach(() => { + vi.clearAllMocks(); + }); + + test('returns orgId from client options when available', () => { + client = new TestClient( + getDefaultTestClientOptions({ + orgId: '00222111', + dsn: 'https://public@sentry.example.com/1', + }), + ); + + const result = deriveOrgIdFromClient(client); + expect(result).toBe('00222111'); + }); + + test('converts non-string orgId to string', () => { + client = new TestClient( + getDefaultTestClientOptions({ + orgId: 12345, + dsn: 'https://public@sentry.example.com/1', + }), + ); + + const result = deriveOrgIdFromClient(client); + expect(result).toBe('12345'); + }); + + test('extracts orgId from DSN host when options.orgId is not available', () => { + client = new TestClient( + getDefaultTestClientOptions({ + dsn: 'https://public@o012300.example.com/1', + }), + ); + + const result = deriveOrgIdFromClient(client); + expect(result).toBe('012300'); + }); + + test('returns undefined when neither options.orgId nor DSN host are available', () => { + client = new TestClient(getDefaultTestClientOptions({})); + + const result = deriveOrgIdFromClient(client); + expect(result).toBeUndefined(); + }); +}); diff --git a/packages/core/tsconfig.test.json b/packages/core/tsconfig.test.json index c2b522d04ee1..0db9ad3bf16c 100644 --- a/packages/core/tsconfig.test.json +++ b/packages/core/tsconfig.test.json @@ -5,6 +5,7 @@ "compilerOptions": { "lib": ["DOM", "ES2018"], + "module": "ESNext", // support dynamic import() // should include all types from `./tsconfig.json` plus types for all test frameworks used "types": ["node"] From cd60e47f8365667f7a9323fcf474645486724110 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Fri, 16 May 2025 15:03:34 +0200 Subject: [PATCH 3/4] add `shouldContinueTrace` util function --- packages/core/src/index.ts | 1 + packages/core/src/tracing/trace.ts | 35 +--------- packages/core/src/utils-hoist/tracing.ts | 41 +++++++++++ .../core/test/utils-hoist/tracing.test.ts | 69 ++++++++++++++++++- 4 files changed, 112 insertions(+), 34 deletions(-) diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 6d281fde0ac9..99a52b9566dc 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -216,6 +216,7 @@ export { extractTraceparentData, generateSentryTraceHeader, propagationContextFromHeaders, + shouldContinueTrace, } from './utils-hoist/tracing'; export { getSDKSource, isBrowserBundle } from './utils-hoist/env'; export type { SdkSource } from './utils-hoist/env'; diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index 6a84f4db31da..51b08c6f0e68 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -19,7 +19,7 @@ import { addChildSpanToSpan, getRootSpan, spanIsSampled, spanTimeInputToSeconds, import { baggageHeaderToDynamicSamplingContext } from '../utils-hoist/baggage'; import { logger } from '../utils-hoist/logger'; import { generateTraceId } from '../utils-hoist/propagationContext'; -import { propagationContextFromHeaders } from '../utils-hoist/tracing'; +import { propagationContextFromHeaders, shouldContinueTrace } from '../utils-hoist/tracing'; import { freezeDscOnSpan, getDynamicSamplingContextFromSpan } from './dynamicSamplingContext'; import { logSpanStart } from './logSpans'; import { sampleSpan } from './sampling'; @@ -216,41 +216,10 @@ export const continueTrace = ( } const { sentryTrace, baggage } = options; - const client = getClient(); - - const clientOptions = client?.getOptions(); - const strictTraceContinuation = clientOptions?.strictTraceContinuation || false; // default for `strictTraceContinuation` is `false` todo(v10): set default to `true` const incomingDsc = baggageHeaderToDynamicSamplingContext(baggage); - const baggageOrgId = incomingDsc?.org_id; - - const sdkOrgId = deriveOrgIdFromClient(client); - - const shouldStartNewTrace = (): boolean => { - // Case: baggage org ID and SDK org ID don't match - always start new trace - if (baggageOrgId && sdkOrgId && baggageOrgId !== sdkOrgId) { - DEBUG_BUILD && - logger.info(`Starting a new trace because org IDs don't match (incoming: ${baggageOrgId}, sdk: ${sdkOrgId})`); - return true; - } - - if (strictTraceContinuation) { - // With strict continuation enabled, start new trace if: - // - Baggage has org ID but SDK doesn't have one - // - SDK has org ID but baggage doesn't have one - if ((baggageOrgId && !sdkOrgId) || (!baggageOrgId && sdkOrgId)) { - DEBUG_BUILD && - logger.info( - `Starting a new trace because strict trace continuation is enabled and one org ID is missing (incoming: ${baggageOrgId}, sdk: ${sdkOrgId})`, - ); - return true; - } - } - - return false; - }; - if (shouldStartNewTrace()) { + if (shouldContinueTrace(getClient(), incomingDsc?.org_id)) { return startNewTrace(callback); } diff --git a/packages/core/src/utils-hoist/tracing.ts b/packages/core/src/utils-hoist/tracing.ts index a3add28426f7..affc57bd6f8c 100644 --- a/packages/core/src/utils-hoist/tracing.ts +++ b/packages/core/src/utils-hoist/tracing.ts @@ -1,7 +1,11 @@ +import type { Client } from '../client'; +import { DEBUG_BUILD } from '../debug-build'; +import { deriveOrgIdFromClient } from '../tracing/utils'; import type { DynamicSamplingContext } from '../types-hoist/envelope'; import type { PropagationContext } from '../types-hoist/tracing'; import type { TraceparentData } from '../types-hoist/transaction'; import { parseSampleRate } from '../utils/parseSampleRate'; +import { logger } from '../utils-hoist/logger'; import { baggageHeaderToDynamicSamplingContext } from './baggage'; import { generateSpanId, generateTraceId } from './propagationContext'; @@ -124,3 +128,40 @@ function getSampleRandFromTraceparentAndDsc( return Math.random(); } } + +/** + * Determines whether a new trace should be continued based on the provided client and baggage org ID. + * If the trace should not be continued, a new trace will be started. + * + * The result is dependent on the `strictTraceContinuation` option in the client. + * See https://develop.sentry.dev/sdk/telemetry/traces/#stricttracecontinuation + */ +export function shouldContinueTrace(client: Client | undefined, baggageOrgId?: string): boolean { + const sdkOptionOrgId = deriveOrgIdFromClient(client); + + // Case: baggage org ID and SDK org ID don't match - always start new trace + if (baggageOrgId && sdkOptionOrgId && baggageOrgId !== sdkOptionOrgId) { + DEBUG_BUILD && + logger.info( + `Starting a new trace because org IDs don't match (incoming baggage: ${baggageOrgId}, SDK options: ${sdkOptionOrgId})`, + ); + return false; + } + + const strictTraceContinuation = client?.getOptions()?.strictTraceContinuation || false; // default for `strictTraceContinuation` is `false` todo(v10): set default to `true` + + if (strictTraceContinuation) { + // With strict continuation enabled, start new trace if: + // - Baggage has org ID but SDK doesn't have one + // - SDK has org ID but baggage doesn't have one + if ((baggageOrgId && !sdkOptionOrgId) || (!baggageOrgId && sdkOptionOrgId)) { + DEBUG_BUILD && + logger.info( + `Starting a new trace because strict trace continuation is enabled and one org ID is missing (incoming baggage: ${baggageOrgId}, SDK options: ${sdkOptionOrgId})`, + ); + return false; + } + } + + return true; +} diff --git a/packages/core/test/utils-hoist/tracing.test.ts b/packages/core/test/utils-hoist/tracing.test.ts index 851ee7b109c4..4e23c7567c79 100644 --- a/packages/core/test/utils-hoist/tracing.test.ts +++ b/packages/core/test/utils-hoist/tracing.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it, test } from 'vitest'; -import { extractTraceparentData, propagationContextFromHeaders } from '../../src/utils-hoist/tracing'; +import { + extractTraceparentData, + propagationContextFromHeaders, + shouldContinueTrace, +} from '../../src/utils-hoist/tracing'; +import { getDefaultTestClientOptions, TestClient } from '../mocks/client'; const EXAMPLE_SENTRY_TRACE = '12312012123120121231201212312012-1121201211212012-1'; const EXAMPLE_BAGGAGE = 'sentry-release=1.2.3,sentry-foo=bar,other=baz,sentry-sample_rand=0.42'; @@ -124,3 +129,65 @@ describe('extractTraceparentData', () => { expect(extractTraceparentData('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb-x')).toBeUndefined(); }); }); + +describe('shouldContinueTrace', () => { + test('returns true when both baggage and SDK org IDs are undefined', () => { + const client = new TestClient(getDefaultTestClientOptions({})); + + const result = shouldContinueTrace(client, undefined); + expect(result).toBe(true); + }); + + test('returns true when org IDs match', () => { + const orgId = '123456'; + const client = new TestClient(getDefaultTestClientOptions({ orgId })); + + const result = shouldContinueTrace(client, orgId); + expect(result).toBe(true); + }); + + test('returns false when org IDs do not match', () => { + const client = new TestClient(getDefaultTestClientOptions({ orgId: '123456' })); + + const result = shouldContinueTrace(client, '654321'); + expect(result).toBe(false); + }); + + test('returns true when baggage org ID is undefined and strictTraceContinuation is false', () => { + const client = new TestClient(getDefaultTestClientOptions({ orgId: '123456', strictTraceContinuation: false })); + + const result = shouldContinueTrace(client, undefined); + expect(result).toBe(true); + }); + + test('returns true when SDK org ID is undefined and strictTraceContinuation is false', () => { + const client = new TestClient(getDefaultTestClientOptions({ strictTraceContinuation: false })); + + const result = shouldContinueTrace(client, '123456'); + expect(result).toBe(true); + }); + + test('returns false when baggage org ID is undefined and strictTraceContinuation is true', () => { + const client = new TestClient(getDefaultTestClientOptions({ orgId: '123456', strictTraceContinuation: true })); + + const result = shouldContinueTrace(client, undefined); + expect(result).toBe(false); + }); + + test('returns false when SDK org ID is undefined and strictTraceContinuation is true', () => { + const client = new TestClient(getDefaultTestClientOptions({ strictTraceContinuation: true })); + + const result = shouldContinueTrace(client, '123456'); + expect(result).toBe(false); + }); + + test('returns true when client is undefined', () => { + const result = shouldContinueTrace(undefined, '123456'); + expect(result).toBe(true); + }); + + test('returns true when both org IDs and client are undefined', () => { + const result = shouldContinueTrace(undefined, undefined); + expect(result).toBe(true); + }); +}); From 484aedf89aad1ef862f95fe2e9925d24c3d56e4a Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Fri, 16 May 2025 15:30:38 +0200 Subject: [PATCH 4/4] shuffle around util functions --- packages/core/src/index.ts | 2 +- .../src/tracing/dynamicSamplingContext.ts | 3 ++- packages/core/src/tracing/trace.ts | 2 +- packages/core/src/tracing/utils.ts | 23 ------------------- packages/core/src/utils-hoist/dsn.ts | 22 ++++++++++++++++++ packages/core/src/utils-hoist/tracing.ts | 2 +- packages/opentelemetry/src/propagator.ts | 5 +++- packages/opentelemetry/src/trace.ts | 2 +- 8 files changed, 32 insertions(+), 29 deletions(-) diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 99a52b9566dc..4cfeb4fab387 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -127,7 +127,7 @@ export type { FeatureFlag } from './featureFlags'; export { applyAggregateErrorsToEvent } from './utils-hoist/aggregate-errors'; export { getBreadcrumbLogLevelFromHttpStatusCode } from './utils-hoist/breadcrumb-log-level'; export { getComponentName, getLocationHref, htmlTreeAsString } from './utils-hoist/browser'; -export { dsnFromString, dsnToString, makeDsn } from './utils-hoist/dsn'; +export { dsnFromString, dsnToString, makeDsn, deriveOrgIdFromClient } from './utils-hoist/dsn'; // eslint-disable-next-line deprecation/deprecation export { SentryError } from './utils-hoist/error'; export { GLOBAL_OBJ } from './utils-hoist/worldwide'; diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index e5858fdd135e..e5a5389f394f 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -15,8 +15,9 @@ import { baggageHeaderToDynamicSamplingContext, dynamicSamplingContextToSentryBaggageHeader, } from '../utils-hoist/baggage'; +import { deriveOrgIdFromClient } from '../utils-hoist/dsn'; import { addNonEnumerableProperty } from '../utils-hoist/object'; -import { deriveOrgIdFromClient, getCapturedScopesOnSpan } from './utils'; +import { getCapturedScopesOnSpan } from './utils'; /** * If you change this value, also update the terser plugin config to diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index 51b08c6f0e68..15cc19dc4739 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -26,7 +26,7 @@ import { sampleSpan } from './sampling'; import { SentryNonRecordingSpan } from './sentryNonRecordingSpan'; import { SentrySpan } from './sentrySpan'; import { SPAN_STATUS_ERROR } from './spanstatus'; -import { deriveOrgIdFromClient, setCapturedScopesOnSpan } from './utils'; +import { setCapturedScopesOnSpan } from './utils'; const SUPPRESS_TRACING_KEY = '__SENTRY_SUPPRESS_TRACING__'; diff --git a/packages/core/src/tracing/utils.ts b/packages/core/src/tracing/utils.ts index 4ffae4ad9113..f7e3d4924b35 100644 --- a/packages/core/src/tracing/utils.ts +++ b/packages/core/src/tracing/utils.ts @@ -1,7 +1,5 @@ -import type { Client } from '../client'; import type { Scope } from '../scope'; import type { Span } from '../types-hoist/span'; -import { extractOrgIdFromDsnHost } from '../utils-hoist/dsn'; import { addNonEnumerableProperty } from '../utils-hoist/object'; const SCOPE_ON_START_SPAN_FIELD = '_sentryScope'; @@ -29,24 +27,3 @@ export function getCapturedScopesOnSpan(span: Span): { scope?: Scope; isolationS isolationScope: (span as SpanWithScopes)[ISOLATION_SCOPE_ON_START_SPAN_FIELD], }; } - -/** - * Returns the organization ID of the client. - * - * The organization ID is extracted from the DSN. If the client options include a `orgId`, this will always take precedence. - */ -export function deriveOrgIdFromClient(client: Client | undefined): string | undefined { - const options = client?.getOptions(); - - const { host } = client?.getDsn() || {}; - - let org_id: string | undefined; - - if (options?.orgId) { - org_id = String(options.orgId); - } else if (host) { - org_id = extractOrgIdFromDsnHost(host); - } - - return org_id; -} diff --git a/packages/core/src/utils-hoist/dsn.ts b/packages/core/src/utils-hoist/dsn.ts index e26cbb4c8bf1..bff1461a29ab 100644 --- a/packages/core/src/utils-hoist/dsn.ts +++ b/packages/core/src/utils-hoist/dsn.ts @@ -1,3 +1,4 @@ +import type { Client } from '../client'; import type { DsnComponents, DsnLike, DsnProtocol } from '../types-hoist/dsn'; import { DEBUG_BUILD } from './../debug-build'; import { consoleSandbox, logger } from './logger'; @@ -129,6 +130,27 @@ export function extractOrgIdFromDsnHost(host: string): string | undefined { return match?.[1]; } +/** + * Returns the organization ID of the client. + * + * The organization ID is extracted from the DSN. If the client options include a `orgId`, this will always take precedence. + */ +export function deriveOrgIdFromClient(client: Client | undefined): string | undefined { + const options = client?.getOptions(); + + const { host } = client?.getDsn() || {}; + + let org_id: string | undefined; + + if (options?.orgId) { + org_id = String(options.orgId); + } else if (host) { + org_id = extractOrgIdFromDsnHost(host); + } + + return org_id; +} + /** * Creates a valid Sentry Dsn object, identifying a Sentry instance and project. * @returns a valid DsnComponents object or `undefined` if @param from is an invalid DSN source diff --git a/packages/core/src/utils-hoist/tracing.ts b/packages/core/src/utils-hoist/tracing.ts index affc57bd6f8c..53209e574569 100644 --- a/packages/core/src/utils-hoist/tracing.ts +++ b/packages/core/src/utils-hoist/tracing.ts @@ -1,10 +1,10 @@ import type { Client } from '../client'; import { DEBUG_BUILD } from '../debug-build'; -import { deriveOrgIdFromClient } from '../tracing/utils'; import type { DynamicSamplingContext } from '../types-hoist/envelope'; import type { PropagationContext } from '../types-hoist/tracing'; import type { TraceparentData } from '../types-hoist/transaction'; import { parseSampleRate } from '../utils/parseSampleRate'; +import { deriveOrgIdFromClient } from '../utils-hoist/dsn'; import { logger } from '../utils-hoist/logger'; import { baggageHeaderToDynamicSamplingContext } from './baggage'; import { generateSpanId, generateTraceId } from './propagationContext'; diff --git a/packages/opentelemetry/src/propagator.ts b/packages/opentelemetry/src/propagator.ts index b5e493b31fd8..1c3b122ef466 100644 --- a/packages/opentelemetry/src/propagator.ts +++ b/packages/opentelemetry/src/propagator.ts @@ -15,9 +15,11 @@ import { parseBaggageHeader, propagationContextFromHeaders, SENTRY_BAGGAGE_KEY_PREFIX, + shouldContinueTrace, spanToJSON, stringMatchesSomePattern, } from '@sentry/core'; +import { baggageHeaderToDynamicSamplingContext } from '@sentry/core/src'; 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'; @@ -210,10 +212,11 @@ function getContextWithRemoteActiveSpan( const propagationContext = propagationContextFromHeaders(sentryTrace, baggage); const { traceId, parentSpanId, sampled, dsc } = propagationContext; + const incomingDsc = baggageHeaderToDynamicSamplingContext(baggage); // We only want to set the virtual span if we are continuing a concrete trace // Otherwise, we ignore the incoming trace here, e.g. if we have no trace headers - if (!parentSpanId) { + if (!parentSpanId || !shouldContinueTrace(getClient(), incomingDsc?.org_id)) { return ctx; } diff --git a/packages/opentelemetry/src/trace.ts b/packages/opentelemetry/src/trace.ts index e604d83bd4d8..c30377fe7763 100644 --- a/packages/opentelemetry/src/trace.ts +++ b/packages/opentelemetry/src/trace.ts @@ -259,7 +259,7 @@ export function continueTrace(options: Parameters[0 /** * Get the trace context for a given scope. - * We have a custom implemention here because we need an OTEL-specific way to get the span from a scope. + * We have a custom implementation here because we need an OTEL-specific way to get the span from a scope. */ export function getTraceContextForScope( client: Client,