Merge pull request #2650 from DataDog/nogorodnikov/rum-9977/update-rum-feature-context-once

RUM-9977: Update RUM feature context only after event processing completion

Author: 0xnm

detekt_custom.yml

@@ -991,8 +991,9 @@ datadog:
       - "kotlin.collections.MutableList.add(com.datadog.android.api.InternalLogger.Target)"
       - "kotlin.collections.MutableList.add(com.datadog.android.core.internal.persistence.Batch)"
       - "kotlin.collections.MutableList.add(com.datadog.android.core.internal.persistence.tlvformat.TLVBlock)"
-      - "kotlin.collections.MutableList.add(com.datadog.android.plugin.DatadogPlugin)"
       - "kotlin.collections.MutableList.add(com.datadog.android.rum.internal.domain.scope.RumScope)"
+      - "kotlin.collections.MutableList.add(com.datadog.android.rum.internal.domain.scope.RumSessionScope)"
+      - "kotlin.collections.MutableList.add(com.datadog.android.rum.internal.domain.scope.RumViewScope)"
       - "kotlin.collections.MutableList.add(com.datadog.android.rum.internal.vitals.FrameStateListener)"
       - "kotlin.collections.MutableList.add(com.datadog.android.rum.model.ActionEvent.Type)"
       - "kotlin.collections.MutableList.add(com.datadog.android.sessionreplay.compose.internal.data.Parameter)"

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

@@ -11,7 +11,6 @@ import androidx.annotation.WorkerThread
 import com.datadog.android.api.InternalLogger
 import com.datadog.android.api.context.DatadogContext
 import com.datadog.android.api.feature.EventWriteScope
-import com.datadog.android.api.feature.Feature
 import com.datadog.android.api.storage.DataWriter
 import com.datadog.android.core.InternalSdkCore
 import com.datadog.android.core.internal.net.FirstPartyHostHeaderTypeResolver
