Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RUM-6375: Force single core for session replay #2324

Merged
merged 2 commits into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions dd-sdk-android-core/api/apiSurface
Original file line number Diff line number Diff line change
Expand Up @@ -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?>)
Expand Down
1 change: 1 addition & 0 deletions dd-sdk-android-core/api/dd-sdk-android-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,8 @@ internal class DatadogCore(
return coreFeature.createScheduledExecutorService(executorContext)
}

override fun isCoreActive(): Boolean = isActive
jonathanmos marked this conversation as resolved.
Show resolved Hide resolved

// endregion

// region InternalSdkCore
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ internal object NoOpInternalSdkCore : InternalSdkCore {

override fun clearAllData() = Unit

override fun isCoreActive(): Boolean = false

// endregion

// region FeatureSdkCore
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions detekt_custom.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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?)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
ambushwork marked this conversation as resolved.
Show resolved Hide resolved

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,
Expand Down Expand Up @@ -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)
}
}
}

Expand Down Expand Up @@ -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 }
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -50,6 +53,7 @@ internal class SessionReplayTest {
@BeforeEach
fun `set up`() {
whenever(mockSdkCore.internalLogger) doReturn mock()
SessionReplay.currentRegisteredCore = null
}

@Test
Expand Down Expand Up @@ -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)
}
}