Skip to content
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

Improve ANRv2 implementation based on customer feedback #2792

Merged
merged 7 commits into from
Jun 22, 2023

Conversation

romtsn
Copy link
Member

@romtsn romtsn commented Jun 20, 2023

📜 Description

The PR introduces various improvement to the existing ANRv2 implementation based on customer feedback, namely:

  • Adds a rule to keep ApplicationNotResponding class from obfuscation so it doesn't look ugly in the issue stream
  • Adds an option setReportHistoricalAnrs, when enabled it will report all of the ANRs from the getHistoricalExitReasons list. By default, the SDK only reports and enriches the latest ANR and only this one counts towards ANR rate. Hence, this reduces the noise quite a bit, but still leaves the possibility to get all of the ANR reports from the past. Worth noting that once the SDK has been updated to the version with the new implementation, we won't need this feature, because we'll always read the latest ANR on the next app restart. These ANRs are reported with the HistoricalAppExitInfo mechanism for distinction
  • Adds an option to send the thread dump plain text as an attachment
  • If getTraceInputStream returns null, we do not report an ANR event because it's pretty much useless without the thread dump and makes it look confusing for our customers
  • If we were not able to parse ANR thread dump due to regex issues or some exception, we'll send an event with a message instead, telling customers to use the flag to send raw thread dump and also let us know about this issue
  • Adds null-check for span context values (I wasn't able to reproduce it, but sometimes they get reported as nulls, so this is just defensive here)
  • Enhances regex patterns for native stackframes

The message looks like this:

image

The ANR thread dump attachment looks like this:

image

💡 Motivation and Context

💚 How did you test it?

Manually and automated

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

@github-actions
Copy link
Contributor

github-actions bot commented Jun 20, 2023

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 316.14 ms 357.92 ms 41.78 ms
Size 1.72 MiB 2.28 MiB 573.38 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
496bdfd 301.22 ms 343.96 ms 42.73 ms
8820c5c 330.60 ms 416.86 ms 86.26 ms
9246ed4 281.79 ms 352.08 ms 70.29 ms
496bdfd 272.86 ms 407.33 ms 134.48 ms

App size

Revision Plain With Sentry Diff
496bdfd 1.72 MiB 2.28 MiB 571.82 KiB
8820c5c 1.72 MiB 2.28 MiB 571.82 KiB
9246ed4 1.72 MiB 2.28 MiB 572.22 KiB
496bdfd 1.72 MiB 2.28 MiB 571.82 KiB

Previous results on branch: rz/enh/anr-v2-feedback

Startup times

Revision Plain With Sentry Diff
608561f 302.62 ms 368.96 ms 66.33 ms

App size

Revision Plain With Sentry Diff
608561f 1.72 MiB 2.28 MiB 573.38 KiB

@codecov
Copy link

codecov bot commented Jun 21, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (9246ed4) 81.14% compared to head (fa1e3c1) 81.15%.

❗ Current head fa1e3c1 differs from pull request most recent head 1e585ee. Consider uploading reports for the commit 1e585ee to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #2792   +/-   ##
=========================================
  Coverage     81.14%   81.15%           
- Complexity     4497     4501    +4     
=========================================
  Files           348      348           
  Lines         16677    16685    +8     
  Branches       2267     2268    +1     
=========================================
+ Hits          13532    13540    +8     
  Misses         2198     2198           
  Partials        947      947           
Impacted Files Coverage Δ
sentry/src/main/java/io/sentry/Attachment.java 100.00% <100.00%> (ø)
sentry/src/main/java/io/sentry/Hint.java 93.84% <100.00%> (+0.40%) ⬆️
sentry/src/main/java/io/sentry/SentryClient.java 87.58% <100.00%> (+0.08%) ⬆️
...y/src/main/java/io/sentry/cache/EnvelopeCache.java 65.36% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@markushi markushi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvements, LGTM!

CHANGELOG.md Outdated Show resolved Hide resolved
@romtsn romtsn enabled auto-merge (squash) June 22, 2023 11:15
@romtsn romtsn merged commit 2c45a25 into main Jun 22, 2023
18 of 19 checks passed
@romtsn romtsn deleted the rz/enh/anr-v2-feedback branch June 22, 2023 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants