From ef3d62c61344ce7a48e12f8d814ee5d6d4454d84 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 28 Mar 2024 11:34:51 +0100 Subject: [PATCH 1/7] Add buffer mode and link replays with events/transactions --- .../api/sentry-android-core.api | 4 - .../core/AndroidOptionsInitializer.java | 5 +- .../sentry/android/core/LifecycleWatcher.java | 8 +- .../io/sentry/android/core/SentryAndroid.java | 59 +-------- .../api/sentry-android-replay.api | 17 +-- .../io/sentry/android/replay/ReplayCache.kt | 18 ++- .../android/replay/ReplayIntegration.kt | 125 +++++++++++++++--- sentry/api/sentry.api | 28 +++- sentry/src/main/java/io/sentry/Baggage.java | 27 +++- .../java/io/sentry/NoOpReplayController.java | 29 ++++ .../main/java/io/sentry/ReplayController.java | 17 +++ .../src/main/java/io/sentry/SentryClient.java | 47 +++++-- .../main/java/io/sentry/SentryOptions.java | 11 ++ .../java/io/sentry/SentryReplayOptions.java | 8 ++ .../src/main/java/io/sentry/SentryTracer.java | 8 +- .../src/main/java/io/sentry/TraceContext.java | 22 ++- .../sentry/TraceContextSerializationTest.kt | 1 + 17 files changed, 310 insertions(+), 124 deletions(-) create mode 100644 sentry/src/main/java/io/sentry/NoOpReplayController.java create mode 100644 sentry/src/main/java/io/sentry/ReplayController.java diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 55278c6356..51eb48f1b2 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -245,10 +245,6 @@ public final class io/sentry/android/core/SentryAndroid { public static fun init (Landroid/content/Context;Lio/sentry/ILogger;)V public static fun init (Landroid/content/Context;Lio/sentry/ILogger;Lio/sentry/Sentry$OptionsConfiguration;)V public static fun init (Landroid/content/Context;Lio/sentry/Sentry$OptionsConfiguration;)V - public static fun pauseReplay ()V - public static fun resumeReplay ()V - public static fun startReplay ()V - public static fun stopReplay ()V } public final class io/sentry/android/core/SentryAndroidDateProvider : io/sentry/SentryDateProvider { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index b58051cee7..27c9d6e674 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -299,7 +299,10 @@ static void installDefaultIntegrations( options.addIntegration(new TempSensorBreadcrumbsIntegration(context)); options.addIntegration(new PhoneStateBreadcrumbsIntegration(context)); if (isReplayAvailable) { - options.addIntegration(new ReplayIntegration(context, CurrentDateProvider.getInstance())); + final ReplayIntegration replay = + new ReplayIntegration(context, CurrentDateProvider.getInstance()); + options.addIntegration(replay); + options.setReplayController(replay); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java b/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java index ff281d2beb..bcdb49e3e6 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java @@ -93,10 +93,10 @@ private void startSession() { addSessionBreadcrumb("start"); hub.startSession(); } - SentryAndroid.startReplay(); + hub.getOptions().getReplayController().start(); } else if (!isFreshSession.getAndSet(false)) { // only resume if it's not a fresh session, which has been started in SentryAndroid.init - SentryAndroid.resumeReplay(); + hub.getOptions().getReplayController().resume(); } this.lastUpdatedSession.set(currentTimeMillis); } @@ -108,7 +108,7 @@ public void onStop(final @NotNull LifecycleOwner owner) { final long currentTimeMillis = currentDateProvider.getCurrentTimeMillis(); this.lastUpdatedSession.set(currentTimeMillis); - SentryAndroid.pauseReplay(); + hub.getOptions().getReplayController().pause(); scheduleEndSession(); AppState.getInstance().setInBackground(true); @@ -127,7 +127,7 @@ public void run() { addSessionBreadcrumb("end"); hub.endSession(); } - SentryAndroid.stopReplay(); + hub.getOptions().getReplayController().stop(); } }; diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java index d6e11d15f5..676bb2173a 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java @@ -15,8 +15,6 @@ import io.sentry.android.core.performance.AppStartMetrics; import io.sentry.android.core.performance.TimeSpan; import io.sentry.android.fragment.FragmentLifecycleIntegration; -import io.sentry.android.replay.ReplayIntegration; -import io.sentry.android.replay.ReplayIntegrationKt; import io.sentry.android.timber.SentryTimberIntegration; import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; @@ -160,7 +158,7 @@ public static synchronized void init( hub.addBreadcrumb(BreadcrumbFactory.forSession("session.start")); hub.startSession(); } - startReplay(); + hub.getOptions().getReplayController().start(); } } catch (IllegalAccessException e) { logger.log(SentryLevel.FATAL, "Fatal error during SentryAndroid.init(...)", e); @@ -225,59 +223,4 @@ private static void deduplicateIntegrations( } } } - - public static synchronized void startReplay() { - if (!ensureReplayIntegration("starting")) { - return; - } - final @NotNull IHub hub = Sentry.getCurrentHub(); - ReplayIntegrationKt.getReplayIntegration(hub).start(); - } - - public static synchronized void stopReplay() { - if (!ensureReplayIntegration("stopping")) { - return; - } - final @NotNull IHub hub = Sentry.getCurrentHub(); - ReplayIntegrationKt.getReplayIntegration(hub).stop(); - } - - public static synchronized void resumeReplay() { - if (!ensureReplayIntegration("resuming")) { - return; - } - final @NotNull IHub hub = Sentry.getCurrentHub(); - ReplayIntegrationKt.getReplayIntegration(hub).resume(); - } - - public static synchronized void pauseReplay() { - if (!ensureReplayIntegration("pausing")) { - return; - } - final @NotNull IHub hub = Sentry.getCurrentHub(); - ReplayIntegrationKt.getReplayIntegration(hub).pause(); - } - - private static boolean ensureReplayIntegration(final @NotNull String actionName) { - final @NotNull IHub hub = Sentry.getCurrentHub(); - if (isReplayAvailable) { - final ReplayIntegration replay = ReplayIntegrationKt.getReplayIntegration(hub); - if (replay != null) { - return true; - } else { - hub.getOptions() - .getLogger() - .log( - SentryLevel.INFO, - "Session Replay wasn't registered yet, not " + actionName + " the replay"); - } - } else { - hub.getOptions() - .getLogger() - .log( - SentryLevel.INFO, - "Session Replay wasn't found on classpath, not " + actionName + " the replay"); - } - return false; - } } diff --git a/sentry-android-replay/api/sentry-android-replay.api b/sentry-android-replay/api/sentry-android-replay.api index b3bc98e15b..82cfece004 100644 --- a/sentry-android-replay/api/sentry-android-replay.api +++ b/sentry-android-replay/api/sentry-android-replay.api @@ -6,22 +6,17 @@ public final class io/sentry/android/replay/BuildConfig { public fun ()V } -public final class io/sentry/android/replay/ReplayIntegration : io/sentry/Integration, io/sentry/android/replay/ScreenshotRecorderCallback, java/io/Closeable { - public static final field Companion Lio/sentry/android/replay/ReplayIntegration$Companion; - public static final field VIDEO_BUFFER_DURATION J - public static final field VIDEO_SEGMENT_DURATION J +public final class io/sentry/android/replay/ReplayIntegration : io/sentry/Integration, io/sentry/ReplayController, io/sentry/android/replay/ScreenshotRecorderCallback, java/io/Closeable { public fun (Landroid/content/Context;Lio/sentry/transport/ICurrentDateProvider;)V public fun close ()V public final fun isRecording ()Z public fun onScreenshotRecorded (Landroid/graphics/Bitmap;)V - public final fun pause ()V + public fun pause ()V public fun register (Lio/sentry/IHub;Lio/sentry/SentryOptions;)V - public final fun resume ()V - public final fun start ()V - public final fun stop ()V -} - -public final class io/sentry/android/replay/ReplayIntegration$Companion { + public fun resume ()V + public fun sendReplayForEvent (Lio/sentry/SentryEvent;)V + public fun start ()V + public fun stop ()V } public final class io/sentry/android/replay/ReplayIntegrationKt { diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayCache.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayCache.kt index e591370444..e271d8dd9a 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayCache.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayCache.kt @@ -113,13 +113,7 @@ internal class ReplayCache( encoder = null } - frames.removeAll { - if (it.timestamp < (from + duration)) { - deleteFile(it.screenshot) - return@removeAll true - } - return@removeAll false - } + rotate(until = (from + duration)) return GeneratedVideo(videoFile, frameCount, videoDuration) } @@ -142,6 +136,16 @@ internal class ReplayCache( } } + fun rotate(until: Long) { + frames.removeAll { + if (it.timestamp < until) { + deleteFile(it.screenshot) + return@removeAll true + } + return@removeAll false + } + } + override fun close() { synchronized(encoderLock) { encoder?.release() diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt index a99c8a9c95..da14c72bd4 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt @@ -7,18 +7,26 @@ import io.sentry.DateUtils import io.sentry.Hint import io.sentry.IHub import io.sentry.Integration +import io.sentry.ReplayController import io.sentry.ReplayRecording +import io.sentry.SentryEvent +import io.sentry.SentryIntegrationPackageStorage import io.sentry.SentryLevel.DEBUG import io.sentry.SentryLevel.INFO import io.sentry.SentryOptions import io.sentry.SentryReplayEvent +import io.sentry.SentryReplayEvent.ReplayType +import io.sentry.SentryReplayEvent.ReplayType.BUFFER +import io.sentry.SentryReplayEvent.ReplayType.SESSION import io.sentry.protocol.SentryId import io.sentry.rrweb.RRWebMetaEvent import io.sentry.rrweb.RRWebVideoEvent import io.sentry.transport.ICurrentDateProvider import io.sentry.util.FileUtils +import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion 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 @@ -32,19 +40,16 @@ import kotlin.LazyThreadSafetyMode.NONE class ReplayIntegration( private val context: Context, private val dateProvider: ICurrentDateProvider -) : Integration, Closeable, ScreenshotRecorderCallback { - - companion object { - const val VIDEO_SEGMENT_DURATION = 5_000L - const val VIDEO_BUFFER_DURATION = 30_000L - } +) : Integration, Closeable, ScreenshotRecorderCallback, ReplayController { private lateinit var options: SentryOptions private var hub: IHub? = null private var recorder: WindowRecorder? = null private var cache: ReplayCache? = null + private val random by lazy { SecureRandom() } // TODO: probably not everything has to be thread-safe here + private val isFullSession = AtomicBoolean(false) private val isEnabled = AtomicBoolean(false) private val isRecording = AtomicBoolean(false) private val currentReplayId = AtomicReference() @@ -61,6 +66,13 @@ class ReplayIntegration( ) } + private fun sample(rate: Double?): Boolean { + if (rate != null) { + return !(rate < random.nextDouble()) // bad luck + } + return false + } + override fun register(hub: IHub, options: SentryOptions) { this.options = options @@ -69,16 +81,26 @@ class ReplayIntegration( return } - // TODO: check for replaysSessionSampleRate and replaysOnErrorSampleRate + if (!options._experimental.replayOptions.isSessionReplayEnabled && + !options._experimental.replayOptions.isSessionReplayForErrorsEnabled + ) { + options.logger.log(INFO, "Session replay is disabled, no sample rate specified") + return + } this.hub = hub recorder = WindowRecorder(options, recorderConfig, this) isEnabled.set(true) + isFullSession.set(sample(options._experimental.replayOptions.sessionSampleRate)) + + addIntegrationToSdkVersion(javaClass) + SentryIntegrationPackageStorage.getInstance() + .addPackage("maven:io.sentry:sentry-android-replay", BuildConfig.VERSION_NAME) } fun isRecording() = isRecording.get() - fun start() { + override fun start() { if (!isEnabled.get()) { options.logger.log( DEBUG, @@ -97,7 +119,11 @@ class ReplayIntegration( currentSegment.set(0) currentReplayId.set(SentryId()) - hub?.configureScope { it.replayId = currentReplayId.get() } + 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 + hub?.configureScope { it.replayId = currentReplayId.get() } + } cache = ReplayCache(options, currentReplayId.get(), recorderConfig) recorder?.startRecording() @@ -105,29 +131,76 @@ class ReplayIntegration( // TODO: finalize old recording if there's some left on disk and send it using the replayId from persisted scope (e.g. for ANRs) } - fun resume() { + override fun resume() { segmentTimestamp.set(DateUtils.getCurrentDateTime()) recorder?.resume() } - fun pause() { + override fun sendReplayForEvent(event: SentryEvent) { + if (isFullSession.get()) { + options.logger.log(DEBUG, "Replay is already running in 'session' mode, not capturing for event %s", event.eventId) + return + } + + if (!(event.isErrored || event.isCrashed)) { + options.logger.log(DEBUG, "Event is not error or crash, not capturing for event %s", event.eventId) + return + } + + if (!sample(options._experimental.replayOptions.errorSampleRate)) { + options.logger.log(INFO, "Replay wasn't sampled by errorSampleRate, not capturing for event %s", event.eventId) + return + } + + val errorReplayDuration = options._experimental.replayOptions.errorReplayDuration + val now = dateProvider.currentTimeMillis + val currentSegmentTimestamp = if (cache?.frames?.isNotEmpty() == true) { + // in buffer mode we have to set the timestamp of the first frame as the actual start + DateUtils.getDateTime(cache!!.frames.first().timestamp) + } else { + DateUtils.getDateTime(now - errorReplayDuration) + } + val segmentId = currentSegment.get() + val replayId = currentReplayId.get() + saver.submit { + val videoDuration = + createAndCaptureSegment(now - currentSegmentTimestamp.time, currentSegmentTimestamp, replayId, segmentId, BUFFER) + if (videoDuration != null) { + currentSegment.getAndIncrement() + } + // since we're switching to session mode, even if the video is not sent for an error + // we still set the timestamp to now, because session is technically started "now" + segmentTimestamp.set(DateUtils.getDateTime(now)) + } + + hub?.configureScope { it.replayId = currentReplayId.get() } + // don't ask me why + event.setTag("replayId", currentReplayId.get().toString()) + isFullSession.set(true) + } + + override fun pause() { val now = dateProvider.currentTimeMillis recorder?.pause() + if (!isFullSession.get()) { + return + } + val currentSegmentTimestamp = segmentTimestamp.get() val segmentId = currentSegment.get() val duration = now - currentSegmentTimestamp.time val replayId = currentReplayId.get() saver.submit { val videoDuration = - createAndCaptureSegment(duration, currentSegmentTimestamp, replayId, segmentId) + createAndCaptureSegment(duration, currentSegmentTimestamp, replayId, segmentId, SESSION) if (videoDuration != null) { currentSegment.getAndIncrement() } } } - fun stop() { + override fun stop() { if (!isEnabled.get()) { options.logger.log( DEBUG, @@ -143,7 +216,10 @@ class ReplayIntegration( val replayId = currentReplayId.get() val replayCacheDir = cache?.replayCacheDir saver.submit { - createAndCaptureSegment(duration, currentSegmentTimestamp, replayId, segmentId) + // 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, SESSION) + } FileUtils.deleteRecursively(replayCacheDir) } @@ -164,23 +240,28 @@ class ReplayIntegration( cache?.addFrame(bitmap, frameTimestamp) val now = dateProvider.currentTimeMillis - if (now - segmentTimestamp.get().time >= VIDEO_SEGMENT_DURATION) { + if (isFullSession.get() && + (now - segmentTimestamp.get().time >= options._experimental.replayOptions.sessionSegmentDuration) + ) { val currentSegmentTimestamp = segmentTimestamp.get() val segmentId = currentSegment.get() val replayId = currentReplayId.get() val videoDuration = createAndCaptureSegment( - VIDEO_SEGMENT_DURATION, + options._experimental.replayOptions.sessionSegmentDuration, currentSegmentTimestamp, replayId, - segmentId + segmentId, + SESSION ) if (videoDuration != null) { currentSegment.getAndIncrement() // 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()) { + cache?.rotate(now - options._experimental.replayOptions.errorReplayDuration) } } } @@ -189,7 +270,8 @@ class ReplayIntegration( duration: Long, currentSegmentTimestamp: Date, replayId: SentryId, - segmentId: Int + segmentId: Int, + replayType: ReplayType ): Long? { val generatedVideo = cache?.createVideoOf( duration, @@ -204,7 +286,8 @@ class ReplayIntegration( currentSegmentTimestamp, segmentId, frameCount, - videoDuration + videoDuration, + replayType ) return videoDuration } @@ -215,7 +298,8 @@ class ReplayIntegration( segmentTimestamp: Date, segmentId: Int, frameCount: Int, - duration: Long + duration: Long, + replayType: ReplayType ) { val replay = SentryReplayEvent().apply { eventId = currentReplayId @@ -225,6 +309,7 @@ class ReplayIntegration( if (segmentId == 0) { replayStartTimestamp = segmentTimestamp } + this.replayType = replayType videoFile = video } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index fcede26562..f2ad30d938 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -45,6 +45,7 @@ public final class io/sentry/Baggage { public fun getEnvironment ()Ljava/lang/String; public fun getPublicKey ()Ljava/lang/String; public fun getRelease ()Ljava/lang/String; + public fun getReplayId ()Ljava/lang/String; public fun getSampleRate ()Ljava/lang/String; public fun getSampleRateDouble ()Ljava/lang/Double; public fun getSampled ()Ljava/lang/String; @@ -59,6 +60,7 @@ public final class io/sentry/Baggage { public fun setEnvironment (Ljava/lang/String;)V public fun setPublicKey (Ljava/lang/String;)V public fun setRelease (Ljava/lang/String;)V + public fun setReplayId (Ljava/lang/String;)V public fun setSampleRate (Ljava/lang/String;)V public fun setSampled (Ljava/lang/String;)V public fun setTraceId (Ljava/lang/String;)V @@ -66,7 +68,7 @@ public final class io/sentry/Baggage { public fun setUserId (Ljava/lang/String;)V public fun setUserSegment (Ljava/lang/String;)V public fun setValuesFromScope (Lio/sentry/IScope;Lio/sentry/SentryOptions;)V - public fun setValuesFromTransaction (Lio/sentry/ITransaction;Lio/sentry/protocol/User;Lio/sentry/SentryOptions;Lio/sentry/TracesSamplingDecision;)V + public fun setValuesFromTransaction (Lio/sentry/ITransaction;Lio/sentry/protocol/User;Lio/sentry/protocol/SentryId;Lio/sentry/SentryOptions;Lio/sentry/TracesSamplingDecision;)V public fun toHeaderString (Ljava/lang/String;)Ljava/lang/String; public fun toTraceContext ()Lio/sentry/TraceContext; } @@ -76,6 +78,7 @@ public final class io/sentry/Baggage$DSCKeys { public static final field ENVIRONMENT Ljava/lang/String; public static final field PUBLIC_KEY Ljava/lang/String; public static final field RELEASE Ljava/lang/String; + public static final field REPLAY_ID Ljava/lang/String; public static final field SAMPLED Ljava/lang/String; public static final field SAMPLE_RATE Ljava/lang/String; public static final field TRACE_ID Ljava/lang/String; @@ -1201,6 +1204,15 @@ public final class io/sentry/NoOpLogger : io/sentry/ILogger { public fun log (Lio/sentry/SentryLevel;Ljava/lang/Throwable;Ljava/lang/String;[Ljava/lang/Object;)V } +public final class io/sentry/NoOpReplayController : io/sentry/ReplayController { + public static fun getInstance ()Lio/sentry/NoOpReplayController; + public fun pause ()V + public fun resume ()V + public fun sendReplayForEvent (Lio/sentry/SentryEvent;)V + public fun start ()V + public fun stop ()V +} + public final class io/sentry/NoOpScope : io/sentry/IScope { public fun addAttachment (Lio/sentry/Attachment;)V public fun addBreadcrumb (Lio/sentry/Breadcrumb;)V @@ -1591,6 +1603,14 @@ public final class io/sentry/PropagationContext { public fun traceContext ()Lio/sentry/TraceContext; } +public abstract interface class io/sentry/ReplayController { + public abstract fun pause ()V + public abstract fun resume ()V + public abstract fun sendReplayForEvent (Lio/sentry/SentryEvent;)V + public abstract fun start ()V + public abstract fun stop ()V +} + public final class io/sentry/ReplayRecording : io/sentry/JsonSerializable, io/sentry/JsonUnknown { public fun ()V public fun equals (Ljava/lang/Object;)Z @@ -2300,6 +2320,7 @@ public class io/sentry/SentryOptions { public fun getProxy ()Lio/sentry/SentryOptions$Proxy; public fun getReadTimeoutMillis ()I public fun getRelease ()Ljava/lang/String; + public fun getReplayController ()Lio/sentry/ReplayController; public fun getSampleRate ()Ljava/lang/Double; public fun getScopeObservers ()Ljava/util/List; public fun getSdkVersion ()Lio/sentry/protocol/SdkVersion; @@ -2406,6 +2427,7 @@ public class io/sentry/SentryOptions { public fun setProxy (Lio/sentry/SentryOptions$Proxy;)V public fun setReadTimeoutMillis (I)V public fun setRelease (Ljava/lang/String;)V + public fun setReplayController (Lio/sentry/ReplayController;)V public fun setSampleRate (Ljava/lang/Double;)V public fun setSdkVersion (Lio/sentry/protocol/SdkVersion;)V public fun setSendClientReports (Z)V @@ -2549,6 +2571,8 @@ public final class io/sentry/SentryReplayOptions { public fun getFrameRate ()I public fun getSessionSampleRate ()Ljava/lang/Double; public fun getSessionSegmentDuration ()J + public fun isSessionReplayEnabled ()Z + public fun isSessionReplayForErrorsEnabled ()Z public fun setErrorSampleRate (Ljava/lang/Double;)V public fun setSessionSampleRate (Ljava/lang/Double;)V } @@ -2899,6 +2923,7 @@ public final class io/sentry/TraceContext : io/sentry/JsonSerializable, io/sentr public fun getEnvironment ()Ljava/lang/String; public fun getPublicKey ()Ljava/lang/String; public fun getRelease ()Ljava/lang/String; + public fun getReplayId ()Lio/sentry/protocol/SentryId; public fun getSampleRate ()Ljava/lang/String; public fun getSampled ()Ljava/lang/String; public fun getTraceId ()Lio/sentry/protocol/SentryId; @@ -2920,6 +2945,7 @@ public final class io/sentry/TraceContext$JsonKeys { public static final field ENVIRONMENT Ljava/lang/String; public static final field PUBLIC_KEY Ljava/lang/String; public static final field RELEASE Ljava/lang/String; + public static final field REPLAY_ID Ljava/lang/String; public static final field SAMPLED Ljava/lang/String; public static final field SAMPLE_RATE Ljava/lang/String; public static final field TRACE_ID Ljava/lang/String; diff --git a/sentry/src/main/java/io/sentry/Baggage.java b/sentry/src/main/java/io/sentry/Baggage.java index 8e19fceaf8..c6a1ee5630 100644 --- a/sentry/src/main/java/io/sentry/Baggage.java +++ b/sentry/src/main/java/io/sentry/Baggage.java @@ -141,6 +141,7 @@ public static Baggage fromEvent( // we don't persist sample rate baggage.setSampleRate(null); baggage.setSampled(null); + // TODO: add replay_id later baggage.freeze(); return baggage; } @@ -345,6 +346,16 @@ public void setSampled(final @Nullable String sampled) { set(DSCKeys.SAMPLED, sampled); } + @ApiStatus.Internal + public @Nullable String getReplayId() { + return get(DSCKeys.REPLAY_ID); + } + + @ApiStatus.Internal + public void setReplayId(final @Nullable String replayId) { + set(DSCKeys.REPLAY_ID, replayId); + } + @ApiStatus.Internal public void set(final @NotNull String key, final @Nullable String value) { if (mutable) { @@ -373,6 +384,7 @@ public void set(final @NotNull String key, final @Nullable String value) { public void setValuesFromTransaction( final @NotNull ITransaction transaction, final @Nullable User user, + final @Nullable SentryId replayId, final @NotNull SentryOptions sentryOptions, final @Nullable TracesSamplingDecision samplingDecision) { setTraceId(transaction.getSpanContext().getTraceId().toString()); @@ -384,6 +396,9 @@ public void setValuesFromTransaction( isHighQualityTransactionName(transaction.getTransactionNameSource()) ? transaction.getName() : null); + if (replayId != null) { + setReplayId(replayId.toString()); + } setSampleRate(sampleRateToString(sampleRate(samplingDecision))); setSampled(StringUtils.toString(sampled(samplingDecision))); } @@ -397,6 +412,10 @@ public void setValuesFromScope( setPublicKey(new Dsn(options.getDsn()).getPublicKey()); setRelease(options.getRelease()); setEnvironment(options.getEnvironment()); + final @Nullable SentryId replayId = scope.getReplayId(); + if (replayId != null) { + setReplayId(replayId.toString()); + } setUserSegment(user != null ? getSegment(user) : null); setTransaction(null); setSampleRate(null); @@ -468,6 +487,7 @@ private static boolean isHighQualityTransactionName( @Nullable public TraceContext toTraceContext() { final String traceIdString = getTraceId(); + final String replayIdString = getReplayId(); final String publicKey = getPublicKey(); if (traceIdString != null && publicKey != null) { @@ -481,7 +501,8 @@ public TraceContext toTraceContext() { getUserSegment(), getTransaction(), getSampleRate(), - getSampled()); + getSampled(), + replayIdString == null ? null : new SentryId(replayIdString)); traceContext.setUnknown(getUnknown()); return traceContext; } else { @@ -500,6 +521,7 @@ public static final class DSCKeys { public static final String TRANSACTION = "sentry-transaction"; public static final String SAMPLE_RATE = "sentry-sample_rate"; public static final String SAMPLED = "sentry-sampled"; + public static final String REPLAY_ID = "sentry-replay_id"; public static final List ALL = Arrays.asList( @@ -511,6 +533,7 @@ public static final class DSCKeys { USER_SEGMENT, TRANSACTION, SAMPLE_RATE, - SAMPLED); + SAMPLED, + REPLAY_ID); } } diff --git a/sentry/src/main/java/io/sentry/NoOpReplayController.java b/sentry/src/main/java/io/sentry/NoOpReplayController.java new file mode 100644 index 0000000000..0a11e71423 --- /dev/null +++ b/sentry/src/main/java/io/sentry/NoOpReplayController.java @@ -0,0 +1,29 @@ +package io.sentry; + +import org.jetbrains.annotations.NotNull; + +public final class NoOpReplayController implements ReplayController { + + private static final NoOpReplayController instance = new NoOpReplayController(); + + public static NoOpReplayController getInstance() { + return instance; + } + + private NoOpReplayController() {} + + @Override + public void start() {} + + @Override + public void stop() {} + + @Override + public void pause() {} + + @Override + public void resume() {} + + @Override + public void sendReplayForEvent(@NotNull SentryEvent event) {} +} diff --git a/sentry/src/main/java/io/sentry/ReplayController.java b/sentry/src/main/java/io/sentry/ReplayController.java new file mode 100644 index 0000000000..2ccc28cb82 --- /dev/null +++ b/sentry/src/main/java/io/sentry/ReplayController.java @@ -0,0 +1,17 @@ +package io.sentry; + +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; + +@ApiStatus.Internal +public interface ReplayController { + void start(); + + void stop(); + + void pause(); + + void resume(); + + void sendReplayForEvent(@NotNull SentryEvent event); +} diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 129d415a8a..a6c3c4f0d4 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -191,6 +191,10 @@ private boolean shouldApplyScopeData(final @NotNull CheckIn event, final @NotNul sentryId = event.getEventId(); } + if (event != null) { + options.getReplayController().sendReplayForEvent(event); + } + try { @Nullable TraceContext traceContext = null; if (HintUtils.hasType(hint, Backfillable.class)) { @@ -227,23 +231,42 @@ private boolean shouldApplyScopeData(final @NotNull CheckIn event, final @NotNul } // if we encountered a crash/abnormal exit finish tracing in order to persist and send - // any running transaction / profiling data + // any running transaction / profiling data. We also finish session replay, and it has priority + // over transactions as it takes longer to finalize replay than transactions, therefore + // the replay_id will be the trigger for flushing and unblocking the thread in case of a crash if (scope != null) { - final @Nullable ITransaction transaction = scope.getTransaction(); - if (transaction != null) { - if (HintUtils.hasType(hint, TransactionEnd.class)) { - final Object sentrySdkHint = HintUtils.getSentrySdkHint(hint); - if (sentrySdkHint instanceof DiskFlushNotification) { - ((DiskFlushNotification) sentrySdkHint).setFlushable(transaction.getEventId()); - transaction.forceFinish(SpanStatus.ABORTED, false, hint); - } else { - transaction.forceFinish(SpanStatus.ABORTED, false, null); - } + finalizeTransaction(scope, hint); + finalizeReplay(scope, hint); + } + + return sentryId; + } + + private void finalizeTransaction(final @NotNull IScope scope, final @NotNull Hint hint) { + final @Nullable ITransaction transaction = scope.getTransaction(); + if (transaction != null) { + if (HintUtils.hasType(hint, TransactionEnd.class)) { + final Object sentrySdkHint = HintUtils.getSentrySdkHint(hint); + if (sentrySdkHint instanceof DiskFlushNotification) { + ((DiskFlushNotification) sentrySdkHint).setFlushable(transaction.getEventId()); + transaction.forceFinish(SpanStatus.ABORTED, false, hint); + } else { + transaction.forceFinish(SpanStatus.ABORTED, false, null); } } } + } - return sentryId; + private void finalizeReplay(final @NotNull IScope scope, final @NotNull Hint hint) { + final @Nullable SentryId replayId = scope.getReplayId(); + if (replayId != null) { + if (HintUtils.hasType(hint, TransactionEnd.class)) { + final Object sentrySdkHint = HintUtils.getSentrySdkHint(hint); + if (sentrySdkHint instanceof DiskFlushNotification) { + ((DiskFlushNotification) sentrySdkHint).setFlushable(replayId); + } + } + } } @Override diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 026f2c9e90..50f724591a 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -462,6 +462,8 @@ public class SentryOptions { private final @NotNull ExperimentalOptions _experimental = new ExperimentalOptions(); + private @NotNull ReplayController replayController = NoOpReplayController.getInstance(); + /** * Adds an event processor * @@ -2281,6 +2283,15 @@ public ExperimentalOptions get_experimental() { return _experimental; } + public @NotNull ReplayController getReplayController() { + return replayController; + } + + public void setReplayController(final @Nullable ReplayController replayController) { + this.replayController = + replayController != null ? replayController : NoOpReplayController.getInstance(); + } + /** The BeforeSend callback */ public interface BeforeSendCallback { diff --git a/sentry/src/main/java/io/sentry/SentryReplayOptions.java b/sentry/src/main/java/io/sentry/SentryReplayOptions.java index df98fc384f..d702d6256b 100644 --- a/sentry/src/main/java/io/sentry/SentryReplayOptions.java +++ b/sentry/src/main/java/io/sentry/SentryReplayOptions.java @@ -51,6 +51,10 @@ public Double getErrorSampleRate() { return errorSampleRate; } + public boolean isSessionReplayEnabled() { + return (getSessionSampleRate() != null && getSessionSampleRate() > 0); + } + public void setErrorSampleRate(final @Nullable Double errorSampleRate) { if (!SampleRateUtils.isValidSampleRate(errorSampleRate)) { throw new IllegalArgumentException( @@ -66,6 +70,10 @@ public Double getSessionSampleRate() { return sessionSampleRate; } + public boolean isSessionReplayForErrorsEnabled() { + return (getErrorSampleRate() != null && getErrorSampleRate() > 0); + } + public void setSessionSampleRate(final @Nullable Double sessionSampleRate) { if (!SampleRateUtils.isValidSampleRate(sessionSampleRate)) { throw new IllegalArgumentException( diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index 8c1536cbbf..320f79680b 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -582,12 +582,18 @@ private void updateBaggageValues() { synchronized (this) { if (baggage.isMutable()) { final AtomicReference userAtomicReference = new AtomicReference<>(); + final AtomicReference replayId = new AtomicReference<>(); hub.configureScope( scope -> { userAtomicReference.set(scope.getUser()); + replayId.set(scope.getReplayId()); }); baggage.setValuesFromTransaction( - this, userAtomicReference.get(), hub.getOptions(), this.getSamplingDecision()); + this, + userAtomicReference.get(), + replayId.get(), + hub.getOptions(), + this.getSamplingDecision()); baggage.freeze(); } } diff --git a/sentry/src/main/java/io/sentry/TraceContext.java b/sentry/src/main/java/io/sentry/TraceContext.java index df799aaa07..da34382d51 100644 --- a/sentry/src/main/java/io/sentry/TraceContext.java +++ b/sentry/src/main/java/io/sentry/TraceContext.java @@ -21,12 +21,13 @@ public final class TraceContext implements JsonUnknown, JsonSerializable { private final @Nullable String transaction; private final @Nullable String sampleRate; private final @Nullable String sampled; + private final @Nullable SentryId replayId; @SuppressWarnings("unused") private @Nullable Map unknown; TraceContext(@NotNull SentryId traceId, @NotNull String publicKey) { - this(traceId, publicKey, null, null, null, null, null, null, null); + this(traceId, publicKey, null, null, null, null, null, null, null, null); } TraceContext( @@ -38,7 +39,8 @@ public final class TraceContext implements JsonUnknown, JsonSerializable { @Nullable String userSegment, @Nullable String transaction, @Nullable String sampleRate, - @Nullable String sampled) { + @Nullable String sampled, + @Nullable SentryId replayId) { this.traceId = traceId; this.publicKey = publicKey; this.release = release; @@ -48,6 +50,7 @@ public final class TraceContext implements JsonUnknown, JsonSerializable { this.transaction = transaction; this.sampleRate = sampleRate; this.sampled = sampled; + this.replayId = replayId; } @SuppressWarnings("UnusedMethod") @@ -96,6 +99,10 @@ public final class TraceContext implements JsonUnknown, JsonSerializable { return sampled; } + public @Nullable SentryId getReplayId() { + return replayId; + } + /** * @deprecated only here to support parsing legacy JSON with non flattened user */ @@ -198,6 +205,7 @@ public static final class JsonKeys { public static final String TRANSACTION = "transaction"; public static final String SAMPLE_RATE = "sample_rate"; public static final String SAMPLED = "sampled"; + public static final String REPLAY_ID = "replay_id"; } @Override @@ -227,6 +235,9 @@ public void serialize(final @NotNull ObjectWriter writer, final @NotNull ILogger if (sampled != null) { writer.name(TraceContext.JsonKeys.SAMPLED).value(sampled); } + if (replayId != null) { + writer.name(TraceContext.JsonKeys.REPLAY_ID).value(logger, replayId); + } if (unknown != null) { for (String key : unknown.keySet()) { Object value = unknown.get(key); @@ -253,6 +264,7 @@ public static final class Deserializer implements JsonDeserializer String transaction = null; String sampleRate = null; String sampled = null; + SentryId replayId = null; Map unknown = null; while (reader.peek() == JsonToken.NAME) { @@ -288,6 +300,9 @@ public static final class Deserializer implements JsonDeserializer case TraceContext.JsonKeys.SAMPLED: sampled = reader.nextStringOrNull(); break; + case TraceContext.JsonKeys.REPLAY_ID: + replayId = new SentryId.Deserializer().deserialize(reader, logger); + break; default: if (unknown == null) { unknown = new ConcurrentHashMap<>(); @@ -320,7 +335,8 @@ public static final class Deserializer implements JsonDeserializer userSegment, transaction, sampleRate, - sampled); + sampled, + replayId); traceContext.setUnknown(unknown); reader.endObject(); return traceContext; diff --git a/sentry/src/test/java/io/sentry/TraceContextSerializationTest.kt b/sentry/src/test/java/io/sentry/TraceContextSerializationTest.kt index e79e5ebf8c..f2a674d554 100644 --- a/sentry/src/test/java/io/sentry/TraceContextSerializationTest.kt +++ b/sentry/src/test/java/io/sentry/TraceContextSerializationTest.kt @@ -62,6 +62,7 @@ class TraceContextSerializationTest { id = "user-id" others = mapOf("segment" to "pro") }, + SentryId(), SentryOptions().apply { dsn = dsnString environment = "prod" From f7ac74f08c79c67b4fcfb22a9ed565d26a5fb413 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 28 Mar 2024 12:32:53 +0100 Subject: [PATCH 2/7] Pass hint to captureReplay --- .../api/sentry-android-replay.api | 2 +- .../android/replay/ReplayIntegration.kt | 23 ++++++++++--------- sentry/api/sentry.api | 4 ++-- .../java/io/sentry/NoOpReplayController.java | 2 +- .../main/java/io/sentry/ReplayController.java | 2 +- .../src/main/java/io/sentry/SentryClient.java | 2 +- 6 files changed, 18 insertions(+), 17 deletions(-) diff --git a/sentry-android-replay/api/sentry-android-replay.api b/sentry-android-replay/api/sentry-android-replay.api index 82cfece004..1e107ca495 100644 --- a/sentry-android-replay/api/sentry-android-replay.api +++ b/sentry-android-replay/api/sentry-android-replay.api @@ -14,7 +14,7 @@ public final class io/sentry/android/replay/ReplayIntegration : io/sentry/Integr public fun pause ()V public fun register (Lio/sentry/IHub;Lio/sentry/SentryOptions;)V public fun resume ()V - public fun sendReplayForEvent (Lio/sentry/SentryEvent;)V + public fun sendReplayForEvent (Lio/sentry/SentryEvent;Lio/sentry/Hint;)V public fun start ()V public fun stop ()V } diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt index da14c72bd4..bedc8bab81 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt @@ -136,7 +136,7 @@ class ReplayIntegration( recorder?.resume() } - override fun sendReplayForEvent(event: SentryEvent) { + override fun sendReplayForEvent(event: SentryEvent, hint: Hint) { if (isFullSession.get()) { options.logger.log(DEBUG, "Replay is already running in 'session' mode, not capturing for event %s", event.eventId) return @@ -164,7 +164,7 @@ class ReplayIntegration( val replayId = currentReplayId.get() saver.submit { val videoDuration = - createAndCaptureSegment(now - currentSegmentTimestamp.time, currentSegmentTimestamp, replayId, segmentId, BUFFER) + createAndCaptureSegment(now - currentSegmentTimestamp.time, currentSegmentTimestamp, replayId, segmentId, BUFFER, hint) if (videoDuration != null) { currentSegment.getAndIncrement() } @@ -193,7 +193,7 @@ class ReplayIntegration( val replayId = currentReplayId.get() saver.submit { val videoDuration = - createAndCaptureSegment(duration, currentSegmentTimestamp, replayId, segmentId, SESSION) + createAndCaptureSegment(duration, currentSegmentTimestamp, replayId, segmentId) if (videoDuration != null) { currentSegment.getAndIncrement() } @@ -218,7 +218,7 @@ class ReplayIntegration( saver.submit { // 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, SESSION) + createAndCaptureSegment(duration, currentSegmentTimestamp, replayId, segmentId) } FileUtils.deleteRecursively(replayCacheDir) } @@ -252,8 +252,7 @@ class ReplayIntegration( options._experimental.replayOptions.sessionSegmentDuration, currentSegmentTimestamp, replayId, - segmentId, - SESSION + segmentId ) if (videoDuration != null) { currentSegment.getAndIncrement() @@ -271,7 +270,8 @@ class ReplayIntegration( currentSegmentTimestamp: Date, replayId: SentryId, segmentId: Int, - replayType: ReplayType + replayType: ReplayType = SESSION, + hint: Hint? = null ): Long? { val generatedVideo = cache?.createVideoOf( duration, @@ -287,7 +287,8 @@ class ReplayIntegration( segmentId, frameCount, videoDuration, - replayType + replayType, + hint ) return videoDuration } @@ -299,7 +300,8 @@ class ReplayIntegration( segmentId: Int, frameCount: Int, duration: Long, - replayType: ReplayType + replayType: ReplayType, + hint: Hint? = null ) { val replay = SentryReplayEvent().apply { eventId = currentReplayId @@ -337,8 +339,7 @@ class ReplayIntegration( ) } - val hint = Hint().apply { replayRecording = recording } - hub?.captureReplay(replay, hint) + hub?.captureReplay(replay, (hint ?: Hint()).apply { replayRecording = recording }) } override fun close() { diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index f2ad30d938..28ffd71885 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -1208,7 +1208,7 @@ public final class io/sentry/NoOpReplayController : io/sentry/ReplayController { public static fun getInstance ()Lio/sentry/NoOpReplayController; public fun pause ()V public fun resume ()V - public fun sendReplayForEvent (Lio/sentry/SentryEvent;)V + public fun sendReplayForEvent (Lio/sentry/SentryEvent;Lio/sentry/Hint;)V public fun start ()V public fun stop ()V } @@ -1606,7 +1606,7 @@ public final class io/sentry/PropagationContext { public abstract interface class io/sentry/ReplayController { public abstract fun pause ()V public abstract fun resume ()V - public abstract fun sendReplayForEvent (Lio/sentry/SentryEvent;)V + public abstract fun sendReplayForEvent (Lio/sentry/SentryEvent;Lio/sentry/Hint;)V public abstract fun start ()V public abstract fun stop ()V } diff --git a/sentry/src/main/java/io/sentry/NoOpReplayController.java b/sentry/src/main/java/io/sentry/NoOpReplayController.java index 0a11e71423..d052fba8b4 100644 --- a/sentry/src/main/java/io/sentry/NoOpReplayController.java +++ b/sentry/src/main/java/io/sentry/NoOpReplayController.java @@ -25,5 +25,5 @@ public void pause() {} public void resume() {} @Override - public void sendReplayForEvent(@NotNull SentryEvent event) {} + public void sendReplayForEvent(@NotNull SentryEvent event, @NotNull Hint hint) {} } diff --git a/sentry/src/main/java/io/sentry/ReplayController.java b/sentry/src/main/java/io/sentry/ReplayController.java index 2ccc28cb82..a45a0ecda2 100644 --- a/sentry/src/main/java/io/sentry/ReplayController.java +++ b/sentry/src/main/java/io/sentry/ReplayController.java @@ -13,5 +13,5 @@ public interface ReplayController { void resume(); - void sendReplayForEvent(@NotNull SentryEvent event); + void sendReplayForEvent(@NotNull SentryEvent event, @NotNull Hint hint); } diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index a6c3c4f0d4..a7af22a615 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -192,7 +192,7 @@ private boolean shouldApplyScopeData(final @NotNull CheckIn event, final @NotNul } if (event != null) { - options.getReplayController().sendReplayForEvent(event); + options.getReplayController().sendReplayForEvent(event, hint); } try { From da37c898f70bb13f712a6cfd22626addd37aabf8 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 28 Mar 2024 20:40:36 +0100 Subject: [PATCH 3/7] Merge conflicts --- .../api/sentry-android-replay.api | 1 + .../sentry/android/replay/ReplayIntegration.kt | 16 ++++++++-------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/sentry-android-replay/api/sentry-android-replay.api b/sentry-android-replay/api/sentry-android-replay.api index a1641def95..0a064b803f 100644 --- a/sentry-android-replay/api/sentry-android-replay.api +++ b/sentry-android-replay/api/sentry-android-replay.api @@ -27,6 +27,7 @@ public final class io/sentry/android/replay/ReplayCache : java/io/Closeable { public fun close ()V public final fun createVideoOf (JJILjava/io/File;)Lio/sentry/android/replay/GeneratedVideo; public static synthetic fun createVideoOf$default (Lio/sentry/android/replay/ReplayCache;JJILjava/io/File;ILjava/lang/Object;)Lio/sentry/android/replay/GeneratedVideo; + public final fun rotate (J)V } public final class io/sentry/android/replay/ReplayIntegration : io/sentry/Integration, io/sentry/ReplayController, io/sentry/android/replay/ScreenshotRecorderCallback, java/io/Closeable { diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt index 267bbb0c01..ea4b365dfd 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt @@ -81,8 +81,8 @@ class ReplayIntegration( return } - if (!options._experimental.replayOptions.isSessionReplayEnabled && - !options._experimental.replayOptions.isSessionReplayForErrorsEnabled + if (!options.experimental.replayOptions.isSessionReplayEnabled && + !options.experimental.replayOptions.isSessionReplayForErrorsEnabled ) { options.logger.log(INFO, "Session replay is disabled, no sample rate specified") return @@ -91,7 +91,7 @@ class ReplayIntegration( this.hub = hub recorder = WindowRecorder(options, recorderConfig, this) isEnabled.set(true) - isFullSession.set(sample(options._experimental.replayOptions.sessionSampleRate)) + isFullSession.set(sample(options.experimental.replayOptions.sessionSampleRate)) addIntegrationToSdkVersion(javaClass) SentryIntegrationPackageStorage.getInstance() @@ -147,12 +147,12 @@ class ReplayIntegration( return } - if (!sample(options._experimental.replayOptions.errorSampleRate)) { + if (!sample(options.experimental.replayOptions.errorSampleRate)) { options.logger.log(INFO, "Replay wasn't sampled by errorSampleRate, not capturing for event %s", event.eventId) return } - val errorReplayDuration = options._experimental.replayOptions.errorReplayDuration + val errorReplayDuration = options.experimental.replayOptions.errorReplayDuration val now = dateProvider.currentTimeMillis val currentSegmentTimestamp = if (cache?.frames?.isNotEmpty() == true) { // in buffer mode we have to set the timestamp of the first frame as the actual start @@ -241,7 +241,7 @@ class ReplayIntegration( val now = dateProvider.currentTimeMillis if (isFullSession.get() && - (now - segmentTimestamp.get().time >= options._experimental.replayOptions.sessionSegmentDuration) + (now - segmentTimestamp.get().time >= options.experimental.replayOptions.sessionSegmentDuration) ) { val currentSegmentTimestamp = segmentTimestamp.get() val segmentId = currentSegment.get() @@ -249,7 +249,7 @@ class ReplayIntegration( val videoDuration = createAndCaptureSegment( - options._experimental.replayOptions.sessionSegmentDuration, + options.experimental.replayOptions.sessionSegmentDuration, currentSegmentTimestamp, replayId, segmentId @@ -260,7 +260,7 @@ class ReplayIntegration( segmentTimestamp.set(DateUtils.getDateTime(currentSegmentTimestamp.time + videoDuration)) } } else if (!isFullSession.get()) { - cache?.rotate(now - options._experimental.replayOptions.errorReplayDuration) + cache?.rotate(now - options.experimental.replayOptions.errorReplayDuration) } } } From c53a975af35370d834f3829d6cf23cd405dc092c Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 2 Apr 2024 21:43:19 +0200 Subject: [PATCH 4/7] Fix tests --- sentry/src/test/java/io/sentry/JsonSerializerTest.kt | 8 ++++---- .../test/java/io/sentry/TraceContextSerializationTest.kt | 3 ++- sentry/src/test/resources/json/trace_state.json | 3 ++- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt index a894fcfff3..7214b3643e 100644 --- a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt @@ -443,16 +443,16 @@ class JsonSerializerTest { @Test fun `serializes trace context`() { - val traceContext = SentryEnvelopeHeader(null, null, TraceContext(SentryId("3367f5196c494acaae85bbbd535379ac"), "key", "release", "environment", "userId", "segment", "transaction", "0.5", "true")) - val expected = """{"trace":{"trace_id":"3367f5196c494acaae85bbbd535379ac","public_key":"key","release":"release","environment":"environment","user_id":"userId","user_segment":"segment","transaction":"transaction","sample_rate":"0.5","sampled":"true"}}""" + val traceContext = SentryEnvelopeHeader(null, null, TraceContext(SentryId("3367f5196c494acaae85bbbd535379ac"), "key", "release", "environment", "userId", "segment", "transaction", "0.5", "true", SentryId("3367f5196c494acaae85bbbd535379aa"))) + val expected = """{"trace":{"trace_id":"3367f5196c494acaae85bbbd535379ac","public_key":"key","release":"release","environment":"environment","user_id":"userId","user_segment":"segment","transaction":"transaction","sample_rate":"0.5","sampled":"true","replay_id":"3367f5196c494acaae85bbbd535379aa"}}""" val json = serializeToString(traceContext) assertEquals(expected, json) } @Test fun `serializes trace context with user having null id and segment`() { - val traceContext = SentryEnvelopeHeader(null, null, TraceContext(SentryId("3367f5196c494acaae85bbbd535379ac"), "key", "release", "environment", null, null, "transaction", "0.6", "false")) - val expected = """{"trace":{"trace_id":"3367f5196c494acaae85bbbd535379ac","public_key":"key","release":"release","environment":"environment","transaction":"transaction","sample_rate":"0.6","sampled":"false"}}""" + val traceContext = SentryEnvelopeHeader(null, null, TraceContext(SentryId("3367f5196c494acaae85bbbd535379ac"), "key", "release", "environment", null, null, "transaction", "0.6", "false", SentryId("3367f5196c494acaae85bbbd535379aa"))) + val expected = """{"trace":{"trace_id":"3367f5196c494acaae85bbbd535379ac","public_key":"key","release":"release","environment":"environment","transaction":"transaction","sample_rate":"0.6","sampled":"false","replay_id":"3367f5196c494acaae85bbbd535379aa"}}""" val json = serializeToString(traceContext) assertEquals(expected, json) } diff --git a/sentry/src/test/java/io/sentry/TraceContextSerializationTest.kt b/sentry/src/test/java/io/sentry/TraceContextSerializationTest.kt index f2a674d554..876ec12831 100644 --- a/sentry/src/test/java/io/sentry/TraceContextSerializationTest.kt +++ b/sentry/src/test/java/io/sentry/TraceContextSerializationTest.kt @@ -24,7 +24,8 @@ class TraceContextSerializationTest { "f7d8662b-5551-4ef8-b6a8-090f0561a530", "0252ec25-cd0a-4230-bd2f-936a4585637e", "0.00000021", - "true" + "true", + SentryId("3367f5196c494acaae85bbbd535379aa") ) } private val fixture = Fixture() diff --git a/sentry/src/test/resources/json/trace_state.json b/sentry/src/test/resources/json/trace_state.json index 17a95fdc33..6ca0e48e61 100644 --- a/sentry/src/test/resources/json/trace_state.json +++ b/sentry/src/test/resources/json/trace_state.json @@ -7,5 +7,6 @@ "user_segment": "f7d8662b-5551-4ef8-b6a8-090f0561a530", "transaction": "0252ec25-cd0a-4230-bd2f-936a4585637e", "sample_rate": "0.00000021", - "sampled": "true" + "sampled": "true", + "replay_id": "3367f5196c494acaae85bbbd535379aa" } From ad7d78d9530689315d815d64c823cc9e83a2e7db Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 3 Apr 2024 00:39:38 +0200 Subject: [PATCH 5/7] More conflicts --- sentry/src/main/java/io/sentry/Baggage.java | 8 ++++---- sentry/src/main/java/io/sentry/SentryClient.java | 2 +- sentry/src/main/java/io/sentry/SentryTracer.java | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/sentry/src/main/java/io/sentry/Baggage.java b/sentry/src/main/java/io/sentry/Baggage.java index c6a1ee5630..fbeddfe1ff 100644 --- a/sentry/src/main/java/io/sentry/Baggage.java +++ b/sentry/src/main/java/io/sentry/Baggage.java @@ -384,7 +384,7 @@ public void set(final @NotNull String key, final @Nullable String value) { public void setValuesFromTransaction( final @NotNull ITransaction transaction, final @Nullable User user, - final @Nullable SentryId replayId, + final @NotNull SentryId replayId, final @NotNull SentryOptions sentryOptions, final @Nullable TracesSamplingDecision samplingDecision) { setTraceId(transaction.getSpanContext().getTraceId().toString()); @@ -396,7 +396,7 @@ public void setValuesFromTransaction( isHighQualityTransactionName(transaction.getTransactionNameSource()) ? transaction.getName() : null); - if (replayId != null) { + if (!SentryId.EMPTY_ID.equals(replayId)) { setReplayId(replayId.toString()); } setSampleRate(sampleRateToString(sampleRate(samplingDecision))); @@ -408,12 +408,12 @@ public void setValuesFromScope( final @NotNull IScope scope, final @NotNull SentryOptions options) { final @NotNull PropagationContext propagationContext = scope.getPropagationContext(); final @Nullable User user = scope.getUser(); + final @NotNull SentryId replayId = scope.getReplayId(); setTraceId(propagationContext.getTraceId().toString()); setPublicKey(new Dsn(options.getDsn()).getPublicKey()); setRelease(options.getRelease()); setEnvironment(options.getEnvironment()); - final @Nullable SentryId replayId = scope.getReplayId(); - if (replayId != null) { + if (!SentryId.EMPTY_ID.equals(replayId)) { setReplayId(replayId.toString()); } setUserSegment(user != null ? getSegment(user) : null); diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index a7af22a615..e0a1177646 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -259,7 +259,7 @@ private void finalizeTransaction(final @NotNull IScope scope, final @NotNull Hin private void finalizeReplay(final @NotNull IScope scope, final @NotNull Hint hint) { final @Nullable SentryId replayId = scope.getReplayId(); - if (replayId != null) { + if (!SentryId.EMPTY_ID.equals(replayId)) { if (HintUtils.hasType(hint, TransactionEnd.class)) { final Object sentrySdkHint = HintUtils.getSentrySdkHint(hint); if (sentrySdkHint instanceof DiskFlushNotification) { diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index 320f79680b..fe8d4d0150 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -591,7 +591,7 @@ private void updateBaggageValues() { baggage.setValuesFromTransaction( this, userAtomicReference.get(), - replayId.get(), + replayId.get() == null ? SentryId.EMPTY_ID : replayId.get(), hub.getOptions(), this.getSamplingDecision()); baggage.freeze(); From ab00547fefe40593c87d2b1ed3d252d09470b3b0 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 3 Apr 2024 00:40:56 +0200 Subject: [PATCH 6/7] More conflicts --- sentry/src/main/java/io/sentry/Baggage.java | 4 ++-- sentry/src/main/java/io/sentry/SentryTracer.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sentry/src/main/java/io/sentry/Baggage.java b/sentry/src/main/java/io/sentry/Baggage.java index fbeddfe1ff..53bd10248e 100644 --- a/sentry/src/main/java/io/sentry/Baggage.java +++ b/sentry/src/main/java/io/sentry/Baggage.java @@ -384,7 +384,7 @@ public void set(final @NotNull String key, final @Nullable String value) { public void setValuesFromTransaction( final @NotNull ITransaction transaction, final @Nullable User user, - final @NotNull SentryId replayId, + final @Nullable SentryId replayId, final @NotNull SentryOptions sentryOptions, final @Nullable TracesSamplingDecision samplingDecision) { setTraceId(transaction.getSpanContext().getTraceId().toString()); @@ -396,7 +396,7 @@ public void setValuesFromTransaction( isHighQualityTransactionName(transaction.getTransactionNameSource()) ? transaction.getName() : null); - if (!SentryId.EMPTY_ID.equals(replayId)) { + if (replayId != null && !SentryId.EMPTY_ID.equals(replayId)) { setReplayId(replayId.toString()); } setSampleRate(sampleRateToString(sampleRate(samplingDecision))); diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index fe8d4d0150..320f79680b 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -591,7 +591,7 @@ private void updateBaggageValues() { baggage.setValuesFromTransaction( this, userAtomicReference.get(), - replayId.get() == null ? SentryId.EMPTY_ID : replayId.get(), + replayId.get(), hub.getOptions(), this.getSamplingDecision()); baggage.freeze(); From 7f78fee236caa6c8a4005883708d57ba777938b4 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 3 Apr 2024 01:03:01 +0200 Subject: [PATCH 7/7] Fix tests --- .../android/core/LifecycleWatcherTest.kt | 3 ++ .../sentry/android/core/SentryAndroidTest.kt | 1 + .../android/replay/ReplayIntegration.kt | 29 +++++++++++++------ sentry/src/test/java/io/sentry/BaggageTest.kt | 8 ++--- .../json/sentry_envelope_header.json | 3 +- 5 files changed, 29 insertions(+), 15 deletions(-) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt index 4b620813bf..2adcc9ee6e 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt @@ -7,6 +7,7 @@ import io.sentry.IHub import io.sentry.IScope import io.sentry.ScopeCallback import io.sentry.SentryLevel +import io.sentry.SentryOptions import io.sentry.Session import io.sentry.Session.State import io.sentry.transport.ICurrentDateProvider @@ -34,6 +35,7 @@ class LifecycleWatcherTest { val ownerMock = mock() val hub = mock() val dateProvider = mock() + val options = SentryOptions() fun getSUT( sessionIntervalMillis: Long = 0L, @@ -47,6 +49,7 @@ class LifecycleWatcherTest { whenever(hub.configureScope(argumentCaptor.capture())).thenAnswer { argumentCaptor.value.run(scope) } + whenever(hub.options).thenReturn(options) return LifecycleWatcher( hub, diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt index b543ae318a..a2d27bb4b5 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt @@ -342,6 +342,7 @@ class SentryAndroidTest { options.release = "prod" options.dsn = "https://key@sentry.io/123" options.isEnableAutoSessionTracking = true + options.experimental.replayOptions.errorSampleRate = 1.0 } var session: Session? = null diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt index f93d35d648..257ab8e047 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt @@ -55,8 +55,11 @@ class ReplayIntegration( private val currentReplayId = AtomicReference(SentryId.EMPTY_ID) private val segmentTimestamp = AtomicReference() private val currentSegment = AtomicInteger(0) - private val saver = + + // TODO: surround with try-catch on the calling site + private val saver by lazy { Executors.newSingleThreadScheduledExecutor(ReplayExecutorServiceThreadFactory()) + } private val recorderConfig by lazy(NONE) { ScreenshotRecorderConfig.from( @@ -103,10 +106,6 @@ class ReplayIntegration( override fun start() { // TODO: add lifecycle state instead and manage it in start/pause/resume/stop if (!isEnabled.get()) { - options.logger.log( - DEBUG, - "Session replay is disabled due to conditions not met in Integration.register" - ) return } @@ -134,12 +133,20 @@ class ReplayIntegration( } override fun resume() { + if (!isEnabled.get()) { + return + } + // TODO: replace it with dateProvider.currentTimeMillis to also test it segmentTimestamp.set(DateUtils.getCurrentDateTime()) recorder?.resume() } override fun sendReplayForEvent(event: SentryEvent, hint: Hint) { + if (!isEnabled.get()) { + return + } + if (isFullSession.get()) { options.logger.log(DEBUG, "Replay is already running in 'session' mode, not capturing for event %s", event.eventId) return @@ -183,6 +190,10 @@ class ReplayIntegration( } override fun pause() { + if (!isEnabled.get()) { + return + } + val now = dateProvider.currentTimeMillis recorder?.pause() @@ -205,10 +216,6 @@ class ReplayIntegration( override fun stop() { if (!isEnabled.get()) { - options.logger.log( - DEBUG, - "Session replay is disabled due to conditions not met in Integration.register" - ) return } @@ -346,6 +353,10 @@ class ReplayIntegration( } override fun close() { + if (!isEnabled.get()) { + return + } + stop() saver.gracefullyShutdown(options) } diff --git a/sentry/src/test/java/io/sentry/BaggageTest.kt b/sentry/src/test/java/io/sentry/BaggageTest.kt index eb1cfa0383..c24731e92a 100644 --- a/sentry/src/test/java/io/sentry/BaggageTest.kt +++ b/sentry/src/test/java/io/sentry/BaggageTest.kt @@ -527,15 +527,13 @@ class BaggageTest { @Test fun `unknown returns sentry- prefixed keys that are not known and passes them on to TraceContext`() { - val baggage = Baggage.fromHeader(listOf("sentry-trace_id=${SentryId()},sentry-public_key=b, sentry-replay_id=def", "sentry-transaction=sentryTransaction, sentry-anewkey=abc")) + val baggage = Baggage.fromHeader(listOf("sentry-trace_id=${SentryId()},sentry-public_key=b, sentry-replay_id=${SentryId()}", "sentry-transaction=sentryTransaction, sentry-anewkey=abc")) val unknown = baggage.unknown - assertEquals(2, unknown.size) - assertEquals("def", unknown["replay_id"]) + assertEquals(1, unknown.size) assertEquals("abc", unknown["anewkey"]) val traceContext = baggage.toTraceContext()!! - assertEquals(2, traceContext.unknown!!.size) - assertEquals("def", traceContext.unknown!!["replay_id"]) + assertEquals(1, traceContext.unknown!!.size) assertEquals("abc", traceContext.unknown!!["anewkey"]) } diff --git a/sentry/src/test/resources/json/sentry_envelope_header.json b/sentry/src/test/resources/json/sentry_envelope_header.json index 14c144f820..5f6b3b25e7 100644 --- a/sentry/src/test/resources/json/sentry_envelope_header.json +++ b/sentry/src/test/resources/json/sentry_envelope_header.json @@ -27,7 +27,8 @@ "user_segment": "f7d8662b-5551-4ef8-b6a8-090f0561a530", "transaction": "0252ec25-cd0a-4230-bd2f-936a4585637e", "sample_rate": "0.00000021", - "sampled": "true" + "sampled": "true", + "replay_id": "3367f5196c494acaae85bbbd535379aa" }, "sent_at": "2020-02-07T14:16:00.000Z" }