diff --git a/CHANGELOG.md b/CHANGELOG.md index 33c9eec938..fe6a553dbd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# Unreleased + +* [IMPROVEMENT] Limit each Flags assignment request to one second and retry transient failures once by default. Add timeout and retry-count configuration. + # 3.12.1 / 2026-07-16 * [BUGFIX] Fix R8 failures due to missing `SourceLines` annotation. See [#3642](https://github.com/DataDog/dd-sdk-android/pull/3642) diff --git a/features/dd-sdk-android-flags/README.md b/features/dd-sdk-android-flags/README.md index d302cb693b..2dadba3435 100644 --- a/features/dd-sdk-android-flags/README.md +++ b/features/dd-sdk-android-flags/README.md @@ -93,6 +93,17 @@ val flagsConfig = FlagsConfiguration.Builder() .build() ``` +#### Configure assignment request limits + +Each assignment request has a one-second timeout and one retry by default. You can change both limits: + +```kotlin +val flagsConfig = FlagsConfiguration.Builder() + .assignmentRequestTimeout(2_000) + .assignmentRequestRetryCount(2) + .build() +``` + ## Use the Feature Flags SDK ### Create a Flags client diff --git a/features/dd-sdk-android-flags/api/apiSurface b/features/dd-sdk-android-flags/api/apiSurface index 8f084a7096..f8db05e2a1 100644 --- a/features/dd-sdk-android-flags/api/apiSurface +++ b/features/dd-sdk-android-flags/api/apiSurface @@ -26,6 +26,8 @@ data class com.datadog.android.flags.FlagsConfiguration fun useCustomEvaluationEndpoint(String): Builder fun evaluationFlushInterval(Long): Builder fun useCustomFlagEndpoint(String): Builder + fun assignmentRequestTimeout(Long): Builder + fun assignmentRequestRetryCount(Int): Builder fun rumIntegrationEnabled(Boolean): Builder fun gracefulModeEnabled(Boolean): Builder fun build(): FlagsConfiguration diff --git a/features/dd-sdk-android-flags/api/dd-sdk-android-flags.api b/features/dd-sdk-android-flags/api/dd-sdk-android-flags.api index d73cd91762..d068530ed7 100644 --- a/features/dd-sdk-android-flags/api/dd-sdk-android-flags.api +++ b/features/dd-sdk-android-flags/api/dd-sdk-android-flags.api @@ -54,6 +54,8 @@ public final class com/datadog/android/flags/FlagsConfiguration { public final class com/datadog/android/flags/FlagsConfiguration$Builder { public fun ()V + public final fun assignmentRequestRetryCount (I)Lcom/datadog/android/flags/FlagsConfiguration$Builder; + public final fun assignmentRequestTimeout (J)Lcom/datadog/android/flags/FlagsConfiguration$Builder; public final fun build ()Lcom/datadog/android/flags/FlagsConfiguration; public final fun evaluationFlushInterval (J)Lcom/datadog/android/flags/FlagsConfiguration$Builder; public final fun gracefulModeEnabled (Z)Lcom/datadog/android/flags/FlagsConfiguration$Builder; diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsClient.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsClient.kt index 4aaa48e9b7..b9044d7d2a 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsClient.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsClient.kt @@ -411,7 +411,9 @@ interface FlagsClient { val assignmentsDownloader = PrecomputedAssignmentsDownloader( internalLogger = featureSdkCore.internalLogger, callFactory = callFactory, - requestFactory = flagsFeature.precomputedRequestFactory + requestFactory = flagsFeature.precomputedRequestFactory, + requestTimeoutMs = configuration.assignmentRequestTimeoutMs, + requestRetryCount = configuration.assignmentRequestRetryCount ) val precomputeMapper = PrecomputeMapper(featureSdkCore.internalLogger) diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt index cc41f61e68..b03fbd058e 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt @@ -6,6 +6,9 @@ package com.datadog.android.flags +private const val DEFAULT_ASSIGNMENT_REQUEST_TIMEOUT_MS = 1_000L +private const val DEFAULT_ASSIGNMENT_REQUEST_RETRY_COUNT = 1 + /** * Describes configuration to be used for the Flags feature. */ @@ -19,15 +22,21 @@ data class FlagsConfiguration internal constructor( internal val rumIntegrationEnabled: Boolean, internal val gracefulModeEnabled: Boolean ) { + internal var assignmentRequestTimeoutMs: Long = DEFAULT_ASSIGNMENT_REQUEST_TIMEOUT_MS + internal var assignmentRequestRetryCount: Int = DEFAULT_ASSIGNMENT_REQUEST_RETRY_COUNT + /** * A Builder class for a [FlagsConfiguration]. */ + @Suppress("TooManyFunctions") class Builder { private var trackExposures: Boolean = true private var trackEvaluations: Boolean = true private var customExposureEndpoint: String? = null private var customEvaluationEndpoint: String? = null private var customFlagEndpoint: String? = null + private var assignmentRequestTimeoutMs: Long = DEFAULT_ASSIGNMENT_REQUEST_TIMEOUT_MS + private var assignmentRequestRetryCount: Int = DEFAULT_ASSIGNMENT_REQUEST_RETRY_COUNT private var evaluationFlushIntervalMs: Long = DEFAULT_EVALUATION_FLUSH_INTERVAL_MS private var rumIntegrationEnabled: Boolean = true private var gracefulModeEnabled: Boolean = true @@ -115,6 +124,34 @@ data class FlagsConfiguration internal constructor( return this } + /** + * Sets the timeout for each precomputed assignment request. + * Values less than or equal to zero use the default timeout of 1,000 milliseconds. + * + * @param timeoutMs The timeout for each request, in milliseconds. + * @return this [Builder] instance for method chaining. + */ + fun assignmentRequestTimeout(timeoutMs: Long): Builder { + assignmentRequestTimeoutMs = if (timeoutMs > 0) { + timeoutMs + } else { + DEFAULT_ASSIGNMENT_REQUEST_TIMEOUT_MS + } + return this + } + + /** + * Sets the number of retries after a transient precomputed assignment request failure. + * Negative values are treated as zero. + * + * @param retryCount The number of retries after the first attempt. + * @return this [Builder] instance for method chaining. + */ + fun assignmentRequestRetryCount(retryCount: Int): Builder { + assignmentRequestRetryCount = retryCount.coerceAtLeast(0) + return this + } + /** * Sets whether RUM evaluation logging is enabled. * This adds the result of evaluating a feature flag to the view. @@ -160,7 +197,10 @@ data class FlagsConfiguration internal constructor( evaluationFlushIntervalMs = evaluationFlushIntervalMs, rumIntegrationEnabled = rumIntegrationEnabled, gracefulModeEnabled = gracefulModeEnabled - ) + ).also { + it.assignmentRequestTimeoutMs = assignmentRequestTimeoutMs + it.assignmentRequestRetryCount = assignmentRequestRetryCount + } internal companion object { private const val DEFAULT_EVALUATION_FLUSH_INTERVAL_MS = 10_000L // 10 seconds diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt index 65311982c4..40c046c5e9 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt @@ -12,7 +12,8 @@ import com.datadog.android.api.context.DatadogContext import com.datadog.android.flags.model.EvaluationContext import okhttp3.Call import okhttp3.Request -import okhttp3.Response +import java.io.IOException +import java.util.concurrent.TimeUnit /** * Downloads precomputed flag assignments from Datadog Feature Flags service. @@ -20,11 +21,15 @@ import okhttp3.Response * @param callFactory Factory for creating HTTP calls * @param internalLogger Logger for error and debug messages * @param requestFactory Factory for creating precomputed assignments requests + * @param requestTimeoutMs Timeout for each request, in milliseconds + * @param requestRetryCount Number of retries after the first attempt */ internal class PrecomputedAssignmentsDownloader( private val callFactory: Call.Factory, private val internalLogger: InternalLogger, - private val requestFactory: PrecomputedAssignmentsRequestFactory + private val requestFactory: PrecomputedAssignmentsRequestFactory, + private val requestTimeoutMs: Long = 1_000L, + private val requestRetryCount: Int = 1 ) : PrecomputedAssignmentsReader { @WorkerThread @@ -34,40 +39,84 @@ internal class PrecomputedAssignmentsDownloader( return executeDownloadRequest(request) } - @Suppress("TooGenericExceptionCaught") - private fun executeDownloadRequest(request: Request): String? = try { - val response = callFactory.newCall(request).execute() - handleResponse(response) + private fun executeDownloadRequest(request: Request): String? { + var attempt = 0 + var result: DownloadResult + do { + result = executeSingleRequest(request) + attempt++ + } while (result.isRetryable && attempt <= requestRetryCount) + + return when (result) { + is DownloadResult.Success -> result.body + is DownloadResult.HttpFailure -> { + internalLogger.log( + InternalLogger.Level.ERROR, + InternalLogger.Target.MAINTAINER, + { "Failed to download flags: ${result.statusCode}" } + ) + internalLogger.log( + level = InternalLogger.Level.ERROR, + target = InternalLogger.Target.TELEMETRY, + messageBuilder = { "Flag assignment server returned error (${result.statusCode})" }, + onlyOnce = true + ) + null + } + is DownloadResult.UnexpectedFailure -> { + internalLogger.log( + InternalLogger.Level.ERROR, + InternalLogger.Target.MAINTAINER, + { "Unexpected error while downloading flags" }, + result.throwable + ) + null + } + } + } + + @Suppress("TooGenericExceptionCaught", "UnsafeThirdPartyFunctionCall") + private fun executeSingleRequest(request: Request): DownloadResult = try { + val call = callFactory.newCall(request) + call.timeout().timeout(requestTimeoutMs, TimeUnit.MILLISECONDS) + val response = call.execute() + if (response.isSuccessful) { + DownloadResult.Success(response.body?.use { it.string() }) + } else { + val statusCode = response.code + response.body?.close() + DownloadResult.HttpFailure(statusCode, isRetryableStatus(statusCode)) + } + } catch (e: IOException) { + DownloadResult.UnexpectedFailure(e, isRetryable = true) } catch (e: Throwable) { - internalLogger.log( - InternalLogger.Level.ERROR, - InternalLogger.Target.MAINTAINER, - { "Unexpected error while downloading flags" }, - e - ) - null + DownloadResult.UnexpectedFailure(e, isRetryable = false) } - private fun handleResponse(response: Response): String? = if (response.isSuccessful) { - @Suppress("UnsafeThirdPartyFunctionCall") // Safe: wrapped in outer try-catch - response.body?.use { it.string() } - } else { - internalLogger.log( - InternalLogger.Level.ERROR, - InternalLogger.Target.MAINTAINER, - { "Failed to download flags: ${response.code}" } - ) + private fun isRetryableStatus(statusCode: Int): Boolean = + statusCode == HTTP_REQUEST_TIMEOUT || + statusCode == HTTP_TOO_MANY_REQUESTS || + statusCode in HTTP_SERVER_ERROR_MIN..HTTP_SERVER_ERROR_MAX - internalLogger.log( - level = InternalLogger.Level.ERROR, - target = InternalLogger.Target.TELEMETRY, - messageBuilder = { "Flag assignment server returned error (${response.code})" }, - onlyOnce = true - ) + private sealed interface DownloadResult { + val isRetryable: Boolean - @Suppress("UnsafeThirdPartyFunctionCall") // Safe: wrapped in outer try-catch - response.body?.close() + data class Success(val body: String?) : DownloadResult { + override val isRetryable: Boolean = false + } + + data class HttpFailure(val statusCode: Int, override val isRetryable: Boolean) : DownloadResult + + data class UnexpectedFailure( + val throwable: Throwable, + override val isRetryable: Boolean + ) : DownloadResult + } - null + private companion object { + const val HTTP_REQUEST_TIMEOUT = 408 + const val HTTP_TOO_MANY_REQUESTS = 429 + const val HTTP_SERVER_ERROR_MIN = 500 + const val HTTP_SERVER_ERROR_MAX = 599 } } diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/FlagsConfigurationTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/FlagsConfigurationTest.kt index 245a973270..01fab2de25 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/FlagsConfigurationTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/FlagsConfigurationTest.kt @@ -29,6 +29,8 @@ internal class FlagsConfigurationTest { assertThat(configuration.customExposureEndpoint).isNull() assertThat(configuration.customFlagEndpoint).isNull() assertThat(configuration.gracefulModeEnabled).isTrue() + assertThat(configuration.assignmentRequestTimeoutMs).isEqualTo(1_000L) + assertThat(configuration.assignmentRequestRetryCount).isEqualTo(1) } @Test @@ -142,5 +144,31 @@ internal class FlagsConfigurationTest { assertThat(returnedBuilder).isSameAs(builder) } + @Test + fun `M set assignment request limits W Builder`() { + // When + val configuration = FlagsConfiguration.Builder() + .assignmentRequestTimeout(2_500L) + .assignmentRequestRetryCount(3) + .build() + + // Then + assertThat(configuration.assignmentRequestTimeoutMs).isEqualTo(2_500L) + assertThat(configuration.assignmentRequestRetryCount).isEqualTo(3) + } + + @Test + fun `M sanitize invalid assignment request limits W Builder`() { + // When + val configuration = FlagsConfiguration.Builder() + .assignmentRequestTimeout(0) + .assignmentRequestRetryCount(-1) + .build() + + // Then + assertThat(configuration.assignmentRequestTimeoutMs).isEqualTo(1_000L) + assertThat(configuration.assignmentRequestRetryCount).isZero() + } + // endregion } diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt index 31a0c48a4a..d54de0b39d 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt @@ -24,6 +24,7 @@ import okhttp3.Request import okhttp3.Response import okhttp3.ResponseBody import okhttp3.ResponseBody.Companion.toResponseBody +import okio.Timeout import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test @@ -38,12 +39,14 @@ import org.mockito.kotlin.doThrow import org.mockito.kotlin.eq import org.mockito.kotlin.isA import org.mockito.kotlin.mock +import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.mockito.quality.Strictness import java.io.IOException import java.net.UnknownHostException import java.util.UUID +import java.util.concurrent.TimeUnit @Extensions( ExtendWith(MockitoExtension::class), @@ -65,6 +68,9 @@ internal class PrecomputedAssignmentsDownloaderTest { @Mock lateinit var mockCall: Call + @Mock + lateinit var mockTimeout: Timeout + private lateinit var testedDownloader: PrecomputedAssignmentsDownloader @Forgery @@ -91,6 +97,8 @@ internal class PrecomputedAssignmentsDownloaderTest { internalLogger = mockInternalLogger, requestFactory = mockRequestFactory ) + whenever(mockCall.timeout()).doReturn(mockTimeout) + whenever(mockTimeout.timeout(1_000L, TimeUnit.MILLISECONDS)).doReturn(mockTimeout) } // region readPrecomputedFlags() - Success cases @@ -229,6 +237,7 @@ internal class PrecomputedAssignmentsDownloaderTest { ) assertThat(messageCaptor.firstValue.invoke()) .isEqualTo("Unexpected error while downloading flags") + verify(mockCallFactory, times(2)).newCall(fakeRequest) } @Test @@ -346,6 +355,56 @@ internal class PrecomputedAssignmentsDownloaderTest { ) assertThat(telemetryMessageCaptor.firstValue.invoke()) .isEqualTo("Flag assignment server returned error (404)") + verify(mockCallFactory).newCall(fakeRequest) + } + + @Test + fun `M retry and return response W readPrecomputedFlags() { transient response }`( + @StringForgery fakeResponseBody: String, + @StringForgery(regex = "https://[a-z]+\\.(com|net)/[a-z]+") fakeUrl: String + ) { + // Given + val fakeRequest = Request.Builder().url(fakeUrl).build() + val failedResponse = createUnsuccessfulResponse(500, fakeUrl) + val successfulResponse = createSuccessfulResponse(fakeResponseBody, fakeUrl) + whenever(mockRequestFactory.create(fakeEvaluationContext, fakeDatadogContext)).doReturn(fakeRequest) + whenever(mockCallFactory.newCall(fakeRequest)).doReturn(mockCall) + whenever(mockCall.execute()).doReturn(failedResponse, successfulResponse) + + // When + val result = testedDownloader.readPrecomputedFlags(fakeEvaluationContext, fakeDatadogContext) + + // Then + assertThat(result).isEqualTo(fakeResponseBody) + verify(mockCallFactory, times(2)).newCall(fakeRequest) + } + + @Test + fun `M use custom request limits W readPrecomputedFlags() { transient failures }`( + @StringForgery(regex = "https://[a-z]+\\.(com|net)/[a-z]+") fakeUrl: String + ) { + // Given + val fakeRequest = Request.Builder().url(fakeUrl).build() + val customTimeout = mock() + testedDownloader = PrecomputedAssignmentsDownloader( + callFactory = mockCallFactory, + internalLogger = mockInternalLogger, + requestFactory = mockRequestFactory, + requestTimeoutMs = 2_500L, + requestRetryCount = 2 + ) + whenever(mockRequestFactory.create(fakeEvaluationContext, fakeDatadogContext)).doReturn(fakeRequest) + whenever(mockCallFactory.newCall(fakeRequest)).doReturn(mockCall) + whenever(mockCall.timeout()).doReturn(customTimeout) + whenever(customTimeout.timeout(2_500L, TimeUnit.MILLISECONDS)).doReturn(customTimeout) + whenever(mockCall.execute()).doThrow(IOException("timeout")) + + // When + testedDownloader.readPrecomputedFlags(fakeEvaluationContext, fakeDatadogContext) + + // Then + verify(mockCallFactory, times(3)).newCall(fakeRequest) + verify(customTimeout, times(3)).timeout(2_500L, TimeUnit.MILLISECONDS) } @Test