Skip to content

Commit ef654e8

Browse files
ambushworkambushwork
authored
Merge pull request #2611 from DataDog/yl/replay-investigation
RUM-7924: Fix Session Replay is not resumed after the session has expired before
2 parents 28504a1 + d9fcc99 commit ef654e8

File tree

2 files changed

+220
-24
lines changed

2 files changed

+220
-24
lines changed

‎features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/SessionReplayFeature.kt

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,15 @@ internal class SessionReplayFeature(
9999
private lateinit var appContext: Context
100100

101101
// should we record the session - a combination of rum sampling, sr sampling
102-
// and sr stop/start state
103-
private val shouldRecord = AtomicBoolean(startRecordingImmediately)
102+
// and user option.
103+
private val shouldRecord = AtomicBoolean(false)
104104

105-
// used to monitor changes to an active session due to manual stop/start
106-
private val recordingStateChanged = AtomicBoolean(false)
105+
// Indicates the user's intend on recording, it starts with `startRecordingImmediately`
106+
// in configuration, can be changed by calling start/stop recordings API.
107+
private val userIntentToRecord = AtomicBoolean(startRecordingImmediately)
108+
109+
// used to monitor changes to user's intend on recording state
110+
private val userIntentToRecordChanged = AtomicBoolean(false)
107111

108112
// are we recording at the moment
109113
private val isRecording = AtomicBoolean(false)
@@ -199,14 +203,14 @@ internal class SessionReplayFeature(
199203
// region Manual Recording
200204

201205
internal fun manuallyStopRecording() {
202-
if (shouldRecord.compareAndSet(true, false)) {
203-
recordingStateChanged.set(true)
206+
if (userIntentToRecord.compareAndSet(true, false)) {
207+
userIntentToRecordChanged.set(true)
204208
}
205209
}
206210

207211
internal fun manuallyStartRecording() {
208-
if (shouldRecord.compareAndSet(false, true)) {
209-
recordingStateChanged.set(true)
212+
if (userIntentToRecord.compareAndSet(false, true)) {
213+
userIntentToRecordChanged.set(true)
210214
}
211215
}
212216

@@ -259,7 +263,7 @@ internal class SessionReplayFeature(
259263
}
260264

261265
private fun shouldHandleSession(alreadySeenSession: Boolean): Boolean {
262-
return !alreadySeenSession || recordingStateChanged.get()
266+
return !alreadySeenSession || userIntentToRecordChanged.get()
263267
}
264268

265269
private fun applySampling(alreadySeenSession: Boolean) {
@@ -269,9 +273,14 @@ internal class SessionReplayFeature(
269273
}
270274

271275
private fun modifyShouldRecordState(sessionData: SessionData) {
272-
val isSampledIn = sessionData.keepSession && isSessionSampledIn.get()
273-
if (!isSampledIn) {
274-
if (shouldRecord.compareAndSet(true, false)) {
276+
val isSessionEligible = sessionData.keepSession && isSessionSampledIn.get()
277+
if (isSessionEligible) {
278+
shouldRecord.set(userIntentToRecord.get())
279+
} else {
280+
shouldRecord.set(false)
281+
if (!sessionData.keepSession) {
282+
logNotKeptMessage()
283+
} else {
275284
logSampledOutMessage()
276285
}
277286
}
@@ -280,19 +289,27 @@ internal class SessionReplayFeature(
280289
private fun logMissingApplicationContextError() {
281290
sdkCore.internalLogger.log(
282291
InternalLogger.Level.WARN,
283-
InternalLogger.Target.USER,
292+
InternalLogger.Target.MAINTAINER,
284293
{ REQUIRES_APPLICATION_CONTEXT_WARN_MESSAGE }
285294
)
286295
}
287296

288297
private fun logEventMissingMandatoryFieldsError() {
289298
sdkCore.internalLogger.log(
290299
InternalLogger.Level.WARN,
291-
InternalLogger.Target.USER,
300+
InternalLogger.Target.MAINTAINER,
292301
{ EVENT_MISSING_MANDATORY_FIELDS }
293302
)
294303
}
295304

305+
private fun logNotKeptMessage() {
306+
sdkCore.internalLogger.log(
307+
InternalLogger.Level.INFO,
308+
InternalLogger.Target.USER,
309+
{ SESSION_NOT_KEPT_MESSAGE }
310+
)
311+
}
312+
296313
private fun logSampledOutMessage() {
297314
sdkCore.internalLogger.log(
298315
InternalLogger.Level.INFO,
@@ -316,7 +333,7 @@ internal class SessionReplayFeature(
316333
stopRecording()
317334
}
318335

319-
recordingStateChanged.set(false)
336+
userIntentToRecordChanged.set(false)
320337
currentRumSessionId.set(sessionData.sessionId)
321338
}
322339

@@ -393,6 +410,8 @@ internal class SessionReplayFeature(
393410
"be initialized without the Application context."
394411
internal const val SESSION_SAMPLED_OUT_MESSAGE = "This session was sampled out from" +
395412
" recording. No replay will be provided for it."
413+
internal const val SESSION_NOT_KEPT_MESSAGE =
414+
"This session was not kept. No replay will be provided for it."
396415
internal const val UNSUPPORTED_EVENT_TYPE =
397416
"Session Replay feature receive an event of unsupported type=%s."
398417
internal const val UNKNOWN_EVENT_TYPE_PROPERTY_VALUE =

‎features/dd-sdk-android-session-replay/src/test/kotlin/com/datadog/android/sessionreplay/internal/SessionReplayFeatureTest.kt

Lines changed: 186 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ internal class SessionReplayFeatureTest {
444444
// Then
445445
mockInternalLogger.verifyLog(
446446
InternalLogger.Level.WARN,
447-
InternalLogger.Target.USER,
447+
InternalLogger.Target.MAINTAINER,
448448
SessionReplayFeature.REQUIRES_APPLICATION_CONTEXT_WARN_MESSAGE
449449
)
450450
verifyNoInteractions(mockRecorder)
@@ -519,7 +519,7 @@ internal class SessionReplayFeatureTest {
519519
mockInternalLogger.verifyLog(
520520
InternalLogger.Level.INFO,
521521
InternalLogger.Target.USER,
522-
SessionReplayFeature.SESSION_SAMPLED_OUT_MESSAGE
522+
SessionReplayFeature.SESSION_NOT_KEPT_MESSAGE
523523
)
524524
}
525525

@@ -621,7 +621,7 @@ internal class SessionReplayFeatureTest {
621621
mockInternalLogger.verifyLog(
622622
InternalLogger.Level.INFO,
623623
InternalLogger.Target.USER,
624-
SessionReplayFeature.SESSION_SAMPLED_OUT_MESSAGE
624+
SessionReplayFeature.SESSION_NOT_KEPT_MESSAGE
625625
)
626626
}
627627

@@ -701,7 +701,7 @@ internal class SessionReplayFeatureTest {
701701
mockInternalLogger.verifyLog(
702702
InternalLogger.Level.INFO,
703703
InternalLogger.Target.USER,
704-
SessionReplayFeature.SESSION_SAMPLED_OUT_MESSAGE
704+
SessionReplayFeature.SESSION_NOT_KEPT_MESSAGE
705705
)
706706
}
707707

@@ -801,6 +801,126 @@ internal class SessionReplayFeatureTest {
801801
verifyNoInteractions(mockRecorder)
802802
}
803803

804+
@Test
805+
fun `M not resume recording in new session W user call stopRecording {startRecordingImmediately = true}`(
806+
@Forgery fakeUUID2: UUID
807+
) {
808+
// Given
809+
val fakeSessionId2 = fakeUUID2.toString()
810+
whenever(mockSampler.sample(any())).thenReturn(true)
811+
testedFeature = SessionReplayFeature(
812+
sdkCore = mockSdkCore,
813+
customEndpointUrl = fakeConfiguration.customEndpointUrl,
814+
privacy = fakeConfiguration.privacy,
815+
textAndInputPrivacy = fakeConfiguration.textAndInputPrivacy,
816+
imagePrivacy = fakeConfiguration.imagePrivacy,
817+
touchPrivacy = fakeConfiguration.touchPrivacy,
818+
startRecordingImmediately = true,
819+
rateBasedSampler = mockSampler
820+
) { _, _, _, _ -> mockRecorder }
821+
testedFeature.onInitialize(appContext.mockInstance)
822+
val rumSessionUpdateBusMessage1 = mapOf(
823+
SessionReplayFeature.SESSION_REPLAY_BUS_MESSAGE_TYPE_KEY to
824+
SessionReplayFeature.RUM_SESSION_RENEWED_BUS_MESSAGE,
825+
SessionReplayFeature.RUM_KEEP_SESSION_BUS_MESSAGE_KEY to
826+
true,
827+
SessionReplayFeature.RUM_SESSION_ID_BUS_MESSAGE_KEY to
828+
fakeSessionId
829+
)
830+
val rumSessionUpdateBusMessage2 = mapOf(
831+
SessionReplayFeature.SESSION_REPLAY_BUS_MESSAGE_TYPE_KEY to
832+
SessionReplayFeature.RUM_SESSION_RENEWED_BUS_MESSAGE,
833+
SessionReplayFeature.RUM_KEEP_SESSION_BUS_MESSAGE_KEY to
834+
true,
835+
SessionReplayFeature.RUM_SESSION_ID_BUS_MESSAGE_KEY to
836+
fakeSessionId2
837+
)
838+
839+
// When
840+
testedFeature.onReceive(rumSessionUpdateBusMessage1)
841+
testedFeature.manuallyStopRecording()
842+
testedFeature.onReceive(rumSessionUpdateBusMessage2)
843+
844+
// Then
845+
inOrder(mockRecorder) {
846+
verify(mockRecorder).registerCallbacks()
847+
verify(mockRecorder).resumeRecorders()
848+
verify(mockRecorder).stopRecorders()
849+
}
850+
verifyNoMoreInteractions(mockRecorder)
851+
}
852+
853+
@Test
854+
fun `M resume recording in new session W user call startRecording {startRecordingImmediately = false}`(
855+
@Forgery fakeUUID3: UUID,
856+
@Forgery fakeUUID4: UUID
857+
) {
858+
// Given
859+
val fakeSessionId3 = fakeUUID3.toString()
860+
val fakeSessionId4 = fakeUUID4.toString()
861+
whenever(mockSampler.sample(any())).thenReturn(true)
862+
testedFeature = SessionReplayFeature(
863+
sdkCore = mockSdkCore,
864+
customEndpointUrl = fakeConfiguration.customEndpointUrl,
865+
privacy = fakeConfiguration.privacy,
866+
textAndInputPrivacy = fakeConfiguration.textAndInputPrivacy,
867+
imagePrivacy = fakeConfiguration.imagePrivacy,
868+
touchPrivacy = fakeConfiguration.touchPrivacy,
869+
startRecordingImmediately = false,
870+
rateBasedSampler = mockSampler
871+
) { _, _, _, _ -> mockRecorder }
872+
testedFeature.onInitialize(appContext.mockInstance)
873+
val rumSessionUpdateBusMessage1 = mapOf(
874+
SessionReplayFeature.SESSION_REPLAY_BUS_MESSAGE_TYPE_KEY to
875+
SessionReplayFeature.RUM_SESSION_RENEWED_BUS_MESSAGE,
876+
SessionReplayFeature.RUM_KEEP_SESSION_BUS_MESSAGE_KEY to
877+
true,
878+
SessionReplayFeature.RUM_SESSION_ID_BUS_MESSAGE_KEY to
879+
fakeSessionId
880+
)
881+
val rumSessionUpdateBusMessage2 = mapOf(
882+
SessionReplayFeature.SESSION_REPLAY_BUS_MESSAGE_TYPE_KEY to
883+
SessionReplayFeature.RUM_SESSION_RENEWED_BUS_MESSAGE,
884+
SessionReplayFeature.RUM_KEEP_SESSION_BUS_MESSAGE_KEY to
885+
true,
886+
SessionReplayFeature.RUM_SESSION_ID_BUS_MESSAGE_KEY to
887+
fakeSessionId
888+
)
889+
val rumSessionUpdateBusMessage3 = mapOf(
890+
SessionReplayFeature.SESSION_REPLAY_BUS_MESSAGE_TYPE_KEY to
891+
SessionReplayFeature.RUM_SESSION_RENEWED_BUS_MESSAGE,
892+
SessionReplayFeature.RUM_KEEP_SESSION_BUS_MESSAGE_KEY to
893+
false,
894+
SessionReplayFeature.RUM_SESSION_ID_BUS_MESSAGE_KEY to
895+
fakeSessionId3
896+
)
897+
val rumSessionUpdateBusMessage4 = mapOf(
898+
SessionReplayFeature.SESSION_REPLAY_BUS_MESSAGE_TYPE_KEY to
899+
SessionReplayFeature.RUM_SESSION_RENEWED_BUS_MESSAGE,
900+
SessionReplayFeature.RUM_KEEP_SESSION_BUS_MESSAGE_KEY to
901+
true,
902+
SessionReplayFeature.RUM_SESSION_ID_BUS_MESSAGE_KEY to
903+
fakeSessionId4
904+
)
905+
906+
// When
907+
testedFeature.onReceive(rumSessionUpdateBusMessage1)
908+
testedFeature.manuallyStartRecording()
909+
// send an event with same session id to process manual start recording.
910+
testedFeature.onReceive(rumSessionUpdateBusMessage2)
911+
testedFeature.onReceive(rumSessionUpdateBusMessage3)
912+
testedFeature.onReceive(rumSessionUpdateBusMessage4)
913+
914+
// Then
915+
inOrder(mockRecorder) {
916+
verify(mockRecorder).registerCallbacks()
917+
verify(mockRecorder).resumeRecorders()
918+
verify(mockRecorder).stopRecorders()
919+
verify(mockRecorder).resumeRecorders()
920+
}
921+
verifyNoMoreInteractions(mockRecorder)
922+
}
923+
804924
@Test
805925
fun `M start recording W rum session is initialized after first message`() {
806926
// Given
@@ -895,15 +1015,15 @@ internal class SessionReplayFeatureTest {
8951015
// Then
8961016
mockInternalLogger.verifyLog(
8971017
InternalLogger.Level.WARN,
898-
InternalLogger.Target.USER,
1018+
InternalLogger.Target.MAINTAINER,
8991019
SessionReplayFeature.EVENT_MISSING_MANDATORY_FIELDS
9001020
)
9011021

9021022
verify(mockRecorder, never()).resumeRecorders()
9031023
}
9041024

9051025
@Test
906-
fun `M log warning and do nothing W onReceive() { missing keep state field }`(
1026+
fun `M log warning and do nothing W onReceive() { missing keep state field }`(
9071027
@Mock fakeContext: Application
9081028
) {
9091029
// Given
@@ -920,7 +1040,7 @@ internal class SessionReplayFeatureTest {
9201040
// Then
9211041
mockInternalLogger.verifyLog(
9221042
InternalLogger.Level.WARN,
923-
InternalLogger.Target.USER,
1043+
InternalLogger.Target.MAINTAINER,
9241044
SessionReplayFeature.EVENT_MISSING_MANDATORY_FIELDS
9251045
)
9261046

@@ -946,7 +1066,7 @@ internal class SessionReplayFeatureTest {
9461066
// Then
9471067
mockInternalLogger.verifyLog(
9481068
InternalLogger.Level.WARN,
949-
InternalLogger.Target.USER,
1069+
InternalLogger.Target.MAINTAINER,
9501070
SessionReplayFeature.EVENT_MISSING_MANDATORY_FIELDS
9511071
)
9521072

@@ -975,7 +1095,7 @@ internal class SessionReplayFeatureTest {
9751095
// Then
9761096
mockInternalLogger.verifyLog(
9771097
InternalLogger.Level.WARN,
978-
InternalLogger.Target.USER,
1098+
InternalLogger.Target.MAINTAINER,
9791099
SessionReplayFeature.EVENT_MISSING_MANDATORY_FIELDS
9801100
)
9811101

@@ -1146,6 +1266,63 @@ internal class SessionReplayFeatureTest {
11461266
verify(mockRecorder).stopRecorders()
11471267
}
11481268

1269+
@Test
1270+
fun `M resume recordings W keepSession changes from false to true`(
1271+
@Mock fakeContext: Application,
1272+
@Forgery fakeUUID1: UUID,
1273+
@Forgery fakeUUID2: UUID,
1274+
@Forgery fakeUUID3: UUID
1275+
) {
1276+
// Given
1277+
val event1 = mapOf(
1278+
SessionReplayFeature.SESSION_REPLAY_BUS_MESSAGE_TYPE_KEY to
1279+
SessionReplayFeature.RUM_SESSION_RENEWED_BUS_MESSAGE,
1280+
SessionReplayFeature.RUM_KEEP_SESSION_BUS_MESSAGE_KEY to true,
1281+
SessionReplayFeature.RUM_SESSION_ID_BUS_MESSAGE_KEY to fakeUUID1.toString()
1282+
)
1283+
1284+
whenever(mockSampler.sample(any())).thenReturn(true)
1285+
1286+
// When
1287+
testedFeature = SessionReplayFeature(
1288+
sdkCore = mockSdkCore,
1289+
customEndpointUrl = fakeConfiguration.customEndpointUrl,
1290+
privacy = fakeConfiguration.privacy,
1291+
textAndInputPrivacy = fakeConfiguration.textAndInputPrivacy,
1292+
imagePrivacy = fakeConfiguration.imagePrivacy,
1293+
touchPrivacy = fakeConfiguration.touchPrivacy,
1294+
startRecordingImmediately = true,
1295+
rateBasedSampler = mockSampler
1296+
) { _, _, _, _ -> mockRecorder }
1297+
testedFeature.onInitialize(fakeContext)
1298+
testedFeature.onReceive(event1)
1299+
1300+
// When
1301+
val event2 = mapOf(
1302+
SessionReplayFeature.SESSION_REPLAY_BUS_MESSAGE_TYPE_KEY to
1303+
SessionReplayFeature.RUM_SESSION_RENEWED_BUS_MESSAGE,
1304+
SessionReplayFeature.RUM_KEEP_SESSION_BUS_MESSAGE_KEY to false,
1305+
SessionReplayFeature.RUM_SESSION_ID_BUS_MESSAGE_KEY to fakeUUID2.toString()
1306+
)
1307+
testedFeature.onReceive(event2)
1308+
1309+
// When
1310+
val event3 = mapOf(
1311+
SessionReplayFeature.SESSION_REPLAY_BUS_MESSAGE_TYPE_KEY to
1312+
SessionReplayFeature.RUM_SESSION_RENEWED_BUS_MESSAGE,
1313+
SessionReplayFeature.RUM_KEEP_SESSION_BUS_MESSAGE_KEY to true,
1314+
SessionReplayFeature.RUM_SESSION_ID_BUS_MESSAGE_KEY to fakeUUID3.toString()
1315+
)
1316+
testedFeature.onReceive(event3)
1317+
1318+
// Then
1319+
inOrder(mockRecorder) {
1320+
verify(mockRecorder).resumeRecorders()
1321+
verify(mockRecorder).stopRecorders()
1322+
verify(mockRecorder).resumeRecorders()
1323+
}
1324+
}
1325+
11491326
// endregion
11501327

11511328
internal data class SessionRecordingScenario(

0 commit comments

Comments
 (0)