diff --git a/.size-limit.js b/.size-limit.js index e97e15f6cd4e..355e484c5bd1 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -40,7 +40,7 @@ module.exports = [ path: 'packages/browser/build/npm/esm/index.js', import: createImport('init', 'browserTracingIntegration'), gzip: true, - limit: '36.5 KB', + limit: '37.5 KB', }, { name: '@sentry/browser (incl. Tracing, Replay)', @@ -124,7 +124,7 @@ module.exports = [ import: createImport('init', 'ErrorBoundary', 'reactRouterV6BrowserTracingIntegration'), ignore: ['react/jsx-runtime'], gzip: true, - limit: '39.5 KB', + limit: '40.5 KB', }, // Vue SDK (ESM) { @@ -139,7 +139,7 @@ module.exports = [ path: 'packages/vue/build/esm/index.js', import: createImport('init', 'browserTracingIntegration'), gzip: true, - limit: '38.5 KB', + limit: '39.5 KB', }, // Svelte SDK (ESM) { @@ -219,7 +219,7 @@ module.exports = [ import: createImport('init'), ignore: ['$app/stores'], gzip: true, - limit: '37 KB', + limit: '38 KB', }, // Node SDK (ESM) { diff --git a/docs/migration/draft-v9-migration-guide.md b/docs/migration/draft-v9-migration-guide.md index c543009d4995..763d0765cff5 100644 --- a/docs/migration/draft-v9-migration-guide.md +++ b/docs/migration/draft-v9-migration-guide.md @@ -2,6 +2,22 @@ # Deprecations +## General + +- **Passing `undefined` to `tracesSampleRate` / `tracesSampler` / `enableTracing` will be handled differently in v9** + +In v8, a setup like the following: + +```ts +Sentry.init({ + tracesSampleRate: undefined, +}); +``` + +Will result in tracing being _enabled_, although no spans will be generated. +In v9, we will streamline this behavior so that passing `undefined` will result in tracing being disabled, the same as not passing the option at all. +If you are relying on `undefined` being passed in and having tracing enabled because of this, you should update your config to set e.g. `tracesSampleRate: 0` instead, which will also enable tracing in v9. + ## `@sentry/utils` - **The `@sentry/utils` package has been deprecated. Import everything from `@sentry/core` instead.** diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 2387523d46ee..d86f170ca25d 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -46,7 +46,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 { logger } from './utils-hoist/logger'; +import { consoleSandbox, logger } from './utils-hoist/logger'; import { checkOrSetAlreadyCaught, uuid4 } from './utils-hoist/misc'; import { dropUndefinedKeys } from './utils-hoist/object'; import { SyncPromise, rejectedSyncPromise, resolvedSyncPromise } from './utils-hoist/syncpromise'; @@ -142,6 +142,18 @@ 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/prepareEvent.ts b/packages/core/src/utils/prepareEvent.ts index bf23a0e3ee79..98f64adbdd62 100644 --- a/packages/core/src/utils/prepareEvent.ts +++ b/packages/core/src/utils/prepareEvent.ts @@ -129,23 +129,26 @@ export function prepareEvent( } /** - * Enhances event using the client configuration. - * It takes care of all "static" values like environment, release and `dist`, - * as well as truncating overly long values. + * Enhances event using the client configuration. + * It takes care of all "static" values like environment, release and `dist`, + * as well as truncating overly long values. + * + * Only exported for tests. + * * @param event event instance to be enhanced */ -function applyClientOptions(event: Event, options: ClientOptions): void { +export function applyClientOptions(event: Event, options: ClientOptions): void { const { environment, release, dist, maxValueLength = 250 } = options; - if (!('environment' in event)) { - event.environment = 'environment' in options ? environment : DEFAULT_ENVIRONMENT; - } + // empty strings do not make sense for environment, release, and dist + // so we handle them the same as if they were not provided + event.environment = event.environment || environment || DEFAULT_ENVIRONMENT; - if (event.release === undefined && release !== undefined) { + if (!event.release && release) { event.release = release; } - if (event.dist === undefined && dist !== undefined) { + if (!event.dist && dist) { event.dist = dist; } diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/baseclient.test.ts similarity index 96% rename from packages/core/test/lib/base.test.ts rename to packages/core/test/lib/baseclient.test.ts index 296a496d2d40..4e2bcdb334ef 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/baseclient.test.ts @@ -77,6 +77,56 @@ describe('BaseClient', () => { }); }); + describe('constructor() / warnings', () => { + test('does not warn for defaults', () => { + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => undefined); + + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); + new TestClient(options); + + 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()', () => { test('returns the options', () => { expect.assertions(1); @@ -552,7 +602,7 @@ describe('BaseClient', () => { ); }); - test('allows for environment to be explicitly set to falsy value', () => { + test('uses default environment when set to falsy value', () => { expect.assertions(1); const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, environment: undefined }); @@ -563,7 +613,7 @@ describe('BaseClient', () => { expect(TestClient.instance!.event!).toEqual( expect.objectContaining({ - environment: undefined, + environment: 'production', event_id: '42', message: 'message', timestamp: 2020, @@ -1122,6 +1172,8 @@ describe('BaseClient', () => { }); test('calls `beforeSendSpan` and discards the span', () => { + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => undefined); + const beforeSendSpan = jest.fn(() => null); const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendSpan }); const client = new TestClient(options); @@ -1150,6 +1202,12 @@ describe('BaseClient', () => { const capturedEvent = TestClient.instance!.event!; expect(capturedEvent.spans).toHaveLength(0); expect(client['_outcomes']).toEqual({ 'before_send:span': 2 }); + + expect(consoleWarnSpy).toHaveBeenCalledTimes(1); + expect(consoleWarnSpy).toBeCalledWith( + '[Sentry] Deprecation warning: Returning null from `beforeSendSpan` will be disallowed from SDK version 9.0.0 onwards. The callback will only support mutating spans. To drop certain spans, configure the respective integrations directly.', + ); + consoleWarnSpy.mockRestore(); }); test('calls `beforeSend` and logs info about invalid return value', () => { diff --git a/packages/core/test/lib/prepareEvent.test.ts b/packages/core/test/lib/prepareEvent.test.ts index a905d948475a..0322afb7e7a8 100644 --- a/packages/core/test/lib/prepareEvent.test.ts +++ b/packages/core/test/lib/prepareEvent.test.ts @@ -12,6 +12,7 @@ import { GLOBAL_OBJ, createStackParser, getGlobalScope, getIsolationScope } from import { Scope } from '../../src/scope'; import { + applyClientOptions, applyDebugIds, applyDebugMeta, parseEventHintOrCaptureContext, @@ -518,3 +519,136 @@ describe('prepareEvent', () => { }); }); }); + +describe('applyClientOptions', () => { + it('works with defaults', () => { + const event: Event = {}; + const options = {} as ClientOptions; + + applyClientOptions(event, options); + + expect(event).toEqual({ + environment: 'production', + }); + + // These should not be set at all on the event + expect('release' in event).toBe(false); + expect('dist' in event).toBe(false); + }); + + it('works with event data and no options', () => { + const event: Event = { + environment: 'blub', + release: 'blab', + dist: 'blib', + }; + const options = {} as ClientOptions; + + applyClientOptions(event, options); + + expect(event).toEqual({ + environment: 'blub', + release: 'blab', + dist: 'blib', + }); + }); + + it('event data has precedence over options', () => { + const event: Event = { + environment: 'blub', + release: 'blab', + dist: 'blib', + }; + const options = { + environment: 'blub2', + release: 'blab2', + dist: 'blib2', + } as ClientOptions; + + applyClientOptions(event, options); + + expect(event).toEqual({ + environment: 'blub', + release: 'blab', + dist: 'blib', + }); + }); + + it('option data is used if no event data exists', () => { + const event: Event = {}; + const options = { + environment: 'blub2', + release: 'blab2', + dist: 'blib2', + } as ClientOptions; + + applyClientOptions(event, options); + + expect(event).toEqual({ + environment: 'blub2', + release: 'blab2', + dist: 'blib2', + }); + }); + + it('option data is ignored if empty string', () => { + const event: Event = {}; + const options = { + environment: '', + release: '', + dist: '', + } as ClientOptions; + + applyClientOptions(event, options); + + expect(event).toEqual({ + environment: 'production', + }); + + // These should not be set at all on the event + expect('release' in event).toBe(false); + expect('dist' in event).toBe(false); + }); + + it('option data is used if event data is undefined', () => { + const event: Event = { + environment: undefined, + release: undefined, + dist: undefined, + }; + const options = { + environment: 'blub2', + release: 'blab2', + dist: 'blib2', + } as ClientOptions; + + applyClientOptions(event, options); + + expect(event).toEqual({ + environment: 'blub2', + release: 'blab2', + dist: 'blib2', + }); + }); + + it('option data is used if event data is empty string', () => { + const event: Event = { + environment: '', + release: '', + dist: '', + }; + const options = { + environment: 'blub2', + release: 'blab2', + dist: 'blib2', + } as ClientOptions; + + applyClientOptions(event, options); + + expect(event).toEqual({ + environment: 'blub2', + release: 'blab2', + dist: 'blib2', + }); + }); +}); diff --git a/packages/core/test/lib/tracing/sentrySpan.test.ts b/packages/core/test/lib/tracing/sentrySpan.test.ts index 447e42328a2e..9a1c9f69255c 100644 --- a/packages/core/test/lib/tracing/sentrySpan.test.ts +++ b/packages/core/test/lib/tracing/sentrySpan.test.ts @@ -161,6 +161,8 @@ describe('SentrySpan', () => { }); test('does not send the span if `beforeSendSpan` drops the span', () => { + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => undefined); + const beforeSendSpan = jest.fn(() => null); const client = new TestClient( getDefaultTestClientOptions({ @@ -185,6 +187,12 @@ describe('SentrySpan', () => { expect(mockSend).not.toHaveBeenCalled(); expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'span'); + + expect(consoleWarnSpy).toHaveBeenCalledTimes(1); + expect(consoleWarnSpy).toBeCalledWith( + '[Sentry] Deprecation warning: Returning null from `beforeSendSpan` will be disallowed from SDK version 9.0.0 onwards. The callback will only support mutating spans. To drop certain spans, configure the respective integrations directly.', + ); + consoleWarnSpy.mockRestore(); }); });