Skip to content

Commit 270a4c3

Browse files
romtsnadinauer
authored andcommitted
fix(breadcrumbs): Deduplicate battery breadcrumbs (#4561)
1 parent 59250c4 commit 270a4c3

File tree

3 files changed

+134
-16
lines changed

3 files changed

+134
-16
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
### Fixes
1010

1111
- Cache network capabilities and status to reduce IPC calls ([#4560](https://github.com/getsentry/sentry-java/pull/4560))
12+
- Deduplicate battery breadcrumbs ([#4561](https://github.com/getsentry/sentry-java/pull/4561))
13+
1214

1315
## 8.18.0
1416

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

Lines changed: 53 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ public final class SystemEventsBreadcrumbsIntegration implements Integration, Cl
7171
private volatile boolean isStopped = false;
7272
private volatile IntentFilter filter = null;
7373
private final @NotNull AutoClosableReentrantLock receiverLock = new AutoClosableReentrantLock();
74+
// Track previous battery state to avoid duplicate breadcrumbs when values haven't changed
75+
private @Nullable BatteryState previousBatteryState;
7476

7577
public SystemEventsBreadcrumbsIntegration(final @NotNull Context context) {
7678
this(context, getDefaultActionsInternal());
@@ -331,7 +333,7 @@ public void onStop(@NonNull LifecycleOwner owner) {
331333
}
332334
}
333335

334-
static final class SystemEventsBroadcastReceiver extends BroadcastReceiver {
336+
final class SystemEventsBroadcastReceiver extends BroadcastReceiver {
335337

336338
private static final long DEBOUNCE_WAIT_TIME_MS = 60 * 1000;
337339
private final @NotNull IScopes scopes;
@@ -350,19 +352,36 @@ public void onReceive(final Context context, final @NotNull Intent intent) {
350352
final @Nullable String action = intent.getAction();
351353
final boolean isBatteryChanged = ACTION_BATTERY_CHANGED.equals(action);
352354

353-
// aligning with iOS which only captures battery status changes every minute at maximum
354-
if (isBatteryChanged && batteryChangedDebouncer.checkForDebounce()) {
355-
return;
355+
@Nullable BatteryState batteryState = null;
356+
if (isBatteryChanged) {
357+
if (batteryChangedDebouncer.checkForDebounce()) {
358+
// aligning with iOS which only captures battery status changes every minute at maximum
359+
return;
360+
}
361+
362+
// For battery changes, check if the actual values have changed
363+
final @Nullable Float batteryLevel = DeviceInfoUtil.getBatteryLevel(intent, options);
364+
final @Nullable Integer currentBatteryLevel =
365+
batteryLevel != null ? batteryLevel.intValue() : null;
366+
final @Nullable Boolean currentChargingState = DeviceInfoUtil.isCharging(intent, options);
367+
batteryState = new BatteryState(currentBatteryLevel, currentChargingState);
368+
369+
// Only create breadcrumb if battery state has actually changed
370+
if (batteryState.equals(previousBatteryState)) {
371+
return;
372+
}
373+
374+
previousBatteryState = batteryState;
356375
}
357376

377+
final BatteryState state = batteryState;
358378
final long now = System.currentTimeMillis();
359379
try {
360380
options
361381
.getExecutorService()
362382
.submit(
363383
() -> {
364-
final Breadcrumb breadcrumb =
365-
createBreadcrumb(now, intent, action, isBatteryChanged);
384+
final Breadcrumb breadcrumb = createBreadcrumb(now, intent, action, state);
366385
final Hint hint = new Hint();
367386
hint.set(ANDROID_INTENT, intent);
368387
scopes.addBreadcrumb(breadcrumb, hint);
@@ -411,7 +430,7 @@ String getStringAfterDotFast(final @Nullable String str) {
411430
final long timeMs,
412431
final @NotNull Intent intent,
413432
final @Nullable String action,
414-
boolean isBatteryChanged) {
433+
final @Nullable BatteryState batteryState) {
415434
final Breadcrumb breadcrumb = new Breadcrumb(timeMs);
416435
breadcrumb.setType("system");
417436
breadcrumb.setCategory("device.event");
@@ -420,14 +439,12 @@ String getStringAfterDotFast(final @Nullable String str) {
420439
breadcrumb.setData("action", shortAction);
421440
}
422441

423-
if (isBatteryChanged) {
424-
final Float batteryLevel = DeviceInfoUtil.getBatteryLevel(intent, options);
425-
if (batteryLevel != null) {
426-
breadcrumb.setData("level", batteryLevel);
442+
if (batteryState != null) {
443+
if (batteryState.level != null) {
444+
breadcrumb.setData("level", batteryState.level);
427445
}
428-
final Boolean isCharging = DeviceInfoUtil.isCharging(intent, options);
429-
if (isCharging != null) {
430-
breadcrumb.setData("charging", isCharging);
446+
if (batteryState.charging != null) {
447+
breadcrumb.setData("charging", batteryState.charging);
431448
}
432449
} else {
433450
final Bundle extras = intent.getExtras();
@@ -458,4 +475,26 @@ String getStringAfterDotFast(final @Nullable String str) {
458475
return breadcrumb;
459476
}
460477
}
478+
479+
static final class BatteryState {
480+
private final @Nullable Integer level;
481+
private final @Nullable Boolean charging;
482+
483+
BatteryState(final @Nullable Integer level, final @Nullable Boolean charging) {
484+
this.level = level;
485+
this.charging = charging;
486+
}
487+
488+
@Override
489+
public boolean equals(final @Nullable Object other) {
490+
if (!(other instanceof BatteryState)) return false;
491+
BatteryState that = (BatteryState) other;
492+
return Objects.equals(level, that.level) && Objects.equals(charging, that.charging);
493+
}
494+
495+
@Override
496+
public int hashCode() {
497+
return Objects.hash(level, charging);
498+
}
499+
}
461500
}

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

Lines changed: 79 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ class SystemEventsBreadcrumbsIntegrationTest {
157157
assertEquals("device.event", it.category)
158158
assertEquals("system", it.type)
159159
assertEquals(SentryLevel.INFO, it.level)
160-
assertEquals(it.data["level"], 75f)
160+
assertEquals(it.data["level"], 75)
161161
assertEquals(it.data["charging"], true)
162162
},
163163
anyOrNull(),
@@ -189,14 +189,91 @@ class SystemEventsBreadcrumbsIntegrationTest {
189189
verify(fixture.scopes)
190190
.addBreadcrumb(
191191
check<Breadcrumb> {
192-
assertEquals(it.data["level"], 80f)
192+
assertEquals(it.data["level"], 80)
193193
assertEquals(it.data["charging"], false)
194194
},
195195
anyOrNull(),
196196
)
197197
verifyNoMoreInteractions(fixture.scopes)
198198
}
199199

200+
@Test
201+
fun `battery changes with identical values do not generate breadcrumbs`() {
202+
val sut = fixture.getSut()
203+
sut.register(fixture.scopes, fixture.options)
204+
205+
val intent1 =
206+
Intent().apply {
207+
action = Intent.ACTION_BATTERY_CHANGED
208+
putExtra(BatteryManager.EXTRA_LEVEL, 80)
209+
putExtra(BatteryManager.EXTRA_SCALE, 100)
210+
putExtra(BatteryManager.EXTRA_PLUGGED, BatteryManager.BATTERY_PLUGGED_USB)
211+
}
212+
val intent2 =
213+
Intent().apply {
214+
action = Intent.ACTION_BATTERY_CHANGED
215+
putExtra(BatteryManager.EXTRA_LEVEL, 80)
216+
putExtra(BatteryManager.EXTRA_SCALE, 100)
217+
putExtra(BatteryManager.EXTRA_PLUGGED, BatteryManager.BATTERY_PLUGGED_USB)
218+
}
219+
220+
// Receive first battery change
221+
sut.receiver!!.onReceive(fixture.context, intent1)
222+
223+
// Receive second battery change with identical values
224+
sut.receiver!!.onReceive(fixture.context, intent2)
225+
226+
// should only add the first crumb since values are identical
227+
verify(fixture.scopes)
228+
.addBreadcrumb(
229+
check<Breadcrumb> {
230+
assertEquals(it.data["level"], 80)
231+
assertEquals(it.data["charging"], true)
232+
},
233+
anyOrNull(),
234+
)
235+
verifyNoMoreInteractions(fixture.scopes)
236+
}
237+
238+
@Test
239+
fun `battery changes with minor level differences do not generate breadcrumbs`() {
240+
val sut = fixture.getSut()
241+
sut.register(fixture.scopes, fixture.options)
242+
243+
val intent1 =
244+
Intent().apply {
245+
action = Intent.ACTION_BATTERY_CHANGED
246+
putExtra(BatteryManager.EXTRA_LEVEL, 80001) // 80.001%
247+
putExtra(BatteryManager.EXTRA_SCALE, 100000)
248+
putExtra(BatteryManager.EXTRA_PLUGGED, BatteryManager.BATTERY_PLUGGED_USB)
249+
}
250+
val intent2 =
251+
Intent().apply {
252+
action = Intent.ACTION_BATTERY_CHANGED
253+
putExtra(BatteryManager.EXTRA_LEVEL, 80002) // 80.002%
254+
putExtra(BatteryManager.EXTRA_SCALE, 100000)
255+
putExtra(BatteryManager.EXTRA_PLUGGED, BatteryManager.BATTERY_PLUGGED_USB)
256+
}
257+
258+
// Receive first battery change
259+
sut.receiver!!.onReceive(fixture.context, intent1)
260+
261+
// Receive second battery change with very minor level difference (rounds to same 3 decimal
262+
// places)
263+
sut.receiver!!.onReceive(fixture.context, intent2)
264+
265+
// should only add the first crumb since both round to 80.000%
266+
verify(fixture.scopes)
267+
.addBreadcrumb(
268+
check<Breadcrumb> {
269+
assertEquals(it.data["level"], 80)
270+
assertEquals(it.data["charging"], true)
271+
},
272+
anyOrNull(),
273+
)
274+
verifyNoMoreInteractions(fixture.scopes)
275+
}
276+
200277
@Test
201278
fun `Do not crash if registerReceiver throws exception`() {
202279
val sut = fixture.getSut()

0 commit comments

Comments
 (0)