Skip to content

Commit

Permalink
[SR] Flutter improvements (#4007)
Browse files Browse the repository at this point in the history
* Get rid of the  lock on touch events

* pre-allocate some things for gesture converter

* have one less thread switch for re]play

* update

* Changelog

* Add option to disable orientation change tracking for session replay

* Make RecorderConfig lazier

* Fix tests

* Changelog
  • Loading branch information
romtsn authored Dec 20, 2024
1 parent 91bb628 commit d410b56
Show file tree
Hide file tree
Showing 14 changed files with 98 additions and 82 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
- Session Replay: Fix memory leak when masking Compose screens ([#3985](https://github.com/getsentry/sentry-java/pull/3985))
- Session Replay: Fix potential ANRs in `GestureRecorder` ([#4001](https://github.com/getsentry/sentry-java/pull/4001))

### Internal

- Session Replay: Flutter improvements ([#4007](https://github.com/getsentry/sentry-java/pull/4007))

## 7.19.0

### Fixes
Expand Down
10 changes: 5 additions & 5 deletions sentry-android-replay/api/sentry-android-replay.api
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,12 @@ public abstract interface class io/sentry/android/replay/Recorder : java/io/Clos
public final class io/sentry/android/replay/ReplayCache : java/io/Closeable {
public static final field $stable I
public static final field Companion Lio/sentry/android/replay/ReplayCache$Companion;
public fun <init> (Lio/sentry/SentryOptions;Lio/sentry/protocol/SentryId;Lio/sentry/android/replay/ScreenshotRecorderConfig;)V
public fun <init> (Lio/sentry/SentryOptions;Lio/sentry/protocol/SentryId;)V
public final fun addFrame (Ljava/io/File;JLjava/lang/String;)V
public static synthetic fun addFrame$default (Lio/sentry/android/replay/ReplayCache;Ljava/io/File;JLjava/lang/String;ILjava/lang/Object;)V
public fun close ()V
public final fun createVideoOf (JJIIILjava/io/File;)Lio/sentry/android/replay/GeneratedVideo;
public static synthetic fun createVideoOf$default (Lio/sentry/android/replay/ReplayCache;JJIIILjava/io/File;ILjava/lang/Object;)Lio/sentry/android/replay/GeneratedVideo;
public final fun createVideoOf (JJIIIIILjava/io/File;)Lio/sentry/android/replay/GeneratedVideo;
public static synthetic fun createVideoOf$default (Lio/sentry/android/replay/ReplayCache;JJIIIIILjava/io/File;ILjava/lang/Object;)Lio/sentry/android/replay/GeneratedVideo;
public final fun persistSegmentValues (Ljava/lang/String;Ljava/lang/String;)V
public final fun rotate (J)Ljava/lang/String;
}
Expand All @@ -60,8 +60,8 @@ public final class io/sentry/android/replay/ReplayCache$Companion {
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 <init> (Landroid/content/Context;Lio/sentry/transport/ICurrentDateProvider;)V
public fun <init> (Landroid/content/Context;Lio/sentry/transport/ICurrentDateProvider;Lkotlin/jvm/functions/Function0;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function2;)V
public synthetic fun <init> (Landroid/content/Context;Lio/sentry/transport/ICurrentDateProvider;Lkotlin/jvm/functions/Function0;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function2;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun <init> (Landroid/content/Context;Lio/sentry/transport/ICurrentDateProvider;Lkotlin/jvm/functions/Function0;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;)V
public synthetic fun <init> (Landroid/content/Context;Lio/sentry/transport/ICurrentDateProvider;Lkotlin/jvm/functions/Function0;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun captureReplay (Ljava/lang/Boolean;)V
public fun close ()V
public fun getBreadcrumbConverter ()Lio/sentry/ReplayBreadcrumbConverter;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ import java.util.concurrent.atomic.AtomicBoolean
*/
public class ReplayCache(
private val options: SentryOptions,
private val replayId: SentryId,
private val recorderConfig: ScreenshotRecorderConfig
private val replayId: SentryId
) : Closeable {

private val isClosed = AtomicBoolean(false)
Expand Down Expand Up @@ -133,6 +132,8 @@ public class ReplayCache(
segmentId: Int,
height: Int,
width: Int,
frameRate: Int,
bitRate: Int,
videoFile: File = File(replayCacheDir, "$segmentId.mp4")
): GeneratedVideo? {
if (videoFile.exists() && videoFile.length() > 0) {
Expand All @@ -146,21 +147,20 @@ public class ReplayCache(
return null
}

// TODO: reuse instance of encoder and just change file path to create a different muxer
encoder = synchronized(encoderLock) {
SimpleVideoEncoder(
options,
MuxerConfig(
file = videoFile,
recordingHeight = height,
recordingWidth = width,
frameRate = recorderConfig.frameRate,
bitRate = recorderConfig.bitRate
frameRate = frameRate,
bitRate = bitRate
)
).also { it.start() }
}

val step = 1000 / recorderConfig.frameRate.toLong()
val step = 1000 / frameRate.toLong()
var frameCount = 0
var lastFrame: ReplayFrame = frames.first()
for (timestamp in from until (from + (duration)) step step) {
Expand Down Expand Up @@ -306,7 +306,7 @@ public class ReplayCache(
}
}

internal fun fromDisk(options: SentryOptions, replayId: SentryId, replayCacheProvider: ((replayId: SentryId, recorderConfig: ScreenshotRecorderConfig) -> ReplayCache)? = null): LastSegmentData? {
internal fun fromDisk(options: SentryOptions, replayId: SentryId, replayCacheProvider: ((replayId: SentryId) -> ReplayCache)? = null): LastSegmentData? {
val replayCacheDir = makeReplayCacheDir(options, replayId)
val lastSegmentFile = File(replayCacheDir, ONGOING_SEGMENT)
if (!lastSegmentFile.exists()) {
Expand Down Expand Up @@ -360,7 +360,7 @@ public class ReplayCache(
scaleFactorY = 1.0f
)

val cache = replayCacheProvider?.invoke(replayId, recorderConfig) ?: ReplayCache(options, replayId, recorderConfig)
val cache = replayCacheProvider?.invoke(replayId) ?: ReplayCache(options, replayId)
cache.replayCacheDir?.listFiles { dir, name ->
if (name.endsWith(".jpg")) {
val file = File(dir, name)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public class ReplayIntegration(
private val dateProvider: ICurrentDateProvider,
private val recorderProvider: (() -> Recorder)? = null,
private val recorderConfigProvider: ((configChanged: Boolean) -> ScreenshotRecorderConfig)? = null,
private val replayCacheProvider: ((replayId: SentryId, recorderConfig: ScreenshotRecorderConfig) -> ReplayCache)? = null
private val replayCacheProvider: ((replayId: SentryId) -> ReplayCache)? = null
) : Integration,
Closeable,
ScreenshotRecorderCallback,
Expand All @@ -80,7 +80,7 @@ public class ReplayIntegration(
dateProvider: ICurrentDateProvider,
recorderProvider: (() -> Recorder)?,
recorderConfigProvider: ((configChanged: Boolean) -> ScreenshotRecorderConfig)?,
replayCacheProvider: ((replayId: SentryId, recorderConfig: ScreenshotRecorderConfig) -> ReplayCache)?,
replayCacheProvider: ((replayId: SentryId) -> ReplayCache)?,
replayCaptureStrategyProvider: ((isFullSession: Boolean) -> CaptureStrategy)? = null,
mainLooperHandler: MainLooperHandler? = null,
gestureRecorderProvider: (() -> GestureRecorder)? = null
Expand Down Expand Up @@ -110,8 +110,6 @@ public class ReplayIntegration(
private var mainLooperHandler: MainLooperHandler = MainLooperHandler()
private var gestureRecorderProvider: (() -> GestureRecorder)? = null

private lateinit var recorderConfig: ScreenshotRecorderConfig

override fun register(hub: IHub, options: SentryOptions) {
this.options = options

Expand All @@ -134,10 +132,16 @@ public class ReplayIntegration(

options.connectionStatusProvider.addConnectionStatusObserver(this)
hub.rateLimiter?.addRateLimitObserver(this)
try {
context.registerComponentCallbacks(this)
} catch (e: Throwable) {
options.logger.log(INFO, "ComponentCallbacks is not available, orientation changes won't be handled by Session replay", e)
if (options.experimental.sessionReplay.isTrackOrientationChange) {
try {
context.registerComponentCallbacks(this)
} catch (e: Throwable) {
options.logger.log(
INFO,
"ComponentCallbacks is not available, orientation changes won't be handled by Session replay",
e
)
}
}

addIntegrationToSdkVersion("Replay")
Expand Down Expand Up @@ -169,7 +173,7 @@ public class ReplayIntegration(
return
}

recorderConfig = recorderConfigProvider?.invoke(false) ?: ScreenshotRecorderConfig.from(context, options.experimental.sessionReplay)
val recorderConfig = recorderConfigProvider?.invoke(false) ?: ScreenshotRecorderConfig.from(context, options.experimental.sessionReplay)
captureStrategy = replayCaptureStrategyProvider?.invoke(isFullSession) ?: if (isFullSession) {
SessionCaptureStrategy(options, hub, dateProvider, replayExecutor, replayCacheProvider)
} else {
Expand Down Expand Up @@ -260,9 +264,11 @@ public class ReplayIntegration(

options.connectionStatusProvider.removeConnectionStatusObserver(this)
hub?.rateLimiter?.removeRateLimitObserver(this)
try {
context.unregisterComponentCallbacks(this)
} catch (ignored: Throwable) {
if (options.experimental.sessionReplay.isTrackOrientationChange) {
try {
context.unregisterComponentCallbacks(this)
} catch (ignored: Throwable) {
}
}
stop()
recorder?.close()
Expand All @@ -279,7 +285,7 @@ public class ReplayIntegration(
recorder?.stop()

// refresh config based on new device configuration
recorderConfig = recorderConfigProvider?.invoke(true) ?: ScreenshotRecorderConfig.from(context, options.experimental.sessionReplay)
val recorderConfig = recorderConfigProvider?.invoke(true) ?: ScreenshotRecorderConfig.from(context, options.experimental.sessionReplay)
captureStrategy?.onConfigurationChanged(recorderConfig)

recorder?.start(recorderConfig)
Expand Down Expand Up @@ -392,6 +398,7 @@ public class ReplayIntegration(
height = lastSegment.recorderConfig.recordingHeight,
width = lastSegment.recorderConfig.recordingWidth,
frameRate = lastSegment.recorderConfig.frameRate,
bitRate = lastSegment.recorderConfig.bitRate,
cache = lastSegment.cache,
replayType = lastSegment.replayType,
screenAtStart = lastSegment.screenAtStart,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ internal abstract class BaseCaptureStrategy(
private val hub: IHub?,
private val dateProvider: ICurrentDateProvider,
protected val replayExecutor: ScheduledExecutorService,
private val replayCacheProvider: ((replayId: SentryId, recorderConfig: ScreenshotRecorderConfig) -> ReplayCache)? = null
private val replayCacheProvider: ((replayId: SentryId) -> ReplayCache)? = null
) : CaptureStrategy {

internal companion object {
Expand Down Expand Up @@ -89,7 +89,7 @@ internal abstract class BaseCaptureStrategy(
replayId: SentryId,
replayType: ReplayType?
) {
cache = replayCacheProvider?.invoke(replayId, recorderConfig) ?: ReplayCache(options, replayId, recorderConfig)
cache = replayCacheProvider?.invoke(replayId) ?: ReplayCache(options, replayId)

this.currentReplayId = replayId
this.currentSegment = segmentId
Expand Down Expand Up @@ -124,6 +124,7 @@ internal abstract class BaseCaptureStrategy(
replayType: ReplayType = this.replayType,
cache: ReplayCache? = this.cache,
frameRate: Int = recorderConfig.frameRate,
bitRate: Int = recorderConfig.bitRate,
screenAtStart: String? = this.screenAtStart,
breadcrumbs: List<Breadcrumb>? = null,
events: Deque<RRWebEvent> = this.currentEvents
Expand All @@ -140,6 +141,7 @@ internal abstract class BaseCaptureStrategy(
replayType,
cache,
frameRate,
bitRate,
screenAtStart,
breadcrumbs,
events
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ internal class BufferCaptureStrategy(
private val dateProvider: ICurrentDateProvider,
private val random: Random,
executor: ScheduledExecutorService,
replayCacheProvider: ((replayId: SentryId, recorderConfig: ScreenshotRecorderConfig) -> ReplayCache)? = null
replayCacheProvider: ((replayId: SentryId) -> ReplayCache)? = null
) : BaseCaptureStrategy(options, hub, dateProvider, executor, replayCacheProvider = replayCacheProvider) {

// TODO: capture envelopes for buffered segments instead, but don't send them until buffer is triggered
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ internal interface CaptureStrategy {
replayType: ReplayType,
cache: ReplayCache?,
frameRate: Int,
bitRate: Int,
screenAtStart: String?,
breadcrumbs: List<Breadcrumb>?,
events: Deque<RRWebEvent>
Expand All @@ -78,7 +79,9 @@ internal interface CaptureStrategy {
currentSegmentTimestamp.time,
segmentId,
height,
width
width,
frameRate,
bitRate
) ?: return ReplaySegment.Failed

val (video, frameCount, videoDuration) = generatedVideo
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ internal class SessionCaptureStrategy(
private val hub: IHub?,
private val dateProvider: ICurrentDateProvider,
executor: ScheduledExecutorService,
replayCacheProvider: ((replayId: SentryId, recorderConfig: ScreenshotRecorderConfig) -> ReplayCache)? = null
replayCacheProvider: ((replayId: SentryId) -> ReplayCache)? = null
) : BaseCaptureStrategy(options, hub, dateProvider, executor, replayCacheProvider) {

internal companion object {
Expand Down
Loading

0 comments on commit d410b56

Please sign in to comment.