diff --git a/CHANGELOG.md b/CHANGELOG.md index 0723a5ae0e..f450d56593 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - Prevent duplicated breadcrumbs on tombstone-merged native crash events ([#5888](https://github.com/getsentry/sentry-java/pull/5888)) - Prevent a class of Session Replay deadlocks by confining lifecycle state changes to Android's main thread ([#5965](https://github.com/getsentry/sentry-java/pull/5965)) - Symbolicate tombstone native frames for libraries loaded directly from APKs ([#5992](https://github.com/getsentry/sentry-java/pull/5992)) +- Report a consistent app start type across the app start measurement, `contexts.app` and the `app.start` span attributes ([#6006](https://github.com/getsentry/sentry-java/pull/6006)) ### Features diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java index 1340ca6fad..7f81ea1b73 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java @@ -86,6 +86,10 @@ public SentryEvent process(@NotNull SentryEvent event, @NotNull Hint hint) { } final @NotNull AppStartMetrics appStartMetrics = AppStartMetrics.getInstance(); + // The app start type can flip (e.g. the last activity being destroyed marks the next start + // as warm) while this runs on the timer thread that finished the transaction. Read it once so + // the measurement key, contexts.app and the span attributes below cannot disagree. + final @NotNull AppStartMetrics.AppStartType appStartType = appStartMetrics.getAppStartType(); // the app start measurement is only sent once and only if the transaction has // the app.start span, which is automatically created by the SDK. if (hasAppStartSpan(transaction)) { @@ -141,14 +145,14 @@ public SentryEvent process(@NotNull SentryEvent event, @NotNull Hint hint) { (float) appStartUpDurationMs, MeasurementUnit.Duration.MILLISECOND.apiName()); final String appStartKey = - appStartMetrics.getAppStartType() == AppStartMetrics.AppStartType.COLD + appStartType == AppStartMetrics.AppStartType.COLD ? MeasurementValue.KEY_APP_START_COLD : MeasurementValue.KEY_APP_START_WARM; transaction.getMeasurements().put(appStartKey, value); } - attachAppStartSpans(appStartMetrics, transaction); + attachAppStartSpans(appStartMetrics, appStartType, transaction); appStartMetrics.onAppStartSpansSent(); } } @@ -158,14 +162,12 @@ public SentryEvent process(@NotNull SentryEvent event, @NotNull Hint hint) { appContext = new App(); transaction.getContexts().setApp(appContext); } - final String appStartType = - appStartMetrics.getAppStartType() == AppStartMetrics.AppStartType.COLD - ? "cold" - : "warm"; - appContext.setStartType(appStartType); + final String appStartTypeName = + appStartType == AppStartMetrics.AppStartType.COLD ? "cold" : "warm"; + appContext.setStartType(appStartTypeName); if (isStandaloneAppStartTxn) { - setAppStartVitals(transaction, appStartType); + setAppStartVitals(transaction, appStartTypeName); } } @@ -293,10 +295,12 @@ private boolean hasAppStartSpan(final @NotNull SentryTransaction txn) { } private void attachAppStartSpans( - final @NotNull AppStartMetrics appStartMetrics, final @NotNull SentryTransaction txn) { + final @NotNull AppStartMetrics appStartMetrics, + final @NotNull AppStartMetrics.AppStartType appStartType, + final @NotNull SentryTransaction txn) { // We include process init, content providers and application.onCreate spans only on cold start - if (appStartMetrics.getAppStartType() != AppStartMetrics.AppStartType.COLD) { + if (appStartType != AppStartMetrics.AppStartType.COLD) { return; } 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 eb1a10dd64..d39b3ff5ac 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 @@ -68,7 +68,9 @@ public enum AppStartType { public static final @NotNull AutoClosableReentrantLock staticLock = new AutoClosableReentrantLock(); - private @NotNull AppStartType appStartType = AppStartType.UNKNOWN; + // Written on the main thread (activity lifecycle, ApplicationStartInfo) and read on whichever + // thread finishes an app start transaction, which can be the Sentry timer thread. + private volatile @NotNull AppStartType appStartType = AppStartType.UNKNOWN; private @Nullable volatile Boolean appLaunchedInForeground; private volatile long firstIdle = -1; 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 0f5c731aaf..0ab922fc32 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 @@ -205,6 +205,63 @@ class PerformanceAndroidEventProcessorTest { } } + @Test + fun `cold standalone app start reports the same type on measurement, contexts and spans`() { + val sut = fixture.getSut(enablePerformanceV2 = true) + AppStartMetrics.getInstance().apply { + appStartType = AppStartType.COLD + isAppLaunchedInForeground = true + classLoadedUptimeMs = 50 + appStartTimeSpan.apply { + setStartedAt(1) + setStoppedAt(100) + } + applicationOnCreateTimeSpan.apply { + setStartedAt(10) + description = "com.example.App.onCreate" + setStoppedAt(42) + } + } + + var tr = createStandaloneAppStartTransaction(appStartScreen = "Activity") + tr = sut.process(tr, Hint()) + + assertThat(tr.measurements).containsKey(MeasurementValue.KEY_APP_START_COLD) + assertThat(tr.measurements).doesNotContainKey(MeasurementValue.KEY_APP_START_WARM) + assertThat(tr.contexts.app!!.startType).isEqualTo("cold") + assertThat(tr.contexts.trace!!.data[APP_START_TYPE_DATA]).isEqualTo("cold") + // cold starts attach the process init / Application.onCreate breakdown children + assertThat(tr.spans).isNotEmpty() + for (span in tr.spans) { + assertThat(span.data?.get(APP_START_TYPE_DATA)).isEqualTo("cold") + } + } + + @Test + fun `warm standalone app start reports the same type on measurement, contexts and spans`() { + val sut = fixture.getSut(enablePerformanceV2 = true) + AppStartMetrics.getInstance().apply { + appStartType = AppStartType.WARM + isAppLaunchedInForeground = true + classLoadedUptimeMs = 50 + appStartTimeSpan.apply { + setStartedAt(1) + setStoppedAt(100) + } + } + + var tr = createStandaloneAppStartTransaction(appStartScreen = "Activity") + tr = sut.process(tr, Hint()) + + assertThat(tr.measurements).containsKey(MeasurementValue.KEY_APP_START_WARM) + assertThat(tr.measurements).doesNotContainKey(MeasurementValue.KEY_APP_START_COLD) + assertThat(tr.contexts.app!!.startType).isEqualTo("warm") + assertThat(tr.contexts.trace!!.data[APP_START_TYPE_DATA]).isEqualTo("warm") + for (span in tr.spans) { + assertThat(span.data?.get(APP_START_TYPE_DATA)).isEqualTo("warm") + } + } + @Test fun `standalone app start sets screen and type on user spans under the extended span`() { val sut = fixture.getSut()