From fadb94b675bb64ef638f4e06c5b6f076a2d067b7 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Thu, 15 May 2025 10:19:17 +0200 Subject: [PATCH 1/5] feat(core): Add `orgId` option to DSC --- .../src/tracing/dynamicSamplingContext.ts | 11 +- packages/core/src/types-hoist/envelope.ts | 1 + packages/core/src/types-hoist/options.ts | 7 ++ packages/core/src/utils-hoist/dsn.ts | 18 +++ .../tracing/dynamicSamplingContext.test.ts | 115 +++++++++++++++++- packages/core/test/utils-hoist/dsn.test.ts | 36 +++++- 6 files changed, 184 insertions(+), 4 deletions(-) diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index 9380c75dd3be..4088f5ef9aaf 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -15,6 +15,7 @@ import { baggageHeaderToDynamicSamplingContext, dynamicSamplingContextToSentryBaggageHeader, } from '../utils-hoist/baggage'; +import { extractOrgIdFromDsnHost } from '../utils-hoist/dsn'; import { addNonEnumerableProperty } from '../utils-hoist/object'; import { getCapturedScopesOnSpan } from './utils'; @@ -44,7 +45,14 @@ export function freezeDscOnSpan(span: Span, dsc: Partial export function getDynamicSamplingContextFromClient(trace_id: string, client: Client): DynamicSamplingContext { const options = client.getOptions(); - const { publicKey: public_key } = client.getDsn() || {}; + const { publicKey: public_key, host } = client.getDsn() || {}; + + let org_id: string | undefined; + if (options.orgId) { + org_id = options.orgId; + } else if (host) { + org_id = extractOrgIdFromDsnHost(host); + } // 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. @@ -53,6 +61,7 @@ export function getDynamicSamplingContextFromClient(trace_id: string, client: Cl release: options.release, public_key, trace_id, + org_id, }; client.emit('createDsc', dsc); diff --git a/packages/core/src/types-hoist/envelope.ts b/packages/core/src/types-hoist/envelope.ts index d874a4e65800..58671c1eba70 100644 --- a/packages/core/src/types-hoist/envelope.ts +++ b/packages/core/src/types-hoist/envelope.ts @@ -25,6 +25,7 @@ export type DynamicSamplingContext = { replay_id?: string; sampled?: string; sample_rand?: string; + org_id?: string; }; // https://github.com/getsentry/relay/blob/311b237cd4471042352fa45e7a0824b8995f216f/relay-server/src/envelope.rs#L154 diff --git a/packages/core/src/types-hoist/options.ts b/packages/core/src/types-hoist/options.ts index 4b0010f2b7d7..093b6203dcd6 100644 --- a/packages/core/src/types-hoist/options.ts +++ b/packages/core/src/types-hoist/options.ts @@ -320,6 +320,13 @@ export interface ClientOptions { }); }); }); + +describe('getDynamicSamplingContextFromClient', () => { + const TRACE_ID = '4b25bc58f14243d8b208d1e22a054164'; + let client: TestClient; + + beforeEach(() => { + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.resetAllMocks(); + }); + + it('creates DSC with basic client information', () => { + client = new TestClient( + getDefaultTestClientOptions({ + release: '1.0.0', + environment: 'test-env', + dsn: 'https://public@sentry.example.com/1', + }), + ); + + const dsc = getDynamicSamplingContextFromClient(TRACE_ID, client); + + expect(dsc).toEqual({ + trace_id: TRACE_ID, + release: '1.0.0', + environment: 'test-env', + public_key: 'public', + org_id: undefined, + }); + }); + + it('uses DEFAULT_ENVIRONMENT when environment is not specified', () => { + client = new TestClient( + getDefaultTestClientOptions({ + release: '1.0.0', + dsn: 'https://public@sentry.example.com/1', + }), + ); + + const dsc = getDynamicSamplingContextFromClient(TRACE_ID, client); + + expect(dsc.environment).toBe(DEFAULT_ENVIRONMENT); + }); + + it('uses orgId from options when specified', () => { + client = new TestClient( + getDefaultTestClientOptions({ + orgId: 'explicit-org-id', + dsn: 'https://public@sentry.example.com/1', + }), + ); + + const dsc = getDynamicSamplingContextFromClient(TRACE_ID, client); + + expect(dsc.org_id).toBe('explicit-org-id'); + }); + + it('infers orgId from DSN host when not explicitly provided', () => { + client = new TestClient( + getDefaultTestClientOptions({ + dsn: 'https://public@o123456.sentry.io/1', + }), + ); + + const dsc = getDynamicSamplingContextFromClient(TRACE_ID, client); + + expect(dsc.org_id).toBe('123456'); + }); + + it('prioritizes explicit orgId over inferred from DSN', () => { + client = new TestClient( + getDefaultTestClientOptions({ + orgId: 'explicit-org-id', + dsn: 'https://public@my-org.sentry.io/1', + }), + ); + + const dsc = getDynamicSamplingContextFromClient(TRACE_ID, client); + + expect(dsc.org_id).toBe('explicit-org-id'); + }); + + it('handles missing DSN gracefully', () => { + client = new TestClient( + getDefaultTestClientOptions({ + release: '1.0.0', + }), + ); + + const dsc = getDynamicSamplingContextFromClient(TRACE_ID, client); + + expect(dsc.public_key).toBeUndefined(); + expect(dsc.org_id).toBeUndefined(); + }); + + it('emits createDsc event with the generated DSC', () => { + client = new TestClient( + getDefaultTestClientOptions({ + release: '1.0.0', + dsn: 'https://public@sentry.example.com/1', + }), + ); + + const emitSpy = vi.spyOn(client, 'emit'); + + const dsc = getDynamicSamplingContextFromClient(TRACE_ID, client); + + expect(emitSpy).toHaveBeenCalledWith('createDsc', dsc); + }); +}); diff --git a/packages/core/test/utils-hoist/dsn.test.ts b/packages/core/test/utils-hoist/dsn.test.ts index 6d34b599c6c9..b5d22130816b 100644 --- a/packages/core/test/utils-hoist/dsn.test.ts +++ b/packages/core/test/utils-hoist/dsn.test.ts @@ -1,6 +1,6 @@ -import { beforeEach, describe, expect, test, vi } from 'vitest'; +import { beforeEach, describe, expect, it, test, vi } from 'vitest'; import { DEBUG_BUILD } from '../../src/debug-build'; -import { dsnToString, makeDsn } from '../../src/utils-hoist/dsn'; +import { dsnToString, extractOrgIdFromDsnHost, makeDsn } from '../../src/utils-hoist/dsn'; import { logger } from '../../src/utils-hoist/logger'; function testIf(condition: boolean) { @@ -215,3 +215,35 @@ describe('Dsn', () => { }); }); }); + +describe('extractOrgIdFromDsnHost', () => { + it('extracts the org ID from a DSN host with standard format', () => { + expect(extractOrgIdFromDsnHost('o123456.sentry.io')).toBe('123456'); + }); + + it('extracts numeric org IDs of different lengths', () => { + expect(extractOrgIdFromDsnHost('o1.ingest.sentry.io')).toBe('1'); + expect(extractOrgIdFromDsnHost('o42.sentry.io')).toBe('42'); + expect(extractOrgIdFromDsnHost('o9999999.sentry.io')).toBe('9999999'); + }); + + it('returns undefined for hosts without an org ID prefix', () => { + expect(extractOrgIdFromDsnHost('sentry.io')).toBeUndefined(); + expect(extractOrgIdFromDsnHost('example.com')).toBeUndefined(); + }); + + it('returns undefined for hosts with invalid org ID format', () => { + expect(extractOrgIdFromDsnHost('oabc.sentry.io')).toBeUndefined(); + expect(extractOrgIdFromDsnHost('o.sentry.io')).toBeUndefined(); + expect(extractOrgIdFromDsnHost('oX123.sentry.io')).toBeUndefined(); + }); + + it('handles different domain variations', () => { + expect(extractOrgIdFromDsnHost('o123456.ingest.sentry.io')).toBe('123456'); + expect(extractOrgIdFromDsnHost('o123456.custom-domain.com')).toBe('123456'); + }); + + it('handles empty string input', () => { + expect(extractOrgIdFromDsnHost('')).toBeUndefined(); + }); +}); From 0d5c6878d2332190ccedbdd67d119bc23eb8b374 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Thu, 15 May 2025 10:48:36 +0200 Subject: [PATCH 2/5] add server integration tests --- .../server-no-explicit-org-id.ts | 34 ++++++++++++++++++ .../baggage-org-id/server.ts | 35 +++++++++++++++++++ .../baggage-org-id/test.ts | 32 +++++++++++++++++ 3 files changed, 101 insertions(+) create mode 100644 dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/baggage-org-id/server-no-explicit-org-id.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/baggage-org-id/server.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/baggage-org-id/test.ts diff --git a/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/baggage-org-id/server-no-explicit-org-id.ts b/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/baggage-org-id/server-no-explicit-org-id.ts new file mode 100644 index 000000000000..1b3afae252d7 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/baggage-org-id/server-no-explicit-org-id.ts @@ -0,0 +1,34 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport, startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; + +export type TestAPIResponse = { test_data: { host: string; 'sentry-trace': string; baggage: string } }; + +Sentry.init({ + dsn: 'https://public@o01234987.ingest.sentry.io/1337', + release: '1.0', + environment: 'prod', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +import cors from 'cors'; +import express from 'express'; +import * as http from 'http'; + +const app = express(); + +app.use(cors()); + +app.get('/test/express', (_req, res) => { + const headers = http + .get({ + hostname: 'example.com', + }) + .getHeaders(); + + res.send({ test_data: headers }); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/baggage-org-id/server.ts b/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/baggage-org-id/server.ts new file mode 100644 index 000000000000..a149f74370f6 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/baggage-org-id/server.ts @@ -0,0 +1,35 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport, startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; + +export type TestAPIResponse = { test_data: { host: string; 'sentry-trace': string; baggage: string } }; + +Sentry.init({ + dsn: 'https://public@o0000987.ingest.sentry.io/1337', + release: '1.0', + environment: 'prod', + orgId: '01234987', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +import cors from 'cors'; +import express from 'express'; +import * as http from 'http'; + +const app = express(); + +app.use(cors()); + +app.get('/test/express', (_req, res) => { + const headers = http + .get({ + hostname: 'example.com', + }) + .getHeaders(); + + res.send({ test_data: headers }); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/baggage-org-id/test.ts b/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/baggage-org-id/test.ts new file mode 100644 index 000000000000..aeaa171da474 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/baggage-org-id/test.ts @@ -0,0 +1,32 @@ +import { afterAll, expect, test } from 'vitest'; +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; +import type { TestAPIResponse } from './server'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('should include explicitly set org_id in the baggage header', async () => { + const runner = createRunner(__dirname, 'server.ts').start(); + + const response = await runner.makeRequest('get', '/test/express'); + + expect(response).toBeDefined(); + + const baggage = response?.test_data.baggage?.split(',').sort(); + + expect(baggage).toContain('sentry-org_id=01234987'); +}); + +test('should extract org_id from DSN host when not explicitly set', async () => { + // This test requires a new server with different configuration + const runner = createRunner(__dirname, 'server-no-explicit-org-id.ts').start(); + + const response = await runner.makeRequest('get', '/test/express'); + + expect(response).toBeDefined(); + + const baggage = response?.test_data.baggage?.split(',').sort(); + + expect(baggage).toContain('sentry-org_id=01234987'); +}); From a8c6e5655784cb2a87e33ccfd8ad55ab741c0ed2 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Thu, 15 May 2025 14:17:36 +0200 Subject: [PATCH 3/5] review suggestions --- .../baggage-org-id/server-no-org-id.ts | 34 +++++++++++++++++++ .../baggage-org-id/test.ts | 19 +++++++---- packages/core/src/types-hoist/options.ts | 7 ++-- packages/core/src/utils-hoist/dsn.ts | 5 +-- .../tracing/dynamicSamplingContext.test.ts | 8 ++--- 5 files changed, 55 insertions(+), 18 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/baggage-org-id/server-no-org-id.ts diff --git a/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/baggage-org-id/server-no-org-id.ts b/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/baggage-org-id/server-no-org-id.ts new file mode 100644 index 000000000000..5fe73e5451a9 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/baggage-org-id/server-no-org-id.ts @@ -0,0 +1,34 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport, startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; + +export type TestAPIResponse = { test_data: { host: string; 'sentry-trace': string; baggage: string } }; + +Sentry.init({ + dsn: 'https://public@public.ingest.sentry.io/1337', + release: '1.0', + environment: 'prod', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +import cors from 'cors'; +import express from 'express'; +import * as http from 'http'; + +const app = express(); + +app.use(cors()); + +app.get('/test/express', (_req, res) => { + const headers = http + .get({ + hostname: 'example.com', + }) + .getHeaders(); + + res.send({ test_data: headers }); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/baggage-org-id/test.ts b/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/baggage-org-id/test.ts index aeaa171da474..732473ac4880 100644 --- a/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/baggage-org-id/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/baggage-org-id/test.ts @@ -10,23 +10,28 @@ test('should include explicitly set org_id in the baggage header', async () => { const runner = createRunner(__dirname, 'server.ts').start(); const response = await runner.makeRequest('get', '/test/express'); - expect(response).toBeDefined(); - const baggage = response?.test_data.baggage?.split(',').sort(); - + const baggage = response?.test_data.baggage; expect(baggage).toContain('sentry-org_id=01234987'); }); test('should extract org_id from DSN host when not explicitly set', async () => { - // This test requires a new server with different configuration const runner = createRunner(__dirname, 'server-no-explicit-org-id.ts').start(); const response = await runner.makeRequest('get', '/test/express'); - expect(response).toBeDefined(); - const baggage = response?.test_data.baggage?.split(',').sort(); - + const baggage = response?.test_data.baggage; expect(baggage).toContain('sentry-org_id=01234987'); }); + +test('should set undefined org_id when it cannot be extracted', async () => { + const runner = createRunner(__dirname, 'server-no-org-id.ts').start(); + + const response = await runner.makeRequest('get', '/test/express'); + expect(response).toBeDefined(); + + const baggage = response?.test_data.baggage; + expect(baggage).not.toContain('sentry-org_id'); +}); diff --git a/packages/core/src/types-hoist/options.ts b/packages/core/src/types-hoist/options.ts index 093b6203dcd6..5f9b5d7f6daa 100644 --- a/packages/core/src/types-hoist/options.ts +++ b/packages/core/src/types-hoist/options.ts @@ -321,11 +321,12 @@ export interface ClientOptions { it('uses orgId from options when specified', () => { client = new TestClient( getDefaultTestClientOptions({ - orgId: 'explicit-org-id', + orgId: '00222111', dsn: 'https://public@sentry.example.com/1', }), ); const dsc = getDynamicSamplingContextFromClient(TRACE_ID, client); - expect(dsc.org_id).toBe('explicit-org-id'); + expect(dsc.org_id).toBe('00222111'); }); it('infers orgId from DSN host when not explicitly provided', () => { @@ -253,14 +253,14 @@ describe('getDynamicSamplingContextFromClient', () => { it('prioritizes explicit orgId over inferred from DSN', () => { client = new TestClient( getDefaultTestClientOptions({ - orgId: 'explicit-org-id', + orgId: '1234560', dsn: 'https://public@my-org.sentry.io/1', }), ); const dsc = getDynamicSamplingContextFromClient(TRACE_ID, client); - expect(dsc.org_id).toBe('explicit-org-id'); + expect(dsc.org_id).toBe('1234560'); }); it('handles missing DSN gracefully', () => { From 668f88cc8c6af348c0136fc02a58a5e1258c23c2 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Thu, 15 May 2025 14:33:00 +0200 Subject: [PATCH 4/5] also accept numbers for orgId --- packages/core/src/tracing/dynamicSamplingContext.ts | 2 +- packages/core/src/types-hoist/options.ts | 2 +- .../test/lib/tracing/dynamicSamplingContext.test.ts | 13 +++++++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index 4088f5ef9aaf..5f10f11db19c 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -49,7 +49,7 @@ export function getDynamicSamplingContextFromClient(trace_id: string, client: Cl let org_id: string | undefined; if (options.orgId) { - org_id = options.orgId; + org_id = String(options.orgId); } else if (host) { org_id = extractOrgIdFromDsnHost(host); } diff --git a/packages/core/src/types-hoist/options.ts b/packages/core/src/types-hoist/options.ts index 5f9b5d7f6daa..09dab550be4c 100644 --- a/packages/core/src/types-hoist/options.ts +++ b/packages/core/src/types-hoist/options.ts @@ -326,7 +326,7 @@ export interface ClientOptions { expect(dsc.org_id).toBe('1234560'); }); + it('handles orgId passed as number', () => { + client = new TestClient( + getDefaultTestClientOptions({ + dsn: 'https://public@my-org.sentry.io/1', + orgId: 123456, + }), + ); + + const dsc = getDynamicSamplingContextFromClient(TRACE_ID, client); + + expect(dsc.org_id).toBe('123456'); + }); + it('handles missing DSN gracefully', () => { client = new TestClient( getDefaultTestClientOptions({ From a955c09b3102772aca14a8d0c24d8162fea9b798 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Thu, 15 May 2025 14:50:32 +0200 Subject: [PATCH 5/5] fix tests and add size limit --- .size-limit.js | 2 +- .../browser/test/tracing/browserTracingIntegration.test.ts | 3 +++ packages/core/test/lib/tracing/dynamicSamplingContext.test.ts | 4 ++++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/.size-limit.js b/.size-limit.js index 0c03c0ff1b8b..10efb849a582 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -8,7 +8,7 @@ module.exports = [ path: 'packages/browser/build/npm/esm/index.js', import: createImport('init'), gzip: true, - limit: '24 KB', + limit: '25 KB', }, { name: '@sentry/browser - with treeshaking flags', diff --git a/packages/browser/test/tracing/browserTracingIntegration.test.ts b/packages/browser/test/tracing/browserTracingIntegration.test.ts index 728bee5fd1dd..b3686272d12e 100644 --- a/packages/browser/test/tracing/browserTracingIntegration.test.ts +++ b/packages/browser/test/tracing/browserTracingIntegration.test.ts @@ -732,6 +732,7 @@ describe('browserTracingIntegration', () => { sampleRand: expect.any(Number), dsc: { release: undefined, + org_id: undefined, environment: 'production', public_key: 'examplePublicKey', sample_rate: '1', @@ -773,6 +774,7 @@ describe('browserTracingIntegration', () => { sampleRand: expect.any(Number), dsc: { release: undefined, + org_id: undefined, environment: 'production', public_key: 'examplePublicKey', sample_rate: '0', @@ -898,6 +900,7 @@ describe('browserTracingIntegration', () => { expect(dynamicSamplingContext).toBeDefined(); expect(dynamicSamplingContext).toStrictEqual({ release: undefined, + org_id: undefined, environment: 'production', public_key: 'examplePublicKey', sample_rate: '1', diff --git a/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts b/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts index 1bcde98e0a3a..65fcf5a71c8e 100644 --- a/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts +++ b/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts @@ -73,6 +73,7 @@ describe('getDynamicSamplingContextFromSpan', () => { expect(dynamicSamplingContext).toStrictEqual({ public_key: undefined, + org_id: undefined, release: '1.0.1', environment: 'production', sampled: 'true', @@ -92,6 +93,7 @@ describe('getDynamicSamplingContextFromSpan', () => { expect(dynamicSamplingContext).toStrictEqual({ public_key: undefined, + org_id: undefined, release: '1.0.1', environment: 'production', sampled: 'true', @@ -116,6 +118,7 @@ describe('getDynamicSamplingContextFromSpan', () => { expect(dynamicSamplingContext).toStrictEqual({ public_key: undefined, + org_id: undefined, release: '1.0.1', environment: 'production', sampled: 'true', @@ -172,6 +175,7 @@ describe('getDynamicSamplingContextFromSpan', () => { expect(dynamicSamplingContext).toStrictEqual({ public_key: undefined, + org_id: undefined, release: '1.0.1', environment: 'production', trace_id: expect.stringMatching(/^[a-f0-9]{32}$/),