diff --git a/CHANGELOG.md b/CHANGELOG.md index 41b74388723..6fe0884a820 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ ### Fixes +- Skip encoding and capturing buffered session replay segments while rate-limited, so we don't waste resources on envelopes the transport will drop ([#5813](https://github.com/getsentry/sentry-java/pull/5813)) + - These skipped replays are now reported as `ratelimit_backoff` discarded events in client reports, so they no longer disappear from drop statistics. One event is recorded per buffer flush rather than per segment. + - Buffer mode is also kept while rate-limited instead of switching to session mode, so the rolling buffer stays warm and the next error after the rate limit expires can send a complete replay. - Reduce main-thread work during `Sentry.init` by resolving the shake-detector accelerometer off the main thread (~1.75ms on a Pixel 10) ([#5784](https://github.com/getsentry/sentry-java/pull/5784)) - Backfill release, environment, distribution, tags, and app version/build—and use the matching replay-on-error sample rate—for `ApplicationExitInfo` ANR and native crash events captured before SDK initialization, without reusing options cached by a later app update ([#5762](https://github.com/getsentry/sentry-java/pull/5762)) - `SentryTagModifierNode.isImportantForBounds` now matches the default behavior and returns `true` ([#5789](https://github.com/getsentry/sentry-java/pull/5789)) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt index f6c6f3997ae..1d7bd1c1126 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt @@ -4,6 +4,8 @@ import android.annotation.SuppressLint import android.annotation.TargetApi import android.graphics.Bitmap import android.view.MotionEvent +import io.sentry.DataCategory.All +import io.sentry.DataCategory.Replay import io.sentry.DateUtils import io.sentry.IScopes import io.sentry.SentryLevel.DEBUG @@ -17,6 +19,7 @@ import io.sentry.android.replay.capture.CaptureStrategy.Companion.rotateEvents import io.sentry.android.replay.capture.CaptureStrategy.ReplaySegment import io.sentry.android.replay.util.ReplayRunnable import io.sentry.android.replay.util.sample +import io.sentry.clientreport.DiscardReason.RATELIMIT_BACKOFF import io.sentry.protocol.SentryId import io.sentry.transport.ICurrentDateProvider import io.sentry.util.FileUtils @@ -100,6 +103,18 @@ internal class BufferCaptureStrategy( return } + if (isReplayRateLimited()) { + // the segment envelopes would be dropped by the transport anyway, so don't waste resources + // encoding videos that will only be discarded + options.logger.log(INFO, "Replay is rate-limited, not capturing for event") + // one lost event per flush, not per segment: the transport would have counted the current + // segment plus every buffered one, but a flush only ever loses a single replay from the + // user's perspective. Under-reporting here is preferable to making replay look like it + // dropped data it never held. + options.clientReportRecorder.recordLostEvent(RATELIMIT_BACKOFF, Replay) + return + } + createCurrentSegment("capture_replay") { segment -> bufferedSegments.capture() @@ -151,6 +166,13 @@ internal class BufferCaptureStrategy( ) return this } + if (isReplayRateLimited()) { + // captureReplay skipped the flush, so there is nothing to continue in session mode. Staying + // in buffer mode keeps the rolling buffer warm, so the next error after the rate limit + // expires can send a complete replay starting at segment 0. + options.logger.log(DEBUG, "Not converting to session mode, because replay is rate-limited") + return this + } // we hand over replayExecutor and persistingExecutor to the new strategy to preserve order of // execution val captureStrategy = @@ -170,6 +192,11 @@ internal class BufferCaptureStrategy( rotateEvents(currentEvents, bufferLimit) } + private fun isReplayRateLimited(): Boolean = + scopes?.rateLimiter?.let { + it.isActiveForCategory(All) || it.isActiveForCategory(Replay) + } == true + private fun deleteFile(file: File?) { if (file == null) { return diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/BufferCaptureStrategyTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/BufferCaptureStrategyTest.kt index b5048e856ff..fc1981a84b1 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/BufferCaptureStrategyTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/BufferCaptureStrategyTest.kt @@ -2,9 +2,12 @@ package io.sentry.android.replay.capture import android.graphics.Bitmap import android.view.MotionEvent +import io.sentry.DataCategory import io.sentry.IScopes import io.sentry.Scope import io.sentry.ScopeCallback +import io.sentry.SentryEnvelope +import io.sentry.SentryEnvelopeHeader import io.sentry.SentryOptions import io.sentry.SentryReplayEvent.ReplayType import io.sentry.android.replay.DefaultReplayBreadcrumbConverter @@ -17,9 +20,12 @@ import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_TIMESTAMP import io.sentry.android.replay.ReplayFrame import io.sentry.android.replay.ScreenshotRecorderConfig import io.sentry.android.replay.capture.BufferCaptureStrategyTest.Fixture.Companion.VIDEO_DURATION +import io.sentry.clientreport.DiscardReason +import io.sentry.clientreport.DiscardedEvent import io.sentry.protocol.SentryId import io.sentry.transport.CurrentDateProvider import io.sentry.transport.ICurrentDateProvider +import io.sentry.transport.RateLimiter import io.sentry.util.Random import java.io.File import kotlin.test.Test @@ -93,6 +99,16 @@ class BufferCaptureStrategyTest { bitRate = 20_000, ) + // client report counts are only readable by draining them onto an envelope + fun discardedEvents(): List = + options.clientReportRecorder + .attachReportToEnvelope(SentryEnvelope(SentryEnvelopeHeader(), emptyList())) + .items + .firstOrNull() + ?.getClientReport(options.serializer) + ?.discardedEvents + .orEmpty() + fun getSut( onErrorSampleRate: Double = 1.0, dateProvider: ICurrentDateProvider = CurrentDateProvider.getInstance(), @@ -239,6 +255,19 @@ class BufferCaptureStrategyTest { assertTrue(converted is BufferCaptureStrategy) } + @Test + fun `convert stays in buffer mode when rate-limited`() { + val rateLimiter = mock { on { isActiveForCategory(any()) }.thenReturn(true) } + whenever(fixture.scopes.rateLimiter).thenReturn(rateLimiter) + val strategy = fixture.getSut() + strategy.start() + + strategy.captureReplay(false) {} + + val converted = strategy.convert() + assertTrue(converted is BufferCaptureStrategy) + } + @Test fun `convert converts to session strategy and sets replayId to scope`() { val strategy = fixture.getSut() @@ -336,6 +365,52 @@ class BufferCaptureStrategyTest { assertEquals(SentryId.EMPTY_ID, fixture.scope.replayId) } + @Test + fun `captureReplay does not capture segments when rate-limited`() { + val rateLimiter = mock { on { isActiveForCategory(any()) }.thenReturn(true) } + whenever(fixture.scopes.rateLimiter).thenReturn(rateLimiter) + val strategy = fixture.getSut() + strategy.start() + strategy.onConfigurationChanged(fixture.recorderConfig) + strategy.pause() + + strategy.captureReplay(false) {} + + // neither the current nor the buffered segment should be sent while rate-limited + verify(fixture.scopes, never()).captureReplay(any(), any()) + // the replayId is still set on the scope so the error that flushed the buffer stays linked to + // the replay that gets recorded once the rate limit lifts + assertEquals(strategy.currentReplayId, fixture.scope.replayId) + } + + @Test + fun `captureReplay records a lost replay event when rate-limited`() { + val rateLimiter = mock { on { isActiveForCategory(any()) }.thenReturn(true) } + whenever(fixture.scopes.rateLimiter).thenReturn(rateLimiter) + val strategy = fixture.getSut() + strategy.start() + strategy.onConfigurationChanged(fixture.recorderConfig) + strategy.pause() + + strategy.captureReplay(false) {} + + val discarded = fixture.discardedEvents() + assertEquals(1, discarded.size) + assertEquals(DiscardReason.RATELIMIT_BACKOFF.reason, discarded.first().reason) + assertEquals(DataCategory.Replay.category, discarded.first().category) + } + + @Test + fun `captureReplay does not record a lost replay event when not rate-limited`() { + val strategy = fixture.getSut() + strategy.start() + strategy.onConfigurationChanged(fixture.recorderConfig) + + strategy.captureReplay(false) {} + + assertTrue(fixture.discardedEvents().none { it.category == DataCategory.Replay.category }) + } + @Test fun `captureReplay sets replayId to scope and captures buffered segments`() { var called = false