Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -272,15 +272,30 @@ internal class AppStartupTraceEmitter(
val startupService = startupServiceProvider() ?: return
val sdkInitEndMs = startupService.getSdkInitEndMs()
if (sdkInitEndMs != null) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main change. Everything else is renaming the callback method to be more appropriate

appStartupRootSpan.get()?.let { startupTrace ->
recordTtid(
val startupTrace = appStartupRootSpan.get()
if (startupTrace != null) {
val uiLoadedMs = if (trackRender) {
firstFrameRenderedMs
} else {
startupActivityResumedMs
}
val activityInitStartMs = cappedBy(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really to deal with the activityInit end event being missing, which is fired on onStarted() fires.

I am not sure if the init start event will ever be missing - for the Activity onCreate() callback to be processed after the draw, but I am handling this case just in case. After this is released, we can detect whether we have zero-duration child spans for activity init to see if it actually happens.

value = startupActivityPreCreatedMs ?: startupActivityInitStartMs,
ceiling = uiLoadedMs
)
val activityInitEndMs = cappedBy(
value = startupActivityInitEndMs,
ceiling = uiLoadedMs
)

recordTrace(
applicationInitEndMs = if (recordColdStart) applicationInitEndMs else null,
sdkInitStartMs = if (recordColdStart) startupService.getSdkInitStartMs() else null,
sdkInitEndMs = if (recordColdStart) sdkInitEndMs else null,
firstActivityInitMs = firstActivityInitStartMs,
activityInitStartMs = startupActivityPreCreatedMs ?: startupActivityInitStartMs,
activityInitEndMs = startupActivityInitEndMs,
uiLoadedMs = if (trackRender) firstFrameRenderedMs else startupActivityResumedMs,
activityInitStartMs = activityInitStartMs,
activityInitEndMs = activityInitEndMs,
uiLoadedMs = uiLoadedMs,
traceEndTimeMs = traceEndTimeMs,
completed = completed,
)
Expand All @@ -289,6 +304,17 @@ internal class AppStartupTraceEmitter(
}
}

/**
* Limit a value based on some ceiling if it is defined.
* That is, return [ceiling] if it is non-null and less than [value].
*/
private fun cappedBy(value: Long?, ceiling: Long?) =
if (ceiling != null && (value == null || value > ceiling)) {
ceiling
} else {
value
}

private fun recordAdditionalIntervals(startupTrace: EmbraceSpan) {
do {
additionalTrackedIntervals.poll()?.let { trackedInterval ->
Expand All @@ -307,7 +333,7 @@ internal class AppStartupTraceEmitter(
}

@Suppress("CyclomaticComplexMethod", "ComplexMethod")
private fun recordTtid(
private fun recordTrace(
applicationInitEndMs: Long?,
sdkInitStartMs: Long?,
sdkInitEndMs: Long?,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ interface DrawEventEmitter {
fun registerFirstDrawCallback(
activity: Activity,
drawBeginCallback: () -> Unit,
firstFrameDeliveredCallback: () -> Unit
drawCompleteCallback: () -> Unit
)

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
override fun registerFirstDrawCallback(
activity: Activity,
drawBeginCallback: () -> Unit,
firstFrameDeliveredCallback: () -> Unit
drawCompleteCallback: () -> Unit
) {
val instanceId = traceInstanceId(activity)
if (!trackingLoad(instanceId)) {
Expand All @@ -41,8 +41,8 @@
decorView.onNextDraw {
if (!trackingLoad(instanceId)) {
drawBeginCallback()
loadingActivities[instanceId] = Runnable { firstFrameDeliveredCallback() }
decorView.viewTreeObserver.registerFrameCommitCallback(firstFrameDeliveredCallback)
loadingActivities[instanceId] = Runnable { drawCompleteCallback() }
decorView.viewTreeObserver.registerFrameCommitCallback(drawCompleteCallback)

Check warning on line 45 in embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/ui/FirstDrawDetector.kt

View check run for this annotation

Codecov / codecov/patch

embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/ui/FirstDrawDetector.kt#L44-L45

Added lines #L44 - L45 were not covered by tests
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ class HandlerMessageDrawDetector(
override fun registerFirstDrawCallback(
activity: Activity,
drawBeginCallback: () -> Unit,
firstFrameDeliveredCallback: () -> Unit,
drawCompleteCallback: () -> Unit,
) {
drawBeginCallback()
handler.sendMessageAtFrontOfQueue(
Message.obtain(handler.wrappedHandler, firstFrameDeliveredCallback).apply {
Message.obtain(handler.wrappedHandler, drawCompleteCallback).apply {
isAsynchronous = true
}
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,8 @@ internal class AppStartupTraceEmitterTest {
)
initActivity(
loadSplashScreen = false,
abortFirstLoad = false
abortFirstLoad = false,
activityInitOptions = ActivityInitOptions.NORMAL,
)
}
val abandonTime = clock.tick()
Expand Down Expand Up @@ -257,7 +258,8 @@ internal class AppStartupTraceEmitterTest {
)
initActivity(
loadSplashScreen = false,
abortFirstLoad = false
abortFirstLoad = false,
activityInitOptions = ActivityInitOptions.NORMAL
)
}
val abandonTime = clock.tick()
Expand Down Expand Up @@ -310,6 +312,30 @@ internal class AppStartupTraceEmitterTest {
createTraceEmitter(manualEnd = true).simulateAppStartup(manualEnd = true)
}

@Config(sdk = [VERSION_CODES.S])
@Test
fun `verify cold start trace with missing activity init events in S`() {
createTraceEmitter().simulateAppStartup(activityInitOptions = ActivityInitOptions.OMIT)
}

@Config(sdk = [VERSION_CODES.S])
@Test
fun `verify cold start trace with missing activity init end event in S`() {
createTraceEmitter().simulateAppStartup(activityInitOptions = ActivityInitOptions.OMIT_END)
}

@Config(sdk = [VERSION_CODES.S])
@Test
fun `verify cold start trace with delayed activity init events in S`() {
createTraceEmitter().simulateAppStartup(activityInitOptions = ActivityInitOptions.DELAY)
}

@Config(sdk = [VERSION_CODES.S])
@Test
fun `verify cold start trace with delayed activity init end event in S`() {
createTraceEmitter().simulateAppStartup(activityInitOptions = ActivityInitOptions.DELAY_END)
}

@Config(sdk = [VERSION_CODES.S])
@Test
fun `verify warm start trace without application init start and end triggered in S`() {
Expand Down Expand Up @@ -461,6 +487,30 @@ internal class AppStartupTraceEmitterTest {
)
}

@Config(sdk = [VERSION_CODES.M])
@Test
fun `verify cold start trace with missing activity init events in M`() {
createTraceEmitter().simulateAppStartup(activityInitOptions = ActivityInitOptions.OMIT)
}

@Config(sdk = [VERSION_CODES.M])
@Test
fun `verify cold start trace with missing activity init end event in M`() {
createTraceEmitter().simulateAppStartup(activityInitOptions = ActivityInitOptions.OMIT_END)
}

@Config(sdk = [VERSION_CODES.M])
@Test
fun `verify cold start trace with delayed activity init events in M`() {
createTraceEmitter().simulateAppStartup(activityInitOptions = ActivityInitOptions.DELAY)
}

@Config(sdk = [VERSION_CODES.M])
@Test
fun `verify cold start trace with delayed activity init end event in M`() {
createTraceEmitter().simulateAppStartup(activityInitOptions = ActivityInitOptions.DELAY_END)
}

@Config(sdk = [VERSION_CODES.M])
@Test
fun `verify warm start trace without application init start and end triggered in M`() {
Expand Down Expand Up @@ -570,6 +620,7 @@ internal class AppStartupTraceEmitterTest {
manualEnd: Boolean = false,
abortFirstActivityLoad: Boolean = false,
loadSplashScreen: Boolean = false,
activityInitOptions: ActivityInitOptions = ActivityInitOptions.NORMAL,
) {
val appInitTimestamps = initApp(
hasAppInitEvents = hasAppInitEvents,
Expand All @@ -583,7 +634,8 @@ internal class AppStartupTraceEmitterTest {

val activityInitTimestamps = initActivity(
loadSplashScreen = loadSplashScreen,
abortFirstLoad = abortFirstActivityLoad
abortFirstLoad = abortFirstActivityLoad,
activityInitOptions = activityInitOptions
)

val traceStart = if (isColdStart) {
Expand Down Expand Up @@ -737,37 +789,72 @@ internal class AppStartupTraceEmitterTest {
private fun AppStartupTraceEmitter.initActivity(
loadSplashScreen: Boolean,
abortFirstLoad: Boolean,
activityInitOptions: ActivityInitOptions,
): ActivityInitTimestamps {
val activityInitTimestamps = ActivityInitTimestamps()
with(activityInitTimestamps) {
firstActivityInit = preActivityInit(loadSplashScreen)
startupActivityStart = createActivity()

clock.tick(180)

if (abortFirstLoad) {
startupActivityStart = createActivity()
if (activityInitOptions.startTime == TimestampOption.NORMAL) {
fireStartEvent(activityInitTimestamps = this, abortFirstLoad = abortFirstLoad)
}

if (firePreAndPostCreate) {
startupActivityPostCreated()
clock.tick()
if (activityInitOptions.endTime == TimestampOption.NORMAL) {
fireEndEvent(activityInitTimestamps = this)
}
startupActivityInitEnd()
startupActivityEnd = clock.now()
clock.tick(15L)

startupActivityResumed(STARTUP_ACTIVITY_NAME)
if (hasRenderEvent) {
clock.tick(199L)
firstFrameRendered(STARTUP_ACTIVITY_NAME)
}

uiLoadEnd = clock.now()

if (activityInitOptions.startTime == TimestampOption.DELAY) {
fireStartEvent(activityInitTimestamps = this, abortFirstLoad = abortFirstLoad)
}

if (activityInitOptions.endTime == TimestampOption.DELAY) {
fireEndEvent(activityInitTimestamps = this)
}

if (activityInitOptions.startTime != TimestampOption.NORMAL) {
startupActivityStart = uiLoadEnd
}

if (activityInitOptions.endTime != TimestampOption.NORMAL) {
startupActivityEnd = uiLoadEnd
}
}
return activityInitTimestamps
}

private fun AppStartupTraceEmitter.fireStartEvent(
activityInitTimestamps: ActivityInitTimestamps,
abortFirstLoad: Boolean,
) {
activityInitTimestamps.startupActivityStart = createActivity()
clock.tick(180)

if (abortFirstLoad) {
activityInitTimestamps.startupActivityStart = createActivity()
}

if (firePreAndPostCreate) {
startupActivityPostCreated()
clock.tick()
}
}

private fun AppStartupTraceEmitter.fireEndEvent(
activityInitTimestamps: ActivityInitTimestamps
) {
startupActivityInitEnd()
activityInitTimestamps.startupActivityEnd = clock.now()
clock.tick(15L)
startupActivityResumed(STARTUP_ACTIVITY_NAME)
}

private fun AppStartupTraceEmitter.createActivity(): Long {
val preCreateTimestamp = if (firePreAndPostCreate) {
clock.tick()
Expand Down Expand Up @@ -847,6 +934,23 @@ internal class AppStartupTraceEmitterTest {
var uiLoadEnd: Long? = null,
)

private enum class ActivityInitOptions(
val startTime: TimestampOption = TimestampOption.NORMAL,
val endTime: TimestampOption = TimestampOption.NORMAL,
) {
NORMAL,
OMIT(startTime = TimestampOption.MISSING, endTime = TimestampOption.MISSING),
OMIT_END(endTime = TimestampOption.MISSING),
DELAY(startTime = TimestampOption.DELAY, endTime = TimestampOption.DELAY),
DELAY_END(endTime = TimestampOption.DELAY)
}

private enum class TimestampOption {
NORMAL,
DELAY,
MISSING
}

companion object {
private const val STARTUP_ACTIVITY_NAME = "StartupActivity"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ internal class HandlerMessageDrawDetectorTest {
detector.registerFirstDrawCallback(
activity = Robolectric.buildActivity(Activity::class.java).get(),
drawBeginCallback = { beginCallbackInvoked = true },
firstFrameDeliveredCallback = { endCallbackInvoked = true }
drawCompleteCallback = { endCallbackInvoked = true }
)
assertTrue(beginCallbackInvoked)
with(handler.messageQueue.single()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ class FakeDrawEventEmitter : DrawEventEmitter {
override fun registerFirstDrawCallback(
activity: Activity,
drawBeginCallback: () -> Unit,
firstFrameDeliveredCallback: () -> Unit
drawCompleteCallback: () -> Unit
) {
registeredActivities[traceInstanceId(activity)] = drawBeginCallback to firstFrameDeliveredCallback
registeredActivities[traceInstanceId(activity)] = drawBeginCallback to drawCompleteCallback
lastRegisteredActivity = activity
lastFirstFrameDeliveredCallback = firstFrameDeliveredCallback
lastFirstFrameDeliveredCallback = drawCompleteCallback
}

override fun unregisterFirstDrawCallback(activity: Activity) {
Expand Down
Loading