Merge pull request #1851 from DataDog/mconstantin/rum-3012/freeze-glo…

…bal-attributes-for-stopped-views

RUM-3012 Do not update RUM View global properties after the view is stopped

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

@@ -58,7 +58,8 @@ internal open class RumViewScope(
 
     internal val url = key.url.replace('.', '/')
 
-    internal val attributes: MutableMap<String, Any?> = initialAttributes.toMutableMap()
+    internal val eventAttributes: MutableMap<String, Any?> = initialAttributes.toMutableMap()
+    private var globalAttributes: MutableMap<String, Any?> = resolveGlobalAttributes(sdkCore)
 
     private var sessionId: String = parentScope.getRumContext().sessionId
     internal var viewId: String = UUID.randomUUID().toString()
@@ -142,7 +143,6 @@ internal open class RumViewScope(
             it.putAll(getRumContext().toMap())
             it[RumFeature.VIEW_TIMESTAMP_OFFSET_IN_MS_KEY] = serverTimeOffsetInMs
         }
-        attributes.putAll(GlobalRumMonitor.get(sdkCore).getAttributes())
         cpuVitalMonitor.register(cpuVitalListener)
         memoryVitalMonitor.register(memoryVitalListener)
         frameRateVitalMonitor.register(frameRateVitalListener)
@@ -161,6 +161,7 @@ internal open class RumViewScope(
         event: RumRawEvent,
         writer: DataWriter<Any>
     ): RumScope? {
+        updateGlobalAttributes(sdkCore, event)
         when (event) {
             is RumRawEvent.ResourceSent -> onResourceSent(event, writer)
             is RumRawEvent.ActionSent -> onActionSent(event, writer)
@@ -279,7 +280,7 @@ internal open class RumViewScope(
                     )
                 }
             }
-            attributes.putAll(event.attributes)
+            eventAttributes.putAll(event.attributes)
             stopped = true
             sendViewUpdate(event, writer)
             sendViewChanged()
@@ -702,7 +703,6 @@ internal open class RumViewScope(
     @Suppress("LongMethod", "ComplexMethod")
     private fun sendViewUpdate(event: RumRawEvent, writer: DataWriter<Any>) {
         val viewComplete = isViewComplete()
-        attributes.putAll(GlobalRumMonitor.get(sdkCore).getAttributes())
         version++
 
         // make a local copy, so that closure captures the state as of now
@@ -735,7 +735,7 @@ internal open class RumViewScope(
         val isSlowRendered = resolveRefreshRateInfo(refreshRateInfo) ?: false
         // make a copy - by the time we iterate over it on another thread, it may already be changed
         val eventFeatureFlags = featureFlags.toMutableMap()
-        val eventAdditionalAttributes = attributes.toMutableMap()
+        val eventAdditionalAttributes = (eventAttributes + globalAttributes).toMutableMap()
 
         sdkCore.newRumEventWriteOperation(writer) { datadogContext ->
             val currentViewId = rumContext.viewId.orEmpty()
@@ -850,6 +850,16 @@ internal open class RumViewScope(
             .submit()
     }
 
+    private fun updateGlobalAttributes(sdkCore: InternalSdkCore, event: RumRawEvent) {
+        if (!stopped && event !is RumRawEvent.StartView) {
+            globalAttributes = resolveGlobalAttributes(sdkCore)
+        }
+    }
+
+    private fun resolveGlobalAttributes(sdkCore: InternalSdkCore): MutableMap<String, Any?> {
+        return GlobalRumMonitor.get(sdkCore).getAttributes().toMutableMap()
+    }
+
     private fun resolveViewDuration(event: RumRawEvent): Long {
         val duration = event.eventTime.nanoTime - startedNanos
         return if (duration <= 0) {
@@ -883,8 +893,7 @@ internal open class RumViewScope(
     private fun addExtraAttributes(
         attributes: Map<String, Any?>
     ): MutableMap<String, Any?> {
-        return attributes.toMutableMap()
-            .apply { putAll(GlobalRumMonitor.get(sdkCore).getAttributes()) }
+        return attributes.toMutableMap().apply { putAll(globalAttributes) }
     }
 
     @Suppress("LongMethod")
@@ -895,9 +904,7 @@ internal open class RumViewScope(
     ) {
         pendingActionCount++
         val rumContext = getRumContext()
-
-        val globalAttributes = GlobalRumMonitor.get(sdkCore).getAttributes().toMutableMap()
-
+        val localCopyOfGlobalAttributes = globalAttributes.toMutableMap()
         sdkCore.newRumEventWriteOperation(writer) { datadogContext ->
             val user = datadogContext.userInfo
             val syntheticsAttribute = if (
@@ -967,7 +974,7 @@ internal open class RumViewScope(
                     architecture = datadogContext.deviceInfo.architecture
                 ),
                 context = ActionEvent.Context(
-                    additionalProperties = globalAttributes
+                    additionalProperties = localCopyOfGlobalAttributes
                 ),
                 dd = ActionEvent.Dd(
                     session = ActionEvent.DdSession(
@@ -1109,17 +1116,17 @@ internal open class RumViewScope(
         viewChangedListener?.onViewChanged(
             RumViewInfo(
                 key = key,
-                attributes = attributes,
+                attributes = eventAttributes,
                 isActive = isActive()
             )
         )
     }
 
     private fun isViewComplete(): Boolean {
         val pending = pendingActionCount +
-            pendingResourceCount +
-            pendingErrorCount +
-            pendingLongTaskCount
+                pendingResourceCount +
+                pendingErrorCount +
+                pendingLongTaskCount
         // we use <= 0 for pending counter as a safety measure to make sure this ViewScope will
         // be closed.
         return stopped && activeResourceScopes.isEmpty() && (pending <= 0L)
@@ -1144,19 +1151,19 @@ internal open class RumViewScope(
         internal val ONE_SECOND_NS = TimeUnit.SECONDS.toNanos(1)
 
         internal const val ACTION_DROPPED_WARNING = "RUM Action (%s on %s) was dropped, because" +
-            " another action is still active for the same view"
+                " 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."
+                    " 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."
+                    " 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 NEGATIVE_DURATION_WARNING_MESSAGE = "The computed duration for the " +
-            "view: %s was 0 or negative. In order to keep the view we forced it to 1ns."
+                "view: %s was 0 or negative. In order to keep the view we forced it to 1ns."
 
         internal fun fromEvent(
             parentScope: RumScope,

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

@@ -285,7 +285,7 @@ internal class RumApplicationScopeTest {
         val viewScope = viewManager.childrenScopes.first()
         check(viewScope is RumViewScope)
         assertThat(viewScope.key).isEqualTo(fakeKey)
-        assertThat(viewScope.attributes).isEqualTo(mockAttributes)
+        assertThat(viewScope.eventAttributes).isEqualTo(mockAttributes)
     }
 
     @Test

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

@@ -214,7 +214,7 @@ internal class RumViewManagerScopeTest {
                     .isEqualTo(resolveExpectedTimestamp(fakeEvent.eventTime.timestamp))
                 assertThat(it.key).isEqualTo(fakeEvent.key)
                 assertThat(it.type).isEqualTo(RumViewScope.RumViewType.FOREGROUND)
-                assertThat(it.attributes).containsAllEntriesOf(fakeEvent.attributes)
+                assertThat(it.eventAttributes).containsAllEntriesOf(fakeEvent.attributes)
                 assertThat(it.firstPartyHostHeaderTypeResolver).isSameAs(mockResolver)
                 assertThat(it.version).isEqualTo(2)
             }
@@ -241,7 +241,7 @@ internal class RumViewManagerScopeTest {
                     .isEqualTo(resolveExpectedTimestamp(fakeEvent.eventTime.timestamp))
                 assertThat(it.key).isEqualTo(fakeEvent.key)
                 assertThat(it.type).isEqualTo(RumViewScope.RumViewType.FOREGROUND)
-                assertThat(it.attributes).containsAllEntriesOf(fakeEvent.attributes)
+                assertThat(it.eventAttributes).containsAllEntriesOf(fakeEvent.attributes)
                 assertThat(it.firstPartyHostHeaderTypeResolver).isSameAs(mockResolver)
                 assertThat(it.version).isEqualTo(2)
             }