Merge pull request #2324 from DataDog/jmoskovich/rum-6375/force-singl…

…e-core

RUM-6375: Force single core for session replay

dd-sdk-android-core/api/apiSurface

@@ -51,6 +51,7 @@ interface com.datadog.android.api.SdkCore
   val name: String
   val time: com.datadog.android.api.context.TimeInfo
   val service: String
+  fun isCoreActive(): Boolean
   fun setTrackingConsent(com.datadog.android.privacy.TrackingConsent)
   fun setUserInfo(String? = null, String? = null, String? = null, Map<String, Any?> = emptyMap())
   fun addUserProperties(Map<String, Any?>)

dd-sdk-android-core/api/dd-sdk-android-core.api

@@ -116,6 +116,7 @@ public abstract interface class com/datadog/android/api/SdkCore {
 	public abstract fun getName ()Ljava/lang/String;
 	public abstract fun getService ()Ljava/lang/String;
 	public abstract fun getTime ()Lcom/datadog/android/api/context/TimeInfo;
+	public abstract fun isCoreActive ()Z
 	public abstract fun setTrackingConsent (Lcom/datadog/android/privacy/TrackingConsent;)V
 	public abstract fun setUserInfo (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/util/Map;)V
 }

dd-sdk-android-core/src/main/kotlin/com/datadog/android/api/SdkCore.kt

