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.

Sign up for GitHub

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

Jump to bottom

Read SR system requirement synchronously with strict mode allowance #2307

Merged
merged 1 commit into from
Oct 4, 2024
Merged

Read SR system requirement synchronously with strict mode allowance #2307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 4 additions & 17 deletions ...android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/SessionReplay.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@

package com.datadog.android.sessionreplay

import android.os.Handler
import android.os.Looper
import com.datadog.android.Datadog
import com.datadog.android.api.SdkCore
import com.datadog.android.api.feature.Feature.Companion.SESSION_REPLAY_FEATURE_NAME
Expand All @@ -21,6 +19,8 @@ object SessionReplay {

/**
* 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,
* ideally within the `Application#onCreate` callback, to ensure proper initialization.
*
* @param sessionReplayConfiguration Configuration to use for the feature.
* @param sdkCore SDK instance to register feature in. If not provided, default SDK instance
Expand All @@ -31,22 +31,10 @@ object SessionReplay {
fun enable(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

side question: while we mitigated the issue for the case when SR is initialized in the Application#onCreate and on the main thread, I believe we can still have a similar issue if SR is initialialized by the customer from the worker thread / later in the app lifecycle. Is there a way to mitigate completely?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With current solution of LifecycleCallback, I don't see a solution, once user misses the onResume event, they need to wait until the next onResume to make the touch recorder work properly

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is no solution for such case, then maybe we should highlight in the docs indeed that SR should be initialized early.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

sessionReplayConfiguration: SessionReplayConfiguration,
sdkCore: SdkCore = Datadog.getInstance()
) {
enable(sessionReplayConfiguration, Handler(Looper.getMainLooper()), sdkCore)
}

internal fun enable(
sessionReplayConfiguration: SessionReplayConfiguration,
uiHandler: Handler,
sdkCore: SdkCore = Datadog.getInstance()
) {
val featureSdkCore = sdkCore as FeatureSdkCore
sessionReplayConfiguration.systemRequirementsConfiguration.let {
it.runIfRequirementsMet(
sdkCore = sdkCore,
uiHandler = uiHandler,
featureSdkCore.internalLogger
) {
sessionReplayConfiguration.systemRequirementsConfiguration
.runIfRequirementsMet(featureSdkCore.internalLogger) {
val sessionReplayFeature = SessionReplayFeature(
sdkCore = featureSdkCore,
customEndpointUrl = sessionReplayConfiguration.customEndpointUrl,
Expand All @@ -62,7 +50,6 @@ object SessionReplay {
)
sdkCore.registerFeature(sessionReplayFeature)
}
}
}

/**
Expand Down
43 changes: 16 additions & 27 deletions ...play/src/main/kotlin/com/datadog/android/sessionreplay/SystemRequirementsConfiguration.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,7 @@

package com.datadog.android.sessionreplay

import android.os.Handler
import com.datadog.android.api.InternalLogger
import com.datadog.android.api.feature.FeatureSdkCore
import com.datadog.android.core.InternalSdkCore
import com.datadog.android.core.internal.utils.submitSafe
import com.datadog.android.sessionreplay.internal.prerequisite.CPURequirementChecker
import com.datadog.android.sessionreplay.internal.prerequisite.MemoryRequirementChecker
import com.datadog.android.sessionreplay.internal.prerequisite.SystemRequirementChecker
Expand All @@ -24,36 +20,30 @@ class SystemRequirementsConfiguration internal constructor(
) {

internal fun runIfRequirementsMet(
sdkCore: FeatureSdkCore,
uiHandler: Handler,
internalLogger: InternalLogger,
runnable: () -> Unit
) {
val checkers = listOf(
CPURequirementChecker(minCPUCores, internalLogger = internalLogger),
MemoryRequirementChecker(minRAMSizeMb, internalLogger = internalLogger)
)
(sdkCore as InternalSdkCore).getPersistenceExecutorService().submitSafe(OPERATION_NAME, internalLogger) {
val checkResult = checkers.all {
it.checkMinimumRequirement()
}
val checkResult = checkers.all {
it.checkMinimumRequirement()
}

if (checkResult) {
uiHandler.post {
runnable()
}
} else {
internalLogger.log(
level = InternalLogger.Level.INFO,
listOf(InternalLogger.Target.TELEMETRY, InternalLogger.Target.USER),
messageBuilder = {
"Session replay is disabled because the system doesn't meet the minimum " +
"Session Replay requirements"
},
onlyOnce = true,
additionalProperties = getCheckerReport(checkers)
)
}
if (checkResult) {
runnable()
} else {
internalLogger.log(
level = InternalLogger.Level.INFO,
listOf(InternalLogger.Target.TELEMETRY, InternalLogger.Target.USER),
messageBuilder = {
"Session replay is disabled because the system doesn't meet the minimum " +
"Session Replay requirements"
},
onlyOnce = true,
additionalProperties = getCheckerReport(checkers)
)
}
}

Expand Down Expand Up @@ -107,7 +97,6 @@ class SystemRequirementsConfiguration internal constructor(
companion object {

private const val ATTRIBUTE_DEVICE_STATS_KEY = "device_stats"
private const val OPERATION_NAME = "Check Session Replay requirements"

/**
* A preconfigured instance representing the basic system requirements for enabling the Session Replay feature.
Expand Down
6 changes: 5 additions & 1 deletion ...n/kotlin/com/datadog/android/sessionreplay/internal/prerequisite/CPURequirementChecker.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package com.datadog.android.sessionreplay.internal.prerequisite

import com.datadog.android.api.InternalLogger
import com.datadog.android.core.allowThreadDiskReads
import com.datadog.android.core.internal.persistence.file.listFilesSafe
import java.io.File

Expand All @@ -22,7 +23,10 @@ internal class CPURequirementChecker(
if (minCPUCores == 0) {
return true
}
return readCPUCoreNumber() >= minCPUCores
val actualCPUCoreNumber = allowThreadDiskReads {
readCPUCoreNumber()
}
return actualCPUCoreNumber >= minCPUCores
}

override fun name(): String = CPU_CHECK_NAME
Expand Down
6 changes: 5 additions & 1 deletion ...otlin/com/datadog/android/sessionreplay/internal/prerequisite/MemoryRequirementChecker.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package com.datadog.android.sessionreplay.internal.prerequisite

import com.datadog.android.api.InternalLogger
import com.datadog.android.core.allowThreadDiskReads
import com.datadog.android.core.internal.persistence.file.canReadSafe
import com.datadog.android.core.internal.persistence.file.existsSafe
import com.datadog.android.core.internal.persistence.file.readLinesSafe
Expand All @@ -24,7 +25,10 @@ internal class MemoryRequirementChecker(
if (minRamSizeMb == 0) {
return true
}
return getMaxRAMSize() >= minRamSizeMb
val actualMaxRamSizeMb = allowThreadDiskReads {
getMaxRAMSize()
}
return actualMaxRamSizeMb >= minRamSizeMb
}

override fun name(): String = MEMORY_CHECK_NAME
Expand Down
7 changes: 2 additions & 5 deletions ...oid-session-replay/src/test/kotlin/com/datadog/android/sessionreplay/SessionReplayTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import org.mockito.kotlin.any
import org.mockito.kotlin.argumentCaptor
import org.mockito.kotlin.doAnswer
import org.mockito.kotlin.doReturn
import org.mockito.kotlin.eq
import org.mockito.kotlin.mock
import org.mockito.kotlin.verify
import org.mockito.kotlin.whenever
Expand Down Expand Up @@ -63,11 +62,9 @@ internal class SessionReplayTest {
systemRequirementsConfiguration = mockSystemRequirementsConfiguration
)
whenever(
mockSystemRequirementsConfiguration.runIfRequirementsMet(
eq(mockSdkCore), any(), any(), any()
)
mockSystemRequirementsConfiguration.runIfRequirementsMet(any(), any())
) doAnswer {
it.getArgument<() -> Unit>(3).invoke()
it.getArgument<() -> Unit>(1).invoke()
}
SessionReplay.enable(
fakeSessionReplayConfigurationWithMockRequirement,
Expand Down
5 changes: 4 additions & 1 deletion sample/kotlin/src/main/kotlin/com/datadog/android/sample/SampleApplication.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,10 @@ class SampleApplication : Application() {

private fun initializeSessionReplay() {
val shouldUseFgm = SecureRandom().nextInt(100) < USE_FGM_PCT
val systemRequirementsConfiguration = SystemRequirementsConfiguration.BASIC
val systemRequirementsConfiguration = SystemRequirementsConfiguration.Builder()
.setMinRAMSizeMb(1024)
.setMinCPUCoreNumber(1)
.build()

@Suppress("DEPRECATION")
val sessionReplayConfig = SessionReplayConfiguration.Builder(SAMPLE_IN_ALL_SESSIONS)
Expand Down