Skip to content
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

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

Merged
merged 9 commits into from
Jun 19, 2023
Merged
2 changes: 1 addition & 1 deletion .github/workflows/agp-matrix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
- Add `lock` attribute to the `SentryStackFrame` protocol to better highlight offending frames in the UI ([#2761](https://github.com/getsentry/sentry-java/pull/2761))
- Enrich database spans with blocked main thread info ([#2760](https://github.com/getsentry/sentry-java/pull/2760))
- Add `api_target` to `Request` and `data` to `Response` Protocols ([#2775](https://github.com/getsentry/sentry-java/pull/2775))
- Add debouncing mechanism and before-capture callbacks for screenshots and view hierarchies ([#2773](https://github.com/getsentry/sentry-java/pull/2773))

## 6.21.0

Expand Down
8 changes: 8 additions & 0 deletions sentry-android-core/api/sentry-android-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
}
Expand All @@ -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();
stefanosiano marked this conversation as resolved.
Show resolved Hide resolved
final @Nullable SentryAndroidOptions.BeforeCaptureCallback beforeCaptureCallback =
options.getBeforeScreenshotCaptureCallback();
if (beforeCaptureCallback != null) {
markushi marked this conversation as resolved.
Show resolved Hide resolved
if (!beforeCaptureCallback.execute(event, hint, shouldDebounce)) {
krystofwoldrich marked this conversation as resolved.
Show resolved Hide resolved
return event;
}
} else if (shouldDebounce) {
return event;
}

final byte[] screenshot =
takeScreenshot(
activity, options.getMainThreadChecker(), options.getLogger(), buildInfoProvider);
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -143,6 +145,23 @@ 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.
*
* @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());
Expand Down Expand Up @@ -441,4 +460,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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
}
Expand All @@ -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();
stefanosiano marked this conversation as resolved.
Show resolved Hide resolved
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(
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -66,6 +67,7 @@ class ScreenshotEventProcessorTest {
@BeforeTest
fun `set up`() {
fixture = Fixture()
CurrentActivityHolder.getInstance().clearActivity()
}

@Test
Expand Down Expand Up @@ -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)
}

stefanosiano marked this conversation as resolved.
Show resolved Hide resolved
private fun getEvent(): SentryEvent = SentryEvent(Throwable("Throwable"))
}
Loading