@@ -46,7 +45,7 @@ internal class RumApplicationScope(
 
     private var rumContext = RumContext(applicationId = applicationId)
 
-    internal val childScopes: MutableList<RumScope> = mutableListOf(
+    internal val childScopes = mutableListOf<RumSessionScope>(
         RumSessionScope(
             this,
             sdkCore,
@@ -67,9 +66,17 @@ internal class RumApplicationScope(
         )
     )
 
-    val activeSession: RumScope?
+    val activeSession: RumSessionScope?
         get() {
-            return childScopes.find { it.isActive() }
+            val activeSessions = childScopes.filter { it.isActive() }
+            if (activeSessions.size > 1) {
+                sdkCore.internalLogger.log(
+                    InternalLogger.Level.ERROR,
+                    InternalLogger.Target.MAINTAINER,
+                    { MULTIPLE_ACTIVE_SESSIONS_ERROR }
+                )
+            }
+            return activeSessions.lastOrNull()
         }
 
     private var lastActiveViewInfo: RumViewInfo? = null
@@ -94,10 +101,6 @@ internal class RumApplicationScope(
         val isInteraction = (event is RumRawEvent.StartView) || (event is RumRawEvent.StartAction)
         if (activeSession == null && isInteraction) {
             startNewSession(event, datadogContext, writeScope, writer)
-        } else if (event is RumRawEvent.StopSession) {
-            sdkCore.updateFeatureContext(Feature.RUM_FEATURE_NAME) {
-                it.putAll(getRumContext().toMap())
-            }
         }
 
         if (event !is RumRawEvent.SdkInit && !isAppStartedEventSent) {
@@ -185,7 +188,7 @@ internal class RumApplicationScope(
             sdkCore.internalLogger.log(
                 InternalLogger.Level.ERROR,
                 InternalLogger.Target.TELEMETRY,
-                { MULTIPLE_ACTIVE_SESSIONS_ERROR }
+                { MULTIPLE_ACTIVE_SESSIONS_SESSION_START_ERROR }
             )
         }
     }
@@ -224,7 +227,9 @@ internal class RumApplicationScope(
     // endregion
 
     companion object {
-        internal const val MULTIPLE_ACTIVE_SESSIONS_ERROR = "Application has multiple active " +
+        internal const val MULTIPLE_ACTIVE_SESSIONS_SESSION_START_ERROR = "Application has multiple active " +
             "sessions when starting a new session"
+        internal const val MULTIPLE_ACTIVE_SESSIONS_ERROR = "Application has multiple active " +
+            "sessions, this shouldn't happen."
     }
 }

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

@@ -62,7 +62,7 @@ internal class RumSessionScope(
     private val noOpWriter = NoOpDataWriter<Any>()
 
     @Suppress("LongParameterList")
-    internal var childScope: RumScope? = RumViewManagerScope(
+    internal var childScope: RumViewManagerScope? = RumViewManagerScope(
         this,
         sdkCore,
         sessionEndedMetricDispatcher,
@@ -80,11 +80,12 @@ internal class RumSessionScope(
         lastInteractionIdentifier
     )
 
-    init {
-        sdkCore.updateFeatureContext(Feature.RUM_FEATURE_NAME) {
-            it.putAll(getRumContext().toMap())
+    internal val activeView: RumViewScope?
+        get() = if (isActive() && childScope != null) {
+            childScope?.activeView
+        } else {
+            null
         }
-    }
 
     enum class State(val asString: String) {
         NOT_TRACKED("NOT_TRACKED"),
@@ -135,7 +136,8 @@ internal class RumSessionScope(
         val actualWriter = if (sessionState == State.TRACKED) writer else noOpWriter
 
         if (event !is RumRawEvent.SdkInit) {
-            childScope = childScope?.handleEvent(event, datadogContext, writeScope, actualWriter)
+            childScope = childScope
+                ?.handleEvent(event, datadogContext, writeScope, actualWriter) as? RumViewManagerScope
         }
 
         return if (isSessionComplete()) {

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

@@ -58,7 +58,25 @@ internal class RumViewManagerScope(
             lastInteractionIdentifier = lastInteractionIdentifier
         )
 
-    internal val childrenScopes = mutableListOf<RumScope>()
+    internal val childrenScopes = mutableListOf<RumViewScope>()
+
+    internal val activeView: RumViewScope?
+        get() {
+            return if (isActive()) {
+                val activeViews = childrenScopes.filter { it.isActive() }
+                if (activeViews.size > 1) {
+                    sdkCore.internalLogger.log(
+                        InternalLogger.Level.ERROR,
+                        InternalLogger.Target.MAINTAINER,
+                        { "Multiple views are active at the same time, this shouldn't happen." }
+                    )
+                }
+                activeViews.lastOrNull()
+            } else {
+                null
+            }
+        }
+
     internal var stopped = false
     private var lastStoppedViewTime: Time? = null
 
@@ -419,6 +437,9 @@ internal class RumViewManagerScope(
         internal const val NO_ACTIVE_VIEW_FOR_LOADING_TIME_WARNING_MESSAGE =
             "No active view found to add the loading time."
 
+        internal const val MULTIPLE_ACTIVE_VIEWS_ERROR =
+            "Multiple views are active at the same time, this shouldn't happen."
+
         internal val THREE_SECONDS_GAP_NS = TimeUnit.SECONDS.toNanos(3)
     }
 }

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

@@ -163,10 +163,6 @@ internal open class RumViewScope(
     // endregion
 
     init {
-        sdkCore.updateFeatureContext(Feature.RUM_FEATURE_NAME) {
-            it.putAll(getRumContext().toMap())
-        }
-
         cpuVitalMonitor.register(cpuVitalListener)
         memoryVitalMonitor.register(memoryVitalListener)
         frameRateVitalMonitor.register(frameRateVitalListener)
@@ -358,38 +354,6 @@ internal open class RumViewScope(
         val shouldStop = (event.key.id == key.id)
         if (shouldStop && !stopped) {
             stopScope(event, datadogContext, writeScope, writer) {
-                // we should not reset the timestamp offset here as due to async nature of feature context update
-                // we still need a stable value for the view timestamp offset for WebView RUM events timestamp
-                // correction
-                val newRumContext = getRumContext().copy(
-                    viewType = RumViewType.NONE,
-                    viewId = null,
-                    viewName = null,
-                    viewUrl = null,
-                    actionId = null
-                )
-                sdkCore.updateFeatureContext(Feature.RUM_FEATURE_NAME) { currentRumContext ->
-                    val canUpdate = when {
-                        currentRumContext[RumContext.SESSION_ID] != this.sessionId -> {
-                            // we have a new session, so whatever is in the Global context is
-                            // not valid anyway
-                            true
-                        }
-
-                        currentRumContext[RumContext.VIEW_ID] == this.viewId -> true
-                        else -> false
-                    }
-                    if (canUpdate) {
-                        currentRumContext.clear()
-                        currentRumContext.putAll(newRumContext.toMap())
-                    } else {
-                        sdkCore.internalLogger.log(
-                            InternalLogger.Level.DEBUG,
-                            InternalLogger.Target.MAINTAINER,
-                            { RUM_CONTEXT_UPDATE_IGNORED_AT_STOP_VIEW_MESSAGE }
-                        )
-                    }
-                }
                 eventAttributes.putAll(event.attributes)
             }
         }
@@ -432,16 +396,14 @@ internal open class RumViewScope(
             }
         }
 
-        updateActiveActionScope(
-            RumActionScope.fromEvent(
-                this,
-                sdkCore,
-                event,
-                serverTimeOffsetInMs,
-                featuresContextResolver,
-                trackFrustrations,
-                sampleRate
-            )
+        activeActionScope = RumActionScope.fromEvent(
+            this,
+            sdkCore,
+            event,
+            serverTimeOffsetInMs,
+            featuresContextResolver,
+            trackFrustrations,
+            sampleRate
         )
         pendingActionCount++
     }
@@ -707,31 +669,7 @@ internal open class RumViewScope(
         if (currentAction != null) {
             val updatedAction = currentAction.handleEvent(event, datadogContext, writeScope, writer)
             if (updatedAction == null) {
-                updateActiveActionScope(null)
-            }
-        }
-    }
-
-    private fun updateActiveActionScope(scope: RumScope?) {
-        activeActionScope = scope
-        // update the Rum Context to make it available for Logs/Trace bundling
-        val newRumContext = getRumContext()
-
-        sdkCore.updateFeatureContext(Feature.RUM_FEATURE_NAME) { currentRumContext ->
-            val canUpdate = when {
-                currentRumContext[RumContext.SESSION_ID] != sessionId -> true
-                currentRumContext[RumContext.VIEW_ID] == viewId -> true
-                else -> false
-            }
-            if (canUpdate) {
-                currentRumContext.clear()
-                currentRumContext.putAll(newRumContext.toMap())
-            } else {
-                sdkCore.internalLogger.log(
-                    InternalLogger.Level.DEBUG,
-                    InternalLogger.Target.MAINTAINER,
-                    { RUM_CONTEXT_UPDATE_IGNORED_AT_ACTION_UPDATE_MESSAGE }
-                )
+                activeActionScope = null
             }
         }
     }
@@ -1476,13 +1414,6 @@ internal open class RumViewScope(
         internal const val ACTION_DROPPED_WARNING = "RUM Action (%s on %s) was dropped, because" +
             " another action is still active for the same view"
 
-        internal const val RUM_CONTEXT_UPDATE_IGNORED_AT_STOP_VIEW_MESSAGE =
-            "Trying to update global RUM context when StopView event arrived, but the context" +
-                " doesn't reference this view."
-        internal const val RUM_CONTEXT_UPDATE_IGNORED_AT_ACTION_UPDATE_MESSAGE =
-            "Trying to update active action in the global RUM context, but the context" +
-                " doesn't reference this view."
-
         internal val FROZEN_FRAME_THRESHOLD_NS = TimeUnit.MILLISECONDS.toNanos(700)
         internal const val SLOW_RENDERED_THRESHOLD_FPS = 55
         internal const val ZERO_DURATION_WARNING_MESSAGE = "The computed duration for the " +

features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/monitor/DatadogRumMonitor.kt

@@ -38,10 +38,8 @@ import com.datadog.android.rum.internal.domain.asTime
 import com.datadog.android.rum.internal.domain.event.ResourceTiming
 import com.datadog.android.rum.internal.domain.scope.RumApplicationScope
 import com.datadog.android.rum.internal.domain.scope.RumRawEvent
-import com.datadog.android.rum.internal.domain.scope.RumScope
 import com.datadog.android.rum.internal.domain.scope.RumScopeKey
 import com.datadog.android.rum.internal.domain.scope.RumSessionScope
-import com.datadog.android.rum.internal.domain.scope.RumViewManagerScope
 import com.datadog.android.rum.internal.domain.scope.RumViewScope
 import com.datadog.android.rum.internal.metric.SessionMetricDispatcher
 import com.datadog.android.rum.internal.metric.slowframes.SlowFramesListener
@@ -80,7 +78,7 @@ internal class DatadogRumMonitor(
     slowFramesListener: SlowFramesListener?
 ) : RumMonitor, AdvancedRumMonitor {
 
-    internal var rootScope: RumScope = RumApplicationScope(
+    internal var rootScope = RumApplicationScope(
         applicationId,
         sdkCore,
         sampleRate,
@@ -120,8 +118,7 @@ internal class DatadogRumMonitor(
             "Get current session ID",
             sdkCore.internalLogger
         ) {
-            val activeSessionId = (rootScope as? RumApplicationScope)
-                ?.activeSession
+            val activeSessionId = rootScope.activeSession
                 ?.getRumContext()
                 ?.let {
                     val sessionId = it.sessionId
@@ -682,6 +679,7 @@ internal class DatadogRumMonitor(
                     val (datadogContext, eventWriteScope) = writeContext
                     @Suppress("ThreadSafety") // Crash handling, can't delegate to another thread
                     rootScope.handleEvent(event, datadogContext, eventWriteScope, writer)
+                    updateFeatureContext()
                 } else {
                     sdkCore.internalLogger.log(
                         InternalLogger.Level.WARN,
@@ -700,6 +698,7 @@ internal class DatadogRumMonitor(
                     executorService.executeSafe("Rum event handling", sdkCore.internalLogger) {
                         synchronized(rootScope) {
                             rootScope.handleEvent(event, datadogContext, writeScope, writer)
+                            updateFeatureContext()
                             notifyDebugListenerWithState()
                         }
                         handler.postDelayed(keepAliveRunnable, KEEP_ALIVE_MS)
@@ -733,15 +732,24 @@ internal class DatadogRumMonitor(
         }
     }
 
+    private fun updateFeatureContext() {
+        sdkCore.updateFeatureContext(Feature.RUM_FEATURE_NAME) {
+            val activeSession = rootScope.activeSession
+            val context = activeSession?.activeView?.getRumContext()
+                ?: activeSession?.getRumContext()
+                ?: rootScope.getRumContext()
+            it.putAll(context.toMap())
+        }
+    }
+
     internal fun stopKeepAliveCallback() {
         handler.removeCallbacks(keepAliveRunnable)
     }
 
     internal fun notifyDebugListenerWithState() {
         debugListener?.let {
-            val applicationScope = rootScope as? RumApplicationScope
-            val sessionScope = applicationScope?.activeSession as? RumSessionScope
-            val viewManagerScope = sessionScope?.childScope as? RumViewManagerScope
+            val sessionScope = rootScope.activeSession
+            val viewManagerScope = sessionScope?.childScope
             if (viewManagerScope != null) {
                 it.onReceiveRumActiveViews(
                     viewManagerScope.childrenScopes

features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/RumTest.kt

@@ -13,7 +13,6 @@ import com.datadog.android.api.feature.FeatureSdkCore
 import com.datadog.android.core.InternalSdkCore
 import com.datadog.android.core.sampling.RateBasedSampler
 import com.datadog.android.rum.internal.RumFeature
-import com.datadog.android.rum.internal.domain.scope.RumApplicationScope
 import com.datadog.android.rum.internal.monitor.DatadogRumMonitor
 import com.datadog.android.rum.internal.monitor.NoOpAdvancedRumMonitor
 import com.datadog.android.rum.internal.net.RumRequestFactory
@@ -135,14 +134,12 @@ internal class RumTest {
         // Then
         val monitor = GlobalRumMonitor.get(mockSdkCore)
         check(monitor is DatadogRumMonitor)
-        assertThat(monitor.rootScope).isInstanceOf(RumApplicationScope::class.java)
         assertThat(monitor.rootScope)
             .overridingErrorMessage(
                 "Expecting root scope to have applicationId ${fakeRumConfiguration.applicationId}"
             )
             .matches {
-                (it as RumApplicationScope)
-                    .getRumContext()
+                it.getRumContext()
                     .applicationId == fakeRumConfiguration.applicationId
             }
         assertThat(monitor.handler.looper).isSameAs(Looper.getMainLooper())
@@ -158,7 +155,7 @@ internal class RumTest {
 
         assertThat(telemetrySampler.getSampleRate())
             .isEqualTo(fakeRumConfiguration.featureConfiguration.telemetrySampleRate)
-        val rumApplicationScope = monitor.rootScope as RumApplicationScope
+        val rumApplicationScope = monitor.rootScope
         assertThat(rumApplicationScope.initialResourceIdentifier)
             .isSameAs(fakeRumConfiguration.featureConfiguration.initialResourceIdentifier)
         assertThat(rumApplicationScope.lastInteractionIdentifier)