From 2599e84a1669b8d9be85c18d4a05ae82fdc01fe5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Santos?= Date: Mon, 2 Dec 2024 16:23:32 +0000 Subject: [PATCH] Enhance missing uploads monitoring on Sentry --- .../probe/background/RunBackgroundTask.kt | 4 +-- .../CheckSkipAutoRunNotUploadedLimit.kt | 32 +++++++++++++------ .../probe/domain/UploadMissingMeasurements.kt | 4 +++ .../probe/background/RunBackgroundTaskTest.kt | 2 +- .../CheckSkipAutoRunNotUploadedLimitTest.kt | 3 +- 5 files changed, 30 insertions(+), 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 index 734422a4..9ee8e1d8 100644 --- 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, private val uploadMissingMeasurements: (ResultModel.Id?) -> Flow, - private val checkSkipAutoRunNotUploadedLimit: () -> Flow, + 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.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 index b5e4bc5c..98bd8048 100644 --- 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, private val getPreferenceValueByKey: (SettingsKey) -> Flow, ) { - operator fun invoke(): Flow = - 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 index 1f286e99..7ec9e57e 100644 --- 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 index d77295fa..88895914 100644 --- 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 = { flowOf(true) }, uploadMissingMeasurements: (ResultModel.Id?) -> Flow = { emptyFlow() }, - checkSkipAutoRunNotUploadedLimit: () -> Flow = { flowOf(false) }, + checkSkipAutoRunNotUploadedLimit: suspend () -> Boolean = { false }, getNetworkType: () -> NetworkType = { NetworkType.Wifi }, getAutoRunSpecification: suspend () -> RunSpecification.Full = { RunSpecification.Full( diff --git a/composeApp/src/commonTest/kotlin/org/ooni/probe/domain/CheckSkipAutoRunNotUploadedLimitTest.kt b/composeApp/src/commonTest/kotlin/org/ooni/probe/domain/CheckSkipAutoRunNotUploadedLimitTest.kt index f24e0fe5..0718d498 100644 --- a/composeApp/src/commonTest/kotlin/org/ooni/probe/domain/CheckSkipAutoRunNotUploadedLimitTest.kt +++ b/composeApp/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(), + )(), ) }