Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): Drop dev server spans #4271

Merged
merged 11 commits into from
Nov 18, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

### Fixes

- Skips development server spans ([#4271](https://github.com/getsentry/sentry-react-native/pull/4271))
- Ignore JavascriptException to filter out obfuscated duplicate JS Errors on Android ([#4232](https://github.com/getsentry/sentry-react-native/pull/4232))
- Skips ignoring require cycle logs for RN 0.70 or newer ([#4214](https://github.com/getsentry/sentry-react-native/pull/4214))
- Enhanced accuracy of time-to-display spans. ([#4189](https://github.com/getsentry/sentry-react-native/pull/4189))
Expand Down
20 changes: 20 additions & 0 deletions packages/core/src/js/tracing/reactnativetracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { getClient } from '@sentry/core';
import type { Client, Event, Integration, StartSpanOptions } from '@sentry/types';

import { isWeb } from '../utils/environment';
import { getDevServer } from './../integrations/debugsymbolicatorutils';
import { addDefaultOpForSpanFrom, defaultIdleOptions } from './span';

export const INTEGRATION_NAME = 'ReactNativeTracing';
Expand Down Expand Up @@ -97,6 +98,25 @@ export const reactNativeTracingIntegration = (
idleTimeoutMs: options.idleTimeoutMs ?? defaultIdleOptions.idleTimeout,
};

const userShouldCreateSpanForRequest = finalOptions.shouldCreateSpanForRequest;

// Drop Dev Server Spans
const devServerUrl = getDevServer()?.url;
const finalShouldCreateSpanForRequest =
devServerUrl === undefined
? userShouldCreateSpanForRequest
: (url: string): boolean => {
if (url.includes(devServerUrl)) {
return false;
}
if (userShouldCreateSpanForRequest) {
return userShouldCreateSpanForRequest(url);
}
return true;
};

finalOptions.shouldCreateSpanForRequest = finalShouldCreateSpanForRequest;

const setup = (client: Client): void => {
addDefaultOpForSpanFrom(client);

Expand Down
65 changes: 65 additions & 0 deletions packages/core/test/tracing/reactnavigation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { DEFAULT_NAVIGATION_SPAN_NAME } from '../../src/js/tracing/span';
import { RN_GLOBAL_OBJ } from '../../src/js/utils/worldwide';
import { getDefaultTestClientOptions, TestClient } from '../mocks/client';
import { NATIVE } from '../mockWrapper';
import { getDevServer } from './../../src/js/integrations/debugsymbolicatorutils';
import { createMockNavigationAndAttachTo } from './reactnavigationutils';

const dummyRoute = {
Expand All @@ -32,6 +33,9 @@ const dummyRoute = {
};

jest.mock('../../src/js/wrapper.ts', () => jest.requireActual('../mockWrapper.ts'));
jest.mock('./../../src/js/integrations/debugsymbolicatorutils', () => ({
getDevServer: jest.fn(),
antonis marked this conversation as resolved.
Show resolved Hide resolved
}));
jest.useFakeTimers({ advanceTimers: true });

class MockNavigationContainer {
Expand Down Expand Up @@ -392,6 +396,67 @@ describe('ReactNavigationInstrumentation', () => {
});
});

describe('shouldCreateSpanForRequest', () => {
it('should return false for Dev Server URLs', () => {
const devServerUrl = 'http://localhost:8081';
(getDevServer as jest.Mock).mockReturnValue({ url: devServerUrl });

const rnTracing = reactNativeTracingIntegration();

const result = rnTracing.options.shouldCreateSpanForRequest(devServerUrl);

expect(result).toBe(false);
});

it('should return true for non Dev Server URLs', () => {
const devServerUrl = 'http://localhost:8081';
(getDevServer as jest.Mock).mockReturnValue({ url: devServerUrl });

const rnTracing = reactNativeTracingIntegration();

const result = rnTracing.options.shouldCreateSpanForRequest('http://some-other-url.com');

expect(result).toBe(true);
});

it('should chain the user defined shouldCreateSpanForRequest if defined', () => {
const devServerUrl = 'http://localhost:8081';
(getDevServer as jest.Mock).mockReturnValue({ url: devServerUrl });

const userShouldCreateSpanForRequest = (_url: string): boolean => {
return false;
};

const rnTracing = reactNativeTracingIntegration({ shouldCreateSpanForRequest: userShouldCreateSpanForRequest });

const result = rnTracing.options.shouldCreateSpanForRequest('http://some-other-url.com');

expect(result).toBe(false);
});

it('should handle undefined devServerUrls by using only the user defined shouldCreateSpanForRequest', () => {
(getDevServer as jest.Mock).mockReturnValue({ url: undefined });

const userShouldCreateSpanForRequest = (_url: string): boolean => {
return true;
};

const rnTracing = reactNativeTracingIntegration({ shouldCreateSpanForRequest: userShouldCreateSpanForRequest });

const result = rnTracing.options.shouldCreateSpanForRequest('http://any-url.com');

expect(result).toBe(true);
});

it('should not set the shouldCreateSpanForRequest if not user provided and the devServerUrl is undefined', () => {
(getDevServer as jest.Mock).mockReturnValue({ url: undefined });

const rnTracing = reactNativeTracingIntegration();

expect(rnTracing.options.shouldCreateSpanForRequest).toBe(undefined);
});
});

function setupTestClient(
setupOptions: {
beforeSpanStart?: (options: StartSpanOptions) => StartSpanOptions;
Expand Down
Loading