Enhance missing uploads monitoring on Sentry

ooni · Dec 2, 2024 · 2599e84 · 2599e84
1 parent 11ccc98
commit 2599e84
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 15 deletions.
diff --git a/composeApp/src/commonMain/kotlin/org/ooni/probe/background/RunBackgroundTask.kt b/composeApp/src/commonMain/kotlin/org/ooni/probe/background/RunBackgroundTask.kt
@@ -26,7 +26,7 @@ import org.ooni.probe.domain.UploadMissingMeasurements
 class RunBackgroundTask(
     private val getPreferenceValueByKey: (SettingsKey) -> Flow<Any?>,
     private val uploadMissingMeasurements: (ResultModel.Id?) -> Flow<UploadMissingMeasurements.State>,
-    private val checkSkipAutoRunNotUploadedLimit: () -> Flow<Boolean>,
+    private val checkSkipAutoRunNotUploadedLimit: suspend () -> Boolean,
     private val getNetworkType: () -> NetworkType,
     private val getAutoRunSpecification: suspend () -> RunSpecification.Full,
     private val runDescriptors: suspend (RunSpecification.Full) -> Unit,
@@ -106,7 +106,7 @@ class RunBackgroundTask(
     }
 
     private suspend fun ProducerScope<RunBackgroundState>.runTests(spec: RunSpecification.Full?) {
-        if (checkSkipAutoRunNotUploadedLimit().first()) {
+        if (checkSkipAutoRunNotUploadedLimit()) {
             Logger.i("Skipping auto-run tests: too many not-uploaded results")
             return
         }

diff --git a/composeApp/src/commonMain/kotlin/org/ooni/probe/domain/CheckSkipAutoRunNotUploadedLimit.kt b/composeApp/src/commonMain/kotlin/org/ooni/probe/domain/CheckSkipAutoRunNotUploadedLimit.kt
@@ -1,21 +1,33 @@
 package org.ooni.probe.domain
 
+import co.touchlab.kermit.Logger
 import kotlinx.coroutines.flow.Flow
-import kotlinx.coroutines.flow.combine
+import kotlinx.coroutines.flow.first
 import org.ooni.probe.data.models.SettingsKey
 
 class CheckSkipAutoRunNotUploadedLimit(
     private val countResultsMissingUpload: () -> Flow<Long>,
     private val getPreferenceValueByKey: (SettingsKey) -> Flow<Any?>,
 ) {
-    operator fun invoke(): Flow<Boolean> =
-        combine(
-            getPreferenceValueByKey(SettingsKey.AUTOMATED_TESTING_NOT_UPLOADED_LIMIT),
-            countResultsMissingUpload(),
-        ) { limit, count ->
-            val notUploadedLimit = (limit as? Int)
-                ?.coerceAtLeast(1)
-                ?: BootstrapPreferences.NOT_UPLOADED_LIMIT_DEFAULT
-            count >= notUploadedLimit
+    suspend operator fun invoke(): Boolean {
+        val limitPreference =
+            getPreferenceValueByKey(SettingsKey.AUTOMATED_TESTING_NOT_UPLOADED_LIMIT).first()
+        val count = countResultsMissingUpload().first()
+
+        val notUploadedLimit = (limitPreference as? Int)
+            ?.coerceAtLeast(1)
+            ?: BootstrapPreferences.NOT_UPLOADED_LIMIT_DEFAULT
+        val shouldSkip = count >= notUploadedLimit
+
+        if (shouldSkip) {
+            Logger.w(
+                "Skipping auto-run due to not uploaded limit",
+                SkipAutoRunException("Results missing upload: $count (limit=$notUploadedLimit)"),
+            )
         }
+
+        return shouldSkip
+    }
 }
+
+class SkipAutoRunException(message: String) : Exception(message)
diff --git a/composeApp/src/commonMain/kotlin/org/ooni/probe/domain/UploadMissingMeasurements.kt b/composeApp/src/commonMain/kotlin/org/ooni/probe/domain/UploadMissingMeasurements.kt
@@ -29,6 +29,10 @@ class UploadMissingMeasurements(
             var uploaded = 0
             var failedToUpload = 0
 
+            if (total > 0) {
+                Logger.i("Uploading missing measurements: $total")
+            }
+
             measurements.forEach { measurement ->
                 if (!isActive) return@channelFlow // Check is coroutine was cancelled
 

diff --git a/composeApp/src/commonTest/kotlin/org/ooni/probe/background/RunBackgroundTaskTest.kt b/composeApp/src/commonTest/kotlin/org/ooni/probe/background/RunBackgroundTaskTest.kt
@@ -70,7 +70,7 @@ class RunBackgroundTaskTest {
     private fun buildSubject(
         getPreferenceValueByKey: (SettingsKey) -> Flow<Any?> = { flowOf(true) },
         uploadMissingMeasurements: (ResultModel.Id?) -> Flow<UploadMissingMeasurements.State> = { emptyFlow() },
-        checkSkipAutoRunNotUploadedLimit: () -> Flow<Boolean> = { flowOf(false) },
+        checkSkipAutoRunNotUploadedLimit: suspend () -> Boolean = { false },
         getNetworkType: () -> NetworkType = { NetworkType.Wifi },
         getAutoRunSpecification: suspend () -> RunSpecification.Full = {
             RunSpecification.Full(

diff --git a/...seApp/src/commonTest/kotlin/org/ooni/probe/domain/CheckSkipAutoRunNotUploadedLimitTest.kt b/...seApp/src/commonTest/kotlin/org/ooni/probe/domain/CheckSkipAutoRunNotUploadedLimitTest.kt
@@ -1,6 +1,5 @@
 package org.ooni.probe.domain
 
-import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.flow.flowOf
 import kotlinx.coroutines.test.runTest
 import kotlin.test.Test
@@ -20,7 +19,7 @@ class CheckSkipAutoRunNotUploadedLimitTest {
                     CheckSkipAutoRunNotUploadedLimit(
                         countResultsMissingUpload = { flowOf(count) },
                         getPreferenceValueByKey = { flowOf(limit) },
-                    )().first(),
+                    )(),
                 )
             }