From 2777cb06e343a9112535d11c8072c4ac0dd51677 Mon Sep 17 00:00:00 2001 From: bidetofevil Date: Tue, 10 Sep 2024 15:57:07 -0700 Subject: [PATCH] Record internal error when startup trace fails to be recorded --- .../capture/startup/AppStartupTraceEmitter.kt | 8 +++-- .../capture/startup/StartupTracker.kt | 16 +++++++-- .../startup/AppStartupTraceEmitterTest.kt | 26 +++++++++----- .../capture/startup/StartupTrackerTest.kt | 35 ++++++------------- .../fakes/FakeAppStartupDataCollector.kt | 2 ++ 5 files changed, 49 insertions(+), 38 deletions(-) diff --git a/embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/capture/startup/AppStartupTraceEmitter.kt b/embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/capture/startup/AppStartupTraceEmitter.kt index bc691bab2..b487b3e90 100644 --- a/embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/capture/startup/AppStartupTraceEmitter.kt +++ b/embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/capture/startup/AppStartupTraceEmitter.kt @@ -98,6 +98,7 @@ internal class AppStartupTraceEmitter( private var sdkInitEndedInForeground: Boolean? = null private val startupRecorded = AtomicBoolean(false) + private val dataCollectionComplete = AtomicBoolean(false) private val endWithFrameDraw: Boolean = versionChecker.isAtLeast(VERSION_CODES.Q) override fun applicationInitStart(timestampMs: Long?) { @@ -162,9 +163,9 @@ internal class AppStartupTraceEmitter( * Called when app startup is considered complete, i.e. the data can be used and any additional updates can be ignored */ private fun dataCollectionComplete(callback: (() -> Unit)?) { - if (!startupRecorded.get()) { - synchronized(startupRecorded) { - if (!startupRecorded.get()) { + if (!dataCollectionComplete.get()) { + synchronized(dataCollectionComplete) { + if (!dataCollectionComplete.get()) { backgroundWorker.submit { recordStartup() if (!startupRecorded.get()) { @@ -175,6 +176,7 @@ internal class AppStartupTraceEmitter( } } callback?.invoke() + dataCollectionComplete.set(true) } } } diff --git a/embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/capture/startup/StartupTracker.kt b/embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/capture/startup/StartupTracker.kt index 8d387820d..7afaae624 100644 --- a/embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/capture/startup/StartupTracker.kt +++ b/embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/capture/startup/StartupTracker.kt @@ -44,6 +44,7 @@ class StartupTracker( private var isFirstDraw = false private var nullWindowCallbackErrorLogged = false private var startupActivityId: Int? = null + private var startupDataCollectionComplete = false override fun onActivityPreCreated(activity: Activity, savedInstanceState: Bundle?) { if (activity.useAsStartupActivity()) { @@ -54,6 +55,7 @@ class StartupTracker( override fun onActivityCreated(activity: Activity, savedInstanceState: Bundle?) { if (activity.useAsStartupActivity()) { val activityName = activity.localClassName + val application = activity.application appStartupDataCollector.startupActivityInitStart() if (versionChecker.isAtLeast(Build.VERSION_CODES.Q)) { if (!isFirstDraw) { @@ -66,7 +68,8 @@ class StartupTracker( isFirstDraw = true val callback = { appStartupDataCollector.firstFrameRendered( - activityName = activityName + activityName = activityName, + collectionCompleteCallback = { startupComplete(application) } ) } decorView.viewTreeObserver.registerFrameCommitCallback(callback) @@ -101,8 +104,10 @@ class StartupTracker( override fun onActivityResumed(activity: Activity) { if (activity.observeForStartup()) { + val application = activity.application appStartupDataCollector.startupActivityResumed( - activityName = activity.localClassName + activityName = activity.localClassName, + collectionCompleteCallback = { startupComplete(application) } ) } } @@ -115,6 +120,13 @@ class StartupTracker( override fun onActivityDestroyed(activity: Activity) {} + private fun startupComplete(application: Application) { + if (!startupDataCollectionComplete) { + application.unregisterActivityLifecycleCallbacks(this) + startupDataCollectionComplete = true + } + } + /** * Returns true if the Activity instance is being used as the startup Activity. It will return false if [useAsStartupActivity] has * not been called previously to setup the Activity instance to be used as the startup Activity. diff --git a/embrace-android-features/src/test/java/io/embrace/android/embracesdk/internal/capture/startup/AppStartupTraceEmitterTest.kt b/embrace-android-features/src/test/java/io/embrace/android/embracesdk/internal/capture/startup/AppStartupTraceEmitterTest.kt index c21db2153..c0bd0e2d3 100644 --- a/embrace-android-features/src/test/java/io/embrace/android/embracesdk/internal/capture/startup/AppStartupTraceEmitterTest.kt +++ b/embrace-android-features/src/test/java/io/embrace/android/embracesdk/internal/capture/startup/AppStartupTraceEmitterTest.kt @@ -20,7 +20,6 @@ import io.embrace.android.embracesdk.internal.spans.findAttributeValue import io.embrace.android.embracesdk.internal.utils.BuildVersionChecker import io.embrace.android.embracesdk.internal.worker.BackgroundWorker import org.junit.Assert.assertEquals -import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -37,7 +36,7 @@ import org.robolectric.annotation.Config @RunWith(AndroidJUnit4::class) internal class AppStartupTraceEmitterTest { private var startupService: StartupService? = null - private var dataCollectionCompletedCallbackInvoked: Boolean = false + private var dataCollectionCompletedCallbackInvokedCount = 0 private lateinit var clock: FakeClock private lateinit var spanSink: SpanSink private lateinit var spanService: SpanService @@ -73,7 +72,7 @@ internal class AppStartupTraceEmitterTest { fun `no crashes if startup service not available in T`() { startupService = null appStartupTraceEmitter.firstFrameRendered(STARTUP_ACTIVITY_NAME, ::dataCollectionCompletedCallback) - assertTrue(dataCollectionCompletedCallbackInvoked) + assertEquals(1, dataCollectionCompletedCallbackInvokedCount) } @Config(sdk = [Build.VERSION_CODES.TIRAMISU]) @@ -100,12 +99,21 @@ internal class AppStartupTraceEmitterTest { verifyWarmStartWithRenderWithoutAppInitEvents(processCreateDelayMs = 0L) } + @Config(sdk = [Build.VERSION_CODES.TIRAMISU]) + @Test + fun `trace end callback will not be invoked twice`() { + startupService = null + appStartupTraceEmitter.firstFrameRendered(STARTUP_ACTIVITY_NAME, ::dataCollectionCompletedCallback) + appStartupTraceEmitter.firstFrameRendered(STARTUP_ACTIVITY_NAME, ::dataCollectionCompletedCallback) + assertEquals(1, dataCollectionCompletedCallbackInvokedCount) + } + @Config(sdk = [Build.VERSION_CODES.S]) @Test fun `no crashes if startup service not available in S`() { startupService = null appStartupTraceEmitter.firstFrameRendered(STARTUP_ACTIVITY_NAME, ::dataCollectionCompletedCallback) - assertTrue(dataCollectionCompletedCallbackInvoked) + assertEquals(1, dataCollectionCompletedCallbackInvokedCount) } @Config(sdk = [Build.VERSION_CODES.S]) @@ -137,7 +145,7 @@ internal class AppStartupTraceEmitterTest { fun `no crashes if startup service not available in P`() { startupService = null appStartupTraceEmitter.startupActivityResumed(STARTUP_ACTIVITY_NAME, ::dataCollectionCompletedCallback) - assertTrue(dataCollectionCompletedCallbackInvoked) + assertEquals(1, dataCollectionCompletedCallbackInvokedCount) } @Config(sdk = [Build.VERSION_CODES.P]) @@ -163,7 +171,7 @@ internal class AppStartupTraceEmitterTest { fun `no crashes if startup service not available in M`() { startupService = null appStartupTraceEmitter.startupActivityResumed(STARTUP_ACTIVITY_NAME, ::dataCollectionCompletedCallback) - assertTrue(dataCollectionCompletedCallbackInvoked) + assertEquals(1, dataCollectionCompletedCallbackInvokedCount) } @Config(sdk = [Build.VERSION_CODES.M]) @@ -189,7 +197,7 @@ internal class AppStartupTraceEmitterTest { fun `no crashes if startup service not available in L`() { startupService = null appStartupTraceEmitter.startupActivityResumed(STARTUP_ACTIVITY_NAME, ::dataCollectionCompletedCallback) - assertTrue(dataCollectionCompletedCallbackInvoked) + assertEquals(1, dataCollectionCompletedCallbackInvokedCount) } @Config(sdk = [Build.VERSION_CODES.LOLLIPOP]) @@ -211,7 +219,7 @@ internal class AppStartupTraceEmitterTest { } private fun dataCollectionCompletedCallback() { - dataCollectionCompletedCallbackInvoked = true + dataCollectionCompletedCallbackInvokedCount++ } private fun verifyColdStartWithRender(processCreateDelayMs: Long? = null) { @@ -559,7 +567,7 @@ internal class AppStartupTraceEmitterTest { ) assertEquals("false", attrs.findAttributeValue("embrace-init-in-foreground")) assertEquals("main", attrs.findAttributeValue("embrace-init-thread-name")) - assertTrue(dataCollectionCompletedCallbackInvoked) + assertEquals(1, dataCollectionCompletedCallbackInvokedCount) } private fun assertChildSpan(span: EmbraceSpanData, expectedStartTimeNanos: Long, expectedEndTimeNanos: Long) { diff --git a/embrace-android-features/src/test/java/io/embrace/android/embracesdk/internal/capture/startup/StartupTrackerTest.kt b/embrace-android-features/src/test/java/io/embrace/android/embracesdk/internal/capture/startup/StartupTrackerTest.kt index a23f8967e..625139230 100644 --- a/embrace-android-features/src/test/java/io/embrace/android/embracesdk/internal/capture/startup/StartupTrackerTest.kt +++ b/embrace-android-features/src/test/java/io/embrace/android/embracesdk/internal/capture/startup/StartupTrackerTest.kt @@ -12,6 +12,7 @@ import io.embrace.android.embracesdk.fakes.FakeSplashScreenActivity import io.embrace.android.embracesdk.internal.logging.EmbLogger import io.embrace.android.embracesdk.internal.utils.BuildVersionChecker import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -97,30 +98,6 @@ internal class StartupTrackerTest { } } - @Config(sdk = [Build.VERSION_CODES.UPSIDE_DOWN_CAKE]) - @Test - fun `cold start with activity instance recreated`() { - defaultActivityController.create() - clock.tick() - val recreateTime = clock.now() - defaultActivityController.recreate() - clock.tick() - val startTime = clock.now() - defaultActivityController.start() - clock.tick() - val resumeTime = clock.now() - defaultActivityController.resume() - clock.tick() - - verifyTiming( - preCreateTime = recreateTime, - createTime = recreateTime, - postCreateTime = recreateTime, - startTime = startTime, - resumeTime = resumeTime - ) - } - @Config(sdk = [Build.VERSION_CODES.UPSIDE_DOWN_CAKE]) @Test fun `cold start with different activities being created and foregrounded first`() { @@ -155,6 +132,16 @@ internal class StartupTrackerTest { } } + @Config(sdk = [Build.VERSION_CODES.LOLLIPOP]) + @Test + fun `verify startup tracker detached after trace recorded`() { + val firstLaunchTimes = launchActivity() + assertEquals(firstLaunchTimes.createTime, dataCollector.startupActivityInitStartMs) + clock.tick(500_000) + val secondLaunchTimes = launchActivity() + assertNotEquals(secondLaunchTimes.createTime, dataCollector.startupActivityInitStartMs) + } + private fun launchActivity(controller: ActivityController<*> = defaultActivityController): ActivityTiming { val createTime = clock.now() controller.create() diff --git a/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeAppStartupDataCollector.kt b/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeAppStartupDataCollector.kt index 03037a54d..799e77eca 100644 --- a/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeAppStartupDataCollector.kt +++ b/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeAppStartupDataCollector.kt @@ -46,6 +46,7 @@ class FakeAppStartupDataCollector( ) { startupActivityName = activityName startupActivityResumedMs = timestampMs ?: clock.now() + collectionCompleteCallback?.invoke() } override fun firstFrameRendered( @@ -55,6 +56,7 @@ class FakeAppStartupDataCollector( ) { startupActivityName = activityName firstFrameRenderedMs = timestampMs ?: clock.now() + collectionCompleteCallback?.invoke() } override fun addTrackedInterval(name: String, startTimeMs: Long, endTimeMs: Long) {