feat(appStart): Trace app start on activity restart

CHANGELOG.md

@@ -12,6 +12,7 @@
 
 - Add thread information to spans ([#4579](https://github.com/getsentry/sentry-react-native/pull/4579))
 - Exposed `getDataFromUri` as a public API to retrieve data from a URI ([#4638](https://github.com/getsentry/sentry-react-native/pull/4638))
+- Improve Warm App Start reporting on Android ([#4641](https://github.com/getsentry/sentry-react-native/pull/4641))
 
 ### Fixes
 

packages/core/RNSentryAndroidTester/app/build.gradle

@@ -46,6 +46,7 @@ dependencies {
     testImplementation 'junit:junit:4.13.2'
     testImplementation 'org.mockito:mockito-core:5.10.0'
     testImplementation 'org.mockito.kotlin:mockito-kotlin:5.2.1'
+    testImplementation 'org.robolectric:robolectric:4.14.1'
     androidTestImplementation 'androidx.test.ext:junit:1.1.3'
     androidTestImplementation 'androidx.test.espresso:espresso-core:3.4.0'
 }

packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryAppStartTest.kt

@@ -0,0 +1,159 @@
+package io.sentry.react
+
+import com.facebook.react.bridge.Arguments
+import com.facebook.react.bridge.JavaOnlyMap
+import com.facebook.react.bridge.Promise
+import com.facebook.react.bridge.WritableMap
+import io.sentry.ILogger
+import io.sentry.SentryLevel
+import io.sentry.android.core.performance.AppStartMetrics
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mockito.ArgumentCaptor
+import org.mockito.Captor
+import org.mockito.MockedStatic
+import org.mockito.Mockito.any
+import org.mockito.Mockito.mock
+import org.mockito.Mockito.mockStatic
+import org.mockito.Mockito.verify
+import org.mockito.MockitoAnnotations
+import org.mockito.kotlin.clearInvocations
+import org.mockito.kotlin.eq
+import org.mockito.kotlin.whenever
+import org.robolectric.RobolectricTestRunner
+
+@RunWith(RobolectricTestRunner::class)
+class RNSentryAppStartTest {
+    private lateinit var module: RNSentryModuleImpl
+    private lateinit var promise: Promise
+    private lateinit var logger: ILogger
+    private lateinit var metrics: AppStartMetrics
+    private lateinit var metricsDataBag: Map<String, Any>
+
+    private var argumentsMock: MockedStatic<Arguments>? = null
+
+    @Captor
+    private lateinit var writableMapCaptor: ArgumentCaptor<WritableMap>
+
+    @Before
+    fun setUp() {
+        MockitoAnnotations.openMocks(this)
+
+        promise = mock(Promise::class.java)
+        logger = mock(ILogger::class.java)
+
+        metrics = AppStartMetrics()
+        metrics.appStartTimeSpan.start()
+        metrics.appStartTimeSpan.stop()
+        metricsDataBag = mapOf()
+
+        RNSentryModuleImpl.lastStartTimestampMs = -1
+
+        module = Utils.createRNSentryModuleWithMockedContext()
+
+        // Mock the Arguments class
+        argumentsMock = mockStatic(Arguments::class.java)
+        whenever(Arguments.createMap()).thenReturn(JavaOnlyMap())
+    }
+
+    @After
+    fun tearDown() {
+        argumentsMock?.close()
+    }
+
+    @Test
+    fun `fetchNativeAppStart resolves promise with null when app is not launched in the foreground`() {
+        val metrics = AppStartMetrics()
+        metrics.isAppLaunchedInForeground = false
+
+        val metricsDataBag = mapOf<String, Any>()
+
+        module.fetchNativeAppStart(promise, metrics, metricsDataBag, logger)
+
+        verifyWarnOnceWith(
+            logger,
+            "Invalid app start data: app not launched in foreground.",
+        )
+
+        verify(promise).resolve(null)
+    }
+
+    @Test
+    fun `fetchNativeAppStart resolves promise with app start data when app is launched in the foreground`() {
+        metrics.isAppLaunchedInForeground = true
+
+        module.fetchNativeAppStart(promise, metrics, metricsDataBag, logger)
+
+        verifyDebugOnceWith(logger, "App Start data reported to the RN layer for the first time.")
+
+        val capturedMap = getWritableMapFromPromiseResolve(promise)
+        assertEquals(false, capturedMap.getBoolean("has_fetched"))
+    }
+
+    @Test
+    fun `fetchNativeAppStart marks data as fetched when retried multiple times`() {
+        metrics.isAppLaunchedInForeground = true
+
+        module.fetchNativeAppStart(promise, metrics, metricsDataBag, logger)
+
+        // Clear invocations from the first call
+        clearInvocations(promise)
+        clearInvocations(logger)
+        module.fetchNativeAppStart(promise, metrics, metricsDataBag, logger)
+
+        verifyDebugOnceWith(logger, "App Start data already fetched from native before.")
+
+        val capturedMap = getWritableMapFromPromiseResolve(promise)
+        assertEquals(true, capturedMap.getBoolean("has_fetched"))
+    }
+
+    @Test
+    fun `fetchNativeAppStart returns updated app start data as not fetched before`() {
+        metrics.isAppLaunchedInForeground = true
+
+        module.fetchNativeAppStart(promise, metrics, metricsDataBag, logger)
+
+        // Clear invocations from the first call
+        clearInvocations(promise)
+        clearInvocations(logger)
+
+        metrics.onAppStartSpansSent()
+        metrics.appStartTimeSpan.setStartUnixTimeMs(1741691014000)
+        metrics.appStartTimeSpan.stop()
+        module.fetchNativeAppStart(promise, metrics, metricsDataBag, logger)
+
+        verifyDebugOnceWith(logger, "App Start data updated, reporting to the RN layer again.")
+
+        val capturedMap = getWritableMapFromPromiseResolve(promise)
+        assertEquals(false, capturedMap.getBoolean("has_fetched"))
+    }
+
+    private fun getWritableMapFromPromiseResolve(promise: Promise): WritableMap {
+        verify(promise).resolve(any(WritableMap::class.java))
+        verify(promise).resolve(writableMapCaptor.capture())
+        return writableMapCaptor.value
+    }
+
+    private fun verifyWarnOnceWith(
+        logger: ILogger,
+        value: String,
+    ) {
+        verify(
+            logger,
+            org.mockito.kotlin.times(1),
+        ).log(eq(SentryLevel.WARNING), eq(value))
+    }
+
+    private fun verifyDebugOnceWith(
+        logger: ILogger,
+        value: String,
+    ) {
+        verify(
+            logger,
+            org.mockito.kotlin.times(1),
+        ).log(eq(SentryLevel.DEBUG), eq(value))
+    }
+}

packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt

@@ -1,18 +1,10 @@
 package io.sentry.react
 
-import android.content.pm.PackageInfo
-import android.content.pm.PackageManager
-import com.facebook.react.bridge.Arguments
 import com.facebook.react.bridge.JavaOnlyMap
-import com.facebook.react.bridge.Promise
-import com.facebook.react.bridge.ReactApplicationContext
-import com.facebook.react.bridge.WritableMap
 import com.facebook.react.common.JavascriptException
 import io.sentry.Breadcrumb
 import io.sentry.ILogger
-import io.sentry.SentryLevel
 import io.sentry.android.core.SentryAndroidOptions
-import org.junit.After
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
 import org.junit.Assert.assertNull
@@ -21,87 +13,18 @@ import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.junit.runners.JUnit4
-import org.mockito.ArgumentCaptor
-import org.mockito.Captor
-import org.mockito.MockedStatic
-import org.mockito.Mockito.any
-import org.mockito.Mockito.anyInt
-import org.mockito.Mockito.anyString
 import org.mockito.Mockito.mock
-import org.mockito.Mockito.mockStatic
-import org.mockito.Mockito.verify
-import org.mockito.MockitoAnnotations
-import org.mockito.kotlin.whenever
 
 @RunWith(JUnit4::class)
 class RNSentryModuleImplTest {
     private lateinit var module: RNSentryModuleImpl
-    private lateinit var promise: Promise
     private lateinit var logger: ILogger
-    private var argumentsMock: MockedStatic<Arguments>? = null
-
-    @Captor
-    private lateinit var writableMapCaptor: ArgumentCaptor<WritableMap>
 
     @Before
     fun setUp() {
-        MockitoAnnotations.openMocks(this)
-        val reactContext = mock(ReactApplicationContext::class.java)
-        promise = mock(Promise::class.java)
         logger = mock(ILogger::class.java)
-        val packageManager = mock(PackageManager::class.java)
-        val packageInfo = mock(PackageInfo::class.java)
-
-        whenever(reactContext.packageManager).thenReturn(packageManager)
-        whenever(packageManager.getPackageInfo(anyString(), anyInt())).thenReturn(packageInfo)
-
-        module = RNSentryModuleImpl(reactContext)
-
-        // Mock the Arguments class
-        argumentsMock = mockStatic(Arguments::class.java)
-        val writableMap = mock(WritableMap::class.java)
-        whenever(Arguments.createMap()).thenReturn(writableMap)
-    }
-
-    @After
-    fun tearDown() {
-        argumentsMock?.close()
-    }
-
-    @Test
-    fun `fetchNativeAppStart resolves promise with null when app is not launched in the foreground`() {
-        // Mock the app start measurement
-        val appStartMeasurement = mapOf<String, Any>()
-
-        // Call the method
-        module.fetchNativeAppStart(promise, appStartMeasurement, logger, false)
-
-        // Verify a warning log is emitted
-        verify(logger, org.mockito.kotlin.times(1)).log(
-            SentryLevel.WARNING,
-            "Invalid app start data: app not launched in foreground.",
-        )
-
-        // Verify the promise is resolved with null
-        verify(promise).resolve(null)
-    }
-
-    @Test
-    fun `fetchNativeAppStart resolves promise with app start data when app is launched in the foreground`() {
-        // Mock the app start measurement
-        val appStartMeasurement = mapOf<String, Any>()
-
-        // Call the method
-        module.fetchNativeAppStart(promise, appStartMeasurement, logger, true)
-
-        // Verify no logs are emitted
-        verify(logger, org.mockito.kotlin.times(0)).log(any(), any())
 
-        // Verify the promise is resolved with the expected data
-        verify(promise).resolve(any(WritableMap::class.java))
-        verify(promise).resolve(writableMapCaptor.capture())
-        val capturedMap = writableMapCaptor.value
-        assertEquals(false, capturedMap.getBoolean("has_fetched"))
+        module = Utils.createRNSentryModuleWithMockedContext()
     }
 
     @Test

packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/Utils.kt

@@ -0,0 +1,26 @@
+package io.sentry.react
+
+import android.content.pm.PackageInfo
+import android.content.pm.PackageManager
+import com.facebook.react.bridge.ReactApplicationContext
+import org.mockito.ArgumentMatchers.anyInt
+import org.mockito.ArgumentMatchers.anyString
+import org.mockito.Mockito.mock
+import org.mockito.kotlin.whenever
+
+class Utils {
+    companion object {
+        fun createRNSentryModuleWithMockedContext(): RNSentryModuleImpl {
+            val packageManager = mock(PackageManager::class.java)
+            val packageInfo = mock(PackageInfo::class.java)
+
+            val reactContext = mock(ReactApplicationContext::class.java)
+            whenever(reactContext.packageManager).thenReturn(packageManager)
+            whenever(packageManager.getPackageInfo(anyString(), anyInt())).thenReturn(packageInfo)
+
+            RNSentryModuleImpl.lastStartTimestampMs = -1
+
+            return RNSentryModuleImpl(reactContext)
+        }
+    }
+}

packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java

@@ -12,6 +12,7 @@
 import android.content.res.AssetManager;
 import android.net.Uri;
 import android.util.SparseIntArray;
+import androidx.annotation.VisibleForTesting;
 import androidx.core.app.FrameMetricsAggregator;
 import androidx.fragment.app.FragmentActivity;
 import androidx.fragment.app.FragmentManager;
@@ -105,7 +106,7 @@ public class RNSentryModuleImpl {
   private FrameMetricsAggregator frameMetricsAggregator = null;
   private boolean androidXAvailable;
 
-  private static boolean hasFetchedAppStart;
+  @VisibleForTesting static long lastStartTimestampMs = -1;
 
   // 700ms to constitute frozen frames.
   private static final int FROZEN_FRAME_THRESHOLD = 700;
@@ -432,31 +433,44 @@ public void fetchNativeRelease(Promise promise) {
 
   public void fetchNativeAppStart(Promise promise) {
     fetchNativeAppStart(
-        promise,
-        InternalSentrySdk.getAppStartMeasurement(),
-        logger,
-        AppStartMetrics.getInstance().isAppLaunchedInForeground());
+        promise, AppStartMetrics.getInstance(), InternalSentrySdk.getAppStartMeasurement(), logger);
   }
 
   protected void fetchNativeAppStart(
       Promise promise,
-      final Map<String, Object> appStartMeasurement,
-      ILogger logger,
-      boolean isAppLaunchedInForeground) {
-    if (!isAppLaunchedInForeground) {
+      final AppStartMetrics metrics,
+      final Map<String, Object> metricsDataBag,
+      ILogger logger) {
+    if (!metrics.isAppLaunchedInForeground()) {
       logger.log(SentryLevel.WARNING, "Invalid app start data: app not launched in foreground.");
       promise.resolve(null);
       return;
     }
 
     WritableMap mutableMeasurement =
-        (WritableMap) RNSentryMapConverter.convertToWritable(appStartMeasurement);
-    mutableMeasurement.putBoolean("has_fetched", hasFetchedAppStart);
+        (WritableMap) RNSentryMapConverter.convertToWritable(metricsDataBag);
 
-    // This is always set to true, as we would only allow an app start fetch to only
-    // happen once in the case of a JS bundle reload, we do not want it to be
-    // instrumented again.
-    hasFetchedAppStart = true;
+    long currentStartTimestampMs = metrics.getAppStartTimeSpan().getStartTimestampMs();
+    boolean hasFetched =
+        lastStartTimestampMs > 0 && lastStartTimestampMs == currentStartTimestampMs;
+    mutableMeasurement.putBoolean("has_fetched", hasFetched);
+
+    if (lastStartTimestampMs < 0) {
+      logger.log(SentryLevel.DEBUG, "App Start data reported to the RN layer for the first time.");
+    } else if (hasFetched) {
+      logger.log(SentryLevel.DEBUG, "App Start data already fetched from native before.");
+    } else {
+      logger.log(SentryLevel.DEBUG, "App Start data updated, reporting to the RN layer again.");
+    }
+
+    // When activity is destroyed but the application process is kept alive
+    // the next activity creation is considered warm start.
+    // The app start metrics will be updated by the the Android SDK.
+    // To let the RN JS layer know these are new start data we compare the start timestamps.
+    lastStartTimestampMs = currentStartTimestampMs;
+
+    // Clears start metrics, making them ready for recording warm app start
+    metrics.onAppStartSpansSent();
 
     promise.resolve(mutableMeasurement);
   }