From 77a228f62596771bde0e7ddc3b01257a863cab7a Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 1 Feb 2023 10:24:48 +0100 Subject: [PATCH 1/6] feat(internal): Add routing instrumentations report in event integrations --- src/js/integrations/factory.ts | 13 +++++++++++++ src/js/tracing/reactnativenavigation.ts | 2 ++ src/js/tracing/reactnativetracing.ts | 6 ++++++ src/js/tracing/reactnavigation.ts | 2 ++ src/js/tracing/reactnavigationv4.ts | 2 ++ src/js/tracing/routingInstrumentation.ts | 6 ++++++ 6 files changed, 31 insertions(+) create mode 100644 src/js/integrations/factory.ts diff --git a/src/js/integrations/factory.ts b/src/js/integrations/factory.ts new file mode 100644 index 0000000000..ed25a8257b --- /dev/null +++ b/src/js/integrations/factory.ts @@ -0,0 +1,13 @@ +import type { + Integration, +} from '@sentry/types'; + +export function createIntegration( + name: Integration['name'], + setupOnce: Integration['setupOnce'] = () => { }, +): Integration { + return { + name: name, + setupOnce, + }; +} diff --git a/src/js/tracing/reactnativenavigation.ts b/src/js/tracing/reactnativenavigation.ts index dfbfe27d87..c8b97412da 100644 --- a/src/js/tracing/reactnativenavigation.ts +++ b/src/js/tracing/reactnativenavigation.ts @@ -70,6 +70,8 @@ export interface NavigationDelegate { export class ReactNativeNavigationInstrumentation extends InternalRoutingInstrumentation { public static instrumentationName: string = 'react-native-navigation'; + public readonly name: string = ReactNativeNavigationInstrumentation.instrumentationName; + private _navigation: NavigationDelegate; private _options: ReactNativeNavigationOptions; diff --git a/src/js/tracing/reactnativetracing.ts b/src/js/tracing/reactnativetracing.ts index ac29c0a762..f6361d6d76 100644 --- a/src/js/tracing/reactnativetracing.ts +++ b/src/js/tracing/reactnativetracing.ts @@ -29,6 +29,9 @@ import { getTimeOriginMilliseconds, isNearToNow, } from './utils'; +import { + createIntegration, +} from '../integrations/factory'; export interface ReactNativeTracingOptions extends RequestInstrumentationOptions { @@ -213,6 +216,9 @@ export class ReactNativeTracing implements Integration { } if (routingInstrumentation) { + getCurrentHub().getClient()?.addIntegration?.(createIntegration(routingInstrumentation.name)); + console.log('routingInstrumentation added to integrations', getCurrentHub().getClient()?.addIntegration); + routingInstrumentation.registerRoutingInstrumentation( this._onRouteWillChange.bind(this), this.options.beforeNavigate, diff --git a/src/js/tracing/reactnavigation.ts b/src/js/tracing/reactnavigation.ts index d1e8afdfeb..89429832e5 100644 --- a/src/js/tracing/reactnavigation.ts +++ b/src/js/tracing/reactnavigation.ts @@ -54,6 +54,8 @@ const defaultOptions: ReactNavigationOptions = { export class ReactNavigationInstrumentation extends InternalRoutingInstrumentation { public static instrumentationName: string = 'react-navigation-v5'; + public readonly name: string = ReactNavigationInstrumentation.instrumentationName; + private _navigationContainer: NavigationContainer | null = null; private readonly _maxRecentRouteLen: number = 200; diff --git a/src/js/tracing/reactnavigationv4.ts b/src/js/tracing/reactnavigationv4.ts index 2040c43240..efd6a411f9 100644 --- a/src/js/tracing/reactnavigationv4.ts +++ b/src/js/tracing/reactnavigationv4.ts @@ -66,6 +66,8 @@ const defaultOptions: ReactNavigationV4Options = { class ReactNavigationV4Instrumentation extends InternalRoutingInstrumentation { public static instrumentationName: string = 'react-navigation-v4'; + public readonly name: string = ReactNavigationV4Instrumentation.instrumentationName; + private _appContainer: AppContainerInstance | null = null; private readonly _maxRecentRouteLen: number = 200; diff --git a/src/js/tracing/routingInstrumentation.ts b/src/js/tracing/routingInstrumentation.ts index 15e3dcf554..303a75e9d6 100644 --- a/src/js/tracing/routingInstrumentation.ts +++ b/src/js/tracing/routingInstrumentation.ts @@ -10,6 +10,10 @@ export type TransactionCreator = ( export type OnConfirmRoute = (context: TransactionContext) => void; export interface RoutingInstrumentationInstance { + /** + * Name of the routing instrumentation + */ + readonly name: string; /** * Registers a listener that's called on every route change with a `TransactionContext`. * @@ -40,6 +44,8 @@ export interface RoutingInstrumentationInstance { export class RoutingInstrumentation implements RoutingInstrumentationInstance { public static instrumentationName: string = 'base-routing-instrumentation'; + public readonly name: string = RoutingInstrumentation.instrumentationName; + protected _getCurrentHub?: () => Hub; protected _beforeNavigate?: BeforeNavigate; protected _onConfirmRoute?: OnConfirmRoute; From 61cf0fe2643a4057ac3cac1453386f5a71075543 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Thu, 2 Feb 2023 17:51:55 +0100 Subject: [PATCH 2/6] Fix lint --- src/js/integrations/factory.ts | 6 +++++- src/js/tracing/reactnativetracing.ts | 7 +++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/js/integrations/factory.ts b/src/js/integrations/factory.ts index ed25a8257b..b5e2116943 100644 --- a/src/js/integrations/factory.ts +++ b/src/js/integrations/factory.ts @@ -2,9 +2,13 @@ import type { Integration, } from '@sentry/types'; +/** + * Creates an integration out of the provided name and setup function. + * @hidden + */ export function createIntegration( name: Integration['name'], - setupOnce: Integration['setupOnce'] = () => { }, + setupOnce: Integration['setupOnce'] = () => { /* noop */ }, ): Integration { return { name: name, diff --git a/src/js/tracing/reactnativetracing.ts b/src/js/tracing/reactnativetracing.ts index f6361d6d76..bbcf712227 100644 --- a/src/js/tracing/reactnativetracing.ts +++ b/src/js/tracing/reactnativetracing.ts @@ -18,6 +18,9 @@ import type { } from '@sentry/types'; import { logger } from '@sentry/utils'; +import { + createIntegration, +} from '../integrations/factory'; import type { NativeAppStartResponse } from '../NativeRNSentry'; import type { RoutingInstrumentationInstance } from '../tracing/routingInstrumentation'; import { NATIVE } from '../wrapper'; @@ -29,9 +32,6 @@ import { getTimeOriginMilliseconds, isNearToNow, } from './utils'; -import { - createIntegration, -} from '../integrations/factory'; export interface ReactNativeTracingOptions extends RequestInstrumentationOptions { @@ -217,7 +217,6 @@ export class ReactNativeTracing implements Integration { if (routingInstrumentation) { getCurrentHub().getClient()?.addIntegration?.(createIntegration(routingInstrumentation.name)); - console.log('routingInstrumentation added to integrations', getCurrentHub().getClient()?.addIntegration); routingInstrumentation.registerRoutingInstrumentation( this._onRouteWillChange.bind(this), From e90f2eea30968c82fdae72d5c4b2390af656b80e Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Thu, 2 Feb 2023 18:18:03 +0100 Subject: [PATCH 3/6] Add profiler and touch events --- src/js/touchevents.tsx | 13 ++++++++++++- src/js/tracing/reactnativeprofiler.tsx | 4 ++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/js/touchevents.tsx b/src/js/touchevents.tsx index 9326eff84a..04cd0d7676 100644 --- a/src/js/touchevents.tsx +++ b/src/js/touchevents.tsx @@ -1,9 +1,11 @@ -import { addBreadcrumb } from '@sentry/core'; +import { addBreadcrumb, getCurrentHub } from '@sentry/core'; import type { SeverityLevel } from '@sentry/types'; import { logger } from '@sentry/utils'; import * as React from 'react'; import { StyleSheet, View } from 'react-native'; +import { createIntegration } from './integrations/factory'; + export type TouchEventBoundaryProps = { /** * The category assigned to the breadcrumb that is logged by the touch event. @@ -70,6 +72,15 @@ class TouchEventBoundary extends React.Component { maxComponentTreeSize: DEFAULT_MAX_COMPONENT_TREE_SIZE, }; + public readonly name: string = 'TouchEventBoundary'; + + /** + * Registers the TouchEventBoundary as a Sentry Integration. + */ + public componentDidMount(): void { + getCurrentHub().getClient()?.addIntegration?.(createIntegration(this.name)); + } + /** * */ diff --git a/src/js/tracing/reactnativeprofiler.tsx b/src/js/tracing/reactnativeprofiler.tsx index 47edf52311..319bbf3228 100644 --- a/src/js/tracing/reactnativeprofiler.tsx +++ b/src/js/tracing/reactnativeprofiler.tsx @@ -1,16 +1,20 @@ import { getCurrentHub, Profiler } from '@sentry/react'; +import { createIntegration } from '../integrations/factory'; import { ReactNativeTracing } from './reactnativetracing'; /** * Custom profiler for the React Native app root. */ export class ReactNativeProfiler extends Profiler { + public readonly name: string = 'ReactNativeProfiler'; + /** * Get the app root mount time. */ public componentDidMount(): void { super.componentDidMount(); + getCurrentHub().getClient()?.addIntegration?.(createIntegration(this.name)); const tracingIntegration = getCurrentHub().getIntegration( ReactNativeTracing From e2cf27f86b7c3b4eea781efa292b699276dbc1e2 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Fri, 3 Feb 2023 11:10:34 +0100 Subject: [PATCH 4/6] Fix integrations can be registered before setup --- src/js/client.ts | 14 ++++++++++++++ src/js/tracing/reactnativetracing.ts | 5 ----- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/js/client.ts b/src/js/client.ts index 7bf220bdac..7d8f7e1623 100644 --- a/src/js/client.ts +++ b/src/js/client.ts @@ -15,9 +15,11 @@ import type { import { dateTimestampInSeconds, logger, SentryError } from '@sentry/utils'; import { Alert } from 'react-native'; +import { createIntegration } from './integrations/factory'; import { Screenshot } from './integrations/screenshot'; import { defaultSdkInfo } from './integrations/sdkinfo'; import type { ReactNativeClientOptions } from './options'; +import { ReactNativeTracing } from './tracing'; import { createUserFeedbackEnvelope, items } from './utils/envelope'; import { ignoreRequireCycleLogs } from './utils/ignorerequirecyclelogs'; import { mergeOutcomes } from './utils/outcome'; @@ -117,6 +119,18 @@ export class ReactNativeClient extends BaseClient { this._sendEnvelope(envelope); } + /** + * Sets up the integrations + */ + public setupIntegrations(): void { + super.setupIntegrations(); + const tracing = this.getIntegration(ReactNativeTracing); + const routingName = tracing?.options.routingInstrumentation?.name + if (routingName) { + this.addIntegration(createIntegration(routingName)); + } + } + /** * @inheritdoc */ diff --git a/src/js/tracing/reactnativetracing.ts b/src/js/tracing/reactnativetracing.ts index bbcf712227..ac29c0a762 100644 --- a/src/js/tracing/reactnativetracing.ts +++ b/src/js/tracing/reactnativetracing.ts @@ -18,9 +18,6 @@ import type { } from '@sentry/types'; import { logger } from '@sentry/utils'; -import { - createIntegration, -} from '../integrations/factory'; import type { NativeAppStartResponse } from '../NativeRNSentry'; import type { RoutingInstrumentationInstance } from '../tracing/routingInstrumentation'; import { NATIVE } from '../wrapper'; @@ -216,8 +213,6 @@ export class ReactNativeTracing implements Integration { } if (routingInstrumentation) { - getCurrentHub().getClient()?.addIntegration?.(createIntegration(routingInstrumentation.name)); - routingInstrumentation.registerRoutingInstrumentation( this._onRouteWillChange.bind(this), this.options.beforeNavigate, From 47a54a8d2bc85c3f6535b55fc015d0366f0bb522 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Fri, 3 Feb 2023 12:03:28 +0100 Subject: [PATCH 5/6] Add tests --- test/client.test.ts | 26 ++++++++++++++++++++++++++ test/touchevents.test.tsx | 17 +++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/test/client.test.ts b/test/client.test.ts index 10eb670a03..f5bef71569 100644 --- a/test/client.test.ts +++ b/test/client.test.ts @@ -5,6 +5,7 @@ import * as RN from 'react-native'; import { ReactNativeClient } from '../src/js/client'; import type { ReactNativeClientOptions } from '../src/js/options'; +import { ReactNativeTracing, RoutingInstrumentationInstance } from '../src/js/tracing'; import { NativeTransport } from '../src/js/transports/native'; import { SDK_NAME, SDK_PACKAGE_NAME, SDK_VERSION } from '../src/js/version'; import { NATIVE } from '../src/js/wrapper'; @@ -29,6 +30,8 @@ interface MockedReactNative { crash: jest.Mock; captureEnvelope: jest.Mock; captureScreenshot: jest.Mock; + fetchNativeAppStart: jest.Mock; + enableNativeFramesTracking: jest.Mock; }; }; Platform: { @@ -54,6 +57,8 @@ jest.mock( crash: jest.fn(), captureEnvelope: jest.fn(), captureScreenshot: jest.fn().mockResolvedValue(null), + fetchNativeAppStart: jest.fn(), + enableNativeFramesTracking: jest.fn(), }, }, Platform: { @@ -530,6 +535,27 @@ describe('Tests ReactNativeClient', () => { client.recordDroppedEvent('before_send', 'error'); } }); + + describe('register enabled instrumentation as integrations', () => { + test('register routing instrumentation', () => { + const mockRoutingInstrumentation: RoutingInstrumentationInstance = { + registerRoutingInstrumentation: jest.fn(), + onRouteWillChange: jest.fn(), + name: 'MockRoutingInstrumentation', + } + const client = new ReactNativeClient(mockedOptions({ + dsn: EXAMPLE_DSN, + integrations: [ + new ReactNativeTracing({ + routingInstrumentation: mockRoutingInstrumentation, + }), + ], + })); + client.setupIntegrations(); + + expect(client.getIntegrationById('MockRoutingInstrumentation')).toBeTruthy(); + }); + }); }); function mockedOptions(options: Partial): ReactNativeClientOptions { diff --git a/test/touchevents.test.tsx b/test/touchevents.test.tsx index ed112bbe27..643205dff0 100644 --- a/test/touchevents.test.tsx +++ b/test/touchevents.test.tsx @@ -6,6 +6,8 @@ import type { SeverityLevel } from '@sentry/types'; import { TouchEventBoundary } from '../src/js/touchevents'; +jest.mock('@sentry/core'); + describe('TouchEventBoundary._onTouchStart', () => { let addBreadcrumb: jest.SpyInstance; @@ -14,6 +16,21 @@ describe('TouchEventBoundary._onTouchStart', () => { addBreadcrumb = jest.spyOn(core, 'addBreadcrumb'); }); + it('register itself as integration', () => { + const mockAddIntegration = jest.fn(); + (core.getCurrentHub as jest.Mock).mockReturnValue({ + getClient: jest.fn().mockReturnValue({ + addIntegration: mockAddIntegration, + }), + }); + const { defaultProps } = TouchEventBoundary; + const boundary = new TouchEventBoundary(defaultProps); + + boundary.componentDidMount(); + + expect(mockAddIntegration).toBeCalledWith(expect.objectContaining({ name: 'TouchEventBoundary' })); + }); + it('tree without displayName or label is not logged', () => { const { defaultProps } = TouchEventBoundary; const boundary = new TouchEventBoundary(defaultProps); From 4a972da55be67706f841fbd406350f62e11842c2 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Fri, 3 Feb 2023 12:26:08 +0100 Subject: [PATCH 6/6] Fix lint --- test/client.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/client.test.ts b/test/client.test.ts index f5bef71569..a823722b4e 100644 --- a/test/client.test.ts +++ b/test/client.test.ts @@ -5,7 +5,8 @@ import * as RN from 'react-native'; import { ReactNativeClient } from '../src/js/client'; import type { ReactNativeClientOptions } from '../src/js/options'; -import { ReactNativeTracing, RoutingInstrumentationInstance } from '../src/js/tracing'; +import type { RoutingInstrumentationInstance } from '../src/js/tracing'; +import { ReactNativeTracing } from '../src/js/tracing'; import { NativeTransport } from '../src/js/transports/native'; import { SDK_NAME, SDK_PACKAGE_NAME, SDK_VERSION } from '../src/js/version'; import { NATIVE } from '../src/js/wrapper';