Improve telemetry from RecordedDataQueueHandler

DataDog · May 27, 2024 · d8e62f3 · d8e62f3
1 parent de8989f
commit d8e62f3
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 32 deletions.
diff --git a/features/dd-sdk-android-session-replay/consumer-rules.pro b/features/dd-sdk-android-session-replay/consumer-rules.pro
@@ -6,3 +6,4 @@
 # Kept for our internal telemetry
 -keepnames class com.datadog.android.sessionreplay.internal.recorder.listener.WindowsOnDrawListener
 -keepnames class * extends com.datadog.android.sessionreplay.recorder.mapper.WireframeMapper
+-keepnames class * extends com.datadog.android.sessionreplay.internal.async.RecordedDataQueueItem
diff --git a/.../main/kotlin/com/datadog/android/sessionreplay/internal/async/RecordedDataQueueHandler.kt b/.../main/kotlin/com/datadog/android/sessionreplay/internal/async/RecordedDataQueueHandler.kt
@@ -15,8 +15,6 @@ import com.datadog.android.sessionreplay.internal.processor.RumContextDataHandle
 import com.datadog.android.sessionreplay.internal.utils.TimeProvider
 import com.datadog.android.sessionreplay.model.MobileSegment
 import com.datadog.android.sessionreplay.recorder.SystemInformation
-import java.lang.ClassCastException
-import java.lang.NullPointerException
 import java.util.Locale
 import java.util.Queue
 import java.util.concurrent.ConcurrentLinkedQueue
@@ -129,9 +127,7 @@ internal class RecordedDataQueueHandler : DataQueueHandler {
     }
 
     @MainThread
