fix: [SDK-4822] make ANR watchdog app-state aware to stop false background ANRs#2676
fix: [SDK-4822] make ANR watchdog app-state aware to stop false background ANRs#2676abdulraqeeb33 wants to merge 5 commits into
Conversation
…round ANRs The ANR watchdog monitored the main thread continuously with no foreground/ background awareness, so backgrounded main-thread blocks were reported as ANRs. Android only raises real ANRs in the foreground, and background wall-clock measurement is unreliable under Doze/cached-process freezing, so these were false positives that inflated the ANR metric. Detection is now classified by app state: - Foreground block > 5s: real ANR (unchanged crash-class report). - Background block > 10s: warning-class log, kept out of the ANR metric. - Watchdog thread oversleeping beyond the interval: treated as a frozen process and suppressed. Foreground state is derived from the existing platformProvider.appState, and the decision is extracted into a pure classifyBlock() for deterministic tests. Co-authored-by: Cursor <cursoragent@cursor.com>
The watchdog measured all durations (lastResponseTime heartbeat, sleep timing, unresponsiveness, and the report dedup window) with wall-clock System.currentTimeMillis(). A clock adjustment mid-session (NTP sync, manual time change, DST) would skew those deltas — a forward jump fabricates a block and fires a false ANR; a backward jump corrupts the dedup window. Switch the interval math to the monotonic SystemClock.uptimeMillis(), which is immune to clock changes, matches the clock the main Looper schedules with, and pauses during deep sleep so a dozing device accumulates no phantom block time. Also reset the baseline in start() so a gap before start() can't read as a block. Co-authored-by: Cursor <cursoragent@cursor.com>
…NR dedup Add an injectable monotonic clock and split the per-iteration evaluation (recordHeartbeat + evaluateCheck) out of the thread/sleep loop, so the full heartbeat -> clock -> classify -> report path is exercised deterministically with a fake clock — no background thread, real sleep, or shadow-clock racing. Writing those tests surfaced a real bug: the report dedup compared now - lastAnrReportTime against a 0 sentinel, so within the first 30s after device boot (small uptimeMillis) the first genuine ANR would be suppressed. Treat the sentinel explicitly as "never reported" so the first block always reports regardless of how small the clock is. Co-authored-by: Cursor <cursoragent@cursor.com>
📊 Diff Coverage ReportDiff Coverage Report (Changed Lines Only)Gate: aggregate coverage on changed executable lines must be ≥ 80% (JaCoCo line data for lines touched in the diff). Changed Files Coverage
Overall (aggregate gate)128/144 touched executable lines covered (88.9% — requires ≥ 80%) Per-file detail (informational; gate is aggregate above):
|
|
One concern: |
…lock stream Extract the watchdog's timing/classification/dedup into a pure, Android-free AnrCheckEvaluator and abstract the Android touch points (main-thread Handler, stack capture, monotonic clock) behind an injectable AnrWatchdogPlatform seam. This lets the whole detector run under plain-JVM tests instead of Robolectric, so JaCoCo actually measures it (the shell went from 0% to ~73%, evaluator 100%, aggregate diff coverage ~84%). Behavior changes: - Foreground blocks past the ANR threshold stay crash-class ANRs; background blocks past the (higher) background threshold are recorded under a distinct BackgroundMainThreadBlockException via the retained, disk-buffered crash path, so they stay out of the ANR metric but remain queryable (app.state + SDK version are auto-attached; the message carries a top-frame/OneSignal-frame fingerprint for triage). - Foreground and background reports now dedup independently, so a stream of background warnings can never suppress a real foreground ANR. - Frozen-process guard and monotonic clock retained; post-boot dedup sentinel keeps the first report from being swallowed near t=0. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Thanks Fadi — both good catches, and I reworked the PR around them. 1. Shared dedup → fixed. Foreground and background now keep independent dedup timestamps ( 2. Non-foreground ANR-relevant work → we no longer drop it. You're right that Android can count some non-foreground work (broadcast Also worth noting: this pass makes the detector JVM-testable (pure |
|
Thanks for the update. The independent foreground/background dedupe addresses my main concern. One follow-up: I’d also still want product/Android validation on the app-state rule. The new background stream preserves visibility, but the SDK still downgrades solely on |
Background blocks were routed through crashReporter.saveCrash, which hardcodes Severity.FATAL — so on the wire a backgrounded block was byte-for-byte a fatal crash, distinguished only by exception.type. Keeping it out of the ANR/crash metric relied entirely on the backend special-casing that classname, an unverified and fragile contract. Add an explicit non-fatal path to the crash reporter and route background blocks through it: - IOtelCrashReporter.saveNonFatal emits on the same retained, disk-buffered crash telemetry but at Severity.WARN. - Records now carry an SDK-owned ossdk.crash.fatal attribute (true for crashes/foreground ANRs, false for background blocks), so the backend can segment on a stable, intentional signal instead of severity or exception.type alone. - OtelAnrDetector.reportBackgroundBlock now calls saveNonFatal; reportAnr and the uncaught-exception handler stay on saveCrash (FATAL). Co-authored-by: Cursor <cursoragent@cursor.com>
|
Good catch — you were right, the FATAL severity wasn't enforced anywhere. Fixed in 08cf970. Background blocks now go through a new One thing I confirmed while in there: crash reports and normal remote logs go through the same network exporter — there's no separate "crash" channel, so the metric is a query over severity/attributes. WARN + the flag should drop these out cleanly, but if you can confirm the metric keys on one of those I'll call it closed. Still want your/product read on the app-state rule. We use a flat 5s foreground / 10s background threshold, not Android's per-context timeouts (broadcast 10s, service 20/200s) — so a backgrounded broadcast/FGS main-thread hang that's technically ANR-relevant would land as a non-fatal background block. Upside: it's retained and queryable, so we can promote specific paths to ANR-class once the data shows real user impact instead of guessing now. OK to ship that as the default? |
|
One API-compat concern: |
Yeah this is internal only and currently used only by Android SDK. |
There was a problem hiding this comment.
LGTM, just calling out that (currently) making background ANRs WARN-level effectively means we're not going to capture them in telemetry to the same extent anymore, because we have so few apps on log level WARN. It may still be enough to actually get usable metrics. And if we were to bump the loglevel to WARN for more apps, the volume would be too great. These would be meaningfully captured if we do something different with our logging in the future.
3 participants
Summary
The Otel ANR watchdog (
OtelAnrDetector) monitored the main thread continuously with no awareness of foreground/background, so any backgrounded main-thread block was reported as a crash-class ANR. A pull of ~25k recent ANRs showed ~87.5% are backgrounded main-thread, and the long-duration reports (median block ~6 min, up to ~25h) are impossible as real hangs — they are freeze/suspension artifacts. The net effect was an ANR metric inflated well beyond what Android actually counts (user-perceived ANRs are foreground input-dispatch timeouts).Approach
Detection is now app-state aware, and background blocks are downgraded but not discarded:
5s→ crash-class ANR (unchanged).10s→ recorded via a distinctBackgroundMainThreadBlockExceptionon the same retained, disk-buffered crash path, so it stays out of the ANR/crash metric but remains queryable.app.state+ SDK version are auto-attached, and the exception message carries a compact fingerprint (top frame + first OneSignal frame) for triage. This preserves visibility into real background regressions (broadcastonReceive,JobScheduler, main-thread SQLite, lock/binder waits) instead of dropping them.SystemClock.uptimeMillis) instead of wall-clock, so NTP / DST / manual clock changes and deep sleep can't fabricate block time.Foreground state is derived from the existing
platformProvider.appState;"unknown"is treated as foreground so a genuine ANR is never silently downgraded.Testability & coverage
AnrCheckEvaluator(100% JVM-covered).Handler, stack capture, monotonic clock) abstracted behind an injectableAnrWatchdogPlatformseam so the whole detector runs under plain-JVM tests instead of Robolectric — which is what lets JaCoCo actually measure it. The shell went from 0% → ~73%, and aggregate diff coverage is ~84% (was 0%, which was failing the gate).Test plan
./gradlew :OneSignal:core:testDebugUnitTest— all passreportAnr(OneSignal-at-fault / not), background-warning path, start/stop lifecycle, unknown-state → foregroundCloses SDK-4822.
Made with Cursor