diff --git a/CHANGELOG.md b/CHANGELOG.md index ba0e84d5d68..3163749f065 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - Ensure android initialization process continues even if options configuration block throws an exception ([#3887](https://github.com/getsentry/sentry-java/pull/3887)) - Do not report parsing ANR error when there are no threads ([#3888](https://github.com/getsentry/sentry-java/pull/3888)) - This should significantly reduce the number of events with message "Sentry Android SDK failed to parse system thread dump..." reported +- Session Replay: Disable replay in session mode when rate limit is active ([#3854](https://github.com/getsentry/sentry-java/pull/3854)) ### Dependencies diff --git a/sentry-android-replay/api/sentry-android-replay.api b/sentry-android-replay/api/sentry-android-replay.api index a08fb1dd988..7e2db5248f0 100644 --- a/sentry-android-replay/api/sentry-android-replay.api +++ b/sentry-android-replay/api/sentry-android-replay.api @@ -57,7 +57,7 @@ public final class io/sentry/android/replay/ReplayCache$Companion { public final fun makeReplayCacheDir (Lio/sentry/SentryOptions;Lio/sentry/protocol/SentryId;)Ljava/io/File; } -public final class io/sentry/android/replay/ReplayIntegration : android/content/ComponentCallbacks, io/sentry/Integration, io/sentry/ReplayController, io/sentry/android/replay/ScreenshotRecorderCallback, io/sentry/android/replay/gestures/TouchRecorderCallback, java/io/Closeable { +public final class io/sentry/android/replay/ReplayIntegration : android/content/ComponentCallbacks, io/sentry/IConnectionStatusProvider$IConnectionStatusObserver, io/sentry/Integration, io/sentry/ReplayController, io/sentry/android/replay/ScreenshotRecorderCallback, io/sentry/android/replay/gestures/TouchRecorderCallback, io/sentry/transport/RateLimiter$IRateLimitObserver, java/io/Closeable { public static final field $stable I public fun (Landroid/content/Context;Lio/sentry/transport/ICurrentDateProvider;)V public fun (Landroid/content/Context;Lio/sentry/transport/ICurrentDateProvider;Lkotlin/jvm/functions/Function0;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function2;)V @@ -69,7 +69,9 @@ public final class io/sentry/android/replay/ReplayIntegration : android/content/ public fun getReplayId ()Lio/sentry/protocol/SentryId; public fun isRecording ()Z public fun onConfigurationChanged (Landroid/content/res/Configuration;)V + public fun onConnectionStatusChanged (Lio/sentry/IConnectionStatusProvider$ConnectionStatus;)V public fun onLowMemory ()V + public fun onRateLimitChanged (Lio/sentry/transport/RateLimiter;)V public fun onScreenshotRecorded (Landroid/graphics/Bitmap;)V public fun onScreenshotRecorded (Ljava/io/File;J)V public fun onTouchEvent (Landroid/view/MotionEvent;)V diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt index 90832585cd8..d644ad096a6 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt @@ -7,6 +7,11 @@ import android.graphics.Bitmap import android.os.Build import android.view.MotionEvent import io.sentry.Breadcrumb +import io.sentry.DataCategory.All +import io.sentry.DataCategory.Replay +import io.sentry.IConnectionStatusProvider.ConnectionStatus +import io.sentry.IConnectionStatusProvider.ConnectionStatus.DISCONNECTED +import io.sentry.IConnectionStatusProvider.IConnectionStatusObserver import io.sentry.IHub import io.sentry.Integration import io.sentry.NoOpReplayBreadcrumbConverter @@ -32,6 +37,8 @@ import io.sentry.cache.PersistingScopeObserver.REPLAY_FILENAME import io.sentry.hints.Backfillable import io.sentry.protocol.SentryId import io.sentry.transport.ICurrentDateProvider +import io.sentry.transport.RateLimiter +import io.sentry.transport.RateLimiter.IRateLimitObserver import io.sentry.util.FileUtils import io.sentry.util.HintUtils import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion @@ -48,7 +55,14 @@ public class ReplayIntegration( private val recorderProvider: (() -> Recorder)? = null, private val recorderConfigProvider: ((configChanged: Boolean) -> ScreenshotRecorderConfig)? = null, private val replayCacheProvider: ((replayId: SentryId, recorderConfig: ScreenshotRecorderConfig) -> ReplayCache)? = null -) : Integration, Closeable, ScreenshotRecorderCallback, TouchRecorderCallback, ReplayController, ComponentCallbacks { +) : Integration, + Closeable, + ScreenshotRecorderCallback, + TouchRecorderCallback, + ReplayController, + ComponentCallbacks, + IConnectionStatusObserver, + IRateLimitObserver { // needed for the Java's call site constructor(context: Context, dateProvider: ICurrentDateProvider) : this( @@ -113,6 +127,8 @@ public class ReplayIntegration( gestureRecorder = gestureRecorderProvider?.invoke() ?: GestureRecorder(options, this) isEnabled.set(true) + options.connectionStatusProvider.addConnectionStatusObserver(this) + hub.rateLimiter?.addRateLimitObserver(this) try { context.registerComponentCallbacks(this) } catch (e: Throwable) { @@ -222,12 +238,14 @@ public class ReplayIntegration( hub?.configureScope { screen = it.screen?.substringAfterLast('.') } captureStrategy?.onScreenshotRecorded(bitmap) { frameTimeStamp -> addFrame(bitmap, frameTimeStamp, screen) + checkCanRecord() } } override fun onScreenshotRecorded(screenshot: File, frameTimestamp: Long) { captureStrategy?.onScreenshotRecorded { _ -> addFrame(screenshot, frameTimestamp) + checkCanRecord() } } @@ -236,6 +254,8 @@ public class ReplayIntegration( return } + options.connectionStatusProvider.removeConnectionStatusObserver(this) + hub?.rateLimiter?.removeRateLimitObserver(this) try { context.unregisterComponentCallbacks(this) } catch (ignored: Throwable) { @@ -259,12 +279,55 @@ public class ReplayIntegration( recorder?.start(recorderConfig) } + override fun onConnectionStatusChanged(status: ConnectionStatus) { + if (captureStrategy !is SessionCaptureStrategy) { + // we only want to stop recording when offline for session mode + return + } + + if (status == DISCONNECTED) { + pause() + } else { + // being positive for other states, even if it's NO_PERMISSION + resume() + } + } + + override fun onRateLimitChanged(rateLimiter: RateLimiter) { + if (captureStrategy !is SessionCaptureStrategy) { + // we only want to stop recording when rate-limited for session mode + return + } + + if (rateLimiter.isActiveForCategory(All) || rateLimiter.isActiveForCategory(Replay)) { + pause() + } else { + resume() + } + } + override fun onLowMemory() = Unit override fun onTouchEvent(event: MotionEvent) { captureStrategy?.onTouchEvent(event) } + /** + * Check if we're offline or rate-limited and pause for session mode to not overflow the + * envelope cache. + */ + private fun checkCanRecord() { + if (captureStrategy is SessionCaptureStrategy && + ( + options.connectionStatusProvider.connectionStatus == DISCONNECTED || + hub?.rateLimiter?.isActiveForCategory(All) == true || + hub?.rateLimiter?.isActiveForCategory(Replay) == true + ) + ) { + pause() + } + } + private fun registerRootViewListeners() { if (recorder is OnRootViewsChangedListener) { rootViewsSpy.listeners += (recorder as OnRootViewsChangedListener) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/ScreenshotRecorder.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/ScreenshotRecorder.kt index 54f92a89587..60249103d09 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/ScreenshotRecorder.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/ScreenshotRecorder.kt @@ -35,7 +35,6 @@ import java.lang.ref.WeakReference import java.util.concurrent.Executors import java.util.concurrent.ThreadFactory import java.util.concurrent.atomic.AtomicBoolean -import java.util.concurrent.atomic.AtomicReference import kotlin.LazyThreadSafetyMode.NONE import kotlin.math.roundToInt @@ -51,7 +50,6 @@ internal class ScreenshotRecorder( Executors.newSingleThreadScheduledExecutor(RecorderExecutorServiceThreadFactory()) } private var rootView: WeakReference? = null - private val pendingViewHierarchy = AtomicReference() private val maskingPaint by lazy(NONE) { Paint() } private val singlePixelBitmap: Bitmap by lazy(NONE) { Bitmap.createBitmap( @@ -230,7 +228,6 @@ internal class ScreenshotRecorder( unbind(rootView?.get()) rootView?.clear() lastScreenshot?.recycle() - pendingViewHierarchy.set(null) isCapturing.set(false) recorder.gracefullyShutdown(options) } diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/CaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/CaptureStrategy.kt index 7e9168df227..53f5c666831 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/CaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/CaptureStrategy.kt @@ -56,6 +56,7 @@ internal interface CaptureStrategy { fun close() companion object { + private const val BREADCRUMB_START_OFFSET = 100L internal val currentEventsLock = Any() fun createSegment( @@ -161,7 +162,10 @@ internal interface CaptureStrategy { val urls = LinkedList() breadcrumbs.forEach { breadcrumb -> - if (breadcrumb.timestamp.time >= segmentTimestamp.time && + // we add some fixed breadcrumb offset to make sure we don't miss any + // breadcrumbs that might be relevant for the current segment, but just happened + // earlier than the current segment (e.g. network connectivity changed) + if ((breadcrumb.timestamp.time + BREADCRUMB_START_OFFSET) >= segmentTimestamp.time && breadcrumb.timestamp.time < endTimestamp.time ) { val rrwebEvent = options diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt index 7b416d18b7d..08aec7e9f7a 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt @@ -1,7 +1,6 @@ package io.sentry.android.replay.capture import android.graphics.Bitmap -import io.sentry.IConnectionStatusProvider.ConnectionStatus.DISCONNECTED import io.sentry.IHub import io.sentry.SentryLevel.DEBUG import io.sentry.SentryLevel.INFO @@ -73,11 +72,6 @@ internal class SessionCaptureStrategy( } override fun onScreenshotRecorded(bitmap: Bitmap?, store: ReplayCache.(frameTimestamp: Long) -> Unit) { - if (options.connectionStatusProvider.connectionStatus == DISCONNECTED) { - options.logger.log(DEBUG, "Skipping screenshot recording, no internet connection") - bitmap?.recycle() - return - } // have to do it before submitting, otherwise if the queue is busy, the timestamp won't be // reflecting the exact time of when it was captured val frameTimestamp = dateProvider.currentTimeMillis diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt index f503268dfff..ff396d04c10 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt @@ -9,6 +9,8 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import io.sentry.Breadcrumb import io.sentry.DateUtils import io.sentry.Hint +import io.sentry.IConnectionStatusProvider.ConnectionStatus.CONNECTED +import io.sentry.IConnectionStatusProvider.ConnectionStatus.DISCONNECTED import io.sentry.IHub import io.sentry.Scope import io.sentry.ScopeCallback @@ -26,6 +28,7 @@ import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_REPLAY_TYPE import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_TIMESTAMP import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_WIDTH import io.sentry.android.replay.capture.CaptureStrategy +import io.sentry.android.replay.capture.SessionCaptureStrategy import io.sentry.android.replay.capture.SessionCaptureStrategyTest.Fixture.Companion.VIDEO_DURATION import io.sentry.android.replay.gestures.GestureRecorder import io.sentry.cache.PersistingScopeObserver @@ -38,6 +41,7 @@ import io.sentry.rrweb.RRWebMetaEvent import io.sentry.rrweb.RRWebVideoEvent import io.sentry.transport.CurrentDateProvider import io.sentry.transport.ICurrentDateProvider +import io.sentry.transport.RateLimiter import org.junit.Rule import org.junit.rules.TemporaryFolder import org.junit.runner.RunWith @@ -82,10 +86,12 @@ class ReplayIntegrationTest { } } val scope = Scope(options) + val rateLimiter = mock() val hub = mock { doAnswer { ((it.arguments[0]) as ScopeCallback).run(scope) }.whenever(mock).configureScope(any()) + on { rateLimiter }.thenReturn(rateLimiter) } val replayCache = mock { @@ -98,6 +104,8 @@ class ReplayIntegrationTest { context: Context, sessionSampleRate: Double = 1.0, onErrorSampleRate: Double = 1.0, + isOffline: Boolean = false, + isRateLimited: Boolean = false, recorderProvider: (() -> Recorder)? = null, replayCaptureStrategyProvider: ((isFullSession: Boolean) -> CaptureStrategy)? = null, recorderConfigProvider: ((configChanged: Boolean) -> ScreenshotRecorderConfig)? = null, @@ -107,6 +115,12 @@ class ReplayIntegrationTest { options.run { experimental.sessionReplay.onErrorSampleRate = onErrorSampleRate experimental.sessionReplay.sessionSampleRate = sessionSampleRate + connectionStatusProvider = mock { + on { connectionStatus }.thenReturn(if (isOffline) DISCONNECTED else CONNECTED) + } + } + if (isRateLimited) { + whenever(rateLimiter.isActiveForCategory(any())).thenReturn(true) } return ReplayIntegration( context, @@ -567,4 +581,123 @@ class ReplayIntegrationTest { verify(fixture.replayCache).addFrame(any(), any(), eq("MainActivity")) } + + @Test + fun `onScreenshotRecorded pauses replay when offline for sessions`() { + val captureStrategy = getSessionCaptureStrategy(fixture.options) + val recorder = mock() + val replay = fixture.getSut( + context, + recorderProvider = { recorder }, + replayCaptureStrategyProvider = { captureStrategy }, + isOffline = true + ) + + replay.register(fixture.hub, fixture.options) + replay.start() + replay.onScreenshotRecorded(mock()) + + verify(recorder).pause() + } + + @Test + fun `onScreenshotRecorded pauses replay when rate-limited for sessions`() { + val captureStrategy = getSessionCaptureStrategy(fixture.options) + val recorder = mock() + val replay = fixture.getSut( + context, + recorderProvider = { recorder }, + replayCaptureStrategyProvider = { captureStrategy }, + isRateLimited = true + ) + + replay.register(fixture.hub, fixture.options) + replay.start() + replay.onScreenshotRecorded(mock()) + + verify(recorder).pause() + } + + @Test + fun `onConnectionStatusChanged pauses replay when offline for sessions`() { + val captureStrategy = getSessionCaptureStrategy(fixture.options) + val recorder = mock() + val replay = fixture.getSut( + context, + recorderProvider = { recorder }, + replayCaptureStrategyProvider = { captureStrategy } + ) + + replay.register(fixture.hub, fixture.options) + replay.start() + replay.onConnectionStatusChanged(DISCONNECTED) + + verify(recorder).pause() + } + + @Test + fun `onConnectionStatusChanged resumes replay when back-online for sessions`() { + val captureStrategy = getSessionCaptureStrategy(fixture.options) + val recorder = mock() + val replay = fixture.getSut( + context, + recorderProvider = { recorder }, + replayCaptureStrategyProvider = { captureStrategy } + ) + + replay.register(fixture.hub, fixture.options) + replay.start() + replay.onConnectionStatusChanged(CONNECTED) + + verify(recorder).resume() + } + + @Test + fun `onRateLimitChanged pauses replay when rate-limited for sessions`() { + val captureStrategy = getSessionCaptureStrategy(fixture.options) + val recorder = mock() + val replay = fixture.getSut( + context, + recorderProvider = { recorder }, + replayCaptureStrategyProvider = { captureStrategy }, + isRateLimited = true + ) + + replay.register(fixture.hub, fixture.options) + replay.start() + replay.onRateLimitChanged(fixture.rateLimiter) + + verify(recorder).pause() + } + + @Test + fun `onRateLimitChanged resumes replay when rate-limit lifted for sessions`() { + val captureStrategy = getSessionCaptureStrategy(fixture.options) + val recorder = mock() + val replay = fixture.getSut( + context, + recorderProvider = { recorder }, + replayCaptureStrategyProvider = { captureStrategy }, + isRateLimited = false + ) + + replay.register(fixture.hub, fixture.options) + replay.start() + replay.onRateLimitChanged(fixture.rateLimiter) + + verify(recorder).resume() + } + + private fun getSessionCaptureStrategy(options: SentryOptions): SessionCaptureStrategy { + return SessionCaptureStrategy( + options, + null, + CurrentDateProvider.getInstance(), + executor = mock { + doAnswer { + (it.arguments[0] as Runnable).run() + }.whenever(mock).submit(any()) + } + ) + } } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 8e266bcdba6..c356b298b68 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -5564,15 +5564,22 @@ public final class io/sentry/transport/NoOpTransportGate : io/sentry/transport/I public fun isConnected ()Z } -public final class io/sentry/transport/RateLimiter { +public final class io/sentry/transport/RateLimiter : java/io/Closeable { public fun (Lio/sentry/SentryOptions;)V public fun (Lio/sentry/transport/ICurrentDateProvider;Lio/sentry/SentryOptions;)V + public fun addRateLimitObserver (Lio/sentry/transport/RateLimiter$IRateLimitObserver;)V + public fun close ()V public fun filter (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)Lio/sentry/SentryEnvelope; public fun isActiveForCategory (Lio/sentry/DataCategory;)Z public fun isAnyRateLimitActive ()Z + public fun removeRateLimitObserver (Lio/sentry/transport/RateLimiter$IRateLimitObserver;)V public fun updateRetryAfterLimits (Ljava/lang/String;Ljava/lang/String;I)V } +public abstract interface class io/sentry/transport/RateLimiter$IRateLimitObserver { + public abstract fun onRateLimitChanged (Lio/sentry/transport/RateLimiter;)V +} + public final class io/sentry/transport/ReusableCountLatch { public fun ()V public fun (I)V diff --git a/sentry/src/main/java/io/sentry/clientreport/ClientReportRecorder.java b/sentry/src/main/java/io/sentry/clientreport/ClientReportRecorder.java index 796a17cb3c4..a88df2824fd 100644 --- a/sentry/src/main/java/io/sentry/clientreport/ClientReportRecorder.java +++ b/sentry/src/main/java/io/sentry/clientreport/ClientReportRecorder.java @@ -174,6 +174,9 @@ private DataCategory categoryFromItemType(SentryItemType itemType) { if (SentryItemType.CheckIn.equals(itemType)) { return DataCategory.Monitor; } + if (SentryItemType.ReplayVideo.equals(itemType)) { + return DataCategory.Replay; + } return DataCategory.Default; } diff --git a/sentry/src/main/java/io/sentry/transport/AsyncHttpTransport.java b/sentry/src/main/java/io/sentry/transport/AsyncHttpTransport.java index 985bbcccbbe..24f954c0c10 100644 --- a/sentry/src/main/java/io/sentry/transport/AsyncHttpTransport.java +++ b/sentry/src/main/java/io/sentry/transport/AsyncHttpTransport.java @@ -170,6 +170,7 @@ public void close() throws IOException { @Override public void close(final boolean isRestarting) throws IOException { + rateLimiter.close(); executor.shutdown(); options.getLogger().log(SentryLevel.DEBUG, "Shutting down"); try { diff --git a/sentry/src/main/java/io/sentry/transport/RateLimiter.java b/sentry/src/main/java/io/sentry/transport/RateLimiter.java index c4598820abf..191e8cbe7ef 100644 --- a/sentry/src/main/java/io/sentry/transport/RateLimiter.java +++ b/sentry/src/main/java/io/sentry/transport/RateLimiter.java @@ -15,16 +15,21 @@ import io.sentry.util.CollectionUtils; import io.sentry.util.HintUtils; import io.sentry.util.StringUtils; +import java.io.Closeable; +import java.io.IOException; import java.util.ArrayList; import java.util.Date; import java.util.List; import java.util.Map; +import java.util.Timer; +import java.util.TimerTask; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; /** Controls retry limits on different category types sent to Sentry. */ -public final class RateLimiter { +public final class RateLimiter implements Closeable { private static final int HTTP_RETRY_AFTER_DEFAULT_DELAY_MILLIS = 60000; @@ -32,6 +37,9 @@ public final class RateLimiter { private final @NotNull SentryOptions options; private final @NotNull Map sentryRetryAfterLimit = new ConcurrentHashMap<>(); + private final @NotNull List rateLimitObservers = new CopyOnWriteArrayList<>(); + private @Nullable Timer timer = null; + private final @NotNull Object timerLock = new Object(); public RateLimiter( final @NotNull ICurrentDateProvider currentDateProvider, @@ -177,6 +185,8 @@ private boolean isRetryAfter(final @NotNull String itemType) { return DataCategory.Transaction; case "check_in": return DataCategory.Monitor; + case "replay_video": + return DataCategory.Replay; default: return DataCategory.Unknown; } @@ -280,6 +290,23 @@ private void applyRetryAfterOnlyIfLonger( // only overwrite its previous date if the limit is even longer if (oldDate == null || date.after(oldDate)) { sentryRetryAfterLimit.put(dataCategory, date); + + notifyRateLimitObservers(); + + synchronized (timerLock) { + if (timer == null) { + timer = new Timer(true); + } + + timer.schedule( + new TimerTask() { + @Override + public void run() { + notifyRateLimitObservers(); + } + }, + date); + } } } @@ -301,4 +328,41 @@ private long parseRetryAfterOrDefault(final @Nullable String retryAfterHeader) { } return retryAfterMillis; } + + private void notifyRateLimitObservers() { + for (IRateLimitObserver observer : rateLimitObservers) { + observer.onRateLimitChanged(this); + } + } + + public void addRateLimitObserver(@NotNull final IRateLimitObserver observer) { + rateLimitObservers.add(observer); + } + + public void removeRateLimitObserver(@NotNull final IRateLimitObserver observer) { + rateLimitObservers.remove(observer); + } + + @Override + public void close() throws IOException { + synchronized (timerLock) { + if (timer != null) { + timer.cancel(); + timer = null; + } + } + rateLimitObservers.clear(); + } + + public interface IRateLimitObserver { + /** + * Invoked whenever the rate limit changed. You should use {@link + * RateLimiter#isActiveForCategory(DataCategory)} to check whether the category you're + * interested in has changed. + * + * @param rateLimiter this {@link RateLimiter} instance which you can use to check if the rate + * limit is active for a specific category + */ + void onRateLimitChanged(@NotNull RateLimiter rateLimiter); + } } diff --git a/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt b/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt index c06e8da1f6f..135cce944be 100644 --- a/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt +++ b/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt @@ -10,12 +10,14 @@ import io.sentry.Hint import io.sentry.IHub import io.sentry.NoOpLogger import io.sentry.ProfilingTraceData +import io.sentry.ReplayRecording import io.sentry.Sentry import io.sentry.SentryEnvelope import io.sentry.SentryEnvelopeHeader import io.sentry.SentryEnvelopeItem import io.sentry.SentryEvent import io.sentry.SentryOptions +import io.sentry.SentryReplayEvent import io.sentry.SentryTracer import io.sentry.Session import io.sentry.TracesSamplingDecision @@ -71,13 +73,14 @@ class ClientReportTest { SentryEnvelopeItem.fromAttachment(opts.serializer, NoOpLogger.getInstance(), Attachment("{ \"number\": 10 }".toByteArray(), "log.json"), 1000), SentryEnvelopeItem.fromProfilingTrace(ProfilingTraceData(File(""), transaction), 1000, opts.serializer), SentryEnvelopeItem.fromCheckIn(opts.serializer, CheckIn("monitor-slug-1", CheckInStatus.ERROR)), - SentryEnvelopeItem.fromMetrics(EncodedMetrics(emptyMap())) + SentryEnvelopeItem.fromMetrics(EncodedMetrics(emptyMap())), + SentryEnvelopeItem.fromReplay(opts.serializer, opts.logger, SentryReplayEvent(), ReplayRecording(), false) ) clientReportRecorder.recordLostEnvelope(DiscardReason.NETWORK_ERROR, envelope) val clientReportAtEnd = clientReportRecorder.resetCountsAndGenerateClientReport() - testHelper.assertTotalCount(15, clientReportAtEnd) + testHelper.assertTotalCount(16, clientReportAtEnd) testHelper.assertCountFor(DiscardReason.SAMPLE_RATE, DataCategory.Error, 3, clientReportAtEnd) testHelper.assertCountFor(DiscardReason.BEFORE_SEND, DataCategory.Error, 2, clientReportAtEnd) testHelper.assertCountFor(DiscardReason.QUEUE_OVERFLOW, DataCategory.Transaction, 1, clientReportAtEnd) @@ -90,6 +93,7 @@ class ClientReportTest { testHelper.assertCountFor(DiscardReason.NETWORK_ERROR, DataCategory.Profile, 1, clientReportAtEnd) testHelper.assertCountFor(DiscardReason.NETWORK_ERROR, DataCategory.Monitor, 1, clientReportAtEnd) testHelper.assertCountFor(DiscardReason.NETWORK_ERROR, DataCategory.MetricBucket, 1, clientReportAtEnd) + testHelper.assertCountFor(DiscardReason.NETWORK_ERROR, DataCategory.Replay, 1, clientReportAtEnd) } @Test diff --git a/sentry/src/test/java/io/sentry/transport/AsyncHttpTransportTest.kt b/sentry/src/test/java/io/sentry/transport/AsyncHttpTransportTest.kt index abaa965175c..ae479ed6cb8 100644 --- a/sentry/src/test/java/io/sentry/transport/AsyncHttpTransportTest.kt +++ b/sentry/src/test/java/io/sentry/transport/AsyncHttpTransportTest.kt @@ -329,6 +329,14 @@ class AsyncHttpTransportTest { ) } + @Test + fun `close closes the rate limiter`() { + val sut = fixture.getSUT() + sut.close() + + verify(fixture.rateLimiter).close() + } + @Test fun `close uses flushTimeoutMillis option to schedule termination`() { fixture.sentryOptions.flushTimeoutMillis = 123 diff --git a/sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt b/sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt index 8346ddf1023..8d8fb9601ef 100644 --- a/sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt +++ b/sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt @@ -3,17 +3,21 @@ package io.sentry.transport import io.sentry.Attachment import io.sentry.CheckIn import io.sentry.CheckInStatus +import io.sentry.DataCategory.Replay import io.sentry.Hint import io.sentry.IHub +import io.sentry.ILogger import io.sentry.ISerializer import io.sentry.NoOpLogger import io.sentry.ProfilingTraceData +import io.sentry.ReplayRecording import io.sentry.SentryEnvelope import io.sentry.SentryEnvelopeHeader import io.sentry.SentryEnvelopeItem import io.sentry.SentryEvent import io.sentry.SentryOptions import io.sentry.SentryOptionsManipulator +import io.sentry.SentryReplayEvent import io.sentry.SentryTracer import io.sentry.Session import io.sentry.TransactionContext @@ -24,6 +28,7 @@ import io.sentry.metrics.EncodedMetrics import io.sentry.protocol.SentryId import io.sentry.protocol.SentryTransaction import io.sentry.protocol.User +import org.awaitility.kotlin.await import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.same @@ -33,6 +38,7 @@ import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever import java.io.File import java.util.UUID +import java.util.concurrent.atomic.AtomicBoolean import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -354,4 +360,70 @@ class RateLimiterTest { assertTrue(rateLimiter.isAnyRateLimitActive) } + + @Test + fun `drop replay items as lost`() { + val rateLimiter = fixture.getSUT() + val hub = mock() + whenever(hub.options).thenReturn(SentryOptions()) + + val replayItem = SentryEnvelopeItem.fromReplay(fixture.serializer, mock(), SentryReplayEvent(), ReplayRecording(), false) + val attachmentItem = SentryEnvelopeItem.fromAttachment(fixture.serializer, NoOpLogger.getInstance(), Attachment("{ \"number\": 10 }".toByteArray(), "log.json"), 1000) + val envelope = SentryEnvelope(SentryEnvelopeHeader(), arrayListOf(replayItem, attachmentItem)) + + rateLimiter.updateRetryAfterLimits("60:replay: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(replayItem)) + verifyNoMoreInteractions(fixture.clientReportRecorder) + } + + @Test + fun `apply rate limits notifies observers`() { + val rateLimiter = fixture.getSUT() + + var applied = false + rateLimiter.addRateLimitObserver { + applied = rateLimiter.isActiveForCategory(Replay) + } + rateLimiter.updateRetryAfterLimits("60:replay:key", null, 1) + + assertTrue(applied) + } + + @Test + fun `apply rate limits schedules a timer to notify observers of lifted limits`() { + val rateLimiter = fixture.getSUT() + whenever(fixture.currentDateProvider.currentTimeMillis).thenReturn(0, 1, 2001) + + val applied = AtomicBoolean(true) + rateLimiter.addRateLimitObserver { + applied.set(rateLimiter.isActiveForCategory(Replay)) + } + rateLimiter.updateRetryAfterLimits("1:replay:key", null, 1) + + await.untilFalse(applied) + assertFalse(applied.get()) + } + + @Test + fun `close cancels the timer`() { + val rateLimiter = fixture.getSUT() + whenever(fixture.currentDateProvider.currentTimeMillis).thenReturn(0, 1, 2001) + + val applied = AtomicBoolean(true) + rateLimiter.addRateLimitObserver { + applied.set(rateLimiter.isActiveForCategory(Replay)) + } + + rateLimiter.updateRetryAfterLimits("1:replay:key", null, 1) + rateLimiter.close() + + // wait for 1.5s to ensure the timer has run after 1s + await.untilTrue(applied) + assertTrue(applied.get()) + } }