Skip to content

Commit 13f4afd

Browse files
authored
Rename less good version of render span for clarity (#2033)
## Goal Rename app startup render child span that tracks less. An alternative implementation is adding a new event to be tracked, and keying off that to create the span. I don't like that because a new event adds little value for android versions that can do frame commit callbacks, and it adds complication to the code. Basically using one or the other and calling both of them render removes that distinction, so we just have to make sure the name is correct. I prefer this.
1 parent 2897b1a commit 13f4afd

3 files changed

Lines changed: 25 additions & 8 deletions

File tree

embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/capture/startup/AppStartupTraceEmitter.kt

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import io.embrace.android.embracesdk.internal.session.lifecycle.ProcessStateList
1111
import io.embrace.android.embracesdk.internal.spans.PersistableEmbraceSpan
1212
import io.embrace.android.embracesdk.internal.spans.SpanService
1313
import io.embrace.android.embracesdk.internal.ui.hasRenderEvent
14+
import io.embrace.android.embracesdk.internal.ui.supportFrameCommitCallback
1415
import io.embrace.android.embracesdk.internal.utils.Provider
1516
import io.embrace.android.embracesdk.internal.utils.VersionChecker
1617
import io.embrace.android.embracesdk.spans.EmbraceSpan
@@ -55,6 +56,7 @@ internal class AppStartupTraceEmitter(
5556
private val additionalTrackedIntervals = ConcurrentLinkedQueue<TrackedInterval>()
5657
private val customAttributes: MutableMap<String, String> = ConcurrentHashMap()
5758
private val trackRender = hasRenderEvent(versionChecker)
59+
private val trackFrameCommit = supportFrameCommitCallback(versionChecker)
5860
private val appStartupRootSpan = AtomicReference<PersistableEmbraceSpan?>(null)
5961
private val dataCollectionComplete = AtomicBoolean(false)
6062
private val traceEnd = if (manualEnd) {
@@ -365,7 +367,11 @@ internal class AppStartupTraceEmitter(
365367

366368
if (activityInitEndMs != null && uiLoadedMs != null) {
367369
val uiLoadSpanName = if (trackRender) {
368-
ACTIVITY_RENDER_SPAN
370+
if (trackFrameCommit) {
371+
ACTIVITY_RENDER_SPAN
372+
} else {
373+
ACTIVITY_FIRST_DRAW_SPAN
374+
}
369375
} else {
370376
ACTIVITY_LOAD_SPAN
371377
}
@@ -436,6 +442,7 @@ internal class AppStartupTraceEmitter(
436442
const val ACTIVITY_INIT_DELAY_SPAN = "activity-init-delay"
437443
const val ACTIVITY_INIT_SPAN = "activity-init"
438444
const val ACTIVITY_RENDER_SPAN = "activity-render"
445+
const val ACTIVITY_FIRST_DRAW_SPAN = "activity-first-draw"
439446
const val ACTIVITY_LOAD_SPAN = "activity-load"
440447
const val APP_READY_SPAN = "app-ready"
441448

embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/ui/DrawEventEmitter.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,5 +40,5 @@ fun createDrawEventEmitter(
4040

4141
fun hasRenderEvent(versionChecker: VersionChecker) = versionChecker.isAtLeast(VERSION_CODES.M)
4242

43-
private fun supportFrameCommitCallback(versionChecker: VersionChecker) = versionChecker.isAtLeast(VERSION_CODES.Q) &&
43+
fun supportFrameCommitCallback(versionChecker: VersionChecker) = versionChecker.isAtLeast(VERSION_CODES.Q) &&
4444
(Build.VERSION.SDK_INT != VERSION_CODES.S && Build.VERSION.SDK_INT != VERSION_CODES.S_V2)

embrace-android-features/src/test/java/io/embrace/android/embracesdk/internal/capture/startup/AppStartupTraceEmitterTest.kt

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import io.embrace.android.embracesdk.fakes.FakeEmbLogger
1010
import io.embrace.android.embracesdk.fakes.injection.FakeInitModule
1111
import io.embrace.android.embracesdk.internal.arch.schema.PrivateSpan
1212
import io.embrace.android.embracesdk.internal.capture.activity.hasPrePostEvents
13+
import io.embrace.android.embracesdk.internal.capture.startup.AppStartupTraceEmitter.Companion.ACTIVITY_FIRST_DRAW_SPAN
1314
import io.embrace.android.embracesdk.internal.capture.startup.AppStartupTraceEmitter.Companion.ACTIVITY_INIT_DELAY_SPAN
1415
import io.embrace.android.embracesdk.internal.capture.startup.AppStartupTraceEmitter.Companion.ACTIVITY_INIT_SPAN
1516
import io.embrace.android.embracesdk.internal.capture.startup.AppStartupTraceEmitter.Companion.ACTIVITY_LOAD_SPAN
@@ -27,6 +28,7 @@ import io.embrace.android.embracesdk.internal.spans.SpanService
2728
import io.embrace.android.embracesdk.internal.spans.SpanSink
2829
import io.embrace.android.embracesdk.internal.spans.findAttributeValue
2930
import io.embrace.android.embracesdk.internal.ui.hasRenderEvent
31+
import io.embrace.android.embracesdk.internal.ui.supportFrameCommitCallback
3032
import io.embrace.android.embracesdk.internal.utils.BuildVersionChecker
3133
import io.embrace.android.embracesdk.spans.ErrorCode
3234
import io.opentelemetry.sdk.common.Clock
@@ -55,6 +57,7 @@ internal class AppStartupTraceEmitterTest {
5557
private var firePreAndPostCreate: Boolean = true
5658
private var trackProcessStart: Boolean = true
5759
private var hasRenderEvent = true
60+
private var hasFrameCommitEvent = true
5861

5962
private lateinit var clock: FakeClock
6063
private lateinit var otelClock: Clock
@@ -79,6 +82,7 @@ internal class AppStartupTraceEmitterTest {
7982
firePreAndPostCreate = hasPrePostEvents(BuildVersionChecker)
8083
trackProcessStart = BuildVersionChecker.isAtLeast(VERSION_CODES.N)
8184
hasRenderEvent = hasRenderEvent(BuildVersionChecker)
85+
hasFrameCommitEvent = supportFrameCommitCallback(BuildVersionChecker)
8286
}
8387

8488
@Config(sdk = [VERSION_CODES.TIRAMISU])
@@ -191,7 +195,7 @@ internal class AppStartupTraceEmitterTest {
191195
assertNotNull(embraceInitSpan())
192196
assertNotNull(initGapSpan())
193197
assertNotNull(activityInitSpan())
194-
assertNull(firstFrameRenderSpan())
198+
assertNull(firstFrameRenderedSpan())
195199
}
196200
}
197201

@@ -219,7 +223,7 @@ internal class AppStartupTraceEmitterTest {
219223
assertNotNull(embraceInitSpan())
220224
assertNotNull(initGapSpan())
221225
assertNotNull(activityInitSpan())
222-
assertNotNull(firstFrameRenderSpan())
226+
assertNotNull(firstFrameRenderedSpan())
223227
assertNull(appReadySpan())
224228
}
225229
}
@@ -264,7 +268,7 @@ internal class AppStartupTraceEmitterTest {
264268
assertEquals(abandonTime, endTimeNanos.nanosToMillis())
265269
}
266270
assertNotNull(activityInitSpan())
267-
assertNotNull(firstFrameRenderSpan())
271+
assertNotNull(firstFrameRenderedSpan())
268272
assertNull(appReadySpan())
269273
}
270274
}
@@ -654,11 +658,16 @@ internal class AppStartupTraceEmitterTest {
654658
with(activityInitTimestamps) {
655659
assertChildSpan(spanMap.activityInitSpan(), startupActivityStart, startupActivityEnd)
656660
if (hasRenderEvent) {
657-
assertChildSpan(spanMap.firstFrameRenderSpan(), startupActivityEnd, uiLoadEnd)
661+
val renderSpan = if (hasFrameCommitEvent) {
662+
spanMap.firstFrameRenderedSpan()
663+
} else {
664+
spanMap.firstFrameDrawSpan()
665+
}
666+
assertChildSpan(renderSpan, startupActivityEnd, uiLoadEnd)
658667
assertNull(spanMap.activityResumeSpan())
659668
} else {
660669
assertChildSpan(spanMap.activityResumeSpan(), startupActivityEnd, uiLoadEnd)
661-
assertNull(spanMap.firstFrameRenderSpan())
670+
assertNull(spanMap.firstFrameRenderedSpan())
662671
}
663672

664673
if (manualEnd) {
@@ -810,7 +819,8 @@ internal class AppStartupTraceEmitterTest {
810819
private fun Map<String, EmbraceSpanData?>.embraceInitSpan() = this["emb-${EMBRACE_INIT_SPAN}"]
811820
private fun Map<String, EmbraceSpanData?>.initGapSpan() = this["emb-${ACTIVITY_INIT_DELAY_SPAN}"]
812821
private fun Map<String, EmbraceSpanData?>.activityInitSpan() = this["emb-${ACTIVITY_INIT_SPAN}"]
813-
private fun Map<String, EmbraceSpanData?>.firstFrameRenderSpan() = this["emb-${ACTIVITY_RENDER_SPAN}"]
822+
private fun Map<String, EmbraceSpanData?>.firstFrameRenderedSpan() = this["emb-${ACTIVITY_RENDER_SPAN}"]
823+
private fun Map<String, EmbraceSpanData?>.firstFrameDrawSpan() = this["emb-${ACTIVITY_FIRST_DRAW_SPAN}"]
814824
private fun Map<String, EmbraceSpanData?>.activityResumeSpan() = this["emb-${ACTIVITY_LOAD_SPAN}"]
815825
private fun Map<String, EmbraceSpanData?>.appReadySpan() = this["emb-${APP_READY_SPAN}"]
816826
private fun Map<String, EmbraceSpanData?>.customSpan() = this["custom-span"]

0 commit comments

Comments
 (0)