Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@

- Change log output to show what paths are considered when collecting modules ([#3316](https://github.com/getsentry/sentry-react-native/pull/3316))
- Ignore defaults when warning about duplicate definition of trace propagation targets ([#3327](https://github.com/getsentry/sentry-react-native/pull/3327))
- Use deprecated `ReactNativeTracingOptions.tracingOrigins` if set in the options ([#3331](https://github.com/getsentry/sentry-react-native/pull/3331))

### Dependencies

Expand Down
35 changes: 23 additions & 12 deletions src/js/tracing/reactnativetracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ export interface ReactNativeTracingOptions extends RequestInstrumentationOptions
enableUserInteractionTracing: boolean;
}

const DEFAULT_TRACE_PROPAGATION_TARGETS = ['localhost', /^\/(?!\/)/];

const defaultReactNativeTracingOptions: ReactNativeTracingOptions = {
...defaultRequestInstrumentationOptions,
idleTimeout: 1000,
Expand Down Expand Up @@ -137,17 +139,19 @@ export class ReactNativeTracing implements Integration {
private _appStartFinishTimestamp?: number;
private _currentRoute?: string;
private _hasSetTracePropagationTargets: boolean;
private _hasSetTracingOrigins: boolean;

public constructor(options: Partial<ReactNativeTracingOptions> = {}) {
this._hasSetTracePropagationTargets = false;

if (__DEV__) {
this._hasSetTracePropagationTargets = !!(
options &&
// eslint-disable-next-line deprecation/deprecation
(options.tracePropagationTargets || options.tracingOrigins)
);
}
this._hasSetTracePropagationTargets = !!(
options &&
// eslint-disable-next-line deprecation/deprecation
options.tracePropagationTargets
);
this._hasSetTracingOrigins = !!(
options &&
// eslint-disable-next-line deprecation/deprecation
options.tracingOrigins
);

this.options = {
...defaultReactNativeTracingOptions,
Expand Down Expand Up @@ -203,8 +207,16 @@ export class ReactNativeTracing implements Integration {
// ReactNativeTracing option `tracePropagationTargets` and then `tracingOrigins` (deprecated).
//
// If both 1 and either one of 2 or 3 are set (from above), we log out a warning.
const tracePropagationTargets = clientOptionsTracePropagationTargets || thisOptionsTracePropagationTargets;
if (__DEV__ && this._hasSetTracePropagationTargets && clientOptionsTracePropagationTargets) {
const tracePropagationTargets =
clientOptionsTracePropagationTargets ||
(this._hasSetTracePropagationTargets && thisOptionsTracePropagationTargets) ||
(this._hasSetTracingOrigins && tracingOrigins) ||
DEFAULT_TRACE_PROPAGATION_TARGETS;
if (
__DEV__ &&
(this._hasSetTracePropagationTargets || this._hasSetTracingOrigins) &&
clientOptionsTracePropagationTargets
) {
logger.warn(
'[ReactNativeTracing] The `tracePropagationTargets` option was set in the ReactNativeTracing integration and top level `Sentry.init`. The top level `Sentry.init` value is being used.',
);
Expand Down Expand Up @@ -246,7 +258,6 @@ export class ReactNativeTracing implements Integration {
instrumentOutgoingRequests({
traceFetch,
traceXHR,
tracingOrigins,
shouldCreateSpanForRequest,
tracePropagationTargets,
});
Expand Down
146 changes: 145 additions & 1 deletion test/tracing/reactnativetracing.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import type { SpanStatusType, User } from '@sentry/browser';
import { BrowserClient } from '@sentry/browser';
import * as SentryBrowser from '@sentry/browser';
import type { IdleTransaction } from '@sentry/core';
import { addGlobalEventProcessor, Hub, Transaction } from '@sentry/core';

import type { NativeAppStartResponse } from '../../src/js/NativeRNSentry';
import { RoutingInstrumentation } from '../../src/js/tracing/routingInstrumentation';

const BrowserClient = SentryBrowser.BrowserClient;

jest.mock('../../src/js/wrapper', () => {
return {
NATIVE: {
Expand Down Expand Up @@ -117,6 +119,148 @@ describe('ReactNativeTracing', () => {
jest.clearAllMocks();
});

describe('trace propagation targets', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l/recommendation: Afaik, in the JS SDK we have tests that show which option is taken if multiple of these properties are specified. Admittedly it's an edge case, so feel free to disregard (not a blocker).

it('uses tracingOrigins', () => {
const instrumentOutgoingRequests = jest.spyOn(SentryBrowser, 'instrumentOutgoingRequests');
const mockedHub = {
getClient: () => ({
getOptions: () => ({}),
}),
};

const integration = new ReactNativeTracing({
tracingOrigins: ['test1', 'test2'],
});
integration.setupOnce(
() => {},
() => mockedHub as unknown as Hub,
);

expect(instrumentOutgoingRequests).toBeCalledWith(
expect.objectContaining({
tracePropagationTargets: ['test1', 'test2'],
}),
);
});

it('uses tracePropagationTargets', () => {
const instrumentOutgoingRequests = jest.spyOn(SentryBrowser, 'instrumentOutgoingRequests');
const mockedHub = {
getClient: () => ({
getOptions: () => ({}),
}),
};

const integration = new ReactNativeTracing({
tracePropagationTargets: ['test1', 'test2'],
});
integration.setupOnce(
() => {},
() => mockedHub as unknown as Hub,
);

expect(instrumentOutgoingRequests).toBeCalledWith(
expect.objectContaining({
tracePropagationTargets: ['test1', 'test2'],
}),
);
});

it('uses tracePropagationTargets from client options', () => {
const instrumentOutgoingRequests = jest.spyOn(SentryBrowser, 'instrumentOutgoingRequests');
const mockedHub = {
getClient: () => ({
getOptions: () => ({
tracePropagationTargets: ['test1', 'test2'],
}),
}),
};

const integration = new ReactNativeTracing({});
integration.setupOnce(
() => {},
() => mockedHub as unknown as Hub,
);

expect(instrumentOutgoingRequests).toBeCalledWith(
expect.objectContaining({
tracePropagationTargets: ['test1', 'test2'],
}),
);
});

it('uses defaults', () => {
const instrumentOutgoingRequests = jest.spyOn(SentryBrowser, 'instrumentOutgoingRequests');
const mockedHub = {
getClient: () => ({
getOptions: () => ({}),
}),
};

const integration = new ReactNativeTracing({});
integration.setupOnce(
() => {},
() => mockedHub as unknown as Hub,
);

expect(instrumentOutgoingRequests).toBeCalledWith(
expect.objectContaining({
tracePropagationTargets: ['localhost', /^\/(?!\/)/],
}),
);
});

it('client tracePropagationTargets takes priority over integration options', () => {
const instrumentOutgoingRequests = jest.spyOn(SentryBrowser, 'instrumentOutgoingRequests');
const mockedHub = {
getClient: () => ({
getOptions: () => ({
tracePropagationTargets: ['test1', 'test2'],
}),
}),
};

const integration = new ReactNativeTracing({
tracePropagationTargets: ['test3', 'test4'],
tracingOrigins: ['test5', 'test6'],
});
integration.setupOnce(
() => {},
() => mockedHub as unknown as Hub,
);

expect(instrumentOutgoingRequests).toBeCalledWith(
expect.objectContaining({
tracePropagationTargets: ['test1', 'test2'],
}),
);
});

it('integration tracePropagationTargets takes priority over tracingOrigins', () => {
const instrumentOutgoingRequests = jest.spyOn(SentryBrowser, 'instrumentOutgoingRequests');
const mockedHub = {
getClient: () => ({
getOptions: () => ({}),
}),
};

const integration = new ReactNativeTracing({
tracePropagationTargets: ['test3', 'test4'],
tracingOrigins: ['test5', 'test6'],
});
integration.setupOnce(
() => {},
() => mockedHub as unknown as Hub,
);

expect(instrumentOutgoingRequests).toBeCalledWith(
expect.objectContaining({
tracePropagationTargets: ['test3', 'test4'],
}),
);
});
});

describe('App Start', () => {
describe('Without routing instrumentation', () => {
it('Starts route transaction (cold)', async () => {
Expand Down