Merge pull request #1725 from DataDog/nogorodnikov/rum-1017/clean-up-…

…registered-rum-monitor-when-rum-feature-is-stopped RUM-1017: Unregister RUM monitor when associated RUM feature is stopped
DataDog · Nov 16, 2023 · 8c57c9c · 8c57c9c
2 parents 327e7ea + ae0408f
commit 8c57c9c
Show file tree

Hide file tree

Showing 8 changed files with 51 additions and 43 deletions.
diff --git a/detekt_custom.yml b/detekt_custom.yml
@@ -902,6 +902,7 @@ datadog:
       - "kotlin.collections.MutableMap.put(kotlin.String, kotlin.String)"
       - "kotlin.collections.MutableMap.putAll(kotlin.collections.Map)"
       - "kotlin.collections.MutableMap.remove(androidx.compose.foundation.interaction.DragInteraction.Start)"
+      - "kotlin.collections.MutableMap.remove(com.datadog.android.api.SdkCore)"
       - "kotlin.collections.MutableMap.remove(com.datadog.android.rum.internal.vitals.VitalListener)"
       - "kotlin.collections.MutableMap.remove(kotlin.Int)"
       - "kotlin.collections.MutableMap.remove(kotlin.Long)"

diff --git a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/GlobalRumMonitor.kt b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/GlobalRumMonitor.kt
@@ -43,6 +43,36 @@ object GlobalRumMonitor {
         }
     }
 
