Merge pull request #2468 from DataDog/xgouchet/view_loading_time_tele…

…metry Fix internal telemetry on invalid loading time usage
DataDog · Dec 20, 2024 · f61a42c · f61a42c
2 parents 3adfe60 + f6ff29b
commit f61a42c
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 50 deletions.
diff --git a/...-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/scope/RumViewManagerScope.kt b/...-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/scope/RumViewManagerScope.kt
@@ -135,10 +135,13 @@ internal class RumViewManagerScope(
         event: RumRawEvent,
         writer: DataWriter<Any>
     ) {
+        val hasNoView = childrenScopes.isEmpty()
         val iterator = childrenScopes.iterator()
+        var hasActiveView = false
         @Suppress("UnsafeThirdPartyFunctionCall") // next/remove can't fail: we checked hasNext
         while (iterator.hasNext()) {
             val childScope = iterator.next()
+            hasActiveView = hasActiveView or childScope.isActive()
             if (event is RumRawEvent.StopView) {
                 if (childScope.isActive() && (childScope as? RumViewScope)?.key?.id == event.key.id) {
                     lastStoppedViewTime = event.eventTime
@@ -149,6 +152,21 @@ internal class RumViewManagerScope(
                 iterator.remove()
             }
         }
+
+        if (event is RumRawEvent.AddViewLoadingTime && !hasActiveView) {
+            sdkCore.internalLogger.log(
+                InternalLogger.Level.WARN,
+                InternalLogger.Target.USER,
+                { NO_ACTIVE_VIEW_FOR_LOADING_TIME_WARNING_MESSAGE }
+            )
+            sdkCore.internalLogger.logApiUsage {
+                InternalTelemetryEvent.ApiUsage.AddViewLoadingTime(
+                    overwrite = event.overwrite,
+                    noView = hasNoView,
+                    noActiveView = !hasNoView
+                )
+            }
+        }
     }
 
     @WorkerThread
@@ -158,19 +176,11 @@ internal class RumViewManagerScope(
         val isForegroundProcess = processFlag == importanceForeground
 
         if (event is RumRawEvent.AddViewLoadingTime) {
-            val internalLogger = sdkCore.internalLogger
-            internalLogger.log(
+            sdkCore.internalLogger.log(
                 InternalLogger.Level.WARN,
                 InternalLogger.Target.USER,
                 { MESSAGE_MISSING_VIEW }
             )
-            internalLogger.logApiUsage {
-                InternalTelemetryEvent.ApiUsage.AddViewLoadingTime(
-                    overwrite = event.overwrite,
-                    noView = true,
-                    noActiveView = false
-                )
-            }
             // we should return here and not add the event to the session ended metric missed events as we already
             // send the API usage telemetry
             return
@@ -359,6 +369,9 @@ internal class RumViewManagerScope(
         internal const val MESSAGE_UNKNOWN_MISSED_TYPE = "An RUM event was detected, but no view is active, " +
             "its missed type is unknown"
 
+        internal const val NO_ACTIVE_VIEW_FOR_LOADING_TIME_WARNING_MESSAGE =
+            "No active view found to add the loading time."
+
         internal val THREE_SECONDS_GAP_NS = TimeUnit.SECONDS.toNanos(3)
     }
 }
diff --git a/...android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/scope/RumViewScope.kt b/...android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/scope/RumViewScope.kt
@@ -251,33 +251,18 @@ internal open class RumViewScope(
 
     @WorkerThread
     private fun onAddViewLoadingTime(event: RumRawEvent.AddViewLoadingTime, writer: DataWriter<Any>) {
-        val internalLogger = sdkCore.internalLogger
         val canUpdateViewLoadingTime = !stopped && (viewLoadingTime == null || event.overwrite)
-        if (stopped) {
-            internalLogger.log(
-                InternalLogger.Level.WARN,
-                InternalLogger.Target.USER,
-                { NO_ACTIVE_VIEW_FOR_LOADING_TIME_WARNING_MESSAGE }
-            )
-            internalLogger.logApiUsage {
-                InternalTelemetryEvent.ApiUsage.AddViewLoadingTime(
-                    overwrite = event.overwrite,
-                    noView = false,
-                    noActiveView = true
-                )
-            }
-        }
 
         if (canUpdateViewLoadingTime) {
-            updateViewLoadingTime(event, internalLogger, writer)
+            updateViewLoadingTime(event, writer)
         }
     }
 
     private fun updateViewLoadingTime(
         event: RumRawEvent.AddViewLoadingTime,
-        internalLogger: InternalLogger,
         writer: DataWriter<Any>
     ) {
+        val internalLogger = sdkCore.internalLogger
         val viewName = key.name
         val previousViewLoadingTime = viewLoadingTime
         val newLoadingTime = event.eventTime.nanoTime - startedNanos
@@ -1382,8 +1367,6 @@ internal open class RumViewScope(
             "view: %s was 0. In order to keep the view we forced it to 1ns."
         internal const val NEGATIVE_DURATION_WARNING_MESSAGE = "The computed duration for the " +
             "view: %s was negative. In order to keep the view we forced it to 1ns."
-        internal const val NO_ACTIVE_VIEW_FOR_LOADING_TIME_WARNING_MESSAGE =
-            "No active view found to add the loading time."
         internal const val ADDING_VIEW_LOADING_TIME_DEBUG_MESSAGE_FORMAT =
             "View loading time %dns added to the view %s"
         internal const val OVERWRITING_VIEW_LOADING_TIME_WARNING_MESSAGE_FORMAT =

diff --git a/.../src/test/kotlin/com/datadog/android/rum/internal/domain/scope/RumViewManagerScopeTest.kt b/.../src/test/kotlin/com/datadog/android/rum/internal/domain/scope/RumViewManagerScopeTest.kt
@@ -59,7 +59,6 @@ import org.mockito.kotlin.mock
 import org.mockito.kotlin.never
 import org.mockito.kotlin.verify
 import org.mockito.kotlin.verifyNoInteractions
-import org.mockito.kotlin.verifyNoMoreInteractions
 import org.mockito.kotlin.whenever
 import org.mockito.quality.Strictness
 import java.util.concurrent.TimeUnit
@@ -845,7 +844,7 @@ internal class RumViewManagerScopeTest {
     // region AddViewLoadingTime
 
     @Test
-    fun `M send a warning log and api usage telemetry W handleEvent { AddViewLoadingTime, no active view }`(
+    fun `M send a warning log and api usage telemetry W handleEvent { AddViewLoadingTime, no view }`(
         forge: Forge
     ) {
         // Given
@@ -856,11 +855,6 @@ internal class RumViewManagerScopeTest {
         testedScope.handleEvent(fakeEvent, mockWriter)
 
         // Then
-        mockInternalLogger.verifyLog(
-            InternalLogger.Level.WARN,
-            InternalLogger.Target.USER,
-            RumViewManagerScope.MESSAGE_MISSING_VIEW
-        )
         mockInternalLogger.verifyApiUsage(
             InternalTelemetryEvent.ApiUsage.AddViewLoadingTime(
                 overwrite = fakeEvent.overwrite,
@@ -869,8 +863,30 @@ internal class RumViewManagerScopeTest {
             ),
             15f
         )
-        verifyNoMoreInteractions(mockInternalLogger)
-        verifyNoInteractions(mockWriter)
+    }
+
+    @Test
+    fun `M send a warning log and api usage telemetry W handleEvent { AddViewLoadingTime, no active view }`(
+        forge: Forge
+    ) {
+        // Given
+        val fakeEvent = forge.addViewLoadingTimeEvent()
+        whenever(mockChildScope.isActive()) doReturn false
+        testedScope.applicationDisplayed = true
+        testedScope.childrenScopes.add(mockChildScope)
+
+        // When
+        testedScope.handleEvent(fakeEvent, mockWriter)
+
+        // Then
+        mockInternalLogger.verifyApiUsage(
+            InternalTelemetryEvent.ApiUsage.AddViewLoadingTime(
+                overwrite = fakeEvent.overwrite,
+                noView = false,
+                noActiveView = true
+            ),
+            15f
+        )
     }
 
     // endregion

diff --git a/...oid-rum/src/test/kotlin/com/datadog/android/rum/internal/domain/scope/RumViewScopeTest.kt b/...oid-rum/src/test/kotlin/com/datadog/android/rum/internal/domain/scope/RumViewScopeTest.kt
@@ -7419,19 +7419,6 @@ internal class RumViewScopeTest {
 
         // Then
         assertThat(testedScope.viewLoadingTime).isNull()
-        mockInternalLogger.verifyLog(
-            InternalLogger.Level.WARN,
-            InternalLogger.Target.USER,
-            RumViewScope.NO_ACTIVE_VIEW_FOR_LOADING_TIME_WARNING_MESSAGE
-        )
-        mockInternalLogger.verifyApiUsage(
-            InternalTelemetryEvent.ApiUsage.AddViewLoadingTime(
-                noActiveView = true,
-                noView = false,
-                overwrite = fakeOverwrite
-            ),
-            15f
-        )
         verifyNoInteractions(mockWriter)
     }