diff --git a/CHANGELOG.md b/CHANGELOG.md index c21a8cce7d7..1bbbede5794 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Fixes + +- Session Replay: Populate `trace_ids` in replay events to enable searching replays by trace ID ([#5473](https://github.com/getsentry/sentry-java/pull/5473)) + ## 8.43.0 ### Features diff --git a/sentry-android-replay/api/sentry-android-replay.api b/sentry-android-replay/api/sentry-android-replay.api index aeabe9c05c1..12fe214176d 100644 --- a/sentry-android-replay/api/sentry-android-replay.api +++ b/sentry-android-replay/api/sentry-android-replay.api @@ -76,6 +76,7 @@ public final class io/sentry/android/replay/ReplayIntegration : io/sentry/IConne public fun onWindowSizeChanged (II)V public fun pause ()V public fun register (Lio/sentry/IScopes;Lio/sentry/SentryOptions;)V + public fun registerTraceId (Lio/sentry/protocol/SentryId;)V public fun resume ()V public fun setBreadcrumbConverter (Lio/sentry/ReplayBreadcrumbConverter;)V public fun start ()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 07e91d76486..116ab45af06 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 @@ -280,6 +280,13 @@ public class ReplayIntegration( override fun isDebugMaskingOverlayEnabled(): Boolean = debugMaskingEnabled + override fun registerTraceId(traceId: SentryId) { + if (!isEnabled.get() || !isRecording()) { + return + } + captureStrategy?.registerTraceId(traceId) + } + private fun pauseInternal() { lifecycleLock.acquire().use { if (!isEnabled.get() || !lifecycle.isAllowed(PAUSED)) { diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt index 2277f6c33a1..dab98ec4e24 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt @@ -54,6 +54,8 @@ internal abstract class BaseCaptureStrategy( ) : CaptureStrategy { internal companion object { private const val TAG = "CaptureStrategy" + // https://github.com/getsentry/sentry-javascript/blob/30eb68fff5077211c30c61ba74625e66ab514870/packages/replay-internal/src/coreHandlers/handleAfterSendEvent.ts#L41 + private const val MAX_TRACE_IDS = 100 } private val persistingExecutor: ScheduledExecutorService by lazy { @@ -96,6 +98,8 @@ internal abstract class BaseCaptureStrategy( override var replayType by persistableAtomic(propertyName = SEGMENT_KEY_REPLAY_TYPE) protected val currentEvents: Deque = ConcurrentLinkedDeque() + private val traceIdsLock = Any() + private val currentTraceIds: MutableList = mutableListOf() override fun start(segmentId: Int, replayId: SentryId, replayType: ReplayType?) { cache = replayCacheProvider?.invoke(replayId) ?: ReplayCache(options, replayId) @@ -135,8 +139,14 @@ internal abstract class BaseCaptureStrategy( screenAtStart: String? = this.screenAtStart, breadcrumbs: List? = null, events: Deque = this.currentEvents, - ): ReplaySegment = - createSegment( + ): ReplaySegment { + val traceIds = + synchronized(traceIdsLock) { + val ids = currentTraceIds.toList() + currentTraceIds.clear() + ids + } + return createSegment( scopes, options, duration, @@ -152,7 +162,9 @@ internal abstract class BaseCaptureStrategy( screenAtStart, breadcrumbs, events, + traceIds, ) + } override fun onConfigurationChanged(recorderConfig: ScreenshotRecorderConfig) { this.recorderConfig = recorderConfig @@ -167,6 +179,19 @@ internal abstract class BaseCaptureStrategy( } } + override fun registerTraceId(traceId: SentryId) { + if (traceId != SentryId.EMPTY_ID) { + synchronized(traceIdsLock) { + if (currentTraceIds.size < MAX_TRACE_IDS) { + val id = traceId.toString() + if (!currentTraceIds.contains(id)) { + currentTraceIds.add(id) + } + } + } + } + } + private class ReplayPersistingExecutorServiceThreadFactory : ThreadFactory { private var cnt = 0 diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/CaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/CaptureStrategy.kt index 8e078161c15..6dc391a15ec 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/CaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/CaptureStrategy.kt @@ -53,6 +53,8 @@ internal interface CaptureStrategy { fun convert(): CaptureStrategy + fun registerTraceId(traceId: SentryId) + companion object { private fun Breadcrumb?.isNetworkAvailable(): Boolean = this != null && @@ -84,6 +86,7 @@ internal interface CaptureStrategy { screenAtStart: String?, breadcrumbs: List?, events: Deque, + traceIds: List = emptyList(), ): ReplaySegment { val generatedVideo = cache?.createVideoOf( @@ -122,6 +125,7 @@ internal interface CaptureStrategy { screenAtStart, replayBreadcrumbs, events, + traceIds, ) } @@ -141,6 +145,7 @@ internal interface CaptureStrategy { screenAtStart: String?, breadcrumbs: List, events: Deque, + traceIds: List, ): ReplaySegment { val endTimestamp = DateUtils.getDateTime(segmentTimestamp.time + videoDuration) val replay = @@ -152,6 +157,7 @@ internal interface CaptureStrategy { this.replayStartTimestamp = segmentTimestamp this.replayType = replayType this.videoFile = video + this.traceIds = traceIds } val recordingPayload = mutableListOf() diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt index 4183fad10ed..3df0c9f005f 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt @@ -1072,6 +1072,38 @@ class ReplayIntegrationTest { verify(fixture.replayCache).addFrame(any(), any(), anyOrNull()) } + @Test + fun `registerTraceId does nothing when replay is not started`() { + val replay = fixture.getSut(context) + + replay.register(fixture.scopes, fixture.options) + // Don't call start() + + // Should not throw + replay.registerTraceId(SentryId()) + } + + @Test + fun `registerTraceId forwards to capture strategy when recording`() { + var traceIdRegistered: SentryId? = null + val captureStrategy = + mock { + on { currentReplayId }.thenReturn(SentryId()) + doAnswer { traceIdRegistered = it.arguments[0] as SentryId } + .whenever(mock) + .registerTraceId(any()) + } + val replay = fixture.getSut(context, replayCaptureStrategyProvider = { captureStrategy }) + + replay.register(fixture.scopes, fixture.options) + replay.start() + + val traceId = SentryId() + replay.registerTraceId(traceId) + + assertEquals(traceId, traceIdRegistered) + } + private fun getSessionCaptureStrategy(options: SentryOptions): SessionCaptureStrategy = SessionCaptureStrategy( options, diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt index 9982c6623b2..b5a00bc624b 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt @@ -475,4 +475,83 @@ class SessionCaptureStrategyTest { }, ) } + + @Test + fun `registerTraceId includes trace IDs in next segment`() { + val now = + System.currentTimeMillis() + (fixture.options.sessionReplay.sessionSegmentDuration * 5) + val strategy = fixture.getSut(dateProvider = { now }) + strategy.start() + strategy.onConfigurationChanged(fixture.recorderConfig) + + val traceId1 = SentryId() + val traceId2 = SentryId() + strategy.registerTraceId(traceId1) + strategy.registerTraceId(traceId2) + + strategy.onScreenshotRecorded(mock()) {} + + verify(fixture.scopes) + .captureReplay( + argThat { event -> + event is SentryReplayEvent && + event.traceIds?.size == 2 && + event.traceIds!!.contains(traceId1.toString()) && + event.traceIds!!.contains(traceId2.toString()) + }, + any(), + ) + } + + @Test + fun `registerTraceId clears trace IDs after segment is created`() { + val now = + System.currentTimeMillis() + (fixture.options.sessionReplay.sessionSegmentDuration * 5) + val strategy = fixture.getSut(dateProvider = { now }) + strategy.start() + strategy.onConfigurationChanged(fixture.recorderConfig) + + val traceId = SentryId() + strategy.registerTraceId(traceId) + + strategy.onScreenshotRecorded(mock()) {} + + verify(fixture.scopes) + .captureReplay( + argThat { event -> + event is SentryReplayEvent && event.traceIds?.contains(traceId.toString()) == true + }, + any(), + ) + + // trigger another segment, trace IDs should be cleared + strategy.onScreenshotRecorded(mock()) {} + + verify(fixture.scopes) + .captureReplay( + argThat { event -> + event is SentryReplayEvent && event.segmentId == 1 && event.traceIds.isNullOrEmpty() + }, + any(), + ) + } + + @Test + fun `registerTraceId ignores empty trace ID`() { + val now = + System.currentTimeMillis() + (fixture.options.sessionReplay.sessionSegmentDuration * 5) + val strategy = fixture.getSut(dateProvider = { now }) + strategy.start() + strategy.onConfigurationChanged(fixture.recorderConfig) + + strategy.registerTraceId(SentryId.EMPTY_ID) + + strategy.onScreenshotRecorded(mock()) {} + + verify(fixture.scopes) + .captureReplay( + argThat { event -> event is SentryReplayEvent && event.traceIds.isNullOrEmpty() }, + any(), + ) + } } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index cb03d8fe708..4757be4894a 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -1703,6 +1703,7 @@ public final class io/sentry/NoOpReplayController : io/sentry/ReplayController { public fun isDebugMaskingOverlayEnabled ()Z public fun isRecording ()Z public fun pause ()V + public fun registerTraceId (Lio/sentry/protocol/SentryId;)V public fun resume ()V public fun setBreadcrumbConverter (Lio/sentry/ReplayBreadcrumbConverter;)V public fun start ()V @@ -2344,6 +2345,7 @@ public abstract interface class io/sentry/ReplayController : io/sentry/IReplayAp public abstract fun isDebugMaskingOverlayEnabled ()Z public abstract fun isRecording ()Z public abstract fun pause ()V + public abstract fun registerTraceId (Lio/sentry/protocol/SentryId;)V public abstract fun resume ()V public abstract fun setBreadcrumbConverter (Lio/sentry/ReplayBreadcrumbConverter;)V public abstract fun start ()V diff --git a/sentry/src/main/java/io/sentry/NoOpReplayController.java b/sentry/src/main/java/io/sentry/NoOpReplayController.java index fec95b5d66d..2f6de9740d2 100644 --- a/sentry/src/main/java/io/sentry/NoOpReplayController.java +++ b/sentry/src/main/java/io/sentry/NoOpReplayController.java @@ -57,4 +57,7 @@ public void enableDebugMaskingOverlay() {} @Override public void disableDebugMaskingOverlay() {} + + @Override + public void registerTraceId(@NotNull SentryId traceId) {} } diff --git a/sentry/src/main/java/io/sentry/ReplayController.java b/sentry/src/main/java/io/sentry/ReplayController.java index dd40bfc9732..f4baba40c9d 100644 --- a/sentry/src/main/java/io/sentry/ReplayController.java +++ b/sentry/src/main/java/io/sentry/ReplayController.java @@ -28,4 +28,12 @@ public interface ReplayController extends IReplayApi { ReplayBreadcrumbConverter getBreadcrumbConverter(); boolean isDebugMaskingOverlayEnabled(); + + /** + * Registers a trace ID to be associated with the current replay. This is called when a + * transaction is captured while replay is recording, to enable searching for replays by trace ID. + * + * @param traceId the trace ID to associate with the current replay + */ + void registerTraceId(@NotNull SentryId traceId); } diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 6f328d0fd58..5ac81c44936 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -1043,6 +1043,13 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint sentryId = SentryId.EMPTY_ID; } + if (!sentryId.equals(SentryId.EMPTY_ID)) { + final @Nullable SpanContext trace = transaction.getContexts().getTrace(); + if (trace != null) { + options.getReplayController().registerTraceId(trace.getTraceId()); + } + } + return sentryId; } diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index 663b1f9bdee..d5b2f0f82a0 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -1958,6 +1958,23 @@ class SentryClientTest { assertEquals("abc", transaction.platform) } + @Test + fun `captureTransaction registers trace ID with replay controller`() { + var registeredTraceId: SentryId? = null + fixture.sentryOptions.setReplayController( + object : ReplayController by NoOpReplayController.getInstance() { + override fun registerTraceId(traceId: SentryId) { + registeredTraceId = traceId + } + } + ) + val sut = fixture.getSut() + val sentryTracer = SentryTracer(TransactionContext("name", "op"), fixture.scopes) + val transaction = SentryTransaction(sentryTracer) + sut.captureTransaction(transaction, sentryTracer.traceContext()) + assertEquals(sentryTracer.spanContext.traceId, registeredTraceId) + } + @Test fun `when exception type is ignored, capturing event does not send it`() { fixture.sentryOptions.addIgnoredExceptionForType(IllegalStateException::class.java)