Skip to content

Commit

Permalink
Add option to enable or disable Activity Frame Tracking (#2314)
Browse files Browse the repository at this point in the history
* Add option to enable or disable Activity Frames Tracker

* Add Activity Frames Tracker to changelog

* Use options flag instead of Noop impl for FrameTracker, improve naming

* Restore ActivityFramesTracker constructor, as it's required for flutter

* Fix missing @deprecated annotation for ActivityFramesTracker

* Fix disable InlineMeSuggester error

* Add missing .api file update

* Adapt ActivityFramesTracker ctor and options nullability

Co-authored-by: Alexander Dinauer <adinauer@users.noreply.github.com>
  • Loading branch information
markushi and adinauer committed Oct 24, 2022
1 parent 649f171 commit 83be7c7
Show file tree
Hide file tree
Showing 10 changed files with 152 additions and 36 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
- Add support for using Encoder with logback.SentryAppender ([#2246](https://github.com/getsentry/sentry-java/pull/2246))
- Report Startup Crashes ([#2277](https://github.com/getsentry/sentry-java/pull/2277))
- HTTP Client errors for OkHttp ([#2287](https://github.com/getsentry/sentry-java/pull/2287))
- Add option to enable or disable Frame Tracking ([#2314](https://github.com/getsentry/sentry-java/pull/2314))

### Dependencies

Expand Down
8 changes: 5 additions & 3 deletions sentry-android-core/api/sentry-android-core.api
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
public final class io/sentry/android/core/ActivityFramesTracker {
public fun <init> (Lio/sentry/android/core/LoadClass;)V
public fun <init> (Lio/sentry/android/core/LoadClass;Lio/sentry/ILogger;)V
public fun <init> (Lio/sentry/android/core/LoadClass;Lio/sentry/ILogger;Lio/sentry/android/core/MainLooperHandler;)V
public fun <init> (Lio/sentry/android/core/LoadClass;Lio/sentry/android/core/SentryAndroidOptions;)V
public fun <init> (Lio/sentry/android/core/LoadClass;Lio/sentry/android/core/SentryAndroidOptions;Lio/sentry/android/core/MainLooperHandler;)V
public fun addActivity (Landroid/app/Activity;)V
public fun isFrameMetricsAggregatorAvailable ()Z
public fun setMetrics (Landroid/app/Activity;Lio/sentry/protocol/SentryId;)V
public fun stop ()V
public fun takeMetrics (Lio/sentry/protocol/SentryId;)Ljava/util/Map;
Expand Down Expand Up @@ -141,6 +141,7 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr
public fun isEnableAppComponentBreadcrumbs ()Z
public fun isEnableAppLifecycleBreadcrumbs ()Z
public fun isEnableAutoActivityLifecycleTracing ()Z
public fun isEnableFramesTracking ()Z
public fun isEnableSystemEventBreadcrumbs ()Z
public fun isEnableUserInteractionBreadcrumbs ()Z
public fun isEnableUserInteractionTracing ()Z
Expand All @@ -155,6 +156,7 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr
public fun setEnableAppComponentBreadcrumbs (Z)V
public fun setEnableAppLifecycleBreadcrumbs (Z)V
public fun setEnableAutoActivityLifecycleTracing (Z)V
public fun setEnableFramesTracking (Z)V
public fun setEnableSystemEventBreadcrumbs (Z)V
public fun setEnableUserInteractionBreadcrumbs (Z)V
public fun setEnableUserInteractionTracing (Z)V
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import android.app.Activity;
import android.util.SparseIntArray;
import androidx.core.app.FrameMetricsAggregator;
import io.sentry.ILogger;
import io.sentry.MeasurementUnit;
import io.sentry.SentryLevel;
import io.sentry.android.core.internal.util.MainThreadChecker;
Expand All @@ -16,6 +15,7 @@
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.jetbrains.annotations.TestOnly;
import org.jetbrains.annotations.VisibleForTesting;

/**
* A class that tracks slow and frozen frames using the FrameMetricsAggregator class from
Expand All @@ -25,48 +25,49 @@
public final class ActivityFramesTracker {

private @Nullable FrameMetricsAggregator frameMetricsAggregator = null;
private boolean androidXAvailable = true;
private @NotNull final SentryAndroidOptions options;

private final @NotNull Map<SentryId, Map<String, @NotNull MeasurementValue>>
activityMeasurements = new ConcurrentHashMap<>();
private final @NotNull Map<Activity, FrameCounts> frameCountAtStartSnapshots =
new WeakHashMap<>();

private final @Nullable ILogger logger;
private final @NotNull MainLooperHandler handler;

public ActivityFramesTracker(
final @NotNull LoadClass loadClass,
final @Nullable ILogger logger,
final @NotNull SentryAndroidOptions options,
final @NotNull MainLooperHandler handler) {
androidXAvailable =
loadClass.isClassAvailable("androidx.core.app.FrameMetricsAggregator", logger);

final boolean androidXAvailable =
loadClass.isClassAvailable("androidx.core.app.FrameMetricsAggregator", options.getLogger());

if (androidXAvailable) {
frameMetricsAggregator = new FrameMetricsAggregator();
}
this.logger = logger;
this.options = options;
this.handler = handler;
}

public ActivityFramesTracker(final @NotNull LoadClass loadClass, final @Nullable ILogger logger) {
this(loadClass, logger, new MainLooperHandler());
}

public ActivityFramesTracker(final @NotNull LoadClass loadClass) {
this(loadClass, null);
public ActivityFramesTracker(
final @NotNull LoadClass loadClass, final @NotNull SentryAndroidOptions options) {
this(loadClass, options, new MainLooperHandler());
}

@TestOnly
ActivityFramesTracker(
final @Nullable FrameMetricsAggregator frameMetricsAggregator,
final @NotNull MainLooperHandler handler) {
final @NotNull LoadClass loadClass,
final @NotNull SentryAndroidOptions options,
final @NotNull MainLooperHandler handler,
final @Nullable FrameMetricsAggregator frameMetricsAggregator) {

this(loadClass, options, handler);
this.frameMetricsAggregator = frameMetricsAggregator;
this.logger = null;
this.handler = handler;
}

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

@SuppressWarnings("NullAway")
Expand Down Expand Up @@ -215,15 +216,15 @@ private void runSafelyOnUiThread(final Runnable runnable, final String tag) {
try {
runnable.run();
} catch (Throwable ignored) {
if (logger != null && tag != null) {
logger.log(SentryLevel.WARNING, "Failed to execute " + tag);
if (tag != null) {
options.getLogger().log(SentryLevel.WARNING, "Failed to execute " + tag);
}
}
});
}
} catch (Throwable ignored) {
if (logger != null && tag != null) {
logger.log(SentryLevel.WARNING, "Failed to execute " + tag);
if (tag != null) {
options.getLogger().log(SentryLevel.WARNING, "Failed to execute " + tag);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,12 @@ WeakHashMap<Activity, ITransaction> getActivitiesWithOngoingTransactions() {
return activitiesWithOngoingTransactions;
}

@TestOnly
@NotNull
ActivityFramesTracker getActivityFramesTracker() {
return activityFramesTracker;
}

@TestOnly
@Nullable
ISpan getAppStartSpan() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ static void init(
options.setEnvelopeDiskCache(new AndroidEnvelopeCache(options));

final ActivityFramesTracker activityFramesTracker =
new ActivityFramesTracker(loadClass, options.getLogger());
new ActivityFramesTracker(loadClass, options);

installDefaultIntegrations(
context,
options,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ final class ManifestMetadataReader {

static final String SEND_DEFAULT_PII = "io.sentry.send-default-pii";

static final String PERFORM_FRAMES_TRACKING = "io.sentry.traces.frames-tracking";

/** ManifestMetadataReader ctor */
private ManifestMetadataReader() {}

Expand Down Expand Up @@ -288,6 +290,8 @@ static void applyMetadata(
options.setTracePropagationTargets(tracePropagationTargets);
}

options.setEnableFramesTracking(readBool(metadata, logger, PERFORM_FRAMES_TRACKING, true));

options.setProguardUuid(
readString(metadata, logger, PROGUARD_UUID, options.getProguardUuid()));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ public final class SentryAndroidOptions extends SentryOptions {
*/
private final long startupCrashDurationThresholdMillis = 2000; // 2s

private boolean enableFramesTracking = true;

public SentryAndroidOptions() {
setSentryClientName(BuildConfig.SENTRY_ANDROID_SDK_NAME + "/" + BuildConfig.VERSION_NAME);
setSdkVersion(createSdkVersion());
Expand Down Expand Up @@ -357,6 +359,19 @@ public void setCollectAdditionalContext(boolean collectAdditionalContext) {
this.collectAdditionalContext = collectAdditionalContext;
}

public boolean isEnableFramesTracking() {
return enableFramesTracking;
}

/**
* Enable or disable Frames Tracking, which is used to report slow and frozen frames.
*
* @param enableFramesTracking true if frames tracking should be enabled, false otherwise.
*/
public void setEnableFramesTracking(boolean enableFramesTracking) {
this.enableFramesTracking = enableFramesTracking;
}

/**
* Returns the Startup Crash flush timeout in Millis
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,14 @@ class ActivityFramesTrackerTest {
val sentryId = SentryId()
val loadClass = mock<LoadClass>()
val handler = mock<MainLooperHandler>()

fun getSut(): ActivityFramesTracker {
return ActivityFramesTracker(aggregator, handler)
val options = SentryAndroidOptions()

fun getSut(mockAggregator: Boolean = true): ActivityFramesTracker {
return if (mockAggregator) {
ActivityFramesTracker(loadClass, options, handler, aggregator)
} else {
ActivityFramesTracker(loadClass, options, handler)
}
}
}
private val fixture = Fixture()
Expand Down Expand Up @@ -287,23 +292,23 @@ class ActivityFramesTrackerTest {
@Test
fun `addActivity does not throw if no AndroidX`() {
whenever(fixture.loadClass.isClassAvailable(any(), any<ILogger>())).thenReturn(false)
val sut = ActivityFramesTracker(fixture.loadClass)
val sut = fixture.getSut(false)

sut.addActivity(fixture.activity)
}

@Test
fun `setMetrics does not throw if no AndroidX`() {
whenever(fixture.loadClass.isClassAvailable(any(), any<ILogger>())).thenReturn(false)
val sut = ActivityFramesTracker(fixture.loadClass)
val sut = fixture.getSut(false)

sut.setMetrics(fixture.activity, fixture.sentryId)
}

@Test
fun `addActivity and setMetrics combined do not throw if no AndroidX`() {
whenever(fixture.loadClass.isClassAvailable(any(), any<ILogger>())).thenReturn(false)
val sut = ActivityFramesTracker(fixture.loadClass)
val sut = fixture.getSut(false)

sut.addActivity(fixture.activity)
sut.setMetrics(fixture.activity, fixture.sentryId)
Expand All @@ -312,15 +317,15 @@ class ActivityFramesTrackerTest {
@Test
fun `setMetrics does not throw if Activity is not added`() {
whenever(fixture.aggregator.metrics).thenThrow(IllegalArgumentException())
val sut = ActivityFramesTracker(fixture.loadClass)
val sut = fixture.getSut()

sut.setMetrics(fixture.activity, fixture.sentryId)
}

@Test
fun `stop does not throw if no AndroidX`() {
whenever(fixture.loadClass.isClassAvailable(any(), any<ILogger>())).thenReturn(false)
val sut = ActivityFramesTracker(fixture.loadClass)
val sut = fixture.getSut(false)

sut.stop()
}
Expand All @@ -337,7 +342,7 @@ class ActivityFramesTrackerTest {
@Test
fun `takeMetrics returns null if no AndroidX`() {
whenever(fixture.loadClass.isClassAvailable(any(), any<ILogger>())).thenReturn(false)
val sut = ActivityFramesTracker(fixture.loadClass)
val sut = fixture.getSut(false)

assertNull(sut.takeMetrics(fixture.sentryId))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -371,4 +371,47 @@ class AndroidOptionsInitializerTest {

assertTrue { fixture.sentryOptions.envelopeDiskCache is AndroidEnvelopeCache }
}

@Test
fun `When Activity Frames Tracking is enabled, the Activity Frames Tracker should be available`() {
fixture.initSut(hasAppContext = true, configureOptions = {
isEnableFramesTracking = true
})

val activityLifeCycleIntegration = fixture.sentryOptions.integrations
.first { it is ActivityLifecycleIntegration }

assertTrue(
(activityLifeCycleIntegration as ActivityLifecycleIntegration).activityFramesTracker.isFrameMetricsAggregatorAvailable
)
}

@Test
fun `When Frames Tracking is disabled, the Activity Frames Tracker should not be available`() {
fixture.initSut(hasAppContext = true, configureOptions = {
isEnableFramesTracking = false
})

val activityLifeCycleIntegration = fixture.sentryOptions.integrations
.first { it is ActivityLifecycleIntegration }

assertFalse(
(activityLifeCycleIntegration as ActivityLifecycleIntegration).activityFramesTracker.isFrameMetricsAggregatorAvailable
)
}

@Test
fun `When Frames Tracking is initially disabled, but enabled via configureOptions it should be available`() {
fixture.sentryOptions.isEnableFramesTracking = false
fixture.initSut(hasAppContext = true, configureOptions = {
isEnableFramesTracking = true
})

val activityLifeCycleIntegration = fixture.sentryOptions.integrations
.first { it is ActivityLifecycleIntegration }

assertTrue(
(activityLifeCycleIntegration as ActivityLifecycleIntegration).activityFramesTracker.isFrameMetricsAggregatorAvailable
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1086,4 +1086,42 @@ class ManifestMetadataReaderTest {
// Assert
assertTrue(fixture.options.isSendDefaultPii)
}

@Test
fun `applyMetadata reads frames tracking flag and keeps default value if not found`() {
// Arrange
val context = fixture.getContext()

// Act
ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider)

// Assert
assertTrue(fixture.options.isEnableFramesTracking)
}

@Test
fun `applyMetadata reads frames tracking and sets it to enabled if true`() {
// Arrange
val bundle = bundleOf(ManifestMetadataReader.PERFORM_FRAMES_TRACKING to true)
val context = fixture.getContext(metaData = bundle)

// Act
ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider)

// Assert
assertTrue(fixture.options.isEnableFramesTracking)
}

@Test
fun `applyMetadata reads frames tracking and sets it to disabled if false`() {
// Arrange
val bundle = bundleOf(ManifestMetadataReader.PERFORM_FRAMES_TRACKING to false)
val context = fixture.getContext(metaData = bundle)

// Act
ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider)

// Assert
assertFalse(fixture.options.isEnableFramesTracking)
}
}

0 comments on commit 83be7c7

Please sign in to comment.