Skip to content

Commit b0e8333

Browse files
stefanosianostefanosiano
authored
Add rate limit for Continuous Profiling v8 (p6) (#3926)
* continuous profiler now doesn't start if offline or rate limited * continuous profiler stops when rate limited * continuous profiler prevents sending chunks after being closed * added profile_chunk rate limit * continuous profiler now reset its id when rate limited or offline
1 parent 6c36c4b commit b0e8333

File tree

9 files changed

+160
-5
lines changed

9 files changed

+160
-5
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,12 @@ public final class io/sentry/android/core/ActivityLifecycleIntegration : android
3636
public fun register (Lio/sentry/IScopes;Lio/sentry/SentryOptions;)V
3737
}
3838

39-
public class io/sentry/android/core/AndroidContinuousProfiler : io/sentry/IContinuousProfiler {
39+
public class io/sentry/android/core/AndroidContinuousProfiler : io/sentry/IContinuousProfiler, io/sentry/transport/RateLimiter$IRateLimitObserver {
4040
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
4141
public fun close ()V
4242
public fun getProfilerId ()Lio/sentry/protocol/SentryId;
4343
public fun isRunning ()Z
44+
public fun onRateLimitChanged (Lio/sentry/transport/RateLimiter;)V
4445
public fun start ()V
4546
public fun stop ()V
4647
}

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

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
package io.sentry.android.core;
22

3+
import static io.sentry.DataCategory.All;
4+
import static io.sentry.IConnectionStatusProvider.ConnectionStatus.DISCONNECTED;
35
import static java.util.concurrent.TimeUnit.SECONDS;
46

57
import android.annotation.SuppressLint;
68
import android.os.Build;
79
import io.sentry.CompositePerformanceCollector;
10+
import io.sentry.DataCategory;
811
import io.sentry.IContinuousProfiler;
912
import io.sentry.ILogger;
1013
import io.sentry.IScopes;
@@ -17,17 +20,20 @@
1720
import io.sentry.SentryOptions;
1821
import io.sentry.android.core.internal.util.SentryFrameMetricsCollector;
1922
import io.sentry.protocol.SentryId;
23+
import io.sentry.transport.RateLimiter;
2024
import java.util.ArrayList;
2125
import java.util.List;
2226
import java.util.concurrent.Future;
2327
import java.util.concurrent.RejectedExecutionException;
28+
import java.util.concurrent.atomic.AtomicBoolean;
2429
import org.jetbrains.annotations.ApiStatus;
2530
import org.jetbrains.annotations.NotNull;
2631
import org.jetbrains.annotations.Nullable;
2732
import org.jetbrains.annotations.VisibleForTesting;
2833

2934
@ApiStatus.Internal
30-
public class AndroidContinuousProfiler implements IContinuousProfiler {
35+
public class AndroidContinuousProfiler
36+
implements IContinuousProfiler, RateLimiter.IRateLimitObserver {
3137
private static final long MAX_CHUNK_DURATION_MILLIS = 10000;
3238

3339
private final @NotNull ILogger logger;
@@ -45,6 +51,7 @@ public class AndroidContinuousProfiler implements IContinuousProfiler {
4551
private final @NotNull List<ProfileChunk.Builder> payloadBuilders = new ArrayList<>();
4652
private @NotNull SentryId profilerId = SentryId.EMPTY_ID;
4753
private @NotNull SentryId chunkId = SentryId.EMPTY_ID;
54+
private final @NotNull AtomicBoolean isClosed = new AtomicBoolean(false);
4855

4956
public AndroidContinuousProfiler(
5057
final @NotNull BuildInfoProvider buildInfoProvider,
@@ -91,11 +98,15 @@ private void init() {
9198
}
9299

93100
public synchronized void start() {
94-
if ((scopes == null || scopes != NoOpScopes.getInstance())
101+
if ((scopes == null || scopes == NoOpScopes.getInstance())
95102
&& Sentry.getCurrentScopes() != NoOpScopes.getInstance()) {
96103
this.scopes = Sentry.getCurrentScopes();
97104
this.performanceCollector =
98105
Sentry.getCurrentScopes().getOptions().getCompositePerformanceCollector();
106+
final @Nullable RateLimiter rateLimiter = scopes.getRateLimiter();
107+
if (rateLimiter != null) {
108+
rateLimiter.addRateLimitObserver(this);
109+
}
99110
}
100111

101112
// Debug.startMethodTracingSampling() is only available since Lollipop, but Android Profiler
@@ -109,6 +120,26 @@ public synchronized void start() {
109120
return;
110121
}
111122

123+
if (scopes != null) {
124+
final @Nullable RateLimiter rateLimiter = scopes.getRateLimiter();
125+
if (rateLimiter != null
126+
&& (rateLimiter.isActiveForCategory(All)
127+
|| rateLimiter.isActiveForCategory(DataCategory.ProfileChunk))) {
128+
logger.log(SentryLevel.WARNING, "SDK is rate limited. Stopping profiler.");
129+
// Let's stop and reset profiler id, as the profile is now broken anyway
130+
stop();
131+
return;
132+
}
133+
134+
// If device is offline, we don't start the profiler, to avoid flooding the cache
135+
if (scopes.getOptions().getConnectionStatusProvider().getConnectionStatus() == DISCONNECTED) {
136+
logger.log(SentryLevel.WARNING, "Device is offline. Stopping profiler.");
137+
// Let's stop and reset profiler id, as the profile is now broken anyway
138+
stop();
139+
return;
140+
}
141+
}
142+
112143
final AndroidProfiler.ProfileStartData startData = profiler.start();
113144
// check if profiling started
114145
if (startData == null) {
@@ -150,6 +181,9 @@ private synchronized void stop(final boolean restartProfiler) {
150181
}
151182
// check if profiler was created and it's running
152183
if (profiler == null || !isRunning) {
184+
// When the profiler is stopped due to an error (e.g. offline or rate limited), reset the ids
185+
profilerId = SentryId.EMPTY_ID;
186+
chunkId = SentryId.EMPTY_ID;
153187
return;
154188
}
155189

@@ -203,6 +237,7 @@ private synchronized void stop(final boolean restartProfiler) {
203237

204238
public synchronized void close() {
205239
stop();
240+
isClosed.set(true);
206241
}
207242

208243
@Override
@@ -216,6 +251,10 @@ private void sendChunks(final @NotNull IScopes scopes, final @NotNull SentryOpti
216251
.getExecutorService()
217252
.submit(
218253
() -> {
254+
// SDK is closed, we don't send the chunks
255+
if (isClosed.get()) {
256+
return;
257+
}
219258
final ArrayList<ProfileChunk> payloads = new ArrayList<>(payloadBuilders.size());
220259
synchronized (payloadBuilders) {
221260
for (ProfileChunk.Builder builder : payloadBuilders) {
@@ -242,4 +281,16 @@ public boolean isRunning() {
242281
Future<?> getStopFuture() {
243282
return stopFuture;
244283
}
284+
285+
@Override
286+
public void onRateLimitChanged(@NotNull RateLimiter rateLimiter) {
287+
// We stop the profiler as soon as we are rate limited, to avoid the performance overhead
288+
if (rateLimiter.isActiveForCategory(All)
289+
|| rateLimiter.isActiveForCategory(DataCategory.ProfileChunk)) {
290+
logger.log(SentryLevel.WARNING, "SDK is rate limited. Stopping profiler.");
291+
stop();
292+
}
293+
// If we are not rate limited anymore, we don't do anything: the profile is broken, so it's
294+
// useless to restart it automatically
295+
}
245296
}

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

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import androidx.test.core.app.ApplicationProvider
66
import androidx.test.ext.junit.runners.AndroidJUnit4
77
import io.sentry.CompositePerformanceCollector
88
import io.sentry.CpuCollectionData
9+
import io.sentry.DataCategory
10+
import io.sentry.IConnectionStatusProvider
911
import io.sentry.ILogger
1012
import io.sentry.IScopes
1113
import io.sentry.ISentryExecutorService
@@ -18,8 +20,10 @@ import io.sentry.SentryTracer
1820
import io.sentry.TransactionContext
1921
import io.sentry.android.core.internal.util.SentryFrameMetricsCollector
2022
import io.sentry.profilemeasurements.ProfileMeasurement
23+
import io.sentry.protocol.SentryId
2124
import io.sentry.test.DeferredExecutorService
2225
import io.sentry.test.getProperty
26+
import io.sentry.transport.RateLimiter
2327
import org.junit.runner.RunWith
2428
import org.mockito.kotlin.any
2529
import org.mockito.kotlin.check
@@ -37,6 +41,7 @@ import kotlin.test.AfterTest
3741
import kotlin.test.BeforeTest
3842
import kotlin.test.Test
3943
import kotlin.test.assertContains
44+
import kotlin.test.assertEquals
4045
import kotlin.test.assertFalse
4146
import kotlin.test.assertNotNull
4247
import kotlin.test.assertNull
@@ -394,4 +399,74 @@ class AndroidContinuousProfilerTest {
394399
executorService.runAll()
395400
verify(fixture.scopes, times(2)).captureProfileChunk(any())
396401
}
402+
403+
@Test
404+
fun `profiler does not send chunks after close`() {
405+
val executorService = DeferredExecutorService()
406+
val profiler = fixture.getSut {
407+
it.executorService = executorService
408+
}
409+
profiler.start()
410+
assertTrue(profiler.isRunning)
411+
412+
// We close the profiler, which should prevent sending additional chunks
413+
profiler.close()
414+
415+
// The executor used to send the chunk doesn't do anything
416+
executorService.runAll()
417+
verify(fixture.scopes, never()).captureProfileChunk(any())
418+
}
419+
420+
@Test
421+
fun `profiler stops when rate limited`() {
422+
val executorService = DeferredExecutorService()
423+
val profiler = fixture.getSut {
424+
it.executorService = executorService
425+
}
426+
val rateLimiter = mock<RateLimiter>()
427+
whenever(rateLimiter.isActiveForCategory(DataCategory.ProfileChunk)).thenReturn(true)
428+
429+
profiler.start()
430+
assertTrue(profiler.isRunning)
431+
432+
// If the SDK is rate limited, the profiler should stop
433+
profiler.onRateLimitChanged(rateLimiter)
434+
assertFalse(profiler.isRunning)
435+
assertEquals(SentryId.EMPTY_ID, profiler.profilerId)
436+
verify(fixture.mockLogger).log(eq(SentryLevel.WARNING), eq("SDK is rate limited. Stopping profiler."))
437+
}
438+
439+
@Test
440+
fun `profiler does not start when rate limited`() {
441+
val executorService = DeferredExecutorService()
442+
val profiler = fixture.getSut {
443+
it.executorService = executorService
444+
}
445+
val rateLimiter = mock<RateLimiter>()
446+
whenever(rateLimiter.isActiveForCategory(DataCategory.ProfileChunk)).thenReturn(true)
447+
whenever(fixture.scopes.rateLimiter).thenReturn(rateLimiter)
448+
449+
// If the SDK is rate limited, the profiler should never start
450+
profiler.start()
451+
assertFalse(profiler.isRunning)
452+
assertEquals(SentryId.EMPTY_ID, profiler.profilerId)
453+
verify(fixture.mockLogger).log(eq(SentryLevel.WARNING), eq("SDK is rate limited. Stopping profiler."))
454+
}
455+
456+
@Test
457+
fun `profiler does not start when offline`() {
458+
val executorService = DeferredExecutorService()
459+
val profiler = fixture.getSut {
460+
it.executorService = executorService
461+
it.connectionStatusProvider = mock { provider ->
462+
whenever(provider.connectionStatus).thenReturn(IConnectionStatusProvider.ConnectionStatus.DISCONNECTED)
463+
}
464+
}
465+
466+
// If the device is offline, the profiler should never start
467+
profiler.start()
468+
assertFalse(profiler.isRunning)
469+
assertEquals(SentryId.EMPTY_ID, profiler.profilerId)
470+
verify(fixture.mockLogger).log(eq(SentryLevel.WARNING), eq("Device is offline. Stopping profiler."))
471+
}
397472
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public final class io/sentry/opentelemetry/OtelSpanFactory : io/sentry/ISpanFact
4141
public fun <init> ()V
4242
public fun <init> (Lio/opentelemetry/api/OpenTelemetry;)V
4343
public fun createSpan (Lio/sentry/IScopes;Lio/sentry/SpanOptions;Lio/sentry/SpanContext;Lio/sentry/ISpan;)Lio/sentry/ISpan;
44-
public fun createTransaction (Lio/sentry/TransactionContext;Lio/sentry/IScopes;Lio/sentry/TransactionOptions;Lio/sentry/TransactionPerformanceCollector;)Lio/sentry/ITransaction;
44+
public fun createTransaction (Lio/sentry/TransactionContext;Lio/sentry/IScopes;Lio/sentry/TransactionOptions;Lio/sentry/CompositePerformanceCollector;)Lio/sentry/ITransaction;
4545
}
4646

4747
public final class io/sentry/opentelemetry/OtelTransactionSpanForwarder : io/sentry/ITransaction {

‎sentry/api/sentry.api‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,7 @@ public final class io/sentry/DataCategory : java/lang/Enum {
341341
public static final field Error Lio/sentry/DataCategory;
342342
public static final field Monitor Lio/sentry/DataCategory;
343343
public static final field Profile Lio/sentry/DataCategory;
344+
public static final field ProfileChunk Lio/sentry/DataCategory;
344345
public static final field Replay Lio/sentry/DataCategory;
345346
public static final field Security Lio/sentry/DataCategory;
346347
public static final field Session Lio/sentry/DataCategory;

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ public enum DataCategory {
1212
Attachment("attachment"),
1313
Monitor("monitor"),
1414
Profile("profile"),
15+
ProfileChunk("profile_chunk"),
1516
Transaction("transaction"),
1617
Replay("replay"),
1718
Span("span"),

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,9 @@ private DataCategory categoryFromItemType(SentryItemType itemType) {
165165
if (SentryItemType.Profile.equals(itemType)) {
166166
return DataCategory.Profile;
167167
}
168+
if (SentryItemType.ProfileChunk.equals(itemType)) {
169+
return DataCategory.ProfileChunk;
170+
}
168171
if (SentryItemType.Attachment.equals(itemType)) {
169172
return DataCategory.Attachment;
170173
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,8 @@ private boolean isRetryAfter(final @NotNull String itemType) {
184184
return DataCategory.Attachment;
185185
case "profile":
186186
return DataCategory.Profile;
187+
case "profile_chunk":
188+
return DataCategory.ProfileChunk;
187189
case "transaction":
188190
return DataCategory.Transaction;
189191
case "check_in":

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

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import io.sentry.ILogger
1010
import io.sentry.IScopes
1111
import io.sentry.ISerializer
1212
import io.sentry.NoOpLogger
13+
import io.sentry.ProfileChunk
1314
import io.sentry.ProfilingTraceData
1415
import io.sentry.ReplayRecording
1516
import io.sentry.SentryEnvelope
@@ -208,8 +209,9 @@ class RateLimiterTest {
208209
val attachmentItem = SentryEnvelopeItem.fromAttachment(fixture.serializer, NoOpLogger.getInstance(), Attachment("{ \"number\": 10 }".toByteArray(), "log.json"), 1000)
209210
val profileItem = SentryEnvelopeItem.fromProfilingTrace(ProfilingTraceData(File(""), transaction), 1000, fixture.serializer)
210211
val checkInItem = SentryEnvelopeItem.fromCheckIn(fixture.serializer, CheckIn("monitor-slug-1", CheckInStatus.ERROR))
212+
val profileChunkItem = SentryEnvelopeItem.fromProfileChunk(ProfileChunk(), fixture.serializer)
211213

212-
val envelope = SentryEnvelope(SentryEnvelopeHeader(), arrayListOf(eventItem, userFeedbackItem, sessionItem, attachmentItem, profileItem, checkInItem))
214+
val envelope = SentryEnvelope(SentryEnvelopeHeader(), arrayListOf(eventItem, userFeedbackItem, sessionItem, attachmentItem, profileItem, checkInItem, profileChunkItem))
213215

214216
rateLimiter.updateRetryAfterLimits(null, null, 429)
215217
val result = rateLimiter.filter(envelope, Hint())
@@ -222,6 +224,7 @@ class RateLimiterTest {
222224
verify(fixture.clientReportRecorder, times(1)).recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(attachmentItem))
223225
verify(fixture.clientReportRecorder, times(1)).recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(profileItem))
224226
verify(fixture.clientReportRecorder, times(1)).recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(checkInItem))
227+
verify(fixture.clientReportRecorder, times(1)).recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(profileChunkItem))
225228
verifyNoMoreInteractions(fixture.clientReportRecorder)
226229
}
227230

@@ -331,6 +334,24 @@ class RateLimiterTest {
331334
verifyNoMoreInteractions(fixture.clientReportRecorder)
332335
}
333336

337+
@Test
338+
fun `drop profileChunk items as lost`() {
339+
val rateLimiter = fixture.getSUT()
340+
341+
val profileChunkItem = SentryEnvelopeItem.fromProfileChunk(ProfileChunk(), fixture.serializer)
342+
val attachmentItem = SentryEnvelopeItem.fromAttachment(fixture.serializer, NoOpLogger.getInstance(), Attachment("{ \"number\": 10 }".toByteArray(), "log.json"), 1000)
343+
val envelope = SentryEnvelope(SentryEnvelopeHeader(), arrayListOf(profileChunkItem, attachmentItem))
344+
345+
rateLimiter.updateRetryAfterLimits("60:profile_chunk:key", null, 1)
346+
val result = rateLimiter.filter(envelope, Hint())
347+
348+
assertNotNull(result)
349+
assertEquals(1, result.items.toList().size)
350+
351+
verify(fixture.clientReportRecorder, times(1)).recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(profileChunkItem))
352+
verifyNoMoreInteractions(fixture.clientReportRecorder)
353+
}
354+
334355
@Test
335356
fun `apply rate limits notifies observers`() {
336357
val rateLimiter = fixture.getSUT()

0 commit comments

Comments
 (0)