Replace ANR profiling boolean flag with sample-rate#5156
Replace ANR profiling boolean flag with sample-rate#5156markushi wants to merge 1 commit intomarkushi/feat/anr-profilingfrom
Conversation
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog. This PR will not appear in the changelog. 🤖 This preview updates automatically when you update the PR. |
| } | ||
|
|
||
| if (options.isEnableAnrProfiling() && hasOnlySystemFrames(event)) { | ||
| if (options.isAnrProfilingEnabled() && hasOnlySystemFrames(event)) { |
There was a problem hiding this comment.
Bug: Unsampled ANR events are incorrectly fingerprinted as "system-frames-only-anr" because the check isAnrProfilingEnabled() doesn't confirm if the specific event was actually profiled.
Severity: MEDIUM
Suggested Fix
The setDefaultAnrFingerprint() method should only apply the "system-frames-only-anr" fingerprint if the ANR event was actually profiled. This can be achieved by having applyAnrProfile() set a flag on the event to indicate a profile was applied. The fingerprinting logic should then check this flag instead of relying on the general isAnrProfilingEnabled() setting.
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/ApplicationExitInfoEventProcessor.java#L737
Potential issue: When ANR profiling is enabled with a sample rate less than 1.0, ANR
events that are not sampled are incorrectly fingerprinted. The logic in
`setDefaultAnrFingerprint` checks if profiling is enabled via `isAnrProfilingEnabled()`,
but does not check if the specific ANR event was actually profiled. If an unsampled ANR
event happens to only contain system frames, it will be assigned the
`"system-frames-only-anr"` fingerprint. This fingerprint is intended for cases where
profiling was performed but found no application frames, leading to incorrect issue
grouping in Sentry for unsampled ANRs.
Did we get this right? 👍 / 👎 to inform future reviews.
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 83a9ec4 | 333.84 ms | 390.30 ms | 56.47 ms |
| 00299fd | 359.87 ms | 424.85 ms | 64.98 ms |
| eb02e45 | 362.67 ms | 431.71 ms | 69.04 ms |
| edecf31 | 310.80 ms | 367.02 ms | 56.22 ms |
| 8c4e7d6 | 309.15 ms | 359.27 ms | 50.12 ms |
| 8d7c3bb | 304.55 ms | 377.88 ms | 73.33 ms |
| 4c0ffee | 314.94 ms | 377.79 ms | 62.86 ms |
| ddbbe91 | 289.51 ms | 359.74 ms | 70.23 ms |
| fca8df8 | 326.79 ms | 379.69 ms | 52.90 ms |
| c10e603 | 367.92 ms | 393.50 ms | 25.58 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 83a9ec4 | 1.58 MiB | 2.29 MiB | 723.99 KiB |
| 00299fd | 1.58 MiB | 2.29 MiB | 723.50 KiB |
| eb02e45 | 1.58 MiB | 2.29 MiB | 725.26 KiB |
| edecf31 | 1.58 MiB | 2.29 MiB | 726.95 KiB |
| 8c4e7d6 | 1.58 MiB | 2.29 MiB | 727.03 KiB |
| 8d7c3bb | 1.58 MiB | 2.29 MiB | 727.01 KiB |
| 4c0ffee | 1.58 MiB | 2.29 MiB | 723.67 KiB |
| ddbbe91 | 1.58 MiB | 2.29 MiB | 724.15 KiB |
| fca8df8 | 1.58 MiB | 2.29 MiB | 723.68 KiB |
| c10e603 | 1.58 MiB | 2.29 MiB | 723.72 KiB |
📜 Description
Replaces the boolean flag with a sample-rate.
#skip-changelog