From 16420880937bf38f6d96cf6ffa3dbaf72f7adc6c Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Fri, 28 Feb 2025 13:25:46 +0100 Subject: [PATCH 01/15] Ensure app start type is set, even when ActivityLifecycleIntegration is not activated --- .../core/SentryPerformanceProvider.java | 29 +++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java index 0aa946c255..8b24e406fe 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java @@ -9,11 +9,11 @@ import android.content.pm.ProviderInfo; import android.net.Uri; import android.os.Build; +import android.os.Bundle; import android.os.Handler; import android.os.Looper; import android.os.Process; import android.os.SystemClock; -import androidx.annotation.NonNull; import io.sentry.ILogger; import io.sentry.ITransactionProfiler; import io.sentry.JsonSerializer; @@ -33,6 +33,7 @@ import java.io.FileNotFoundException; import java.io.InputStreamReader; import java.io.Reader; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -204,8 +205,32 @@ private void onAppLaunched( activityCallback = new ActivityLifecycleCallbacksAdapter() { + + @Override + public void onActivityCreated( + @NotNull Activity activity, @Nullable Bundle savedInstanceState) { + if (appStartMetrics.getAppStartType() == AppStartMetrics.AppStartType.UNKNOWN) { + // We consider pre-loaded application loads as warm starts + // This usually happens e.g. due to BroadcastReceivers triggering + // Application.onCreate only, but no Activity.onCreate + final long now = SystemClock.uptimeMillis(); + final long durationMs = + now - appStartMetrics.getAppStartTimeSpan().getStartUptimeMs(); + if (durationMs > TimeUnit.SECONDS.toMillis(1)) { + appStartMetrics.restartAppStart(now); + appStartMetrics.setAppStartType(AppStartMetrics.AppStartType.WARM); + } else { + // Otherwise a non-null bundle determines the behavior + appStartMetrics.setAppStartType( + savedInstanceState == null + ? AppStartMetrics.AppStartType.COLD + : AppStartMetrics.AppStartType.WARM); + } + } + } + @Override - public void onActivityStarted(@NonNull Activity activity) { + public void onActivityStarted(@NotNull Activity activity) { if (firstDrawDone.get()) { return; } From 98fc65d730f5f97eeaf45df0622767d5ea0bdf1b Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Fri, 28 Feb 2025 13:31:04 +0100 Subject: [PATCH 02/15] Update Changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9bfc5eeeb7..983bbc2968 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Fixes + +- Fix Ensure app start type is set, even when ActivityLifecycleIntegration is not running ([#4216](https://github.com/getsentry/sentry-java/pull/4216)) + ## 7.22.0 ### Fixes From c4adbdab1b10e76288cbf6e41d141e0fa1172cab Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Fri, 28 Feb 2025 13:43:41 +0100 Subject: [PATCH 03/15] Add proper tests --- .../core/SentryPerformanceProviderTest.kt | 77 ++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt index 9f868d701b..b56254260f 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt @@ -1,8 +1,11 @@ package io.sentry.android.core +import android.app.Activity import android.app.Application +import android.app.Application.ActivityLifecycleCallbacks import android.content.pm.ProviderInfo import android.os.Build +import android.os.Bundle import androidx.test.ext.junit.runners.AndroidJUnit4 import io.sentry.ILogger import io.sentry.JsonSerializer @@ -26,6 +29,7 @@ import java.nio.file.Files import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test +import kotlin.test.assertEquals import kotlin.test.assertFailsWith import kotlin.test.assertFalse import kotlin.test.assertNotNull @@ -48,6 +52,7 @@ class SentryPerformanceProviderTest { val providerInfo = ProviderInfo() val logger = mock() lateinit var configFile: File + var activityLifecycleCallback: ActivityLifecycleCallbacks? = null fun getSut(sdkVersion: Int = Build.VERSION_CODES.S, authority: String = AUTHORITY, handleFile: ((config: File) -> Unit)? = null): SentryPerformanceProvider { val buildInfoProvider: BuildInfoProvider = mock() @@ -56,7 +61,10 @@ class SentryPerformanceProviderTest { whenever(mockContext.applicationContext).thenReturn(mockContext) configFile = File(sentryCache, Sentry.APP_START_PROFILING_CONFIG_FILE_NAME) handleFile?.invoke(configFile) - + whenever(mockContext.registerActivityLifecycleCallbacks(any())).then { + activityLifecycleCallback = it.arguments[0] as ActivityLifecycleCallbacks + return@then Unit + } providerInfo.authority = authority return SentryPerformanceProvider(logger, buildInfoProvider).apply { attachInfo(mockContext, providerInfo) @@ -232,6 +240,73 @@ class SentryPerformanceProviderTest { assertFalse(AppStartMetrics.getInstance().appStartProfiler!!.isRunning) } + @Test + fun `Sets app launch type to cold`() { + val provider = fixture.getSut() + val activity = mock() + provider.onCreate() + + assertEquals(AppStartMetrics.AppStartType.UNKNOWN, AppStartMetrics.getInstance().appStartType) + + // when the first activity has no bundle + fixture.activityLifecycleCallback!!.onActivityCreated(activity, null) + + // then the app start is considered cold + assertEquals(AppStartMetrics.AppStartType.COLD, AppStartMetrics.getInstance().appStartType) + + // when any subsequent activity launches + fixture.activityLifecycleCallback!!.onActivityCreated(activity, mock()) + + // then the app start is still considered cold + assertEquals(AppStartMetrics.AppStartType.COLD, AppStartMetrics.getInstance().appStartType) + } + + @Test + fun `Sets app launch type to warm if process init is too old`() { + val provider = fixture.getSut() + val activity = mock() + provider.onCreate() + + assertEquals(AppStartMetrics.AppStartType.UNKNOWN, AppStartMetrics.getInstance().appStartType) + + AppStartMetrics.getInstance().appStartTimeSpan.setStartedAt( + AppStartMetrics.getInstance().appStartTimeSpan.startUptimeMs - 20000 + ) + + // when the first activity has no bundle + fixture.activityLifecycleCallback!!.onActivityCreated(activity, null) + + // then the app start is considered warm + assertEquals(AppStartMetrics.AppStartType.WARM, AppStartMetrics.getInstance().appStartType) + + // when any subsequent activity launches + fixture.activityLifecycleCallback!!.onActivityCreated(activity, mock()) + + // then the app start is still considered warm + assertEquals(AppStartMetrics.AppStartType.WARM, AppStartMetrics.getInstance().appStartType) + } + + @Test + fun `Sets app launch type to warm`() { + val provider = fixture.getSut() + val activity = mock() + provider.onCreate() + + assertEquals(AppStartMetrics.AppStartType.UNKNOWN, AppStartMetrics.getInstance().appStartType) + + // when the first activity has a bundle + fixture.activityLifecycleCallback!!.onActivityCreated(activity, mock()) + + // then the app start is considered WARM + assertEquals(AppStartMetrics.AppStartType.WARM, AppStartMetrics.getInstance().appStartType) + + // when any subsequent activity launches + fixture.activityLifecycleCallback!!.onActivityCreated(activity, null) + + // then the app start is still considered warm + assertEquals(AppStartMetrics.AppStartType.WARM, AppStartMetrics.getInstance().appStartType) + } + private fun writeConfig( configFile: File, profilingEnabled: Boolean = true, From c8abed55801b302e99d4625920a5011dd1a34970 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Mon, 3 Mar 2025 09:51:49 +0100 Subject: [PATCH 04/15] Add code comments --- .../io/sentry/android/core/SentryPerformanceProvider.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java index 8b24e406fe..638baf7bf8 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java @@ -209,6 +209,10 @@ private void onAppLaunched( @Override public void onActivityCreated( @NotNull Activity activity, @Nullable Bundle savedInstanceState) { + // In case the SDK gets initialized async or the + // ActivityLifecycleIntegration is not enabled (e.g on RN due to Context not being + // instanceof Application) + // the app start type never gets set if (appStartMetrics.getAppStartType() == AppStartMetrics.AppStartType.UNKNOWN) { // We consider pre-loaded application loads as warm starts // This usually happens e.g. due to BroadcastReceivers triggering From 8823143b81093d9f81877aa3cb66b428522bfa4c Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Mon, 3 Mar 2025 15:59:44 +0100 Subject: [PATCH 05/15] Unify handling --- .../api/sentry-android-core.api | 1 + .../core/ActivityLifecycleIntegration.java | 21 ++------ .../core/SentryPerformanceProvider.java | 51 +++++++++---------- .../core/performance/AppStartMetrics.java | 22 ++++++++ 4 files changed, 50 insertions(+), 45 deletions(-) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index ca1f067552..e1d8ed09b2 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -462,6 +462,7 @@ public class io/sentry/android/core/performance/AppStartMetrics : io/sentry/andr public fun setAppStartType (Lio/sentry/android/core/performance/AppStartMetrics$AppStartType;)V public fun setClassLoadedUptimeMs (J)V public fun shouldSendStartMeasurements ()Z + public fun updateAppStartType (ZJ)V } public final class io/sentry/android/core/performance/AppStartMetrics$AppStartType : java/lang/Enum { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java index 0912051dd7..cc44f24c2f 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java @@ -397,7 +397,7 @@ public synchronized void onActivityCreated( if (!isAllActivityCallbacksAvailable) { onActivityPreCreated(activity, savedInstanceState); } - setColdStart(savedInstanceState); + setColdStart(savedInstanceState != null); if (hub != null && options != null && options.isEnableScreenTracking()) { final @Nullable String activityClassName = ClassUtil.getClassName(activity); hub.configureScope(scope -> scope.setScreen(activityClassName)); @@ -705,24 +705,9 @@ WeakHashMap getTtfdSpanMap() { return ttfdSpanMap; } - private void setColdStart(final @Nullable Bundle savedInstanceState) { + private void setColdStart(final boolean hasBundle) { if (!firstActivityCreated) { - final @NotNull TimeSpan appStartSpan = AppStartMetrics.getInstance().getAppStartTimeSpan(); - // If the app start span already started and stopped, it means the app restarted without - // killing the process, so we are in a warm start - // If the app has an invalid cold start, it means it was started in the background, like - // via BroadcastReceiver, so we consider it a warm start - if ((appStartSpan.hasStarted() && appStartSpan.hasStopped()) - || (!AppStartMetrics.getInstance().isColdStartValid())) { - AppStartMetrics.getInstance().restartAppStart(lastPausedUptimeMillis); - AppStartMetrics.getInstance().setAppStartType(AppStartMetrics.AppStartType.WARM); - } else { - AppStartMetrics.getInstance() - .setAppStartType( - savedInstanceState == null - ? AppStartMetrics.AppStartType.COLD - : AppStartMetrics.AppStartType.WARM); - } + AppStartMetrics.getInstance().updateAppStartType(hasBundle, lastPausedUptimeMillis); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java index 638baf7bf8..75ab67e6a5 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java @@ -14,6 +14,8 @@ import android.os.Looper; import android.os.Process; import android.os.SystemClock; +import android.util.Log; +import androidx.annotation.NonNull; import io.sentry.ILogger; import io.sentry.ITransactionProfiler; import io.sentry.JsonSerializer; @@ -33,8 +35,8 @@ import java.io.FileNotFoundException; import java.io.InputStreamReader; import java.io.Reader; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -52,6 +54,8 @@ public final class SentryPerformanceProvider extends EmptySecureContentProvider private final @NotNull ILogger logger; private final @NotNull BuildInfoProvider buildInfoProvider; + private final AtomicInteger activeActivitiesCounter = new AtomicInteger(); + private final AtomicBoolean firstDrawDone = new AtomicBoolean(false); @TestOnly SentryPerformanceProvider( @@ -201,35 +205,22 @@ private void onAppLaunched( appStartTimespan.setStartedAt(Process.getStartUptimeMillis()); appStartMetrics.registerApplicationForegroundCheck(app); - final AtomicBoolean firstDrawDone = new AtomicBoolean(false); - activityCallback = new ActivityLifecycleCallbacksAdapter() { @Override public void onActivityCreated( @NotNull Activity activity, @Nullable Bundle savedInstanceState) { + Log.d("TAG", "onActivityCreated"); + activeActivitiesCounter.incrementAndGet(); + // In case the SDK gets initialized async or the // ActivityLifecycleIntegration is not enabled (e.g on RN due to Context not being // instanceof Application) // the app start type never gets set - if (appStartMetrics.getAppStartType() == AppStartMetrics.AppStartType.UNKNOWN) { - // We consider pre-loaded application loads as warm starts - // This usually happens e.g. due to BroadcastReceivers triggering - // Application.onCreate only, but no Activity.onCreate + if (!firstDrawDone.get()) { final long now = SystemClock.uptimeMillis(); - final long durationMs = - now - appStartMetrics.getAppStartTimeSpan().getStartUptimeMs(); - if (durationMs > TimeUnit.SECONDS.toMillis(1)) { - appStartMetrics.restartAppStart(now); - appStartMetrics.setAppStartType(AppStartMetrics.AppStartType.WARM); - } else { - // Otherwise a non-null bundle determines the behavior - appStartMetrics.setAppStartType( - savedInstanceState == null - ? AppStartMetrics.AppStartType.COLD - : AppStartMetrics.AppStartType.WARM); - } + AppStartMetrics.getInstance().updateAppStartType(savedInstanceState != null, now); } } @@ -245,20 +236,26 @@ public void onActivityStarted(@NotNull Activity activity) { new Handler(Looper.getMainLooper()).post(() -> onAppStartDone()); } } + + @Override + public void onActivityDestroyed(@NonNull Activity activity) { + final int remainingActivities = activeActivitiesCounter.decrementAndGet(); + // if the app is moving into background, reset firstDrawDone + // as the next Activity is considered like a new app start + if (remainingActivities == 0 && !activity.isChangingConfigurations()) { + firstDrawDone.set(false); + } + } }; app.registerActivityLifecycleCallbacks(activityCallback); } synchronized void onAppStartDone() { - final @NotNull AppStartMetrics appStartMetrics = AppStartMetrics.getInstance(); - appStartMetrics.getSdkInitTimeSpan().stop(); - appStartMetrics.getAppStartTimeSpan().stop(); - - if (app != null) { - if (activityCallback != null) { - app.unregisterActivityLifecycleCallbacks(activityCallback); - } + if (!firstDrawDone.getAndSet(true)) { + final @NotNull AppStartMetrics appStartMetrics = AppStartMetrics.getInstance(); + appStartMetrics.getSdkInitTimeSpan().stop(); + appStartMetrics.getAppStartTimeSpan().stop(); } } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java b/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java index 2e249d6dcc..a14ed86712 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java @@ -354,4 +354,26 @@ public static void onContentProviderPostCreate(final @NotNull ContentProvider co measurement.setStoppedAt(now); } } + + /** + * @param hasBundle true if the activity onCreate had a non-null bundle + * @param lastKnownStart in case the app start is too long, resets the app start timestamp to this + * value + */ + public void updateAppStartType(final boolean hasBundle, final long lastKnownStart) { + final @NotNull TimeSpan appStartSpan = getInstance().getAppStartTimeSpan(); + // If the app start span already started and stopped, it means the app restarted without + // killing the process, so we are in a warm start + // If the app has an invalid cold start, it means it was started in the background, like + // via BroadcastReceiver, so we consider it a warm start + if ((appStartSpan.hasStarted() && appStartSpan.hasStopped()) + || (!getInstance().isColdStartValid())) { + getInstance().restartAppStart(lastKnownStart); + getInstance().setAppStartType(AppStartMetrics.AppStartType.WARM); + } else { + getInstance() + .setAppStartType( + hasBundle ? AppStartMetrics.AppStartType.WARM : AppStartMetrics.AppStartType.COLD); + } + } } From 7c4b39adb2746c21ca1ec368794c3f119b987b3f Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Mon, 3 Mar 2025 21:33:16 +0100 Subject: [PATCH 06/15] Move all app start handling to AppStartMetrics --- .../api/sentry-android-core.api | 6 +- .../core/ActivityLifecycleIntegration.java | 18 --- .../io/sentry/android/core/SentryAndroid.java | 2 +- .../core/SentryPerformanceProvider.java | 74 +-------- .../core/performance/AppStartMetrics.java | 141 ++++++++++-------- .../core/SentryPerformanceProviderTest.kt | 94 +----------- .../core/performance/AppStartMetricsTest.kt | 129 ++++++++++++---- 7 files changed, 191 insertions(+), 273 deletions(-) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index e1d8ed09b2..05fb487b82 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -447,14 +447,15 @@ public class io/sentry/android/core/performance/AppStartMetrics : io/sentry/andr public static fun getInstance ()Lio/sentry/android/core/performance/AppStartMetrics; public fun getSdkInitTimeSpan ()Lio/sentry/android/core/performance/TimeSpan; public fun isAppLaunchedInForeground ()Z - public fun isColdStartValid ()Z public fun onActivityCreated (Landroid/app/Activity;Landroid/os/Bundle;)V + public fun onActivityDestroyed (Landroid/app/Activity;)V + public fun onActivityStarted (Landroid/app/Activity;)V public fun onAppStartSpansSent ()V public static fun onApplicationCreate (Landroid/app/Application;)V public static fun onApplicationPostCreate (Landroid/app/Application;)V public static fun onContentProviderCreate (Landroid/content/ContentProvider;)V public static fun onContentProviderPostCreate (Landroid/content/ContentProvider;)V - public fun registerApplicationForegroundCheck (Landroid/app/Application;)V + public fun registerLifecycleCallbacks (Landroid/app/Application;)V public fun restartAppStart (J)V public fun setAppLaunchedInForeground (Z)V public fun setAppStartProfiler (Lio/sentry/ITransactionProfiler;)V @@ -462,7 +463,6 @@ public class io/sentry/android/core/performance/AppStartMetrics : io/sentry/andr public fun setAppStartType (Lio/sentry/android/core/performance/AppStartMetrics$AppStartType;)V public fun setClassLoadedUptimeMs (J)V public fun shouldSendStartMeasurements ()Z - public fun updateAppStartType (ZJ)V } public final class io/sentry/android/core/performance/AppStartMetrics$AppStartType : java/lang/Enum { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java index cc44f24c2f..61d93653cd 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java @@ -397,7 +397,6 @@ public synchronized void onActivityCreated( if (!isAllActivityCallbacksAvailable) { onActivityPreCreated(activity, savedInstanceState); } - setColdStart(savedInstanceState != null); if (hub != null && options != null && options.isEnableScreenTracking()) { final @Nullable String activityClassName = ClassUtil.getClassName(activity); hub.configureScope(scope -> scope.setScreen(activityClassName)); @@ -553,17 +552,6 @@ public synchronized void onActivityDestroyed(final @NotNull Activity activity) { // activity stack still. // if the activity is opened again and not in memory, transactions will be created normally. activitiesWithOngoingTransactions.remove(activity); - - if (activitiesWithOngoingTransactions.isEmpty()) { - clear(); - } - } - - private void clear() { - firstActivityCreated = false; - lastPausedTime = new SentryNanotimeDate(new Date(0), 0); - lastPausedUptimeMillis = 0; - activityLifecycleMap.clear(); } private void finishSpan(final @Nullable ISpan span) { @@ -705,12 +693,6 @@ WeakHashMap getTtfdSpanMap() { return ttfdSpanMap; } - private void setColdStart(final boolean hasBundle) { - if (!firstActivityCreated) { - AppStartMetrics.getInstance().updateAppStartType(hasBundle, lastPausedUptimeMillis); - } - } - private @NotNull String getTtidDesc(final @NotNull String activityName) { return activityName + " initial display"; } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java index adeb451332..9fee04f251 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java @@ -152,7 +152,7 @@ public static synchronized void init( } } if (context.getApplicationContext() instanceof Application) { - appStartMetrics.registerApplicationForegroundCheck( + appStartMetrics.registerLifecycleCallbacks( (Application) context.getApplicationContext()); } final @NotNull TimeSpan sdkInitTimeSpan = appStartMetrics.getSdkInitTimeSpan(); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java index 75ab67e6a5..ade1826c2c 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java @@ -3,19 +3,13 @@ import static io.sentry.Sentry.APP_START_PROFILING_CONFIG_FILE_NAME; import android.annotation.SuppressLint; -import android.app.Activity; import android.app.Application; import android.content.Context; import android.content.pm.ProviderInfo; import android.net.Uri; import android.os.Build; -import android.os.Bundle; -import android.os.Handler; -import android.os.Looper; import android.os.Process; import android.os.SystemClock; -import android.util.Log; -import androidx.annotation.NonNull; import io.sentry.ILogger; import io.sentry.ITransactionProfiler; import io.sentry.JsonSerializer; @@ -24,9 +18,7 @@ import io.sentry.SentryLevel; import io.sentry.SentryOptions; import io.sentry.TracesSamplingDecision; -import io.sentry.android.core.internal.util.FirstDrawDoneListener; import io.sentry.android.core.internal.util.SentryFrameMetricsCollector; -import io.sentry.android.core.performance.ActivityLifecycleCallbacksAdapter; import io.sentry.android.core.performance.AppStartMetrics; import io.sentry.android.core.performance.TimeSpan; import java.io.BufferedReader; @@ -35,8 +27,6 @@ import java.io.FileNotFoundException; import java.io.InputStreamReader; import java.io.Reader; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -54,8 +44,6 @@ public final class SentryPerformanceProvider extends EmptySecureContentProvider private final @NotNull ILogger logger; private final @NotNull BuildInfoProvider buildInfoProvider; - private final AtomicInteger activeActivitiesCounter = new AtomicInteger(); - private final AtomicBoolean firstDrawDone = new AtomicBoolean(false); @TestOnly SentryPerformanceProvider( @@ -190,8 +178,9 @@ private void onAppLaunched( // performance v2: Uses Process.getStartUptimeMillis() // requires API level 24+ - if (buildInfoProvider.getSdkInfoVersion() < android.os.Build.VERSION_CODES.N) { - return; + if (buildInfoProvider.getSdkInfoVersion() >= android.os.Build.VERSION_CODES.N) { + final @NotNull TimeSpan appStartTimespan = appStartMetrics.getAppStartTimeSpan(); + appStartTimespan.setStartedAt(Process.getStartUptimeMillis()); } if (context instanceof Application) { @@ -201,61 +190,6 @@ private void onAppLaunched( return; } - final @NotNull TimeSpan appStartTimespan = appStartMetrics.getAppStartTimeSpan(); - appStartTimespan.setStartedAt(Process.getStartUptimeMillis()); - appStartMetrics.registerApplicationForegroundCheck(app); - - activityCallback = - new ActivityLifecycleCallbacksAdapter() { - - @Override - public void onActivityCreated( - @NotNull Activity activity, @Nullable Bundle savedInstanceState) { - Log.d("TAG", "onActivityCreated"); - activeActivitiesCounter.incrementAndGet(); - - // In case the SDK gets initialized async or the - // ActivityLifecycleIntegration is not enabled (e.g on RN due to Context not being - // instanceof Application) - // the app start type never gets set - if (!firstDrawDone.get()) { - final long now = SystemClock.uptimeMillis(); - AppStartMetrics.getInstance().updateAppStartType(savedInstanceState != null, now); - } - } - - @Override - public void onActivityStarted(@NotNull Activity activity) { - if (firstDrawDone.get()) { - return; - } - if (activity.getWindow() != null) { - FirstDrawDoneListener.registerForNextDraw( - activity, () -> onAppStartDone(), buildInfoProvider); - } else { - new Handler(Looper.getMainLooper()).post(() -> onAppStartDone()); - } - } - - @Override - public void onActivityDestroyed(@NonNull Activity activity) { - final int remainingActivities = activeActivitiesCounter.decrementAndGet(); - // if the app is moving into background, reset firstDrawDone - // as the next Activity is considered like a new app start - if (remainingActivities == 0 && !activity.isChangingConfigurations()) { - firstDrawDone.set(false); - } - } - }; - - app.registerActivityLifecycleCallbacks(activityCallback); - } - - synchronized void onAppStartDone() { - if (!firstDrawDone.getAndSet(true)) { - final @NotNull AppStartMetrics appStartMetrics = AppStartMetrics.getInstance(); - appStartMetrics.getSdkInitTimeSpan().stop(); - appStartMetrics.getAppStartTimeSpan().stop(); - } + appStartMetrics.registerLifecycleCallbacks(app); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java b/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java index a14ed86712..0dfd0b4d61 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java @@ -11,17 +11,20 @@ import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; import io.sentry.ITransactionProfiler; -import io.sentry.SentryDate; -import io.sentry.SentryNanotimeDate; +import io.sentry.NoOpLogger; import io.sentry.TracesSamplingDecision; +import io.sentry.android.core.BuildInfoProvider; import io.sentry.android.core.ContextUtils; import io.sentry.android.core.SentryAndroidOptions; +import io.sentry.android.core.internal.util.FirstDrawDoneListener; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.TestOnly; @@ -30,6 +33,9 @@ * An in-memory representation for app-metrics during app start. As the SDK can't be initialized * that early, we can't use transactions or spans directly. Thus simple TimeSpans are used and later * transformed into SDK specific txn/span data structures. + * + *

This class is also responsible for - determining the app start type (cold, warm) - determining + * if the app was launched in foreground */ @ApiStatus.Internal public class AppStartMetrics extends ActivityLifecycleCallbacksAdapter { @@ -45,7 +51,7 @@ public enum AppStartType { private static volatile @Nullable AppStartMetrics instance; private @NotNull AppStartType appStartType = AppStartType.UNKNOWN; - private boolean appLaunchedInForeground = false; + private boolean appLaunchedInForeground; private final @NotNull TimeSpan appStartSpan; private final @NotNull TimeSpan sdkInitTimeSpan; @@ -54,10 +60,10 @@ public enum AppStartType { private final @NotNull List activityLifecycles; private @Nullable ITransactionProfiler appStartProfiler = null; private @Nullable TracesSamplingDecision appStartSamplingDecision = null; - private @Nullable SentryDate onCreateTime = null; - private boolean appLaunchTooLong = false; private boolean isCallbackRegistered = false; private boolean shouldSendStartMeasurements = true; + private final AtomicInteger activeActivitiesCounter = new AtomicInteger(); + private final AtomicBoolean firstDrawDone = new AtomicBoolean(false); public static @NotNull AppStartMetrics getInstance() { @@ -116,10 +122,6 @@ public boolean isAppLaunchedInForeground() { return appLaunchedInForeground; } - public boolean isColdStartValid() { - return appLaunchedInForeground && !appLaunchTooLong; - } - @VisibleForTesting public void setAppLaunchedInForeground(final boolean appLaunchedInForeground) { this.appLaunchedInForeground = appLaunchedInForeground; @@ -158,7 +160,6 @@ public boolean shouldSendStartMeasurements() { public void restartAppStart(final long uptimeMillis) { shouldSendStartMeasurements = true; - appLaunchTooLong = false; appLaunchedInForeground = true; appStartSpan.reset(); appStartSpan.start(); @@ -177,13 +178,15 @@ public long getClassLoadedUptimeMs() { public @NotNull TimeSpan getAppStartTimeSpanWithFallback( final @NotNull SentryAndroidOptions options) { // If the app launch took too long or it was launched in the background we return an empty span - if (!isColdStartValid()) { + if (appStartType == AppStartType.UNKNOWN) { return new TimeSpan(); } + if (options.isEnablePerformanceV2()) { // Only started when sdk version is >= N final @NotNull TimeSpan appStartSpan = getAppStartTimeSpan(); - if (appStartSpan.hasStarted()) { + if (appStartSpan.hasStarted() + && appStartSpan.getDurationMs() <= TimeUnit.MINUTES.toMillis(1)) { return appStartSpan; } } @@ -205,11 +208,11 @@ public void clear() { } appStartProfiler = null; appStartSamplingDecision = null; - appLaunchTooLong = false; appLaunchedInForeground = false; - onCreateTime = null; isCallbackRegistered = false; shouldSendStartMeasurements = true; + firstDrawDone.set(false); + activeActivitiesCounter.set(0); } public @Nullable ITransactionProfiler getAppStartProfiler() { @@ -247,7 +250,23 @@ public static void onApplicationCreate(final @NotNull Application application) { final @NotNull AppStartMetrics instance = getInstance(); if (instance.applicationOnCreate.hasNotStarted()) { instance.applicationOnCreate.setStartedAt(now); - instance.registerApplicationForegroundCheck(application); + instance.registerLifecycleCallbacks(application); + } + } + + /** + * Called by instrumentation + * + * @param application The application object where onCreate was called on + * @noinspection unused + */ + public static void onApplicationPostCreate(final @NotNull Application application) { + final long now = SystemClock.uptimeMillis(); + + final @NotNull AppStartMetrics instance = getInstance(); + if (instance.applicationOnCreate.hasNotStopped()) { + instance.applicationOnCreate.setDescription(application.getClass().getName() + ".onCreate"); + instance.applicationOnCreate.setStoppedAt(now); } } @@ -256,7 +275,7 @@ public static void onApplicationCreate(final @NotNull Application application) { * * @param application The application object to register the callback to */ - public void registerApplicationForegroundCheck(final @NotNull Application application) { + public void registerLifecycleCallbacks(final @NotNull Application application) { if (isCallbackRegistered) { return; } @@ -267,15 +286,15 @@ public void registerApplicationForegroundCheck(final @NotNull Application applic // (possibly others) the first task posted on the main thread is called before the // Activity.onCreate callback. This is a workaround for that, so that the Activity.onCreate // callback is called before the application one. - new Handler(Looper.getMainLooper()).post(() -> checkCreateTimeOnMain(application)); + new Handler(Looper.getMainLooper()).post(() -> checkCreateTimeOnMain()); } - private void checkCreateTimeOnMain(final @NotNull Application application) { + private void checkCreateTimeOnMain() { new Handler(Looper.getMainLooper()) .post( () -> { // if no activity has ever been created, app was launched in background - if (onCreateTime == null) { + if (activeActivitiesCounter.get() == 0) { appLaunchedInForeground = false; // we stop the app start profiler, as it's useless and likely to timeout @@ -284,43 +303,49 @@ private void checkCreateTimeOnMain(final @NotNull Application application) { appStartProfiler = null; } } - application.unregisterActivityLifecycleCallbacks(instance); }); } @Override public void onActivityCreated(@NonNull Activity activity, @Nullable Bundle savedInstanceState) { - // An activity already called onCreate() - if (!appLaunchedInForeground || onCreateTime != null) { + final long nowUptimeMs = SystemClock.uptimeMillis(); + activeActivitiesCounter.incrementAndGet(); + appLaunchedInForeground = true; + + // the first undrawn activity determines the app start type + if (activeActivitiesCounter.get() == 1 && !firstDrawDone.get()) { + // If the app (process) was launched more than 1 minute ago, it's likely wrong + final long durationSinceAppStartMillis = nowUptimeMs - appStartSpan.getStartUptimeMs(); + if (durationSinceAppStartMillis > TimeUnit.MINUTES.toMillis(1)) { + appStartType = AppStartType.WARM; + } else { + appStartType = savedInstanceState == null ? AppStartType.COLD : AppStartType.WARM; + } + } + } + + @Override + public void onActivityStarted(@NonNull Activity activity) { + if (firstDrawDone.get()) { return; } - onCreateTime = new SentryNanotimeDate(); - - final long spanStartMillis = appStartSpan.getStartTimestampMs(); - final long spanEndMillis = - appStartSpan.hasStopped() - ? appStartSpan.getProjectedStopTimestampMs() - : System.currentTimeMillis(); - final long durationMillis = spanEndMillis - spanStartMillis; - // If the app was launched more than 1 minute ago, it's likely wrong - if (durationMillis > TimeUnit.MINUTES.toMillis(1)) { - appLaunchTooLong = true; + if (activity.getWindow() != null) { + FirstDrawDoneListener.registerForNextDraw( + activity, () -> onFirstFrameDrawn(), new BuildInfoProvider(NoOpLogger.getInstance())); + } else { + new Handler(Looper.getMainLooper()).post(() -> onFirstFrameDrawn()); } } - /** - * Called by instrumentation - * - * @param application The application object where onCreate was called on - * @noinspection unused - */ - public static void onApplicationPostCreate(final @NotNull Application application) { - final long now = SystemClock.uptimeMillis(); - - final @NotNull AppStartMetrics instance = getInstance(); - if (instance.applicationOnCreate.hasNotStopped()) { - instance.applicationOnCreate.setDescription(application.getClass().getName() + ".onCreate"); - instance.applicationOnCreate.setStoppedAt(now); + @Override + public void onActivityDestroyed(@NonNull Activity activity) { + final int remainingActivities = activeActivitiesCounter.decrementAndGet(); + // if the app is moving into background + // as the next Activity is considered like a new app start + if (remainingActivities == 0 && !activity.isChangingConfigurations()) { + appLaunchedInForeground = false; + shouldSendStartMeasurements = true; + firstDrawDone.set(false); } } @@ -355,25 +380,11 @@ public static void onContentProviderPostCreate(final @NotNull ContentProvider co } } - /** - * @param hasBundle true if the activity onCreate had a non-null bundle - * @param lastKnownStart in case the app start is too long, resets the app start timestamp to this - * value - */ - public void updateAppStartType(final boolean hasBundle, final long lastKnownStart) { - final @NotNull TimeSpan appStartSpan = getInstance().getAppStartTimeSpan(); - // If the app start span already started and stopped, it means the app restarted without - // killing the process, so we are in a warm start - // If the app has an invalid cold start, it means it was started in the background, like - // via BroadcastReceiver, so we consider it a warm start - if ((appStartSpan.hasStarted() && appStartSpan.hasStopped()) - || (!getInstance().isColdStartValid())) { - getInstance().restartAppStart(lastKnownStart); - getInstance().setAppStartType(AppStartMetrics.AppStartType.WARM); - } else { - getInstance() - .setAppStartType( - hasBundle ? AppStartMetrics.AppStartType.WARM : AppStartMetrics.AppStartType.COLD); + synchronized void onFirstFrameDrawn() { + if (!firstDrawDone.getAndSet(true)) { + final @NotNull AppStartMetrics appStartMetrics = getInstance(); + appStartMetrics.getSdkInitTimeSpan().stop(); + appStartMetrics.getAppStartTimeSpan().stop(); } } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt index b56254260f..5e816f6ca5 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt @@ -1,11 +1,9 @@ package io.sentry.android.core -import android.app.Activity import android.app.Application import android.app.Application.ActivityLifecycleCallbacks import android.content.pm.ProviderInfo import android.os.Build -import android.os.Bundle import androidx.test.ext.junit.runners.AndroidJUnit4 import io.sentry.ILogger import io.sentry.JsonSerializer @@ -19,7 +17,6 @@ import org.mockito.kotlin.any import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never -import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.robolectric.annotation.Config @@ -29,7 +26,6 @@ import java.nio.file.Files import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test -import kotlin.test.assertEquals import kotlin.test.assertFailsWith import kotlin.test.assertFalse import kotlin.test.assertNotNull @@ -52,7 +48,7 @@ class SentryPerformanceProviderTest { val providerInfo = ProviderInfo() val logger = mock() lateinit var configFile: File - var activityLifecycleCallback: ActivityLifecycleCallbacks? = null + var activityLifecycleCallbacks: MutableList = mutableListOf() fun getSut(sdkVersion: Int = Build.VERSION_CODES.S, authority: String = AUTHORITY, handleFile: ((config: File) -> Unit)? = null): SentryPerformanceProvider { val buildInfoProvider: BuildInfoProvider = mock() @@ -62,7 +58,11 @@ class SentryPerformanceProviderTest { configFile = File(sentryCache, Sentry.APP_START_PROFILING_CONFIG_FILE_NAME) handleFile?.invoke(configFile) whenever(mockContext.registerActivityLifecycleCallbacks(any())).then { - activityLifecycleCallback = it.arguments[0] as ActivityLifecycleCallbacks + activityLifecycleCallbacks.add(it.arguments[0] as ActivityLifecycleCallbacks) + return@then Unit + } + whenever(mockContext.unregisterActivityLifecycleCallbacks(any())).then { + activityLifecycleCallbacks.remove(it.arguments[0] as ActivityLifecycleCallbacks) return@then Unit } providerInfo.authority = authority @@ -112,24 +112,11 @@ class SentryPerformanceProviderTest { @Test fun `provider sets both appstart and sdk init start + end times`() { val provider = fixture.getSut() - provider.onAppStartDone() + provider.onCreate() val metrics = AppStartMetrics.getInstance() assertTrue(metrics.appStartTimeSpan.hasStarted()) - assertTrue(metrics.appStartTimeSpan.hasStopped()) - assertTrue(metrics.sdkInitTimeSpan.hasStarted()) - assertTrue(metrics.sdkInitTimeSpan.hasStopped()) - } - - @Test - fun `provider properly registers and unregisters ActivityLifecycleCallbacks`() { - val provider = fixture.getSut() - - // It register once for the provider itself and once for the appStartMetrics - verify(fixture.mockContext, times(2)).registerActivityLifecycleCallbacks(any()) - provider.onAppStartDone() - verify(fixture.mockContext).unregisterActivityLifecycleCallbacks(any()) } //region app start profiling @@ -240,73 +227,6 @@ class SentryPerformanceProviderTest { assertFalse(AppStartMetrics.getInstance().appStartProfiler!!.isRunning) } - @Test - fun `Sets app launch type to cold`() { - val provider = fixture.getSut() - val activity = mock() - provider.onCreate() - - assertEquals(AppStartMetrics.AppStartType.UNKNOWN, AppStartMetrics.getInstance().appStartType) - - // when the first activity has no bundle - fixture.activityLifecycleCallback!!.onActivityCreated(activity, null) - - // then the app start is considered cold - assertEquals(AppStartMetrics.AppStartType.COLD, AppStartMetrics.getInstance().appStartType) - - // when any subsequent activity launches - fixture.activityLifecycleCallback!!.onActivityCreated(activity, mock()) - - // then the app start is still considered cold - assertEquals(AppStartMetrics.AppStartType.COLD, AppStartMetrics.getInstance().appStartType) - } - - @Test - fun `Sets app launch type to warm if process init is too old`() { - val provider = fixture.getSut() - val activity = mock() - provider.onCreate() - - assertEquals(AppStartMetrics.AppStartType.UNKNOWN, AppStartMetrics.getInstance().appStartType) - - AppStartMetrics.getInstance().appStartTimeSpan.setStartedAt( - AppStartMetrics.getInstance().appStartTimeSpan.startUptimeMs - 20000 - ) - - // when the first activity has no bundle - fixture.activityLifecycleCallback!!.onActivityCreated(activity, null) - - // then the app start is considered warm - assertEquals(AppStartMetrics.AppStartType.WARM, AppStartMetrics.getInstance().appStartType) - - // when any subsequent activity launches - fixture.activityLifecycleCallback!!.onActivityCreated(activity, mock()) - - // then the app start is still considered warm - assertEquals(AppStartMetrics.AppStartType.WARM, AppStartMetrics.getInstance().appStartType) - } - - @Test - fun `Sets app launch type to warm`() { - val provider = fixture.getSut() - val activity = mock() - provider.onCreate() - - assertEquals(AppStartMetrics.AppStartType.UNKNOWN, AppStartMetrics.getInstance().appStartType) - - // when the first activity has a bundle - fixture.activityLifecycleCallback!!.onActivityCreated(activity, mock()) - - // then the app start is considered WARM - assertEquals(AppStartMetrics.AppStartType.WARM, AppStartMetrics.getInstance().appStartType) - - // when any subsequent activity launches - fixture.activityLifecycleCallback!!.onActivityCreated(activity, null) - - // then the app start is still considered warm - assertEquals(AppStartMetrics.AppStartType.WARM, AppStartMetrics.getInstance().appStartType) - } - private fun writeConfig( configFile: File, profilingEnabled: Boolean = true, diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt index d8b9e727e2..7ed8499792 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt @@ -1,9 +1,12 @@ package io.sentry.android.core.performance +import android.app.Activity import android.app.Application import android.content.ContentProvider import android.os.Build +import android.os.Bundle import android.os.Looper +import android.os.SystemClock import androidx.test.ext.junit.runners.AndroidJUnit4 import io.sentry.ITransactionProfiler import io.sentry.android.core.SentryAndroidOptions @@ -75,6 +78,7 @@ class AppStartMetricsTest { @Test fun `if perf-2 is enabled and app start time span is started, appStartTimeSpanWithFallback returns it`() { val appStartTimeSpan = AppStartMetrics.getInstance().appStartTimeSpan + AppStartMetrics.getInstance().appStartType = AppStartMetrics.AppStartType.WARM appStartTimeSpan.start() val options = SentryAndroidOptions().apply { @@ -88,6 +92,8 @@ class AppStartMetricsTest { @Test fun `if perf-2 is disabled but app start time span has started, appStartTimeSpanWithFallback returns the sdk init span instead`() { val appStartTimeSpan = AppStartMetrics.getInstance().appStartTimeSpan + AppStartMetrics.getInstance().appStartType = AppStartMetrics.AppStartType.COLD + appStartTimeSpan.start() val options = SentryAndroidOptions().apply { @@ -102,6 +108,7 @@ class AppStartMetricsTest { @Test fun `if perf-2 is enabled but app start time span has not started, appStartTimeSpanWithFallback returns the sdk init span instead`() { val appStartTimeSpan = AppStartMetrics.getInstance().appStartTimeSpan + AppStartMetrics.getInstance().appStartType = AppStartMetrics.AppStartType.COLD assertTrue(appStartTimeSpan.hasNotStarted()) val options = SentryAndroidOptions().apply { @@ -121,6 +128,8 @@ class AppStartMetricsTest { @Test fun `if app is launched in background, appStartTimeSpanWithFallback returns an empty span`() { AppStartMetrics.getInstance().isAppLaunchedInForeground = false + AppStartMetrics.getInstance().appStartType = AppStartMetrics.AppStartType.COLD + val appStartTimeSpan = AppStartMetrics.getInstance().appStartTimeSpan appStartTimeSpan.start() assertTrue(appStartTimeSpan.hasStarted()) @@ -172,14 +181,15 @@ class AppStartMetricsTest { @Test fun `if activity is never started, returns an empty span`() { - AppStartMetrics.getInstance().registerApplicationForegroundCheck(mock()) + AppStartMetrics.getInstance().registerLifecycleCallbacks(mock()) val appStartTimeSpan = AppStartMetrics.getInstance().appStartTimeSpan appStartTimeSpan.setStartedAt(1) assertTrue(appStartTimeSpan.hasStarted()) // Job on main thread checks if activity was launched Shadows.shadowOf(Looper.getMainLooper()).idle() - val timeSpan = AppStartMetrics.getInstance().getAppStartTimeSpanWithFallback(SentryAndroidOptions()) + val timeSpan = + AppStartMetrics.getInstance().getAppStartTimeSpanWithFallback(SentryAndroidOptions()) assertFalse(timeSpan.hasStarted()) } @@ -189,7 +199,7 @@ class AppStartMetricsTest { whenever(profiler.isRunning).thenReturn(true) AppStartMetrics.getInstance().appStartProfiler = profiler - AppStartMetrics.getInstance().registerApplicationForegroundCheck(mock()) + AppStartMetrics.getInstance().registerLifecycleCallbacks(mock()) // Job on main thread checks if activity was launched Shadows.shadowOf(Looper.getMainLooper()).idle() @@ -203,7 +213,7 @@ class AppStartMetricsTest { AppStartMetrics.getInstance().appStartProfiler = profiler AppStartMetrics.getInstance().onActivityCreated(mock(), mock()) - AppStartMetrics.getInstance().registerApplicationForegroundCheck(mock()) + AppStartMetrics.getInstance().registerLifecycleCallbacks(mock()) // Job on main thread checks if activity was launched Shadows.shadowOf(Looper.getMainLooper()).idle() @@ -231,33 +241,26 @@ class AppStartMetricsTest { @Test fun `when multiple registerApplicationForegroundCheck, only one callback is registered to application`() { val application = mock() - AppStartMetrics.getInstance().registerApplicationForegroundCheck(application) - AppStartMetrics.getInstance().registerApplicationForegroundCheck(application) - verify(application, times(1)).registerActivityLifecycleCallbacks(eq(AppStartMetrics.getInstance())) + AppStartMetrics.getInstance().registerLifecycleCallbacks(application) + AppStartMetrics.getInstance().registerLifecycleCallbacks(application) + verify( + application, + times(1) + ).registerActivityLifecycleCallbacks(eq(AppStartMetrics.getInstance())) } @Test fun `when registerApplicationForegroundCheck, a callback is registered to application`() { val application = mock() - AppStartMetrics.getInstance().registerApplicationForegroundCheck(application) + AppStartMetrics.getInstance().registerLifecycleCallbacks(application) verify(application).registerActivityLifecycleCallbacks(eq(AppStartMetrics.getInstance())) } - @Test - fun `when registerApplicationForegroundCheck, a job is posted on main thread to unregistered the callback`() { - val application = mock() - AppStartMetrics.getInstance().registerApplicationForegroundCheck(application) - verify(application).registerActivityLifecycleCallbacks(eq(AppStartMetrics.getInstance())) - verify(application, never()).unregisterActivityLifecycleCallbacks(eq(AppStartMetrics.getInstance())) - Shadows.shadowOf(Looper.getMainLooper()).idle() - verify(application).unregisterActivityLifecycleCallbacks(eq(AppStartMetrics.getInstance())) - } - @Test fun `registerApplicationForegroundCheck set foreground state to false if no activity is running`() { val application = mock() AppStartMetrics.getInstance().isAppLaunchedInForeground = true - AppStartMetrics.getInstance().registerApplicationForegroundCheck(application) + AppStartMetrics.getInstance().registerLifecycleCallbacks(application) assertTrue(AppStartMetrics.getInstance().isAppLaunchedInForeground) // Main thread performs the check and sets the flag to false if no activity was created Shadows.shadowOf(Looper.getMainLooper()).idle() @@ -268,7 +271,7 @@ class AppStartMetricsTest { fun `registerApplicationForegroundCheck keeps foreground state to true if an activity is running`() { val application = mock() AppStartMetrics.getInstance().isAppLaunchedInForeground = true - AppStartMetrics.getInstance().registerApplicationForegroundCheck(application) + AppStartMetrics.getInstance().registerLifecycleCallbacks(application) assertTrue(AppStartMetrics.getInstance().isAppLaunchedInForeground) // An activity was created AppStartMetrics.getInstance().onActivityCreated(mock(), null) @@ -277,12 +280,6 @@ class AppStartMetricsTest { assertTrue(AppStartMetrics.getInstance().isAppLaunchedInForeground) } - @Test - fun `isColdStartValid is false if app was launched in background`() { - AppStartMetrics.getInstance().isAppLaunchedInForeground = false - assertFalse(AppStartMetrics.getInstance().isColdStartValid) - } - @Test fun `isColdStartValid is false if app launched in more than 1 minute`() { val appStartTimeSpan = AppStartMetrics.getInstance().appStartTimeSpan @@ -291,7 +288,6 @@ class AppStartMetricsTest { appStartTimeSpan.setStartedAt(1) appStartTimeSpan.setStoppedAt(TimeUnit.MINUTES.toMillis(1) + 2) AppStartMetrics.getInstance().onActivityCreated(mock(), mock()) - assertFalse(AppStartMetrics.getInstance().isColdStartValid) } @Test @@ -312,14 +308,89 @@ class AppStartMetricsTest { appStartMetrics.onAppStartSpansSent() appStartMetrics.isAppLaunchedInForeground = false assertFalse(appStartMetrics.shouldSendStartMeasurements()) - assertFalse(appStartMetrics.isColdStartValid) appStartMetrics.restartAppStart(10) assertTrue(appStartMetrics.shouldSendStartMeasurements()) - assertTrue(appStartMetrics.isColdStartValid) assertTrue(appStartMetrics.appStartTimeSpan.hasStarted()) assertTrue(appStartMetrics.appStartTimeSpan.hasNotStopped()) assertEquals(10, appStartMetrics.appStartTimeSpan.startUptimeMs) } + + @Test + fun `provider sets both appstart and sdk init start + end times`() { + val metrics = AppStartMetrics.getInstance() + metrics.appStartTimeSpan.start() + metrics.sdkInitTimeSpan.start() + + assertFalse(metrics.appStartTimeSpan.hasStopped()) + assertFalse(metrics.sdkInitTimeSpan.hasStopped()) + + metrics.onFirstFrameDrawn() + + assertTrue(metrics.appStartTimeSpan.hasStopped()) + assertTrue(metrics.sdkInitTimeSpan.hasStopped()) + } + + @Test + fun `Sets app launch type to cold`() { + val metrics = AppStartMetrics.getInstance() + assertEquals( + AppStartMetrics.AppStartType.UNKNOWN, + AppStartMetrics.getInstance().appStartType + ) + + val app = mock() + metrics.registerLifecycleCallbacks(app) + metrics.onActivityCreated(mock(), null) + + // then the app start is considered cold + assertEquals(AppStartMetrics.AppStartType.COLD, AppStartMetrics.getInstance().appStartType) + + // when any subsequent activity launches + metrics.onActivityCreated(mock(), mock()) + + // then the app start is still considered cold + assertEquals(AppStartMetrics.AppStartType.COLD, AppStartMetrics.getInstance().appStartType) + } + + @Test + fun `Sets app launch type to warm if process init was too long ago`() { + val metrics = AppStartMetrics.getInstance() + assertEquals( + AppStartMetrics.AppStartType.UNKNOWN, + AppStartMetrics.getInstance().appStartType + ) + val app = mock() + metrics.registerLifecycleCallbacks(app) + + // when an activity is created later with a null bundle + SystemClock.setCurrentTimeMillis(TimeUnit.MINUTES.toMillis(2)) + metrics.onActivityCreated(mock(), null) + + // then the app start is considered warm + assertEquals(AppStartMetrics.AppStartType.WARM, AppStartMetrics.getInstance().appStartType) + } + + @Test + fun `Sets app launch type to warm`() { + val metrics = AppStartMetrics.getInstance() + assertEquals( + AppStartMetrics.AppStartType.UNKNOWN, + AppStartMetrics.getInstance().appStartType + ) + + val app = mock() + metrics.registerLifecycleCallbacks(app) + metrics.onActivityCreated(mock(), mock()) + + // then the app start is considered warm + assertEquals(AppStartMetrics.AppStartType.WARM, AppStartMetrics.getInstance().appStartType) + + // when any subsequent activity launches + metrics.onActivityCreated(mock(), null) + + // then the app start is still considered warm + assertEquals(AppStartMetrics.AppStartType.WARM, AppStartMetrics.getInstance().appStartType) + } } From 9289aa74d1d074d87d51fbc95a7cc133a17750c1 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 4 Mar 2025 15:25:57 +0100 Subject: [PATCH 07/15] Make tests happy --- .../core/performance/AppStartMetrics.java | 4 +- .../core/ActivityLifecycleIntegrationTest.kt | 138 +----------------- .../PerformanceAndroidEventProcessorTest.kt | 3 +- .../core/performance/AppStartMetricsTest.kt | 16 -- 4 files changed, 11 insertions(+), 150 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java b/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java index 0dfd0b4d61..0bf2476438 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java @@ -155,7 +155,7 @@ public void onAppStartSpansSent() { } public boolean shouldSendStartMeasurements() { - return shouldSendStartMeasurements; + return shouldSendStartMeasurements && appLaunchedInForeground; } public void restartAppStart(final long uptimeMillis) { @@ -344,7 +344,7 @@ public void onActivityDestroyed(@NonNull Activity activity) { // as the next Activity is considered like a new app start if (remainingActivities == 0 && !activity.isChangingConfigurations()) { appLaunchedInForeground = false; - shouldSendStartMeasurements = true; + shouldSendStartMeasurements = false; firstDrawDone.set(false); } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt index b212ed2fea..c14d6c82c4 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt @@ -54,7 +54,6 @@ import org.robolectric.shadow.api.Shadow import org.robolectric.shadows.ShadowActivityManager import java.util.Date import java.util.concurrent.Future -import java.util.concurrent.TimeUnit import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test @@ -94,7 +93,10 @@ class ActivityLifecycleIntegrationTest { whenever(hub.options).thenReturn(options) - AppStartMetrics.getInstance().isAppLaunchedInForeground = true + val metrics = AppStartMetrics.getInstance() + metrics.isAppLaunchedInForeground = true + metrics.appStartTimeSpan.start() + // We let the ActivityLifecycleIntegration create the proper transaction here val optionCaptor = argumentCaptor() val contextCaptor = argumentCaptor() @@ -594,45 +596,6 @@ class ActivityLifecycleIntegrationTest { verify(ttfdReporter, never()).registerFullyDrawnListener(any()) } - @Test - fun `App start is Cold when savedInstanceState is null`() { - val sut = fixture.getSut() - fixture.options.tracesSampleRate = 1.0 - sut.register(fixture.hub, fixture.options) - - val activity = mock() - sut.onActivityCreated(activity, null) - - assertEquals(AppStartType.COLD, AppStartMetrics.getInstance().appStartType) - } - - @Test - fun `App start is Warm when savedInstanceState is not null`() { - val sut = fixture.getSut() - fixture.options.tracesSampleRate = 1.0 - sut.register(fixture.hub, fixture.options) - - val activity = mock() - val bundle = Bundle() - sut.onActivityCreated(activity, bundle) - - assertEquals(AppStartType.WARM, AppStartMetrics.getInstance().appStartType) - } - - @Test - fun `Do not overwrite App start type after set`() { - val sut = fixture.getSut() - fixture.options.tracesSampleRate = 1.0 - sut.register(fixture.hub, fixture.options) - - val activity = mock() - val bundle = Bundle() - sut.onActivityCreated(activity, bundle) - sut.onActivityCreated(activity, null) - - assertEquals(AppStartType.WARM, AppStartMetrics.getInstance().appStartType) - } - @Test fun `When firstActivityCreated is false, start transaction with given appStartTime`() { val sut = fixture.getSut() @@ -883,86 +846,6 @@ class ActivityLifecycleIntegrationTest { ) } - @Test - fun `When firstActivityCreated is false and bundle is not null, start app start warm span with given appStartTime`() { - val sut = fixture.getSut() - fixture.options.tracesSampleRate = 1.0 - sut.register(fixture.hub, fixture.options) - sut.setFirstActivityCreated(false) - - val date = SentryNanotimeDate(Date(1), 0) - setAppStartTime(date) - - val activity = mock() - sut.onActivityCreated(activity, fixture.bundle) - - val span = fixture.transaction.children.first() - assertEquals(span.operation, "app.start.warm") - assertEquals(span.description, "Warm Start") - assertEquals(span.startDate.nanoTimestamp(), date.nanoTimestamp()) - } - - @Test - fun `When firstActivityCreated is false and bundle is not null, start app start cold span with given appStartTime`() { - val sut = fixture.getSut() - fixture.options.tracesSampleRate = 1.0 - sut.register(fixture.hub, fixture.options) - sut.setFirstActivityCreated(false) - - val date = SentryNanotimeDate(Date(1), 0) - setAppStartTime(date) - - val activity = mock() - sut.onActivityCreated(activity, null) - - val span = fixture.transaction.children.first() - assertEquals(span.operation, "app.start.cold") - assertEquals(span.description, "Cold Start") - assertEquals(span.startDate.nanoTimestamp(), date.nanoTimestamp()) - } - - @Test - fun `When firstActivityCreated is false and app started more than 1 minute ago, start app with Warm start`() { - val sut = fixture.getSut() - fixture.options.tracesSampleRate = 1.0 - sut.register(fixture.hub, fixture.options) - sut.setFirstActivityCreated(false) - - val date = SentryNanotimeDate(Date(1), 0) - val duration = TimeUnit.MINUTES.toMillis(1) + 2 - val durationNanos = TimeUnit.MILLISECONDS.toNanos(duration) - val stopDate = SentryNanotimeDate(Date(duration), durationNanos) - setAppStartTime(date, stopDate) - - val activity = mock() - sut.onActivityCreated(activity, null) - - val span = fixture.transaction.children.first() - assertEquals(span.operation, "app.start.warm") - assertEquals(span.description, "Warm Start") - assertEquals(span.startDate.nanoTimestamp(), date.nanoTimestamp()) - } - - @Test - fun `When firstActivityCreated is false and app started in background, start app with Warm start`() { - val sut = fixture.getSut() - AppStartMetrics.getInstance().isAppLaunchedInForeground = false - fixture.options.tracesSampleRate = 1.0 - sut.register(fixture.hub, fixture.options) - sut.setFirstActivityCreated(false) - - val date = SentryNanotimeDate(Date(1), 0) - setAppStartTime(date) - - val activity = mock() - sut.onActivityCreated(activity, null) - - val span = fixture.transaction.children.first() - assertEquals(span.operation, "app.start.warm") - assertEquals(span.description, "Warm Start") - assertEquals(span.startDate.nanoTimestamp(), date.nanoTimestamp()) - } - @Test fun `When firstActivityCreated is true, start transaction but not with given appStartTime`() { val sut = fixture.getSut() @@ -1467,7 +1350,6 @@ class ActivityLifecycleIntegrationTest { assertEquals(startDate.nanoTimestamp(), sut.getProperty("lastPausedTime").nanoTimestamp()) sut.onActivityCreated(activity, null) - assertNotNull(sut.appStartSpan) sut.onActivityPostCreated(activity, null) assertTrue(activityLifecycleSpan.onCreate.hasStopped()) @@ -1556,15 +1438,6 @@ class ActivityLifecycleIntegrationTest { // lastPausedUptimeMillis is set to current SystemClock.uptimeMillis() val lastUptimeMillis = sut.getProperty("lastPausedUptimeMillis") assertNotEquals(0, lastUptimeMillis) - - sut.onActivityCreated(activity, null) - // AppStartMetrics app start time is set to Activity preCreated timestamp - assertEquals(lastUptimeMillis, appStartMetrics.appStartTimeSpan.startUptimeMs) - // AppStart type is considered warm - assertEquals(AppStartType.WARM, appStartMetrics.appStartType) - - // Activity appStart span timestamp is the same of AppStartMetrics.appStart timestamp - assertEquals(sut.appStartSpan!!.startDate.nanoTimestamp(), appStartMetrics.getAppStartTimeSpanWithFallback(fixture.options).startTimestamp!!.nanoTimestamp()) } private fun SentryTracer.isFinishing() = getProperty("finishStatus").getProperty("isFinishing") @@ -1578,6 +1451,9 @@ class ActivityLifecycleIntegrationTest { private fun setAppStartTime(date: SentryDate = SentryNanotimeDate(Date(1), 0), stopDate: SentryDate? = null) { // set by SentryPerformanceProvider so forcing it here + AppStartMetrics.getInstance().appStartType = AppStartType.COLD + AppStartMetrics.getInstance().isAppLaunchedInForeground = true + val sdkAppStartTimeSpan = AppStartMetrics.getInstance().sdkInitTimeSpan val appStartTimeSpan = AppStartMetrics.getInstance().appStartTimeSpan val millis = DateUtils.nanosToMillis(date.nanoTimestamp().toDouble()).toLong() diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt index 35e0f5257b..c0d18e5db7 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt @@ -75,7 +75,7 @@ class PerformanceAndroidEventProcessorTest { null, null ).also { - AppStartMetrics.getInstance().onActivityCreated(mock(), mock()) + AppStartMetrics.getInstance().onActivityCreated(mock(), if (coldStart) null else mock()) } @BeforeTest @@ -225,6 +225,7 @@ class PerformanceAndroidEventProcessorTest { fun `adds app start metrics to app start txn`() { // given some app start metrics val appStartMetrics = AppStartMetrics.getInstance() + appStartMetrics.isAppLaunchedInForeground = true appStartMetrics.appStartType = AppStartType.COLD appStartMetrics.appStartTimeSpan.setStartedAt(123) appStartMetrics.appStartTimeSpan.setStoppedAt(456) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt index 7ed8499792..0c7f28d19a 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt @@ -144,22 +144,6 @@ class AppStartMetricsTest { assertFalse(timeSpan.hasStarted()) } - @Test - fun `if app is launched in background with perfV2, appStartTimeSpanWithFallback returns an empty span`() { - val appStartTimeSpan = AppStartMetrics.getInstance().appStartTimeSpan - appStartTimeSpan.start() - assertTrue(appStartTimeSpan.hasStarted()) - AppStartMetrics.getInstance().isAppLaunchedInForeground = false - AppStartMetrics.getInstance().onActivityCreated(mock(), mock()) - - val options = SentryAndroidOptions().apply { - isEnablePerformanceV2 = true - } - - val timeSpan = AppStartMetrics.getInstance().getAppStartTimeSpanWithFallback(options) - assertFalse(timeSpan.hasStarted()) - } - @Test fun `if app start span is at most 1 minute, appStartTimeSpanWithFallback returns the app start span`() { val appStartTimeSpan = AppStartMetrics.getInstance().appStartTimeSpan From d564f99ce40abce60309c66ceeec31d95069148a Mon Sep 17 00:00:00 2001 From: Stefano Date: Thu, 23 Jan 2025 17:14:04 +0100 Subject: [PATCH 08/15] Fix flaky RateLimiter test (#4100) * changed RateLimiterTest `close cancels the timer` to use reflection --- .../io/sentry/transport/RateLimiterTest.kt | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt b/sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt index 8d8fb9601e..9fe0cd4087 100644 --- a/sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt +++ b/sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt @@ -28,6 +28,8 @@ import io.sentry.metrics.EncodedMetrics import io.sentry.protocol.SentryId import io.sentry.protocol.SentryTransaction import io.sentry.protocol.User +import io.sentry.test.getProperty +import io.sentry.test.injectForField import org.awaitility.kotlin.await import org.mockito.kotlin.eq import org.mockito.kotlin.mock @@ -37,6 +39,7 @@ import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever import java.io.File +import java.util.Timer import java.util.UUID import java.util.concurrent.atomic.AtomicBoolean import kotlin.test.Test @@ -412,18 +415,16 @@ class RateLimiterTest { @Test fun `close cancels the timer`() { val rateLimiter = fixture.getSUT() - whenever(fixture.currentDateProvider.currentTimeMillis).thenReturn(0, 1, 2001) - - val applied = AtomicBoolean(true) - rateLimiter.addRateLimitObserver { - applied.set(rateLimiter.isActiveForCategory(Replay)) - } + val timer = mock() + rateLimiter.injectForField("timer", timer) - rateLimiter.updateRetryAfterLimits("1:replay:key", null, 1) + // When the rate limiter is closed rateLimiter.close() - // wait for 1.5s to ensure the timer has run after 1s - await.untilTrue(applied) - assertTrue(applied.get()) + // Then the timer is cancelled + verify(timer).cancel() + + // And is removed by the rateLimiter + assertNull(rateLimiter.getProperty("timer")) } } From edb476417f7a3edba8f4acb24bcf0b822f2dedad Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 4 Mar 2025 19:52:22 +0100 Subject: [PATCH 09/15] Update sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java Co-authored-by: Stefano --- .../android/core/ActivityLifecycleIntegration.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java index 61d93653cd..71a849bee8 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java @@ -554,6 +554,15 @@ public synchronized void onActivityDestroyed(final @NotNull Activity activity) { activitiesWithOngoingTransactions.remove(activity); } + if (activitiesWithOngoingTransactions.isEmpty() && !activity.isChangingConfigurations()) { + clear(); + } + } + + private void clear() { + firstActivityCreated = false; + activityLifecycleMap.clear(); + } private void finishSpan(final @Nullable ISpan span) { if (span != null && !span.isFinished()) { span.finish(); From dc65675b547a6bc07df94c31c19faa50b21e3ddb Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 4 Mar 2025 20:36:05 +0100 Subject: [PATCH 10/15] Address PR feedback --- .../api/sentry-android-core.api | 1 - .../core/ActivityLifecycleIntegration.java | 11 +++++ .../core/performance/AppStartMetrics.java | 40 +++++++++---------- .../core/performance/AppStartMetricsTest.kt | 35 +++++++++++----- 4 files changed, 54 insertions(+), 33 deletions(-) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 05fb487b82..8096a5058e 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -456,7 +456,6 @@ public class io/sentry/android/core/performance/AppStartMetrics : io/sentry/andr public static fun onContentProviderCreate (Landroid/content/ContentProvider;)V public static fun onContentProviderPostCreate (Landroid/content/ContentProvider;)V public fun registerLifecycleCallbacks (Landroid/app/Application;)V - public fun restartAppStart (J)V public fun setAppLaunchedInForeground (Z)V public fun setAppStartProfiler (Lio/sentry/ITransactionProfiler;)V public fun setAppStartSamplingDecision (Lio/sentry/TracesSamplingDecision;)V diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java index 61d93653cd..d6299f2a7f 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java @@ -552,6 +552,17 @@ public synchronized void onActivityDestroyed(final @NotNull Activity activity) { // activity stack still. // if the activity is opened again and not in memory, transactions will be created normally. activitiesWithOngoingTransactions.remove(activity); + + if (activitiesWithOngoingTransactions.isEmpty() && !activity.isChangingConfigurations()) { + clear(); + } + } + + private void clear() { + firstActivityCreated = false; + lastPausedTime = new SentryNanotimeDate(new Date(0), 0); + lastPausedUptimeMillis = 0; + activityLifecycleMap.clear(); } private void finishSpan(final @Nullable ISpan span) { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java b/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java index 0bf2476438..95bfa1d5c7 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java @@ -158,15 +158,6 @@ public boolean shouldSendStartMeasurements() { return shouldSendStartMeasurements && appLaunchedInForeground; } - public void restartAppStart(final long uptimeMillis) { - shouldSendStartMeasurements = true; - appLaunchedInForeground = true; - appStartSpan.reset(); - appStartSpan.start(); - appStartSpan.setStartedAt(uptimeMillis); - CLASS_LOADED_UPTIME_MS = appStartSpan.getStartUptimeMs(); - } - public long getClassLoadedUptimeMs() { return CLASS_LOADED_UPTIME_MS; } @@ -177,22 +168,27 @@ public long getClassLoadedUptimeMs() { */ public @NotNull TimeSpan getAppStartTimeSpanWithFallback( final @NotNull SentryAndroidOptions options) { - // If the app launch took too long or it was launched in the background we return an empty span - if (appStartType == AppStartType.UNKNOWN) { - return new TimeSpan(); - } + // If the app start type was never determined or app wasn't launched in foreground, + // the app start is considered invalid + if (appStartType != AppStartType.UNKNOWN && appLaunchedInForeground) { + if (options.isEnablePerformanceV2()) { + // Only started when sdk version is >= N + final @NotNull TimeSpan appStartSpan = getAppStartTimeSpan(); + if (appStartSpan.hasStarted() + && appStartSpan.getDurationMs() <= TimeUnit.MINUTES.toMillis(1)) { + return appStartSpan; + } + } - if (options.isEnablePerformanceV2()) { - // Only started when sdk version is >= N - final @NotNull TimeSpan appStartSpan = getAppStartTimeSpan(); - if (appStartSpan.hasStarted() - && appStartSpan.getDurationMs() <= TimeUnit.MINUTES.toMillis(1)) { - return appStartSpan; + // fallback: use sdk init time span, as it will always have a start time set + final @NotNull TimeSpan sdkInitTimeSpan = getSdkInitTimeSpan(); + if (sdkInitTimeSpan.hasStarted() + && sdkInitTimeSpan.getDurationMs() <= TimeUnit.MINUTES.toMillis(1)) { + return sdkInitTimeSpan; } } - // fallback: use sdk init time span, as it will always have a start time set - return getSdkInitTimeSpan(); + return new TimeSpan(); } @TestOnly @@ -344,7 +340,7 @@ public void onActivityDestroyed(@NonNull Activity activity) { // as the next Activity is considered like a new app start if (remainingActivities == 0 && !activity.isChangingConfigurations()) { appLaunchedInForeground = false; - shouldSendStartMeasurements = false; + shouldSendStartMeasurements = true; firstDrawDone.set(false); } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt index 0c7f28d19a..de8ab871ec 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt @@ -93,8 +93,11 @@ class AppStartMetricsTest { fun `if perf-2 is disabled but app start time span has started, appStartTimeSpanWithFallback returns the sdk init span instead`() { val appStartTimeSpan = AppStartMetrics.getInstance().appStartTimeSpan AppStartMetrics.getInstance().appStartType = AppStartMetrics.AppStartType.COLD - - appStartTimeSpan.start() + AppStartMetrics.getInstance().sdkInitTimeSpan.apply { + setStartedAt(123) + setStoppedAt(456) + } + appStartTimeSpan.setStartedAt(123) val options = SentryAndroidOptions().apply { isEnablePerformanceV2 = false @@ -107,9 +110,11 @@ class AppStartMetricsTest { @Test fun `if perf-2 is enabled but app start time span has not started, appStartTimeSpanWithFallback returns the sdk init span instead`() { - val appStartTimeSpan = AppStartMetrics.getInstance().appStartTimeSpan AppStartMetrics.getInstance().appStartType = AppStartMetrics.AppStartType.COLD - assertTrue(appStartTimeSpan.hasNotStarted()) + AppStartMetrics.getInstance().sdkInitTimeSpan.apply { + setStartedAt(123) + setStoppedAt(456) + } val options = SentryAndroidOptions().apply { isEnablePerformanceV2 = true @@ -287,18 +292,28 @@ class AppStartMetricsTest { } @Test - fun `restartAppStart set measurement flag and clear internal lists`() { + fun `a warm start gets reported after a cold start`() { val appStartMetrics = AppStartMetrics.getInstance() + + // when the first activity launches and gets destroyed + val activity0 = mock() + whenever(activity0.isChangingConfigurations).thenReturn(false) + appStartMetrics.onActivityCreated(activity0, null) + + // then the app start type should be cold and measurements should be sent + assertEquals(AppStartMetrics.AppStartType.COLD, appStartMetrics.appStartType) + assertTrue(appStartMetrics.shouldSendStartMeasurements()) + + // when the activity gets destroyed appStartMetrics.onAppStartSpansSent() - appStartMetrics.isAppLaunchedInForeground = false assertFalse(appStartMetrics.shouldSendStartMeasurements()) - appStartMetrics.restartAppStart(10) + appStartMetrics.onActivityDestroyed(activity0) + // then it should reset sending the measurements for the next warm activity + appStartMetrics.onActivityCreated(mock(), mock()) + assertEquals(AppStartMetrics.AppStartType.WARM, appStartMetrics.appStartType) assertTrue(appStartMetrics.shouldSendStartMeasurements()) - assertTrue(appStartMetrics.appStartTimeSpan.hasStarted()) - assertTrue(appStartMetrics.appStartTimeSpan.hasNotStopped()) - assertEquals(10, appStartMetrics.appStartTimeSpan.startUptimeMs) } @Test From 3aee7d970ac7b5b5f585e8bda5822df9538609b1 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 4 Mar 2025 20:40:24 +0100 Subject: [PATCH 11/15] Fix post-merge conflict --- .../android/core/ActivityLifecycleIntegration.java | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java index 05c08753d8..073b8780f0 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java @@ -558,18 +558,6 @@ public synchronized void onActivityDestroyed(final @NotNull Activity activity) { } } - private void clear() { - firstActivityCreated = false; - lastPausedTime = new SentryNanotimeDate(new Date(0), 0); - lastPausedUptimeMillis = 0; - activityLifecycleMap.clear(); - } - - if (activitiesWithOngoingTransactions.isEmpty() && !activity.isChangingConfigurations()) { - clear(); - } - } - private void clear() { firstActivityCreated = false; activityLifecycleMap.clear(); From 73fd1a145230a8b2223fded918234c2c9c0e61be Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Tue, 4 Mar 2025 19:42:34 +0000 Subject: [PATCH 12/15] Format code --- .../io/sentry/android/core/ActivityLifecycleIntegration.java | 1 + 1 file changed, 1 insertion(+) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java index 073b8780f0..5ddf706d16 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java @@ -562,6 +562,7 @@ private void clear() { firstActivityCreated = false; activityLifecycleMap.clear(); } + private void finishSpan(final @Nullable ISpan span) { if (span != null && !span.isFinished()) { span.finish(); From 84f56cd7a482815275bcf2fad93afab5e794a8a1 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Wed, 5 Mar 2025 14:57:32 +0100 Subject: [PATCH 13/15] Address PR feedback --- .../core/ActivityLifecycleIntegration.java | 1 + .../core/performance/AppStartMetrics.java | 6 +++--- .../core/performance/AppStartMetricsTest.kt | 21 +++++++++++++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java index 073b8780f0..5ddf706d16 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java @@ -562,6 +562,7 @@ private void clear() { firstActivityCreated = false; activityLifecycleMap.clear(); } + private void finishSpan(final @Nullable ISpan span) { if (span != null && !span.isFinished()) { span.finish(); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java b/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java index 95bfa1d5c7..41736ef709 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java @@ -306,18 +306,18 @@ private void checkCreateTimeOnMain() { public void onActivityCreated(@NonNull Activity activity, @Nullable Bundle savedInstanceState) { final long nowUptimeMs = SystemClock.uptimeMillis(); activeActivitiesCounter.incrementAndGet(); - appLaunchedInForeground = true; - // the first undrawn activity determines the app start type + // the first activity determines the app start type if (activeActivitiesCounter.get() == 1 && !firstDrawDone.get()) { // If the app (process) was launched more than 1 minute ago, it's likely wrong final long durationSinceAppStartMillis = nowUptimeMs - appStartSpan.getStartUptimeMs(); - if (durationSinceAppStartMillis > TimeUnit.MINUTES.toMillis(1)) { + if (!appLaunchedInForeground || durationSinceAppStartMillis > TimeUnit.MINUTES.toMillis(1)) { appStartType = AppStartType.WARM; } else { appStartType = savedInstanceState == null ? AppStartType.COLD : AppStartType.WARM; } } + appLaunchedInForeground = true; } @Override diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt index de8ab871ec..8238f659f7 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt @@ -149,6 +149,27 @@ class AppStartMetricsTest { assertFalse(timeSpan.hasStarted()) } + @Test + fun `if app is launched in background, the first created activity assumes a warm start`() { + val metrics = AppStartMetrics.getInstance() + metrics.appStartTimeSpan.start() + metrics.sdkInitTimeSpan.start() + metrics.registerLifecycleCallbacks(mock()) + + // when the handler callback is executed and no activity was launched + Shadows.shadowOf(Looper.getMainLooper()).idle() + + // isAppLaunchedInForeground should be false + assertFalse(metrics.isAppLaunchedInForeground) + + // but when the first activity launches + metrics.onActivityCreated(mock(), null) + + // then a warm start should be set + assertTrue(metrics.isAppLaunchedInForeground) + assertEquals(AppStartMetrics.AppStartType.WARM, metrics.appStartType) + } + @Test fun `if app start span is at most 1 minute, appStartTimeSpanWithFallback returns the app start span`() { val appStartTimeSpan = AppStartMetrics.getInstance().appStartTimeSpan From 8541dc0d3fd52883d310ae44dd12e87d5fc53b85 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Thu, 6 Mar 2025 06:58:03 +0100 Subject: [PATCH 14/15] Address PR feedback --- .../core/performance/AppStartMetrics.java | 7 ++++- .../core/performance/AppStartMetricsTest.kt | 26 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java b/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java index 41736ef709..5970fc0b84 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java @@ -305,7 +305,6 @@ private void checkCreateTimeOnMain() { @Override public void onActivityCreated(@NonNull Activity activity, @Nullable Bundle savedInstanceState) { final long nowUptimeMs = SystemClock.uptimeMillis(); - activeActivitiesCounter.incrementAndGet(); // the first activity determines the app start type if (activeActivitiesCounter.get() == 1 && !firstDrawDone.get()) { @@ -313,6 +312,12 @@ public void onActivityCreated(@NonNull Activity activity, @Nullable Bundle saved final long durationSinceAppStartMillis = nowUptimeMs - appStartSpan.getStartUptimeMs(); if (!appLaunchedInForeground || durationSinceAppStartMillis > TimeUnit.MINUTES.toMillis(1)) { appStartType = AppStartType.WARM; + + shouldSendStartMeasurements = true; + appStartSpan.reset(); + appStartSpan.start(); + appStartSpan.setStartedAt(nowUptimeMs); + CLASS_LOADED_UPTIME_MS = nowUptimeMs; } else { appStartType = savedInstanceState == null ? AppStartType.COLD : AppStartType.WARM; } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt index 8238f659f7..d36ff01a5f 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt @@ -149,6 +149,32 @@ class AppStartMetricsTest { assertFalse(timeSpan.hasStarted()) } + @Test + fun `if app is launched in background, but an activity launches later, a new warm start is reported with correct timings`() { + val metrics = AppStartMetrics.getInstance() + metrics.registerLifecycleCallbacks(mock()) + + // when the looper runs + Shadows.shadowOf(Looper.getMainLooper()).idle() + + // but no activity creation happened + // then the app wasn't launched in foreground and nothing should be sent + assertFalse(metrics.isAppLaunchedInForeground) + assertFalse(metrics.shouldSendStartMeasurements()) + + val now = TimeUnit.MINUTES.toMillis(2) + 1234567 + SystemClock.setCurrentTimeMillis(now) + + // once an activity launches + AppStartMetrics.getInstance().onActivityCreated(mock(), null) + + // then it should restart the timespan + assertTrue(metrics.isAppLaunchedInForeground) + assertTrue(metrics.shouldSendStartMeasurements()) + assertTrue(metrics.appStartTimeSpan.hasStarted()) + assertEquals(now, metrics.appStartTimeSpan.startUptimeMs) + } + @Test fun `if app is launched in background, the first created activity assumes a warm start`() { val metrics = AppStartMetrics.getInstance() From 441675cfb70d00ce5bb145621b179a00f98d8ecd Mon Sep 17 00:00:00 2001 From: Stefano Date: Thu, 6 Mar 2025 13:13:15 +0100 Subject: [PATCH 15/15] Update sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java --- .../io/sentry/android/core/performance/AppStartMetrics.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java b/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java index 5970fc0b84..20a8558486 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java @@ -307,7 +307,7 @@ public void onActivityCreated(@NonNull Activity activity, @Nullable Bundle saved final long nowUptimeMs = SystemClock.uptimeMillis(); // the first activity determines the app start type - if (activeActivitiesCounter.get() == 1 && !firstDrawDone.get()) { + if (activeActivitiesCounter.incrementAndGet() == 1 && !firstDrawDone.get()) { // If the app (process) was launched more than 1 minute ago, it's likely wrong final long durationSinceAppStartMillis = nowUptimeMs - appStartSpan.getStartUptimeMs(); if (!appLaunchedInForeground || durationSinceAppStartMillis > TimeUnit.MINUTES.toMillis(1)) {