-    override fun addSnapshotItem(
-        systemInformation: SystemInformation
-    ): SnapshotRecordedDataQueueItem? {
+    override fun addSnapshotItem(systemInformation: SystemInformation): SnapshotRecordedDataQueueItem? {
         val rumContextData = rumContextDataHandler.createRumContextData()
             ?: return null
 
@@ -190,18 +186,25 @@ internal class RecordedDataQueueHandler : DataQueueHandler {
             val nextItem = recordedDataQueue.peek()
 
             if (nextItem != null) {
-                if (shouldRemoveItem(nextItem, currentTime)) {
-                    // this should never happen, so if it does we should send telemetry
+                val nextItemAge = currentTime - nextItem.recordedQueuedItemContext.timestamp
+                if (!nextItem.isValid()) {
                     internalLogger.log(
                         InternalLogger.Level.WARN,
                         listOf(
                             InternalLogger.Target.MAINTAINER,
                             InternalLogger.Target.TELEMETRY
                         ),
-                        {
-                            ITEM_DROPPED_FROM_QUEUE_ERROR_MESSAGE
-                                .format(Locale.US, nextItem.isValid())
-                        }
+                        { ITEM_DROPPED_INVALID_MESSAGE.format(Locale.US, nextItem.javaClass.simpleName) }
+                    )
+                    recordedDataQueue.poll()
+                } else if (nextItemAge > MAX_DELAY_MS) {
+                    internalLogger.log(
+                        InternalLogger.Level.WARN,
+                        listOf(
+                            InternalLogger.Target.MAINTAINER,
+                            InternalLogger.Target.TELEMETRY
+                        ),
+                        { ITEM_DROPPED_EXPIRED_MESSAGE.format(Locale.US, nextItemAge) }
                     )
                     recordedDataQueue.poll()
                 } else if (nextItem.isReady()) {
@@ -242,12 +245,6 @@ internal class RecordedDataQueueHandler : DataQueueHandler {
         processor.processTouchEventsRecords(item)
     }
 
-    private fun shouldRemoveItem(recordedDataQueueItem: RecordedDataQueueItem, currentTime: Long) =
-        !recordedDataQueueItem.isValid() || isTooOld(currentTime, recordedDataQueueItem)
-
-    private fun isTooOld(currentTime: Long, recordedDataQueueItem: RecordedDataQueueItem): Boolean =
-        (currentTime - recordedDataQueueItem.recordedQueuedItemContext.timestamp) > MAX_DELAY_MS
-
     private fun insertIntoRecordedDataQueue(recordedDataQueueItem: RecordedDataQueueItem) {
         @Suppress("SwallowedException", "TooGenericExceptionCaught")
         try {
@@ -293,7 +290,11 @@ internal class RecordedDataQueueHandler : DataQueueHandler {
             "SR RecordedDataQueueHandler: failed to add records into the queue"
 
         @VisibleForTesting
-        internal const val ITEM_DROPPED_FROM_QUEUE_ERROR_MESSAGE =
-            "SR RecordedDataQueueHandler: dropped item from the queue. Valid: %s"
+        internal const val ITEM_DROPPED_INVALID_MESSAGE =
+            "SR RecordedDataQueueHandler: dropped item from the queue. isValid=false, type=%s"
+
+        @VisibleForTesting
+        internal const val ITEM_DROPPED_EXPIRED_MESSAGE =
+            "SR RecordedDataQueueHandler: dropped item from the queue. age=%d ms"
     }
 }
diff --git a/...src/main/kotlin/com/datadog/android/sessionreplay/internal/async/RecordedDataQueueItem.kt b/...src/main/kotlin/com/datadog/android/sessionreplay/internal/async/RecordedDataQueueItem.kt
@@ -12,5 +12,6 @@ internal abstract class RecordedDataQueueItem(
     internal val recordedQueuedItemContext: RecordedQueuedItemContext
 ) {
     internal abstract fun isValid(): Boolean
+
     internal abstract fun isReady(): Boolean
 }
diff --git a/...t/kotlin/com/datadog/android/sessionreplay/internal/async/RecordedDataQueueHandlerTest.kt b/...t/kotlin/com/datadog/android/sessionreplay/internal/async/RecordedDataQueueHandlerTest.kt
@@ -8,7 +8,6 @@ package com.datadog.android.sessionreplay.internal.async
 
 import com.datadog.android.api.InternalLogger
 import com.datadog.android.sessionreplay.forge.ForgeConfigurator
-import com.datadog.android.sessionreplay.internal.async.RecordedDataQueueHandler.Companion.ITEM_DROPPED_FROM_QUEUE_ERROR_MESSAGE
 import com.datadog.android.sessionreplay.internal.async.RecordedDataQueueHandler.Companion.MAX_DELAY_MS
 import com.datadog.android.sessionreplay.internal.processor.RecordedDataProcessor
 import com.datadog.android.sessionreplay.internal.processor.RecordedQueuedItemContext
@@ -48,7 +47,6 @@ import org.mockito.kotlin.verifyNoInteractions
 import org.mockito.kotlin.verifyNoMoreInteractions
 import org.mockito.kotlin.whenever
 import org.mockito.quality.Strictness
-import java.util.Locale
 import java.util.Queue
 import java.util.UUID
 import java.util.concurrent.ConcurrentLinkedQueue
@@ -331,15 +329,13 @@ internal class RecordedDataQueueHandlerTest {
 
         // Then
         assertThat(testedHandler.recordedDataQueue.isEmpty()).isTrue
-        val expectedLogMessage = ITEM_DROPPED_FROM_QUEUE_ERROR_MESSAGE
-            .format(Locale.US, true)
         mockInternalLogger.verifyLog(
             InternalLogger.Level.WARN,
             listOf(
                 InternalLogger.Target.MAINTAINER,
                 InternalLogger.Target.TELEMETRY
             ),
-            expectedLogMessage
+            { it.startsWith("SR RecordedDataQueueHandler: dropped item from the queue. age=") }
         )
         verifyNoMoreInteractions(mockProcessor)
     }
@@ -366,14 +362,14 @@ internal class RecordedDataQueueHandlerTest {
 
         // Then
         assertThat(testedHandler.recordedDataQueue).isEmpty()
-        val expectedLogMessage = ITEM_DROPPED_FROM_QUEUE_ERROR_MESSAGE.format(Locale.US, false)
         mockInternalLogger.verifyLog(
             InternalLogger.Level.WARN,
             listOf(
                 InternalLogger.Target.MAINTAINER,
                 InternalLogger.Target.TELEMETRY
             ),
-            expectedLogMessage
+            "SR RecordedDataQueueHandler: dropped item from the queue. isValid=false, " +
+                "type=SnapshotRecordedDataQueueItem"
         )
         verifyNoMoreInteractions(mockProcessor)
     }
@@ -399,15 +395,14 @@ internal class RecordedDataQueueHandlerTest {
 
         // Then
         assertThat(testedHandler.recordedDataQueue).isEmpty()
-        val expectedLogMessage = ITEM_DROPPED_FROM_QUEUE_ERROR_MESSAGE
-            .format(Locale.US, false)
         mockInternalLogger.verifyLog(
             InternalLogger.Level.WARN,
             listOf(
                 InternalLogger.Target.MAINTAINER,
                 InternalLogger.Target.TELEMETRY
             ),
-            expectedLogMessage
+            "SR RecordedDataQueueHandler: dropped item from the queue. isValid=false, " +
+                "type=TouchEventRecordedDataQueueItem"
         )
         verifyNoMoreInteractions(mockProcessor)
     }
@@ -432,15 +427,14 @@ internal class RecordedDataQueueHandlerTest {
 
         // Then
         assertThat(testedHandler.recordedDataQueue).isEmpty()
-        val expectedLogMessage = ITEM_DROPPED_FROM_QUEUE_ERROR_MESSAGE
-            .format(Locale.US, false)
         mockInternalLogger.verifyLog(
             InternalLogger.Level.WARN,
             listOf(
                 InternalLogger.Target.MAINTAINER,
                 InternalLogger.Target.TELEMETRY
             ),
-            expectedLogMessage
+            "SR RecordedDataQueueHandler: dropped item from the queue. isValid=false, " +
+                "type=ResourceRecordedDataQueueItem"
         )
         verifyNoMoreInteractions(mockProcessor)
     }