Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### 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))
- 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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -76,6 +78,13 @@ internal class BufferCaptureStrategy(
}

override fun captureReplay(isTerminating: Boolean, onSegmentSent: (Date) -> Unit) {
if (isReplayRateLimited()) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

h: I think we're calling this a bit early -- should likely do it after the if (isTerminating) check, otherwise we'd be dropping replays for crashed events (which are anyway not being sent in this process, but only on next launch, where the rate limit likely will have already been lifted).

Another side effect of not setting isTerminating would be that the following convert() call would convert to session mode and try to keep recording while the process is terminating. Probs, needs a test for this to not regress in the future too

// 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")
return

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m: another thing -- previously we'd record a client report with the reason ratelimit_backoff on the envelope being dropped, but now we're not doing it. I think after addressing the point above (moving this to after if (isTerminating), we could also record the ratelimit_backoff client report for a genuinely lost event (which would be also called after sampling) inside the if (isReplayRateLimited()) block.

}

val sampled = random.sample(options.sessionReplay.onErrorSampleRate)

if (!sampled) {
Expand Down Expand Up @@ -170,6 +179,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import io.sentry.android.replay.capture.BufferCaptureStrategyTest.Fixture.Compan
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
Expand Down Expand Up @@ -336,6 +337,22 @@ class BufferCaptureStrategyTest {
assertEquals(SentryId.EMPTY_ID, fixture.scope.replayId)
}

@Test
fun `captureReplay does nothing when rate-limited`() {
val rateLimiter = mock<RateLimiter> { 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())
assertEquals(SentryId.EMPTY_ID, fixture.scope.replayId)
}

@Test
fun `captureReplay sets replayId to scope and captures buffered segments`() {
var called = false
Expand Down
Loading