Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@
### Fixes

- 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
28 changes: 25 additions & 3 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 @@ -136,8 +138,21 @@ export class ReactNativeTracing implements Integration {
private _awaitingAppStartData?: NativeAppStartResponse;
private _appStartFinishTimestamp?: number;
private _currentRoute?: string;
private _hasSetTracePropagationTargets: boolean;
private _hasSetTracingOrigins: boolean;

public constructor(options: Partial<ReactNativeTracingOptions> = {}) {
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,
...options,
Expand Down Expand Up @@ -192,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__ && (thisOptionsTracePropagationTargets || tracingOrigins) && 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 @@ -235,7 +258,6 @@ export class ReactNativeTracing implements Integration {
instrumentOutgoingRequests({
traceFetch,
traceXHR,
tracingOrigins,
shouldCreateSpanForRequest,
tracePropagationTargets,
});
Expand Down
96 changes: 95 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,98 @@ 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', /^\/(?!\/)/],
}),
);
});
});

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