Make 'room list catch-up' analytics transaction network aware (#6233)

* Make 'room list catch-up' analytics transaction network aware.
* Add `RoomListService.isInitialSyncDone`. Use this to simplify `DefaultAnalyticsRoomListStateWatcher`'s logic.

libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/roomlist/RoomListService.kt

@@ -35,6 +35,11 @@ interface RoomListService {
         data object Hide : SyncIndicator
     }
 
+    /**
+     * Indicates whether the initial sliding sync request is done or not.
+     */
+    val isInitialSyncDone: Boolean
+
     /**
      * Creates a room list that can be used to load more rooms and filter them dynamically.
      * @param pageSize the number of rooms to load at once.

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/roomlist/RustRoomListService.kt

@@ -24,6 +24,7 @@ import kotlinx.coroutines.flow.stateIn
 import org.matrix.rustcomponents.sdk.RoomListServiceState
 import org.matrix.rustcomponents.sdk.RoomListServiceSyncIndicator
 import timber.log.Timber
+import java.util.concurrent.atomic.AtomicBoolean
 import org.matrix.rustcomponents.sdk.RoomListService as InnerRustRoomListService
 
 internal class RustRoomListService(
@@ -33,6 +34,9 @@ internal class RustRoomListService(
     private val roomSyncSubscriber: RoomSyncSubscriber,
     private val sessionCoroutineScope: CoroutineScope,
 ) : RoomListService {
+    private val _isInitialSyncDone = AtomicBoolean(false)
+    override val isInitialSyncDone: Boolean get() = _isInitialSyncDone.get()
+
     override fun createRoomList(
         pageSize: Int,
         source: RoomList.Source,
@@ -75,6 +79,9 @@ internal class RustRoomListService(
             .map { it.toRoomListState() }
             .onEach { state ->
                 Timber.d("RoomList state=$state")
+                if (state == RoomListService.State.Running) {
+                    _isInitialSyncDone.set(true)
+                }
             }
             .distinctUntilChanged()
             .stateIn(sessionCoroutineScope, SharingStarted.Eagerly, RoomListService.State.Idle)

libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/roomlist/FakeRoomListService.kt

@@ -20,10 +20,14 @@ class FakeRoomListService(
     private val subscribeToVisibleRoomsLambda: (List<RoomId>) -> Unit = {},
     private val createRoomListLambda: (pageSize: Int) -> DynamicRoomList = { pageSize -> FakeDynamicRoomList(pageSize = pageSize) },
     override val allRooms: RoomList = createRoomListLambda(Int.MAX_VALUE),
+    private val isInitialSyncLambda: () -> Boolean = { true },
 ) : RoomListService {
     private val roomListStateFlow = MutableStateFlow<RoomListService.State>(RoomListService.State.Idle)
     private val syncIndicatorStateFlow = MutableStateFlow<RoomListService.SyncIndicator>(RoomListService.SyncIndicator.Hide)
 
+    override val isInitialSyncDone: Boolean
+        get() = isInitialSyncLambda()
+
     suspend fun postState(state: RoomListService.State) {
         roomListStateFlow.emit(state)
     }

services/analytics/api/src/main/kotlin/io/element/android/services/analytics/api/NoopAnalyticsTransaction.kt

@@ -8,8 +8,11 @@
 package io.element.android.services.analytics.api
 
 import io.element.android.services.analyticsproviders.api.AnalyticsTransaction
+import kotlin.time.Duration
+import kotlin.time.Duration.Companion.seconds
 
 object NoopAnalyticsTransaction : AnalyticsTransaction {
+    override val duration: Duration = 0.seconds
     override fun startChild(operation: String, description: String?): AnalyticsTransaction = NoopAnalyticsTransaction
     override fun putExtraData(key: String, value: String) {}
     override fun putIndexableData(key: String, value: String) {}

services/analytics/impl/build.gradle.kts

@@ -26,6 +26,7 @@ dependencies {
     implementation(projects.libraries.architecture)
     implementation(projects.libraries.designsystem)
     implementation(projects.libraries.matrix.api)
+    implementation(projects.features.networkmonitor.api)
     implementation(projects.libraries.preferences.api)
     implementation(projects.libraries.sessionStorage.api)
     implementation(projects.services.appnavstate.api)
@@ -37,6 +38,7 @@ dependencies {
     testCommonDependencies(libs)
     testImplementation(projects.libraries.matrix.test)
     testImplementation(projects.libraries.sessionStorage.test)
+    testImplementation(projects.features.networkmonitor.test)
     testImplementation(projects.services.analytics.test)
     testImplementation(projects.services.analyticsproviders.test)
     testImplementation(projects.services.appnavstate.test)

services/analytics/impl/src/main/kotlin/io/element/android/services/analytics/impl/watchers/DefaultAnalyticsRoomListStateWatcher.kt

@@ -8,29 +8,32 @@
 package io.element.android.services.analytics.impl.watchers
 
 import dev.zacsweers.metro.ContributesBinding
+import io.element.android.features.networkmonitor.api.NetworkMonitor
+import io.element.android.features.networkmonitor.api.NetworkStatus
 import io.element.android.libraries.core.coroutine.CoroutineDispatchers
 import io.element.android.libraries.core.coroutine.childScope
-import io.element.android.libraries.core.coroutine.withPreviousValue
 import io.element.android.libraries.di.SessionScope
 import io.element.android.libraries.di.annotations.SessionCoroutineScope
 import io.element.android.libraries.matrix.api.roomlist.RoomListService
 import io.element.android.services.analytics.api.AnalyticsLongRunningTransaction
 import io.element.android.services.analytics.api.AnalyticsService
-import io.element.android.services.analytics.api.finishLongRunningTransaction
 import io.element.android.services.analytics.api.watchers.AnalyticsRoomListStateWatcher
-import io.element.android.services.appnavstate.api.AppNavigationStateService
+import io.element.android.services.appnavstate.api.AppForegroundStateService
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.cancelChildren
+import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.onEach
 import timber.log.Timber
 import java.util.concurrent.atomic.AtomicBoolean
+import kotlin.time.Duration.Companion.minutes
 
 @ContributesBinding(SessionScope::class)
 class DefaultAnalyticsRoomListStateWatcher(
-    private val appNavigationStateService: AppNavigationStateService,
+    private val appForegroundStateService: AppForegroundStateService,
+    private val networkMonitor: NetworkMonitor,
     private val roomListService: RoomListService,
     private val analyticsService: AnalyticsService,
     @SessionCoroutineScope sessionCoroutineScope: CoroutineScope,
@@ -38,7 +41,7 @@ class DefaultAnalyticsRoomListStateWatcher(
 ) : AnalyticsRoomListStateWatcher {
     private val coroutineScope: CoroutineScope = sessionCoroutineScope.childScope(dispatchers.computation, "AnalyticsRoomListStateWatcher")
     private val isStarted = AtomicBoolean(false)
-    private val isWarmState = AtomicBoolean(false)
+    private val isNotInitialSync get() = roomListService.isInitialSyncDone
 
     override fun start() {
         if (isStarted.getAndSet(true)) {
@@ -48,27 +51,40 @@ class DefaultAnalyticsRoomListStateWatcher(
 
         val longRunningTransaction = AnalyticsLongRunningTransaction.CatchUp
 
-        appNavigationStateService.appNavigationState
-            .map { it.isInForeground }
+        val hasNetworkConnectivityFlow = networkMonitor.connectivity
+            .map { it == NetworkStatus.Connected }
             .distinctUntilChanged()
-            .withPreviousValue()
-            .onEach { (wasInForeground, isInForeground) ->
-                if (isInForeground && roomListService.state.value != RoomListService.State.Running) {
-                    analyticsService.startLongRunningTransaction(longRunningTransaction)
-                } else if (!isInForeground) {
-                    analyticsService.removeLongRunningTransaction(longRunningTransaction)
-                }
 
-                if (wasInForeground == false && isInForeground) {
-                    isWarmState.set(true)
+        combine(
+            appForegroundStateService.isInForeground,
+            hasNetworkConnectivityFlow,
+        ) { isInForeground, hasNetworkConnectivity ->
+            val canSync = isInForeground && hasNetworkConnectivity
+            val isNotSyncing = roomListService.state.value != RoomListService.State.Running
+                if (isNotInitialSync && canSync && isNotSyncing) {
+                    Timber.d("Catch-up transaction: starting")
+                    analyticsService.startLongRunningTransaction(longRunningTransaction)
+                } else if (!isInForeground || !hasNetworkConnectivity) {
+                    analyticsService.removeLongRunningTransaction(longRunningTransaction)?.let {
+                        Timber.d("Catch-up transaction: stopping")
+                    }
                 }
             }
             .launchIn(coroutineScope)
 
         roomListService.state
             .onEach { state ->
-                if (state == RoomListService.State.Running && isWarmState.get()) {
-                    analyticsService.finishLongRunningTransaction(longRunningTransaction)
+                if (state == RoomListService.State.Running && isNotInitialSync) {
+                    val transaction = analyticsService.removeLongRunningTransaction(longRunningTransaction)
+                    if (transaction != null && !transaction.isFinished()) {
+                        val duration = transaction.duration
+                        if (duration > 3.minutes) {
+                            Timber.d("Cancelling catch-up transaction, the elapsed time is too long ($duration), something probably went wrong while measuring")
+                        } else {
+                            Timber.d("Catch-up transaction finished in $duration")
+                            transaction.finish()
+                        }
+                    }
                 }
             }
             .launchIn(coroutineScope)

services/analytics/impl/src/test/kotlin/io/element/android/services/analytics/impl/watchers/DefaultAnalyticsRoomListStateWatcherTest.kt

@@ -8,13 +8,12 @@
 package io.element.android.services.analytics.impl.watchers
 
 import com.google.common.truth.Truth.assertThat
+import io.element.android.features.networkmonitor.test.FakeNetworkMonitor
 import io.element.android.libraries.matrix.api.roomlist.RoomListService
 import io.element.android.libraries.matrix.test.roomlist.FakeRoomListService
 import io.element.android.services.analytics.api.AnalyticsLongRunningTransaction.CatchUp
 import io.element.android.services.analytics.test.FakeAnalyticsService
-import io.element.android.services.appnavstate.api.AppNavigationState
-import io.element.android.services.appnavstate.api.NavigationState
-import io.element.android.services.appnavstate.test.FakeAppNavigationStateService
+import io.element.android.services.appnavstate.test.FakeAppForegroundStateService
 import io.element.android.tests.testutils.testCoroutineDispatchers
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.test.TestScope
@@ -26,13 +25,13 @@ import org.junit.Test
 class DefaultAnalyticsRoomListStateWatcherTest {
     @Test
     fun `Opening the app in a warm state tracks the time until the room list is synced`() = runTest {
-        val navigationStateService = FakeAppNavigationStateService()
+        val appForegroundStateService = FakeAppForegroundStateService()
         val roomListService = FakeRoomListService().apply {
             postState(RoomListService.State.Idle)
         }
         val analyticsService = FakeAnalyticsService()
         val watcher = createAnalyticsRoomListStateWatcher(
-            appNavigationStateService = navigationStateService,
+            appForegroundStateService = appForegroundStateService,
             roomListService = roomListService,
             analyticsService = analyticsService,
         )
@@ -43,9 +42,9 @@ class DefaultAnalyticsRoomListStateWatcherTest {
         runCurrent()
 
         // Make sure it's warm by changing its internal state
-        navigationStateService.emitNavigationState(AppNavigationState(navigationState = NavigationState.Root, isInForeground = false))
+        appForegroundStateService.givenIsInForeground(false)
         runCurrent()
-        navigationStateService.emitNavigationState(AppNavigationState(navigationState = NavigationState.Root, isInForeground = true))
+        appForegroundStateService.givenIsInForeground(true)
         runCurrent()
 
         // The transaction should be present now
@@ -63,15 +62,15 @@ class DefaultAnalyticsRoomListStateWatcherTest {
 
     @Test
     fun `Opening the app in a cold state does nothing`() = runTest {
-        val navigationStateService = FakeAppNavigationStateService(
-            initialAppNavigationState = AppNavigationState(NavigationState.Root, false)
+        val appForegroundStateService = FakeAppForegroundStateService(
+            initialForegroundValue = false
         )
         val roomListService = FakeRoomListService().apply {
             postState(RoomListService.State.Idle)
         }
         val analyticsService = FakeAnalyticsService()
         val watcher = createAnalyticsRoomListStateWatcher(
-            appNavigationStateService = navigationStateService,
+            appForegroundStateService = appForegroundStateService,
             roomListService = roomListService,
             analyticsService = analyticsService,
         )
@@ -93,13 +92,13 @@ class DefaultAnalyticsRoomListStateWatcherTest {
 
     @Test
     fun `The transaction won't be finished until the room list is synchronised`() = runTest {
-        val navigationStateService = FakeAppNavigationStateService()
+        val appForegroundStateService = FakeAppForegroundStateService()
         val roomListService = FakeRoomListService().apply {
             postState(RoomListService.State.Idle)
         }
         val analyticsService = FakeAnalyticsService()
         val watcher = createAnalyticsRoomListStateWatcher(
-            appNavigationStateService = navigationStateService,
+            appForegroundStateService = appForegroundStateService,
             roomListService = roomListService,
             analyticsService = analyticsService,
         )
@@ -110,9 +109,9 @@ class DefaultAnalyticsRoomListStateWatcherTest {
         runCurrent()
 
         // Make sure it's warm by changing its internal state
-        navigationStateService.emitNavigationState(AppNavigationState(navigationState = NavigationState.Root, isInForeground = false))
+        appForegroundStateService.givenIsInForeground(false)
         runCurrent()
-        navigationStateService.emitNavigationState(AppNavigationState(navigationState = NavigationState.Root, isInForeground = true))
+        appForegroundStateService.givenIsInForeground(true)
         runCurrent()
 
         // The transaction should be present now
@@ -128,13 +127,13 @@ class DefaultAnalyticsRoomListStateWatcherTest {
 
     @Test
     fun `Opening the app when the room list state was already Running does nothing`() = runTest {
-        val navigationStateService = FakeAppNavigationStateService()
+        val appForegroundStateService = FakeAppForegroundStateService()
         val roomListService = FakeRoomListService().apply {
             postState(RoomListService.State.Running)
         }
         val analyticsService = FakeAnalyticsService()
         val watcher = createAnalyticsRoomListStateWatcher(
-            appNavigationStateService = navigationStateService,
+            appForegroundStateService = appForegroundStateService,
             roomListService = roomListService,
             analyticsService = analyticsService,
         )
@@ -145,9 +144,9 @@ class DefaultAnalyticsRoomListStateWatcherTest {
         runCurrent()
 
         // Make sure it's warm by changing its internal state
-        navigationStateService.emitNavigationState(AppNavigationState(navigationState = NavigationState.Root, isInForeground = false))
+        appForegroundStateService.givenIsInForeground(false)
         runCurrent()
-        navigationStateService.emitNavigationState(AppNavigationState(navigationState = NavigationState.Root, isInForeground = true))
+        appForegroundStateService.givenIsInForeground(true)
         runCurrent()
 
         // The transaction was never added
@@ -157,14 +156,16 @@ class DefaultAnalyticsRoomListStateWatcherTest {
     }
 
     private fun TestScope.createAnalyticsRoomListStateWatcher(
-        appNavigationStateService: FakeAppNavigationStateService = FakeAppNavigationStateService(),
+        appForegroundStateService: FakeAppForegroundStateService = FakeAppForegroundStateService(),
         roomListService: FakeRoomListService = FakeRoomListService(),
         analyticsService: FakeAnalyticsService = FakeAnalyticsService(),
+        networkMonitor: FakeNetworkMonitor = FakeNetworkMonitor(),
     ) = DefaultAnalyticsRoomListStateWatcher(
-        appNavigationStateService = appNavigationStateService,
+        appForegroundStateService = appForegroundStateService,
         roomListService = roomListService,
         analyticsService = analyticsService,
         sessionCoroutineScope = backgroundScope,
         dispatchers = testCoroutineDispatchers(),
+        networkMonitor = networkMonitor,
     )
 }

services/analyticsproviders/api/src/main/kotlin/io/element/android/services/analyticsproviders/api/AnalyticsTransaction.kt

@@ -7,7 +7,14 @@
 
 package io.element.android.services.analyticsproviders.api
 
+import kotlin.time.Duration
+
 interface AnalyticsTransaction {
+    /**
+     * The time elapsed since the transaction started until now if the transaction is ongoing or the time it finished.
+     */
+    val duration: Duration
+
     /**
      * Start a child span from this transaction.
      */

services/analyticsproviders/sentry/src/main/kotlin/io/element/android/services/analyticsproviders/sentry/SentryAnalyticsTransaction.kt

@@ -11,7 +11,10 @@ import io.element.android.services.analyticsproviders.api.AnalyticsTransaction
 import io.sentry.ISpan
 import io.sentry.ITransaction
 import io.sentry.Sentry
+import io.sentry.SentryInstantDate
 import timber.log.Timber
+import kotlin.time.Duration
+import kotlin.time.Duration.Companion.nanoseconds
 
 class SentryAnalyticsTransaction private constructor(span: ISpan) : AnalyticsTransaction {
     constructor(name: String, operation: String?, description: String? = null) : this(
@@ -19,6 +22,11 @@ class SentryAnalyticsTransaction private constructor(span: ISpan) : AnalyticsTra
     )
     private val inner = span
 
+    @Suppress("UnstableApiUsage")
+    override val duration: Duration get() {
+        return (inner.finishDate ?: SentryInstantDate()).diff(inner.startDate).nanoseconds
+    }
+
     override fun startChild(operation: String, description: String?): AnalyticsTransaction = SentryAnalyticsTransaction(
         inner.startChild(operation, description)
     )