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

improve default debouncing mechanism #2945

Merged
merged 9 commits into from
Sep 29, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Features

- Improve default debouncing mechanism ([#2945](https://github.com/getsentry/sentry-java/pull/2945))
- Add `sendModules` option for disable sending modules ([#2926](https://github.com/getsentry/sentry-java/pull/2926))
- Send `db.system` and `db.name` in span data for androidx.sqlite spans ([#2928](https://github.com/getsentry/sentry-java/pull/2928))
- Add API for sending checkins (CRONS) manually ([#2935](https://github.com/getsentry/sentry-java/pull/2935))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,19 @@ public final class ScreenshotEventProcessor implements EventProcessor, Integrati

private final @NotNull Debouncer debouncer;
private static final long DEBOUNCE_WAIT_TIME_MS = 2000;
private static final int DEBOUNCE_MAX_EXECUTIONS = 3;

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);
this.debouncer =
new Debouncer(
AndroidCurrentDateProvider.getInstance(),
DEBOUNCE_WAIT_TIME_MS,
DEBOUNCE_MAX_EXECUTIONS);

if (options.isAttachScreenshot()) {
addIntegrationToSdkVersion();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,15 @@ public final class ViewHierarchyEventProcessor implements EventProcessor, Integr

private static final long CAPTURE_TIMEOUT_MS = 1000;
private static final long DEBOUNCE_WAIT_TIME_MS = 2000;
private static final int DEBOUNCE_MAX_EXECUTIONS = 3;

public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) {
this.options = Objects.requireNonNull(options, "SentryAndroidOptions is required");
this.debouncer = new Debouncer(AndroidCurrentDateProvider.getInstance(), DEBOUNCE_WAIT_TIME_MS);
this.debouncer =
new Debouncer(
AndroidCurrentDateProvider.getInstance(),
DEBOUNCE_WAIT_TIME_MS,
DEBOUNCE_MAX_EXECUTIONS);

if (options.isAttachViewHierarchy()) {
addIntegrationToSdkVersion();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,18 @@ public class Debouncer {

private final long waitTimeMs;
private final @NotNull ICurrentDateProvider timeProvider;
private int executions = 0;
stefanosiano marked this conversation as resolved.
Show resolved Hide resolved
private final int maxExecutions;

private Long lastExecutionTime = null;

public Debouncer(final @NotNull ICurrentDateProvider timeProvider, final long waitTimeMs) {
public Debouncer(
final @NotNull ICurrentDateProvider timeProvider,
final long waitTimeMs,
final int maxExecutions) {
this.timeProvider = timeProvider;
this.waitTimeMs = waitTimeMs;
this.maxExecutions = maxExecutions <= 0 ? 1 : maxExecutions;
}

/**
Expand All @@ -25,9 +31,15 @@ public Debouncer(final @NotNull ICurrentDateProvider timeProvider, final long wa
public boolean checkForDebounce() {
final long now = timeProvider.getCurrentTimeMillis();
if (lastExecutionTime == null || (lastExecutionTime + waitTimeMs) <= now) {
executions = 0;
lastExecutionTime = now;
return false;
}
executions++;
if (executions < maxExecutions) {
return false;
}
executions = 0;
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,13 @@ class ScreenshotEventProcessorTest {
val event = SentryEvent().apply {
exceptions = listOf(SentryException())
}
val hint0 = Hint()
var hint0 = Hint()
processor.process(event, hint0)
assertNotNull(hint0.screenshot)
hint0 = Hint()
processor.process(event, hint0)
assertNotNull(hint0.screenshot)
hint0 = Hint()
processor.process(event, hint0)
assertNotNull(hint0.screenshot)

Expand All @@ -236,6 +242,10 @@ class ScreenshotEventProcessorTest {
val hint0 = Hint()
processor.process(event, hint0)
assertFalse(debounceFlag)
processor.process(event, hint0)
assertFalse(debounceFlag)
processor.process(event, hint0)
assertFalse(debounceFlag)

val hint1 = Hint()
processor.process(event, hint1)
Expand All @@ -256,6 +266,8 @@ class ScreenshotEventProcessorTest {
}
val hint0 = Hint()
processor.process(event, hint0)
processor.process(event, hint0)
processor.process(event, hint0)
assertNotNull(hint0.screenshot)

val hint1 = Hint()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,13 @@ class ViewHierarchyEventProcessorTest {
val event = SentryEvent().apply {
exceptions = listOf(SentryException())
}
val hint0 = Hint()
var hint0 = Hint()
processor.process(event, hint0)
assertNotNull(hint0.viewHierarchy)
hint0 = Hint()
processor.process(event, hint0)
assertNotNull(hint0.viewHierarchy)
hint0 = Hint()
processor.process(event, hint0)
assertNotNull(hint0.viewHierarchy)

Expand All @@ -345,6 +351,10 @@ class ViewHierarchyEventProcessorTest {
val hint0 = Hint()
processor.process(event, hint0)
assertFalse(debounceFlag)
processor.process(event, hint0)
assertFalse(debounceFlag)
processor.process(event, hint0)
assertFalse(debounceFlag)

val hint1 = Hint()
processor.process(event, hint1)
Expand All @@ -363,6 +373,8 @@ class ViewHierarchyEventProcessorTest {
}
val hint0 = Hint()
processor.process(event, hint0)
processor.process(event, hint0)
processor.process(event, hint0)
assertNotNull(hint0.viewHierarchy)

val hint1 = Hint()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ class DebouncerTest {

override fun getCurrentTimeMillis(): Long = currentTimeMs

fun getDebouncer(waitTimeMs: Long = 3000): Debouncer {
return Debouncer(this, waitTimeMs)
fun getDebouncer(waitTimeMs: Long = 3000, maxExecutions: Int = 1): Debouncer {
return Debouncer(this, waitTimeMs, maxExecutions)
}
}

Expand Down Expand Up @@ -59,4 +59,48 @@ class DebouncerTest {
fixture.currentTimeMs = 4000
assertFalse(debouncer.checkForDebounce())
}

@Test
fun `Debouncer maxExecutions is always greater than 0`() {
fixture.currentTimeMs = 1000
val debouncer = fixture.getDebouncer(3000, -1)
assertFalse(debouncer.checkForDebounce())
assertTrue(debouncer.checkForDebounce())
}

@Test
fun `Debouncer should signal debounce after maxExecutions calls`() {
fixture.currentTimeMs = 1000
val debouncer = fixture.getDebouncer(3000, 3)
assertFalse(debouncer.checkForDebounce())
assertFalse(debouncer.checkForDebounce())
assertFalse(debouncer.checkForDebounce())
assertTrue(debouncer.checkForDebounce())
}

@Test
fun `Debouncer maxExecutions counter resets if the other invocation is late enough`() {
fixture.currentTimeMs = 1000
val debouncer = fixture.getDebouncer(3000, 3)
assertFalse(debouncer.checkForDebounce())
assertFalse(debouncer.checkForDebounce())

// After waitTimeMs passes, the maxExecutions counter is reset
fixture.currentTimeMs = 4000
assertFalse(debouncer.checkForDebounce())
assertFalse(debouncer.checkForDebounce())
assertFalse(debouncer.checkForDebounce())
assertTrue(debouncer.checkForDebounce())
}

@Test
fun `Debouncer maxExecutions counter resets after maxExecutions`() {
fixture.currentTimeMs = 1000
val debouncer = fixture.getDebouncer(3000, 3)
assertFalse(debouncer.checkForDebounce())
assertFalse(debouncer.checkForDebounce())
assertFalse(debouncer.checkForDebounce())
assertTrue(debouncer.checkForDebounce())
assertFalse(debouncer.checkForDebounce())
}
}
10 changes: 10 additions & 0 deletions sentry/src/test/java/io/sentry/NoOpHubTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,14 @@ class NoOpHubTest {

@Test
fun `reportFullyDrawn doesnt throw`() = sut.reportFullyDisplayed()

@Test
fun `getBaggage returns null`() {
assertNull(sut.baggage)
}

@Test
fun `captureCheckIn returns empty id`() {
assertEquals(SentryId.EMPTY_ID, sut.captureCheckIn(mock()))
}
}
5 changes: 5 additions & 0 deletions sentry/src/test/java/io/sentry/NoOpSentryClientTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,9 @@ class NoOpSentryClientTest {
@Test
fun `captureTransaction returns empty SentryId`() =
assertEquals(SentryId.EMPTY_ID, sut.captureTransaction(mock(), mock()))

@Test
fun `captureCheckIn returns empty id`() {
assertEquals(SentryId.EMPTY_ID, sut.captureCheckIn(mock(), mock(), mock()))
}
}