diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f7fa0aae2e..f8f0c0c1ad8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ ### Fixes - Cache network capabilities and status to reduce IPC calls ([#4560](https://github.com/getsentry/sentry-java/pull/4560)) +- Deduplicate battery breadcrumbs ([#4561](https://github.com/getsentry/sentry-java/pull/4561)) + ## 8.18.0 diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java index 27dcbbbd725..f4dd4fbc94f 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java @@ -71,6 +71,8 @@ public final class SystemEventsBreadcrumbsIntegration implements Integration, Cl private volatile boolean isStopped = false; private volatile IntentFilter filter = null; private final @NotNull AutoClosableReentrantLock receiverLock = new AutoClosableReentrantLock(); + // Track previous battery state to avoid duplicate breadcrumbs when values haven't changed + private @Nullable BatteryState previousBatteryState; public SystemEventsBreadcrumbsIntegration(final @NotNull Context context) { this(context, getDefaultActionsInternal()); @@ -331,7 +333,7 @@ public void onStop(@NonNull LifecycleOwner owner) { } } - static final class SystemEventsBroadcastReceiver extends BroadcastReceiver { + final class SystemEventsBroadcastReceiver extends BroadcastReceiver { private static final long DEBOUNCE_WAIT_TIME_MS = 60 * 1000; private final @NotNull IScopes scopes; @@ -350,19 +352,36 @@ public void onReceive(final Context context, final @NotNull Intent intent) { final @Nullable String action = intent.getAction(); final boolean isBatteryChanged = ACTION_BATTERY_CHANGED.equals(action); - // aligning with iOS which only captures battery status changes every minute at maximum - if (isBatteryChanged && batteryChangedDebouncer.checkForDebounce()) { - return; + @Nullable BatteryState batteryState = null; + if (isBatteryChanged) { + if (batteryChangedDebouncer.checkForDebounce()) { + // aligning with iOS which only captures battery status changes every minute at maximum + return; + } + + // For battery changes, check if the actual values have changed + final @Nullable Float batteryLevel = DeviceInfoUtil.getBatteryLevel(intent, options); + final @Nullable Integer currentBatteryLevel = + batteryLevel != null ? batteryLevel.intValue() : null; + final @Nullable Boolean currentChargingState = DeviceInfoUtil.isCharging(intent, options); + batteryState = new BatteryState(currentBatteryLevel, currentChargingState); + + // Only create breadcrumb if battery state has actually changed + if (batteryState.equals(previousBatteryState)) { + return; + } + + previousBatteryState = batteryState; } + final BatteryState state = batteryState; final long now = System.currentTimeMillis(); try { options .getExecutorService() .submit( () -> { - final Breadcrumb breadcrumb = - createBreadcrumb(now, intent, action, isBatteryChanged); + final Breadcrumb breadcrumb = createBreadcrumb(now, intent, action, state); final Hint hint = new Hint(); hint.set(ANDROID_INTENT, intent); scopes.addBreadcrumb(breadcrumb, hint); @@ -411,7 +430,7 @@ String getStringAfterDotFast(final @Nullable String str) { final long timeMs, final @NotNull Intent intent, final @Nullable String action, - boolean isBatteryChanged) { + final @Nullable BatteryState batteryState) { final Breadcrumb breadcrumb = new Breadcrumb(timeMs); breadcrumb.setType("system"); breadcrumb.setCategory("device.event"); @@ -420,14 +439,12 @@ String getStringAfterDotFast(final @Nullable String str) { breadcrumb.setData("action", shortAction); } - if (isBatteryChanged) { - final Float batteryLevel = DeviceInfoUtil.getBatteryLevel(intent, options); - if (batteryLevel != null) { - breadcrumb.setData("level", batteryLevel); + if (batteryState != null) { + if (batteryState.level != null) { + breadcrumb.setData("level", batteryState.level); } - final Boolean isCharging = DeviceInfoUtil.isCharging(intent, options); - if (isCharging != null) { - breadcrumb.setData("charging", isCharging); + if (batteryState.charging != null) { + breadcrumb.setData("charging", batteryState.charging); } } else { final Bundle extras = intent.getExtras(); @@ -458,4 +475,26 @@ String getStringAfterDotFast(final @Nullable String str) { return breadcrumb; } } + + static final class BatteryState { + private final @Nullable Integer level; + private final @Nullable Boolean charging; + + BatteryState(final @Nullable Integer level, final @Nullable Boolean charging) { + this.level = level; + this.charging = charging; + } + + @Override + public boolean equals(final @Nullable Object other) { + if (!(other instanceof BatteryState)) return false; + BatteryState that = (BatteryState) other; + return Objects.equals(level, that.level) && Objects.equals(charging, that.charging); + } + + @Override + public int hashCode() { + return Objects.hash(level, charging); + } + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt index 99a80f361c5..650c36868ba 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt @@ -157,7 +157,7 @@ class SystemEventsBreadcrumbsIntegrationTest { assertEquals("device.event", it.category) assertEquals("system", it.type) assertEquals(SentryLevel.INFO, it.level) - assertEquals(it.data["level"], 75f) + assertEquals(it.data["level"], 75) assertEquals(it.data["charging"], true) }, anyOrNull(), @@ -189,7 +189,7 @@ class SystemEventsBreadcrumbsIntegrationTest { verify(fixture.scopes) .addBreadcrumb( check { - assertEquals(it.data["level"], 80f) + assertEquals(it.data["level"], 80) assertEquals(it.data["charging"], false) }, anyOrNull(), @@ -197,6 +197,83 @@ class SystemEventsBreadcrumbsIntegrationTest { verifyNoMoreInteractions(fixture.scopes) } + @Test + fun `battery changes with identical values do not generate breadcrumbs`() { + val sut = fixture.getSut() + sut.register(fixture.scopes, fixture.options) + + val intent1 = + Intent().apply { + action = Intent.ACTION_BATTERY_CHANGED + putExtra(BatteryManager.EXTRA_LEVEL, 80) + putExtra(BatteryManager.EXTRA_SCALE, 100) + putExtra(BatteryManager.EXTRA_PLUGGED, BatteryManager.BATTERY_PLUGGED_USB) + } + val intent2 = + Intent().apply { + action = Intent.ACTION_BATTERY_CHANGED + putExtra(BatteryManager.EXTRA_LEVEL, 80) + putExtra(BatteryManager.EXTRA_SCALE, 100) + putExtra(BatteryManager.EXTRA_PLUGGED, BatteryManager.BATTERY_PLUGGED_USB) + } + + // Receive first battery change + sut.receiver!!.onReceive(fixture.context, intent1) + + // Receive second battery change with identical values + sut.receiver!!.onReceive(fixture.context, intent2) + + // should only add the first crumb since values are identical + verify(fixture.scopes) + .addBreadcrumb( + check { + assertEquals(it.data["level"], 80) + assertEquals(it.data["charging"], true) + }, + anyOrNull(), + ) + verifyNoMoreInteractions(fixture.scopes) + } + + @Test + fun `battery changes with minor level differences do not generate breadcrumbs`() { + val sut = fixture.getSut() + sut.register(fixture.scopes, fixture.options) + + val intent1 = + Intent().apply { + action = Intent.ACTION_BATTERY_CHANGED + putExtra(BatteryManager.EXTRA_LEVEL, 80001) // 80.001% + putExtra(BatteryManager.EXTRA_SCALE, 100000) + putExtra(BatteryManager.EXTRA_PLUGGED, BatteryManager.BATTERY_PLUGGED_USB) + } + val intent2 = + Intent().apply { + action = Intent.ACTION_BATTERY_CHANGED + putExtra(BatteryManager.EXTRA_LEVEL, 80002) // 80.002% + putExtra(BatteryManager.EXTRA_SCALE, 100000) + putExtra(BatteryManager.EXTRA_PLUGGED, BatteryManager.BATTERY_PLUGGED_USB) + } + + // Receive first battery change + sut.receiver!!.onReceive(fixture.context, intent1) + + // Receive second battery change with very minor level difference (rounds to same 3 decimal + // places) + sut.receiver!!.onReceive(fixture.context, intent2) + + // should only add the first crumb since both round to 80.000% + verify(fixture.scopes) + .addBreadcrumb( + check { + assertEquals(it.data["level"], 80) + assertEquals(it.data["charging"], true) + }, + anyOrNull(), + ) + verifyNoMoreInteractions(fixture.scopes) + } + @Test fun `Do not crash if registerReceiver throws exception`() { val sut = fixture.getSut()