diff --git a/docs/migration/v8-to-v9.md b/docs/migration/v8-to-v9.md index c18131900393..60ac0bb4f535 100644 --- a/docs/migration/v8-to-v9.md +++ b/docs/migration/v8-to-v9.md @@ -68,6 +68,16 @@ Sentry.init({ }); ``` +- In previous versions, we determined if tracing is enabled (for Tracing Without Performance) by checking if either `tracesSampleRate` or `traceSampler` are _defined_ at all, in `Sentry.init()`. This means that e.g. the following config would lead to tracing without performance (=tracing being enabled, even if no spans would be started): + +```js +Sentry.init({ + tracesSampleRate: undefined, +}); +``` + +In v9, an `undefined` value will be treated the same as if the value is not defined at all. You'll need to set `tracesSampleRate: 0` if you want to enable tracing without performance. + ### `@sentry/node` - When `skipOpenTelemetrySetup: true` is configured, `httpIntegration({ spans: false })` will be configured by default. This means that you no longer have to specify this yourself in this scenario. With this change, no spans are emitted once `skipOpenTelemetrySetup: true` is configured, without any further configuration being needed. diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts index 163e17b014d7..0b3eb7f5ac00 100644 --- a/packages/browser/src/sdk.ts +++ b/packages/browser/src/sdk.ts @@ -1,3 +1,4 @@ +import type { Client, DsnLike, Integration, Options } from '@sentry/core'; import { consoleSandbox, dedupeIntegration, @@ -12,7 +13,6 @@ import { stackParserFromStackParserOptions, supportsFetch, } from '@sentry/core'; -import type { Client, DsnLike, Integration, Options } from '@sentry/core'; import type { BrowserClientOptions, BrowserOptions } from './client'; import { BrowserClient } from './client'; import { DEBUG_BUILD } from './debug-build'; @@ -51,7 +51,8 @@ export function getDefaultIntegrations(options: Options): Integration[] { return integrations; } -function applyDefaultOptions(optionsArg: BrowserOptions = {}): BrowserOptions { +/** Exported only for tests. */ +export function applyDefaultOptions(optionsArg: BrowserOptions = {}): BrowserOptions { const defaultOptions: BrowserOptions = { defaultIntegrations: getDefaultIntegrations(optionsArg), release: @@ -64,15 +65,27 @@ function applyDefaultOptions(optionsArg: BrowserOptions = {}): BrowserOptions { sendClientReports: true, }; - // TODO: Instead of dropping just `defaultIntegrations`, we should simply - // call `dropUndefinedKeys` on the entire `optionsArg`. - // However, for this to work we need to adjust the `hasTracingEnabled()` logic - // first as it differentiates between `undefined` and the key not being in the object. - if (optionsArg.defaultIntegrations == null) { - delete optionsArg.defaultIntegrations; + return { + ...defaultOptions, + ...dropTopLevelUndefinedKeys(optionsArg), + }; +} + +/** + * In contrast to the regular `dropUndefinedKeys` method, + * this one does not deep-drop keys, but only on the top level. + */ +function dropTopLevelUndefinedKeys(obj: T): Partial { + const mutatetedObj: Partial = {}; + + for (const k of Object.getOwnPropertyNames(obj)) { + const key = k as keyof T; + if (obj[key] !== undefined) { + mutatetedObj[key] = obj[key]; + } } - return { ...defaultOptions, ...optionsArg }; + return mutatetedObj; } type ExtensionProperties = { diff --git a/packages/browser/test/sdk.test.ts b/packages/browser/test/sdk.test.ts index 7cb69541086b..d3dee47741be 100644 --- a/packages/browser/test/sdk.test.ts +++ b/packages/browser/test/sdk.test.ts @@ -13,7 +13,7 @@ import type { Client, Integration } from '@sentry/core'; import type { BrowserOptions } from '../src'; import { WINDOW } from '../src'; -import { init } from '../src/sdk'; +import { applyDefaultOptions, getDefaultIntegrations, init } from '../src/sdk'; const PUBLIC_DSN = 'https://username@domain/123'; @@ -277,3 +277,100 @@ describe('init', () => { expect(client).not.toBeUndefined(); }); }); + +describe('applyDefaultOptions', () => { + test('it works with empty options', () => { + const options = {}; + const actual = applyDefaultOptions(options); + + expect(actual).toEqual({ + defaultIntegrations: expect.any(Array), + release: undefined, + autoSessionTracking: true, + sendClientReports: true, + }); + + expect(actual.defaultIntegrations && actual.defaultIntegrations.map(i => i.name)).toEqual( + getDefaultIntegrations(options).map(i => i.name), + ); + }); + + test('it works with options', () => { + const options = { + tracesSampleRate: 0.5, + release: '1.0.0', + autoSessionTracking: false, + }; + const actual = applyDefaultOptions(options); + + expect(actual).toEqual({ + defaultIntegrations: expect.any(Array), + release: '1.0.0', + autoSessionTracking: false, + sendClientReports: true, + tracesSampleRate: 0.5, + }); + + expect(actual.defaultIntegrations && actual.defaultIntegrations.map(i => i.name)).toEqual( + getDefaultIntegrations(options).map(i => i.name), + ); + }); + + test('it works with defaultIntegrations=false', () => { + const options = { + defaultIntegrations: false, + } as const; + const actual = applyDefaultOptions(options); + + expect(actual.defaultIntegrations).toStrictEqual(false); + }); + + test('it works with defaultIntegrations=[]', () => { + const options = { + defaultIntegrations: [], + }; + const actual = applyDefaultOptions(options); + + expect(actual.defaultIntegrations).toEqual([]); + }); + + test('it works with tracesSampleRate=undefined', () => { + const options = { + tracesSampleRate: undefined, + } as const; + const actual = applyDefaultOptions(options); + + // Not defined, not even undefined + expect('tracesSampleRate' in actual).toBe(false); + }); + + test('it works with tracesSampleRate=null', () => { + const options = { + tracesSampleRate: null, + } as any; + const actual = applyDefaultOptions(options); + + expect(actual.tracesSampleRate).toStrictEqual(null); + }); + + test('it works with tracesSampleRate=0', () => { + const options = { + tracesSampleRate: 0, + } as const; + const actual = applyDefaultOptions(options); + + expect(actual.tracesSampleRate).toStrictEqual(0); + }); + + test('it does not deep-drop undefined keys', () => { + const options = { + obj: { + prop: undefined, + }, + } as any; + const actual = applyDefaultOptions(options) as any; + + expect('prop' in actual.obj).toBe(true); + expect(actual.obj.prop).toStrictEqual(undefined); + }); +}); diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 80badfe3fa9d..32cef2752509 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -47,7 +47,7 @@ import { dsnToString, makeDsn } from './utils-hoist/dsn'; import { addItemToEnvelope, createAttachmentEnvelopeItem } from './utils-hoist/envelope'; import { SentryError } from './utils-hoist/error'; import { isParameterizedString, isPlainObject, isPrimitive, isThenable } from './utils-hoist/is'; -import { consoleSandbox, logger } from './utils-hoist/logger'; +import { logger } from './utils-hoist/logger'; import { checkOrSetAlreadyCaught, uuid4 } from './utils-hoist/misc'; import { SyncPromise, rejectedSyncPromise, resolvedSyncPromise } from './utils-hoist/syncpromise'; import { getPossibleEventMessages } from './utils/eventUtils'; @@ -144,18 +144,6 @@ export abstract class BaseClient implements Client { url, }); } - - // TODO(v9): Remove this deprecation warning - const tracingOptions = ['enableTracing', 'tracesSampleRate', 'tracesSampler'] as const; - const undefinedOption = tracingOptions.find(option => option in options && options[option] == undefined); - if (undefinedOption) { - consoleSandbox(() => { - // eslint-disable-next-line no-console - console.warn( - `[Sentry] Deprecation warning: \`${undefinedOption}\` is set to undefined, which leads to tracing being enabled. In v9, a value of \`undefined\` will result in tracing being disabled.`, - ); - }); - } } /** diff --git a/packages/core/src/utils/hasTracingEnabled.ts b/packages/core/src/utils/hasTracingEnabled.ts index 6d99eede931e..65c7f16701ea 100644 --- a/packages/core/src/utils/hasTracingEnabled.ts +++ b/packages/core/src/utils/hasTracingEnabled.ts @@ -19,5 +19,5 @@ export function hasTracingEnabled( const client = getClient(); const options = maybeOptions || (client && client.getOptions()); // eslint-disable-next-line deprecation/deprecation - return !!options && (options.enableTracing || 'tracesSampleRate' in options || 'tracesSampler' in options); + return !!options && (options.enableTracing || options.tracesSampleRate != null || !!options.tracesSampler); } diff --git a/packages/core/test/lib/baseclient.test.ts b/packages/core/test/lib/baseclient.test.ts index 0432235a17a5..d30ab7bb626b 100644 --- a/packages/core/test/lib/baseclient.test.ts +++ b/packages/core/test/lib/baseclient.test.ts @@ -87,44 +87,6 @@ describe('BaseClient', () => { expect(consoleWarnSpy).toHaveBeenCalledTimes(0); consoleWarnSpy.mockRestore(); }); - - describe.each(['tracesSampleRate', 'tracesSampler', 'enableTracing'])('%s', key => { - it('warns when set to undefined', () => { - const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => undefined); - - const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, [key]: undefined }); - new TestClient(options); - - expect(consoleWarnSpy).toHaveBeenCalledTimes(1); - expect(consoleWarnSpy).toBeCalledWith( - `[Sentry] Deprecation warning: \`${key}\` is set to undefined, which leads to tracing being enabled. In v9, a value of \`undefined\` will result in tracing being disabled.`, - ); - consoleWarnSpy.mockRestore(); - }); - - it('warns when set to null', () => { - const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => undefined); - - const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, [key]: null }); - new TestClient(options); - - expect(consoleWarnSpy).toHaveBeenCalledTimes(1); - expect(consoleWarnSpy).toBeCalledWith( - `[Sentry] Deprecation warning: \`${key}\` is set to undefined, which leads to tracing being enabled. In v9, a value of \`undefined\` will result in tracing being disabled.`, - ); - consoleWarnSpy.mockRestore(); - }); - - it('does not warn when set to 0', () => { - const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => undefined); - - const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, [key]: 0 }); - new TestClient(options); - - expect(consoleWarnSpy).toHaveBeenCalledTimes(0); - consoleWarnSpy.mockRestore(); - }); - }); }); describe('getOptions()', () => { diff --git a/packages/core/test/lib/utils/hasTracingEnabled.test.ts b/packages/core/test/lib/utils/hasTracingEnabled.test.ts index a03ff25c9be9..3ae7066ac0f0 100644 --- a/packages/core/test/lib/utils/hasTracingEnabled.test.ts +++ b/packages/core/test/lib/utils/hasTracingEnabled.test.ts @@ -10,6 +10,10 @@ describe('hasTracingEnabled', () => { ['With tracesSampleRate', { tracesSampleRate }, true], ['With enableTracing=true', { enableTracing: true }, true], ['With enableTracing=false', { enableTracing: false }, false], + ['With enableTracing=undefined', { enableTracing: undefined }, false], + ['With tracesSampleRate=undefined', { tracesSampleRate: undefined }, false], + ['With tracesSampleRate=0', { tracesSampleRate: 0 }, true], + ['With tracesSampler=undefined', { tracesSampler: undefined }, false], ['With tracesSampler && enableTracing=false', { tracesSampler, enableTracing: false }, true], ['With tracesSampleRate && enableTracing=false', { tracesSampler, enableTracing: false }, true], ['With tracesSampler and tracesSampleRate', { tracesSampler, tracesSampleRate }, true],