@@ -32,6 +32,12 @@ interface SdkCore {
      */
     val service: String
 
+    /**
+     * Returns true if the core is active.
+     */
+    @AnyThread
+    fun isCoreActive(): Boolean
+
     /**
      * Sets the tracking consent regarding the data collection for this instance of the Datadog SDK.
      *

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/DatadogCore.kt

@@ -266,6 +266,8 @@ internal class DatadogCore(
         return coreFeature.createScheduledExecutorService(executorContext)
     }
 
+    override fun isCoreActive(): Boolean = isActive
+
     // endregion
 
     // region InternalSdkCore

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/NoOpInternalSdkCore.kt

@@ -89,6 +89,8 @@ internal object NoOpInternalSdkCore : InternalSdkCore {
 
     override fun clearAllData() = Unit
 
+    override fun isCoreActive(): Boolean = false
+
     // endregion
 
     // region FeatureSdkCore

dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/DatadogCoreTest.kt

@@ -885,6 +885,34 @@ internal class DatadogCoreTest {
         }
     }
 
+    @Test
+    fun `M return false W isActiveCore() { CoreFeature inactive }`() {
+        // Given
+        val mockCoreFeature = mock<CoreFeature>()
+        whenever(mockCoreFeature.initialized).thenReturn(AtomicBoolean(false))
+        testedCore.coreFeature = mockCoreFeature
+
+        // When
+        val isActive = testedCore.isCoreActive()
+
+        // Then
+        assertThat(isActive).isFalse()
+    }
+
+    @Test
+    fun `M return true W isActiveCore() { CoreFeature active }`() {
+        // Given
+        val mockCoreFeature = mock<CoreFeature>()
+        whenever(mockCoreFeature.initialized).thenReturn(AtomicBoolean(true))
+        testedCore.coreFeature = mockCoreFeature
+
+        // When
+        val isActive = testedCore.isCoreActive()
+
+        // Then
+        assertThat(isActive).isTrue()
+    }
+
     class ErrorRecordingRunnable(
         private val collector: MutableList<Throwable>,
         private val delegate: Runnable

detekt_custom.yml

@@ -718,6 +718,7 @@ datadog:
       - "java.lang.ref.WeakReference.constructor(android.content.Context?)"
       - "java.lang.ref.WeakReference.constructor(android.view.View?)"
       - "java.lang.ref.WeakReference.constructor(android.view.Window?)"
+      - "java.lang.ref.WeakReference.constructor(com.datadog.android.api.SdkCore?)"
       - "java.lang.ref.WeakReference.constructor(kotlin.Any?)"
       - "java.lang.ref.WeakReference.constructor(kotlin.Nothing?)"
       - "java.lang.ref.WeakReference.constructor(kotlin.String?)"

features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/SessionReplay.kt

@@ -6,18 +6,27 @@
 
 package com.datadog.android.sessionreplay
 
+import androidx.annotation.VisibleForTesting
 import com.datadog.android.Datadog
+import com.datadog.android.api.InternalLogger
 import com.datadog.android.api.SdkCore
 import com.datadog.android.api.feature.Feature.Companion.SESSION_REPLAY_FEATURE_NAME
 import com.datadog.android.api.feature.FeatureSdkCore
 import com.datadog.android.sessionreplay.internal.SessionReplayFeature
 import com.datadog.android.sessionreplay.internal.TouchPrivacyManager
+import java.lang.ref.WeakReference
 
 /**
  * An entry point to Datadog Session Replay feature.
  */
 object SessionReplay {
 
+    @VisibleForTesting internal var currentRegisteredCore: WeakReference<SdkCore>? = null
+
+    internal const val IS_ALREADY_REGISTERED_WARNING =
+        "Session Replay is already enabled and does not support multiple instances. " +
+            "The existing instance will continue to be used."
+
     /**
      * Enables a SessionReplay feature based on the configuration provided.
      * It is recommended to invoke this function as early as possible in the app's lifecycle,
@@ -51,7 +60,13 @@ object SessionReplay {
                     startRecordingImmediately = sessionReplayConfiguration.startRecordingImmediately,
                     dynamicOptimizationEnabled = sessionReplayConfiguration.dynamicOptimizationEnabled
                 )
-                sdkCore.registerFeature(sessionReplayFeature)
+
+                if (isAlreadyRegistered()) {
+                    logAlreadyRegisteredWarning(sdkCore.internalLogger)
+                } else {
+                    currentRegisteredCore = WeakReference(sdkCore)
+                    sdkCore.registerFeature(sessionReplayFeature)
+                }
             }
     }
 
@@ -86,4 +101,14 @@ object SessionReplay {
 
         sessionReplayFeature?.manuallyStopRecording()
     }
+
+    private fun isAlreadyRegistered() =
+        currentRegisteredCore?.get()?.isCoreActive() == true
+
+    private fun logAlreadyRegisteredWarning(internalLogger: InternalLogger) =
+        internalLogger.log(
+            level = InternalLogger.Level.WARN,
+            targets = listOf(InternalLogger.Target.MAINTAINER, InternalLogger.Target.TELEMETRY),
+            messageBuilder = { IS_ALREADY_REGISTERED_WARNING }
+        )
 }

features/dd-sdk-android-session-replay/src/test/kotlin/com/datadog/android/sessionreplay/SessionReplayTest.kt

@@ -6,12 +6,15 @@
 
 package com.datadog.android.sessionreplay
 
+import com.datadog.android.api.InternalLogger
 import com.datadog.android.api.feature.Feature.Companion.SESSION_REPLAY_FEATURE_NAME
 import com.datadog.android.api.feature.FeatureScope
 import com.datadog.android.api.feature.FeatureSdkCore
+import com.datadog.android.sessionreplay.SessionReplay.IS_ALREADY_REGISTERED_WARNING
 import com.datadog.android.sessionreplay.forge.ForgeConfigurator
 import com.datadog.android.sessionreplay.internal.SessionReplayFeature
 import com.datadog.android.sessionreplay.internal.net.SegmentRequestFactory
+import com.datadog.android.sessionreplay.utils.verifyLog
 import fr.xgouchet.elmyr.annotation.Forgery
 import fr.xgouchet.elmyr.annotation.StringForgery
 import fr.xgouchet.elmyr.junit5.ForgeConfiguration
@@ -50,6 +53,7 @@ internal class SessionReplayTest {
     @BeforeEach
     fun `set up`() {
         whenever(mockSdkCore.internalLogger) doReturn mock()
+        SessionReplay.currentRegisteredCore = null
     }
 
     @Test
@@ -119,4 +123,79 @@ internal class SessionReplayTest {
         // Then
         verify(mockSessionReplayFeature).manuallyStopRecording()
     }
+
+    @Test
+    fun `M warn and send telemetry W enable { session replay feature already registered with another core }`(
+        @Forgery fakeSessionReplayConfiguration: SessionReplayConfiguration,
+        @Mock mockCore1: FeatureSdkCore,
+        @Mock mockCore2: FeatureSdkCore,
+        @Mock mockInternalLogger: InternalLogger
+    ) {
+        // Given
+        whenever(mockCore1.isCoreActive()).thenReturn(true)
+        whenever(mockCore1.internalLogger).thenReturn(mockInternalLogger)
+        whenever(mockCore2.internalLogger).thenReturn(mockInternalLogger)
+        val fakeSessionReplayConfigurationWithMockRequirement = fakeSessionReplayConfiguration.copy(
+            systemRequirementsConfiguration = mockSystemRequirementsConfiguration
+        )
+        whenever(
+            mockSystemRequirementsConfiguration.runIfRequirementsMet(any(), any())
+        ) doAnswer {
+            it.getArgument<() -> Unit>(1).invoke()
+        }
+        SessionReplay.enable(
+            sessionReplayConfiguration = fakeSessionReplayConfigurationWithMockRequirement,
+            sdkCore = mockCore1
+        )
+
+        // When
+        SessionReplay.enable(
+            sessionReplayConfiguration = fakeSessionReplayConfigurationWithMockRequirement,
+            sdkCore = mockCore2
+        )
+
+        // Then
+        mockInternalLogger.verifyLog(
+            level = InternalLogger.Level.WARN,
+            targets = listOf(InternalLogger.Target.MAINTAINER, InternalLogger.Target.TELEMETRY),
+            message = IS_ALREADY_REGISTERED_WARNING
+        )
+        assertThat(SessionReplay.currentRegisteredCore?.get()).isEqualTo(mockCore1)
+    }
+
+    @Test
+    fun `M allow changing cores W enable { Session Replay already enabled but old core inactive }`(
+        @Forgery fakeSessionReplayConfiguration: SessionReplayConfiguration,
+        @Mock mockCore1: FeatureSdkCore,
+        @Mock mockCore2: FeatureSdkCore,
+        @Mock mockInternalLogger: InternalLogger
+    ) {
+        // Given
+        whenever(mockCore1.internalLogger).thenReturn(mockInternalLogger)
+        whenever(mockCore2.internalLogger).thenReturn(mockInternalLogger)
+        val fakeSessionReplayConfigurationWithMockRequirement = fakeSessionReplayConfiguration.copy(
+            systemRequirementsConfiguration = mockSystemRequirementsConfiguration
+        )
+        whenever(
+            mockSystemRequirementsConfiguration.runIfRequirementsMet(any(), any())
+        ) doAnswer {
+            it.getArgument<() -> Unit>(1).invoke()
+        }
+        whenever(mockCore1.isCoreActive()).thenReturn(true)
+        SessionReplay.enable(
+            sessionReplayConfiguration = fakeSessionReplayConfigurationWithMockRequirement,
+            sdkCore = mockCore1
+        )
+        assertThat(SessionReplay.currentRegisteredCore?.get()).isEqualTo(mockCore1)
+
+        // When
+        whenever(mockCore1.isCoreActive()).thenReturn(false)
+        SessionReplay.enable(
+            sessionReplayConfiguration = fakeSessionReplayConfigurationWithMockRequirement,
+            sdkCore = mockCore2
+        )
+
+        // Then
+        assertThat(SessionReplay.currentRegisteredCore?.get()).isEqualTo(mockCore2)
+    }
 }