Skip to content

Commit

Permalink
Add warning if both options are set
Browse files Browse the repository at this point in the history
  • Loading branch information
AbhiPrasad committed Jun 26, 2023
1 parent cd93c1c commit b098188
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 2 deletions.
29 changes: 27 additions & 2 deletions packages/tracing-internal/src/browser/browsertracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -229,8 +239,23 @@ export class BrowserTracing implements Integration {
_experiments,
} = this.options;

const tracePropagationTargets =
(clientOptions && clientOptions.tracePropagationTargets) || this.options.tracePropagationTargets;
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) => {
Expand Down
43 changes: 43 additions & 0 deletions packages/tracing-internal/test/browser/browsertracing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,49 @@ describe('BrowserTracing', () => {
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', () => {
Expand Down

0 comments on commit b098188

Please sign in to comment.