-
Notifications
You must be signed in to change notification settings - Fork 104
Refactor jank to use events #1175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
LikeTheSalad
merged 18 commits into
open-telemetry:main
from
breedx-splk:refactor_jank_for_events
Aug 28, 2025
Merged
Changes from 14 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
7869654
initial refactor of slow renders, extract PerActivityListener and add…
breedx-splk 510c5f8
add test
breedx-splk 06669d8
add test
breedx-splk 8d21dc7
Rename .java to .kt
breedx-splk 15945bc
convert to kotlin
breedx-splk 87e2b89
change signature to reuse same histogram, because reset clears it ato…
breedx-splk 171f980
begin EventsJankReporter
breedx-splk 752793b
add tests
breedx-splk 0be4373
spotless
breedx-splk 0657bab
wire up jank events and allow enabling legacy/deprecated zero duratio…
breedx-splk bf3977d
update detekt baseline
breedx-splk dd6219e
fix api
breedx-splk 961512b
fix test
breedx-splk 32a2d8f
update api
breedx-splk e1e95cc
change to singular to match SpanBasedJankReporter
breedx-splk e07eb66
use map instead of SparseIntArray
breedx-splk 6fa166f
only log if verbose debug is enabled.
breedx-splk 5826afc
add new api
breedx-splk File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| <?xml version="1.0" ?> | ||
| <SmellBaseline> | ||
| <ManuallySuppressedIssues/> | ||
| <CurrentIssues> | ||
| <ID>EmptyFunctionBlock:JankReporterTest.kt$JankReporterTest.<no name provided>${ }</ID> | ||
| <ID>ForbiddenComment:EventsJankReporter.kt$// TODO: Replace with semconv constants</ID> | ||
| <ID>MagicNumber:EventsJankReporter.kt$EventsJankReporter$1000.0</ID> | ||
| <ID>MagicNumber:SlowRenderingInstrumentation.kt$SlowRenderingInstrumentation$1000.0</ID> | ||
| <ID>TooGenericExceptionCaught:SlowRenderListener.kt$SlowRenderListener$e: Exception</ID> | ||
| </CurrentIssues> | ||
| </SmellBaseline> |
58 changes: 58 additions & 0 deletions
58
...rc/main/java/io/opentelemetry/android/instrumentation/slowrendering/EventsJankReporter.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| /* | ||
| * Copyright The OpenTelemetry Authors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package io.opentelemetry.android.instrumentation.slowrendering | ||
|
|
||
| import android.util.Log | ||
| import android.util.SparseIntArray | ||
| import io.opentelemetry.android.common.RumConstants | ||
| import io.opentelemetry.api.common.AttributeKey | ||
| import io.opentelemetry.api.common.Attributes | ||
| import io.opentelemetry.api.incubator.logs.ExtendedLogRecordBuilder | ||
| import io.opentelemetry.api.logs.Logger | ||
|
|
||
| // TODO: Replace with semconv constants | ||
| internal val FRAME_COUNT: AttributeKey<Long> = AttributeKey.longKey("app.jank.frame_count") | ||
| internal val PERIOD: AttributeKey<Double> = AttributeKey.doubleKey("app.jank.period") | ||
| internal val THRESHOLD: AttributeKey<Double> = AttributeKey.doubleKey("app.jank.threshold") | ||
|
|
||
| internal class EventsJankReporter( | ||
| private val eventLogger: Logger, | ||
| private val threshold: Double, | ||
| ) : JankReporter { | ||
| override fun reportSlow( | ||
| durationToCountHistogram: SparseIntArray, | ||
| periodSeconds: Double, | ||
| activityName: String, | ||
| ) { | ||
| var frameCount: Long = 0 | ||
| for (i in 0 until durationToCountHistogram.size()) { | ||
| val durationMillis = durationToCountHistogram.keyAt(i) | ||
| if ((durationMillis / 1000.0) > threshold) { | ||
| val count = durationToCountHistogram.get(durationMillis) | ||
| Log.d( | ||
|
breedx-splk marked this conversation as resolved.
Outdated
|
||
| RumConstants.OTEL_RUM_LOG_TAG, | ||
| "* Slow render detected: $durationMillis ms. $count times", | ||
| ) | ||
| frameCount += count | ||
| } | ||
| } | ||
|
|
||
| if (frameCount > 0) { | ||
| val eventBuilder = eventLogger.logRecordBuilder() as ExtendedLogRecordBuilder | ||
| val attributes = | ||
| Attributes | ||
| .builder() | ||
| .put(FRAME_COUNT, frameCount) | ||
| .put(PERIOD, periodSeconds) | ||
| .put(THRESHOLD, threshold) | ||
| .build() | ||
| eventBuilder | ||
| .setEventName("app.jank") | ||
| .setAllAttributes(attributes) | ||
| .emit() | ||
| } | ||
| } | ||
| } | ||
39 changes: 39 additions & 0 deletions
39
...ring/src/main/java/io/opentelemetry/android/instrumentation/slowrendering/JankReporter.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| /* | ||
| * Copyright The OpenTelemetry Authors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package io.opentelemetry.android.instrumentation.slowrendering | ||
|
|
||
| import android.util.SparseIntArray | ||
|
|
||
| /** | ||
| * Responsible for sending telemetry. This is a temporary class that we can remove | ||
| * after the jank semconv becomes more stable. | ||
| */ | ||
| internal fun interface JankReporter { | ||
| fun reportSlow( | ||
| durationToCountHistogram: SparseIntArray, | ||
|
breedx-splk marked this conversation as resolved.
Outdated
|
||
| periodSeconds: Double, | ||
| activityName: String, | ||
| ) | ||
|
|
||
| /** | ||
| * Creates a combined JankReporter that will first report slow for this | ||
| * instance and then delegate to another JankReporter instance. | ||
| */ | ||
| fun combine(jankReporter: JankReporter): JankReporter { | ||
| require(jankReporter != this) { "cannot combine with self" } | ||
| val exec = this::reportSlow | ||
| return object : JankReporter { | ||
| override fun reportSlow( | ||
| durationToCountHistogram: SparseIntArray, | ||
| periodSeconds: Double, | ||
| activityName: String, | ||
| ) { | ||
| exec(durationToCountHistogram, periodSeconds, activityName) | ||
| jankReporter.reportSlow(durationToCountHistogram, periodSeconds, activityName) | ||
| } | ||
| } | ||
| } | ||
| } | ||
63 changes: 63 additions & 0 deletions
63
...c/main/java/io/opentelemetry/android/instrumentation/slowrendering/PerActivityListener.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| /* | ||
| * Copyright The OpenTelemetry Authors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package io.opentelemetry.android.instrumentation.slowrendering | ||
|
|
||
| import android.app.Activity | ||
| import android.os.Build | ||
| import android.util.SparseIntArray | ||
| import android.view.FrameMetrics | ||
| import android.view.Window | ||
| import android.view.Window.OnFrameMetricsAvailableListener | ||
| import androidx.annotation.GuardedBy | ||
| import androidx.annotation.RequiresApi | ||
| import java.util.concurrent.TimeUnit | ||
|
|
||
| private val NANOS_PER_MS = TimeUnit.MILLISECONDS.toNanos(1).toInt() | ||
|
|
||
| // rounding value adds half a millisecond, for rounding to nearest ms | ||
| private val NANOS_ROUNDING_VALUE: Int = NANOS_PER_MS / 2 | ||
|
|
||
| @RequiresApi(api = Build.VERSION_CODES.N) | ||
| internal class PerActivityListener( | ||
| private val activity: Activity, | ||
| ) : OnFrameMetricsAvailableListener { | ||
| private val lock = Any() | ||
|
|
||
| @GuardedBy("lock") | ||
| private var drawDurationHistogram = SparseIntArray() | ||
|
|
||
| fun getActivityName(): String = activity.componentName.flattenToShortString() | ||
|
|
||
| override fun onFrameMetricsAvailable( | ||
| window: Window?, | ||
| frameMetrics: FrameMetrics, | ||
| dropCountSinceLastInvocation: Int, | ||
| ) { | ||
| val firstDrawFrame = frameMetrics.getMetric(FrameMetrics.FIRST_DRAW_FRAME) | ||
| if (firstDrawFrame == 1L) { | ||
| return | ||
| } | ||
|
|
||
| val drawDurationsNs = frameMetrics.getMetric(FrameMetrics.DRAW_DURATION) | ||
| // ignore values < 0; something must have gone wrong | ||
| if (drawDurationsNs >= 0) { | ||
| synchronized(lock) { | ||
| // calculation copied from FrameMetricsAggregator | ||
| val durationMs = ((drawDurationsNs + NANOS_ROUNDING_VALUE) / NANOS_PER_MS).toInt() | ||
| val oldValue = drawDurationHistogram.get(durationMs) | ||
| drawDurationHistogram.put(durationMs, (oldValue + 1)) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fun resetMetrics(): SparseIntArray { | ||
| synchronized(lock) { | ||
| val metrics = drawDurationHistogram | ||
| drawDurationHistogram = SparseIntArray() | ||
| return metrics | ||
| } | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.