diff --git a/CHANGELOG.md b/CHANGELOG.md index 6dcaf1307d..45b532758e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - Bump: sentry-android to v4.0.0 #1309 - build(ios): Bump sentry-cocoa to 6.1.4 #1308 - fix: Handle auto session tracking start on iOS #1308 +- feat: Use beforeNavigate in routing instrumentation to match behavior on JS #1313 ## 2.2.0-beta.0 diff --git a/sample/src/App.tsx b/sample/src/App.tsx index 2401604fb5..43ce2d585d 100644 --- a/sample/src/App.tsx +++ b/sample/src/App.tsx @@ -20,17 +20,7 @@ import {store} from './reduxApp'; import {version as packageVersion} from '../../package.json'; import {SENTRY_INTERNAL_DSN} from './dsn'; -const reactNavigationV5Instrumentation = new Sentry.ReactNavigationV5Instrumentation( - { - shouldSendTransaction: (route, previousRoute) => { - if (route.name === 'ManualTracker') { - return false; - } - - return true; - }, - }, -); +const reactNavigationV5Instrumentation = new Sentry.ReactNavigationV5Instrumentation(); Sentry.init({ // Replace the example DSN below with your own DSN: @@ -46,6 +36,14 @@ Sentry.init({ idleTimeout: 5000, routingInstrumentation: reactNavigationV5Instrumentation, tracingOrigins: ['localhost', /^\//, /^https:\/\//], + beforeNavigate: (context: Sentry.ReactNavigationTransactionContext) => { + // Example of not sending a transaction for the screen with the name "Manual Tracker" + if (context.data.route.name === 'ManualTracker') { + context.sampled = false; + } + + return context; + }, }), ], enableAutoSessionTracking: true, diff --git a/src/js/index.ts b/src/js/index.ts index 70cdfdc6f7..eef2f1e8e4 100644 --- a/src/js/index.ts +++ b/src/js/index.ts @@ -62,6 +62,7 @@ export { ReactNavigationV4Instrumentation, ReactNavigationV5Instrumentation, RoutingInstrumentation, + ReactNavigationTransactionContext, } from "./tracing"; /** diff --git a/src/js/tracing/index.ts b/src/js/tracing/index.ts index 9cc1e65b09..09ea506d3d 100644 --- a/src/js/tracing/index.ts +++ b/src/js/tracing/index.ts @@ -1,7 +1,14 @@ export { ReactNativeTracing } from "./reactnativetracing"; -export { ReactNavigationV5Instrumentation } from "./reactnavigationv5"; -export { ReactNavigationV4Instrumentation } from "./reactnavigationv4"; + export { RoutingInstrumentation, RoutingInstrumentationInstance, } from "./routingInstrumentation"; + +export { ReactNavigationV5Instrumentation } from "./reactnavigationv5"; +export { ReactNavigationV4Instrumentation } from "./reactnavigationv4"; +export { + ReactNavigationCurrentRoute, + ReactNavigationRoute, + ReactNavigationTransactionContext, +} from "./types"; diff --git a/src/js/tracing/reactnativetracing.ts b/src/js/tracing/reactnativetracing.ts index 41ab0605e4..eeac7fe490 100644 --- a/src/js/tracing/reactnativetracing.ts +++ b/src/js/tracing/reactnativetracing.ts @@ -16,6 +16,10 @@ import { logger } from "@sentry/utils"; import { RoutingInstrumentationInstance } from "../tracing/routingInstrumentation"; import { adjustTransactionDuration } from "./utils"; +export type BeforeNavigate = ( + context: TransactionContext +) => TransactionContext; + export interface ReactNativeTracingOptions extends RequestInstrumentationOptions { /** @@ -48,7 +52,7 @@ export interface ReactNativeTracingOptions * * Default: true */ - ignoreEmptyBackNavigationTransactions?: boolean; + ignoreEmptyBackNavigationTransactions: boolean; /** * beforeNavigate is called before a navigation transaction is created and allows users to modify transaction @@ -58,7 +62,7 @@ export interface ReactNativeTracingOptions * * @returns A (potentially) modified context object, with `sampled = false` if the transaction should be dropped. */ - beforeNavigate?(context: TransactionContext): TransactionContext; + beforeNavigate: BeforeNavigate; } const defaultReactNativeTracingOptions: ReactNativeTracingOptions = { @@ -66,6 +70,7 @@ const defaultReactNativeTracingOptions: ReactNativeTracingOptions = { idleTimeout: 1000, maxTransactionDuration: 600, ignoreEmptyBackNavigationTransactions: true, + beforeNavigate: (context) => context, }; /** @@ -114,7 +119,8 @@ export class ReactNativeTracing implements Integration { this._getCurrentHub = getCurrentHub; routingInstrumentation?.registerRoutingInstrumentation( - this._onRouteWillChange.bind(this) + this._onRouteWillChange.bind(this), + this.options.beforeNavigate ); if (!routingInstrumentation) { @@ -129,14 +135,6 @@ export class ReactNativeTracing implements Integration { tracingOrigins, shouldCreateSpanForRequest, }); - - addGlobalEventProcessor((event) => { - // eslint-disable-next-line no-empty - if (event.type === "transaction") { - } - - return event; - }); } /** To be called when the route changes, but BEFORE the components of the new route mount. */ @@ -159,37 +157,22 @@ export class ReactNativeTracing implements Integration { } // eslint-disable-next-line @typescript-eslint/unbound-method - const { - beforeNavigate, - idleTimeout, - maxTransactionDuration, - } = this.options; + const { idleTimeout, maxTransactionDuration } = this.options; const expandedContext = { ...context, trimEnd: true, }; - const modifiedContext = - typeof beforeNavigate === "function" - ? beforeNavigate(expandedContext) - : expandedContext; - - if (modifiedContext.sampled === false) { - logger.log( - `[ReactNativeTracing] Will not send ${context.op} transaction.` - ); - } - const hub = this._getCurrentHub(); const idleTransaction = startIdleTransaction( hub as any, - context, + expandedContext, idleTimeout, true ); logger.log( - `[ReactNativeTracing] Starting ${context.op} transaction on scope` + `[ReactNativeTracing] Starting ${context.op} transaction "${context.name}" on scope` ); idleTransaction.registerBeforeFinishCallback( (transaction, endTimestamp) => { @@ -204,12 +187,16 @@ export class ReactNativeTracing implements Integration { if (this.options.ignoreEmptyBackNavigationTransactions) { idleTransaction.registerBeforeFinishCallback((transaction) => { if ( - transaction.data["routing.route.hasBeenSeen"] && + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + transaction.data?.route?.hasBeenSeen && (!transaction.spanRecorder || transaction.spanRecorder.spans.filter( (span) => span.spanId !== transaction.spanId ).length === 0) ) { + logger.log( + `[ReactNativeTracing] Not sampling transaction as route has been seen before. Pass ignoreEmptyBackNavigationTransactions = false to disable this feature.` + ); // Route has been seen before and has no child spans. transaction.sampled = false; } diff --git a/src/js/tracing/reactnavigationv4.ts b/src/js/tracing/reactnavigationv4.ts index 547b6d4d23..44bc802d6c 100644 --- a/src/js/tracing/reactnavigationv4.ts +++ b/src/js/tracing/reactnavigationv4.ts @@ -1,12 +1,13 @@ -import { TransactionContext } from "@sentry/types"; import { logger } from "@sentry/utils"; import { RoutingInstrumentation } from "./routingInstrumentation"; +import { ReactNavigationTransactionContext } from "./types"; export interface NavigationRouteV4 { routeName: string; key: string; - params?: Record; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + params?: Record; } export interface NavigationStateV4 { @@ -22,6 +23,7 @@ export interface AppContainerInstance { state: NavigationStateV4; router: { getStateForAction: ( + // eslint-disable-next-line @typescript-eslint/no-explicit-any action: any, state: NavigationStateV4 ) => NavigationStateV4; @@ -33,22 +35,6 @@ interface AppContainerRef { current?: AppContainerInstance | null; } -type ReactNavigationV4ShouldAttachTransaction = ( - route: NavigationRouteV4, - previousRoute: NavigationRouteV4 | null -) => boolean; - -interface ReactNavigationV4InstrumentationOptions { - shouldSendTransaction: ReactNavigationV4ShouldAttachTransaction; -} - -const defaultShouldAttachTransaction: ReactNavigationV4ShouldAttachTransaction = () => - true; - -const DEFAULT_OPTIONS: ReactNavigationV4InstrumentationOptions = { - shouldSendTransaction: defaultShouldAttachTransaction, -}; - /** * Instrumentation for React-Navigation V4. * Register the app container with `registerAppContainer` to use, or see docs for more details. @@ -58,22 +44,11 @@ class ReactNavigationV4Instrumentation extends RoutingInstrumentation { private _appContainerRef: AppContainerRef = { current: null }; - private _options: ReactNavigationV4InstrumentationOptions; - private readonly _maxRecentRouteLen: number = 200; - private _prevRoute: NavigationRouteV4 | null = null; + private _prevRoute?: NavigationRouteV4; private _recentRouteKeys: string[] = []; - constructor(options: Partial = {}) { - super(); - - this._options = { - ...DEFAULT_OPTIONS, - ...options, - }; - } - /** * Pass the ref to the app container to register it to the instrumentation * @param appContainerRef Ref to an `AppContainer` @@ -142,16 +117,31 @@ class ReactNavigationV4Instrumentation extends RoutingInstrumentation { // If the route is a different key, this is so we ignore actions that pertain to the same screen. if (!this._prevRoute || currentRoute.key !== this._prevRoute.key) { - const context = this._getTransactionContext(currentRoute); + const originalContext = this._getTransactionContext( + currentRoute, + this._prevRoute + ); + let finalContext = this._beforeNavigate?.(originalContext); + + // This block is to catch users not returning a transaction context + if (!finalContext) { + logger.error( + `[ReactNavigationV4Instrumentation] beforeNavigate returned ${finalContext}, return context.sampled = false to not send transaction.` + ); + + finalContext = { + ...originalContext, + sampled: false, + }; + } - if (!this._options.shouldSendTransaction(currentRoute, this._prevRoute)) { - context.sampled = false; + if (finalContext.sampled === false) { logger.log( - `[ReactNavigationV4Instrumentation] Will not send transaction "${context.name}" due to shouldSendTransaction.` + `[ReactNavigationV4Instrumentation] Will not send transaction "${finalContext.name}" due to beforeNavigate.` ); } - this.onRouteWillChange(context); + this.onRouteWillChange(finalContext); this._pushRecentRouteKey(currentRoute.key); this._prevRoute = currentRoute; @@ -161,7 +151,10 @@ class ReactNavigationV4Instrumentation extends RoutingInstrumentation { /** * Gets the transaction context for a `NavigationRouteV4` */ - private _getTransactionContext(route: NavigationRouteV4): TransactionContext { + private _getTransactionContext( + route: NavigationRouteV4, + previousRoute?: NavigationRouteV4 + ): ReactNavigationTransactionContext { return { name: route.routeName, op: "navigation", @@ -171,9 +164,19 @@ class ReactNavigationV4Instrumentation extends RoutingInstrumentation { "routing.route.name": route.routeName, }, data: { - "routing.route.key": route.key, - "routing.route.params": route.params, - "routing.route.hasBeenSeen": this._recentRouteKeys.includes(route.key), + route: { + name: route.routeName, // Include name here too for use in `beforeNavigate` + key: route.key, + params: route.params ?? {}, + hasBeenSeen: this._recentRouteKeys.includes(route.key), + }, + previousRoute: previousRoute + ? { + name: previousRoute.routeName, + key: previousRoute.key, + params: previousRoute.params ?? {}, + } + : null, }, }; } diff --git a/src/js/tracing/reactnavigationv5.ts b/src/js/tracing/reactnavigationv5.ts index 88e31c370f..b76ba038e0 100644 --- a/src/js/tracing/reactnavigationv5.ts +++ b/src/js/tracing/reactnavigationv5.ts @@ -2,11 +2,13 @@ import { Transaction as TransactionType } from "@sentry/types"; import { logger } from "@sentry/utils"; import { RoutingInstrumentation } from "./routingInstrumentation"; +import { ReactNavigationTransactionContext } from "./types"; export interface NavigationRouteV5 { name: string; key: string; - params?: Record; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + params?: Record; } interface NavigationContainerV5 { @@ -14,13 +16,6 @@ interface NavigationContainerV5 { getCurrentRoute: () => NavigationRouteV5; } -interface ReactNavigationV5InstrumentationOptions { - shouldSendTransaction?( - route: NavigationRouteV5, - previousRoute?: NavigationRouteV5 - ): boolean; -} - type NavigationContainerV5Ref = { current: NavigationContainerV5 | null; }; @@ -38,8 +33,6 @@ const STATE_CHANGE_TIMEOUT_DURATION = 200; export class ReactNavigationV5Instrumentation extends RoutingInstrumentation { static instrumentationName: string = "react-navigation-v5"; - private _options: ReactNavigationV5InstrumentationOptions; - private _navigationContainerRef: NavigationContainerV5Ref = { current: null, }; @@ -51,12 +44,6 @@ export class ReactNavigationV5Instrumentation extends RoutingInstrumentation { private _stateChangeTimeout?: number | undefined; private _recentRouteKeys: string[] = []; - constructor(_options: Partial = {}) { - super(); - - this._options = _options; - } - /** * Pass the ref to the navigation container to register it to the instrumentation * @param navigationContainerRef Ref to a `NavigationContainer` @@ -85,14 +72,7 @@ export class ReactNavigationV5Instrumentation extends RoutingInstrumentation { * and gets the route information from there, @see _onStateChange */ private _onDispatch(): void { - this._latestTransaction = this.onRouteWillChange({ - name: "Route Change", - op: "navigation", - tags: { - "routing.instrumentation": - ReactNavigationV5Instrumentation.instrumentationName, - }, - }); + this._latestTransaction = this.onRouteWillChange(BLANK_TRANSACTION_CONTEXT); this._stateChangeTimeout = setTimeout( this._discardLatestTransaction.bind(this), @@ -113,23 +93,49 @@ export class ReactNavigationV5Instrumentation extends RoutingInstrumentation { this._latestTransaction && (!previousRoute || previousRoute.key !== route.key) ) { - this._latestTransaction.setName(route.name); - this._latestTransaction.setTag("routing.route.name", route.name); - this._latestTransaction.setData("routing.route.key", route.key); - this._latestTransaction.setData("routing.route.params", route.params); - + const originalContext = this._latestTransaction.toContext() as typeof BLANK_TRANSACTION_CONTEXT; const routeHasBeenSeen = this._recentRouteKeys.includes(route.key); - this._latestTransaction.setData( - "routing.route.hasBeenSeen", - routeHasBeenSeen - ); - const willSendTransaction = - typeof this._options.shouldSendTransaction === "function" - ? this._options.shouldSendTransaction(route, previousRoute) - : true; + const updatedContext: ReactNavigationTransactionContext = { + ...originalContext, + name: route.name, + tags: { + ...originalContext.tags, + "routing.route.name": route.name, + }, + data: { + ...originalContext.data, + route: { + name: route.name, + key: route.key, + params: route.params ?? {}, + hasBeenSeen: routeHasBeenSeen, + }, + previousRoute: previousRoute + ? { + name: previousRoute.name, + key: previousRoute.key, + params: previousRoute.params ?? {}, + } + : null, + }, + }; + + let finalContext = this._beforeNavigate?.(updatedContext); + + // This block is to catch users not returning a transaction context + if (!finalContext) { + logger.error( + `[ReactNavigationV5Instrumentation] beforeNavigate returned ${finalContext}, return context.sampled = false to not send transaction.` + ); + + finalContext = { + ...updatedContext, + sampled: false, + }; + } - if (willSendTransaction) { + if (finalContext.sampled) { // Clear the timeout so the transaction does not get cancelled. if (typeof this._stateChangeTimeout !== "undefined") { clearTimeout(this._stateChangeTimeout); @@ -137,9 +143,11 @@ export class ReactNavigationV5Instrumentation extends RoutingInstrumentation { } } else { logger.log( - `[ReactNavigationV5Instrumentation] Will not send transaction "${this._latestTransaction.name}" due to shouldSendTransaction.` + `[ReactNavigationV5Instrumentation] Will not send transaction "${finalContext.name}" due to beforeNavigate.` ); } + + this._latestTransaction.updateWithContext(finalContext); } this._pushRecentRouteKey(route.key); @@ -167,3 +175,13 @@ export class ReactNavigationV5Instrumentation extends RoutingInstrumentation { } } } + +export const BLANK_TRANSACTION_CONTEXT = { + name: "Route Change", + op: "navigation", + tags: { + "routing.instrumentation": + ReactNavigationV5Instrumentation.instrumentationName, + }, + data: {}, +}; diff --git a/src/js/tracing/routingInstrumentation.ts b/src/js/tracing/routingInstrumentation.ts index 7ce54a54e8..eb468a7a28 100644 --- a/src/js/tracing/routingInstrumentation.ts +++ b/src/js/tracing/routingInstrumentation.ts @@ -1,6 +1,8 @@ import { Hub } from "@sentry/hub"; import { Transaction, TransactionContext } from "@sentry/types"; +import { BeforeNavigate } from "./reactnativetracing"; + export type TransactionCreator = ( context: TransactionContext ) => Transaction | undefined; @@ -12,8 +14,12 @@ export interface RoutingInstrumentationInstance { * Do not overwrite this unless you know what you are doing. * * @param listener A `RouteListener` + * @param beforeNavigate BeforeNavigate */ - registerRoutingInstrumentation(listener: TransactionCreator): void; + registerRoutingInstrumentation( + listener: TransactionCreator, + beforeNavigate: BeforeNavigate + ): void; /** * To be called when the route changes, BEFORE the new route mounts. * If this is called after a route mounts the child spans will not be correctly attached. @@ -29,12 +35,17 @@ export interface RoutingInstrumentationInstance { */ export class RoutingInstrumentation implements RoutingInstrumentationInstance { protected _getCurrentHub?: () => Hub; + protected _beforeNavigate?: BeforeNavigate; private _tracingListener?: TransactionCreator; /** @inheritdoc */ - registerRoutingInstrumentation(listener: TransactionCreator): void { + registerRoutingInstrumentation( + listener: TransactionCreator, + beforeNavigate: BeforeNavigate + ): void { this._tracingListener = listener; + this._beforeNavigate = beforeNavigate; } /** @inheritdoc */ diff --git a/src/js/tracing/types.ts b/src/js/tracing/types.ts new file mode 100644 index 0000000000..233e18c6ac --- /dev/null +++ b/src/js/tracing/types.ts @@ -0,0 +1,22 @@ +import { TransactionContext } from "@sentry/types"; + +export interface ReactNavigationRoute { + name: string; + key: string; + params: Record; +} + +export interface ReactNavigationCurrentRoute extends ReactNavigationRoute { + hasBeenSeen: boolean; +} + +export interface ReactNavigationTransactionContext extends TransactionContext { + tags: { + "routing.instrumentation": string; + "routing.route.name": string; + }; + data: { + route: ReactNavigationCurrentRoute; + previousRoute: ReactNavigationRoute | null; + }; +} diff --git a/test/tracing/reactnavigationv4.test.ts b/test/tracing/reactnavigationv4.test.ts index 81c3dc79f4..3b67a57768 100644 --- a/test/tracing/reactnavigationv4.test.ts +++ b/test/tracing/reactnavigationv4.test.ts @@ -6,6 +6,14 @@ import { ReactNavigationV4Instrumentation, } from "../../src/js/tracing/reactnavigationv4"; +const initialRoute = { + routeName: "Initial Route", + key: "route0", + params: { + hello: true, + }, +}; + class MockAppContainer implements AppContainerInstance { _navigation: { state: NavigationStateV4; @@ -53,15 +61,7 @@ class MockAppContainer implements AppContainerInstance { index: 0, key: "0", isTransitioning: false, - routes: [ - { - routeName: "Initial Route", - key: "route0", - params: { - hello: true, - }, - }, - ], + routes: [initialRoute], }, router, }; @@ -75,7 +75,10 @@ describe("ReactNavigationV4Instrumentation", () => { instrumentation.onRouteWillChange = jest.fn(); const tracingListener = jest.fn(); - instrumentation.registerRoutingInstrumentation(tracingListener as any); + instrumentation.registerRoutingInstrumentation( + tracingListener as any, + (context) => context + ); const mockAppContainerRef = { current: new MockAppContainer(), @@ -99,9 +102,13 @@ describe("ReactNavigationV4Instrumentation", () => { "routing.route.name": firstRoute.routeName, }, data: { - "routing.route.key": firstRoute.key, - "routing.route.params": firstRoute.params, - "routing.route.hasBeenSeen": false, + route: { + name: firstRoute.routeName, + key: firstRoute.key, + params: firstRoute.params, + hasBeenSeen: false, + }, + previousRoute: null, }, }); }); @@ -112,7 +119,10 @@ describe("ReactNavigationV4Instrumentation", () => { instrumentation.onRouteWillChange = jest.fn(); const tracingListener = jest.fn(); - instrumentation.registerRoutingInstrumentation(tracingListener as any); + instrumentation.registerRoutingInstrumentation( + tracingListener as any, + (context) => context + ); const mockAppContainerRef = { current: new MockAppContainer(), @@ -142,28 +152,38 @@ describe("ReactNavigationV4Instrumentation", () => { "routing.route.name": action.routeName, }, data: { - "routing.route.key": action.key, - "routing.route.params": action.params, - "routing.route.hasBeenSeen": false, + route: { + name: action.routeName, + key: action.key, + params: action.params, + hasBeenSeen: false, + }, + previousRoute: { + name: "Initial Route", + key: "route0", + params: { + hello: true, + }, + }, }, }); }); - test("not sampled with shouldSendTransaction", () => { - const instrumentation = new ReactNavigationV4Instrumentation({ - shouldSendTransaction: (newRoute) => { - if (newRoute.routeName === "DoNotSend") { - return false; - } - - return true; - }, - }); - - instrumentation.onRouteWillChange = jest.fn(); + test("transaction context changed with beforeNavigate", () => { + const instrumentation = new ReactNavigationV4Instrumentation(); const tracingListener = jest.fn(); - instrumentation.registerRoutingInstrumentation(tracingListener as any); + instrumentation.registerRoutingInstrumentation( + tracingListener as any, + (context) => { + context.sampled = false; + context.description = "Description"; + context.name = "New Name"; + context.tags = {}; + + return context; + } + ); const mockAppContainerRef = { current: new MockAppContainer(), @@ -181,21 +201,28 @@ describe("ReactNavigationV4Instrumentation", () => { mockAppContainerRef.current._navigation.router.dispatchAction(action); // eslint-disable-next-line @typescript-eslint/unbound-method - expect(instrumentation.onRouteWillChange).toHaveBeenCalledTimes(2); + expect(tracingListener).toHaveBeenCalledTimes(2); // eslint-disable-next-line @typescript-eslint/unbound-method - expect(instrumentation.onRouteWillChange).toHaveBeenLastCalledWith({ - name: action.routeName, + expect(tracingListener).toHaveBeenLastCalledWith({ + name: "New Name", op: "navigation", - tags: { - "routing.instrumentation": - ReactNavigationV4Instrumentation.instrumentationName, - "routing.route.name": action.routeName, - }, + description: "Description", + tags: {}, data: { - "routing.route.key": action.key, - "routing.route.params": action.params, - "routing.route.hasBeenSeen": false, + route: { + name: action.routeName, + key: action.key, + params: action.params, + hasBeenSeen: false, + }, + previousRoute: { + name: "Initial Route", + key: "route0", + params: { + hello: true, + }, + }, }, sampled: false, }); @@ -207,7 +234,10 @@ describe("ReactNavigationV4Instrumentation", () => { instrumentation.onRouteWillChange = jest.fn(); const tracingListener = jest.fn(); - instrumentation.registerRoutingInstrumentation(tracingListener as any); + instrumentation.registerRoutingInstrumentation( + tracingListener as any, + (context) => context + ); const mockAppContainerRef = { current: new MockAppContainer(), diff --git a/test/tracing/reactnavigationv5.test.ts b/test/tracing/reactnavigationv5.test.ts index d3265d5102..bc7246c5fe 100644 --- a/test/tracing/reactnavigationv5.test.ts +++ b/test/tracing/reactnavigationv5.test.ts @@ -1,5 +1,8 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ +import { Transaction } from "@sentry/tracing"; + import { + BLANK_TRANSACTION_CONTEXT, NavigationRouteV5, ReactNavigationV5Instrumentation, } from "../../src/js/tracing/reactnavigationv5"; @@ -20,13 +23,13 @@ class MockNavigationContainer { } } -const getMockTransaction = () => ({ - sampled: false, - setName: jest.fn(), - setTag: jest.fn(), - setData: jest.fn(), - finish: jest.fn(), -}); +const getMockTransaction = () => { + const transaction = new Transaction(BLANK_TRANSACTION_CONTEXT); + + transaction.sampled = false; + + return transaction; +}; describe("ReactNavigationV5Instrumentation", () => { test("transaction set on initialize", () => { @@ -34,7 +37,10 @@ describe("ReactNavigationV5Instrumentation", () => { const mockTransaction = getMockTransaction(); const tracingListener = jest.fn(() => mockTransaction); - instrumentation.registerRoutingInstrumentation(tracingListener as any); + instrumentation.registerRoutingInstrumentation( + tracingListener as any, + (context) => context + ); const mockNavigationContainerRef = { current: new MockNavigationContainer(), @@ -44,26 +50,20 @@ describe("ReactNavigationV5Instrumentation", () => { mockNavigationContainerRef as any ); - expect(mockTransaction.setName).toBeCalledWith(dummyRoute.name); - expect(mockTransaction.setTag).toBeCalledWith( - "routing.route.name", - dummyRoute.name - ); - expect(mockTransaction.setData).toHaveBeenNthCalledWith( - 1, - "routing.route.key", - dummyRoute.key - ); - expect(mockTransaction.setData).toHaveBeenNthCalledWith( - 2, - "routing.route.params", - undefined - ); - expect(mockTransaction.setData).toHaveBeenNthCalledWith( - 3, - "routing.route.hasBeenSeen", - false - ); + expect(mockTransaction.name).toBe(dummyRoute.name); + expect(mockTransaction.tags).toStrictEqual({ + ...BLANK_TRANSACTION_CONTEXT.tags, + "routing.route.name": dummyRoute.name, + }); + expect(mockTransaction.data).toStrictEqual({ + route: { + name: dummyRoute.name, + key: dummyRoute.key, + params: {}, + hasBeenSeen: false, + }, + previousRoute: null, + }); }); test("transaction sent on navigation", async () => { @@ -75,7 +75,10 @@ describe("ReactNavigationV5Instrumentation", () => { current: mockTransactionDummy, }; const tracingListener = jest.fn(() => transactionRef.current); - instrumentation.registerRoutingInstrumentation(tracingListener as any); + instrumentation.registerRoutingInstrumentation( + tracingListener as any, + (context) => context + ); const mockNavigationContainerRef = { current: new MockNavigationContainer(), @@ -102,41 +105,32 @@ describe("ReactNavigationV5Instrumentation", () => { mockNavigationContainerRef.current.currentRoute = route; mockNavigationContainerRef.current.listeners["state"]({}); - expect(mockTransaction.setName).toBeCalledWith(route.name); - expect(mockTransaction.setTag).toBeCalledWith( - "routing.route.name", - route.name - ); - expect(mockTransaction.setData).toHaveBeenNthCalledWith( - 1, - "routing.route.key", - route.key - ); - expect(mockTransaction.setData).toHaveBeenNthCalledWith( - 2, - "routing.route.params", - route.params - ); - expect(mockTransaction.setData).toHaveBeenNthCalledWith( - 3, - "routing.route.hasBeenSeen", - false - ); + expect(mockTransaction.name).toBe(route.name); + expect(mockTransaction.tags).toStrictEqual({ + ...BLANK_TRANSACTION_CONTEXT.tags, + "routing.route.name": route.name, + }); + expect(mockTransaction.data).toStrictEqual({ + route: { + name: route.name, + key: route.key, + params: route.params, + hasBeenSeen: false, + }, + previousRoute: { + name: dummyRoute.name, + key: dummyRoute.key, + params: {}, + }, + }); + resolve(); }, 50); }); }); - test("transaction not sent on shouldSendTransaction: false", async () => { - const instrumentation = new ReactNavigationV5Instrumentation({ - shouldSendTransaction: (route) => { - if (route.name === "DoNotSend") { - return false; - } - - return true; - }, - }); + test("transaction context changed with beforeNavigate", async () => { + const instrumentation = new ReactNavigationV5Instrumentation(); // Need a dummy transaction as the instrumentation will start a transaction right away when the first navigation container is attached. const mockTransactionDummy = getMockTransaction(); @@ -144,7 +138,16 @@ describe("ReactNavigationV5Instrumentation", () => { current: mockTransactionDummy, }; const tracingListener = jest.fn(() => transactionRef.current); - instrumentation.registerRoutingInstrumentation(tracingListener as any); + instrumentation.registerRoutingInstrumentation( + tracingListener as any, + (context) => { + context.sampled = false; + context.description = "Description"; + context.name = "New Name"; + + return context; + } + ); const mockNavigationContainerRef = { current: new MockNavigationContainer(), @@ -169,6 +172,8 @@ describe("ReactNavigationV5Instrumentation", () => { mockNavigationContainerRef.current.listeners["state"]({}); expect(mockTransaction.sampled).toBe(false); + expect(mockTransaction.name).toBe("New Name"); + expect(mockTransaction.description).toBe("Description"); resolve(); }, 50); }); @@ -183,7 +188,10 @@ describe("ReactNavigationV5Instrumentation", () => { current: mockTransactionDummy, }; const tracingListener = jest.fn(() => transactionRef.current); - instrumentation.registerRoutingInstrumentation(tracingListener as any); + instrumentation.registerRoutingInstrumentation( + tracingListener as any, + (context) => context + ); const mockNavigationContainerRef = { current: new MockNavigationContainer(), @@ -201,9 +209,13 @@ describe("ReactNavigationV5Instrumentation", () => { await new Promise((resolve) => { setTimeout(() => { expect(mockTransaction.sampled).toBe(false); - expect(mockTransaction.setName).not.toHaveBeenCalled(); - expect(mockTransaction.setTag).not.toHaveBeenCalled(); - expect(mockTransaction.setData).not.toHaveBeenCalled(); + expect(mockTransaction.name).toStrictEqual( + BLANK_TRANSACTION_CONTEXT.name + ); + expect(mockTransaction.tags).toStrictEqual( + BLANK_TRANSACTION_CONTEXT.tags + ); + expect(mockTransaction.data).toStrictEqual({}); resolve(); }, 250); });