-
-
Notifications
You must be signed in to change notification settings - Fork 468
feat(android): Add queryable getFramesDelay API to SentryFrameMetricsCollector #5248
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
base: main
Are you sure you want to change the base?
Changes from all commits
44e21df
3e247bb
9f94607
6b99b10
99acb6a
2ee6e8c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| package io.sentry.android.core; | ||
|
|
||
| import org.jetbrains.annotations.ApiStatus; | ||
|
|
||
| /** Result of querying frame delay for a given time range. */ | ||
| @ApiStatus.Internal | ||
| public final class SentryFramesDelayResult { | ||
|
|
||
| private final double delaySeconds; | ||
| private final int framesContributingToDelayCount; | ||
|
|
||
| public SentryFramesDelayResult( | ||
| final double delaySeconds, final int framesContributingToDelayCount) { | ||
| this.delaySeconds = delaySeconds; | ||
| this.framesContributingToDelayCount = framesContributingToDelayCount; | ||
| } | ||
|
|
||
| /** | ||
| * @return the total frame delay in seconds, or -1 if incalculable (e.g. no frame data available) | ||
| */ | ||
| public double getDelaySeconds() { | ||
| return delaySeconds; | ||
| } | ||
|
|
||
| /** | ||
| * @return the number of frames that contributed to the delay (slow + frozen frames) | ||
| */ | ||
| public int getFramesContributingToDelayCount() { | ||
| return framesContributingToDelayCount; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,12 +19,15 @@ | |
| import io.sentry.SentryUUID; | ||
| import io.sentry.android.core.BuildInfoProvider; | ||
| import io.sentry.android.core.ContextUtils; | ||
| import io.sentry.android.core.SentryFramesDelayResult; | ||
| import io.sentry.util.Objects; | ||
| import java.lang.ref.WeakReference; | ||
| import java.lang.reflect.Field; | ||
| import java.util.Iterator; | ||
| import java.util.Map; | ||
| import java.util.Set; | ||
| import java.util.concurrent.ConcurrentHashMap; | ||
| import java.util.concurrent.ConcurrentSkipListSet; | ||
| import java.util.concurrent.CopyOnWriteArraySet; | ||
| import java.util.concurrent.TimeUnit; | ||
| import org.jetbrains.annotations.ApiStatus; | ||
|
|
@@ -35,6 +38,8 @@ | |
| public final class SentryFrameMetricsCollector implements Application.ActivityLifecycleCallbacks { | ||
| private static final long oneSecondInNanos = TimeUnit.SECONDS.toNanos(1); | ||
| private static final long frozenFrameThresholdNanos = TimeUnit.MILLISECONDS.toNanos(700); | ||
| private static final int MAX_FRAMES_COUNT = 3600; | ||
| private static final long MAX_FRAME_AGE_NANOS = 5L * 60 * 1_000_000_000L; // 5 minutes | ||
|
|
||
| private final @NotNull BuildInfoProvider buildInfoProvider; | ||
| private final @NotNull Set<Window> trackedWindows = new CopyOnWriteArraySet<>(); | ||
|
|
@@ -53,6 +58,10 @@ public final class SentryFrameMetricsCollector implements Application.ActivityLi | |
| private long lastFrameStartNanos = 0; | ||
| private long lastFrameEndNanos = 0; | ||
|
|
||
| // frame buffer for getFramesDelay queries, sorted by frame end time | ||
| private final @NotNull ConcurrentSkipListSet<DelayedFrame> delayedFrames = | ||
| new ConcurrentSkipListSet<>(); | ||
|
|
||
| @SuppressLint("NewApi") | ||
| public SentryFrameMetricsCollector( | ||
| final @NotNull Context context, | ||
|
|
@@ -177,6 +186,16 @@ public SentryFrameMetricsCollector( | |
| isSlow(cpuDuration, (long) ((float) oneSecondInNanos / (refreshRate - 1.0f))); | ||
| final boolean isFrozen = isSlow && isFrozen(cpuDuration); | ||
|
|
||
| final long frameStartTime = startTime; | ||
|
|
||
| // store frames with delay for getFramesDelay queries | ||
| if (delayNanos > 0) { | ||
| if (delayedFrames.size() <= MAX_FRAMES_COUNT) { | ||
| delayedFrames.add(new DelayedFrame(frameStartTime, lastFrameEndNanos, delayNanos)); | ||
| } | ||
| pruneOldFrames(lastFrameEndNanos); | ||
| } | ||
|
|
||
| for (FrameMetricsCollectorListener l : listenerMap.values()) { | ||
| l.onFrameMetricCollected( | ||
| startTime, | ||
|
|
@@ -354,6 +373,87 @@ public long getLastKnownFrameStartTimeNanos() { | |
| return -1; | ||
| } | ||
|
|
||
| /** | ||
| * Queries the frame delay for a given time range. | ||
| * | ||
| * <p>This is useful for external consumers (e.g. React Native SDK) that need to query frame delay | ||
| * for an arbitrary time range without registering their own frame listener. | ||
| * | ||
| * @param startSystemNanos start of the time range in {@link System#nanoTime()} units | ||
| * @param endSystemNanos end of the time range in {@link System#nanoTime()} units | ||
| * @return a {@link SentryFramesDelayResult} with the delay in seconds and the number of frames | ||
| * contributing to delay, or a result with delaySeconds=-1 if incalculable | ||
| */ | ||
| public @NotNull SentryFramesDelayResult getFramesDelay( | ||
| final long startSystemNanos, final long endSystemNanos) { | ||
| if (!isAvailable) { | ||
| return new SentryFramesDelayResult(-1, 0); | ||
| } | ||
|
|
||
| if (endSystemNanos <= startSystemNanos) { | ||
| return new SentryFramesDelayResult(-1, 0); | ||
| } | ||
|
|
||
| long totalDelayNanos = 0; | ||
| int delayFrameCount = 0; | ||
|
|
||
| if (!delayedFrames.isEmpty()) { | ||
| final Iterator<DelayedFrame> iterator = | ||
| delayedFrames.tailSet(new DelayedFrame(startSystemNanos)).iterator(); | ||
|
|
||
| while (iterator.hasNext()) { | ||
| final @NotNull DelayedFrame frame = iterator.next(); | ||
|
|
||
| if (frame.startNanos >= endSystemNanos) { | ||
| break; | ||
| } | ||
|
|
||
| // The delay portion of a frame is at the end: [frameEnd - delay, frameEnd] | ||
| final long delayStart = frame.endNanos - frame.delayNanos; | ||
| final long delayEnd = frame.endNanos; | ||
|
|
||
| // Intersect the delay interval with the query range | ||
| final long overlapStart = Math.max(delayStart, startSystemNanos); | ||
| final long overlapEnd = Math.min(delayEnd, endSystemNanos); | ||
|
|
||
| if (overlapEnd > overlapStart) { | ||
| totalDelayNanos += (overlapEnd - overlapStart); | ||
| delayFrameCount++; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| final double delaySeconds = totalDelayNanos / 1e9d; | ||
| return new SentryFramesDelayResult(delaySeconds, delayFrameCount); | ||
| } | ||
|
|
||
| private void pruneOldFrames(final long currentNanos) { | ||
| final long cutoff = currentNanos - MAX_FRAME_AGE_NANOS; | ||
| delayedFrames.headSet(new DelayedFrame(cutoff)).clear(); | ||
| } | ||
|
|
||
| private static class DelayedFrame implements Comparable<DelayedFrame> { | ||
| final long startNanos; | ||
| final long endNanos; | ||
| final long delayNanos; | ||
|
|
||
| /** Sentinel constructor for set range queries (tailSet/headSet). */ | ||
| DelayedFrame(final long timestampNanos) { | ||
| this(timestampNanos, timestampNanos, 0); | ||
| } | ||
|
|
||
| DelayedFrame(final long startNanos, final long endNanos, final long delayNanos) { | ||
| this.startNanos = startNanos; | ||
| this.endNanos = endNanos; | ||
| this.delayNanos = delayNanos; | ||
| } | ||
|
|
||
| @Override | ||
| public int compareTo(final @NotNull DelayedFrame o) { | ||
| return Long.compare(this.endNanos, o.endNanos); | ||
| } | ||
|
Comment on lines
+452
to
+454
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: The Suggested FixUpdate the Prompt for AI AgentDid we get this right? ๐ / ๐ to inform future reviews. |
||
| } | ||
|
|
||
| @ApiStatus.Internal | ||
| public interface FrameMetricsCollectorListener { | ||
| /** | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: An off-by-one error in the size check
delayedFrames.size() <= MAX_FRAMES_COUNTallows the buffer to grow to 3601 elements, exceeding the intended maximum of 3600.Severity: LOW
Suggested Fix
Change the buffer capacity check from
delayedFrames.size() <= MAX_FRAMES_COUNTtodelayedFrames.size() < MAX_FRAMES_COUNT. This will correctly prevent additions once the buffer size reaches the maximum limit, ensuring it does not exceed 3600 elements.Prompt for AI Agent
Did we get this right? ๐ / ๐ to inform future reviews.