Skip to content

Commit 3d5dac9

Browse files
ambushworkambushwork
committed
RUM-16113: Prevent stale views from overwriting last_view_event
1 parent 69bfb31 commit 3d5dac9

2 files changed

Lines changed: 162 additions & 1 deletion

File tree

‎features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/RumDataWriter.kt‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ internal class RumDataWriter(
3636
private val sdkCore: InternalSdkCore
3737
) : DataWriter<Any> {
3838

39+
private var lastPersistedViewId: String? = null
40+
41+
private val seenViewIds = LinkedHashSet<String>()
42+
3943
// region DataWriter
4044

4145
@WorkerThread
@@ -83,7 +87,22 @@ internal class RumDataWriter(
8387
@WorkerThread
8488
internal fun onDataWritten(data: Any, rawData: ByteArray) {
8589
when (data) {
86-
is ViewEvent -> sdkCore.writeLastViewEvent(rawData)
90+
is ViewEvent -> onViewEventWritten(data, rawData)
91+
}
92+
}
93+
94+
@WorkerThread
95+
private fun onViewEventWritten(data: ViewEvent, rawData: ByteArray) {
96+
val viewId = data.view.id
97+
val isPersistedView = viewId == lastPersistedViewId
98+
val isNewView = seenViewIds.add(viewId)
99+
while (seenViewIds.size > MAX_TRACKED_VIEW_IDS) {
100+
val oldest = seenViewIds.firstOrNull() ?: break
101+
seenViewIds.remove(oldest)
102+
}
103+
if (isPersistedView || isNewView) {
104+
sdkCore.writeLastViewEvent(rawData)
105+
lastPersistedViewId = viewId
87106
}
88107
}
89108

@@ -92,6 +111,8 @@ internal class RumDataWriter(
92111
companion object {
93112
val EMPTY_BYTE_ARRAY = ByteArray(0)
94113

114+
internal const val MAX_TRACKED_VIEW_IDS = 16
115+
95116
private const val UNKNOWN_EVENT_TYPE = "unknown"
96117

97118
private fun resolveEventType(event: Any): String = when (event) {

‎features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/domain/RumDataWriterTest.kt‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import org.mockito.kotlin.anyOrNull
4646
import org.mockito.kotlin.argumentCaptor
4747
import org.mockito.kotlin.doReturn
4848
import org.mockito.kotlin.doThrow
49+
import org.mockito.kotlin.never
4950
import org.mockito.kotlin.times
5051
import org.mockito.kotlin.verify
5152
import org.mockito.kotlin.verifyNoInteractions
@@ -344,6 +345,142 @@ internal class RumDataWriterTest {
344345
verifyNoInteractions(mockInternalLogger)
345346
}
346347

348+
@Test
349+
fun `M call writeLastViewEvent W onDataWritten() { ViewEvent of a new view }`(
350+
@Forgery viewEvent: ViewEvent
351+
) {
352+
// Given - view A is persisted
353+
writeViewEvent(viewEvent, VIEW_A_ID, isActive = true)
354+
355+
// When - view B starts and emits its first event
356+
val fakeViewBData = writeViewEvent(viewEvent, VIEW_B_ID, isActive = true)
357+
358+
// Then - the new view takes over the persisted snapshot
359+
verify(rumMonitor.mockSdkCore).writeLastViewEvent(fakeViewBData)
360+
}
361+
362+
@Test
363+
fun `M call writeLastViewEvent W onDataWritten() { ViewEvent, update of the persisted view }`(
364+
@Forgery viewEvent: ViewEvent
365+
) {
366+
// Given - view A is persisted
367+
writeViewEvent(viewEvent, VIEW_A_ID, isActive = true)
368+
369+
// When - the same view completes (stopped, no newer view)
370+
val fakeCompletedViewAData = writeViewEvent(viewEvent, VIEW_A_ID, isActive = false, marker = "complete")
371+
372+
// Then - the snapshot is refreshed in place with the final event
373+
verify(rumMonitor.mockSdkCore).writeLastViewEvent(fakeCompletedViewAData)
374+
}
375+
376+
@Test
377+
fun `M NOT call writeLastViewEvent W onDataWritten() { ViewEvent, stale view still marked active }`(
378+
@Forgery viewEvent: ViewEvent
379+
) {
380+
// Given - view A was stopped while resources were still pending, then view B started
381+
writeViewEvent(viewEvent, VIEW_A_ID, isActive = true)
382+
writeViewEvent(viewEvent, VIEW_B_ID, isActive = true)
383+
384+
// When - a pending resource of view A completes: view A is not complete yet, so it still
385+
// emits an event with isActive = true
386+
val fakeStaleViewAData = writeViewEvent(viewEvent, VIEW_A_ID, isActive = true, marker = "stale")
387+
388+
// Then - the stale view does not overwrite the snapshot of view B
389+
verify(rumMonitor.mockSdkCore, never()).writeLastViewEvent(fakeStaleViewAData)
390+
}
391+
392+
@Test
393+
fun `M NOT call writeLastViewEvent W onDataWritten() { ViewEvent, stale view completing }`(
394+
@Forgery viewEvent: ViewEvent
395+
) {
396+
// Given - view A was stopped while resources were still pending, view B started, then one
397+
// of view A's pending resources completed
398+
writeViewEvent(viewEvent, VIEW_A_ID, isActive = true)
399+
writeViewEvent(viewEvent, VIEW_B_ID, isActive = true)
400+
writeViewEvent(viewEvent, VIEW_A_ID, isActive = true, marker = "stale")
401+
402+
// When - the last pending event of view A completes it
403+
val fakeCompletedViewAData = writeViewEvent(viewEvent, VIEW_A_ID, isActive = false, marker = "complete")
404+
405+
// Then - the completion of the stale view does not overwrite the snapshot of view B
406+
verify(rumMonitor.mockSdkCore, never()).writeLastViewEvent(fakeCompletedViewAData)
407+
}
408+
409+
@Test
410+
fun `M NOT call writeLastViewEvent W onDataWritten() { ViewEvent, stale view, no active view }`(
411+
@Forgery viewEvent: ViewEvent
412+
) {
413+
// Given - view A was stopped while resources were still pending, view B started and then
414+
// completed too, so no view is active anymore
415+
writeViewEvent(viewEvent, VIEW_A_ID, isActive = true)
416+
writeViewEvent(viewEvent, VIEW_B_ID, isActive = true)
417+
val fakeCompletedViewBData = writeViewEvent(viewEvent, VIEW_B_ID, isActive = false, marker = "complete")
418+
419+
// When - a pending event of view A completes it, after the last view completed
420+
val fakeCompletedViewAData = writeViewEvent(viewEvent, VIEW_A_ID, isActive = false, marker = "complete")
421+
422+
// Then - view A does not overwrite the snapshot of the last view
423+
verify(rumMonitor.mockSdkCore).writeLastViewEvent(fakeCompletedViewBData)
424+
verify(rumMonitor.mockSdkCore, never()).writeLastViewEvent(fakeCompletedViewAData)
425+
}
426+
427+
@Test
428+
fun `M keep the newest view persisted W onDataWritten() { ViewEvent, interleaved views }`(
429+
@Forgery viewEvent: ViewEvent
430+
) {
431+
// When
432+
writeViewEvent(viewEvent, VIEW_A_ID, isActive = true)
433+
writeViewEvent(viewEvent, VIEW_B_ID, isActive = true)
434+
val fakeStaleViewAData = writeViewEvent(viewEvent, VIEW_A_ID, isActive = true, marker = "stale")
435+
val fakeViewBUpdateData = writeViewEvent(viewEvent, VIEW_B_ID, isActive = false, marker = "complete")
436+
437+
// Then
438+
verify(rumMonitor.mockSdkCore, never()).writeLastViewEvent(fakeStaleViewAData)
439+
argumentCaptor<ByteArray> {
440+
verify(rumMonitor.mockSdkCore, times(3)).writeLastViewEvent(capture())
441+
assertThat(lastValue).isEqualTo(fakeViewBUpdateData)
442+
}
443+
}
444+
445+
@Test
446+
fun `M call writeLastViewEvent W onDataWritten() { ViewEvent, stale view beyond tracking limit }`(
447+
@Forgery viewEvent: ViewEvent
448+
) {
449+
// Given - view A is persisted, then more views than the writer can track are created
450+
writeViewEvent(viewEvent, VIEW_A_ID, isActive = true)
451+
repeat(RumDataWriter.MAX_TRACKED_VIEW_IDS) {
452+
writeViewEvent(viewEvent, "view-$it", isActive = true)
453+
}
454+
455+
// When - view A emits a late update, its id is not tracked anymore
456+
val fakeStaleViewAData = writeViewEvent(viewEvent, VIEW_A_ID, isActive = true, marker = "stale")
457+
458+
// Then - it falls back to the legacy behaviour and overwrites the snapshot
459+
verify(rumMonitor.mockSdkCore).writeLastViewEvent(fakeStaleViewAData)
460+
}
461+
462+
// endregion
463+
464+
// region Internal
465+
466+
/**
467+
* Notifies the writer that a view event was written for the given view, and returns the
468+
* serialized data used for that event.
469+
*/
470+
private fun writeViewEvent(
471+
viewEvent: ViewEvent,
472+
viewId: String,
473+
isActive: Boolean,
474+
marker: String = "update"
475+
): ByteArray {
476+
val serializedData = "$viewId-$marker".toByteArray(Charsets.UTF_8)
477+
testedWriter.onDataWritten(
478+
viewEvent.copy(view = viewEvent.view.copy(id = viewId, isActive = isActive)),
479+
serializedData
480+
)
481+
return serializedData
482+
}
483+
347484
// endregion
348485

349486
// region accessibility
@@ -401,6 +538,9 @@ internal class RumDataWriterTest {
401538
// endregion
402539

403540
companion object {
541+
private const val VIEW_A_ID = "view-a"
542+
private const val VIEW_B_ID = "view-b"
543+
404544
val rumMonitor = GlobalRumMonitorTestConfiguration()
405545

406546
@TestConfigurationsProvider

0 commit comments

Comments
 (0)