fix(appStart): Attach App Start spans to the first created not the first processed root span (#4618)

Author: krystofwoldrich

CHANGELOG.md

@@ -15,6 +15,7 @@
 ### Fixes
 
 - Considers the `SENTRY_DISABLE_AUTO_UPLOAD` and `SENTRY_DISABLE_NATIVE_DEBUG_UPLOAD` environment variables in the configuration of the Sentry Android Gradle Plugin for Expo plugin ([#4583](https://github.com/getsentry/sentry-react-native/pull/4583))
+- Attach App Start spans to the first created not the first processed root span ([#4618](https://github.com/getsentry/sentry-react-native/pull/4618))
 
 ### Dependencies
 

packages/core/src/js/tracing/integrations/appStart.ts

@@ -1,5 +1,5 @@
 /* eslint-disable complexity, max-lines */
-import type { Client, Event, Integration, SpanJSON, TransactionEvent } from '@sentry/core';
+import type { Client, Event, Integration, Span, SpanJSON, TransactionEvent } from '@sentry/core';
 import {
   getCapturedScopesOnSpan,
   getClient,
@@ -17,7 +17,7 @@ import {
 } from '../../measurements';
 import type { NativeAppStartResponse } from '../../NativeRNSentry';
 import type { ReactNativeClientOptions } from '../../options';
-import { convertSpanToTransaction, setEndTimeValue } from '../../utils/span';
+import { convertSpanToTransaction, isRootSpan, setEndTimeValue } from '../../utils/span';
 import { NATIVE } from '../../wrapper';
 import {
   APP_START_COLD as APP_START_COLD_OP,
@@ -137,16 +137,18 @@ export const appStartIntegration = ({
   let _client: Client | undefined = undefined;
   let isEnabled = true;
   let appStartDataFlushed = false;
+  let firstStartedActiveRootSpanId: string | undefined = undefined;
 
   const setup = (client: Client): void => {
     _client = client;
-    const clientOptions = client.getOptions() as ReactNativeClientOptions;
+    const { enableAppStartTracking } = client.getOptions() as ReactNativeClientOptions;
 
-    const { enableAppStartTracking } = clientOptions;
     if (!enableAppStartTracking) {
       isEnabled = false;
       logger.warn('[AppStart] App start tracking is disabled.');
     }
+
+    client.on('spanStart', recordFirstStartedActiveRootSpanId);
   };
 
   const afterAllSetup = (_client: Client): void => {
@@ -168,6 +170,27 @@ export const appStartIntegration = ({
     return event;
   };
 
+  const recordFirstStartedActiveRootSpanId = (rootSpan: Span): void => {
+    if (firstStartedActiveRootSpanId) {
+      return;
+    }
+
+    if (!isRootSpan(rootSpan)) {
+      return;
+    }
+
+    setFirstStartedActiveRootSpanId(rootSpan.spanContext().spanId);
+  };
+
+  /**
+   * For testing purposes only.
+   * @private
+   */
+  const setFirstStartedActiveRootSpanId = (spanId: string | undefined): void => {
+    firstStartedActiveRootSpanId = spanId;
+    logger.debug('[AppStart] First started active root span id recorded.', firstStartedActiveRootSpanId);
+  };
+
   async function captureStandaloneAppStart(): Promise<void> {
     if (!standalone) {
       logger.debug(
@@ -213,11 +236,23 @@ export const appStartIntegration = ({
       return;
     }
 
+    if (!firstStartedActiveRootSpanId) {
+      logger.warn('[AppStart] No first started active root span id recorded. Can not attach app start.');
+      return;
+    }
+
     if (!event.contexts || !event.contexts.trace) {
       logger.warn('[AppStart] Transaction event is missing trace context. Can not attach app start.');
       return;
     }
 
+    if (firstStartedActiveRootSpanId !== event.contexts.trace.span_id) {
+      logger.warn(
+        '[AppStart] First started active root span id does not match the transaction event span id. Can not attached app start.',
+      );
+      return;
+    }
+
     const appStart = await NATIVE.fetchNativeAppStart();
     if (!appStart) {
       logger.warn('[AppStart] Failed to retrieve the app start metrics from the native layer.');
@@ -333,7 +368,8 @@ export const appStartIntegration = ({
     afterAllSetup,
     processEvent,
     captureStandaloneAppStart,
-  };
+    setFirstStartedActiveRootSpanId,
+  } as AppStartIntegration;
 };
 
 function setSpanDurationAsMeasurementOnTransactionEvent(event: TransactionEvent, label: string, span: SpanJSON): void {

packages/core/test/profiling/integration.test.ts

@@ -262,8 +262,9 @@ describe('profiling integration', () => {
       const transaction1 = Sentry.startSpanManual({ name: 'test-name-1' }, span => span);
       const transaction2 = Sentry.startSpanManual({ name: 'test-name-2' }, span => span);
       transaction1.end();
-      transaction2.end();
+      jest.runOnlyPendingTimers();
 
+      transaction2.end();
       jest.runAllTimers();
 
       expectEnvelopeToContainProfile(

packages/core/test/tracing/integrations/appStart.test.ts

@@ -34,6 +34,10 @@ import { NATIVE } from '../../../src/js/wrapper';
 import { getDefaultTestClientOptions, TestClient } from '../../mocks/client';
 import { mockFunction } from '../../testutils';
 
+type AppStartIntegrationTest = ReturnType<typeof appStartIntegration> & {
+  setFirstStartedActiveRootSpanId: (spanId: string | undefined) => void;
+};
+
 let dateNowSpy: jest.SpyInstance;
 
 jest.mock('../../../src/js/wrapper', () => {
@@ -692,7 +696,10 @@ describe('App Start Integration', () => {
       const integration = appStartIntegration();
       const client = new TestClient(getDefaultTestClientOptions());
 
-      const actualEvent = await integration.processEvent(getMinimalTransactionEvent(), {}, client);
+      const firstEvent = getMinimalTransactionEvent();
+      (integration as AppStartIntegrationTest).setFirstStartedActiveRootSpanId(firstEvent.contexts?.trace?.span_id);
+
+      const actualEvent = await integration.processEvent(firstEvent, {}, client);
       expect(actualEvent).toEqual(
         expectEventWithAttachedColdAppStart({ timeOriginMilliseconds, appStartTimeMilliseconds }),
       );
@@ -725,6 +732,7 @@ describe('App Start Integration', () => {
 
 function processEvent(event: Event): PromiseLike<Event | null> | Event | null {
   const integration = appStartIntegration();
+  (integration as AppStartIntegrationTest).setFirstStartedActiveRootSpanId(event.contexts?.trace?.span_id);
   return integration.processEvent(event, {}, new TestClient(getDefaultTestClientOptions()));
 }
 

packages/core/test/tracing/reactnavigation.ttid.test.tsx

@@ -367,6 +367,7 @@ describe('React Navigation - TTID', () => {
     });
 
     test('idle transaction should cancel the ttid span if new frame not received', () => {
+      jest.runOnlyPendingTimers(); // Flush app start transaction
       mockedNavigation.navigateToNewScreen();
       jest.runOnlyPendingTimers(); // Flush ttid transaction