getsentry · mydea · Nov 25, 2024 · Nov 25, 2024 · Nov 25, 2024 · Nov 25, 2024
diff --git 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
@@ -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
@@ -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<O extends ClientOptions> implements Client<O> {
         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
@@ -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 → packages/core/test/lib/baseclient.test.ts b/packages/core/test/lib/base.test.ts → 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
@@ -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
@@ -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();
     });
   });