From 8f2bf86b1fdbed98893a692c79a778a00ed56b9b Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Fri, 24 Jul 2026 16:41:02 +0200 Subject: [PATCH 1/3] fix(clientreport): Stop deserializing discarded logs (JAVA-662) 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 --- sentry/api/sentry.api | 1 + .../io/sentry/SentryEnvelopeItemHeader.java | 4 + .../clientreport/ClientReportRecorder.java | 50 +++---- .../sentry/clientreport/ClientReportTest.kt | 126 ++++++++++++++---- 4 files changed, 125 insertions(+), 56 deletions(-) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 7e258b0d4b1..e23f283d942 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -3120,6 +3120,7 @@ public final class io/sentry/SentryEnvelopeItemHeader : io/sentry/JsonSerializab public fun getAttachmentType ()Ljava/lang/String; public fun getContentType ()Ljava/lang/String; public fun getFileName ()Ljava/lang/String; + public fun getItemCount ()Ljava/lang/Integer; public fun getLength ()I public fun getPlatform ()Ljava/lang/String; public fun getType ()Lio/sentry/SentryItemType; diff --git a/sentry/src/main/java/io/sentry/SentryEnvelopeItemHeader.java b/sentry/src/main/java/io/sentry/SentryEnvelopeItemHeader.java index fea98a66767..eaacee9957a 100644 --- a/sentry/src/main/java/io/sentry/SentryEnvelopeItemHeader.java +++ b/sentry/src/main/java/io/sentry/SentryEnvelopeItemHeader.java @@ -53,6 +53,10 @@ public int getLength() { return platform; } + public @Nullable Integer getItemCount() { + return itemCount; + } + @Nullable Integer getMetaLength() { if (calculateMetaLength != null) { diff --git a/sentry/src/main/java/io/sentry/clientreport/ClientReportRecorder.java b/sentry/src/main/java/io/sentry/clientreport/ClientReportRecorder.java index b3756ed32e4..ccd7c5dc402 100644 --- a/sentry/src/main/java/io/sentry/clientreport/ClientReportRecorder.java +++ b/sentry/src/main/java/io/sentry/clientreport/ClientReportRecorder.java @@ -6,10 +6,6 @@ import io.sentry.SentryEnvelopeItem; import io.sentry.SentryItemType; import io.sentry.SentryLevel; -import io.sentry.SentryLogEvent; -import io.sentry.SentryLogEvents; -import io.sentry.SentryMetricsEvent; -import io.sentry.SentryMetricsEvents; import io.sentry.SentryOptions; import io.sentry.protocol.SentrySpan; import io.sentry.protocol.SentryTransaction; @@ -105,34 +101,18 @@ public void recordLostEnvelopeItem( recordLostEventInternal(reason.getReason(), itemCategory.getCategory(), 1L); executeOnDiscard(reason, itemCategory, 1L); } else if (itemCategory.equals(DataCategory.LogItem)) { - final @Nullable SentryLogEvents logs = envelopeItem.getLogs(options.getSerializer()); - if (logs != null) { - final @NotNull List items = logs.getItems(); - final long count = items.size(); - recordLostEventInternal(reason.getReason(), itemCategory.getCategory(), count); - final long logBytes = envelopeItem.getData().length; - recordLostEventInternal( - reason.getReason(), DataCategory.LogByte.getCategory(), logBytes); - executeOnDiscard(reason, itemCategory, count); - } else { - options.getLogger().log(SentryLevel.ERROR, "Unable to parse lost logs envelope item."); - } + final long count = itemCountFromHeader(envelopeItem); + recordLostEventInternal(reason.getReason(), itemCategory.getCategory(), count); + final long logBytes = envelopeItem.getData().length; + recordLostEventInternal(reason.getReason(), DataCategory.LogByte.getCategory(), logBytes); + executeOnDiscard(reason, itemCategory, count); } else if (itemCategory.equals(DataCategory.TraceMetric)) { - final @Nullable SentryMetricsEvents metrics = - envelopeItem.getMetrics(options.getSerializer()); - if (metrics != null) { - final @NotNull List items = metrics.getItems(); - final long count = items.size(); - recordLostEventInternal(reason.getReason(), itemCategory.getCategory(), count); - final long metricBytes = envelopeItem.getData().length; - recordLostEventInternal( - reason.getReason(), DataCategory.TraceMetricByte.getCategory(), metricBytes); - executeOnDiscard(reason, itemCategory, count); - } else { - options - .getLogger() - .log(SentryLevel.ERROR, "Unable to parse lost metrics envelope item."); - } + final long count = itemCountFromHeader(envelopeItem); + recordLostEventInternal(reason.getReason(), itemCategory.getCategory(), count); + final long metricBytes = envelopeItem.getData().length; + recordLostEventInternal( + reason.getReason(), DataCategory.TraceMetricByte.getCategory(), metricBytes); + executeOnDiscard(reason, itemCategory, count); } else { recordLostEventInternal(reason.getReason(), itemCategory.getCategory(), 1L); executeOnDiscard(reason, itemCategory, 1L); @@ -176,6 +156,14 @@ private void recordLostEventInternal( storage.addCount(key, countToAdd); } + // The number of items batched into a log or metric envelope item is stored in its header, so we + // read it from there instead of deserializing the payload. Deserializing on the discard path is + // expensive and, under sustained rate limiting, caused a CPU busy-loop (JAVA-662). + private long itemCountFromHeader(final @NotNull SentryEnvelopeItem envelopeItem) { + final @Nullable Integer itemCount = envelopeItem.getHeader().getItemCount(); + return itemCount != null ? itemCount : 1L; + } + @Nullable ClientReport resetCountsAndGenerateClientReport() { final Date currentDate = DateUtils.getCurrentDateTime(); diff --git a/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt b/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt index ceb254213cb..66117c2a6db 100644 --- a/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt +++ b/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt @@ -15,7 +15,9 @@ import io.sentry.Sentry import io.sentry.SentryEnvelope import io.sentry.SentryEnvelopeHeader import io.sentry.SentryEnvelopeItem +import io.sentry.SentryEnvelopeItemHeader import io.sentry.SentryEvent +import io.sentry.SentryItemType import io.sentry.SentryLogEvent import io.sentry.SentryLogEvents import io.sentry.SentryLogLevel @@ -46,11 +48,17 @@ import java.util.UUID import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertTrue +import org.mockito.kotlin.any +import org.mockito.kotlin.doReturn import org.mockito.kotlin.mock +import org.mockito.kotlin.never import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.whenever +private const val LOG_CONTENT_TYPE = "application/vnd.sentry.items.log+json" +private const val METRIC_CONTENT_TYPE = "application/vnd.sentry.items.trace-metric+json" + class ClientReportTest { lateinit var opts: SentryOptions lateinit var clientReportRecorder: ClientReportRecorder @@ -312,7 +320,7 @@ class ClientReportTest { } @Test - fun `recording envelope with lost client report does not duplicate onDiscard executions`() { + fun `restoring counts from an attached client report does not fire onDiscard again`() { val onDiscardMock = mock() givenClientReportRecorder { options -> options.onDiscard = onDiscardMock } @@ -332,27 +340,6 @@ class ClientReportTest { verify(onDiscardMock, times(1)).execute(DiscardReason.BEFORE_SEND, DataCategory.Profile, 1) } - @Test - fun `recording lost client report does not duplicate onDiscard executions`() { - val onDiscardMock = mock() - givenClientReportRecorder { options -> options.onDiscard = onDiscardMock } - - clientReportRecorder.recordLostEvent(DiscardReason.CACHE_OVERFLOW, DataCategory.Attachment) - clientReportRecorder.recordLostEvent(DiscardReason.CACHE_OVERFLOW, DataCategory.Attachment) - clientReportRecorder.recordLostEvent(DiscardReason.RATELIMIT_BACKOFF, DataCategory.Error) - clientReportRecorder.recordLostEvent(DiscardReason.QUEUE_OVERFLOW, DataCategory.Error) - clientReportRecorder.recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Profile) - - val envelope = clientReportRecorder.attachReportToEnvelope(testHelper.newEnvelope()) - clientReportRecorder.recordLostEnvelopeItem(DiscardReason.NETWORK_ERROR, envelope.items.first()) - - verify(onDiscardMock, times(2)) - .execute(DiscardReason.CACHE_OVERFLOW, DataCategory.Attachment, 1) - verify(onDiscardMock, times(1)).execute(DiscardReason.RATELIMIT_BACKOFF, DataCategory.Error, 1) - verify(onDiscardMock, times(1)).execute(DiscardReason.QUEUE_OVERFLOW, DataCategory.Error, 1) - verify(onDiscardMock, times(1)).execute(DiscardReason.BEFORE_SEND, DataCategory.Profile, 1) - } - @Test fun `recording lost client report counts log entries`() { val onDiscardMock = mock() @@ -417,6 +404,98 @@ class ClientReportTest { assertEquals(envelope.items.first().data.size.toLong(), metricByteItem.quantity) } + @Test + fun `recording lost log item reads count from the header without deserializing the payload`() { + val onDiscardMock = mock() + givenClientReportRecorder { options -> options.onDiscard = onDiscardMock } + + val payload = "irrelevant payload".toByteArray() + val item = mockEnvelopeItem(SentryItemType.Log, LOG_CONTENT_TYPE, itemCount = 5, data = payload) + + clientReportRecorder.recordLostEnvelopeItem(DiscardReason.NETWORK_ERROR, item) + + // Deserializing here is what pinned CPU cores under sustained rate limiting (JAVA-662), so the + // count must come from the header and the payload must stay untouched. + verify(item, never()).getLogs(any()) + verify(onDiscardMock, times(1)).execute(DiscardReason.NETWORK_ERROR, DataCategory.LogItem, 5) + + val clientReport = clientReportRecorder.resetCountsAndGenerateClientReport() + assertEquals(5, clientReport.quantityOf(DataCategory.LogItem)) + assertEquals(payload.size.toLong(), clientReport.quantityOf(DataCategory.LogByte)) + } + + @Test + fun `recording lost metric item reads count from the header without deserializing the payload`() { + val onDiscardMock = mock() + givenClientReportRecorder { options -> options.onDiscard = onDiscardMock } + + val payload = "irrelevant payload".toByteArray() + val item = + mockEnvelopeItem( + SentryItemType.TraceMetric, + METRIC_CONTENT_TYPE, + itemCount = 5, + data = payload, + ) + + clientReportRecorder.recordLostEnvelopeItem(DiscardReason.NETWORK_ERROR, item) + + verify(item, never()).getMetrics(any()) + verify(onDiscardMock, times(1)) + .execute(DiscardReason.NETWORK_ERROR, DataCategory.TraceMetric, 5) + + val clientReport = clientReportRecorder.resetCountsAndGenerateClientReport() + assertEquals(5, clientReport.quantityOf(DataCategory.TraceMetric)) + assertEquals(payload.size.toLong(), clientReport.quantityOf(DataCategory.TraceMetricByte)) + } + + @Test + fun `recording lost log item without item count in header falls back to one`() { + givenClientReportRecorder() + + val item = + mockEnvelopeItem(SentryItemType.Log, LOG_CONTENT_TYPE, itemCount = null, data = ByteArray(0)) + + clientReportRecorder.recordLostEnvelopeItem(DiscardReason.NETWORK_ERROR, item) + + val clientReport = clientReportRecorder.resetCountsAndGenerateClientReport() + assertEquals(1, clientReport.quantityOf(DataCategory.LogItem)) + } + + @Test + fun `recording lost metric item without item count in header falls back to one`() { + givenClientReportRecorder() + + val item = + mockEnvelopeItem( + SentryItemType.TraceMetric, + METRIC_CONTENT_TYPE, + itemCount = null, + data = ByteArray(0), + ) + + clientReportRecorder.recordLostEnvelopeItem(DiscardReason.NETWORK_ERROR, item) + + val clientReport = clientReportRecorder.resetCountsAndGenerateClientReport() + assertEquals(1, clientReport.quantityOf(DataCategory.TraceMetric)) + } + + private fun mockEnvelopeItem( + type: SentryItemType, + contentType: String, + itemCount: Int?, + data: ByteArray, + ): SentryEnvelopeItem { + val itemHeader = SentryEnvelopeItemHeader(type, 0, contentType, null, null, null, itemCount) + return mock { + on { it.header } doReturn itemHeader + on { it.data } doReturn data + } + } + + private fun ClientReport?.quantityOf(category: DataCategory): Long = + this!!.discardedEvents!!.first { it.category == category.category }.quantity + private fun givenClientReportRecorder( callback: Sentry.OptionsConfiguration? = null ) { @@ -470,9 +549,6 @@ class ClientReportTestHelper(val options: SentryOptions) { return SentryEnvelope(header, items.toList()) } - fun toEnvelopeItem(clientReport: ClientReport): SentryEnvelopeItem = - SentryEnvelopeItem.fromClientReport(options.serializer, clientReport) - companion object { fun retryableHint() = HintUtils.createWithTypeCheckHint(TestRetryable()) From 35304fcd1b46606af0169570f7e9a14e8d31c713 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Fri, 24 Jul 2026 16:41:49 +0200 Subject: [PATCH 2/3] changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b52621fae5a..89d412bcfdd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +### Fixes + +- Avoid a CPU busy-loop when recording discarded log or metric envelopes under rate limiting ([#5835](https://github.com/getsentry/sentry-java/pull/5835)) + - `ClientReportRecorder` now reads the item count from the envelope item header instead of deserializing the payload, which under sustained rate limiting could pin CPU cores while repeatedly throwing exceptions + ### Performance - Remove an unused lock from `SentryPerformanceProvider`, which was allocated on every cold start in `ContentProvider.onCreate` without ever being acquired ([#5871](https://github.com/getsentry/sentry-java/pull/5871)) From ddea7d5781b71ac292f4f3f86b2c4bab6b76e401 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Wed, 29 Jul 2026 16:55:12 +0200 Subject: [PATCH 3/3] test(clientreport): Cover both onDiscard restore entry points (JAVA-662) The two tests asserting that restoring counts from an attached client report does not re-fire onDiscard were named for their setup rather than for what actually differed between them, which made the pair read as an accidental duplicate. Name each for its entry point and share the setup and verifications, so it is clear the property is being pinned for both recordLostEnvelope and recordLostEnvelopeItem. Co-Authored-By: Claude Opus 5 (1M context) --- .../sentry/clientreport/ClientReportTest.kt | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt b/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt index 66117c2a6db..b89b1894f32 100644 --- a/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt +++ b/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt @@ -320,7 +320,24 @@ class ClientReportTest { } @Test - fun `restoring counts from an attached client report does not fire onDiscard again`() { + fun `restoring counts via recordLostEnvelope does not fire onDiscard again`() { + assertRestoringCountsDoesNotFireOnDiscard { recorder, envelope -> + recorder.recordLostEnvelope(DiscardReason.EVENT_PROCESSOR, envelope) + } + } + + @Test + fun `restoring counts via recordLostEnvelopeItem does not fire onDiscard again`() { + assertRestoringCountsDoesNotFireOnDiscard { recorder, envelope -> + recorder.recordLostEnvelopeItem(DiscardReason.NETWORK_ERROR, envelope.items.first()) + } + } + + // Counts restored from an attached client report were already reported once, so replaying them + // must not fire onDiscard a second time. Both public entry points have to hold the property. + private fun assertRestoringCountsDoesNotFireOnDiscard( + recordLost: (ClientReportRecorder, SentryEnvelope) -> Unit + ) { val onDiscardMock = mock() givenClientReportRecorder { options -> options.onDiscard = onDiscardMock } @@ -331,7 +348,7 @@ class ClientReportTest { clientReportRecorder.recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Profile) val envelope = clientReportRecorder.attachReportToEnvelope(testHelper.newEnvelope()) - clientReportRecorder.recordLostEnvelope(DiscardReason.EVENT_PROCESSOR, envelope) + recordLost(clientReportRecorder, envelope) verify(onDiscardMock, times(2)) .execute(DiscardReason.CACHE_OVERFLOW, DataCategory.Attachment, 1)