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

Enhance missing uploads monitoring on Sentry #325

Merged
merged 1 commit into from
Dec 3, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -20,7 +19,7 @@ class CheckSkipAutoRunNotUploadedLimitTest {
CheckSkipAutoRunNotUploadedLimit(
countResultsMissingUpload = { flowOf(count) },
getPreferenceValueByKey = { flowOf(limit) },
)().first(),
)(),
)
}

Expand Down