+    /**
+     * Returns the constant [RumMonitor] instance.
+     *
+     * Until a RUM feature is enabled, a no-op implementation is returned.
+     *
+     * @return The monitor associated with the instance given instance, or a no-op monitor. If SDK
+     * instance is not provided, default instance will be used.
+     */
+    @JvmOverloads
+    @JvmStatic
+    fun get(sdkCore: SdkCore = Datadog.getInstance()): RumMonitor {
+        return synchronized(registeredMonitors) {
+            val monitor = registeredMonitors[sdkCore]
+            if (monitor == null) {
+                (sdkCore as? FeatureSdkCore)
+                    ?.internalLogger
+                    ?.log(
+                        InternalLogger.Level.WARN,
+                        InternalLogger.Target.USER,
+                        { NO_MONITOR_REGISTERED_MESSAGE.format(Locale.US, sdkCore.name) }
+                    )
+                NoOpRumMonitor()
+            } else {
+                monitor
+            }
+        }
+    }
+
+    // region Internal
+
     /**
      * Register a [RumMonitor] with an [SdkCore] to back the behaviour of the [get] function.
      *
@@ -74,36 +104,12 @@ object GlobalRumMonitor {
         }
     }
 
-    /**
-     * Returns the constant [RumMonitor] instance.
-     *
-     * Until a RUM feature is enabled, a no-op implementation is returned.
-     *
-     * @return The monitor associated with the instance given instance, or a no-op monitor. If SDK
-     * instance is not provided, default instance will be used.
-     */
-    @JvmOverloads
-    @JvmStatic
-    fun get(sdkCore: SdkCore = Datadog.getInstance()): RumMonitor {
-        return synchronized(registeredMonitors) {
-            val monitor = registeredMonitors[sdkCore]
-            if (monitor == null) {
-                (sdkCore as? FeatureSdkCore)
-                    ?.internalLogger
-                    ?.log(
-                        InternalLogger.Level.WARN,
-                        InternalLogger.Target.USER,
-                        { NO_MONITOR_REGISTERED_MESSAGE.format(Locale.US, sdkCore.name) }
-                    )
-                NoOpRumMonitor()
-            } else {
-                monitor
-            }
+    internal fun unregister(sdkCore: SdkCore = Datadog.getInstance()) {
+        synchronized(registeredMonitors) {
+            registeredMonitors.remove(sdkCore)
         }
     }
 
-    // region Internal
-
     internal fun clear() {
         synchronized(registeredMonitors) {
             registeredMonitors.clear()

diff --git a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/RumFeature.kt b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/RumFeature.kt
@@ -217,6 +217,8 @@ internal class RumFeature constructor(
         anrDetectorRunnable.stop()
         vitalExecutorService = NoOpScheduledExecutorService()
         sessionListener = NoOpRumSessionListener()
+
+        GlobalRumMonitor.unregister(sdkCore)
     }
 
     // endregion

diff --git a/...res/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/RumFeatureTest.kt b/...res/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/RumFeatureTest.kt
@@ -13,6 +13,7 @@ import com.datadog.android.core.InternalSdkCore
 import com.datadog.android.event.EventMapper
 import com.datadog.android.event.MapperSerializer
 import com.datadog.android.rum.GlobalRumMonitor
+import com.datadog.android.rum.NoOpRumMonitor
 import com.datadog.android.rum.RumErrorSource
 import com.datadog.android.rum.assertj.RumFeatureAssert
 import com.datadog.android.rum.configuration.VitalsUpdateFrequency
@@ -572,6 +573,20 @@ internal class RumFeatureTest {
         assertThat(testedFeature.dataWriter).isInstanceOf(NoOpDataWriter::class.java)
     }
 
+    @Test
+    fun `𝕄 remove associated monitor 𝕎 onStop()`() {
+        // Given
+        testedFeature.onInitialize(appContext.mockInstance)
+        GlobalRumMonitor.registerIfAbsent(mockRumMonitor, mockSdkCore)
+
+        // When
+        testedFeature.onStop()
+
+        // Then
+        assertThat(GlobalRumMonitor.isRegistered(mockSdkCore)).isFalse
+        assertThat(GlobalRumMonitor.get(mockSdkCore)).isInstanceOf(NoOpRumMonitor::class.java)
+    }
+
     @ParameterizedTest
     @EnumSource(VitalsUpdateFrequency::class, names = ["NEVER"], mode = EnumSource.Mode.EXCLUDE)
     fun `𝕄 initialize vital executor 𝕎 initialize { frequency != NEVER }()`(

diff --git a/...ion/src/androidTest/kotlin/com/datadog/android/sdk/integration/security/EncryptionTest.kt b/...ion/src/androidTest/kotlin/com/datadog/android/sdk/integration/security/EncryptionTest.kt
@@ -205,10 +205,6 @@ internal class EncryptionTest {
     private fun stopSdk() {
         Datadog.stopInstance()
         GlobalTracer::class.java.setStaticValue("isRegistered", false)
-        GlobalRumMonitor::class.java.getDeclaredMethod("reset").apply {
-            isAccessible = true
-            invoke(null)
-        }
     }
 
     private fun flushAndShutdownExecutors() {

diff --git a/...oidTest/kotlin/com/datadog/android/sdk/integration/sessionreplay/BaseSessionReplayTest.kt b/...oidTest/kotlin/com/datadog/android/sdk/integration/sessionreplay/BaseSessionReplayTest.kt
@@ -40,10 +40,6 @@ internal abstract class BaseSessionReplayTest<R : Activity> {
     fun tearDown() {
         GlobalRumMonitor.get().stopSession()
         Datadog.stopInstance()
-        GlobalRumMonitor::class.java.getDeclaredMethod("reset").apply {
-            isAccessible = true
-            invoke(null)
-        }
     }
 
     protected fun assessSrPayload(payloadFileName: String, rule: SessionReplayTestRule<R>) {

diff --git a/...ration/src/androidTest/kotlin/com/datadog/android/sdk/rules/MockServerActivityTestRule.kt b/...ration/src/androidTest/kotlin/com/datadog/android/sdk/rules/MockServerActivityTestRule.kt
@@ -105,10 +105,6 @@ internal open class MockServerActivityTestRule<T : Activity>(
             .cacheDir.deleteRecursively()
         GlobalRumMonitor.get().stopSession()
         Datadog.stopInstance()
-        GlobalRumMonitor::class.java.getDeclaredMethod("reset").apply {
-            isAccessible = true
-            invoke(null)
-        }
 
         super.afterActivityFinished()
     }

diff --git a/...ented/nightly-tests/src/androidTest/kotlin/com/datadog/android/nightly/utils/MiscUtils.kt b/...ented/nightly-tests/src/androidTest/kotlin/com/datadog/android/nightly/utils/MiscUtils.kt
@@ -80,10 +80,6 @@ fun stopSdk() {
 
     // Reset Global states
     GlobalTracer::class.java.setStaticValue("isRegistered", false)
-    GlobalRumMonitor::class.java.getDeclaredMethod("reset").apply {
-        isAccessible = true
-        invoke(null)
-    }
 }
 
 fun flushAndShutdownExecutors() {