feat(core)!: Update hasTracingEnabled to consider empty trace config (#14857)

This PR updates the behavior of passing `undefined` to
`tracesSampleRate` (or `enabledTracing`). now, this will _not_ trigger
TWP, but tracing will be disabled. If you really want TWP, you need to
configure `tracesSampleRate: 0`.

Closes #13262

Author: mydea

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.

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<T extends object>(obj: T): Partial<T> {
+  const mutatetedObj: Partial<T> = {};
+
+  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 = {

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);
+  });
+});

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<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.`,
-        );
-      });
-    }
   }
 
   /**

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);
 }

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()', () => {

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],