Add rate limit for Continuous Profiling v8 (p6)

sentry-android-core/api/sentry-android-core.api

@@ -36,11 +36,12 @@ public final class io/sentry/android/core/ActivityLifecycleIntegration : android
 	public fun register (Lio/sentry/IScopes;Lio/sentry/SentryOptions;)V
 }
 
-public class io/sentry/android/core/AndroidContinuousProfiler : io/sentry/IContinuousProfiler {
+public class io/sentry/android/core/AndroidContinuousProfiler : io/sentry/IContinuousProfiler, io/sentry/transport/RateLimiter$IRateLimitObserver {
 	public fun <init> (Lio/sentry/android/core/BuildInfoProvider;Lio/sentry/android/core/internal/util/SentryFrameMetricsCollector;Lio/sentry/ILogger;Ljava/lang/String;ILio/sentry/ISentryExecutorService;)V
 	public fun close ()V
 	public fun getProfilerId ()Lio/sentry/protocol/SentryId;
 	public fun isRunning ()Z
+	public fun onRateLimitChanged (Lio/sentry/transport/RateLimiter;)V
 	public fun start ()V
 	public fun stop ()V
 }

sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java

@@ -1,10 +1,13 @@
 package io.sentry.android.core;
 
+import static io.sentry.DataCategory.All;
+import static io.sentry.IConnectionStatusProvider.ConnectionStatus.DISCONNECTED;
 import static java.util.concurrent.TimeUnit.SECONDS;
 
 import android.annotation.SuppressLint;
 import android.os.Build;
 import io.sentry.CompositePerformanceCollector;
+import io.sentry.DataCategory;
 import io.sentry.IContinuousProfiler;
 import io.sentry.ILogger;
 import io.sentry.IScopes;
@@ -17,6 +20,7 @@
 import io.sentry.SentryOptions;
 import io.sentry.android.core.internal.util.SentryFrameMetricsCollector;
 import io.sentry.protocol.SentryId;
+import io.sentry.transport.RateLimiter;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.Future;
@@ -27,7 +31,8 @@
 import org.jetbrains.annotations.VisibleForTesting;
 
 @ApiStatus.Internal
-public class AndroidContinuousProfiler implements IContinuousProfiler {
+public class AndroidContinuousProfiler
+    implements IContinuousProfiler, RateLimiter.IRateLimitObserver {
   private static final long MAX_CHUNK_DURATION_MILLIS = 10000;
 
   private final @NotNull ILogger logger;
@@ -45,6 +50,7 @@ public class AndroidContinuousProfiler implements IContinuousProfiler {
   private final @NotNull List<ProfileChunk.Builder> payloadBuilders = new ArrayList<>();
   private @NotNull SentryId profilerId = SentryId.EMPTY_ID;
   private @NotNull SentryId chunkId = SentryId.EMPTY_ID;
+  private boolean isClosed = false;
 
   public AndroidContinuousProfiler(
       final @NotNull BuildInfoProvider buildInfoProvider,
@@ -91,11 +97,15 @@ private void init() {
   }
 
   public synchronized void start() {
-    if ((scopes == null || scopes != NoOpScopes.getInstance())
+    if ((scopes == null || scopes == NoOpScopes.getInstance())
         && Sentry.getCurrentScopes() != NoOpScopes.getInstance()) {
       this.scopes = Sentry.getCurrentScopes();
       this.performanceCollector =
           Sentry.getCurrentScopes().getOptions().getCompositePerformanceCollector();
+      final @Nullable RateLimiter rateLimiter = scopes.getRateLimiter();
+      if (rateLimiter != null) {
+        rateLimiter.addRateLimitObserver(this);
+      }
     }
 
     // Debug.startMethodTracingSampling() is only available since Lollipop, but Android Profiler
@@ -109,6 +119,26 @@ public synchronized void start() {
       return;
     }
 
+    if (scopes != null) {
+      final @Nullable RateLimiter rateLimiter = scopes.getRateLimiter();
+      if (rateLimiter != null
+          && (rateLimiter.isActiveForCategory(All)
+              || rateLimiter.isActiveForCategory(DataCategory.ProfileChunk))) {
+        logger.log(SentryLevel.WARNING, "SDK is rate limited. Stopping profiler.");
+        // Let's stop and reset profiler id, as the profile is now broken anyway
+        stop();
+        return;
+      }
+
+      // If device is offline, we don't start the profiler, to avoid flooding the cache
+      if (scopes.getOptions().getConnectionStatusProvider().getConnectionStatus() == DISCONNECTED) {
+        logger.log(SentryLevel.WARNING, "Device is offline. Stopping profiler.");
+        // Let's stop and reset profiler id, as the profile is now broken anyway
+        stop();
+        return;
+      }
+    }
+
     final AndroidProfiler.ProfileStartData startData = profiler.start();
     // check if profiling started
     if (startData == null) {
@@ -150,6 +180,9 @@ private synchronized void stop(final boolean restartProfiler) {
     }
     // check if profiler was created and it's running
     if (profiler == null || !isRunning) {
+      // When the profiler is stopped due to an error (e.g. offline or rate limited), reset the ids
+      profilerId = SentryId.EMPTY_ID;
+      chunkId = SentryId.EMPTY_ID;
       return;
     }
 
@@ -203,6 +236,7 @@ private synchronized void stop(final boolean restartProfiler) {
 
   public synchronized void close() {
     stop();
+    isClosed = true;
   }
 
   @Override
@@ -216,6 +250,10 @@ private void sendChunks(final @NotNull IScopes scopes, final @NotNull SentryOpti
           .getExecutorService()
           .submit(
               () -> {
+                // SDK is closed, we don't send the chunks
+                if (isClosed) {
+                  return;
+                }
                 final ArrayList<ProfileChunk> payloads = new ArrayList<>(payloadBuilders.size());
                 synchronized (payloadBuilders) {
                   for (ProfileChunk.Builder builder : payloadBuilders) {
@@ -242,4 +280,16 @@ public boolean isRunning() {
   Future<?> getStopFuture() {
     return stopFuture;
   }
+
+  @Override
+  public void onRateLimitChanged(@NotNull RateLimiter rateLimiter) {
+    // We stop the profiler as soon as we are rate limited, to avoid the performance overhead
+    if (rateLimiter.isActiveForCategory(All)
+        || rateLimiter.isActiveForCategory(DataCategory.ProfileChunk)) {
+      logger.log(SentryLevel.WARNING, "SDK is rate limited. Stopping profiler.");
+      stop();
+    }
+    // If we are not rate limited anymore, we don't do anything: the profile is broken, so it's
+    // useless to restart it automatically
+  }
 }

sentry-android-core/src/test/java/io/sentry/android/core/AndroidContinuousProfilerTest.kt

@@ -6,6 +6,8 @@ import androidx.test.core.app.ApplicationProvider
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import io.sentry.CompositePerformanceCollector
 import io.sentry.CpuCollectionData
+import io.sentry.DataCategory
+import io.sentry.IConnectionStatusProvider
 import io.sentry.ILogger
 import io.sentry.IScopes
 import io.sentry.ISentryExecutorService
@@ -18,8 +20,10 @@ import io.sentry.SentryTracer
 import io.sentry.TransactionContext
 import io.sentry.android.core.internal.util.SentryFrameMetricsCollector
 import io.sentry.profilemeasurements.ProfileMeasurement
+import io.sentry.protocol.SentryId
 import io.sentry.test.DeferredExecutorService
 import io.sentry.test.getProperty
+import io.sentry.transport.RateLimiter
 import org.junit.runner.RunWith
 import org.mockito.kotlin.any
 import org.mockito.kotlin.check
@@ -37,6 +41,7 @@ import kotlin.test.AfterTest
 import kotlin.test.BeforeTest
 import kotlin.test.Test
 import kotlin.test.assertContains
+import kotlin.test.assertEquals
 import kotlin.test.assertFalse
 import kotlin.test.assertNotNull
 import kotlin.test.assertNull
@@ -394,4 +399,74 @@ class AndroidContinuousProfilerTest {
         executorService.runAll()
         verify(fixture.scopes, times(2)).captureProfileChunk(any())
     }
+
+    @Test
+    fun `profiler does not send chunks after close`() {
+        val executorService = DeferredExecutorService()
+        val profiler = fixture.getSut {
+            it.executorService = executorService
+        }
+        profiler.start()
+        assertTrue(profiler.isRunning)
+
+        // We close the profiler, which should prevent sending additional chunks
+        profiler.close()
+
+        // The executor used to send the chunk doesn't do anything
+        executorService.runAll()
+        verify(fixture.scopes, never()).captureProfileChunk(any())
+    }
+
+    @Test
+    fun `profiler stops when rate limited`() {
+        val executorService = DeferredExecutorService()
+        val profiler = fixture.getSut {
+            it.executorService = executorService
+        }
+        val rateLimiter = mock<RateLimiter>()
+        whenever(rateLimiter.isActiveForCategory(DataCategory.ProfileChunk)).thenReturn(true)
+
+        profiler.start()
+        assertTrue(profiler.isRunning)
+
+        // If the SDK is rate limited, the profiler should stop
+        profiler.onRateLimitChanged(rateLimiter)
+        assertFalse(profiler.isRunning)
+        assertEquals(SentryId.EMPTY_ID, profiler.profilerId)
+        verify(fixture.mockLogger).log(eq(SentryLevel.WARNING), eq("SDK is rate limited. Stopping profiler."))
+    }
+
+    @Test
+    fun `profiler does not start when rate limited`() {
+        val executorService = DeferredExecutorService()
+        val profiler = fixture.getSut {
+            it.executorService = executorService
+        }
+        val rateLimiter = mock<RateLimiter>()
+        whenever(rateLimiter.isActiveForCategory(DataCategory.ProfileChunk)).thenReturn(true)
+        whenever(fixture.scopes.rateLimiter).thenReturn(rateLimiter)
+
+        // If the SDK is rate limited, the profiler should never start
+        profiler.start()
+        assertFalse(profiler.isRunning)
+        assertEquals(SentryId.EMPTY_ID, profiler.profilerId)
+        verify(fixture.mockLogger).log(eq(SentryLevel.WARNING), eq("SDK is rate limited. Stopping profiler."))
+    }
+
+    @Test
+    fun `profiler does not start when offline`() {
+        val executorService = DeferredExecutorService()
+        val profiler = fixture.getSut {
+            it.executorService = executorService
+            it.connectionStatusProvider = mock { provider ->
+                whenever(provider.connectionStatus).thenReturn(IConnectionStatusProvider.ConnectionStatus.DISCONNECTED)
+            }
+        }
+
+        // If the device is offline, the profiler should never start
+        profiler.start()
+        assertFalse(profiler.isRunning)
+        assertEquals(SentryId.EMPTY_ID, profiler.profilerId)
+        verify(fixture.mockLogger).log(eq(SentryLevel.WARNING), eq("Device is offline. Stopping profiler."))
+    }
 }

sentry-opentelemetry/sentry-opentelemetry-bootstrap/api/sentry-opentelemetry-bootstrap.api

@@ -41,7 +41,7 @@ public final class io/sentry/opentelemetry/OtelSpanFactory : io/sentry/ISpanFact
 	public fun <init> ()V
 	public fun <init> (Lio/opentelemetry/api/OpenTelemetry;)V
 	public fun createSpan (Lio/sentry/IScopes;Lio/sentry/SpanOptions;Lio/sentry/SpanContext;Lio/sentry/ISpan;)Lio/sentry/ISpan;
-	public fun createTransaction (Lio/sentry/TransactionContext;Lio/sentry/IScopes;Lio/sentry/TransactionOptions;Lio/sentry/TransactionPerformanceCollector;)Lio/sentry/ITransaction;
+	public fun createTransaction (Lio/sentry/TransactionContext;Lio/sentry/IScopes;Lio/sentry/TransactionOptions;Lio/sentry/CompositePerformanceCollector;)Lio/sentry/ITransaction;
 }
 
 public final class io/sentry/opentelemetry/OtelTransactionSpanForwarder : io/sentry/ITransaction {

sentry/api/sentry.api

@@ -341,6 +341,7 @@ public final class io/sentry/DataCategory : java/lang/Enum {
 	public static final field Error Lio/sentry/DataCategory;
 	public static final field Monitor Lio/sentry/DataCategory;
 	public static final field Profile Lio/sentry/DataCategory;
+	public static final field ProfileChunk Lio/sentry/DataCategory;
 	public static final field Replay Lio/sentry/DataCategory;
 	public static final field Security Lio/sentry/DataCategory;
 	public static final field Session Lio/sentry/DataCategory;

sentry/src/main/java/io/sentry/DataCategory.java

@@ -12,6 +12,7 @@ public enum DataCategory {
   Attachment("attachment"),
   Monitor("monitor"),
   Profile("profile"),
+  ProfileChunk("profile_chunk"),
   Transaction("transaction"),
   Replay("replay"),
   Span("span"),

sentry/src/main/java/io/sentry/clientreport/ClientReportRecorder.java

@@ -165,6 +165,9 @@ private DataCategory categoryFromItemType(SentryItemType itemType) {
     if (SentryItemType.Profile.equals(itemType)) {
       return DataCategory.Profile;
     }
+    if (SentryItemType.ProfileChunk.equals(itemType)) {
+      return DataCategory.ProfileChunk;
+    }
     if (SentryItemType.Attachment.equals(itemType)) {
       return DataCategory.Attachment;
     }

sentry/src/main/java/io/sentry/transport/RateLimiter.java

@@ -184,6 +184,8 @@ private boolean isRetryAfter(final @NotNull String itemType) {
         return DataCategory.Attachment;
       case "profile":
         return DataCategory.Profile;
+      case "profile_chunk":
+        return DataCategory.ProfileChunk;
       case "transaction":
         return DataCategory.Transaction;
       case "check_in":

sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt

@@ -10,6 +10,7 @@ import io.sentry.ILogger
 import io.sentry.IScopes
 import io.sentry.ISerializer
 import io.sentry.NoOpLogger
+import io.sentry.ProfileChunk
 import io.sentry.ProfilingTraceData
 import io.sentry.ReplayRecording
 import io.sentry.SentryEnvelope
@@ -208,8 +209,9 @@ class RateLimiterTest {
         val attachmentItem = SentryEnvelopeItem.fromAttachment(fixture.serializer, NoOpLogger.getInstance(), Attachment("{ \"number\": 10 }".toByteArray(), "log.json"), 1000)
         val profileItem = SentryEnvelopeItem.fromProfilingTrace(ProfilingTraceData(File(""), transaction), 1000, fixture.serializer)
         val checkInItem = SentryEnvelopeItem.fromCheckIn(fixture.serializer, CheckIn("monitor-slug-1", CheckInStatus.ERROR))
+        val profileChunkItem = SentryEnvelopeItem.fromProfileChunk(ProfileChunk(), fixture.serializer)
 
-        val envelope = SentryEnvelope(SentryEnvelopeHeader(), arrayListOf(eventItem, userFeedbackItem, sessionItem, attachmentItem, profileItem, checkInItem))
+        val envelope = SentryEnvelope(SentryEnvelopeHeader(), arrayListOf(eventItem, userFeedbackItem, sessionItem, attachmentItem, profileItem, checkInItem, profileChunkItem))
 
         rateLimiter.updateRetryAfterLimits(null, null, 429)
         val result = rateLimiter.filter(envelope, Hint())
@@ -222,6 +224,7 @@ class RateLimiterTest {
         verify(fixture.clientReportRecorder, times(1)).recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(attachmentItem))
         verify(fixture.clientReportRecorder, times(1)).recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(profileItem))
         verify(fixture.clientReportRecorder, times(1)).recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(checkInItem))
+        verify(fixture.clientReportRecorder, times(1)).recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(profileChunkItem))
         verifyNoMoreInteractions(fixture.clientReportRecorder)
     }
 
@@ -331,6 +334,24 @@ class RateLimiterTest {
         verifyNoMoreInteractions(fixture.clientReportRecorder)
     }
 
+    @Test
+    fun `drop profileChunk items as lost`() {
+        val rateLimiter = fixture.getSUT()
+
+        val profileChunkItem = SentryEnvelopeItem.fromProfileChunk(ProfileChunk(), fixture.serializer)
+        val attachmentItem = SentryEnvelopeItem.fromAttachment(fixture.serializer, NoOpLogger.getInstance(), Attachment("{ \"number\": 10 }".toByteArray(), "log.json"), 1000)
+        val envelope = SentryEnvelope(SentryEnvelopeHeader(), arrayListOf(profileChunkItem, attachmentItem))
+
+        rateLimiter.updateRetryAfterLimits("60:profile_chunk:key", null, 1)
+        val result = rateLimiter.filter(envelope, Hint())
+
+        assertNotNull(result)
+        assertEquals(1, result.items.toList().size)
+
+        verify(fixture.clientReportRecorder, times(1)).recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(profileChunkItem))
+        verifyNoMoreInteractions(fixture.clientReportRecorder)
+    }
+
     @Test
     fun `apply rate limits notifies observers`() {
         val rateLimiter = fixture.getSUT()