-
-
Notifications
You must be signed in to change notification settings - Fork 465
feat(anr): Profile main thread when ANR and report ANR profiles to Sentry #4899
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
base: main
Are you sure you want to change the base?
Conversation
|
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java
Outdated
Show resolved
Hide resolved
sentry-android-core/src/main/java/io/sentry/android/core/anr/AnrProfileManager.java
Show resolved
Hide resolved
sentry-android-core/src/main/java/io/sentry/android/core/anr/AnrCulpritIdentifier.java
Show resolved
Hide resolved
sentry-android-core/src/main/java/io/sentry/android/core/anr/AggregatedStackTrace.java
Outdated
Show resolved
Hide resolved
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3d205d0 | 352.15 ms | 432.53 ms | 80.38 ms |
| fc5ccaf | 256.80 ms | 322.36 ms | 65.56 ms |
| ee747ae | 374.71 ms | 455.18 ms | 80.47 ms |
| 6edfca2 | 314.02 ms | 383.20 ms | 69.18 ms |
| 674d437 | 355.28 ms | 504.18 ms | 148.90 ms |
| 6405ec5 | 310.88 ms | 354.56 ms | 43.69 ms |
| bbc35bb | 324.88 ms | 425.73 ms | 100.85 ms |
| dba088c | 321.78 ms | 364.59 ms | 42.82 ms |
| fc5ccaf | 279.11 ms | 353.34 ms | 74.23 ms |
| 3699cd5 | 423.60 ms | 495.52 ms | 71.92 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3d205d0 | 1.58 MiB | 2.10 MiB | 532.97 KiB |
| fc5ccaf | 1.58 MiB | 2.13 MiB | 557.54 KiB |
| ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
| 6edfca2 | 1.58 MiB | 2.13 MiB | 559.07 KiB |
| 674d437 | 1.58 MiB | 2.10 MiB | 530.94 KiB |
| 6405ec5 | 1.58 MiB | 2.12 MiB | 552.23 KiB |
| bbc35bb | 1.58 MiB | 2.12 MiB | 553.01 KiB |
| dba088c | 1.58 MiB | 2.13 MiB | 558.99 KiB |
| fc5ccaf | 1.58 MiB | 2.13 MiB | 557.54 KiB |
| 3699cd5 | 1.58 MiB | 2.10 MiB | 533.45 KiB |
Previous results on branch: markushi/feat/anr-profiling
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| be4960b | 322.67 ms | 380.85 ms | 58.18 ms |
| 184b846 | 276.09 ms | 351.65 ms | 75.56 ms |
| b238cff | 332.00 ms | 401.47 ms | 69.47 ms |
| c22421f | 331.24 ms | 367.91 ms | 36.67 ms |
| a4af52f | 304.52 ms | 364.92 ms | 60.40 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| be4960b | 1.58 MiB | 2.13 MiB | 562.64 KiB |
| 184b846 | 1.58 MiB | 2.13 MiB | 558.70 KiB |
| b238cff | 1.58 MiB | 2.13 MiB | 562.66 KiB |
| c22421f | 1.58 MiB | 2.13 MiB | 563.53 KiB |
| a4af52f | 1.58 MiB | 2.13 MiB | 563.53 KiB |
sentry-android-core/src/main/java/io/sentry/android/core/anr/AnrCulpritIdentifier.java
Show resolved
Hide resolved
…ntry-java into markushi/feat/anr-profiling
sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java
Outdated
Show resolved
Hide resolved
sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java
Show resolved
Hide resolved
sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java
Show resolved
Hide resolved
sentry-android-core/src/main/java/io/sentry/android/core/anr/AnrStackTrace.java
Outdated
Show resolved
Hide resolved
sentry-android-core/src/main/java/io/sentry/android/core/anr/AnrProfilingIntegration.java
Show resolved
Hide resolved
|
@sentry review |
sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java
Show resolved
Hide resolved
| // into the same issue | ||
| event.setFingerprints( | ||
| Arrays.asList( | ||
| "{{ system-frames-only-anr }}", | ||
| isBackground ? "background-anr" : "foreground-anr")); | ||
| } | ||
| } | ||
|
|
||
| final @NotNull SentryId sentryId = scopes.captureEvent(event, hint); | ||
| final boolean isEventDropped = sentryId.equals(SentryId.EMPTY_ID); | ||
| if (!isEventDropped) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: When ANR profiling fails, hasOnlySystemFrames() incorrectly returns true because no exceptions are added, causing a "system-frames-only-anr" fingerprint to be applied to events with unverified frames.
Severity: HIGH | Confidence: High
🔍 Detailed Analysis
When ANR profiling is enabled but fails to apply a profile for any reason (e.g., missing file, timestamp mismatch), the event will have no exceptions. The hasOnlySystemFrames() method incorrectly returns true when the exception list is null. As a result, a special "system-frames-only-anr" fingerprint is applied to the event, even though it contains a valid thread dump with potentially non-system frames. This leads to incorrect event grouping, merging diverse ANRs into a single issue and obscuring their true root causes when profiling fails.
💡 Suggested Fix
Modify the logic to only apply the "system-frames-only-anr" fingerprint when the ANR profile has been successfully applied and exceptions have been added to the event. The hasOnlySystemFrames() method should return false if the exception list is null or empty, ensuring the fingerprint is only applied after frames have been verified.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location:
sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java#L302-L321
Potential issue: When ANR profiling is enabled but fails to apply a profile for any
reason (e.g., missing file, timestamp mismatch), the event will have no exceptions. The
`hasOnlySystemFrames()` method incorrectly returns `true` when the exception list is
`null`. As a result, a special `"system-frames-only-anr"` fingerprint is applied to the
event, even though it contains a valid thread dump with potentially non-system frames.
This leads to incorrect event grouping, merging diverse ANRs into a single issue and
obscuring their true root causes when profiling fails.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7690328
| } | ||
| } | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Fingerprinting applied when no exceptions exist
The hasOnlySystemFrames method returns true when the event has no exceptions (null), treating "no exceptions to check" the same as "only system frames present". When ANR profiling is enabled but no profile matches (e.g., profile file doesn't exist or timestamp mismatch), applyAnrProfile returns without setting exceptions, leaving only threads from the thread dump. The hasOnlySystemFrames check then returns true, causing the "system-frames-only" fingerprint to be applied to all ANR events without matching profiles, regardless of their actual stack content. This could incorrectly group unrelated ANRs into a single issue.
📜 Description
Adds ANR (Application Not Responding) profiling integration that profiles the main thread when an ANR is detected and reports the captured profiles to Sentry.
Key Changes:
AnrProfilingIntegrationto capture profiles during ANR eventsAnrV2Integrationnow takes care of matching and capturing the profile on the next start💡 Motivation and Context
This feature enables better ANR diagnostics by capturing profiling data at the time of ANR detection, allowing developers to identify performance bottlenecks and problematic code paths causing application hangs.
💚 How did you test it?
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps