Skip to content

Commit

Permalink
Record internal error when startup trace fails to be recorded
Browse files Browse the repository at this point in the history
  • Loading branch information
bidetofevil committed Nov 29, 2024
1 parent 8e2cb02 commit 2777cb0
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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?) {
Expand Down Expand Up @@ -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()) {
Expand All @@ -175,6 +176,7 @@ internal class AppStartupTraceEmitter(
}
}
callback?.invoke()
dataCollectionComplete.set(true)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand All @@ -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) {
Expand All @@ -66,7 +68,8 @@ class StartupTracker(
isFirstDraw = true
val callback = {
appStartupDataCollector.firstFrameRendered(
activityName = activityName
activityName = activityName,
collectionCompleteCallback = { startupComplete(application) }

Check warning on line 72 in embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/capture/startup/StartupTracker.kt

View check run for this annotation

Codecov / codecov/patch

embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/capture/startup/StartupTracker.kt#L71-L72

Added lines #L71 - L72 were not covered by tests
)
}
decorView.viewTreeObserver.registerFrameCommitCallback(callback)
Expand Down Expand Up @@ -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) }
)
}
}
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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])
Expand All @@ -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])
Expand Down Expand Up @@ -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])
Expand All @@ -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])
Expand All @@ -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])
Expand All @@ -211,7 +219,7 @@ internal class AppStartupTraceEmitterTest {
}

private fun dataCollectionCompletedCallback() {
dataCollectionCompletedCallbackInvoked = true
dataCollectionCompletedCallbackInvokedCount++
}

private fun verifyColdStartWithRender(processCreateDelayMs: Long? = null) {
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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`() {
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class FakeAppStartupDataCollector(
) {
startupActivityName = activityName
startupActivityResumedMs = timestampMs ?: clock.now()
collectionCompleteCallback?.invoke()
}

override fun firstFrameRendered(
Expand All @@ -55,6 +56,7 @@ class FakeAppStartupDataCollector(
) {
startupActivityName = activityName
firstFrameRenderedMs = timestampMs ?: clock.now()
collectionCompleteCallback?.invoke()
}

override fun addTrackedInterval(name: String, startTimeMs: Long, endTimeMs: Long) {
Expand Down

0 comments on commit 2777cb0

Please sign in to comment.