Skip to content

Commit 02b196f

Browse files
runningcodeclaude
andauthored
fix(replay): Skip buffer-mode replay capture when rate-limited (DART-313) (#5813)
* fix(replay): Skip buffer-mode replay capture when rate-limited (DART-313) In buffer (on-error) mode the recorder keeps running while rate-limited so the rolling buffer stays warm, but capturing on an error still encoded the current and buffered segments and handed them to the transport, which then dropped them. That wasted CPU, I/O, and MediaMuxer file descriptors on envelopes that could never be sent. Bail out of BufferCaptureStrategy.captureReplay when the Replay (or All) category is rate-limited, mirroring the guard session mode already applies. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * changelog * fix(replay): Record a lost replay event when buffer capture is rate-limited (DART-313) Skipping the encode when rate-limited meant the segments never reached the transport, so RateLimiter.filter never recorded them as lost. Replay drops in buffer mode silently vanished from client reports. Record a RATELIMIT_BACKOFF lost event for the Replay category at the bail-out, and move the rate-limit check below the sampling and isTerminating guards so we only report replays that would genuinely have been sent — a replay dropped by onErrorSampleRate is not a rate-limit loss, and a terminating one is deferred to the next launch rather than lost. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * changelog * fix(replay): Keep buffer mode while rate-limited (DART-313) Bailing out of BufferCaptureStrategy.captureReplay while rate-limited left isTerminating unset, so ReplayIntegration's unconditional convert() still swapped in a SessionCaptureStrategy. That discarded the rolling buffer, and the next recorded frame then hit checkCanRecord(), which pauses session mode when rate-limited and encodes a segment on the way out - exactly the work the bail-out was meant to avoid. It also left recording paused for the rest of the rate-limit window, contradicting onRateLimitChanged, which deliberately keeps buffer mode running. Stay in buffer mode while rate-limited so the buffer keeps rolling and the next error after the limit expires can send a complete replay. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent a30048b commit 02b196f

3 files changed

Lines changed: 105 additions & 0 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@
3737

3838
### Fixes
3939

40+
- 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))
41+
- 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.
42+
- 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.
4043
- 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))
4144
- 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))
4245
- `SentryTagModifierNode.isImportantForBounds` now matches the default behavior and returns `true` ([#5789](https://github.com/getsentry/sentry-java/pull/5789))

‎sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import android.annotation.SuppressLint
44
import android.annotation.TargetApi
55
import android.graphics.Bitmap
66
import android.view.MotionEvent
7+
import io.sentry.DataCategory.All
8+
import io.sentry.DataCategory.Replay
79
import io.sentry.DateUtils
810
import io.sentry.IScopes
911
import io.sentry.SentryLevel.DEBUG
@@ -17,6 +19,7 @@ import io.sentry.android.replay.capture.CaptureStrategy.Companion.rotateEvents
1719
import io.sentry.android.replay.capture.CaptureStrategy.ReplaySegment
1820
import io.sentry.android.replay.util.ReplayRunnable
1921
import io.sentry.android.replay.util.sample
22+
import io.sentry.clientreport.DiscardReason.RATELIMIT_BACKOFF
2023
import io.sentry.protocol.SentryId
2124
import io.sentry.transport.ICurrentDateProvider
2225
import io.sentry.util.FileUtils
@@ -100,6 +103,18 @@ internal class BufferCaptureStrategy(
100103
return
101104
}
102105

106+
if (isReplayRateLimited()) {
107+
// the segment envelopes would be dropped by the transport anyway, so don't waste resources
108+
// encoding videos that will only be discarded
109+
options.logger.log(INFO, "Replay is rate-limited, not capturing for event")
110+
// one lost event per flush, not per segment: the transport would have counted the current
111+
// segment plus every buffered one, but a flush only ever loses a single replay from the
112+
// user's perspective. Under-reporting here is preferable to making replay look like it
113+
// dropped data it never held.
114+
options.clientReportRecorder.recordLostEvent(RATELIMIT_BACKOFF, Replay)
115+
return
116+
}
117+
103118
createCurrentSegment("capture_replay") { segment ->
104119
bufferedSegments.capture()
105120

@@ -151,6 +166,13 @@ internal class BufferCaptureStrategy(
151166
)
152167
return this
153168
}
169+
if (isReplayRateLimited()) {
170+
// captureReplay skipped the flush, so there is nothing to continue in session mode. Staying
171+
// in buffer mode keeps the rolling buffer warm, so the next error after the rate limit
172+
// expires can send a complete replay starting at segment 0.
173+
options.logger.log(DEBUG, "Not converting to session mode, because replay is rate-limited")
174+
return this
175+
}
154176
// we hand over replayExecutor and persistingExecutor to the new strategy to preserve order of
155177
// execution
156178
val captureStrategy =
@@ -170,6 +192,11 @@ internal class BufferCaptureStrategy(
170192
rotateEvents(currentEvents, bufferLimit)
171193
}
172194

195+
private fun isReplayRateLimited(): Boolean =
196+
scopes?.rateLimiter?.let {
197+
it.isActiveForCategory(All) || it.isActiveForCategory(Replay)
198+
} == true
199+
173200
private fun deleteFile(file: File?) {
174201
if (file == null) {
175202
return

‎sentry-android-replay/src/test/java/io/sentry/android/replay/capture/BufferCaptureStrategyTest.kt‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,12 @@ package io.sentry.android.replay.capture
22

33
import android.graphics.Bitmap
44
import android.view.MotionEvent
5+
import io.sentry.DataCategory
56
import io.sentry.IScopes
67
import io.sentry.Scope
78
import io.sentry.ScopeCallback
9+
import io.sentry.SentryEnvelope
10+
import io.sentry.SentryEnvelopeHeader
811
import io.sentry.SentryOptions
912
import io.sentry.SentryReplayEvent.ReplayType
1013
import io.sentry.android.replay.DefaultReplayBreadcrumbConverter
@@ -17,9 +20,12 @@ import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_TIMESTAMP
1720
import io.sentry.android.replay.ReplayFrame
1821
import io.sentry.android.replay.ScreenshotRecorderConfig
1922
import io.sentry.android.replay.capture.BufferCaptureStrategyTest.Fixture.Companion.VIDEO_DURATION
23+
import io.sentry.clientreport.DiscardReason
24+
import io.sentry.clientreport.DiscardedEvent
2025
import io.sentry.protocol.SentryId
2126
import io.sentry.transport.CurrentDateProvider
2227
import io.sentry.transport.ICurrentDateProvider
28+
import io.sentry.transport.RateLimiter
2329
import io.sentry.util.Random
2430
import java.io.File
2531
import kotlin.test.Test
@@ -93,6 +99,16 @@ class BufferCaptureStrategyTest {
9399
bitRate = 20_000,
94100
)
95101

102+
// client report counts are only readable by draining them onto an envelope
103+
fun discardedEvents(): List<DiscardedEvent> =
104+
options.clientReportRecorder
105+
.attachReportToEnvelope(SentryEnvelope(SentryEnvelopeHeader(), emptyList()))
106+
.items
107+
.firstOrNull()
108+
?.getClientReport(options.serializer)
109+
?.discardedEvents
110+
.orEmpty()
111+
96112
fun getSut(
97113
onErrorSampleRate: Double = 1.0,
98114
dateProvider: ICurrentDateProvider = CurrentDateProvider.getInstance(),
@@ -239,6 +255,19 @@ class BufferCaptureStrategyTest {
239255
assertTrue(converted is BufferCaptureStrategy)
240256
}
241257

258+
@Test
259+
fun `convert stays in buffer mode when rate-limited`() {
260+
val rateLimiter = mock<RateLimiter> { on { isActiveForCategory(any()) }.thenReturn(true) }
261+
whenever(fixture.scopes.rateLimiter).thenReturn(rateLimiter)
262+
val strategy = fixture.getSut()
263+
strategy.start()
264+
265+
strategy.captureReplay(false) {}
266+
267+
val converted = strategy.convert()
268+
assertTrue(converted is BufferCaptureStrategy)
269+
}
270+
242271
@Test
243272
fun `convert converts to session strategy and sets replayId to scope`() {
244273
val strategy = fixture.getSut()
@@ -336,6 +365,52 @@ class BufferCaptureStrategyTest {
336365
assertEquals(SentryId.EMPTY_ID, fixture.scope.replayId)
337366
}
338367

368+
@Test
369+
fun `captureReplay does not capture segments when rate-limited`() {
370+
val rateLimiter = mock<RateLimiter> { on { isActiveForCategory(any()) }.thenReturn(true) }
371+
whenever(fixture.scopes.rateLimiter).thenReturn(rateLimiter)
372+
val strategy = fixture.getSut()
373+
strategy.start()
374+
strategy.onConfigurationChanged(fixture.recorderConfig)
375+
strategy.pause()
376+
377+
strategy.captureReplay(false) {}
378+
379+
// neither the current nor the buffered segment should be sent while rate-limited
380+
verify(fixture.scopes, never()).captureReplay(any(), any())
381+
// the replayId is still set on the scope so the error that flushed the buffer stays linked to
382+
// the replay that gets recorded once the rate limit lifts
383+
assertEquals(strategy.currentReplayId, fixture.scope.replayId)
384+
}
385+
386+
@Test
387+
fun `captureReplay records a lost replay event when rate-limited`() {
388+
val rateLimiter = mock<RateLimiter> { on { isActiveForCategory(any()) }.thenReturn(true) }
389+
whenever(fixture.scopes.rateLimiter).thenReturn(rateLimiter)
390+
val strategy = fixture.getSut()
391+
strategy.start()
392+
strategy.onConfigurationChanged(fixture.recorderConfig)
393+
strategy.pause()
394+
395+
strategy.captureReplay(false) {}
396+
397+
val discarded = fixture.discardedEvents()
398+
assertEquals(1, discarded.size)
399+
assertEquals(DiscardReason.RATELIMIT_BACKOFF.reason, discarded.first().reason)
400+
assertEquals(DataCategory.Replay.category, discarded.first().category)
401+
}
402+
403+
@Test
404+
fun `captureReplay does not record a lost replay event when not rate-limited`() {
405+
val strategy = fixture.getSut()
406+
strategy.start()
407+
strategy.onConfigurationChanged(fixture.recorderConfig)
408+
409+
strategy.captureReplay(false) {}
410+
411+
assertTrue(fixture.discardedEvents().none { it.category == DataCategory.Replay.category })
412+
}
413+
339414
@Test
340415
fun `captureReplay sets replayId to scope and captures buffered segments`() {
341416
var called = false

0 commit comments

Comments
 (0)