Skip to content

Commit

Permalink
Collect slow and frozen frames for spans (#3111)
Browse files Browse the repository at this point in the history
* Collect slow and frozen frames using SentryFrameMetricsCollector and SpanFrameMetricsCollector

* Rename fast frame to normal frame, remove fastFrameDuration

* Extend performance collector APIs

* Update changelog

* Collect slow and frozen frames using SentryFrameMetricsCollector and SpanFrameMetricsCollector

* Rename fast frame to normal frame, remove fastFrameDuration

* Fix PR feedback

* Fix nullability and remove unused field

* Rename FrameMetrics to SentryFrameMetrics

* Collect delay instead of total duration

* Fix tests

* Revert "Fix tests"

This reverts commit 5b9c3c6.

* Properly fix tests
  • Loading branch information
markushi authored Jan 18, 2024
1 parent 2657f1d commit a45911f
Show file tree
Hide file tree
Showing 13 changed files with 611 additions and 42 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

- Handle `monitor`/`check_in` in client reports and rate limiter ([#3096](https://github.com/getsentry/sentry-java/pull/3096))
- Extend internal performance collector APIs ([#3102](https://github.com/getsentry/sentry-java/pull/3102))
- Collect slow and frozen frames for spans using `OnFrameMetricsAvailableListener` ([#3111](https://github.com/getsentry/sentry-java/pull/3111))

### Fixes

Expand Down
10 changes: 10 additions & 0 deletions sentry-android-core/api/sentry-android-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr
public fun getBeforeScreenshotCaptureCallback ()Lio/sentry/android/core/SentryAndroidOptions$BeforeCaptureCallback;
public fun getBeforeViewHierarchyCaptureCallback ()Lio/sentry/android/core/SentryAndroidOptions$BeforeCaptureCallback;
public fun getDebugImagesLoader ()Lio/sentry/android/core/IDebugImagesLoader;
public fun getFrameMetricsCollector ()Lio/sentry/android/core/internal/util/SentryFrameMetricsCollector;
public fun getNativeSdkName ()Ljava/lang/String;
public fun getProfilingTracesHz ()I
public fun getProfilingTracesIntervalMillis ()I
Expand Down Expand Up @@ -304,6 +305,7 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr
public fun setEnableRootCheck (Z)V
public fun setEnableScopeSync (Z)V
public fun setEnableSystemEventBreadcrumbs (Z)V
public fun setFrameMetricsCollector (Lio/sentry/android/core/internal/util/SentryFrameMetricsCollector;)V
public fun setNativeSdkName (Ljava/lang/String;)V
public fun setProfilingTracesHz (I)V
public fun setProfilingTracesIntervalMillis (I)V
Expand Down Expand Up @@ -347,6 +349,14 @@ public final class io/sentry/android/core/SentryPerformanceProvider {
public fun onCreate ()Z
}

public class io/sentry/android/core/SpanFrameMetricsCollector : io/sentry/IPerformanceContinuousCollector, io/sentry/android/core/internal/util/SentryFrameMetricsCollector$FrameMetricsCollectorListener {
public fun <init> (Lio/sentry/android/core/SentryAndroidOptions;)V
public fun clear ()V
public fun onFrameMetricCollected (JJJJZZF)V
public fun onSpanFinished (Lio/sentry/ISpan;)V
public fun onSpanStarted (Lio/sentry/ISpan;)V
}

public final class io/sentry/android/core/SystemEventsBreadcrumbsIntegration : io/sentry/Integration, java/io/Closeable {
public fun <init> (Landroid/content/Context;)V
public fun <init> (Landroid/content/Context;Ljava/util/List;)V
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@
* A class that tracks slow and frozen frames using the FrameMetricsAggregator class from
* androidx.core package. It also checks if the FrameMetricsAggregator class is available at
* runtime.
*
* <p>If performance-v2 is enabled, frame metrics are recorded using {@link
* io.sentry.android.core.internal.util.SentryFrameMetricsCollector} via {@link
* SpanFrameMetricsCollector} instead and this implementation will no-op.
*/
public final class ActivityFramesTracker {

Expand Down Expand Up @@ -67,7 +71,9 @@ public ActivityFramesTracker(

@VisibleForTesting
public boolean isFrameMetricsAggregatorAvailable() {
return frameMetricsAggregator != null && options.isEnableFramesTracking();
return frameMetricsAggregator != null
&& options.isEnableFramesTracking()
&& !options.isEnablePerformanceV2();
}

@SuppressWarnings("NullAway")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ static void loadDefaultAndMetadataOptions(
// set a lower flush timeout on Android to avoid ANRs
options.setFlushTimeoutMillis(DEFAULT_FLUSH_TIMEOUT_MS);

options.setFrameMetricsCollector(
new SentryFrameMetricsCollector(context, logger, buildInfoProvider));

ManifestMetadataReader.applyMetadata(context, options, buildInfoProvider);
initializeCacheDirs(context, options);

Expand Down Expand Up @@ -145,10 +148,14 @@ static void initializeIntegrationsAndProcessors(
options.addEventProcessor(new ViewHierarchyEventProcessor(options));
options.addEventProcessor(new AnrV2EventProcessor(context, options, buildInfoProvider));
options.setTransportGate(new AndroidTransportGate(options));
final SentryFrameMetricsCollector frameMetricsCollector =
new SentryFrameMetricsCollector(context, options, buildInfoProvider);
options.setTransactionProfiler(
new AndroidTransactionProfiler(context, options, buildInfoProvider, frameMetricsCollector));
new AndroidTransactionProfiler(
context,
options,
buildInfoProvider,
Objects.requireNonNull(
options.getFrameMetricsCollector(),
"options.getFrameMetricsCollector cannot be null.")));
options.setModulesLoader(new AssetsModulesLoader(context, options.getLogger()));
options.setDebugMetaLoader(new AssetsDebugMetaLoader(context, options.getLogger()));

Expand Down Expand Up @@ -187,6 +194,10 @@ static void initializeIntegrationsAndProcessors(
options.addPerformanceCollector(new AndroidMemoryCollector());
options.addPerformanceCollector(
new AndroidCpuCollector(options.getLogger(), buildInfoProvider));

if (options.isEnablePerformanceV2()) {
options.addPerformanceCollector(new SpanFrameMetricsCollector(options));
}
}
options.setTransactionPerformanceCollector(new DefaultTransactionPerformanceCollector(options));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import io.sentry.SentryOptions;
import io.sentry.SpanStatus;
import io.sentry.android.core.internal.util.RootChecker;
import io.sentry.android.core.internal.util.SentryFrameMetricsCollector;
import io.sentry.protocol.Mechanism;
import io.sentry.protocol.SdkVersion;
import org.jetbrains.annotations.ApiStatus;
Expand Down Expand Up @@ -214,6 +215,8 @@ public interface BeforeCaptureCallback {

private boolean enablePerformanceV2 = false;

private @Nullable SentryFrameMetricsCollector frameMetricsCollector;

public SentryAndroidOptions() {
setSentryClientName(BuildConfig.SENTRY_ANDROID_SDK_NAME + "/" + BuildConfig.VERSION_NAME);
setSdkVersion(createSdkVersion());
Expand Down Expand Up @@ -613,4 +616,15 @@ public boolean isEnablePerformanceV2() {
public void setEnablePerformanceV2(final boolean enablePerformanceV2) {
this.enablePerformanceV2 = enablePerformanceV2;
}

@ApiStatus.Internal
public @Nullable SentryFrameMetricsCollector getFrameMetricsCollector() {
return frameMetricsCollector;
}

@ApiStatus.Internal
public void setFrameMetricsCollector(
final @Nullable SentryFrameMetricsCollector frameMetricsCollector) {
this.frameMetricsCollector = frameMetricsCollector;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
package io.sentry.android.core;

import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;

@ApiStatus.Internal
final class SentryFrameMetrics {

private int normalFrameCount;
private int slowFrameCount;
private int frozenFrameCount;

private long slowFrameDelayNanos;
private long frozenFrameDelayNanos;

public SentryFrameMetrics() {}

public SentryFrameMetrics(
int normalFrameCount,
int slowFrameCount,
long slowFrameDelayNanos,
int frozenFrameCount,
long frozenFrameDelayNanos) {
this.normalFrameCount = normalFrameCount;

this.slowFrameCount = slowFrameCount;
this.slowFrameDelayNanos = slowFrameDelayNanos;

this.frozenFrameCount = frozenFrameCount;
this.frozenFrameDelayNanos = frozenFrameDelayNanos;
}

public void addSlowFrame(final long delayNanos) {
slowFrameDelayNanos += delayNanos;
slowFrameCount++;
}

public void addFrozenFrame(final long delayNanos) {
frozenFrameDelayNanos += delayNanos;
frozenFrameCount++;
}

public void addNormalFrame() {
normalFrameCount++;
}

public int getNormalFrameCount() {
return normalFrameCount;
}

public int getSlowFrameCount() {
return slowFrameCount;
}

public int getFrozenFrameCount() {
return frozenFrameCount;
}

public long getSlowFrameDelayNanos() {
return slowFrameDelayNanos;
}

public long getFrozenFrameDelayNanos() {
return frozenFrameDelayNanos;
}

public int getTotalFrameCount() {
return normalFrameCount + slowFrameCount + frozenFrameCount;
}

public void clear() {
normalFrameCount = 0;

slowFrameCount = 0;
slowFrameDelayNanos = 0;

frozenFrameCount = 0;
frozenFrameDelayNanos = 0;
}

@NotNull
public SentryFrameMetrics duplicate() {
return new SentryFrameMetrics(
normalFrameCount,
slowFrameCount,
slowFrameDelayNanos,
frozenFrameCount,
frozenFrameDelayNanos);
}

/**
* @param other the other frame metrics to compare to, usually the older one
* @return the difference between two frame metrics (this minus other)
*/
@NotNull
public SentryFrameMetrics diffTo(final @NotNull SentryFrameMetrics other) {
return new SentryFrameMetrics(
normalFrameCount - other.normalFrameCount,
slowFrameCount - other.slowFrameCount,
slowFrameDelayNanos - other.slowFrameDelayNanos,
frozenFrameCount - other.frozenFrameCount,
frozenFrameDelayNanos - other.frozenFrameDelayNanos);
}

public boolean containsValidData() {
// TODO sanity check durations?
return getTotalFrameCount() > 0;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
package io.sentry.android.core;

import io.sentry.IPerformanceContinuousCollector;
import io.sentry.ISpan;
import io.sentry.NoOpSpan;
import io.sentry.NoOpTransaction;
import io.sentry.SpanDataConvention;
import io.sentry.SpanId;
import io.sentry.android.core.internal.util.SentryFrameMetricsCollector;
import java.util.HashMap;
import java.util.Map;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

@ApiStatus.Internal
public class SpanFrameMetricsCollector
implements IPerformanceContinuousCollector,
SentryFrameMetricsCollector.FrameMetricsCollectorListener {
private @NotNull final Object lock = new Object();
private @Nullable final SentryFrameMetricsCollector frameMetricsCollector;
private @Nullable volatile String listenerId;
private @NotNull final Map<SpanId, SentryFrameMetrics> metricsAtSpanStart;

private @NotNull final SentryFrameMetrics currentFrameMetrics;
private final boolean enabled;

public SpanFrameMetricsCollector(final @NotNull SentryAndroidOptions options) {
frameMetricsCollector = options.getFrameMetricsCollector();
enabled = options.isEnablePerformanceV2() && options.isEnableFramesTracking();

metricsAtSpanStart = new HashMap<>();
currentFrameMetrics = new SentryFrameMetrics();
}

@Override
public void onSpanStarted(final @NotNull ISpan span) {
if (!enabled) {
return;
}
if (span instanceof NoOpSpan) {
return;
}
if (span instanceof NoOpTransaction) {
return;
}

synchronized (lock) {
metricsAtSpanStart.put(span.getSpanContext().getSpanId(), currentFrameMetrics.duplicate());

if (listenerId == null) {
if (frameMetricsCollector != null) {
listenerId = frameMetricsCollector.startCollection(this);
}
}
}
}

@Override
public void onSpanFinished(final @NotNull ISpan span) {
if (!enabled) {
return;
}
if (span instanceof NoOpSpan) {
return;
}
if (span instanceof NoOpTransaction) {
return;
}

@Nullable SentryFrameMetrics diff = null;
synchronized (lock) {
final @Nullable SentryFrameMetrics metricsAtStart =
metricsAtSpanStart.remove(span.getSpanContext().getSpanId());
if (metricsAtStart != null) {
diff = currentFrameMetrics.diffTo(metricsAtStart);
}
}
if (diff != null && diff.containsValidData()) {
span.setData(SpanDataConvention.FRAMES_SLOW, diff.getSlowFrameCount());
span.setData(SpanDataConvention.FRAMES_FROZEN, diff.getFrozenFrameCount());
span.setData(SpanDataConvention.FRAMES_TOTAL, diff.getTotalFrameCount());
}

synchronized (lock) {
if (metricsAtSpanStart.isEmpty()) {
clear();
}
}
}

@Override
public void clear() {
synchronized (lock) {
if (listenerId != null) {
if (frameMetricsCollector != null) {
frameMetricsCollector.stopCollection(listenerId);
}
listenerId = null;
}
metricsAtSpanStart.clear();
currentFrameMetrics.clear();
}
}

@Override
public void onFrameMetricCollected(
final long frameStartNanos,
final long frameEndNanos,
final long durationNanos,
final long delayNanos,
final boolean isSlow,
final boolean isFrozen,
final float refreshRate) {

if (isFrozen) {
currentFrameMetrics.addFrozenFrame(delayNanos);
} else if (isSlow) {
currentFrameMetrics.addSlowFrame(delayNanos);
} else {
currentFrameMetrics.addNormalFrame();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ import org.mockito.kotlin.verify
import org.mockito.kotlin.whenever
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertNull
import kotlin.test.assertTrue

@RunWith(AndroidJUnit4::class)
class ActivityFramesTrackerTest {
Expand Down Expand Up @@ -384,6 +386,20 @@ class ActivityFramesTrackerTest {
verify(fixture.handler).post(any())
}

@Test
fun `when perf-2 is enabled, activity frame metrics tracker is disabled`() {
fixture.options.isEnablePerformanceV2 = true
val sut = fixture.getSut()
assertFalse(sut.isFrameMetricsAggregatorAvailable)
}

@Test
fun `when perf-2 is disabled, activity frame metrics tracker is enabled`() {
fixture.options.isEnablePerformanceV2 = false
val sut = fixture.getSut()
assertTrue(sut.isFrameMetricsAggregatorAvailable)
}

private fun getArray(frameTime: Int = 1, numFrames: Int = 1): Array<SparseIntArray?> {
val totalArray = SparseIntArray()
totalArray.put(frameTime, numFrames)
Expand Down
Loading

0 comments on commit a45911f

Please sign in to comment.