Add debouncing mechanism and before-capture callbacks for screenshots/vh (#2773)

Author: markushi

.github/workflows/agp-matrix.yml

@@ -17,7 +17,7 @@ jobs:
           access_token: ${{ github.token }}
 
   agp-matrix-compatibility:
-    timeout-minutes: 25
+    timeout-minutes: 30
     runs-on: macos-latest
     strategy:
       fail-fast: false

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Features
+
+- Add debouncing mechanism and before-capture callbacks for screenshots and view hierarchies ([#2773](https://github.com/getsentry/sentry-java/pull/2773))
+
 ## 6.23.0
 
 ### Features

sentry-android-core/api/sentry-android-core.api

@@ -207,6 +207,8 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr
 	public fun <init> ()V
 	public fun enableAllAutoBreadcrumbs (Z)V
 	public fun getAnrTimeoutIntervalMillis ()J
+	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 getNativeSdkName ()Ljava/lang/String;
 	public fun getProfilingTracesHz ()I
@@ -231,6 +233,8 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr
 	public fun setAnrTimeoutIntervalMillis (J)V
 	public fun setAttachScreenshot (Z)V
 	public fun setAttachViewHierarchy (Z)V
+	public fun setBeforeScreenshotCaptureCallback (Lio/sentry/android/core/SentryAndroidOptions$BeforeCaptureCallback;)V
+	public fun setBeforeViewHierarchyCaptureCallback (Lio/sentry/android/core/SentryAndroidOptions$BeforeCaptureCallback;)V
 	public fun setCollectAdditionalContext (Z)V
 	public fun setDebugImagesLoader (Lio/sentry/android/core/IDebugImagesLoader;)V
 	public fun setEnableActivityLifecycleBreadcrumbs (Z)V
@@ -247,6 +251,10 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr
 	public fun setProfilingTracesIntervalMillis (I)V
 }
 
+public abstract interface class io/sentry/android/core/SentryAndroidOptions$BeforeCaptureCallback {
+	public abstract fun execute (Lio/sentry/SentryEvent;Lio/sentry/Hint;Z)Z
+}
+
 public final class io/sentry/android/core/SentryInitProvider {
 	public fun <init> ()V
 	public fun attachInfo (Landroid/content/Context;Landroid/content/pm/ProviderInfo;)V

sentry-android-core/src/main/java/io/sentry/android/core/ScreenshotEventProcessor.java

@@ -10,6 +10,8 @@
 import io.sentry.IntegrationName;
 import io.sentry.SentryEvent;
 import io.sentry.SentryLevel;
+import io.sentry.android.core.internal.util.AndroidCurrentDateProvider;
+import io.sentry.android.core.internal.util.Debouncer;
 import io.sentry.util.HintUtils;
 import io.sentry.util.Objects;
 import org.jetbrains.annotations.ApiStatus;
@@ -26,12 +28,17 @@ public final class ScreenshotEventProcessor implements EventProcessor, Integrati
   private final @NotNull SentryAndroidOptions options;
   private final @NotNull BuildInfoProvider buildInfoProvider;
 
+  private final @NotNull Debouncer debouncer;
+  private static final long DEBOUNCE_WAIT_TIME_MS = 2000;
+
   public ScreenshotEventProcessor(
       final @NotNull SentryAndroidOptions options,
       final @NotNull BuildInfoProvider buildInfoProvider) {
     this.options = Objects.requireNonNull(options, "SentryAndroidOptions is required");
     this.buildInfoProvider =
         Objects.requireNonNull(buildInfoProvider, "BuildInfoProvider is required");
+    this.debouncer = new Debouncer(AndroidCurrentDateProvider.getInstance(), DEBOUNCE_WAIT_TIME_MS);
+
     if (options.isAttachScreenshot()) {
       addIntegrationToSdkVersion();
     }
@@ -52,6 +59,19 @@ public ScreenshotEventProcessor(
       return event;
     }
 
+    // skip capturing in case of debouncing (=too many frequent capture requests)
+    // the BeforeCaptureCallback may overrules the debouncing decision
+    final boolean shouldDebounce = debouncer.checkForDebounce();
+    final @Nullable SentryAndroidOptions.BeforeCaptureCallback beforeCaptureCallback =
+        options.getBeforeScreenshotCaptureCallback();
+    if (beforeCaptureCallback != null) {
+      if (!beforeCaptureCallback.execute(event, hint, shouldDebounce)) {
+        return event;
+      }
+    } else if (shouldDebounce) {
+      return event;
+    }
+
     final byte[] screenshot =
         takeScreenshot(
             activity, options.getMainThreadChecker(), options.getLogger(), buildInfoProvider);

sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java

@@ -1,8 +1,10 @@
 package io.sentry.android.core;
 
+import io.sentry.Hint;
 import io.sentry.ISpan;
 import io.sentry.Scope;
 import io.sentry.Sentry;
+import io.sentry.SentryEvent;
 import io.sentry.SentryOptions;
 import io.sentry.SpanStatus;
 import io.sentry.android.core.internal.util.RootChecker;
@@ -98,10 +100,18 @@ public final class SentryAndroidOptions extends SentryOptions {
   /** Interface that loads the debug images list */
   private @NotNull IDebugImagesLoader debugImagesLoader = NoOpDebugImagesLoader.getInstance();
 
-  /** Enables or disables the attach screenshot feature when an error happened. */
+  /**
+   * Enables or disables the attach screenshot feature when an error happened. Use {@link
+   * SentryAndroidOptions#setBeforeScreenshotCaptureCallback(BeforeCaptureCallback)} ()} to control
+   * when a screenshot should be captured.
+   */
   private boolean attachScreenshot;
 
-  /** Enables or disables the attach view hierarchy feature when an error happened. */
+  /**
+   * Enables or disables the attach view hierarchy feature when an error happened. Use {@link
+   * SentryAndroidOptions#setBeforeViewHierarchyCaptureCallback(BeforeCaptureCallback)} ()} to
+   * control when a view hierarchy should be captured.
+   */
   private boolean attachViewHierarchy;
 
   /**
@@ -143,6 +153,35 @@ public final class SentryAndroidOptions extends SentryOptions {
    */
   private boolean enableRootCheck = true;
 
+  private @Nullable BeforeCaptureCallback beforeScreenshotCaptureCallback;
+
+  private @Nullable BeforeCaptureCallback beforeViewHierarchyCaptureCallback;
+
+  public interface BeforeCaptureCallback {
+
+    /**
+     * A callback which can be used to suppress capturing of screenshots or view hierarchies. This
+     * gives more fine grained control when capturing should be performed. E.g. - only capture
+     * screenshots for fatal events - overrule any debouncing for important events <br>
+     * As capturing can be resource-intensive, the debounce parameter should be respected if
+     * possible.
+     *
+     * <pre>
+     *  if (debounce) {
+     *    return false;
+     *  } else {
+     *    // check event and hint
+     *  }
+     *  </pre>
+     *
+     * @param event the event
+     * @param hint the hints
+     * @param debounce true if capturing is marked for being debounced
+     * @return true if capturing should be performed, false otherwise
+     */
+    boolean execute(@NotNull SentryEvent event, @NotNull Hint hint, boolean debounce);
+  }
+
   public SentryAndroidOptions() {
     setSentryClientName(BuildConfig.SENTRY_ANDROID_SDK_NAME + "/" + BuildConfig.VERSION_NAME);
     setSdkVersion(createSdkVersion());
@@ -441,4 +480,34 @@ public boolean isEnableRootCheck() {
   public void setEnableRootCheck(final boolean enableRootCheck) {
     this.enableRootCheck = enableRootCheck;
   }
+
+  public @Nullable BeforeCaptureCallback getBeforeScreenshotCaptureCallback() {
+    return beforeScreenshotCaptureCallback;
+  }
+
+  /**
+   * Sets a callback which is executed before capturing screenshots. Only relevant if
+   * attachScreenshot is set to true.
+   *
+   * @param beforeScreenshotCaptureCallback the callback to execute
+   */
+  public void setBeforeScreenshotCaptureCallback(
+      final @NotNull BeforeCaptureCallback beforeScreenshotCaptureCallback) {
+    this.beforeScreenshotCaptureCallback = beforeScreenshotCaptureCallback;
+  }
+
+  public @Nullable BeforeCaptureCallback getBeforeViewHierarchyCaptureCallback() {
+    return beforeViewHierarchyCaptureCallback;
+  }
+
+  /**
+   * Sets a callback which is executed before capturing view hierarchies. Only relevant if
+   * attachViewHierarchy is set to true.
+   *
+   * @param beforeViewHierarchyCaptureCallback the callback to execute
+   */
+  public void setBeforeViewHierarchyCaptureCallback(
+      final @NotNull BeforeCaptureCallback beforeViewHierarchyCaptureCallback) {
+    this.beforeViewHierarchyCaptureCallback = beforeViewHierarchyCaptureCallback;
+  }
 }

sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java

@@ -13,7 +13,9 @@
 import io.sentry.SentryEvent;
 import io.sentry.SentryLevel;
 import io.sentry.android.core.internal.gestures.ViewUtils;
+import io.sentry.android.core.internal.util.AndroidCurrentDateProvider;
 import io.sentry.android.core.internal.util.AndroidMainThreadChecker;
+import io.sentry.android.core.internal.util.Debouncer;
 import io.sentry.internal.viewhierarchy.ViewHierarchyExporter;
 import io.sentry.protocol.ViewHierarchy;
 import io.sentry.protocol.ViewHierarchyNode;
@@ -35,10 +37,15 @@
 public final class ViewHierarchyEventProcessor implements EventProcessor, IntegrationName {
 
   private final @NotNull SentryAndroidOptions options;
+  private final @NotNull Debouncer debouncer;
+
   private static final long CAPTURE_TIMEOUT_MS = 1000;
+  private static final long DEBOUNCE_WAIT_TIME_MS = 2000;
 
   public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) {
     this.options = Objects.requireNonNull(options, "SentryAndroidOptions is required");
+    this.debouncer = new Debouncer(AndroidCurrentDateProvider.getInstance(), DEBOUNCE_WAIT_TIME_MS);
+
     if (options.isAttachViewHierarchy()) {
       addIntegrationToSdkVersion();
     }
@@ -59,6 +66,19 @@ public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options)
       return event;
     }
 
+    // skip capturing in case of debouncing (=too many frequent capture requests)
+    // the BeforeCaptureCallback may overrules the debouncing decision
+    final boolean shouldDebounce = debouncer.checkForDebounce();
+    final @Nullable SentryAndroidOptions.BeforeCaptureCallback beforeCaptureCallback =
+        options.getBeforeViewHierarchyCaptureCallback();
+    if (beforeCaptureCallback != null) {
+      if (!beforeCaptureCallback.execute(event, hint, shouldDebounce)) {
+        return event;
+      }
+    } else if (shouldDebounce) {
+      return event;
+    }
+
     final @Nullable Activity activity = CurrentActivityHolder.getInstance().getActivity();
     final @Nullable ViewHierarchy viewHierarchy =
         snapshotViewHierarchy(

sentry-android-core/src/main/java/io/sentry/android/core/internal/util/Debouncer.java

@@ -0,0 +1,33 @@
+package io.sentry.android.core.internal.util;
+
+import io.sentry.transport.ICurrentDateProvider;
+import org.jetbrains.annotations.ApiStatus;
+import org.jetbrains.annotations.NotNull;
+
+/** A simple time-based debouncing mechanism */
+@ApiStatus.Internal
+public class Debouncer {
+
+  private final long waitTimeMs;
+  private final @NotNull ICurrentDateProvider timeProvider;
+
+  private Long lastExecutionTime = null;
+
+  public Debouncer(final @NotNull ICurrentDateProvider timeProvider, final long waitTimeMs) {
+    this.timeProvider = timeProvider;
+    this.waitTimeMs = waitTimeMs;
+  }
+
+  /**
+   * @return true if the execution should be debounced due to the last execution being within within
+   *     waitTimeMs, otherwise false.
+   */
+  public boolean checkForDebounce() {
+    final long now = timeProvider.getCurrentTimeMillis();
+    if (lastExecutionTime == null || (lastExecutionTime + waitTimeMs) <= now) {
+      lastExecutionTime = now;
+      return false;
+    }
+    return true;
+  }
+}

sentry-android-core/src/test/java/io/sentry/android/core/ScreenshotEventProcessorTest.kt

@@ -10,6 +10,7 @@ import io.sentry.MainEventProcessor
 import io.sentry.SentryEvent
 import io.sentry.SentryIntegrationPackageStorage
 import io.sentry.TypeCheckHint.ANDROID_ACTIVITY
+import io.sentry.protocol.SentryException
 import io.sentry.util.thread.IMainThreadChecker
 import org.junit.runner.RunWith
 import org.mockito.kotlin.any
@@ -66,6 +67,7 @@ class ScreenshotEventProcessorTest {
     @BeforeTest
     fun `set up`() {
         fixture = Fixture()
+        CurrentActivityHolder.getInstance().clearActivity()
     }
 
     @Test
@@ -200,5 +202,102 @@ class ScreenshotEventProcessorTest {
         assertFalse(fixture.options.sdkVersion!!.integrationSet.contains("Screenshot"))
     }
 
+    @Test
+    fun `when screenshots are captured rapidly, capturing should be debounced`() {
+        CurrentActivityHolder.getInstance().setActivity(fixture.activity)
+
+        val processor = fixture.getSut(true)
+        val event = SentryEvent().apply {
+            exceptions = listOf(SentryException())
+        }
+        val hint0 = Hint()
+        processor.process(event, hint0)
+        assertNotNull(hint0.screenshot)
+
+        val hint1 = Hint()
+        processor.process(event, hint1)
+        assertNull(hint1.screenshot)
+    }
+
+    @Test
+    fun `when screenshots are captured rapidly, debounce flag should be propagated`() {
+        CurrentActivityHolder.getInstance().setActivity(fixture.activity)
+
+        var debounceFlag = false
+        fixture.options.setBeforeScreenshotCaptureCallback { _, _, debounce ->
+            debounceFlag = debounce
+            true
+        }
+
+        val processor = fixture.getSut(true)
+        val event = SentryEvent().apply {
+            exceptions = listOf(SentryException())
+        }
+        val hint0 = Hint()
+        processor.process(event, hint0)
+        assertFalse(debounceFlag)
+
+        val hint1 = Hint()
+        processor.process(event, hint1)
+        assertTrue(debounceFlag)
+    }
+
+    @Test
+    fun `when screenshots are captured rapidly, capture callback can still overrule debouncing`() {
+        CurrentActivityHolder.getInstance().setActivity(fixture.activity)
+
+        val processor = fixture.getSut(true)
+
+        fixture.options.setBeforeScreenshotCaptureCallback { _, _, _ ->
+            true
+        }
+        val event = SentryEvent().apply {
+            exceptions = listOf(SentryException())
+        }
+        val hint0 = Hint()
+        processor.process(event, hint0)
+        assertNotNull(hint0.screenshot)
+
+        val hint1 = Hint()
+        processor.process(event, hint1)
+        assertNotNull(hint1.screenshot)
+    }
+
+    @Test
+    fun `when capture callback returns false, no screenshot should be captured`() {
+        CurrentActivityHolder.getInstance().setActivity(fixture.activity)
+
+        fixture.options.setBeforeScreenshotCaptureCallback { _, _, _ ->
+            false
+        }
+        val processor = fixture.getSut(true)
+
+        val event = SentryEvent().apply {
+            exceptions = listOf(SentryException())
+        }
+        val hint = Hint()
+
+        processor.process(event, hint)
+        assertNull(hint.screenshot)
+    }
+
+    @Test
+    fun `when capture callback returns true, a screenshot should be captured`() {
+        CurrentActivityHolder.getInstance().setActivity(fixture.activity)
+
+        fixture.options.setBeforeViewHierarchyCaptureCallback { _, _, _ ->
+            true
+        }
+        val processor = fixture.getSut(true)
+
+        val event = SentryEvent().apply {
+            exceptions = listOf(SentryException())
+        }
+        val hint = Hint()
+
+        processor.process(event, hint)
+        assertNotNull(hint.screenshot)
+    }
+
     private fun getEvent(): SentryEvent = SentryEvent(Throwable("Throwable"))
 }