feat: add render instrumentation #20
Conversation
MustafaHaddara
force-pushed
the
mh/render-instrumentation
branch
from
3c4751d to
2e11d22
Compare
| } | ||
|
|
||
| val LocalOtelComposition = compositionLocalOf<OpenTelemetryRum?> { null } |
There was a problem hiding this comment.
I saw that the CorePlayground accepts the OpenTelemetryRum object as a parameter but I didn't want to pass it through the many layers of nested Composables in the ViewInstrumentationPlayground, and I thought it was easier for HoneycombInstrumentedComposable to read from a context-like thing instead of accepting an OpenTelemetryRum object.
I'm open to push this back to a parameter if we think that's a better idea, though.
There was a problem hiding this comment.
I think this makes a lot of sense, and fits the criteria for when to use it as described on https://developer.android.com/develop/ui/compose/compositionlocal#intro
But this would effectively become part of our public API at that point, right? Our developers would have to use this line?
CompositionLocalProvider(LocalOtelComposition provides otelRum) {
If so, we should think carefully about how to name LocalOtelComposition.
There was a problem hiding this comment.
Yeah, that's true. IntelliJ reports that the convention here is that the key starts with the Local prefix:
The CompositionLocalProvider pattern reminds me of React Context. In that ecosystem, it's common for libraries to export their own Providers and consumers use that as an entry point. ex:
// MainActivity.kt
setContent {
HoneycombLocalProvider( /* honeycomb config */ ) {
// body content
}
}and
// HoneycombLocalProvider.kt
val LocalOpenTelemetryRum = compositionLocalOf<OpenTelemetryRum?> { null }
@Composable
fun HoneycombLocalProvder( /* config */ ) {
val options = HoneycombOptions.builder(this)
// config
.build()
otelRum = Honeycomb.configure(this, options)
CompositionLocalProvider(LocalOpenTelemetryRum provides otelRum) {
// child content
}
}
| PlaygroundTab.UI -> { | ||
| UIPlayground() | ||
| } |
There was a problem hiding this comment.
this was just to reorder the tabs to match the iOS smoke test app
| @Composable | ||
| private fun HoneycombInstrumentedComposable( | ||
| name: String, | ||
| composable: @Composable (() -> Unit), | ||
| ) { | ||
| val tracer = LocalOtelComposition.current!!.openTelemetry.tracerProvider.tracerBuilder("ViewInstrumentationPlayground").build() | ||
| val span = tracer.spanBuilder("Render").setAttribute("view.name", name).startSpan() | ||
| span.makeCurrent() | ||
|
|
||
| val duration = | ||
| measureTime { | ||
| composable() | ||
| } | ||
|
|
||
| // renderDuration is in seconds | ||
| // calling duration.inWholeSeconds would lose precision | ||
| span.setAttribute("view.renderDuration", duration.inWholeMicroseconds / 1_000_000.toDouble()) | ||
|
|
||
| span.end() | ||
| } |
There was a problem hiding this comment.
This lives here for now while I'm prototyping this approach/interface
Once we are happy with the general API, I can move it in the core library.
| @Composable | ||
| internal fun ViewInstrumentationPlayground() { | ||
| val (enabled, setEnabled) = remember { mutableStateOf(false) } | ||
|
|
||
| Column( | ||
| verticalArrangement = Arrangement.Center, | ||
| horizontalAlignment = Alignment.CenterHorizontally, | ||
| modifier = Modifier.fillMaxWidth(), | ||
| ) { | ||
| Row( | ||
| verticalAlignment = Alignment.CenterVertically, | ||
| horizontalArrangement = Arrangement.SpaceBetween, | ||
| modifier = Modifier.fillMaxWidth(), | ||
| ) { | ||
| Text(text = "enable slow render") | ||
| Switch(checked = enabled, onCheckedChange = setEnabled) | ||
| } | ||
| if (enabled) { | ||
| ExpensiveView() | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This structure mirrors the same playground view in iOS: https://github.com/honeycombio/honeycomb-opentelemetry-swift/pull/20/files#diff-3ed6499a52b381eba3646ace172cde5bfa2464d33360b412f51b80b35d120c2bR85-R100
Which problem is this PR solving?
Adds manual instrumentation for rendering times of Composables in Jetpack Compose
Short description of the changes
heavily inspired by honeycombio/honeycomb-opentelemetry-swift#20
How to verify that this has the expected result