fix(tracing): ReactNativeTracing and initial navigation spans have to…

… be created after integrations setup (#4000)
getsentry · Aug 14, 2024 · eeb83da · eeb83da
1 parent d838577
commit eeb83da
Show file tree

Hide file tree

Showing 4 changed files with 96 additions and 12 deletions.
diff --git a/src/js/tracing/integrations/nativeFrames.ts b/src/js/tracing/integrations/nativeFrames.ts
@@ -73,7 +73,6 @@ export const nativeFramesIntegration = (): Integration => {
 
     NATIVE.enableNativeFramesTracking();
 
-    // TODO: Ensure other integrations like ReactNativeTracing and ReactNavigation create spans after all integration are setup.
     client.on('spanStart', _onSpanStart);
     client.on('spanEnd', _onSpanFinish);
     logger.log('[ReactNativeTracing] Native frames instrumentation initialized.');

diff --git a/src/js/tracing/reactnativetracing.ts b/src/js/tracing/reactnativetracing.ts
@@ -111,6 +111,17 @@ export const reactNativeTracingIntegration = (
   };
 
   const setup = (client: Client): void => {
+    addDefaultOpForSpanFrom(client);
+
+    instrumentOutgoingRequests({
+      traceFetch: finalOptions.traceFetch,
+      traceXHR: finalOptions.traceXHR,
+      shouldCreateSpanForRequest: finalOptions.shouldCreateSpanForRequest,
+      tracePropagationTargets: client.getOptions().tracePropagationTargets || DEFAULT_TRACE_PROPAGATION_TARGETS,
+    });
+  };
+
+  const afterAllSetup = (): void => {
     if (finalOptions.routingInstrumentation) {
       const idleNavigationSpanOptions = {
         finalTimeout: finalOptions.finalTimeoutMs,
@@ -139,15 +150,6 @@ export const reactNativeTracingIntegration = (
     } else {
       logger.log(`[${INTEGRATION_NAME}] Not instrumenting route changes as routingInstrumentation has not been set.`);
     }
-
-    addDefaultOpForSpanFrom(client);
-
-    instrumentOutgoingRequests({
-      traceFetch: finalOptions.traceFetch,
-      traceXHR: finalOptions.traceXHR,
-      shouldCreateSpanForRequest: finalOptions.shouldCreateSpanForRequest,
-      tracePropagationTargets: client.getOptions().tracePropagationTargets || DEFAULT_TRACE_PROPAGATION_TARGETS,
-    });
   };
 
   const processEvent = (event: Event): Event => {
@@ -160,6 +162,7 @@ export const reactNativeTracingIntegration = (
   return {
     name: INTEGRATION_NAME,
     setup,
+    afterAllSetup,
     processEvent,
     options: finalOptions,
     state,

diff --git a/test/tracing/reactnativetracing.test.ts b/test/tracing/reactnativetracing.test.ts
@@ -129,6 +129,7 @@ describe('ReactNativeTracing', () => {
         });
 
         integration.setup(client);
+        integration.afterAllSetup(client);
         // wait for internal promises to resolve, fetch app start data from mocked native
         await Promise.resolve();
 

diff --git a/test/tracing/reactnavigation.test.ts b/test/tracing/reactnavigation.test.ts
@@ -1,9 +1,9 @@
 /* eslint-disable deprecation/deprecation */
 /* eslint-disable @typescript-eslint/no-explicit-any */
 import { getCurrentScope, getGlobalScope, getIsolationScope, SentrySpan, setCurrentClient } from '@sentry/core';
-import type { StartSpanOptions } from '@sentry/types';
+import type { Event, Measurements, StartSpanOptions } from '@sentry/types';
 
-import { reactNativeTracingIntegration } from '../../src/js';
+import { nativeFramesIntegration, reactNativeTracingIntegration } from '../../src/js';
 import { DEFAULT_NAVIGATION_SPAN_NAME } from '../../src/js/tracing/reactnativetracing';
 import type { NavigationRoute } from '../../src/js/tracing/reactnavigation';
 import { ReactNavigationInstrumentation } from '../../src/js/tracing/reactnavigation';
@@ -21,13 +21,15 @@ import {
 } from '../../src/js/tracing/semanticAttributes';
 import { RN_GLOBAL_OBJ } from '../../src/js/utils/worldwide';
 import { getDefaultTestClientOptions, TestClient } from '../mocks/client';
+import { NATIVE } from '../mockWrapper';
 import { createMockNavigationAndAttachTo } from './reactnavigationutils';
 
 const dummyRoute = {
   name: 'Route',
   key: '0',
 };
 
+jest.mock('../../src/js/wrapper.ts', () => jest.requireActual('../mockWrapper.ts'));
 jest.useFakeTimers({ advanceTimers: true });
 
 class MockNavigationContainer {
@@ -82,6 +84,85 @@ describe('ReactNavigationInstrumentation', () => {
     );
   });
 
+  describe('initial navigation span is created after all integrations are setup', () => {
+    let rnTracing: ReturnType<typeof reactNativeTracingIntegration>;
+
+    beforeEach(() => {
+      const startFrames = {
+        totalFrames: 100,
+        slowFrames: 20,
+        frozenFrames: 5,
+      };
+      const finishFrames = {
+        totalFrames: 200,
+        slowFrames: 40,
+        frozenFrames: 10,
+      };
+      NATIVE.fetchNativeFrames.mockResolvedValueOnce(startFrames).mockResolvedValueOnce(finishFrames);
+
+      const rNavigation = new ReactNavigationInstrumentation({
+        routeChangeTimeoutMs: 200,
+      });
+      mockNavigation = createMockNavigationAndAttachTo(rNavigation);
+
+      rnTracing = reactNativeTracingIntegration({
+        routingInstrumentation: rNavigation,
+      });
+    });
+
+    test('initial navigation span contains native frames when nativeFrames integration is after react native tracing', async () => {
+      const options = getDefaultTestClientOptions({
+        enableNativeFramesTracking: true,
+        enableStallTracking: false,
+        tracesSampleRate: 1.0,
+        integrations: [rnTracing, nativeFramesIntegration()],
+        enableAppStartTracking: false,
+      });
+      client = new TestClient(options);
+      setCurrentClient(client);
+      client.init();
+
+      // Flush the init transaction, must be async to allow for the native start frames to be fetched
+      await jest.runOnlyPendingTimersAsync();
+      await client.flush();
+
+      expectInitNavigationSpanWithNativeFrames(client.event);
+    });
+
+    test('initial navigation span contains native frames when nativeFrames integration is before react native tracing', async () => {
+      const options = getDefaultTestClientOptions({
+        enableNativeFramesTracking: true,
+        enableStallTracking: false,
+        tracesSampleRate: 1.0,
+        integrations: [nativeFramesIntegration(), rnTracing],
+        enableAppStartTracking: false,
+      });
+      client = new TestClient(options);
+      setCurrentClient(client);
+      client.init();
+
+      // Flush the init transaction, must be async to allow for the native start frames to be fetched
+      await jest.runOnlyPendingTimersAsync();
+      await client.flush();
+
+      expectInitNavigationSpanWithNativeFrames(client.event);
+    });
+
+    function expectInitNavigationSpanWithNativeFrames(event: Event): void {
+      expect(event).toEqual(
+        expect.objectContaining<Event>({
+          type: 'transaction',
+          transaction: 'Initial Screen',
+          measurements: expect.objectContaining<Measurements>({
+            frames_total: expect.toBeObject(),
+            frames_slow: expect.toBeObject(),
+            frames_frozen: expect.toBeObject(),
+          }),
+        }),
+      );
+    }
+  });
+
   test('transaction sent on navigation', async () => {
     setupTestClient();
     jest.runOnlyPendingTimers(); // Flush the init transaction