Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SR] Cleanups and session deadline #3316

Merged
merged 5 commits into from
Apr 9, 2024
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import io.sentry.android.core.cache.AndroidEnvelopeCache
import io.sentry.android.core.performance.AppStartMetrics
import io.sentry.android.fragment.FragmentLifecycleIntegration
import io.sentry.android.replay.ReplayIntegration
import io.sentry.android.replay.getReplayIntegration
import io.sentry.android.timber.SentryTimberIntegration
import io.sentry.cache.IEnvelopeCache
import io.sentry.cache.PersistingOptionsObserver
Expand Down Expand Up @@ -319,15 +318,15 @@ class SentryAndroidTest {
@Config(sdk = [26])
fun `init starts session replay if app is in foreground`() {
initSentryWithForegroundImportance(true) { _ ->
assertTrue(Sentry.getCurrentHub().getReplayIntegration()!!.isRecording())
assertTrue(Sentry.getCurrentHub().options.replayController.isRecording())
}
}

@Test
@Config(sdk = [26])
fun `init does not start session replay if the app is in background`() {
initSentryWithForegroundImportance(false) { _ ->
assertFalse(Sentry.getCurrentHub().getReplayIntegration()!!.isRecording())
assertFalse(Sentry.getCurrentHub().options.replayController.isRecording())
}
}

Expand Down
7 changes: 1 addition & 6 deletions sentry-android-replay/api/sentry-android-replay.api
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public final class io/sentry/android/replay/ReplayCache : java/io/Closeable {
public final class io/sentry/android/replay/ReplayIntegration : io/sentry/Integration, io/sentry/ReplayController, io/sentry/android/replay/ScreenshotRecorderCallback, java/io/Closeable {
public fun <init> (Landroid/content/Context;Lio/sentry/transport/ICurrentDateProvider;)V
public fun close ()V
public final fun isRecording ()Z
public fun isRecording ()Z
public fun onScreenshotRecorded (Landroid/graphics/Bitmap;)V
public fun pause ()V
public fun register (Lio/sentry/IHub;Lio/sentry/SentryOptions;)V
Expand All @@ -43,11 +43,6 @@ public final class io/sentry/android/replay/ReplayIntegration : io/sentry/Integr
public fun stop ()V
}

public final class io/sentry/android/replay/ReplayIntegrationKt {
public static final fun getReplayIntegration (Lio/sentry/IHub;)Lio/sentry/android/replay/ReplayIntegration;
public static final fun gracefullyShutdown (Ljava/util/concurrent/ExecutorService;Lio/sentry/SentryOptions;)V
}

public abstract interface class io/sentry/android/replay/ScreenshotRecorderCallback {
public abstract fun onScreenshotRecorded (Landroid/graphics/Bitmap;)V
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import io.sentry.SentryReplayEvent
import io.sentry.SentryReplayEvent.ReplayType
import io.sentry.SentryReplayEvent.ReplayType.BUFFER
import io.sentry.SentryReplayEvent.ReplayType.SESSION
import io.sentry.android.replay.util.gracefullyShutdown
import io.sentry.android.replay.util.submitSafely
import io.sentry.protocol.SentryId
import io.sentry.rrweb.RRWebMetaEvent
import io.sentry.rrweb.RRWebVideoEvent
Expand All @@ -28,12 +30,11 @@ import java.io.Closeable
import java.io.File
import java.security.SecureRandom
import java.util.Date
import java.util.concurrent.ExecutorService
import java.util.concurrent.Executors
import java.util.concurrent.ThreadFactory
import java.util.concurrent.TimeUnit.MILLISECONDS
import java.util.concurrent.atomic.AtomicBoolean
import java.util.concurrent.atomic.AtomicInteger
import java.util.concurrent.atomic.AtomicLong
import java.util.concurrent.atomic.AtomicReference
import kotlin.LazyThreadSafetyMode.NONE

Expand All @@ -42,6 +43,10 @@ class ReplayIntegration(
private val dateProvider: ICurrentDateProvider
) : Integration, Closeable, ScreenshotRecorderCallback, ReplayController {

internal companion object {
private const val TAG = "ReplayIntegration"
}

private lateinit var options: SentryOptions
private var hub: IHub? = null
private var recorder: WindowRecorder? = null
Expand All @@ -54,10 +59,11 @@ class ReplayIntegration(
private val isRecording = AtomicBoolean(false)
private val currentReplayId = AtomicReference(SentryId.EMPTY_ID)
private val segmentTimestamp = AtomicReference<Date>()
private val replayStartTimestamp = AtomicLong()
private val currentSegment = AtomicInteger(0)

// TODO: surround with try-catch on the calling site
private val saver by lazy {
private val replayExecutor by lazy {
Executors.newSingleThreadScheduledExecutor(ReplayExecutorServiceThreadFactory())
}

Expand Down Expand Up @@ -108,7 +114,7 @@ class ReplayIntegration(
.addPackage("maven:io.sentry:sentry-android-replay", BuildConfig.VERSION_NAME)
}

fun isRecording() = isRecording.get()
override fun isRecording() = isRecording.get()

override fun start() {
// TODO: add lifecycle state instead and manage it in start/pause/resume/stop
Expand All @@ -126,6 +132,18 @@ class ReplayIntegration(

currentSegment.set(0)
currentReplayId.set(SentryId())
replayExecutor.submitSafely(options, "$TAG.replays_cleanup") {
// clean up old replays
options.cacheDirPath?.let { cacheDir ->
File(cacheDir).listFiles { dir, name ->
// TODO: also exclude persisted replay_id from scope when implementing ANRs
if (name.startsWith("replay_") && !name.contains(currentReplayId.get().toString())) {
FileUtils.deleteRecursively(File(dir, name))
}
false
}
}
}
if (isFullSession.get()) {
// only set replayId on the scope if it's a full session, otherwise all events will be
// tagged with the replay that might never be sent when we're recording in buffer mode
Expand All @@ -136,6 +154,7 @@ class ReplayIntegration(
recorder?.startRecording()
// TODO: replace it with dateProvider.currentTimeMillis to also test it
segmentTimestamp.set(DateUtils.getCurrentDateTime())
replayStartTimestamp.set(dateProvider.currentTimeMillis)
// TODO: finalize old recording if there's some left on disk and send it using the replayId from persisted scope (e.g. for ANRs)
}

Expand Down Expand Up @@ -179,7 +198,7 @@ class ReplayIntegration(
}
val segmentId = currentSegment.get()
val replayId = currentReplayId.get()
saver.submit {
replayExecutor.submitSafely(options, "$TAG.send_replay_for_event") {
val videoDuration =
createAndCaptureSegment(now - currentSegmentTimestamp.time, currentSegmentTimestamp, replayId, segmentId, BUFFER, hint)
if (videoDuration != null) {
Expand Down Expand Up @@ -212,7 +231,7 @@ class ReplayIntegration(
val segmentId = currentSegment.get()
val duration = now - currentSegmentTimestamp.time
val replayId = currentReplayId.get()
saver.submit {
replayExecutor.submitSafely(options, "$TAG.pause") {
val videoDuration =
createAndCaptureSegment(duration, currentSegmentTimestamp, replayId, segmentId)
if (videoDuration != null) {
Expand All @@ -232,7 +251,7 @@ class ReplayIntegration(
val duration = now - currentSegmentTimestamp.time
val replayId = currentReplayId.get()
val replayCacheDir = cache?.replayCacheDir
saver.submit {
replayExecutor.submitSafely(options, "$TAG.stop") {
// we don't flush the segment, but we still wanna clean up the folder for buffer mode
if (isFullSession.get()) {
createAndCaptureSegment(duration, currentSegmentTimestamp, replayId, segmentId)
Expand All @@ -243,6 +262,7 @@ class ReplayIntegration(
recorder?.stopRecording()
cache?.close()
currentSegment.set(0)
replayStartTimestamp.set(0)
segmentTimestamp.set(null)
currentReplayId.set(SentryId.EMPTY_ID)
hub?.configureScope { it.replayId = SentryId.EMPTY_ID }
Expand All @@ -253,7 +273,7 @@ class ReplayIntegration(
// 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
saver.submit {
replayExecutor.submitSafely(options, "$TAG.add_frame") {
cache?.addFrame(bitmap, frameTimestamp)

val now = dateProvider.currentTimeMillis
Expand All @@ -276,6 +296,11 @@ class ReplayIntegration(
// set next segment timestamp as close to the previous one as possible to avoid gaps
segmentTimestamp.set(DateUtils.getDateTime(currentSegmentTimestamp.time + videoDuration))
}
} else if (isFullSession.get() &&
(now - replayStartTimestamp.get() >= options.experimental.sessionReplayOptions.sessionDuration)
) {
stop()
options.logger.log(INFO, "Session replay deadline exceeded (1h), stopping recording")
} else if (!isFullSession.get()) {
cache?.rotate(now - options.experimental.sessionReplayOptions.errorReplayDuration)
}
Expand Down Expand Up @@ -365,7 +390,7 @@ class ReplayIntegration(
}

stop()
saver.gracefullyShutdown(options)
replayExecutor.gracefullyShutdown(options)
}

private class ReplayExecutorServiceThreadFactory : ThreadFactory {
Expand All @@ -377,25 +402,3 @@ class ReplayIntegration(
}
}
}

/**
* Retrieves the [ReplayIntegration] from the list of integrations in [SentryOptions]
*/
fun IHub.getReplayIntegration(): ReplayIntegration? =
options.integrations.find { it is ReplayIntegration } as? ReplayIntegration

fun ExecutorService.gracefullyShutdown(options: SentryOptions) {
synchronized(this) {
if (!isShutdown) {
shutdown()
}
try {
if (!awaitTermination(options.shutdownTimeoutMillis, MILLISECONDS)) {
shutdownNow()
}
} catch (e: InterruptedException) {
shutdownNow()
Thread.currentThread().interrupt()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ package io.sentry.android.replay

import android.annotation.TargetApi
import android.view.View
import io.sentry.SentryLevel.ERROR
import io.sentry.SentryOptions
import io.sentry.android.replay.util.gracefullyShutdown
import io.sentry.android.replay.util.scheduleAtFixedRateSafely
import java.io.Closeable
import java.lang.ref.WeakReference
import java.util.concurrent.Executors
Expand All @@ -20,6 +21,10 @@ internal class WindowRecorder(
private val screenshotRecorderCallback: ScreenshotRecorderCallback
) : Closeable {

internal companion object {
private const val TAG = "WindowRecorder"
}

private val rootViewsSpy by lazy(NONE) {
RootViewsSpy.install()
}
Expand Down Expand Up @@ -57,14 +62,15 @@ internal class WindowRecorder(

recorder = ScreenshotRecorder(recorderConfig, options, screenshotRecorderCallback)
rootViewsSpy.listeners += onRootViewsChangedListener
capturingTask = capturer.scheduleAtFixedRate({
try {
recorder?.capture()
} catch (e: Throwable) {
options.logger.log(ERROR, "Failed to capture a screenshot with exception:", e)
// TODO: I guess schedule the capturer again, cause it will stop executing the runnable?
}
}, 0L, 1000L / recorderConfig.frameRate, MILLISECONDS)
capturingTask = capturer.scheduleAtFixedRateSafely(
options,
"$TAG.capture",
0L,
1000L / recorderConfig.frameRate,
MILLISECONDS
) {
recorder?.capture()
}
}

fun resume() = recorder?.resume()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package io.sentry.android.replay.util

import io.sentry.SentryLevel.ERROR
import io.sentry.SentryOptions
import java.util.concurrent.ExecutorService
import java.util.concurrent.Future
import java.util.concurrent.ScheduledExecutorService
import java.util.concurrent.ScheduledFuture
import java.util.concurrent.TimeUnit
import java.util.concurrent.TimeUnit.MILLISECONDS

internal fun ExecutorService.gracefullyShutdown(options: SentryOptions) {
synchronized(this) {
if (!isShutdown) {
shutdown()
}
try {
if (!awaitTermination(options.shutdownTimeoutMillis, MILLISECONDS)) {
shutdownNow()
}
} catch (e: InterruptedException) {
shutdownNow()
Thread.currentThread().interrupt()
}
}
}

internal fun ExecutorService.submitSafely(
romtsn marked this conversation as resolved.
Show resolved Hide resolved
options: SentryOptions,
taskName: String,
task: Runnable
): Future<*>? {
return try {
submit {
try {
task.run()
} catch (e: Throwable) {
options.logger.log(ERROR, "Failed to execute task $taskName", e)
}
}
} catch (e: Throwable) {
options.logger.log(ERROR, "Failed to submit task $taskName to executor", e)
null
}
}

internal fun ScheduledExecutorService.scheduleAtFixedRateSafely(
options: SentryOptions,
taskName: String,
initialDelay: Long,
period: Long,
unit: TimeUnit,
task: Runnable
): ScheduledFuture<*>? {
return try {
scheduleAtFixedRate({
try {
task.run()
} catch (e: Throwable) {
options.logger.log(ERROR, "Failed to execute task $taskName", e)
}
}, initialDelay, period, unit)
} catch (e: Throwable) {
options.logger.log(ERROR, "Failed to submit task $taskName to executor", e)
null
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -156,5 +156,7 @@

<meta-data android:name="io.sentry.performance-v2.enable" android:value="true" />

<meta-data android:name="io.sentry.session-replay.session-sample-rate" android:value="1.0" />

</application>
</manifest>
3 changes: 3 additions & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -1206,6 +1206,7 @@ public final class io/sentry/NoOpLogger : io/sentry/ILogger {

public final class io/sentry/NoOpReplayController : io/sentry/ReplayController {
public static fun getInstance ()Lio/sentry/NoOpReplayController;
public fun isRecording ()Z
public fun pause ()V
public fun resume ()V
public fun sendReplayForEvent (Lio/sentry/SentryEvent;Lio/sentry/Hint;)V
Expand Down Expand Up @@ -1604,6 +1605,7 @@ public final class io/sentry/PropagationContext {
}

public abstract interface class io/sentry/ReplayController {
public abstract fun isRecording ()Z
public abstract fun pause ()V
public abstract fun resume ()V
public abstract fun sendReplayForEvent (Lio/sentry/SentryEvent;Lio/sentry/Hint;)V
Expand Down Expand Up @@ -2569,6 +2571,7 @@ public final class io/sentry/SentryReplayOptions {
public fun getErrorReplayDuration ()J
public fun getErrorSampleRate ()Ljava/lang/Double;
public fun getFrameRate ()I
public fun getSessionDuration ()J
public fun getSessionSampleRate ()Ljava/lang/Double;
public fun getSessionSegmentDuration ()J
public fun isSessionReplayEnabled ()Z
Expand Down
5 changes: 5 additions & 0 deletions sentry/src/main/java/io/sentry/NoOpReplayController.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ public void pause() {}
@Override
public void resume() {}

@Override
public boolean isRecording() {
return false;
}

@Override
public void sendReplayForEvent(@NotNull SentryEvent event, @NotNull Hint hint) {}
}
2 changes: 2 additions & 0 deletions sentry/src/main/java/io/sentry/ReplayController.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,7 @@ public interface ReplayController {

void resume();

boolean isRecording();

void sendReplayForEvent(@NotNull SentryEvent event, @NotNull Hint hint);
}
Loading
Loading