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-1017: Unregister RUM monitor when associated RUM feature is stopped #1725

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions detekt_custom.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,8 @@ internal class RumFeature constructor(
anrDetectorRunnable.stop()
vitalExecutorService = NoOpScheduledExecutorService()
sessionListener = NoOpRumSessionListener()

GlobalRumMonitor.unregister(sdkCore)
}

// endregion
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 }()`(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down