fix(clientreport): Stop deserializing discarded logs (JAVA-662)#5835
Draft
runningcode wants to merge 2 commits into
Draft
fix(clientreport): Stop deserializing discarded logs (JAVA-662)#5835runningcode wants to merge 2 commits into
runningcode wants to merge 2 commits into
Conversation
ClientReportRecorder counted discarded log and metric items by fully deserializing the envelope payload just to read its size. On the discard path this runs continuously under sustained rate limiting, and the JSON reader's error-tolerant recovery throws an exception per token, pinning CPU cores in a busy-loop (fillInStackTrace dominated the profile). The item count is already stored in the envelope item header, so read it from there instead of deserializing. Byte counts still come from the raw data. This makes the discard path O(1) and allocation/exception-free. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📲 Install BuildsAndroid
|
Contributor
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 22f4345 | 307.87 ms | 354.51 ms | 46.64 ms |
| 8c7718c | 307.42 ms | 374.84 ms | 67.42 ms |
| bb0ff41 | 321.00 ms | 378.28 ms | 57.28 ms |
| c8125f3 | 397.65 ms | 485.14 ms | 87.49 ms |
| eb95ded | 317.51 ms | 369.08 ms | 51.57 ms |
| f634d01 | 375.06 ms | 420.04 ms | 44.98 ms |
| 9fbb112 | 359.71 ms | 421.85 ms | 62.14 ms |
| d501a7e | 307.33 ms | 341.94 ms | 34.61 ms |
| bb0ff41 | 317.76 ms | 384.66 ms | 66.90 ms |
| 9054d65 | 330.94 ms | 403.24 ms | 72.30 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 22f4345 | 1.58 MiB | 2.29 MiB | 719.83 KiB |
| 8c7718c | 0 B | 0 B | 0 B |
| bb0ff41 | 0 B | 0 B | 0 B |
| c8125f3 | 1.58 MiB | 2.10 MiB | 532.32 KiB |
| eb95ded | 0 B | 0 B | 0 B |
| f634d01 | 1.58 MiB | 2.10 MiB | 533.40 KiB |
| 9fbb112 | 1.58 MiB | 2.11 MiB | 539.18 KiB |
| d501a7e | 0 B | 0 B | 0 B |
| bb0ff41 | 0 B | 0 B | 0 B |
| 9054d65 | 1.58 MiB | 2.29 MiB | 723.38 KiB |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📜 Description
ClientReportRecorder.recordLostEnvelopeItemcounted discarded log and metric items by fully deserializing the envelope payload just to readgetItems().size(). The item count is already carried in the envelope item header (item_count, set bySentryEnvelopeItem.fromLogs/fromMetricsand round-tripped through the disk cache), so this change reads it from the header instead of deserializing. Byte counts still come from the rawgetData()bytes.SentryEnvelopeItemHeader.getItemCount().recordLostEnvelopeItemnow use the header count; when it is absent (legacy/foreign envelopes) they fall back to1and never deserialize.💡 Motivation and Context
Under sustained rate limiting (e.g. transaction/span quota exhausted while
logs.enabled=true), envelopes are discarded continuously via rate limiting → queue/cache overflow → send/network errors. Each discard ran a full JSON deserialization on theSentryAsyncConnection/SentryExecutorServicethreads. The gson reader's error-tolerant recovery machinery throws an exception per token, andThrowable.fillInStackTracedominated the CPU profile — the cost then fed back into more queue overflow and more discards, pinning CPU cores in a busy-loop indefinitely. Delivery of non-rate-limited categories (errors) stopped, and the only workaround was disabling logs.Reading the count from the header makes the discard path O(1) and allocation/exception-free.
Fixes GH-5830
Fixes JAVA-662
💚 How did you test it?
Unit tests in
ClientReportTest:getLogs()is verified never to be called.item_countfallback (counts as1).📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
None.