fix(tracing): Cancel auto tx when app goes to background (#3307)

krystofwoldrich · web-flow · commit 20b0c03f589c · 2023-09-29T07:40:41.000Z
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,10 @@
   - This flag equals `true` when Hermes Bundle contains Debug Info (Hermes Source Map was not emitted)
 - Add `enableNdk` property to ReactNativeOptions for Android. ([#3304](https://github.com/getsentry/sentry-react-native/pull/3304))
 
+### Fixes
+
+- Cancel auto instrumentation transaction when app goes to background ([#3307])(https://github.com/getsentry/sentry-react-native/pull/3307)
+
 ## 5.9.2
 
 ### Fixes
diff --git a/sample-new-architecture/metro.config.js b/sample-new-architecture/metro.config.js
@@ -20,7 +20,7 @@ const config = {
   resolver: {
     blacklistRE: blacklist([
       new RegExp(`${parentDir}/node_modules/react-native/.*`),
-      new RegExp(`.*\\android\\.*`), // Required for Windows in order to run the Sample.
+      new RegExp('.*\\android\\.*'), // Required for Windows in order to run the Sample.
     ]),
     extraNodeModules: new Proxy(
       {
diff --git a/sample-new-architecture/src/App.tsx b/sample-new-architecture/src/App.tsx
@@ -35,6 +35,10 @@ Sentry.init({
     console.log('Event beforeSend:', event.event_id);
     return event;
   },
+  beforeSendTransaction(event) {
+    console.log('Transaction beforeSend:', event.event_id);
+    return event;
+  },
   // This will be called with a boolean `didCallNativeInit` when the native SDK has been contacted.
   onReady: ({ didCallNativeInit }) => {
     console.log('onReady called with didCallNativeInit:', didCallNativeInit);
diff --git a/src/js/tracing/reactnativetracing.ts b/src/js/tracing/reactnativetracing.ts
@@ -13,7 +13,7 @@ import { NATIVE } from '../wrapper';
 import { NativeFramesInstrumentation } from './nativeframes';
 import { APP_START_COLD as APP_START_COLD_OP, APP_START_WARM as APP_START_WARM_OP, UI_LOAD } from './ops';
 import { StallTrackingInstrumentation } from './stalltracking';
-import { onlySampleIfChildSpans } from './transaction';
+import { cancelInBackground, onlySampleIfChildSpans } from './transaction';
 import type { BeforeNavigate, RouteChangeContextData } from './types';
 import { adjustTransactionDuration, getTimeOriginMilliseconds, isNearToNow } from './utils';
 
@@ -306,8 +306,6 @@ export class ReactNativeTracing implements Integration {
       return;
     }
 
-    const { idleTimeoutMs, finalTimeoutMs } = this.options;
-
     if (this._inflightInteractionTransaction) {
       this._inflightInteractionTransaction.cancelIdleTimeout(undefined, { restartOnChildSpanChange: false });
       this._inflightInteractionTransaction = undefined;
@@ -319,7 +317,7 @@ export class ReactNativeTracing implements Integration {
       op,
       trimEnd: true,
     };
-    this._inflightInteractionTransaction = startIdleTransaction(hub, context, idleTimeoutMs, finalTimeoutMs, true);
+    this._inflightInteractionTransaction = this._startIdleTransaction(context);
     this._inflightInteractionTransaction.registerBeforeFinishCallback((transaction: IdleTransaction) => {
       this._inflightInteractionTransaction = undefined;
       this.onTransactionFinish(transaction);
@@ -445,16 +443,14 @@ export class ReactNativeTracing implements Integration {
       this._inflightInteractionTransaction.finish();
     }
 
-    // eslint-disable-next-line @typescript-eslint/unbound-method
-    const { idleTimeoutMs, finalTimeoutMs } = this.options;
+    const { finalTimeoutMs } = this.options;
 
     const expandedContext = {
       ...context,
       trimEnd: true,
     };
 
-    const hub = this._getCurrentHub();
-    const idleTransaction = startIdleTransaction(hub as Hub, expandedContext, idleTimeoutMs, finalTimeoutMs, true);
+    const idleTransaction = this._startIdleTransaction(expandedContext);
 
     this.onTransactionStart(idleTransaction);
 
@@ -498,4 +494,15 @@ export class ReactNativeTracing implements Integration {
 
     return idleTransaction;
   }
+
+  /**
+   * Start app state aware idle transaction on the scope.
+   */
+  private _startIdleTransaction(context: TransactionContext): IdleTransaction {
+    const { idleTimeoutMs, finalTimeoutMs } = this.options;
+    const hub = this._getCurrentHub?.() || getCurrentHub();
+    const tx = startIdleTransaction(hub, context, idleTimeoutMs, finalTimeoutMs, true);
+    cancelInBackground(tx);
+    return tx;
+  }
 }
diff --git a/src/js/tracing/transaction.ts b/src/js/tracing/transaction.ts
@@ -1,5 +1,7 @@
-import type { BeforeFinishCallback, IdleTransaction } from '@sentry/core';
+import { type BeforeFinishCallback, type IdleTransaction } from '@sentry/core';
 import { logger } from '@sentry/utils';
+import type { AppStateStatus } from 'react-native';
+import { AppState } from 'react-native';
 
 /**
  * Idle Transaction callback to only sample transactions with child spans.
@@ -15,3 +17,20 @@ export const onlySampleIfChildSpans: BeforeFinishCallback = (transaction: IdleTr
     transaction.sampled = false;
   }
 };
+
+/**
+ * Hooks on AppState change to cancel the transaction if the app goes background.
+ */
+export const cancelInBackground = (transaction: IdleTransaction): void => {
+  const subscription = AppState.addEventListener('change', (newState: AppStateStatus) => {
+    if (newState === 'background') {
+      logger.debug(`Setting ${transaction.op} transaction to cancelled because the app is in the background.`);
+      transaction.setStatus('cancelled');
+      transaction.finish();
+    }
+  });
+  transaction.registerBeforeFinishCallback(() => {
+    logger.debug(`Removing AppState listener for ${transaction.op} transaction.`);
+    subscription.remove();
+  });
+};
diff --git a/test/tracing/reactnativetracing.test.ts b/test/tracing/reactnativetracing.test.ts
@@ -28,6 +28,29 @@ jest.mock('../../src/js/tracing/utils', () => {
   };
 });
 
+type MockAppState = {
+  setState: (state: AppStateStatus) => void;
+  listener: (newState: AppStateStatus) => void;
+  removeSubscription: jest.Func;
+};
+const mockedAppState: AppState & MockAppState = {
+  removeSubscription: jest.fn(),
+  listener: jest.fn(),
+  isAvailable: true,
+  currentState: 'active',
+  addEventListener: (_, listener) => {
+    mockedAppState.listener = listener;
+    return {
+      remove: mockedAppState.removeSubscription,
+    };
+  },
+  setState: (state: AppStateStatus) => {
+    mockedAppState.currentState = state;
+    mockedAppState.listener(state);
+  },
+};
+jest.mock('react-native/Libraries/AppState/AppState', () => mockedAppState);
+
 const getMockScope = () => {
   let scopeTransaction: Transaction | undefined;
   let scopeUser: User | undefined;
@@ -62,6 +85,7 @@ const getMockHub = () => {
 
 import type { BrowserClientOptions } from '@sentry/browser/types/client';
 import type { Scope } from '@sentry/types';
+import type { AppState, AppStateStatus } from 'react-native';
 
 import { APP_START_COLD, APP_START_WARM } from '../../src/js/measurements';
 import {
@@ -100,16 +124,7 @@ describe('ReactNativeTracing', () => {
           enableNativeFramesTracking: false,
         });
 
-        const timeOriginMilliseconds = Date.now();
-        const appStartTimeMilliseconds = timeOriginMilliseconds - 100;
-        const mockAppStartResponse: NativeAppStartResponse = {
-          isColdStart: true,
-          appStartTime: appStartTimeMilliseconds,
-          didFetchAppStart: false,
-        };
-
-        mockFunction(getTimeOriginMilliseconds).mockReturnValue(timeOriginMilliseconds);
-        mockFunction(NATIVE.fetchNativeAppStart).mockResolvedValue(mockAppStartResponse);
+        const [timeOriginMilliseconds, appStartTimeMilliseconds] = mockAppStartResponse({ cold: true });
 
         const mockHub = getMockHub();
         integration.setupOnce(addGlobalEventProcessor, () => mockHub);
@@ -139,16 +154,7 @@ describe('ReactNativeTracing', () => {
       it('Starts route transaction (warm)', async () => {
         const integration = new ReactNativeTracing();
 
-        const timeOriginMilliseconds = Date.now();
-        const appStartTimeMilliseconds = timeOriginMilliseconds - 100;
-        const mockAppStartResponse: NativeAppStartResponse = {
-          isColdStart: false,
-          appStartTime: appStartTimeMilliseconds,
-          didFetchAppStart: false,
-        };
-
-        mockFunction(getTimeOriginMilliseconds).mockReturnValue(timeOriginMilliseconds);
-        mockFunction(NATIVE.fetchNativeAppStart).mockResolvedValue(mockAppStartResponse);
+        const [timeOriginMilliseconds, appStartTimeMilliseconds] = mockAppStartResponse({ cold: false });
 
         const mockHub = getMockHub();
         integration.setupOnce(addGlobalEventProcessor, () => mockHub);
@@ -173,6 +179,24 @@ describe('ReactNativeTracing', () => {
         }
       });
 
+      it('Cancels route transaction when app goes to background', async () => {
+        const integration = new ReactNativeTracing();
+
+        mockAppStartResponse({ cold: false });
+
+        const mockHub = getMockHub();
+        integration.setupOnce(addGlobalEventProcessor, () => mockHub);
+
+        await jest.advanceTimersByTimeAsync(500);
+        const transaction = mockHub.getScope()?.getTransaction();
+
+        mockedAppState.setState('background');
+        jest.runAllTimers();
+
+        expect(transaction?.status).toBe('cancelled');
+        expect(mockedAppState.removeSubscription).toBeCalledTimes(1);
+      });
+
       it('Does not add app start measurement if more than 60s', async () => {
         const integration = new ReactNativeTracing();
 
@@ -212,16 +236,7 @@ describe('ReactNativeTracing', () => {
       it('Does not create app start transaction if didFetchAppStart == true', async () => {
         const integration = new ReactNativeTracing();
 
-        const timeOriginMilliseconds = Date.now();
-        const appStartTimeMilliseconds = timeOriginMilliseconds - 100;
-        const mockAppStartResponse: NativeAppStartResponse = {
-          isColdStart: true,
-          appStartTime: appStartTimeMilliseconds,
-          didFetchAppStart: true,
-        };
-
-        mockFunction(getTimeOriginMilliseconds).mockReturnValue(timeOriginMilliseconds);
-        mockFunction(NATIVE.fetchNativeAppStart).mockResolvedValue(mockAppStartResponse);
+        mockAppStartResponse({ cold: false, didFetchAppStart: true });
 
         const mockHub = getMockHub();
         integration.setupOnce(addGlobalEventProcessor, () => mockHub);
@@ -235,22 +250,38 @@ describe('ReactNativeTracing', () => {
     });
 
     describe('With routing instrumentation', () => {
-      it('Adds measurements and child span onto existing routing transaction and sets the op (cold)', async () => {
+      it('Cancels route transaction when app goes to background', async () => {
         const routingInstrumentation = new RoutingInstrumentation();
         const integration = new ReactNativeTracing({
           routingInstrumentation,
         });
 
-        const timeOriginMilliseconds = Date.now();
-        const appStartTimeMilliseconds = timeOriginMilliseconds - 100;
-        const mockAppStartResponse: NativeAppStartResponse = {
-          isColdStart: true,
-          appStartTime: appStartTimeMilliseconds,
-          didFetchAppStart: false,
-        };
+        mockAppStartResponse({ cold: true });
 
-        mockFunction(getTimeOriginMilliseconds).mockReturnValue(timeOriginMilliseconds);
-        mockFunction(NATIVE.fetchNativeAppStart).mockResolvedValue(mockAppStartResponse);
+        const mockHub = getMockHub();
+        integration.setupOnce(addGlobalEventProcessor, () => mockHub);
+        // wait for internal promises to resolve, fetch app start data from mocked native
+        await Promise.resolve();
+
+        const routeTransaction = routingInstrumentation.onRouteWillChange({
+          name: 'test',
+        }) as IdleTransaction;
+
+        mockedAppState.setState('background');
+
+        jest.runAllTimers();
+
+        expect(routeTransaction.status).toBe('cancelled');
+        expect(mockedAppState.removeSubscription).toBeCalledTimes(1);
+      });
+
+      it('Adds measurements and child span onto existing routing transaction and sets the op (cold)', async () => {
+        const routingInstrumentation = new RoutingInstrumentation();
+        const integration = new ReactNativeTracing({
+          routingInstrumentation,
+        });
+
+        const [timeOriginMilliseconds, appStartTimeMilliseconds] = mockAppStartResponse({ cold: true });
 
         const mockHub = getMockHub();
         integration.setupOnce(addGlobalEventProcessor, () => mockHub);
@@ -297,16 +328,7 @@ describe('ReactNativeTracing', () => {
           routingInstrumentation,
         });
 
-        const timeOriginMilliseconds = Date.now();
-        const appStartTimeMilliseconds = timeOriginMilliseconds - 100;
-        const mockAppStartResponse: NativeAppStartResponse = {
-          isColdStart: false,
-          appStartTime: appStartTimeMilliseconds,
-          didFetchAppStart: false,
-        };
-
-        mockFunction(getTimeOriginMilliseconds).mockReturnValue(timeOriginMilliseconds);
-        mockFunction(NATIVE.fetchNativeAppStart).mockResolvedValue(mockAppStartResponse);
+        const [timeOriginMilliseconds, appStartTimeMilliseconds] = mockAppStartResponse({ cold: false });
 
         const mockHub = getMockHub();
         integration.setupOnce(addGlobalEventProcessor, () => mockHub);
@@ -353,16 +375,7 @@ describe('ReactNativeTracing', () => {
           routingInstrumentation,
         });
 
-        const timeOriginMilliseconds = Date.now();
-        const appStartTimeMilliseconds = timeOriginMilliseconds - 100;
-        const mockAppStartResponse: NativeAppStartResponse = {
-          isColdStart: false,
-          appStartTime: appStartTimeMilliseconds,
-          didFetchAppStart: true,
-        };
-
-        mockFunction(getTimeOriginMilliseconds).mockReturnValue(timeOriginMilliseconds);
-        mockFunction(NATIVE.fetchNativeAppStart).mockResolvedValue(mockAppStartResponse);
+        const [, appStartTimeMilliseconds] = mockAppStartResponse({ cold: false, didFetchAppStart: true });
 
         const mockHub = getMockHub();
         integration.setupOnce(addGlobalEventProcessor, () => mockHub);
@@ -641,6 +654,24 @@ describe('ReactNativeTracing', () => {
         expect(actualTransactionContext?.sampled).toEqual(false);
       });
 
+      test('does cancel UI event transaction when app goes to background', () => {
+        tracing.startUserInteractionTransaction(mockedUserInteractionId);
+
+        const actualTransaction = mockedScope.getTransaction() as Transaction | undefined;
+
+        mockedAppState.setState('background');
+        jest.runAllTimers();
+
+        const actualTransactionContext = actualTransaction?.toContext();
+        expect(actualTransactionContext).toEqual(
+          expect.objectContaining({
+            endTimestamp: expect.any(Number),
+            status: 'cancelled',
+          }),
+        );
+        expect(mockedAppState.removeSubscription).toBeCalledTimes(1);
+      });
+
       test('do not overwrite existing status of UI event transactions', () => {
         tracing.startUserInteractionTransaction(mockedUserInteractionId);
 
@@ -792,3 +823,18 @@ describe('ReactNativeTracing', () => {
     });
   });
 });
+
+function mockAppStartResponse({ cold, didFetchAppStart }: { cold: boolean; didFetchAppStart?: boolean }) {
+  const timeOriginMilliseconds = Date.now();
+  const appStartTimeMilliseconds = timeOriginMilliseconds - 100;
+  const mockAppStartResponse: NativeAppStartResponse = {
+    isColdStart: cold,
+    appStartTime: appStartTimeMilliseconds,
+    didFetchAppStart: didFetchAppStart ?? false,
+  };
+
+  mockFunction(getTimeOriginMilliseconds).mockReturnValue(timeOriginMilliseconds);
+  mockFunction(NATIVE.fetchNativeAppStart).mockResolvedValue(mockAppStartResponse);
+
+  return [timeOriginMilliseconds, appStartTimeMilliseconds];
+}

Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@ const config = {`
`20`	`20`	`resolver: {`
`21`	`21`	`blacklistRE: blacklist([`
`22`	`22`	new RegExp(`${parentDir}/node_modules/react-native/.*`),
`23`		- new RegExp(`.\\android\\.`), // Required for Windows in order to run the Sample.
	`23`	`+ new RegExp('.\\android\\.'), // Required for Windows in order to run the Sample.`
`24`	`24`	`]),`
`25`	`25`	`extraNodeModules: new Proxy(`
`26`	`26`	`{`