Merge branch 'develop' into kw-add-http-response-status-code

getsentry · Jun 27, 2023 · 0fc01d3 · 0fc01d3
2 parents fe7330c + ac435dd
commit 0fc01d3
Show file tree

Hide file tree

Showing 2 changed files with 90 additions and 1 deletion.
diff --git a/packages/tracing-internal/src/browser/browsertracing.ts b/packages/tracing-internal/src/browser/browsertracing.ts
@@ -177,9 +177,19 @@ export class BrowserTracing implements Integration {
 
   private _collectWebVitals: () => void;
 
+  private _hasSetTracePropagationTargets: boolean = false;
+
   public constructor(_options?: Partial<BrowserTracingOptions>) {
     addTracingExtensions();
 
+    if (__DEBUG_BUILD__) {
+      this._hasSetTracePropagationTargets = !!(
+        _options &&
+        // eslint-disable-next-line deprecation/deprecation
+        (_options.tracePropagationTargets || _options.tracingOrigins)
+      );
+    }
+
     this.options = {
       ...DEFAULT_BROWSER_TRACING_OPTIONS,
       ..._options,
@@ -214,6 +224,9 @@ export class BrowserTracing implements Integration {
    */
   public setupOnce(_: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
     this._getCurrentHub = getCurrentHub;
+    const hub = getCurrentHub();
+    const client = hub.getClient();
+    const clientOptions = client && client.getOptions();
 
     const {
       routingInstrumentation: instrumentRouting,
@@ -222,11 +235,28 @@ export class BrowserTracing implements Integration {
       markBackgroundTransactions,
       traceFetch,
       traceXHR,
-      tracePropagationTargets,
       shouldCreateSpanForRequest,
       _experiments,
     } = this.options;
 
+    const clientOptionsTracePropagationTargets = clientOptions && clientOptions.tracePropagationTargets;
+    // There are three ways to configure tracePropagationTargets:
+    // 1. via top level client option `tracePropagationTargets`
+    // 2. via BrowserTracing option `tracePropagationTargets`
+    // 3. via BrowserTracing option `tracingOrigins` (deprecated)
+    //
+    // To avoid confusion, favour top level client option `tracePropagationTargets`, and fallback to
+    // BrowserTracing option `tracePropagationTargets` and then `tracingOrigins` (deprecated).
+    // This is done as it minimizes bundle size (we don't have to have undefined checks).
+    //
+    // If both 1 and either one of 2 or 3 are set (from above), we log out a warning.
+    const tracePropagationTargets = clientOptionsTracePropagationTargets || this.options.tracePropagationTargets;
+    if (__DEBUG_BUILD__ && this._hasSetTracePropagationTargets && clientOptionsTracePropagationTargets) {
+      logger.warn(
+        '[Tracing] The `tracePropagationTargets` option was set in the BrowserTracing integration and top level `Sentry.init`. The top level `Sentry.init` value is being used.',
+      );
+    }
+
     instrumentRouting(
       (context: TransactionContext) => {
         const transaction = this._createRouteTransaction(context);

diff --git a/packages/tracing-internal/test/browser/browsertracing.test.ts b/packages/tracing-internal/test/browser/browsertracing.test.ts
@@ -250,6 +250,65 @@ describe('BrowserTracing', () => {
           tracePropagationTargets: ['something'],
         });
       });
+
+      it('uses `tracePropagationTargets` set by client over integration set targets', () => {
+        jest.clearAllMocks();
+        hub.getClient()!.getOptions().tracePropagationTargets = ['something-else'];
+        const sampleTracePropagationTargets = ['something'];
+        createBrowserTracing(true, {
+          routingInstrumentation: customInstrumentRouting,
+          tracePropagationTargets: sampleTracePropagationTargets,
+        });
+
+        expect(instrumentOutgoingRequestsMock).toHaveBeenCalledWith({
+          traceFetch: true,
+          traceXHR: true,
+          tracePropagationTargets: ['something-else'],
+        });
+      });
+
+      it.each([
+        [true, 'tracePropagationTargets', 'defined', { tracePropagationTargets: ['something'] }],
+        [false, 'tracePropagationTargets', 'undefined', { tracePropagationTargets: undefined }],
+        [true, 'tracingOrigins', 'defined', { tracingOrigins: ['something'] }],
+        [false, 'tracingOrigins', 'undefined', { tracingOrigins: undefined }],
+        [
+          true,
+          'tracePropagationTargets and tracingOrigins',
+          'defined',
+          { tracePropagationTargets: ['something'], tracingOrigins: ['something-else'] },
+        ],
+        [
+          false,
+          'tracePropagationTargets and tracingOrigins',
+          'undefined',
+          { tracePropagationTargets: undefined, tracingOrigins: undefined },
+        ],
+        [
+          true,
+          'tracePropagationTargets and tracingOrigins',
+          'defined and undefined',
+          { tracePropagationTargets: ['something'], tracingOrigins: undefined },
+        ],
+        [
+          true,
+          'tracePropagationTargets and tracingOrigins',
+          'undefined and defined',
+          { tracePropagationTargets: undefined, tracingOrigins: ['something'] },
+        ],
+      ])(
+        'sets `_hasSetTracePropagationTargets` to %s if %s is %s',
+        (hasSet: boolean, _: string, __: string, options: Partial<BrowserTracingOptions>) => {
+          jest.clearAllMocks();
+          const inst = createBrowserTracing(true, {
+            routingInstrumentation: customInstrumentRouting,
+            ...options,
+          });
+
+          // @ts-ignore accessing private property
+          expect(inst._hasSetTracePropagationTargets).toBe(hasSet);
+        },
+      );
     });
 
     describe('beforeNavigate', () => {