Skip to content

Commit

Permalink
feat(core): Drop dev server spans (#4271)
Browse files Browse the repository at this point in the history
* Drop dev server spans

* Adds changelog

* Adds tests

* Revert unneeded change

* Handle undefined dev server urls

Co-authored-by: LucasZF <[email protected]>

* Adds tests for the case when the dev server url is undefined

* Use startsWith to check url matching

* Moves changelog to the unreleased section

---------

Co-authored-by: LucasZF <[email protected]>
  • Loading branch information
antonis and lucas-zimerman authored Nov 18, 2024
1 parent 112c4f8 commit 13f280b
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

- Prevents exception capture context from being overwritten by native scope sync ([#4124](https://github.com/getsentry/sentry-react-native/pull/4124))
- Excludes Dev Server and Sentry Dsn requests from Breadcrumbs ([#4240](https://github.com/getsentry/sentry-react-native/pull/4240))
- Skips development server spans ([#4271](https://github.com/getsentry/sentry-react-native/pull/4271))

### Dependencies

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.startsWith(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(),
}));
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

0 comments on commit 13f280b

Please sign in to comment.