fix(clientreport): Stop deserializing discarded logs (JAVA-662)

CHANGELOG.md

@@ -8,6 +8,8 @@
 
 ### 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
 - Release `MediaMuxer` when the replay video encoder fails to start to avoid a resource leak ([#5607](https://github.com/getsentry/sentry-java/pull/5607))
 
 ### Performance

sentry/api/sentry.api

@@ -3112,6 +3112,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;

sentry/src/main/java/io/sentry/SentryEnvelopeItemHeader.java

@@ -52,6 +52,10 @@ public int getLength() {
     return platform;
   }
 
+  public @Nullable Integer getItemCount() {
+    return itemCount;
+  }
+
   @ApiStatus.Internal
   public SentryEnvelopeItemHeader(
       final @NotNull SentryItemType type,

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<SentryLogEvent> 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<SentryMetricsEvent> 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();

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,7 +48,10 @@ 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
@@ -417,6 +422,82 @@ 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<SentryOptions.OnDiscardCallback>()
+    givenClientReportRecorder { options -> options.onDiscard = onDiscardMock }
+
+    // A payload that would throw if deserialized; the count must come from the header instead.
+    val brokenPayload = "not valid json".toByteArray()
+    val itemHeader =
+      SentryEnvelopeItemHeader(
+        SentryItemType.Log,
+        0,
+        "application/vnd.sentry.items.log+json",
+        null,
+        null,
+        null,
+        5,
+      )
+    val item =
+      mock<SentryEnvelopeItem> {
+        on { it.header } doReturn itemHeader
+        on { it.data } doReturn brokenPayload
+      }
+
+    clientReportRecorder.recordLostEnvelopeItem(DiscardReason.NETWORK_ERROR, item)
+
+    verify(item, never()).getLogs(any())
+    verify(onDiscardMock, times(1)).execute(DiscardReason.NETWORK_ERROR, DataCategory.LogItem, 5)
+
+    val clientReport = clientReportRecorder.resetCountsAndGenerateClientReport()
+    assertEquals(
+      5,
+      clientReport!!
+        .discardedEvents!!
+        .first { it.category == DataCategory.LogItem.category }
+        .quantity,
+    )
+    assertEquals(
+      brokenPayload.size.toLong(),
+      clientReport.discardedEvents!!
+        .first { it.category == DataCategory.LogByte.category }
+        .quantity,
+    )
+  }
+
+  @Test
+  fun `recording lost log item without item count in header falls back to one`() {
+    givenClientReportRecorder()
+
+    val itemHeader =
+      SentryEnvelopeItemHeader(
+        SentryItemType.Log,
+        0,
+        "application/vnd.sentry.items.log+json",
+        null,
+        null,
+        null,
+        null,
+      )
+    val item =
+      mock<SentryEnvelopeItem> {
+        on { it.header } doReturn itemHeader
+        on { it.data } doReturn ByteArray(0)
+      }
+
+    clientReportRecorder.recordLostEnvelopeItem(DiscardReason.NETWORK_ERROR, item)
+
+    val clientReport = clientReportRecorder.resetCountsAndGenerateClientReport()
+    assertEquals(
+      1,
+      clientReport!!
+        .discardedEvents!!
+        .first { it.category == DataCategory.LogItem.category }
+        .quantity,
+    )
+  }
+
   private fun givenClientReportRecorder(
     callback: Sentry.OptionsConfiguration<SentryOptions>? = null
   ) {