Skip to content

feat(core): Implement strictTraceContinuation #16313

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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';
Expand Down
13 changes: 3 additions & 10 deletions packages/core/src/tracing/dynamicSamplingContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
baggageHeaderToDynamicSamplingContext,
dynamicSamplingContextToSentryBaggageHeader,
} from '../utils-hoist/baggage';
import { extractOrgIdFromDsnHost } from '../utils-hoist/dsn';
import { deriveOrgIdFromClient } from '../utils-hoist/dsn';
import { addNonEnumerableProperty } from '../utils-hoist/object';
import { getCapturedScopesOnSpan } from './utils';

Expand Down Expand Up @@ -45,14 +45,7 @@ export function freezeDscOnSpan(span: Span, dsc: Partial<DynamicSamplingContext>
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.
Expand All @@ -61,7 +54,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);
Expand Down
9 changes: 8 additions & 1 deletion packages/core/src/tracing/trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@ 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 { 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';
Expand Down Expand Up @@ -216,6 +217,12 @@ export const continueTrace = <V>(

const { sentryTrace, baggage } = options;

const incomingDsc = baggageHeaderToDynamicSamplingContext(baggage);

if (shouldContinueTrace(getClient(), incomingDsc?.org_id)) {
return startNewTrace(callback);
}

return withScope(scope => {
const propagationContext = propagationContextFromHeaders(sentryTrace, baggage);
scope.setPropagationContext(propagationContext);
Expand Down
12 changes: 12 additions & 0 deletions packages/core/src/types-hoist/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,18 @@ export interface ClientOptions<TO extends BaseTransportOptions = BaseTransportOp
*/
tracePropagationTargets?: TracePropagationTargets;

/**
* Controls whether trace continuation should be strict about matching organization IDs.
*
* When set to `true`, the SDK will only continue a trace if the organization ID in the incoming baggage
* matches the organization ID of the current SDK (extracted from the DSN).
*
* If there is no match, the SDK will start a new trace instead of continuing the incoming one.
*
* @default false
*/
strictTraceContinuation?: boolean;

/**
* The organization ID of the current SDK. The organization ID is a string containing only numbers. This ID is used to
* propagate traces to other Sentry services.
Expand Down
22 changes: 22 additions & 0 deletions packages/core/src/utils-hoist/dsn.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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
Expand Down
41 changes: 41 additions & 0 deletions packages/core/src/utils-hoist/tracing.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import type { Client } from '../client';
import { DEBUG_BUILD } from '../debug-build';
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';

Expand Down Expand Up @@ -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;
}
146 changes: 146 additions & 0 deletions packages/core/test/lib/tracing/trace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import type { Span } from '../../../src/types-hoist/span';
import type { StartSpanOptions } from '../../../src/types-hoist/startSpanOptions';
import { _setSpanForScope } from '../../../src/utils/spanOnScope';
import { getActiveSpan, getRootSpan, getSpanDescendants, spanIsSampled } from '../../../src/utils/spanUtils';
import { extractOrgIdFromDsnHost } from '../../../src/utils-hoist/dsn';
import { getDefaultTestClientOptions, TestClient } from '../../mocks/client';

const enum Type {
Expand Down Expand Up @@ -1733,6 +1734,151 @@ describe('continueTrace', () => {

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', () => {
Expand Down
53 changes: 53 additions & 0 deletions packages/core/test/lib/tracing/utils.test.ts
Original file line number Diff line number Diff line change
@@ -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://[email protected]/1',
}),
);

const result = deriveOrgIdFromClient(client);
expect(result).toBe('00222111');
});

test('converts non-string orgId to string', () => {
client = new TestClient(
getDefaultTestClientOptions({
orgId: 12345,
dsn: 'https://[email protected]/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://[email protected]/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();
});
});
Loading
Loading