ADFA-2314 | Enrich Sentry reports with diagnostic context#1468
Conversation
Register a single Sentry EventProcessor that attaches app-specific diagnostics to every event (fatal crashes, caught exceptions and plugin crashes alike): - SELinux contexts (process, private-data-dir file label, seinfo) - boot mode (direct-boot vs credential-unlocked) + locked duration - install location (internal vs external) - signing certificate SHA-256 digest + official-build flag - active plugins (id, version, minIdeVersion, recent crash count) - release name/code + git commit, process ABI/arch, device posture Each field is collected inside its own runCatching so a single failing collector (e.g. an SELinux denial, or credential storage being inaccessible in direct boot) drops only that field and the event is still reported. Running at capture time means the processor sees live boot state and the currently loaded plugins with no per-call-site wiring. Only the SELinux file label is stored, never any path or source. Wiring: install() in SentryAndroid.init; onUserUnlocked() in the existing ACTION_USER_UNLOCKED receiver. Adds a Robolectric test proving enrich populates the event and that a throwing collector never breaks the rest of the report.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
📝 Walkthrough
WalkthroughIntroduces a ChangesSentry Diagnostics Context
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Loader as DeviceProtectedApplicationLoader
participant App as IDEApplication
participant Diag as SentryDiagnosticsContext
participant Sentry as Sentry SDK
Loader->>Sentry: SentryAndroid.init(options)
Loader->>Diag: install(options)
Diag->>Sentry: register EventProcessor
App->>App: onReceive(ACTION_USER_UNLOCKED)
App->>Diag: onUserUnlocked()
Diag->>Diag: record unlockElapsedMs
Sentry->>Diag: process(event)
Diag->>Diag: enrich(event) - selinux, boot mode, plugins, version, posture
Diag-->>Sentry: enriched event
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/src/main/java/com/itsaky/androidide/handlers/SentryDiagnosticsContext.kt (1)
184-186: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCache device posture like the other static values.
DeviceUtils.isEmulator()/isDeviceRooted()are recomputed on every enriched event.isDeviceRooted()loops over ~11 hardcoded paths doingFile(...).exists()I/O each call — this doesn't change during the process lifetime, so it should be memoized likeappInfo/seInfo/signingDigest/installLocationrather than re-run per event, especially sinceenrich()may run on the crashing thread for fatal events.⚡ Proposed fix
+ private val isEmulator: Boolean by lazy { DeviceUtils.isEmulator() } + private val isRooted: Boolean by lazy { DeviceUtils.isDeviceRooted() } + // H — device posture (emulator vs physical, rooted). - tag(event, "device_emulator") { DeviceUtils.isEmulator().toString() } - tag(event, "device_rooted") { DeviceUtils.isDeviceRooted().toString() } + tag(event, "device_emulator") { isEmulator.toString() } + tag(event, "device_rooted") { isRooted.toString() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/itsaky/androidide/handlers/SentryDiagnosticsContext.kt` around lines 184 - 186, Cache the device posture values in SentryDiagnosticsContext the same way as appInfo, seInfo, signingDigest, and installLocation so they are computed once per process instead of on every enrich() call. Update the enrich() tagging for device_emulator and device_rooted to read from memoized values, and keep the caching near the existing static diagnostics fields so DeviceUtils.isEmulator() and DeviceUtils.isDeviceRooted() are not re-run per event.app/src/test/java/com/itsaky/androidide/handlers/SentryDiagnosticsContextTest.kt (1)
69-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSimplify processor lookup.
Since a fresh
SentryOptions()has no default processors,options.eventProcessors.single()is simpler than matching on the anonymous class name, and doubles as an assertion thatinstall()registers exactly one processor.🧹 Proposed simplification
private fun enrichNewEvent(): SentryEvent { val options = SentryOptions() SentryDiagnosticsContext.install(options) - val processor = options.eventProcessors - .first { it.javaClass.name.contains("SentryDiagnosticsContext") } + val processor = options.eventProcessors.single() return processor.process(SentryEvent(), Hint())!! }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/test/java/com/itsaky/androidide/handlers/SentryDiagnosticsContextTest.kt` around lines 69 - 75, Simplify the processor lookup in enrichNewEvent() by using the single processor registered on SentryOptions after SentryDiagnosticsContext.install(options) instead of searching by anonymous class name. This should rely on options.eventProcessors.single() so the test both locates the processor and asserts that install() registers exactly one processor.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/handlers/SentryDiagnosticsContext.kt`:
- Around line 184-186: Cache the device posture values in
SentryDiagnosticsContext the same way as appInfo, seInfo, signingDigest, and
installLocation so they are computed once per process instead of on every
enrich() call. Update the enrich() tagging for device_emulator and device_rooted
to read from memoized values, and keep the caching near the existing static
diagnostics fields so DeviceUtils.isEmulator() and DeviceUtils.isDeviceRooted()
are not re-run per event.
In
`@app/src/test/java/com/itsaky/androidide/handlers/SentryDiagnosticsContextTest.kt`:
- Around line 69-75: Simplify the processor lookup in enrichNewEvent() by using
the single processor registered on SentryOptions after
SentryDiagnosticsContext.install(options) instead of searching by anonymous
class name. This should rely on options.eventProcessors.single() so the test
both locates the processor and asserts that install() registers exactly one
processor.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a88af3c6-7fc8-4df2-98b5-7db70aecd309
📒 Files selected for processing (4)
app/src/main/java/com/itsaky/androidide/app/DeviceProtectedApplicationLoader.ktapp/src/main/java/com/itsaky/androidide/app/IDEApplication.ktapp/src/main/java/com/itsaky/androidide/handlers/SentryDiagnosticsContext.ktapp/src/test/java/com/itsaky/androidide/handlers/SentryDiagnosticsContextTest.kt
2 participants
Summary
Sentry crash reports were missing app-specific diagnostic info needed to reproduce and triage bugs without round-tripping the reporter (ADFA-2285, the SELinux-mislabel-on-update issue, is the motivating example).
This wires a single Sentry
EventProcessor, registered inSentryAndroid.init, that enriches every event — fatal crashes, caught exceptions (ReportCaughtExceptionEvent) and plugin crashes alike — at capture time. Running at capture time (rather than a one-shotconfigureScopeat startup) means it sees live boot state and the currently loaded plugins, with no per-call-site wiring.Nearly every field already had a helper in the codebase, so this is primarily a wiring task.
Fields attached
direct_bootvscredential_unlocked, queried live, plusboot_locked_duration_mswhen the process started lockedReliability & PII
runCatching, so a single failing collector (an SELinux denial, or credential storage being inaccessible in direct boot) drops only that field — the event is still reported with everything else intact.Wiring (2 edits)
DeviceProtectedApplicationLoader.kt—SentryDiagnosticsContext.install(options)inside the existingSentryAndroid.initblock.IDEApplication.kt—SentryDiagnosticsContext.onUserUnlocked()in the existingACTION_USER_UNLOCKEDreceiver (no new receiver needed).No changes to
CrashEventSubscriber,handleUncaughtException, orhandlePluginCrash— the processor enriches those events automatically.Implementation note
Sentry's
Contextstype is not aMap, so context groups are attached viaevent.contexts.put(...).EventProcessorhas onlydefaultmethods (not SAM-convertible), so it's implemented as an anonymous object rather than a lambda.Testing
:app:compileV8DebugKotlin— clean:app:testV8DebugUnitTest— newSentryDiagnosticsContextTest(Robolectric + mockk), 2 tests, 0 failures:🤖 Generated with Claude Code