From fc8ac7f8aac7105ea63e395b8650f7afb0a394b2 Mon Sep 17 00:00:00 2001 From: Brian Giori Date: Tue, 18 Jun 2024 09:13:42 -0700 Subject: [PATCH 01/15] wip --- src/main/kotlin/ExperimentalApi.kt | 18 +++++++ src/main/kotlin/LocalEvaluationConfig.kt | 59 ++++++++++++++++++++- src/main/kotlin/cohort/Cohort.kt | 9 ++++ src/main/kotlin/cohort/CohortDownloadApi.kt | 5 ++ 4 files changed, 89 insertions(+), 2 deletions(-) create mode 100644 src/main/kotlin/ExperimentalApi.kt create mode 100644 src/main/kotlin/cohort/Cohort.kt create mode 100644 src/main/kotlin/cohort/CohortDownloadApi.kt diff --git a/src/main/kotlin/ExperimentalApi.kt b/src/main/kotlin/ExperimentalApi.kt new file mode 100644 index 0000000..e17cc44 --- /dev/null +++ b/src/main/kotlin/ExperimentalApi.kt @@ -0,0 +1,18 @@ +package com.amplitude.experiment + +@RequiresOptIn(level = RequiresOptIn.Level.WARNING) +@Retention(AnnotationRetention.BINARY) +@Target( + AnnotationTarget.CLASS, + AnnotationTarget.ANNOTATION_CLASS, + AnnotationTarget.PROPERTY, + AnnotationTarget.FIELD, + AnnotationTarget.LOCAL_VARIABLE, + AnnotationTarget.VALUE_PARAMETER, + AnnotationTarget.CONSTRUCTOR, + AnnotationTarget.FUNCTION, + AnnotationTarget.PROPERTY_GETTER, + AnnotationTarget.PROPERTY_SETTER, + AnnotationTarget.TYPEALIAS +) +annotation class ExperimentalApi diff --git a/src/main/kotlin/LocalEvaluationConfig.kt b/src/main/kotlin/LocalEvaluationConfig.kt index 692db2f..d1f00c4 100644 --- a/src/main/kotlin/LocalEvaluationConfig.kt +++ b/src/main/kotlin/LocalEvaluationConfig.kt @@ -1,5 +1,9 @@ +@file:OptIn(ExperimentalApi::class) + package com.amplitude.experiment +import com.amplitude.Middleware + /** * Configuration options. This is an immutable object that can be created using * a [LocalEvaluationConfig.Builder]. Example usage: @@ -17,6 +21,10 @@ class LocalEvaluationConfig internal constructor( val flagConfigPollerRequestTimeoutMillis: Long = Defaults.FLAG_CONFIG_POLLER_REQUEST_TIMEOUT_MILLIS, @JvmField val assignmentConfiguration: AssignmentConfiguration? = Defaults.ASSIGNMENT_CONFIGURATION, + @JvmField + val cohortSyncConfiguration: CohortSyncConfiguration? = Defaults.COHORT_SYNC_CONFIGURATION, + @JvmField + val metrics: LocalEvaluationMetrics? = Defaults.LOCAL_EVALUATION_METRICS, ) { /** @@ -53,6 +61,16 @@ class LocalEvaluationConfig internal constructor( * null */ val ASSIGNMENT_CONFIGURATION: AssignmentConfiguration? = null + + /** + * null + */ + val COHORT_SYNC_CONFIGURATION: CohortSyncConfiguration? = null + + /** + * null + */ + val LOCAL_EVALUATION_METRICS: LocalEvaluationMetrics? = null } companion object { @@ -69,6 +87,8 @@ class LocalEvaluationConfig internal constructor( private var flagConfigPollerIntervalMillis = Defaults.FLAG_CONFIG_POLLER_INTERVAL_MILLIS private var flagConfigPollerRequestTimeoutMillis = Defaults.FLAG_CONFIG_POLLER_REQUEST_TIMEOUT_MILLIS private var assignmentConfiguration = Defaults.ASSIGNMENT_CONFIGURATION + private var cohortSyncConfiguration = Defaults.COHORT_SYNC_CONFIGURATION + private var metrics = Defaults.LOCAL_EVALUATION_METRICS fun debug(debug: Boolean) = apply { this.debug = debug @@ -90,6 +110,15 @@ class LocalEvaluationConfig internal constructor( this.assignmentConfiguration = assignmentConfiguration } + fun enableCohortSync(cohortSyncConfiguration: CohortSyncConfiguration) = apply { + this.cohortSyncConfiguration = cohortSyncConfiguration + } + + @ExperimentalApi + fun metrics(metrics: LocalEvaluationMetrics) = apply { + this.metrics = metrics + } + fun build(): LocalEvaluationConfig { return LocalEvaluationConfig( debug = debug, @@ -97,14 +126,19 @@ class LocalEvaluationConfig internal constructor( flagConfigPollerIntervalMillis = flagConfigPollerIntervalMillis, flagConfigPollerRequestTimeoutMillis = flagConfigPollerRequestTimeoutMillis, assignmentConfiguration = assignmentConfiguration, + cohortSyncConfiguration = cohortSyncConfiguration, + metrics = metrics, ) } } override fun toString(): String { - return "ExperimentConfig(debug=$debug, serverUrl='$serverUrl', " + + return "LocalEvaluationConfig(debug=$debug, serverUrl='$serverUrl', " + "flagConfigPollerIntervalMillis=$flagConfigPollerIntervalMillis, " + - "flagConfigPollerRequestTimeoutMillis=$flagConfigPollerRequestTimeoutMillis)" + "flagConfigPollerRequestTimeoutMillis=$flagConfigPollerRequestTimeoutMillis, " + + "assignmentConfiguration=$assignmentConfiguration, " + + "cohortSyncConfiguration=$cohortSyncConfiguration, " + + "metrics=$metrics)" } } @@ -115,4 +149,25 @@ data class AssignmentConfiguration( val eventUploadPeriodMillis: Int = 10000, val useBatchMode: Boolean = true, val serverUrl: String = "https://api2.amplitude.com/2/httpapi", + val middleware: List = listOf(), ) + +data class CohortSyncConfiguration( + val apiKey: String, + val secretKey: String, + val maxCohortSize: Int = 15000, +) + +@ExperimentalApi +interface LocalEvaluationMetrics { + fun onEvaluation() + fun onEvaluationFailure(exception: Exception) + fun onAssignment() + fun onAssignmentFilter() + fun onAssignmentEvent() + fun onAssignmentEventFailure(exception: Exception) + fun onFlagConfigFetch() + fun onFlagConfigFetchFailure(exception: Exception) + fun onCohortDownload() + fun onCohortDownloadFailure(exception: Exception) +} diff --git a/src/main/kotlin/cohort/Cohort.kt b/src/main/kotlin/cohort/Cohort.kt new file mode 100644 index 0000000..ea1553b --- /dev/null +++ b/src/main/kotlin/cohort/Cohort.kt @@ -0,0 +1,9 @@ +package com.amplitude.experiment.cohort + +data class Cohort( + val id: String, + val groupType: String, + val size: Int, + val lastModified: Long, + val members: Set, +) diff --git a/src/main/kotlin/cohort/CohortDownloadApi.kt b/src/main/kotlin/cohort/CohortDownloadApi.kt new file mode 100644 index 0000000..c641bb0 --- /dev/null +++ b/src/main/kotlin/cohort/CohortDownloadApi.kt @@ -0,0 +1,5 @@ +package com.amplitude.experiment.cohort + +interface CohortDownloadApi { + fun getCohort(id: String, maxCohortSize: Int): Cohort +} From 932e1f97f80e79c7372a9f4c948a700a8a2fdbd7 Mon Sep 17 00:00:00 2001 From: Brian Giori Date: Fri, 28 Jun 2024 16:37:09 -0700 Subject: [PATCH 02/15] feat: local evaluation cohorts - wip --- src/main/kotlin/ExperimentUser.kt | 23 ++++ src/main/kotlin/LocalEvaluationClient.kt | 111 +++++++++++++++-- src/main/kotlin/LocalEvaluationConfig.kt | 14 +++ src/main/kotlin/RemoteEvaluationClient.kt | 13 +- src/main/kotlin/RemoteEvaluationConfig.kt | 32 ++++- src/main/kotlin/ServerZone.kt | 10 ++ src/main/kotlin/cohort/CohortDownloadApi.kt | 82 +++++++++++- src/main/kotlin/cohort/CohortLoader.kt | 44 +++++++ src/main/kotlin/cohort/CohortStorage.kt | 36 ++++++ .../kotlin/deployment/DeploymentRunner.kt | 117 ++++++++++++++++++ src/main/kotlin/flag/FlagConfigApi.kt | 33 ++--- src/main/kotlin/flag/FlagConfigService.kt | 78 ------------ src/main/kotlin/flag/FlagConfigStorage.kt | 34 +++++ src/main/kotlin/util/Executors.kt | 16 +++ src/main/kotlin/util/ExperimentUser.kt | 4 + src/main/kotlin/util/FlagConfig.kt | 68 ++++++++++ src/main/kotlin/util/Metrics.kt | 78 ++++++++++++ src/main/kotlin/util/Request.kt | 82 ++++++++++++ 18 files changed, 756 insertions(+), 119 deletions(-) create mode 100644 src/main/kotlin/ServerZone.kt create mode 100644 src/main/kotlin/cohort/CohortLoader.kt create mode 100644 src/main/kotlin/cohort/CohortStorage.kt create mode 100644 src/main/kotlin/deployment/DeploymentRunner.kt delete mode 100644 src/main/kotlin/flag/FlagConfigService.kt create mode 100644 src/main/kotlin/flag/FlagConfigStorage.kt create mode 100644 src/main/kotlin/util/Executors.kt create mode 100644 src/main/kotlin/util/FlagConfig.kt create mode 100644 src/main/kotlin/util/Metrics.kt diff --git a/src/main/kotlin/ExperimentUser.kt b/src/main/kotlin/ExperimentUser.kt index e65ab16..9050144 100644 --- a/src/main/kotlin/ExperimentUser.kt +++ b/src/main/kotlin/ExperimentUser.kt @@ -39,6 +39,7 @@ data class ExperimentUser internal constructor( @JvmField val cohortIds: Set? = null, @JvmField val groups: Map>? = null, @JvmField val groupProperties: Map>>? = null, + @JvmField val groupCohortIds: Map>>? = null, ) { /** @@ -65,6 +66,9 @@ data class ExperimentUser internal constructor( .library(this.library) .userProperties(this.userProperties) .cohortIds(this.cohortIds) + .groups(this.groups) + .groupProperties(this.groupProperties) + .groupCohortIds(this.groupCohortIds) } companion object { @@ -94,6 +98,7 @@ data class ExperimentUser internal constructor( private var cohortIds: Set? = null private var groups: MutableMap>? = null private var groupProperties: MutableMap>>? = null + private var groupCohortIds: MutableMap>>? = null fun userId(userId: String?) = apply { this.userId = userId } fun deviceId(deviceId: String?) = apply { this.deviceId = deviceId } @@ -123,12 +128,15 @@ data class ExperimentUser internal constructor( fun cohortIds(cohortIds: Set?) = apply { this.cohortIds = cohortIds } + fun groups(groups: Map>?) = apply { this.groups = groups?.toMutableMap() } + fun group(groupType: String, groupName: String) = apply { this.groups = (this.groups ?: mutableMapOf()).apply { put(groupType, setOf(groupName)) } } + fun groupProperties(groupProperties: Map>>?) = apply { this.groupProperties = groupProperties?.mapValues { groupTypes -> groupTypes.value.toMutableMap().mapValues { groupNames -> @@ -136,6 +144,7 @@ data class ExperimentUser internal constructor( }.toMutableMap() }?.toMutableMap() } + fun groupProperty(groupType: String, groupName: String, key: String, value: Any?) = apply { this.groupProperties = (this.groupProperties ?: mutableMapOf()).apply { getOrPut(groupType) { mutableMapOf(groupName to mutableMapOf()) } @@ -143,6 +152,19 @@ data class ExperimentUser internal constructor( } } + internal fun groupCohortIds(groupCohortIds: Map>>?) = apply { + this.groupCohortIds = groupCohortIds?.mapValues { groupTypes -> + groupTypes.value.toMutableMap() + }?.toMutableMap() + } + + fun groupCohortIds(groupType: String, groupName: String, cohortIds: Set) = apply { + this.groupCohortIds = (this.groupCohortIds ?: mutableMapOf()).apply { + val groupNames = getOrPut(groupType) { mutableMapOf() } + groupNames[groupName] = cohortIds + } + } + fun build(): ExperimentUser { return ExperimentUser( userId = userId, @@ -164,6 +186,7 @@ data class ExperimentUser internal constructor( cohortIds = cohortIds, groups = groups, groupProperties = groupProperties, + groupCohortIds = groupCohortIds, ) } } diff --git a/src/main/kotlin/LocalEvaluationClient.kt b/src/main/kotlin/LocalEvaluationClient.kt index d0d6be5..9685b4e 100644 --- a/src/main/kotlin/LocalEvaluationClient.kt +++ b/src/main/kotlin/LocalEvaluationClient.kt @@ -6,18 +6,24 @@ import com.amplitude.experiment.assignment.AmplitudeAssignmentService import com.amplitude.experiment.assignment.Assignment import com.amplitude.experiment.assignment.AssignmentService import com.amplitude.experiment.assignment.InMemoryAssignmentFilter +import com.amplitude.experiment.cohort.DirectCohortDownloadApi +import com.amplitude.experiment.cohort.InMemoryCohortStorage +import com.amplitude.experiment.deployment.DeploymentRunner import com.amplitude.experiment.evaluation.EvaluationEngine import com.amplitude.experiment.evaluation.EvaluationEngineImpl +import com.amplitude.experiment.evaluation.EvaluationFlag import com.amplitude.experiment.evaluation.topologicalSort -import com.amplitude.experiment.flag.FlagConfigApiImpl -import com.amplitude.experiment.flag.FlagConfigService -import com.amplitude.experiment.flag.FlagConfigServiceConfig -import com.amplitude.experiment.flag.FlagConfigServiceImpl +import com.amplitude.experiment.flag.DirectFlagConfigApi +import com.amplitude.experiment.flag.InMemoryFlagConfigStorage +import com.amplitude.experiment.util.LocalEvaluationMetricsWrapper import com.amplitude.experiment.util.Logger import com.amplitude.experiment.util.Once +import com.amplitude.experiment.util.USER_GROUP_TYPE import com.amplitude.experiment.util.filterDefaultVariants +import com.amplitude.experiment.util.getGroupedCohortIds import com.amplitude.experiment.util.toEvaluationContext import com.amplitude.experiment.util.toVariants +import com.amplitude.experiment.util.wrapMetrics import okhttp3.HttpUrl import okhttp3.HttpUrl.Companion.toHttpUrl import okhttp3.OkHttpClient @@ -29,16 +35,41 @@ class LocalEvaluationClient internal constructor( private val startLock = Once() private val httpClient = OkHttpClient() private val assignmentService: AssignmentService? = createAssignmentService(apiKey) - private val serverUrl: HttpUrl = config.serverUrl.toHttpUrl() + private val serverUrl: HttpUrl = getServerUrl(config) + private val cohortServerUrl: HttpUrl = getCohortServerUrl(config) private val evaluation: EvaluationEngine = EvaluationEngineImpl() - private val flagConfigService: FlagConfigService = FlagConfigServiceImpl( - FlagConfigServiceConfig(config.flagConfigPollerIntervalMillis), - FlagConfigApiImpl(apiKey, serverUrl, httpClient), + private val metrics: LocalEvaluationMetrics = LocalEvaluationMetricsWrapper(config.metrics) + private val flagConfigApi = DirectFlagConfigApi(apiKey, serverUrl, httpClient) + private val flagConfigStorage = InMemoryFlagConfigStorage() + private val cohortDownloadApi = if (config.cohortSyncConfiguration != null) { + DirectCohortDownloadApi( + apiKey = config.cohortSyncConfiguration.apiKey, + secretKey = config.cohortSyncConfiguration.secretKey, + maxCohortSize = config.cohortSyncConfiguration.maxCohortSize, + serverUrl = cohortServerUrl, + httpClient = httpClient, + ) + } else { + null + } + private val cohortStorage = if (config.cohortSyncConfiguration != null) { + InMemoryCohortStorage() + } else { + null + } + + private val deploymentRunner = DeploymentRunner( + config = config, + flagConfigApi = flagConfigApi, + flagConfigStorage = flagConfigStorage, + cohortDownloadApi = cohortDownloadApi, + cohortStorage = cohortStorage, + metrics = metrics, ) fun start() { startLock.once { - flagConfigService.start() + deploymentRunner.start() } } @@ -67,15 +98,71 @@ class LocalEvaluationClient internal constructor( @JvmOverloads fun evaluateV2(user: ExperimentUser, flagKeys: Set = setOf()): Map { - val flagConfigs = flagConfigService.getFlagConfigs().toMap() + val flagConfigs = flagConfigStorage.getFlagConfigs() val sortedFlagConfigs = topologicalSort(flagConfigs, flagKeys) if (sortedFlagConfigs.isEmpty()) { return mapOf() } - val evaluationResults = evaluation.evaluate(user.toEvaluationContext(), sortedFlagConfigs) - Logger.d("evaluate - user=$user, result=$evaluationResults") + val enrichedUser = enrichUser(user, flagConfigs) + val evaluationResults = wrapMetrics( + metric = metrics::onEvaluation, + failure = metrics::onEvaluationFailure, + ) { + evaluation.evaluate(enrichedUser.toEvaluationContext(), sortedFlagConfigs) + } + Logger.d("evaluate - user=$enrichedUser, result=$evaluationResults") val variants = evaluationResults.toVariants() assignmentService?.track(Assignment(user, variants)) return variants } + + private fun enrichUser(user: ExperimentUser, flagConfigs: Map): ExperimentUser { + if (cohortStorage == null) { + return user + } + val groupedCohortIds = flagConfigs.values.getGroupedCohortIds() + return user.copyToBuilder().apply { + val userCohortsIds = groupedCohortIds[USER_GROUP_TYPE] + if (!userCohortsIds.isNullOrEmpty() && user.userId != null) { + cohortIds(cohortStorage.getCohortsForUser(user.userId, userCohortsIds)) + } + if (user.groups != null) { + for (group in user.groups) { + val groupType = group.key + val groupName = group.value.firstOrNull() ?: continue + val cohortIds = groupedCohortIds[groupType] + if (cohortIds.isNullOrEmpty()) { + continue + } + groupCohortIds( + groupType, + groupName, + cohortStorage.getCohortsForGroup(groupType, groupName, cohortIds) + ) + } + } + }.build() + } +} + +private fun getServerUrl(config: LocalEvaluationConfig): HttpUrl { + if (config.serverZone == LocalEvaluationConfig.Defaults.SERVER_ZONE) { + return config.serverUrl.toHttpUrl() + } else { + return when (config.serverZone) { + ServerZone.US -> US_SERVER_URL.toHttpUrl() + ServerZone.EU -> EU_SERVER_URL.toHttpUrl() + } + } +} + +private fun getCohortServerUrl(config: LocalEvaluationConfig): HttpUrl { + if (config.serverZone == LocalEvaluationConfig.Defaults.SERVER_ZONE) { + return config.cohortServerUrl.toHttpUrl() + } else { + return when (config.serverZone) { + ServerZone.US -> US_SERVER_URL.toHttpUrl() + ServerZone.EU -> EU_SERVER_URL.toHttpUrl() + } + } } diff --git a/src/main/kotlin/LocalEvaluationConfig.kt b/src/main/kotlin/LocalEvaluationConfig.kt index d1f00c4..d2a4b02 100644 --- a/src/main/kotlin/LocalEvaluationConfig.kt +++ b/src/main/kotlin/LocalEvaluationConfig.kt @@ -16,6 +16,10 @@ class LocalEvaluationConfig internal constructor( @JvmField val serverUrl: String = Defaults.SERVER_URL, @JvmField + val cohortServerUrl: String = Defaults.COHORT_SERVER_URL, + @JvmField + val serverZone: ServerZone = Defaults.SERVER_ZONE, + @JvmField val flagConfigPollerIntervalMillis: Long = Defaults.FLAG_CONFIG_POLLER_INTERVAL_MILLIS, @JvmField val flagConfigPollerRequestTimeoutMillis: Long = Defaults.FLAG_CONFIG_POLLER_REQUEST_TIMEOUT_MILLIS, @@ -47,6 +51,16 @@ class LocalEvaluationConfig internal constructor( */ const val SERVER_URL = "https://api.lab.amplitude.com/" + /** + * "https://api.lab.amplitude.com/" + */ + const val COHORT_SERVER_URL = "https://cohorts-v2.lab.amplitude.com/" + + /** + * ServerZone.US + */ + val SERVER_ZONE = ServerZone.US + /** * 30,000 */ diff --git a/src/main/kotlin/RemoteEvaluationClient.kt b/src/main/kotlin/RemoteEvaluationClient.kt index c0974ff..8e3517a 100644 --- a/src/main/kotlin/RemoteEvaluationClient.kt +++ b/src/main/kotlin/RemoteEvaluationClient.kt @@ -29,7 +29,7 @@ class RemoteEvaluationClient internal constructor( private val httpClient = OkHttpClient.Builder().proxy(config.httpProxy).build() private val retry: Boolean = config.fetchRetries > 0 - private val serverUrl: HttpUrl = config.serverUrl.toHttpUrl() + private val serverUrl: HttpUrl = getServerUrl(config) private val backoffConfig = BackoffConfig( attempts = config.fetchRetries, min = config.fetchRetryBackoffMinMillis, @@ -116,3 +116,14 @@ private fun shouldRetryFetch(t: Throwable): Boolean { } return true } + +private fun getServerUrl(config: RemoteEvaluationConfig): HttpUrl { + if (config.serverZone == RemoteEvaluationConfig.Defaults.SERVER_ZONE) { + return config.serverUrl.toHttpUrl() + } else { + return when (config.serverZone) { + ServerZone.US -> US_SERVER_URL.toHttpUrl() + ServerZone.EU -> EU_SERVER_URL.toHttpUrl() + } + } +} diff --git a/src/main/kotlin/RemoteEvaluationConfig.kt b/src/main/kotlin/RemoteEvaluationConfig.kt index 2b5d988..486cc7e 100644 --- a/src/main/kotlin/RemoteEvaluationConfig.kt +++ b/src/main/kotlin/RemoteEvaluationConfig.kt @@ -14,6 +14,10 @@ class RemoteEvaluationConfig internal constructor( @JvmField val serverUrl: String = Defaults.SERVER_URL, @JvmField + val cohortServerUrl: String = Defaults.COHORT_SERVER_URL, + @JvmField + val serverZone: ServerZone = Defaults.SERVER_ZONE, + @JvmField val fetchTimeoutMillis: Long = Defaults.FETCH_TIMEOUT_MILLIS, @JvmField val fetchRetries: Int = Defaults.FETCH_RETRIES, @@ -47,6 +51,16 @@ class RemoteEvaluationConfig internal constructor( */ const val SERVER_URL = "https://api.lab.amplitude.com/" + /** + * "https://api.lab.amplitude.com/" + */ + const val COHORT_SERVER_URL = "https://cohorts-v2.lab.amplitude.com/" + + /** + * ServerZone.US + */ + val SERVER_ZONE = ServerZone.US + /** * 500 */ @@ -89,6 +103,8 @@ class RemoteEvaluationConfig internal constructor( private var debug = Defaults.DEBUG private var serverUrl = Defaults.SERVER_URL + private var cohortServerUrl = Defaults.COHORT_SERVER_URL + private var serverZone = Defaults.SERVER_ZONE private var fetchTimeoutMillis = Defaults.FETCH_TIMEOUT_MILLIS private var fetchRetries = Defaults.FETCH_RETRIES private var fetchRetryBackoffMinMillis = Defaults.FETCH_RETRY_BACKOFF_MIN_MILLIS @@ -104,6 +120,14 @@ class RemoteEvaluationConfig internal constructor( this.serverUrl = serverUrl } + fun cohortServerUrl(cohortServerUrl: String) = apply { + this.cohortServerUrl = cohortServerUrl + } + + fun serverZone(serverZone: ServerZone) = apply { + this.serverZone = serverZone + } + fun fetchTimeoutMillis(fetchTimeoutMillis: Long) = apply { this.fetchTimeoutMillis = fetchTimeoutMillis } @@ -143,10 +167,10 @@ class RemoteEvaluationConfig internal constructor( } override fun toString(): String { - return "ExperimentConfig(debug=$debug, serverUrl='$serverUrl', fetchTimeoutMillis=$fetchTimeoutMillis, " + - "fetchRetries=$fetchRetries, fetchRetryBackoffMinMillis=$fetchRetryBackoffMinMillis, " + + return "RemoteEvaluationConfig(debug=$debug, serverUrl='$serverUrl', cohortServerUrl='$cohortServerUrl', " + + "serverZone=$serverZone, fetchTimeoutMillis=$fetchTimeoutMillis, fetchRetries=$fetchRetries, " + + "fetchRetryBackoffMinMillis=$fetchRetryBackoffMinMillis, " + "fetchRetryBackoffMaxMillis=$fetchRetryBackoffMaxMillis, " + - "fetchRetryBackoffScalar=$fetchRetryBackoffScalar), " + - "httpProxy=$httpProxy" + "fetchRetryBackoffScalar=$fetchRetryBackoffScalar, httpProxy=$httpProxy)" } } diff --git a/src/main/kotlin/ServerZone.kt b/src/main/kotlin/ServerZone.kt new file mode 100644 index 0000000..eba3a00 --- /dev/null +++ b/src/main/kotlin/ServerZone.kt @@ -0,0 +1,10 @@ +package com.amplitude.experiment + +internal const val US_SERVER_URL = "https://api.lab.amplitude.com" +internal const val EU_SERVER_URL = "https://api.lab.eu.amplitude.com" +internal const val US_COHORT_SERVER_URL = "https://cohort-v2.lab.amplitude.com" +internal const val EU_COHORT_SERVER_URL = "https://cohort-v2.lab.eu.amplitude.com" + +enum class ServerZone { + US, EU +} diff --git a/src/main/kotlin/cohort/CohortDownloadApi.kt b/src/main/kotlin/cohort/CohortDownloadApi.kt index c641bb0..7e18993 100644 --- a/src/main/kotlin/cohort/CohortDownloadApi.kt +++ b/src/main/kotlin/cohort/CohortDownloadApi.kt @@ -1,5 +1,83 @@ package com.amplitude.experiment.cohort -interface CohortDownloadApi { - fun getCohort(id: String, maxCohortSize: Int): Cohort +import com.amplitude.experiment.LIBRARY_VERSION +import com.amplitude.experiment.util.HttpErrorResponseException +import com.amplitude.experiment.util.Logger +import com.amplitude.experiment.util.get +import kotlinx.serialization.Serializable +import okhttp3.HttpUrl +import okhttp3.OkHttpClient +import java.util.Base64 + +internal class CohortTooLargeException(cohortId: String, maxCohortSize: Int) : Exception( + "Cohort $cohortId exceeds the maximum cohort size defined in the SDK configuration $maxCohortSize" +) + +internal class CohortNotModifiedException(cohortId: String) : Exception( + "Cohort $cohortId has not been modified." +) + +@Serializable +private data class GetCohortResponse( + private val id: String, + private val lastModified: Long, + private val size: Int, + private val memberIds: Set = setOf(), + private val groupType: String, +) { + fun toCohort() = Cohort(id, groupType, size, lastModified, memberIds) +} + +internal interface CohortDownloadApi { + fun getCohort(cohortId: String, cohort: Cohort?): Cohort +} + +internal class DirectCohortDownloadApi( + apiKey: String, + secretKey: String, + private val maxCohortSize: Int, + private val serverUrl: HttpUrl, + private val httpClient: OkHttpClient, +) : CohortDownloadApi { + private val bearerToken = Base64.getEncoder().encodeToString("$apiKey:$secretKey".toByteArray()) + override fun getCohort(cohortId: String, cohort: Cohort?): Cohort { + Logger.d("getCohortMembers($cohortId): start") + var errors = 0 + while (true) { + val headers = mapOf( + "Authorization" to "Bearer $bearerToken", + "X-Amp-Exp-Library" to "experiment-jvm-server/$LIBRARY_VERSION", + ) + val queries = mutableMapOf( + "maxCohortSize" to "$maxCohortSize", + ) + if (cohort != null && cohort.lastModified > 0) { + queries["lastModified"] = "${cohort.lastModified}" + } + try { + return httpClient.get( + serverUrl = serverUrl, + path = "sdk/v1/cohort/$cohortId", + headers = headers, + queries = queries, + ) { response -> + Logger.d("getCohortMembers($cohortId): status=${response.code}") + when (response.code) { + 200 -> return@get + 204 -> throw CohortNotModifiedException(cohortId) + 413 -> throw CohortTooLargeException(cohortId, maxCohortSize) + else -> throw HttpErrorResponseException(response.code) + } + }.toCohort() + } catch (e: HttpErrorResponseException) { + if (e.code == 429) { + continue + } + if (errors >= 3) { + throw e + } + errors++ + } + } + } } diff --git a/src/main/kotlin/cohort/CohortLoader.kt b/src/main/kotlin/cohort/CohortLoader.kt new file mode 100644 index 0000000..fbe1fd2 --- /dev/null +++ b/src/main/kotlin/cohort/CohortLoader.kt @@ -0,0 +1,44 @@ +package com.amplitude.experiment.cohort + +import com.amplitude.experiment.util.Logger +import com.amplitude.experiment.util.daemonFactory +import java.util.concurrent.CompletableFuture +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.LinkedBlockingQueue +import java.util.concurrent.ThreadPoolExecutor +import java.util.concurrent.TimeUnit + +internal class CohortLoader( + private val cohortDownloadApi: CohortDownloadApi, + private val cohortStorage: CohortStorage, +) { + + private val jobs = ConcurrentHashMap>() + private val executor = ThreadPoolExecutor( + 32, + 32, + 60L, + TimeUnit.SECONDS, + LinkedBlockingQueue(), + daemonFactory, + ).apply { + allowCoreThreadTimeOut(true) + } + + fun loadCohort(cohortId: String): CompletableFuture<*> { + return jobs.getOrPut(cohortId) { + CompletableFuture.runAsync({ + Logger.d("Loading cohort $cohortId") + val storageCohort = cohortStorage.getCohort(cohortId) + try { + val cohort = cohortDownloadApi.getCohort(cohortId, storageCohort) + cohortStorage.putCohort(cohort) + } catch (e: CohortNotModifiedException) { + // Do nothing + } catch (e: CohortTooLargeException) { + Logger.e("Cohort too large", e) + } + }, executor).whenComplete { _, _ -> jobs.remove(cohortId) } + } + } +} diff --git a/src/main/kotlin/cohort/CohortStorage.kt b/src/main/kotlin/cohort/CohortStorage.kt new file mode 100644 index 0000000..c2843ef --- /dev/null +++ b/src/main/kotlin/cohort/CohortStorage.kt @@ -0,0 +1,36 @@ +package com.amplitude.experiment.cohort + +internal interface CohortStorage { + fun getCohort(cohortId: String): Cohort? + fun getCohorts(): List + fun getCohortsForUser(userId: String, cohortIds: Set): Set + fun getCohortsForGroup(groupType: String, groupName: String, cohortIds: Set): Set + fun putCohort(cohort: Cohort) + fun deleteCohort(groupType: String, cohortId: String) +} + +internal class InMemoryCohortStorage : CohortStorage { + override fun getCohort(cohortId: String): Cohort? { + TODO("Not yet implemented") + } + + override fun getCohorts(): List { + TODO("Not yet implemented") + } + + override fun getCohortsForUser(userId: String, cohortIds: Set): Set { + TODO("Not yet implemented") + } + + override fun getCohortsForGroup(groupType: String, groupName: String, cohortIds: Set): Set { + TODO("Not yet implemented") + } + + override fun putCohort(cohort: Cohort) { + TODO("Not yet implemented") + } + + override fun deleteCohort(groupType: String, cohortId: String) { + TODO("Not yet implemented") + } +} diff --git a/src/main/kotlin/deployment/DeploymentRunner.kt b/src/main/kotlin/deployment/DeploymentRunner.kt new file mode 100644 index 0000000..f6ae4bb --- /dev/null +++ b/src/main/kotlin/deployment/DeploymentRunner.kt @@ -0,0 +1,117 @@ +package com.amplitude.experiment.deployment + +import com.amplitude.experiment.LocalEvaluationConfig +import com.amplitude.experiment.LocalEvaluationMetrics +import com.amplitude.experiment.cohort.CohortDownloadApi +import com.amplitude.experiment.cohort.CohortLoader +import com.amplitude.experiment.cohort.CohortStorage +import com.amplitude.experiment.flag.FlagConfigApi +import com.amplitude.experiment.flag.FlagConfigStorage +import com.amplitude.experiment.util.LocalEvaluationMetricsWrapper +import com.amplitude.experiment.util.Logger +import com.amplitude.experiment.util.Once +import com.amplitude.experiment.util.daemonFactory +import com.amplitude.experiment.util.getAllCohortIds +import com.amplitude.experiment.util.wrapMetrics +import java.util.concurrent.CompletableFuture +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit + +internal class DeploymentRunner( + private val config: LocalEvaluationConfig, + private val flagConfigApi: FlagConfigApi, + private val flagConfigStorage: FlagConfigStorage, + cohortDownloadApi: CohortDownloadApi?, + private val cohortStorage: CohortStorage?, + private val metrics: LocalEvaluationMetrics = LocalEvaluationMetricsWrapper() +) { + private val lock = Once() + private val poller = Executors.newScheduledThreadPool(1, daemonFactory) + private val cohortLoader = if (cohortDownloadApi != null && cohortStorage != null) { + CohortLoader(cohortDownloadApi, cohortStorage) + } else { + null + } + + fun start() = lock.once { + refresh() + poller.scheduleWithFixedDelay( + { + try { + refresh() + } catch (t: Throwable) { + Logger.e("Refresh flag configs failed.", t) + } + }, + config.flagConfigPollerIntervalMillis, + config.flagConfigPollerIntervalMillis, + TimeUnit.MILLISECONDS + ) + if (cohortLoader != null) { + poller.scheduleWithFixedDelay( + { + try { + val cohortIds = flagConfigStorage.getFlagConfigs().values.getAllCohortIds() + for (cohortId in cohortIds) { + cohortLoader.loadCohort(cohortId) + } + } catch (t: Throwable) { + Logger.e("Refresh cohorts failed.", t) + } + }, + 60, + 60, + TimeUnit.SECONDS + ) + } + } + + fun stop() { + poller.shutdown() + } + + fun refresh() { + Logger.d("Refreshing flag configs.") + // Get updated flags from the network. + val flagConfigs = wrapMetrics( + metric = metrics::onFlagConfigFetch, + failure = metrics::onFlagConfigFetchFailure, + ) { + flagConfigApi.getFlagConfigs() + } + // Remove flags that no longer exist. + val flagKeys = flagConfigs.map { it.key }.toSet() + flagConfigStorage.removeIf { !flagKeys.contains(it.key) } + + // Load cohorts for each flag if applicable and put the flag in storage. + val futures = ConcurrentHashMap>() + for (flagConfig in flagConfigs) { + val cohortIds = flagConfig.getAllCohortIds() + if (cohortLoader == null || cohortIds.isEmpty()) { + flagConfigStorage.putFlagConfig(flagConfig) + continue + } + for (cohortId in cohortIds) { + futures.putIfAbsent( + cohortId, + cohortLoader.loadCohort(cohortId).thenRun { + flagConfigStorage.putFlagConfig(flagConfig) + } + ) + } + } + futures.values.forEach { it.join() } + + // Delete unused cohorts + if (cohortStorage != null) { + val flagCohortIds = flagConfigStorage.getFlagConfigs().values.toList().getAllCohortIds() + val storageCohorts = cohortStorage.getCohorts().associateBy { it.id } + val deletedCohorts = storageCohorts - flagCohortIds + for (deletedCohort in deletedCohorts) { + cohortStorage.deleteCohort(deletedCohort.value.groupType, deletedCohort.key) + } + } + Logger.d("Refreshed ${flagConfigs.size} flag configs.") + } +} diff --git a/src/main/kotlin/flag/FlagConfigApi.kt b/src/main/kotlin/flag/FlagConfigApi.kt index 06324a1..4fc3c5a 100644 --- a/src/main/kotlin/flag/FlagConfigApi.kt +++ b/src/main/kotlin/flag/FlagConfigApi.kt @@ -1,39 +1,28 @@ package com.amplitude.experiment.flag -import com.amplitude.experiment.LIBRARY_VERSION import com.amplitude.experiment.evaluation.EvaluationFlag -import com.amplitude.experiment.util.request +import com.amplitude.experiment.util.get import okhttp3.HttpUrl import okhttp3.OkHttpClient -import okhttp3.Request -import java.util.concurrent.CompletableFuture - -internal object GetFlagConfigsRequest - -internal typealias GetFlagConfigsResponse = List internal interface FlagConfigApi { - fun getFlagConfigs(request: GetFlagConfigsRequest): CompletableFuture + fun getFlagConfigs(): List } -internal class FlagConfigApiImpl( +internal class DirectFlagConfigApi( private val deploymentKey: String, private val serverUrl: HttpUrl, private val httpClient: OkHttpClient, ) : FlagConfigApi { - override fun getFlagConfigs(request: GetFlagConfigsRequest): CompletableFuture { - val url = serverUrl.newBuilder() - .addPathSegments("sdk/v2/flags") - .addQueryParameter("v", "0") - .build() - return httpClient.request>( - Request.Builder() - .get() - .url(url) - .addHeader("Authorization", "Api-Key $deploymentKey") - .addHeader("X-Amp-Exp-Library", "experiment-jvm-server/$LIBRARY_VERSION") - .build() + override fun getFlagConfigs(): List { + return httpClient.get>( + serverUrl = serverUrl, + path = "sdk/v2/flags", + headers = mapOf( + "Authorization" to "Api-Key $deploymentKey", + ), + queries = mapOf("v" to "0") ) } } diff --git a/src/main/kotlin/flag/FlagConfigService.kt b/src/main/kotlin/flag/FlagConfigService.kt deleted file mode 100644 index 506314f..0000000 --- a/src/main/kotlin/flag/FlagConfigService.kt +++ /dev/null @@ -1,78 +0,0 @@ -package com.amplitude.experiment.flag - -import com.amplitude.experiment.evaluation.EvaluationFlag -import com.amplitude.experiment.util.Logger -import com.amplitude.experiment.util.Once -import java.util.concurrent.Executors -import java.util.concurrent.TimeUnit -import java.util.concurrent.locks.ReentrantReadWriteLock -import kotlin.concurrent.read -import kotlin.concurrent.write - -internal data class FlagConfigServiceConfig( - val flagConfigPollerIntervalMillis: Long, -) - -internal interface FlagConfigService { - fun start() - fun stop() - fun getFlagConfigs(): Map -} - -internal class FlagConfigServiceImpl( - private val config: FlagConfigServiceConfig, - private val flagConfigApi: FlagConfigApi, -) : FlagConfigService { - - private val lock = Once() - private val poller = Executors.newSingleThreadScheduledExecutor() - - private val flagConfigsLock = ReentrantReadWriteLock() - private val flagConfigs: MutableMap = mutableMapOf() - - private fun refresh() { - Logger.d("Refreshing flag configs.") - val flagConfigs = fetchFlagConfigs() - storeFlagConfigs(flagConfigs) - Logger.d("Refreshed ${flagConfigs.size} flag configs.") - } - - override fun start() { - lock.once { - poller.scheduleAtFixedRate( - { - try { - refresh() - } catch (e: Exception) { - Logger.e("Failed to refresh flag configs.", e) - } - }, - config.flagConfigPollerIntervalMillis, - config.flagConfigPollerIntervalMillis, - TimeUnit.MILLISECONDS - ) - refresh() - } - } - - override fun stop() { - poller.shutdown() - } - - override fun getFlagConfigs(): Map { - return flagConfigsLock.read { - flagConfigs - } - } - - private fun fetchFlagConfigs(): List { - return flagConfigApi.getFlagConfigs(GetFlagConfigsRequest).get() - } - - private fun storeFlagConfigs(flagConfigs: List) { - flagConfigsLock.write { - this.flagConfigs.clear() - this.flagConfigs.putAll(flagConfigs.associateBy { it.key }) - } - } -} diff --git a/src/main/kotlin/flag/FlagConfigStorage.kt b/src/main/kotlin/flag/FlagConfigStorage.kt new file mode 100644 index 0000000..46412ae --- /dev/null +++ b/src/main/kotlin/flag/FlagConfigStorage.kt @@ -0,0 +1,34 @@ +package com.amplitude.experiment.flag + +import com.amplitude.experiment.evaluation.EvaluationFlag +import java.util.concurrent.locks.ReentrantReadWriteLock +import kotlin.concurrent.read +import kotlin.concurrent.write + +internal interface FlagConfigStorage { + fun getFlagConfigs(): Map + fun putFlagConfig(flagConfig: EvaluationFlag) + fun removeIf(condition: (EvaluationFlag) -> Boolean) +} + +internal class InMemoryFlagConfigStorage : FlagConfigStorage { + + private val flagConfigs = mutableMapOf() + private val flagConfigsLock = ReentrantReadWriteLock() + + override fun getFlagConfigs(): Map { + return flagConfigsLock.read { flagConfigs.toMap() } + } + + override fun putFlagConfig(flagConfig: EvaluationFlag) { + flagConfigsLock.write { + flagConfigs.put(flagConfig.key, flagConfig) + } + } + + override fun removeIf(condition: (EvaluationFlag) -> Boolean) { + return flagConfigsLock.write { + flagConfigs.entries.removeIf { condition(it.value) } + } + } +} diff --git a/src/main/kotlin/util/Executors.kt b/src/main/kotlin/util/Executors.kt new file mode 100644 index 0000000..e856cc5 --- /dev/null +++ b/src/main/kotlin/util/Executors.kt @@ -0,0 +1,16 @@ +package com.amplitude.experiment.util + +import java.util.concurrent.ThreadFactory + +internal val daemonFactory: ThreadFactory = DaemonThreadFactory() + +private class DaemonThreadFactory( + private val baseName: String = "experiment" +) : ThreadFactory { + private var count = 0 + override fun newThread(r: Runnable): Thread { + val t = Thread(r, baseName + "-" + (++count)) + t.isDaemon = true + return t + } +} diff --git a/src/main/kotlin/util/ExperimentUser.kt b/src/main/kotlin/util/ExperimentUser.kt index f537a4e..22a2ae9 100644 --- a/src/main/kotlin/util/ExperimentUser.kt +++ b/src/main/kotlin/util/ExperimentUser.kt @@ -17,6 +17,10 @@ internal fun ExperimentUser.toEvaluationContext(): EvaluationContext { if (!groupProperties.isNullOrEmpty()) { groupNameMap["group_properties"] = groupProperties } + val groupCohortIds = this.groupCohortIds?.get(groupType)?.get(groupName) + if (!groupCohortIds.isNullOrEmpty()) { + groupNameMap["cohort_ids"] = groupCohortIds + } groups[groupType] = groupNameMap } } diff --git a/src/main/kotlin/util/FlagConfig.kt b/src/main/kotlin/util/FlagConfig.kt new file mode 100644 index 0000000..4ea1f03 --- /dev/null +++ b/src/main/kotlin/util/FlagConfig.kt @@ -0,0 +1,68 @@ +package com.amplitude.experiment.util + +import com.amplitude.experiment.evaluation.EvaluationCondition +import com.amplitude.experiment.evaluation.EvaluationFlag +import com.amplitude.experiment.evaluation.EvaluationOperator +import com.amplitude.experiment.evaluation.EvaluationSegment + +internal const val USER_GROUP_TYPE = "User" + +internal fun Collection.getAllCohortIds(): Set { + return getGroupedCohortIds().flatMap { it.value }.toSet() +} + +internal fun Collection.getGroupedCohortIds(): Map> { + val cohortIds = mutableMapOf>() + for (flag in this) { + for (entry in flag.getGroupedCohortIds()) { + cohortIds.getOrPut(entry.key) { mutableSetOf() } += entry.value + } + } + return cohortIds +} + +internal fun EvaluationFlag.getAllCohortIds(): Set { + return getGroupedCohortIds().flatMap { it.value }.toSet() +} + +internal fun EvaluationFlag.getGroupedCohortIds(): Map> { + val cohortIds = mutableMapOf>() + for (segment in this.segments) { + for (entry in segment.getGroupedCohortConditionIds()) { + cohortIds.getOrPut(entry.key) { mutableSetOf() } += entry.value + } + } + return cohortIds +} + +private fun EvaluationSegment.getGroupedCohortConditionIds(): Map> { + val cohortIds = mutableMapOf>() + if (conditions == null) { + return cohortIds + } + for (outer in conditions!!) { + for (condition in outer) { + if (condition.isCohortFilter()) { + // User cohort selector is [context, user, cohort_ids] + // Groups cohort selector is [context, groups, {group_type}, cohort_ids] + if (condition.selector.size > 2) { + val contextSubtype = condition.selector[1] + val groupType = if (contextSubtype == "user") { + USER_GROUP_TYPE + } else if (condition.selector.contains("groups")) { + condition.selector[2] + } else { + continue + } + cohortIds.getOrPut(groupType) { mutableSetOf() } += condition.values + } + } + } + } + return cohortIds +} + +// Only cohort filters use these operators. +private fun EvaluationCondition.isCohortFilter(): Boolean = + (this.op == EvaluationOperator.SET_CONTAINS_ANY || this.op == EvaluationOperator.SET_DOES_NOT_CONTAIN_ANY) && + this.selector.isNotEmpty() && this.selector.last() == "cohort_ids" diff --git a/src/main/kotlin/util/Metrics.kt b/src/main/kotlin/util/Metrics.kt new file mode 100644 index 0000000..065d02f --- /dev/null +++ b/src/main/kotlin/util/Metrics.kt @@ -0,0 +1,78 @@ +@file:OptIn(ExperimentalApi::class) + +package com.amplitude.experiment.util + +import com.amplitude.experiment.ExperimentalApi +import com.amplitude.experiment.LocalEvaluationMetrics +import java.util.concurrent.Executors + +internal fun wrapMetrics(metric: (() -> Unit)?, failure: ((e: Exception) -> Unit)?, block: () -> R): R { + try { + metric?.invoke() + return block.invoke() + } catch (e: Exception) { + failure?.invoke(e) + throw e + } +} + +internal class LocalEvaluationMetricsWrapper( + private val metrics: LocalEvaluationMetrics? = null +) : LocalEvaluationMetrics { + + private val executor = if (metrics != null) { + Executors.newFixedThreadPool(1, daemonFactory) + } else { + null + } + + override fun onEvaluation() { + val metrics = metrics ?: return + executor?.execute { metrics.onEvaluation() } + } + + override fun onEvaluationFailure(exception: Exception) { + val metrics = metrics ?: return + executor?.execute { metrics.onEvaluationFailure(exception) } + } + + override fun onAssignment() { + val metrics = metrics ?: return + executor?.execute { metrics.onAssignment() } + } + + override fun onAssignmentFilter() { + val metrics = metrics ?: return + executor?.execute { metrics.onAssignmentFilter() } + } + + override fun onAssignmentEvent() { + val metrics = metrics ?: return + executor?.execute { metrics.onAssignmentEvent() } + } + + override fun onAssignmentEventFailure(exception: Exception) { + val metrics = metrics ?: return + executor?.execute { metrics.onAssignmentEventFailure(exception) } + } + + override fun onFlagConfigFetch() { + val metrics = metrics ?: return + executor?.execute { metrics.onFlagConfigFetch() } + } + + override fun onFlagConfigFetchFailure(exception: Exception) { + val metrics = metrics ?: return + executor?.execute { metrics.onFlagConfigFetchFailure(exception) } + } + + override fun onCohortDownload() { + val metrics = metrics ?: return + executor?.execute { metrics.onCohortDownload() } + } + + override fun onCohortDownloadFailure(exception: Exception) { + val metrics = metrics ?: return + executor?.execute { metrics.onCohortDownloadFailure(exception) } + } +} diff --git a/src/main/kotlin/util/Request.kt b/src/main/kotlin/util/Request.kt index 443d2d8..a9cdf75 100644 --- a/src/main/kotlin/util/Request.kt +++ b/src/main/kotlin/util/Request.kt @@ -1,8 +1,10 @@ package com.amplitude.experiment.util +import com.amplitude.experiment.LIBRARY_VERSION import kotlinx.serialization.decodeFromString import okhttp3.Call import okhttp3.Callback +import okhttp3.HttpUrl import okhttp3.OkHttpClient import okhttp3.Request import okhttp3.Response @@ -36,3 +38,83 @@ internal inline fun OkHttpClient.request( }) return future } + +internal open class HttpErrorResponseException( + val code: Int, + override val message: String? = null +) : IOException() + +private fun OkHttpClient.submit( + request: Request, +): CompletableFuture { + val future = CompletableFuture() + val call = newCall(request) + call.enqueue(object : Callback { + override fun onResponse(call: Call, response: Response) { + future.complete(response) + } + + override fun onFailure(call: Call, e: IOException) { + future.completeExceptionally(e) + } + }) + return future +} + +private fun newGet( + serverUrl: HttpUrl, + path: String? = null, + headers: Map? = null, + queries: Map? = null, +): Request { + val url = serverUrl.newBuilder().apply { + if (path != null) { + addPathSegments(path) + } + queries?.forEach { + addQueryParameter(it.key, it.value) + } + }.build() + val builder = Request.Builder().get().url(url) + headers?.forEach { + builder.addHeader(it.key, it.value) + } + builder.addHeader("X-Amp-Exp-Library", "experiment-jvm-server/$LIBRARY_VERSION") + return builder.build() +} + +internal fun OkHttpClient.get( + serverUrl: HttpUrl, + path: String? = null, + headers: Map? = null, + queries: Map? = null, +): Response { + val request = newGet(serverUrl, path, headers, queries) + return submit(request).thenApply { it.apply { close() } }.get() +} + +internal inline fun OkHttpClient.get( + serverUrl: HttpUrl, + path: String? = null, + headers: Map? = null, + queries: Map? = null, + crossinline block: (Response) -> Unit, +): T { + val request = newGet(serverUrl, path, headers, queries) + return submit(request).thenApply { + it.use { response -> + block(response) + val body = response.body?.string() ?: throw IOException("null response body") + json.decodeFromString(body) + } + }.get() +} + +internal inline fun OkHttpClient.get( + serverUrl: HttpUrl, + path: String? = null, + headers: Map? = null, + queries: Map? = null, +): T { + return this.get(serverUrl, path, headers, queries) {} +} From f47912ba5e18557aa7c7300f0d06c8fa2aa9319b Mon Sep 17 00:00:00 2001 From: Brian Giori Date: Wed, 31 Jul 2024 17:21:30 -0700 Subject: [PATCH 03/15] feat: cohort download, tests --- build.gradle.kts | 2 + buildSrc/src/main/kotlin/Versions.kt | 2 + src/main/kotlin/ExperimentException.kt | 6 + src/main/kotlin/LocalEvaluationClient.kt | 72 +++++++--- src/main/kotlin/LocalEvaluationConfig.kt | 4 +- src/main/kotlin/ServerZone.kt | 2 + .../kotlin/assignment/AssignmentService.kt | 35 ++++- src/main/kotlin/cohort/CohortDownloadApi.kt | 82 ++++++----- src/main/kotlin/cohort/CohortLoader.kt | 23 +++- src/main/kotlin/cohort/CohortStorage.kt | 49 +++++-- .../kotlin/deployment/DeploymentRunner.kt | 27 +++- src/main/kotlin/flag/FlagConfigApi.kt | 2 +- src/main/kotlin/flag/FlagConfigStorage.kt | 9 ++ src/main/kotlin/util/Backoff.kt | 44 ++++-- src/main/kotlin/util/Once.kt | 9 +- src/main/kotlin/util/Request.kt | 19 +-- src/test/kotlin/LocalEvaluationClientTest.kt | 122 +++++++++++++++++ .../kotlin/cohort/CohortDownloadApiTest.kt | 128 ++++++++++++++++++ src/test/kotlin/cohort/CohortLoaderTest.kt | 90 ++++++++++++ .../kotlin/deployment/DeploymentRunnerTest.kt | 83 ++++++++++++ src/test/kotlin/util/CacheTest.kt | 110 +++++++++++++++ src/test/kotlin/util/FlagConfigTest.kt | 22 +++ 22 files changed, 836 insertions(+), 106 deletions(-) create mode 100644 src/main/kotlin/ExperimentException.kt create mode 100644 src/test/kotlin/cohort/CohortDownloadApiTest.kt create mode 100644 src/test/kotlin/cohort/CohortLoaderTest.kt create mode 100644 src/test/kotlin/deployment/DeploymentRunnerTest.kt create mode 100644 src/test/kotlin/util/CacheTest.kt create mode 100644 src/test/kotlin/util/FlagConfigTest.kt diff --git a/build.gradle.kts b/build.gradle.kts index 4016d38..f8d832e 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -19,6 +19,8 @@ java { dependencies { implementation(kotlin("stdlib")) testImplementation(kotlin("test")) + testImplementation("org.mockito:mockito-core:${Versions.mockito}") + testImplementation("com.squareup.okhttp3:mockwebserver:${Versions.mockwebserver}") testImplementation("io.mockk:mockk:${Versions.mockk}") implementation("org.jetbrains.kotlinx:kotlinx-serialization-json:${Versions.serializationRuntime}") implementation("com.squareup.okhttp3:okhttp:${Versions.okhttp}") diff --git a/buildSrc/src/main/kotlin/Versions.kt b/buildSrc/src/main/kotlin/Versions.kt index cecb4b7..cfc233d 100644 --- a/buildSrc/src/main/kotlin/Versions.kt +++ b/buildSrc/src/main/kotlin/Versions.kt @@ -9,4 +9,6 @@ object Versions { const val evaluationCore = "2.0.0-beta.2" const val amplitudeAnalytics = "1.12.0" const val mockk = "1.13.9" + const val mockito = "4.8.1" + const val mockwebserver = "4.12.0" } diff --git a/src/main/kotlin/ExperimentException.kt b/src/main/kotlin/ExperimentException.kt new file mode 100644 index 0000000..1c94e9c --- /dev/null +++ b/src/main/kotlin/ExperimentException.kt @@ -0,0 +1,6 @@ +package com.amplitude.experiment + +class ExperimentException( + override val message: String? = null, + override val cause: Throwable? = null +) : Exception() diff --git a/src/main/kotlin/LocalEvaluationClient.kt b/src/main/kotlin/LocalEvaluationClient.kt index 9685b4e..6299c7e 100644 --- a/src/main/kotlin/LocalEvaluationClient.kt +++ b/src/main/kotlin/LocalEvaluationClient.kt @@ -6,6 +6,7 @@ import com.amplitude.experiment.assignment.AmplitudeAssignmentService import com.amplitude.experiment.assignment.Assignment import com.amplitude.experiment.assignment.AssignmentService import com.amplitude.experiment.assignment.InMemoryAssignmentFilter +import com.amplitude.experiment.cohort.CohortDownloadApi import com.amplitude.experiment.cohort.DirectCohortDownloadApi import com.amplitude.experiment.cohort.InMemoryCohortStorage import com.amplitude.experiment.deployment.DeploymentRunner @@ -29,29 +30,18 @@ import okhttp3.HttpUrl.Companion.toHttpUrl import okhttp3.OkHttpClient class LocalEvaluationClient internal constructor( - private val apiKey: String, + apiKey: String, private val config: LocalEvaluationConfig = LocalEvaluationConfig(), + private val httpClient: OkHttpClient = OkHttpClient(), + cohortDownloadApi: CohortDownloadApi? = getCohortDownloadApi(config, httpClient) ) { private val startLock = Once() - private val httpClient = OkHttpClient() private val assignmentService: AssignmentService? = createAssignmentService(apiKey) private val serverUrl: HttpUrl = getServerUrl(config) - private val cohortServerUrl: HttpUrl = getCohortServerUrl(config) private val evaluation: EvaluationEngine = EvaluationEngineImpl() private val metrics: LocalEvaluationMetrics = LocalEvaluationMetricsWrapper(config.metrics) private val flagConfigApi = DirectFlagConfigApi(apiKey, serverUrl, httpClient) private val flagConfigStorage = InMemoryFlagConfigStorage() - private val cohortDownloadApi = if (config.cohortSyncConfiguration != null) { - DirectCohortDownloadApi( - apiKey = config.cohortSyncConfiguration.apiKey, - secretKey = config.cohortSyncConfiguration.secretKey, - maxCohortSize = config.cohortSyncConfiguration.maxCohortSize, - serverUrl = cohortServerUrl, - httpClient = httpClient, - ) - } else { - null - } private val cohortStorage = if (config.cohortSyncConfiguration != null) { InMemoryCohortStorage() } else { @@ -68,8 +58,13 @@ class LocalEvaluationClient internal constructor( ) fun start() { - startLock.once { + try { deploymentRunner.start() + } catch (t: Throwable) { + throw ExperimentException( + message = "Failed to start local evaluation client.", + cause = t + ) } } @@ -82,7 +77,7 @@ class LocalEvaluationClient internal constructor( setEventUploadPeriodMillis(config.assignmentConfiguration.eventUploadPeriodMillis) useBatchMode(config.assignmentConfiguration.useBatchMode) setOptions(Options().setMinIdLength(1)) - setServerUrl(config.assignmentConfiguration.serverUrl) + setServerUrl(getEventServerUrl(config, config.assignmentConfiguration)) }, InMemoryAssignmentFilter(config.assignmentConfiguration.cacheCapacity) ) @@ -103,7 +98,7 @@ class LocalEvaluationClient internal constructor( if (sortedFlagConfigs.isEmpty()) { return mapOf() } - val enrichedUser = enrichUser(user, flagConfigs) + val enrichedUser = enrichUser(user, sortedFlagConfigs) val evaluationResults = wrapMetrics( metric = metrics::onEvaluation, failure = metrics::onEvaluationFailure, @@ -116,11 +111,20 @@ class LocalEvaluationClient internal constructor( return variants } - private fun enrichUser(user: ExperimentUser, flagConfigs: Map): ExperimentUser { + private fun enrichUser(user: ExperimentUser, flagConfigs: List): ExperimentUser { + val groupedCohortIds = flagConfigs.getGroupedCohortIds() + val allCohortIds = groupedCohortIds.values.flatten().toSet() if (cohortStorage == null) { + if (groupedCohortIds.isNotEmpty()) { + Logger.e("Flags are targeting local evaluation cohorts $allCohortIds, but cohort downloads have not been configured in the SDK on initialization.") + } return user } - val groupedCohortIds = flagConfigs.values.getGroupedCohortIds() + for (cohortId in allCohortIds) { + if (cohortStorage.getCohort(cohortId) == null) { + throw ExperimentException("Targeted cohort $cohortId has not been downloaded indicating an error has occurred.") + } + } return user.copyToBuilder().apply { val userCohortsIds = groupedCohortIds[USER_GROUP_TYPE] if (!userCohortsIds.isNullOrEmpty() && user.userId != null) { @@ -143,6 +147,22 @@ class LocalEvaluationClient internal constructor( } }.build() } + + +} + +private fun getCohortDownloadApi(config: LocalEvaluationConfig, httpClient: OkHttpClient): CohortDownloadApi? { + return if (config.cohortSyncConfiguration != null) { + DirectCohortDownloadApi( + apiKey = config.cohortSyncConfiguration.apiKey, + secretKey = config.cohortSyncConfiguration.secretKey, + maxCohortSize = config.cohortSyncConfiguration.maxCohortSize, + serverUrl = getCohortServerUrl(config), + httpClient = httpClient, + ) + } else { + null + } } private fun getServerUrl(config: LocalEvaluationConfig): HttpUrl { @@ -166,3 +186,17 @@ private fun getCohortServerUrl(config: LocalEvaluationConfig): HttpUrl { } } } + +private fun getEventServerUrl( + config: LocalEvaluationConfig, + assignmentConfiguration: AssignmentConfiguration +): String { + if (config.serverZone == LocalEvaluationConfig.Defaults.SERVER_ZONE) { + return assignmentConfiguration.serverUrl + } else { + return when (config.serverZone) { + ServerZone.US -> US_EVENT_SERVER_URL + ServerZone.EU -> EU_EVENT_SERVER_URL + } + } +} diff --git a/src/main/kotlin/LocalEvaluationConfig.kt b/src/main/kotlin/LocalEvaluationConfig.kt index d2a4b02..7923798 100644 --- a/src/main/kotlin/LocalEvaluationConfig.kt +++ b/src/main/kotlin/LocalEvaluationConfig.kt @@ -54,7 +54,7 @@ class LocalEvaluationConfig internal constructor( /** * "https://api.lab.amplitude.com/" */ - const val COHORT_SERVER_URL = "https://cohorts-v2.lab.amplitude.com/" + const val COHORT_SERVER_URL = "https://cohort-v2.lab.amplitude.com/" /** * ServerZone.US @@ -169,7 +169,7 @@ data class AssignmentConfiguration( data class CohortSyncConfiguration( val apiKey: String, val secretKey: String, - val maxCohortSize: Int = 15000, + val maxCohortSize: Int = Int.MAX_VALUE, ) @ExperimentalApi diff --git a/src/main/kotlin/ServerZone.kt b/src/main/kotlin/ServerZone.kt index eba3a00..c5d5dc3 100644 --- a/src/main/kotlin/ServerZone.kt +++ b/src/main/kotlin/ServerZone.kt @@ -4,6 +4,8 @@ internal const val US_SERVER_URL = "https://api.lab.amplitude.com" internal const val EU_SERVER_URL = "https://api.lab.eu.amplitude.com" internal const val US_COHORT_SERVER_URL = "https://cohort-v2.lab.amplitude.com" internal const val EU_COHORT_SERVER_URL = "https://cohort-v2.lab.eu.amplitude.com" +internal const val US_EVENT_SERVER_URL = "https://api2.amplitude.com/2/httpapi" +internal const val EU_EVENT_SERVER_URL = "https://api.eu.amplitude.com/2/httpapi" enum class ServerZone { US, EU diff --git a/src/main/kotlin/assignment/AssignmentService.kt b/src/main/kotlin/assignment/AssignmentService.kt index ac795a2..322dd47 100644 --- a/src/main/kotlin/assignment/AssignmentService.kt +++ b/src/main/kotlin/assignment/AssignmentService.kt @@ -1,7 +1,12 @@ package com.amplitude.experiment.assignment import com.amplitude.Amplitude +import com.amplitude.AmplitudeCallbacks import com.amplitude.Event +import com.amplitude.experiment.ExperimentalApi +import com.amplitude.experiment.LocalEvaluationMetrics +import com.amplitude.experiment.util.LocalEvaluationMetricsWrapper +import com.amplitude.experiment.util.wrapMetrics import org.json.JSONObject private object FlagType { @@ -16,14 +21,42 @@ internal interface AssignmentService { fun track(assignment: Assignment) } +class EventTrackingException( + event: Event, + status: Int, + message: String? +) : Exception( + "Failed to track event to amplitude: event=${event.eventType}, status=$status, msg=$message" +) + internal class AmplitudeAssignmentService( private val amplitude: Amplitude, private val assignmentFilter: AssignmentFilter, + private val metrics: LocalEvaluationMetrics = LocalEvaluationMetricsWrapper(), ) : AssignmentService { + init { + amplitude.setCallbacks(object : AmplitudeCallbacks() { + override fun onLogEventServerResponse(event: Event?, status: Int, message: String?) { + if (event == null) return + if (status != 200) { + metrics.onAssignmentEventFailure(EventTrackingException(event, status, message)) + } + } + }) + } + override fun track(assignment: Assignment) { + metrics.onAssignment() if (assignmentFilter.shouldTrack(assignment)) { - amplitude.logEvent(assignment.toAmplitudeEvent()) + wrapMetrics( + metric = metrics::onAssignmentEvent, + failure = metrics::onAssignmentEventFailure, + ) { + amplitude.logEvent(assignment.toAmplitudeEvent()) + } + } else { + metrics.onAssignmentFilter() } } } diff --git a/src/main/kotlin/cohort/CohortDownloadApi.kt b/src/main/kotlin/cohort/CohortDownloadApi.kt index 7e18993..63d35a6 100644 --- a/src/main/kotlin/cohort/CohortDownloadApi.kt +++ b/src/main/kotlin/cohort/CohortDownloadApi.kt @@ -1,31 +1,40 @@ package com.amplitude.experiment.cohort import com.amplitude.experiment.LIBRARY_VERSION +import com.amplitude.experiment.util.BackoffConfig import com.amplitude.experiment.util.HttpErrorResponseException import com.amplitude.experiment.util.Logger +import com.amplitude.experiment.util.backoff import com.amplitude.experiment.util.get import kotlinx.serialization.Serializable import okhttp3.HttpUrl import okhttp3.OkHttpClient import java.util.Base64 +import java.util.concurrent.ExecutionException -internal class CohortTooLargeException(cohortId: String, maxCohortSize: Int) : Exception( +internal class CohortTooLargeException(cohortId: String, maxCohortSize: Int) : RuntimeException( "Cohort $cohortId exceeds the maximum cohort size defined in the SDK configuration $maxCohortSize" ) -internal class CohortNotModifiedException(cohortId: String) : Exception( +internal class CohortNotModifiedException(cohortId: String) : RuntimeException( "Cohort $cohortId has not been modified." ) @Serializable -private data class GetCohortResponse( - private val id: String, +internal data class GetCohortResponse( + private val cohortId: String, private val lastModified: Long, private val size: Int, private val memberIds: Set = setOf(), private val groupType: String, ) { - fun toCohort() = Cohort(id, groupType, size, lastModified, memberIds) + fun toCohort() = Cohort( + id = cohortId, + groupType = groupType, + size = size, + lastModified = lastModified, + members = memberIds + ) } internal interface CohortDownloadApi { @@ -39,45 +48,54 @@ internal class DirectCohortDownloadApi( private val serverUrl: HttpUrl, private val httpClient: OkHttpClient, ) : CohortDownloadApi { - private val bearerToken = Base64.getEncoder().encodeToString("$apiKey:$secretKey".toByteArray()) + + private val token = Base64.getEncoder().encodeToString("$apiKey:$secretKey".toByteArray()) + private val backoffConfig = BackoffConfig( + attempts = 3, + min = 500, + max = 2000, + scalar = 2.0, + ) + override fun getCohort(cohortId: String, cohort: Cohort?): Cohort { Logger.d("getCohortMembers($cohortId): start") - var errors = 0 - while (true) { + val future = backoff(backoffConfig, { val headers = mapOf( - "Authorization" to "Bearer $bearerToken", + "Authorization" to "Basic $token", "X-Amp-Exp-Library" to "experiment-jvm-server/$LIBRARY_VERSION", ) val queries = mutableMapOf( "maxCohortSize" to "$maxCohortSize", ) - if (cohort != null && cohort.lastModified > 0) { + if (cohort != null) { queries["lastModified"] = "${cohort.lastModified}" } - try { - return httpClient.get( - serverUrl = serverUrl, - path = "sdk/v1/cohort/$cohortId", - headers = headers, - queries = queries, - ) { response -> - Logger.d("getCohortMembers($cohortId): status=${response.code}") - when (response.code) { - 200 -> return@get - 204 -> throw CohortNotModifiedException(cohortId) - 413 -> throw CohortTooLargeException(cohortId, maxCohortSize) - else -> throw HttpErrorResponseException(response.code) - } - }.toCohort() - } catch (e: HttpErrorResponseException) { - if (e.code == 429) { - continue + httpClient.get( + serverUrl = serverUrl, + path = "sdk/v1/cohort/$cohortId", + headers = headers, + queries = queries, + ) { response -> + Logger.d("getCohortMembers($cohortId): status=${response.code}") + when (response.code) { + 200 -> return@get + 204 -> throw CohortNotModifiedException(cohortId) + 413 -> throw CohortTooLargeException(cohortId, maxCohortSize) + else -> throw HttpErrorResponseException(response.code) } - if (errors >= 3) { - throw e - } - errors++ } + }, { e -> + // Don't retry on expected responses + when (e) { + is CohortNotModifiedException -> false + is CohortTooLargeException -> false + else -> true + } + }) + try { + return future.get().toCohort() + } catch(e: ExecutionException) { + throw e.cause ?: e } } } diff --git a/src/main/kotlin/cohort/CohortLoader.kt b/src/main/kotlin/cohort/CohortLoader.kt index fbe1fd2..3fbe01d 100644 --- a/src/main/kotlin/cohort/CohortLoader.kt +++ b/src/main/kotlin/cohort/CohortLoader.kt @@ -1,7 +1,10 @@ package com.amplitude.experiment.cohort +import com.amplitude.experiment.LocalEvaluationMetrics +import com.amplitude.experiment.util.LocalEvaluationMetricsWrapper import com.amplitude.experiment.util.Logger import com.amplitude.experiment.util.daemonFactory +import com.amplitude.experiment.util.wrapMetrics import java.util.concurrent.CompletableFuture import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.LinkedBlockingQueue @@ -11,6 +14,7 @@ import java.util.concurrent.TimeUnit internal class CohortLoader( private val cohortDownloadApi: CohortDownloadApi, private val cohortStorage: CohortStorage, + private val metrics: LocalEvaluationMetrics = LocalEvaluationMetricsWrapper() ) { private val jobs = ConcurrentHashMap>() @@ -30,13 +34,18 @@ internal class CohortLoader( CompletableFuture.runAsync({ Logger.d("Loading cohort $cohortId") val storageCohort = cohortStorage.getCohort(cohortId) - try { - val cohort = cohortDownloadApi.getCohort(cohortId, storageCohort) - cohortStorage.putCohort(cohort) - } catch (e: CohortNotModifiedException) { - // Do nothing - } catch (e: CohortTooLargeException) { - Logger.e("Cohort too large", e) + wrapMetrics( + metrics::onCohortDownload, + metrics::onCohortDownloadFailure + ) { + try { + val cohort = cohortDownloadApi.getCohort(cohortId, storageCohort) + cohortStorage.putCohort(cohort) + } catch (e: CohortNotModifiedException) { + // Do nothing + } catch (e: CohortTooLargeException) { + Logger.e("Cohort too large", e) + } } }, executor).whenComplete { _, _ -> jobs.remove(cohortId) } } diff --git a/src/main/kotlin/cohort/CohortStorage.kt b/src/main/kotlin/cohort/CohortStorage.kt index c2843ef..5cfe3a4 100644 --- a/src/main/kotlin/cohort/CohortStorage.kt +++ b/src/main/kotlin/cohort/CohortStorage.kt @@ -1,36 +1,65 @@ package com.amplitude.experiment.cohort +import com.amplitude.experiment.util.USER_GROUP_TYPE +import java.util.concurrent.locks.ReentrantReadWriteLock +import kotlin.concurrent.read +import kotlin.concurrent.write + internal interface CohortStorage { fun getCohort(cohortId: String): Cohort? - fun getCohorts(): List + fun getCohorts(): Map fun getCohortsForUser(userId: String, cohortIds: Set): Set fun getCohortsForGroup(groupType: String, groupName: String, cohortIds: Set): Set fun putCohort(cohort: Cohort) - fun deleteCohort(groupType: String, cohortId: String) + fun deleteCohort(cohortId: String) } internal class InMemoryCohortStorage : CohortStorage { + private val lock = ReentrantReadWriteLock() + private val cohortStore = mutableMapOf() + override fun getCohort(cohortId: String): Cohort? { - TODO("Not yet implemented") + lock.read { + return cohortStore[cohortId] + } } - override fun getCohorts(): List { - TODO("Not yet implemented") + override fun getCohorts(): Map { + lock.read { + return cohortStore.toMap() + } } override fun getCohortsForUser(userId: String, cohortIds: Set): Set { - TODO("Not yet implemented") + return getCohortsForGroup(USER_GROUP_TYPE, userId, cohortIds) } override fun getCohortsForGroup(groupType: String, groupName: String, cohortIds: Set): Set { - TODO("Not yet implemented") + val result = mutableSetOf() + lock.read { + for (cohortId in cohortIds) { + val cohort = cohortStore[cohortId] ?: continue + if (cohort.groupType != groupType) { + continue + } + if (cohort.members.contains(groupName)) { + result.add(cohortId) + } + } + } + return result } + override fun putCohort(cohort: Cohort) { - TODO("Not yet implemented") + lock.write { + cohortStore[cohort.id] = cohort + } } - override fun deleteCohort(groupType: String, cohortId: String) { - TODO("Not yet implemented") + override fun deleteCohort(cohortId: String) { + lock.write { + cohortStore.remove(cohortId) + } } } diff --git a/src/main/kotlin/deployment/DeploymentRunner.kt b/src/main/kotlin/deployment/DeploymentRunner.kt index f6ae4bb..b82c556 100644 --- a/src/main/kotlin/deployment/DeploymentRunner.kt +++ b/src/main/kotlin/deployment/DeploymentRunner.kt @@ -80,22 +80,35 @@ internal class DeploymentRunner( ) { flagConfigApi.getFlagConfigs() } + // Remove flags that no longer exist. val flagKeys = flagConfigs.map { it.key }.toSet() flagConfigStorage.removeIf { !flagKeys.contains(it.key) } + // Get all flags from storage + val storageFlags = flagConfigStorage.getFlagConfigs() + // Load cohorts for each flag if applicable and put the flag in storage. val futures = ConcurrentHashMap>() for (flagConfig in flagConfigs) { + if (cohortLoader == null) { + flagConfigStorage.putFlagConfig(flagConfig) + continue + } val cohortIds = flagConfig.getAllCohortIds() - if (cohortLoader == null || cohortIds.isEmpty()) { + val storageCohortIds = storageFlags[flagConfig.key]?.getAllCohortIds() ?: emptySet() + val cohortsToLoad = cohortIds - storageCohortIds + if (cohortsToLoad.isEmpty()) { flagConfigStorage.putFlagConfig(flagConfig) continue } - for (cohortId in cohortIds) { + for (cohortId in cohortsToLoad) { futures.putIfAbsent( cohortId, - cohortLoader.loadCohort(cohortId).thenRun { + cohortLoader.loadCohort(cohortId).handle { _, exception -> + if (exception != null) { + Logger.e("Failed to load cohort $cohortId", exception) + } flagConfigStorage.putFlagConfig(flagConfig) } ) @@ -106,10 +119,10 @@ internal class DeploymentRunner( // Delete unused cohorts if (cohortStorage != null) { val flagCohortIds = flagConfigStorage.getFlagConfigs().values.toList().getAllCohortIds() - val storageCohorts = cohortStorage.getCohorts().associateBy { it.id } - val deletedCohorts = storageCohorts - flagCohortIds - for (deletedCohort in deletedCohorts) { - cohortStorage.deleteCohort(deletedCohort.value.groupType, deletedCohort.key) + val storageCohortIds = cohortStorage.getCohorts().keys + val deletedCohortIds = storageCohortIds - flagCohortIds + for (deletedCohortId in deletedCohortIds) { + cohortStorage.deleteCohort(deletedCohortId) } } Logger.d("Refreshed ${flagConfigs.size} flag configs.") diff --git a/src/main/kotlin/flag/FlagConfigApi.kt b/src/main/kotlin/flag/FlagConfigApi.kt index 4fc3c5a..9829393 100644 --- a/src/main/kotlin/flag/FlagConfigApi.kt +++ b/src/main/kotlin/flag/FlagConfigApi.kt @@ -23,6 +23,6 @@ internal class DirectFlagConfigApi( "Authorization" to "Api-Key $deploymentKey", ), queries = mapOf("v" to "0") - ) + ).get() } } diff --git a/src/main/kotlin/flag/FlagConfigStorage.kt b/src/main/kotlin/flag/FlagConfigStorage.kt index 46412ae..b3ae6e5 100644 --- a/src/main/kotlin/flag/FlagConfigStorage.kt +++ b/src/main/kotlin/flag/FlagConfigStorage.kt @@ -9,6 +9,7 @@ internal interface FlagConfigStorage { fun getFlagConfigs(): Map fun putFlagConfig(flagConfig: EvaluationFlag) fun removeIf(condition: (EvaluationFlag) -> Boolean) + fun update(put: List, remove: List) } internal class InMemoryFlagConfigStorage : FlagConfigStorage { @@ -31,4 +32,12 @@ internal class InMemoryFlagConfigStorage : FlagConfigStorage { flagConfigs.entries.removeIf { condition(it.value) } } } + + override fun update(put: List, remove: List) { + + return flagConfigsLock.write { + flagConfigs.putAll(put.associateBy { it.key }) + remove.forEach { flagConfigs.remove(it.key) } + } + } } diff --git a/src/main/kotlin/util/Backoff.kt b/src/main/kotlin/util/Backoff.kt index 710fc5b..2ef13aa 100644 --- a/src/main/kotlin/util/Backoff.kt +++ b/src/main/kotlin/util/Backoff.kt @@ -2,14 +2,24 @@ package com.amplitude.experiment.util import com.amplitude.experiment.Experiment import java.util.concurrent.CompletableFuture +import java.util.concurrent.CompletionException import java.util.concurrent.TimeUnit import kotlin.math.min +import kotlin.random.Random internal fun backoff( config: BackoffConfig, function: () -> CompletableFuture, ): CompletableFuture { - return Backoff(config).start(function) + return Backoff(config).start(function, null) +} + +internal fun backoff( + config: BackoffConfig, + function: () -> CompletableFuture, + retry: (Throwable) -> Boolean, +): CompletableFuture { + return Backoff(config).start(function, retry) } internal data class BackoffConfig( @@ -25,32 +35,44 @@ private class Backoff( private val completableFuture = CompletableFuture() - fun start(function: () -> CompletableFuture): CompletableFuture { - backoff(0, config.min, function) + fun start( + function: () -> CompletableFuture, + retry: ((Throwable) -> Boolean)? + ): CompletableFuture { + backoff(0, config.min, function, retry) return completableFuture } private fun backoff( attempt: Int, delay: Long, - function: () -> CompletableFuture + function: () -> CompletableFuture, + retry: ((Throwable) -> Boolean)? = null ) { Experiment.scheduler.schedule({ if (completableFuture.isCancelled) { return@schedule } - function.invoke().whenComplete { variants, t -> - if (t != null || variants == null) { - // Retry the request function + function.invoke().whenComplete { result, t -> + if (t != null || result == null) { + val unwrapped = when (t) { + is CompletionException -> t.cause ?: t + else -> t + } + val shouldRetry = retry?.invoke(unwrapped) ?: true val nextAttempt = attempt + 1 - if (nextAttempt < config.attempts) { + if (shouldRetry && nextAttempt < config.attempts) { val nextDelay = min(delay * config.scalar, config.max.toDouble()).toLong() - backoff(nextAttempt, nextDelay, function) + val jitter = Random.nextLong( + (nextDelay - (nextDelay*0.1).toLong()) * -1, + nextDelay + (nextDelay+0.1).toLong() + ) + backoff(nextAttempt, nextDelay + jitter, function, retry) } else { - completableFuture.completeExceptionally(t) + completableFuture.completeExceptionally(unwrapped) } } else { - completableFuture.complete(variants) + completableFuture.complete(result) } } }, delay, TimeUnit.MILLISECONDS) diff --git a/src/main/kotlin/util/Once.kt b/src/main/kotlin/util/Once.kt index 0ff7adb..9e25dc8 100644 --- a/src/main/kotlin/util/Once.kt +++ b/src/main/kotlin/util/Once.kt @@ -7,6 +7,13 @@ class Once { if (done) return@once done = true } - block.invoke() + try { + block.invoke() + } catch (t: Throwable) { + synchronized(this) { + done = false + } + throw t + } } } diff --git a/src/main/kotlin/util/Request.kt b/src/main/kotlin/util/Request.kt index a9cdf75..5627bdb 100644 --- a/src/main/kotlin/util/Request.kt +++ b/src/main/kotlin/util/Request.kt @@ -41,8 +41,7 @@ internal inline fun OkHttpClient.request( internal open class HttpErrorResponseException( val code: Int, - override val message: String? = null -) : IOException() +) : IOException("Request resulted error response $code") private fun OkHttpClient.submit( request: Request, @@ -83,23 +82,13 @@ private fun newGet( return builder.build() } -internal fun OkHttpClient.get( - serverUrl: HttpUrl, - path: String? = null, - headers: Map? = null, - queries: Map? = null, -): Response { - val request = newGet(serverUrl, path, headers, queries) - return submit(request).thenApply { it.apply { close() } }.get() -} - internal inline fun OkHttpClient.get( serverUrl: HttpUrl, path: String? = null, headers: Map? = null, queries: Map? = null, crossinline block: (Response) -> Unit, -): T { +): CompletableFuture { val request = newGet(serverUrl, path, headers, queries) return submit(request).thenApply { it.use { response -> @@ -107,7 +96,7 @@ internal inline fun OkHttpClient.get( val body = response.body?.string() ?: throw IOException("null response body") json.decodeFromString(body) } - }.get() + } } internal inline fun OkHttpClient.get( @@ -115,6 +104,6 @@ internal inline fun OkHttpClient.get( path: String? = null, headers: Map? = null, queries: Map? = null, -): T { +): CompletableFuture { return this.get(serverUrl, path, headers, queries) {} } diff --git a/src/test/kotlin/LocalEvaluationClientTest.kt b/src/test/kotlin/LocalEvaluationClientTest.kt index 2ab8577..31fd208 100644 --- a/src/test/kotlin/LocalEvaluationClientTest.kt +++ b/src/test/kotlin/LocalEvaluationClientTest.kt @@ -1,6 +1,12 @@ package com.amplitude.experiment +import com.amplitude.experiment.cohort.Cohort +import com.amplitude.experiment.cohort.CohortDownloadApi +import io.mockk.every +import io.mockk.mockk import org.junit.Assert +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull import kotlin.system.measureNanoTime import kotlin.test.Test @@ -139,4 +145,120 @@ class LocalEvaluationClientTest { val variant = variants["sdk-ci-local-dependencies-test-holdout"] Assert.assertEquals(variant, null) } + + @Test + fun `evaluate with user and group, not targeted`() { + val cohortConfig = LocalEvaluationConfig( + cohortSyncConfiguration = CohortSyncConfiguration("api", "secret") + ) + val cohortDownloadApi = mockk().apply { + every { getCohort(eq("52gz3yi7"), allAny()) } returns Cohort("52gz3yi7", "User", 2, 1722363790000, setOf("1", "2")) + every { getCohort(eq("mv7fn2bp"), allAny()) } returns Cohort("mv7fn2bp", "User", 1, 1719350216000, setOf("67890", "12345")) + every { getCohort(eq("s4t57y32"), allAny()) } returns Cohort("s4t57y32", "org name", 1, 1722368285000, setOf("Amplitude Website (Portfolio)")) + every { getCohort(eq("k1lklnnb"), allAny()) } returns Cohort("k1lklnnb", "org id", 1, 1722466388000, setOf("1")) + } + val client = LocalEvaluationClient(API_KEY, cohortConfig, cohortDownloadApi = cohortDownloadApi) + client.start() + val user = ExperimentUser( + userId = "2333", + deviceId = "device_id", + groups = mapOf("org name" to setOf("Amplitude Inc sth sth sth")) + ) + val userVariant = client.evaluateV2(user, setOf("sdk-local-evaluation-user-cohort-ci-test"))["sdk-local-evaluation-user-cohort-ci-test"] + assertEquals("off", userVariant?.key) + assertNull(userVariant?.value) + val groupVariant = client.evaluateV2(user, setOf("sdk-local-evaluation-group-cohort-ci-test"))["sdk-local-evaluation-group-cohort-ci-test"] + assertEquals("off", groupVariant?.key) + assertNull(groupVariant?.value) + } + + @Test + fun `evaluate with user, cohort segment targeted`() { + val cohortConfig = LocalEvaluationConfig( + cohortSyncConfiguration = CohortSyncConfiguration("api", "secret") + ) + val cohortDownloadApi = mockk().apply { + every { getCohort(eq("52gz3yi7"), allAny()) } returns Cohort("52gz3yi7", "User", 2, 1722363790000, setOf("1", "2")) + every { getCohort(eq("mv7fn2bp"), allAny()) } returns Cohort("mv7fn2bp", "User", 1, 1719350216000, setOf("67890", "12345")) + every { getCohort(eq("s4t57y32"), allAny()) } returns Cohort("s4t57y32", "org name", 1, 1722368285000, setOf("Amplitude Website (Portfolio)")) + every { getCohort(eq("k1lklnnb"), allAny()) } returns Cohort("k1lklnnb", "org id", 1, 1722466388000, setOf("1")) + } + val client = LocalEvaluationClient(API_KEY, cohortConfig, cohortDownloadApi = cohortDownloadApi) + client.start() + val user = ExperimentUser( + userId = "12345", + deviceId = "device_id", + ) + val userVariant = client.evaluateV2(user, setOf("sdk-local-evaluation-user-cohort-ci-test"))["sdk-local-evaluation-user-cohort-ci-test"] + assertEquals("on", userVariant?.key) + assertEquals("on", userVariant?.value) + } + @Test + fun `evaluate with user, cohort tester targeted`() { + val cohortConfig = LocalEvaluationConfig( + cohortSyncConfiguration = CohortSyncConfiguration("api", "secret") + ) + val cohortDownloadApi = mockk().apply { + every { getCohort(eq("52gz3yi7"), allAny()) } returns Cohort("52gz3yi7", "User", 2, 1722363790000, setOf("1", "2")) + every { getCohort(eq("mv7fn2bp"), allAny()) } returns Cohort("mv7fn2bp", "User", 1, 1719350216000, setOf("67890", "12345")) + every { getCohort(eq("s4t57y32"), allAny()) } returns Cohort("s4t57y32", "org name", 1, 1722368285000, setOf("Amplitude Website (Portfolio)")) + every { getCohort(eq("k1lklnnb"), allAny()) } returns Cohort("k1lklnnb", "org id", 1, 1722466388000, setOf("1")) + } + val client = LocalEvaluationClient(API_KEY, cohortConfig, cohortDownloadApi = cohortDownloadApi) + client.start() + val user = ExperimentUser( + userId = "1", + deviceId = "device_id", + ) + val userVariant = client.evaluateV2(user, setOf("sdk-local-evaluation-user-cohort-ci-test"))["sdk-local-evaluation-user-cohort-ci-test"] + assertEquals("on", userVariant?.key) + assertEquals("on", userVariant?.value) + } + + @Test + fun `evaluate with group, cohort segment targeted`() { + val cohortConfig = LocalEvaluationConfig( + cohortSyncConfiguration = CohortSyncConfiguration("api", "secret") + ) + val cohortDownloadApi = mockk().apply { + every { getCohort(eq("52gz3yi7"), allAny()) } returns Cohort("52gz3yi7", "User", 2, 1722363790000, setOf("1", "2")) + every { getCohort(eq("mv7fn2bp"), allAny()) } returns Cohort("mv7fn2bp", "User", 1, 1719350216000, setOf("67890", "12345")) + every { getCohort(eq("s4t57y32"), allAny()) } returns Cohort("s4t57y32", "org name", 1, 1722368285000, setOf("Amplitude Website (Portfolio)")) + every { getCohort(eq("k1lklnnb"), allAny()) } returns Cohort("k1lklnnb", "org id", 1, 1722466388000, setOf("1")) + } + val client = LocalEvaluationClient(API_KEY, cohortConfig, cohortDownloadApi = cohortDownloadApi) + client.start() + val user = ExperimentUser( + userId = "2333", + deviceId = "device_id", + groups = mapOf("org id" to setOf("1")) + + ) + val groupVariant = client.evaluateV2(user, setOf("sdk-local-evaluation-group-cohort-ci-test"))["sdk-local-evaluation-group-cohort-ci-test"] + assertEquals("on", groupVariant?.key) + assertEquals("on", groupVariant?.value) + } + @Test + fun `evaluate with group, cohort tester targeted`() { + val cohortConfig = LocalEvaluationConfig( + cohortSyncConfiguration = CohortSyncConfiguration("api", "secret") + ) + val cohortDownloadApi = mockk().apply { + every { getCohort(eq("52gz3yi7"), allAny()) } returns Cohort("52gz3yi7", "User", 2, 1722363790000, setOf("1", "2")) + every { getCohort(eq("mv7fn2bp"), allAny()) } returns Cohort("mv7fn2bp", "User", 1, 1719350216000, setOf("67890", "12345")) + every { getCohort(eq("s4t57y32"), allAny()) } returns Cohort("s4t57y32", "org name", 1, 1722368285000, setOf("Amplitude Website (Portfolio)")) + every { getCohort(eq("k1lklnnb"), allAny()) } returns Cohort("k1lklnnb", "org id", 1, 1722466388000, setOf("1")) + } + val client = LocalEvaluationClient(API_KEY, cohortConfig, cohortDownloadApi = cohortDownloadApi) + client.start() + val user = ExperimentUser( + userId = "2333", + deviceId = "device_id", + groups = mapOf("org name" to setOf("Amplitude Website (Portfolio)")) + + ) + val groupVariant = client.evaluateV2(user, setOf("sdk-local-evaluation-group-cohort-ci-test"))["sdk-local-evaluation-group-cohort-ci-test"] + assertEquals("on", groupVariant?.key) + assertEquals("on", groupVariant?.value) + } } diff --git a/src/test/kotlin/cohort/CohortDownloadApiTest.kt b/src/test/kotlin/cohort/CohortDownloadApiTest.kt new file mode 100644 index 0000000..bbcf801 --- /dev/null +++ b/src/test/kotlin/cohort/CohortDownloadApiTest.kt @@ -0,0 +1,128 @@ +package com.amplitude.experiment.cohort + +import com.amplitude.experiment.util.HttpErrorResponseException +import com.amplitude.experiment.util.json +import kotlinx.serialization.encodeToString +import okhttp3.OkHttpClient +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import java.util.Base64 +import kotlin.test.Test +import kotlin.test.fail + +class CohortDownloadApiTest { + + private val apiKey = "api" + private val secretKey = "secret" + private val maxCohortSize = Int.MAX_VALUE + private val httpClient = OkHttpClient() + private val server = MockWebServer() + private val url = server.url("/") + private val api = DirectCohortDownloadApi(apiKey,secretKey, maxCohortSize, url, httpClient) + @Test + fun `cohort download, success`() { + val response = cohortResponse("a", setOf("1")) + server.enqueue(MockResponse().setResponseCode(200).setBody(json.encodeToString(response))) + val cohort = api.getCohort("a", null) + assertEquals(cohort("a", setOf("1")), cohort) + } + + @Test + fun `cohort download, null cohort input request validation`() { + val response = cohortResponse("a", setOf("1")) + server.enqueue(MockResponse().setResponseCode(200).setBody(json.encodeToString(response))) + api.getCohort("a", null) + val request = server.takeRequest() + val expectedAuth = Base64.getEncoder().encodeToString("$apiKey:$secretKey".toByteArray()) + val actualAuth = request.headers["Authorization"] + assertEquals("Basic $expectedAuth", actualAuth) + val actualMaxCohortSize = request.requestUrl?.queryParameter("maxCohortSize") + assertEquals("$maxCohortSize", actualMaxCohortSize) + val actualLastModified = request.requestUrl?.queryParameter("lastModified") + assertNull(actualLastModified) + } + + @Test + fun `cohort download, with cohort input request validation`() { + val response = cohortResponse("a", setOf("1")) + server.enqueue(MockResponse().setResponseCode(200).setBody(json.encodeToString(response))) + val cohort = cohort("a", setOf("1")) + api.getCohort("a", cohort) + val request = server.takeRequest() + val expectedAuth = Base64.getEncoder().encodeToString("$apiKey:$secretKey".toByteArray()) + val actualAuth = request.headers["Authorization"] + assertEquals("Basic $expectedAuth", actualAuth) + val actualMaxCohortSize = request.requestUrl?.queryParameter("maxCohortSize") + assertEquals("$maxCohortSize", actualMaxCohortSize) + val actualLastModified = request.requestUrl?.queryParameter("lastModified") + assertEquals("${cohort.lastModified}", actualLastModified) + } + + @Test + fun `cohort download, 204 not modified, throws exception`() { + server.enqueue(MockResponse().setResponseCode(204)) + try { + api.getCohort("a", null) + fail("Expected getCohort to throw CohortNotModifiedException") + } catch (e: CohortNotModifiedException) { + // Expected + } + } + + @Test + fun `cohort download, 413 too large, throws exception`() { + server.enqueue(MockResponse().setResponseCode(413)) + try { + api.getCohort("a", null) + fail("Expected getCohort to throw CohortTooLargeException") + } catch (e: CohortTooLargeException) { + // Expected + } + } + + @Test + fun `cohort download, 503 service unavailable, retries 3 times then throws`() { + server.enqueue(MockResponse().setResponseCode(501)) + server.enqueue(MockResponse().setResponseCode(502)) + server.enqueue(MockResponse().setResponseCode(503)) + // Should not be sent in response + server.enqueue(MockResponse().setResponseCode(204)) + try { + api.getCohort("a", null) + fail("Expected getCohort to throw HttpErrorResponseException") + } catch (e: HttpErrorResponseException) { + // Expected + assertEquals(503, e.code) + } + } + + @Test + fun `cohort download, 503 service unavailable, 2 errors, 3rd attempt success`() { + val response = cohortResponse("a", setOf("1")) + server.enqueue(MockResponse().setResponseCode(501)) + server.enqueue(MockResponse().setResponseCode(502)) + server.enqueue(MockResponse().setResponseCode(200).setBody(json.encodeToString(response))) + val cohort = api.getCohort("a", null) + assertEquals(cohort("a", setOf("1")), cohort) + } +} + +private fun cohortResponse(id: String, members: Set, lastModified: Long = 1234) = + GetCohortResponse( + cohortId = id, + memberIds = members, + lastModified = lastModified, + size = members.size, + groupType = "User", + ) + +private fun cohort(id: String, members: Set, lastModified: Long = 1234) = + Cohort( + id = id, + members = members, + lastModified = lastModified, + size = members.size, + groupType = "User", + ) diff --git a/src/test/kotlin/cohort/CohortLoaderTest.kt b/src/test/kotlin/cohort/CohortLoaderTest.kt new file mode 100644 index 0000000..9a71577 --- /dev/null +++ b/src/test/kotlin/cohort/CohortLoaderTest.kt @@ -0,0 +1,90 @@ +package com.amplitude.experiment.cohort + +import junit.framework.TestCase.assertEquals +import junit.framework.TestCase.assertNull +import org.junit.Assert.assertThrows +import org.mockito.Mockito.mock +import org.mockito.Mockito.`when` +import java.util.concurrent.ExecutionException +import kotlin.test.Test + +class CohortLoaderTest { + + @Test + fun `test load, success`() { + val cohortA = cohort("a", setOf("1")) + val cohortB = cohort("b", setOf("1", "2")) + val api = mock(CohortDownloadApi::class.java) + `when`(api.getCohort("a", null)).thenReturn(cohortA) + `when`(api.getCohort("b", null)).thenReturn(cohortB) + val storage = InMemoryCohortStorage() + val loader = CohortLoader(api, storage) + loader.loadCohort("a").get() + loader.loadCohort("b").get() + val storageCohortA = storage.getCohort("a") + val storageCohortB = storage.getCohort("b") + assertEquals(cohortA, storageCohortA) + assertEquals(cohortB, storageCohortB) + } + + @Test + fun `test load, cohorts greater than max cohort size are not downloaded`() { + val cohortB = cohort("b", setOf("1", "2")) + val api = mock(CohortDownloadApi::class.java) + `when`(api.getCohort("a", null)).thenThrow(CohortTooLargeException("a", 15000)) + `when`(api.getCohort("b", null)).thenReturn(cohortB) + val storage = InMemoryCohortStorage() + val loader = CohortLoader(api, storage) + loader.loadCohort("a").get() + loader.loadCohort("b").get() + val storageDescriptionA = storage.getCohort("a") + val storageDescriptionB = storage.getCohort("b") + assertNull(storageDescriptionA) + assertEquals(cohortB, storageDescriptionB) + } + + @Test + fun `test load, unchanged cohorts dont change`() { + val storageCohortA = cohort("a", setOf("1")) + val storageCohortB = cohort("b", setOf("1", "2")) + val storage = InMemoryCohortStorage() + storage.putCohort(storageCohortA) + storage.putCohort(storageCohortB) + val networkCohortB = cohort("b", setOf("1", "2")) + val api = mock(CohortDownloadApi::class.java) + `when`(api.getCohort("a", storageCohortA)).thenThrow(CohortNotModifiedException("a")) + `when`(api.getCohort("b", storageCohortB)).thenReturn(networkCohortB) + val loader = CohortLoader(api, storage) + loader.loadCohort("a").get() + loader.loadCohort("b").get() + val newStorageCohortA = storage.getCohort("a") + val newStorageCohortB = storage.getCohort("b") + assertEquals(storageCohortA, newStorageCohortA) + assertEquals(networkCohortB, newStorageCohortB) + } + + @Test + fun `test load, download failure throws`() { + val cohortA = cohort("a", setOf("1")) + val cohortC = cohort("c", setOf("1", "2", "3")) + val api = mock(CohortDownloadApi::class.java) + val storage = InMemoryCohortStorage() + val loader = CohortLoader(api, storage) + `when`(api.getCohort("a", null)).thenReturn(cohortA) + `when`(api.getCohort("b", null)).thenThrow(RuntimeException("Connection timed out")) + `when`(api.getCohort("c", null)).thenReturn(cohortC) + loader.loadCohort("a").get() + assertThrows(ExecutionException::class.java) { loader.loadCohort("b").get() } + loader.loadCohort("c").get() + assertEquals(setOf("a", "c"), storage.getCohortsForUser("1", setOf("a", "b", "c"))) + } +} + +private fun cohort(id: String, members: Set, lastModified: Long = 1234) = + Cohort( + id = id, + members = members, + lastModified = lastModified, + size = members.size, + groupType = "User", + ) diff --git a/src/test/kotlin/deployment/DeploymentRunnerTest.kt b/src/test/kotlin/deployment/DeploymentRunnerTest.kt new file mode 100644 index 0000000..ba76dd7 --- /dev/null +++ b/src/test/kotlin/deployment/DeploymentRunnerTest.kt @@ -0,0 +1,83 @@ +package com.amplitude.experiment.deployment + +import com.amplitude.experiment.LocalEvaluationConfig +import com.amplitude.experiment.cohort.CohortDownloadApi +import com.amplitude.experiment.cohort.CohortStorage +import com.amplitude.experiment.evaluation.EvaluationCondition +import com.amplitude.experiment.evaluation.EvaluationFlag +import com.amplitude.experiment.evaluation.EvaluationOperator +import com.amplitude.experiment.evaluation.EvaluationSegment +import com.amplitude.experiment.flag.FlagConfigApi +import com.amplitude.experiment.flag.FlagConfigStorage +import org.mockito.Mockito +import kotlin.test.Test +import kotlin.test.fail + +private const val COHORT_ID = "1234" + +class DeploymentRunnerTest { + + val flag = EvaluationFlag( + key = "flag", + variants = mapOf(), + segments = listOf( + EvaluationSegment( + conditions = listOf( + listOf( + EvaluationCondition( + selector = listOf("context", "user", "cohort_ids"), + op = EvaluationOperator.SET_CONTAINS_ANY, + values = setOf(COHORT_ID), + ) + ) + ), + ) + ) + ) + + @Test + fun `test start throws if first flag config load fails`() { + val flagApi = Mockito.mock(FlagConfigApi::class.java) + val cohortDownloadApi = Mockito.mock(CohortDownloadApi::class.java) + val flagConfigStorage = Mockito.mock(FlagConfigStorage::class.java) + val cohortStorage = Mockito.mock(CohortStorage::class.java) + val runner = DeploymentRunner( + LocalEvaluationConfig(), + flagApi, + flagConfigStorage, + cohortDownloadApi, + cohortStorage, + ) + Mockito.`when`(flagApi.getFlagConfigs()).thenThrow(RuntimeException("test")) + try { + runner.start() + fail("expected start() to throw an exception") + } catch (e: Exception) { + // pass + } + // Should be able to call start again + try { + runner.start() + fail("expected start() to throw an exception") + } catch (e: Exception) { + // pass + } + } + + @Test + fun `test start does not throw if first cohort load fails`() { + val flagApi = Mockito.mock(FlagConfigApi::class.java) + val cohortDownloadApi = Mockito.mock(CohortDownloadApi::class.java) + val flagConfigStorage = Mockito.mock(FlagConfigStorage::class.java) + val cohortStorage = Mockito.mock(CohortStorage::class.java) + val runner = DeploymentRunner( + LocalEvaluationConfig(), + flagApi, flagConfigStorage, + cohortDownloadApi, + cohortStorage, + ) + Mockito.`when`(flagApi.getFlagConfigs()).thenReturn(listOf(flag)) + Mockito.`when`(cohortDownloadApi.getCohort(COHORT_ID, null)).thenThrow(RuntimeException("test")) + runner.start() + } +} diff --git a/src/test/kotlin/util/CacheTest.kt b/src/test/kotlin/util/CacheTest.kt new file mode 100644 index 0000000..0916197 --- /dev/null +++ b/src/test/kotlin/util/CacheTest.kt @@ -0,0 +1,110 @@ +package com.amplitude.experiment.util + +import org.junit.Assert +import org.junit.Test +import java.util.concurrent.Executors +import java.util.concurrent.Future + +class CacheTest { + + @Test + fun `test get no entry`() { + val cache = Cache(4) + val value = cache[0] + Assert.assertNull(value) + } + + @Test + fun `test set and get`() { + val cache = Cache(4) + cache[0] = 0 + val value = cache[0] + Assert.assertEquals(0, value) + } + + @Test + fun `test least recently used entry is removed`() { + val cache = Cache(4) + repeat(4) { i -> + cache[i] = i + } + cache[4] = 4 + val value = cache[0] + Assert.assertNull(value) + } + + @Test + fun `test first set then get entry is not removed`() { + val cache = Cache(4) + repeat(4) { i -> + cache[i] = i + } + val expectedValue = cache[0] + cache[4] = 4 + val actualValue = cache[0] + Assert.assertEquals(expectedValue, actualValue) + val removedValue = cache[1] + Assert.assertNull(removedValue) + } + + @Test + fun `test first set then re-set entry is not removed`() { + val cache = Cache(4) + repeat(4) { i -> + cache[i] = i + } + cache[0] = 0 + cache[4] = 4 + val actualValue = cache[0] + Assert.assertEquals(0, actualValue) + val removedValue = cache[1] + Assert.assertNull(removedValue) + } + + @Test + fun `test first set then re-set with different value entry is not removed`() { + val cache = Cache(4) + repeat(4) { i -> + cache[i] = i + } + cache[0] = 100 + cache[4] = 4 + val actualValue = cache[0] + Assert.assertEquals(100, actualValue) + val removedValue = cache[1] + Assert.assertNull(removedValue) + } + + @Test + fun `test concurrent access`() { + val n = 100 + val executor = Executors.newFixedThreadPool(n, daemonFactory) + val cache = Cache(n) + val futures = mutableListOf>() + repeat(n) { i -> + futures.add( + executor.submit { + cache[i] = i + } + ) + } + futures.forEach { f -> f.get() } + repeat(n) { i -> + Assert.assertEquals(i, cache[i]) + } + futures.clear() + val k = 50 + repeat(k) { i -> + futures.add( + executor.submit { + cache[i + k] = i + k + } + ) + } + futures.forEach { f -> f.get() } + repeat(k) { i -> + Assert.assertEquals(i, cache[i]) + Assert.assertEquals(i + k, cache[i + k]) + } + } +} diff --git a/src/test/kotlin/util/FlagConfigTest.kt b/src/test/kotlin/util/FlagConfigTest.kt new file mode 100644 index 0000000..989d6ce --- /dev/null +++ b/src/test/kotlin/util/FlagConfigTest.kt @@ -0,0 +1,22 @@ +package com.amplitude.experiment.util + +import com.amplitude.experiment.evaluation.EvaluationFlag +import kotlinx.serialization.decodeFromString +import org.junit.Assert.assertEquals +import kotlin.test.Test + +class FlagConfigTest { + + val testFlagsJson = """[{"key":"flag1","segments":[{"conditions":[[{"op":"set contains any","selector":["context","user","cohort_ids"],"values":["hahahaha1"]}]]},{"metadata":{"segmentName":"All Other Users"},"variant":"off"}],"variants":{}},{"key":"flag2","segments":[{"conditions":[[{"op":"set contains any","selector":["context","user","cohort_ids"],"values":["hahahaha2"]}]],"metadata":{"segmentName":"Segment 1"},"variant":"off"},{"metadata":{"segmentName":"All Other Users"},"variant":"off"}],"variants":{}},{"key":"flag3","metadata":{"deployed":true,"evaluationMode":"local","experimentKey":"exp-1","flagType":"experiment","flagVersion":6},"segments":[{"conditions":[[{"op":"set contains any","selector":["context","user","cohort_ids"],"values":["hahahaha3"]}]],"variant":"off"},{"conditions":[[{"op":"set contains any","selector":["context","user","cocoids"],"values":["nohaha"]}]],"variant":"off"},{"metadata":{"segmentName":"All Other Users"},"variant":"off"}],"variants":{}},{"key":"flag5","segments":[{"conditions":[[{"op":"set contains any","selector":["context","user","cohort_ids"],"values":["hahahaha3","hahahaha4"]}]]},{"conditions":[[{"op":"set contains any","selector":["context","groups","org name","cohort_ids"],"values":["hahaorgname1"]}]],"metadata":{"segmentName":"Segment 1"}},{"conditions":[[{"op":"set contains any","selector":["context","gg","org name","cohort_ids"],"values":["nohahaorgname"]}]],"metadata":{"segmentName":"Segment 1"}}],"variants":{}}]""" + val testFlags = json.decodeFromString>(testFlagsJson) + + @Test + fun `test get grouped cohort ids from flags`() { + val result = testFlags.getGroupedCohortIds() + val expected = mapOf( + "User" to setOf("hahahaha1", "hahahaha2", "hahahaha3", "hahahaha4"), + "org name" to setOf("hahaorgname1"), + ) + assertEquals(expected, result) + } +} From 13d4c49612162b8b31ec260e2e9ee9a19ae640b3 Mon Sep 17 00:00:00 2001 From: Brian Giori Date: Thu, 1 Aug 2024 15:29:08 -0700 Subject: [PATCH 04/15] feat: add proxy support --- src/main/kotlin/LocalEvaluationClient.kt | 21 +--- src/main/kotlin/LocalEvaluationConfig.kt | 23 ++++ .../{CohortDownloadApi.kt => CohortApi.kt} | 6 +- src/main/kotlin/cohort/CohortLoader.kt | 4 +- src/main/kotlin/cohort/CohortMembershipApi.kt | 26 ++++ src/main/kotlin/cohort/CohortStorage.kt | 76 +++++++++++ .../kotlin/deployment/DeploymentRunner.kt | 8 +- src/main/kotlin/util/Metrics.kt | 10 ++ src/test/kotlin/LocalEvaluationClientTest.kt | 22 ++-- .../kotlin/cohort/CohortDownloadApiTest.kt | 2 +- src/test/kotlin/cohort/CohortLoaderTest.kt | 8 +- src/test/kotlin/cohort/CohortStorageTest.kt | 119 ++++++++++++++++++ .../kotlin/deployment/DeploymentRunnerTest.kt | 12 +- 13 files changed, 291 insertions(+), 46 deletions(-) rename src/main/kotlin/cohort/{CohortDownloadApi.kt => CohortApi.kt} (96%) create mode 100644 src/main/kotlin/cohort/CohortMembershipApi.kt create mode 100644 src/test/kotlin/cohort/CohortStorageTest.kt diff --git a/src/main/kotlin/LocalEvaluationClient.kt b/src/main/kotlin/LocalEvaluationClient.kt index 6299c7e..a2f07a3 100644 --- a/src/main/kotlin/LocalEvaluationClient.kt +++ b/src/main/kotlin/LocalEvaluationClient.kt @@ -6,8 +6,8 @@ import com.amplitude.experiment.assignment.AmplitudeAssignmentService import com.amplitude.experiment.assignment.Assignment import com.amplitude.experiment.assignment.AssignmentService import com.amplitude.experiment.assignment.InMemoryAssignmentFilter -import com.amplitude.experiment.cohort.CohortDownloadApi -import com.amplitude.experiment.cohort.DirectCohortDownloadApi +import com.amplitude.experiment.cohort.CohortApi +import com.amplitude.experiment.cohort.DirectCohortApi import com.amplitude.experiment.cohort.InMemoryCohortStorage import com.amplitude.experiment.deployment.DeploymentRunner import com.amplitude.experiment.evaluation.EvaluationEngine @@ -33,7 +33,7 @@ class LocalEvaluationClient internal constructor( apiKey: String, private val config: LocalEvaluationConfig = LocalEvaluationConfig(), private val httpClient: OkHttpClient = OkHttpClient(), - cohortDownloadApi: CohortDownloadApi? = getCohortDownloadApi(config, httpClient) + cohortApi: CohortApi? = getCohortDownloadApi(config, httpClient) ) { private val startLock = Once() private val assignmentService: AssignmentService? = createAssignmentService(apiKey) @@ -52,7 +52,7 @@ class LocalEvaluationClient internal constructor( config = config, flagConfigApi = flagConfigApi, flagConfigStorage = flagConfigStorage, - cohortDownloadApi = cohortDownloadApi, + cohortApi = cohortApi, cohortStorage = cohortStorage, metrics = metrics, ) @@ -113,18 +113,9 @@ class LocalEvaluationClient internal constructor( private fun enrichUser(user: ExperimentUser, flagConfigs: List): ExperimentUser { val groupedCohortIds = flagConfigs.getGroupedCohortIds() - val allCohortIds = groupedCohortIds.values.flatten().toSet() if (cohortStorage == null) { - if (groupedCohortIds.isNotEmpty()) { - Logger.e("Flags are targeting local evaluation cohorts $allCohortIds, but cohort downloads have not been configured in the SDK on initialization.") - } return user } - for (cohortId in allCohortIds) { - if (cohortStorage.getCohort(cohortId) == null) { - throw ExperimentException("Targeted cohort $cohortId has not been downloaded indicating an error has occurred.") - } - } return user.copyToBuilder().apply { val userCohortsIds = groupedCohortIds[USER_GROUP_TYPE] if (!userCohortsIds.isNullOrEmpty() && user.userId != null) { @@ -151,9 +142,9 @@ class LocalEvaluationClient internal constructor( } -private fun getCohortDownloadApi(config: LocalEvaluationConfig, httpClient: OkHttpClient): CohortDownloadApi? { +private fun getCohortDownloadApi(config: LocalEvaluationConfig, httpClient: OkHttpClient): CohortApi? { return if (config.cohortSyncConfiguration != null) { - DirectCohortDownloadApi( + DirectCohortApi( apiKey = config.cohortSyncConfiguration.apiKey, secretKey = config.cohortSyncConfiguration.secretKey, maxCohortSize = config.cohortSyncConfiguration.maxCohortSize, diff --git a/src/main/kotlin/LocalEvaluationConfig.kt b/src/main/kotlin/LocalEvaluationConfig.kt index 7923798..bf45fb9 100644 --- a/src/main/kotlin/LocalEvaluationConfig.kt +++ b/src/main/kotlin/LocalEvaluationConfig.kt @@ -28,6 +28,8 @@ class LocalEvaluationConfig internal constructor( @JvmField val cohortSyncConfiguration: CohortSyncConfiguration? = Defaults.COHORT_SYNC_CONFIGURATION, @JvmField + val evaluationProxyConfiguration: EvaluationProxyConfiguration? = Defaults.EVALUATION_PROXY_CONFIGURATION, + @JvmField val metrics: LocalEvaluationMetrics? = Defaults.LOCAL_EVALUATION_METRICS, ) { @@ -81,6 +83,11 @@ class LocalEvaluationConfig internal constructor( */ val COHORT_SYNC_CONFIGURATION: CohortSyncConfiguration? = null + /** + * null + */ + val EVALUATION_PROXY_CONFIGURATION: EvaluationProxyConfiguration? = null + /** * null */ @@ -102,6 +109,7 @@ class LocalEvaluationConfig internal constructor( private var flagConfigPollerRequestTimeoutMillis = Defaults.FLAG_CONFIG_POLLER_REQUEST_TIMEOUT_MILLIS private var assignmentConfiguration = Defaults.ASSIGNMENT_CONFIGURATION private var cohortSyncConfiguration = Defaults.COHORT_SYNC_CONFIGURATION + private var evaluationProxyConfiguration = Defaults.EVALUATION_PROXY_CONFIGURATION private var metrics = Defaults.LOCAL_EVALUATION_METRICS fun debug(debug: Boolean) = apply { @@ -128,6 +136,10 @@ class LocalEvaluationConfig internal constructor( this.cohortSyncConfiguration = cohortSyncConfiguration } + fun enableEvaluationProxy(evaluationProxyConfiguration: EvaluationProxyConfiguration) = apply { + this.evaluationProxyConfiguration = evaluationProxyConfiguration + } + @ExperimentalApi fun metrics(metrics: LocalEvaluationMetrics) = apply { this.metrics = metrics @@ -141,6 +153,7 @@ class LocalEvaluationConfig internal constructor( flagConfigPollerRequestTimeoutMillis = flagConfigPollerRequestTimeoutMillis, assignmentConfiguration = assignmentConfiguration, cohortSyncConfiguration = cohortSyncConfiguration, + evaluationProxyConfiguration = evaluationProxyConfiguration, metrics = metrics, ) } @@ -152,6 +165,7 @@ class LocalEvaluationConfig internal constructor( "flagConfigPollerRequestTimeoutMillis=$flagConfigPollerRequestTimeoutMillis, " + "assignmentConfiguration=$assignmentConfiguration, " + "cohortSyncConfiguration=$cohortSyncConfiguration, " + + "evaluationProxyConfiguration=$evaluationProxyConfiguration, " + "metrics=$metrics)" } } @@ -172,6 +186,13 @@ data class CohortSyncConfiguration( val maxCohortSize: Int = Int.MAX_VALUE, ) +@ExperimentalApi +data class EvaluationProxyConfiguration( + val proxyUrl: String, + val cohortCacheCapacity: Int = 1000000, + val cohortCacheTtlMillis: Long = 60000L, +) + @ExperimentalApi interface LocalEvaluationMetrics { fun onEvaluation() @@ -184,4 +205,6 @@ interface LocalEvaluationMetrics { fun onFlagConfigFetchFailure(exception: Exception) fun onCohortDownload() fun onCohortDownloadFailure(exception: Exception) + fun onProxyCohortMembership() + fun onProxyCohortMembershipFailure(exception: Exception) } diff --git a/src/main/kotlin/cohort/CohortDownloadApi.kt b/src/main/kotlin/cohort/CohortApi.kt similarity index 96% rename from src/main/kotlin/cohort/CohortDownloadApi.kt rename to src/main/kotlin/cohort/CohortApi.kt index 63d35a6..5e9dedd 100644 --- a/src/main/kotlin/cohort/CohortDownloadApi.kt +++ b/src/main/kotlin/cohort/CohortApi.kt @@ -37,17 +37,17 @@ internal data class GetCohortResponse( ) } -internal interface CohortDownloadApi { +internal interface CohortApi { fun getCohort(cohortId: String, cohort: Cohort?): Cohort } -internal class DirectCohortDownloadApi( +internal class DirectCohortApi( apiKey: String, secretKey: String, private val maxCohortSize: Int, private val serverUrl: HttpUrl, private val httpClient: OkHttpClient, -) : CohortDownloadApi { +) : CohortApi { private val token = Base64.getEncoder().encodeToString("$apiKey:$secretKey".toByteArray()) private val backoffConfig = BackoffConfig( diff --git a/src/main/kotlin/cohort/CohortLoader.kt b/src/main/kotlin/cohort/CohortLoader.kt index 3fbe01d..f43d2a4 100644 --- a/src/main/kotlin/cohort/CohortLoader.kt +++ b/src/main/kotlin/cohort/CohortLoader.kt @@ -12,7 +12,7 @@ import java.util.concurrent.ThreadPoolExecutor import java.util.concurrent.TimeUnit internal class CohortLoader( - private val cohortDownloadApi: CohortDownloadApi, + private val cohortApi: CohortApi, private val cohortStorage: CohortStorage, private val metrics: LocalEvaluationMetrics = LocalEvaluationMetricsWrapper() ) { @@ -39,7 +39,7 @@ internal class CohortLoader( metrics::onCohortDownloadFailure ) { try { - val cohort = cohortDownloadApi.getCohort(cohortId, storageCohort) + val cohort = cohortApi.getCohort(cohortId, storageCohort) cohortStorage.putCohort(cohort) } catch (e: CohortNotModifiedException) { // Do nothing diff --git a/src/main/kotlin/cohort/CohortMembershipApi.kt b/src/main/kotlin/cohort/CohortMembershipApi.kt new file mode 100644 index 0000000..70bbdf9 --- /dev/null +++ b/src/main/kotlin/cohort/CohortMembershipApi.kt @@ -0,0 +1,26 @@ +package com.amplitude.experiment.cohort + +import com.amplitude.experiment.util.get +import okhttp3.HttpUrl +import okhttp3.OkHttpClient + +internal interface CohortMembershipApi { + fun getCohortMemberships(groupType: String, groupName: String): Set +} + +internal class ProxyCohortMembershipApi( + private val deploymentKey: String, + private val serverUrl: HttpUrl, + private val httpClient: OkHttpClient, +) : CohortMembershipApi { + + override fun getCohortMemberships(groupType: String, groupName: String): Set { + return httpClient.get>( + serverUrl, + "sdk/v2/memberships/$groupType/$groupName", + headers = mapOf( + "Authorization" to "Api-Key $deploymentKey", + ) + ).get() + } +} diff --git a/src/main/kotlin/cohort/CohortStorage.kt b/src/main/kotlin/cohort/CohortStorage.kt index 5cfe3a4..706ef28 100644 --- a/src/main/kotlin/cohort/CohortStorage.kt +++ b/src/main/kotlin/cohort/CohortStorage.kt @@ -1,8 +1,19 @@ +@file:OptIn(ExperimentalApi::class) + package com.amplitude.experiment.cohort +import com.amplitude.experiment.EvaluationProxyConfiguration +import com.amplitude.experiment.ExperimentalApi +import com.amplitude.experiment.LocalEvaluationMetrics +import com.amplitude.experiment.util.Cache +import com.amplitude.experiment.util.LocalEvaluationMetricsWrapper +import com.amplitude.experiment.util.Logger import com.amplitude.experiment.util.USER_GROUP_TYPE +import com.amplitude.experiment.util.wrapMetrics +import java.util.concurrent.locks.ReentrantLock import java.util.concurrent.locks.ReentrantReadWriteLock import kotlin.concurrent.read +import kotlin.concurrent.withLock import kotlin.concurrent.write internal interface CohortStorage { @@ -14,6 +25,71 @@ internal interface CohortStorage { fun deleteCohort(cohortId: String) } +internal class ProxyCohortStorage( + private val proxyConfig: EvaluationProxyConfiguration, + private val membershipApi: CohortMembershipApi, + private val metrics: LocalEvaluationMetrics = LocalEvaluationMetricsWrapper() +) : CohortStorage { + + private val storage: CohortStorage = InMemoryCohortStorage() + private val membershipCacheLock = ReentrantLock() + private val membershipCache = mutableMapOf>>() + + override fun getCohort(cohortId: String): Cohort? { + return storage.getCohort(cohortId) + } + + override fun getCohorts(): Map { + return storage.getCohorts() + } + + override fun getCohortsForUser(userId: String, cohortIds: Set): Set { + return getCohortsForGroup(USER_GROUP_TYPE, userId, cohortIds) + } + + override fun getCohortsForGroup(groupType: String, groupName: String, cohortIds: Set): Set { + val localCohortIds = storage.getCohorts().keys + return if (localCohortIds.containsAll(cohortIds)) { + storage.getCohortsForGroup(groupType, groupName, cohortIds) + } else { + getMembershipCache(groupType)[groupName] + ?: try { + wrapMetrics( + metric = metrics::onProxyCohortMembership, + failure = metrics::onProxyCohortMembershipFailure + ) { + membershipApi.getCohortMemberships(groupType, groupName).also { proxyCohortMemberships -> + getMembershipCache(groupType)[groupName] = proxyCohortMemberships + } + } + } catch (e: Exception) { + Logger.e("Failed to get cohort membership from proxy.", e) + // Fall back on in memory storage in the case of proxy failure. + storage.getCohortsForGroup(groupType, groupName, cohortIds) + } + } + } + + override fun putCohort(cohort: Cohort) { + storage.putCohort(cohort) + } + + override fun deleteCohort(cohortId: String) { + storage.deleteCohort(cohortId) + } + + private fun getMembershipCache(groupType: String): Cache> { + return membershipCacheLock.withLock { + membershipCache.getOrPut(groupType) { + Cache( + proxyConfig.cohortCacheCapacity, + proxyConfig.cohortCacheTtlMillis + ) + } + } + } +} + internal class InMemoryCohortStorage : CohortStorage { private val lock = ReentrantReadWriteLock() private val cohortStore = mutableMapOf() diff --git a/src/main/kotlin/deployment/DeploymentRunner.kt b/src/main/kotlin/deployment/DeploymentRunner.kt index b82c556..a55578a 100644 --- a/src/main/kotlin/deployment/DeploymentRunner.kt +++ b/src/main/kotlin/deployment/DeploymentRunner.kt @@ -2,7 +2,7 @@ package com.amplitude.experiment.deployment import com.amplitude.experiment.LocalEvaluationConfig import com.amplitude.experiment.LocalEvaluationMetrics -import com.amplitude.experiment.cohort.CohortDownloadApi +import com.amplitude.experiment.cohort.CohortApi import com.amplitude.experiment.cohort.CohortLoader import com.amplitude.experiment.cohort.CohortStorage import com.amplitude.experiment.flag.FlagConfigApi @@ -22,14 +22,14 @@ internal class DeploymentRunner( private val config: LocalEvaluationConfig, private val flagConfigApi: FlagConfigApi, private val flagConfigStorage: FlagConfigStorage, - cohortDownloadApi: CohortDownloadApi?, + cohortApi: CohortApi?, private val cohortStorage: CohortStorage?, private val metrics: LocalEvaluationMetrics = LocalEvaluationMetricsWrapper() ) { private val lock = Once() private val poller = Executors.newScheduledThreadPool(1, daemonFactory) - private val cohortLoader = if (cohortDownloadApi != null && cohortStorage != null) { - CohortLoader(cohortDownloadApi, cohortStorage) + private val cohortLoader = if (cohortApi != null && cohortStorage != null) { + CohortLoader(cohortApi, cohortStorage) } else { null } diff --git a/src/main/kotlin/util/Metrics.kt b/src/main/kotlin/util/Metrics.kt index 065d02f..cd11d5e 100644 --- a/src/main/kotlin/util/Metrics.kt +++ b/src/main/kotlin/util/Metrics.kt @@ -75,4 +75,14 @@ internal class LocalEvaluationMetricsWrapper( val metrics = metrics ?: return executor?.execute { metrics.onCohortDownloadFailure(exception) } } + + override fun onProxyCohortMembership() { + val metrics = metrics ?: return + executor?.execute { metrics.onProxyCohortMembership() } + } + + override fun onProxyCohortMembershipFailure(exception: Exception) { + val metrics = metrics ?: return + executor?.execute { metrics.onProxyCohortMembershipFailure(exception) } + } } diff --git a/src/test/kotlin/LocalEvaluationClientTest.kt b/src/test/kotlin/LocalEvaluationClientTest.kt index 31fd208..6aeb642 100644 --- a/src/test/kotlin/LocalEvaluationClientTest.kt +++ b/src/test/kotlin/LocalEvaluationClientTest.kt @@ -1,7 +1,7 @@ package com.amplitude.experiment import com.amplitude.experiment.cohort.Cohort -import com.amplitude.experiment.cohort.CohortDownloadApi +import com.amplitude.experiment.cohort.CohortApi import io.mockk.every import io.mockk.mockk import org.junit.Assert @@ -151,13 +151,13 @@ class LocalEvaluationClientTest { val cohortConfig = LocalEvaluationConfig( cohortSyncConfiguration = CohortSyncConfiguration("api", "secret") ) - val cohortDownloadApi = mockk().apply { + val cohortApi = mockk().apply { every { getCohort(eq("52gz3yi7"), allAny()) } returns Cohort("52gz3yi7", "User", 2, 1722363790000, setOf("1", "2")) every { getCohort(eq("mv7fn2bp"), allAny()) } returns Cohort("mv7fn2bp", "User", 1, 1719350216000, setOf("67890", "12345")) every { getCohort(eq("s4t57y32"), allAny()) } returns Cohort("s4t57y32", "org name", 1, 1722368285000, setOf("Amplitude Website (Portfolio)")) every { getCohort(eq("k1lklnnb"), allAny()) } returns Cohort("k1lklnnb", "org id", 1, 1722466388000, setOf("1")) } - val client = LocalEvaluationClient(API_KEY, cohortConfig, cohortDownloadApi = cohortDownloadApi) + val client = LocalEvaluationClient(API_KEY, cohortConfig, cohortApi = cohortApi) client.start() val user = ExperimentUser( userId = "2333", @@ -177,13 +177,13 @@ class LocalEvaluationClientTest { val cohortConfig = LocalEvaluationConfig( cohortSyncConfiguration = CohortSyncConfiguration("api", "secret") ) - val cohortDownloadApi = mockk().apply { + val cohortApi = mockk().apply { every { getCohort(eq("52gz3yi7"), allAny()) } returns Cohort("52gz3yi7", "User", 2, 1722363790000, setOf("1", "2")) every { getCohort(eq("mv7fn2bp"), allAny()) } returns Cohort("mv7fn2bp", "User", 1, 1719350216000, setOf("67890", "12345")) every { getCohort(eq("s4t57y32"), allAny()) } returns Cohort("s4t57y32", "org name", 1, 1722368285000, setOf("Amplitude Website (Portfolio)")) every { getCohort(eq("k1lklnnb"), allAny()) } returns Cohort("k1lklnnb", "org id", 1, 1722466388000, setOf("1")) } - val client = LocalEvaluationClient(API_KEY, cohortConfig, cohortDownloadApi = cohortDownloadApi) + val client = LocalEvaluationClient(API_KEY, cohortConfig, cohortApi = cohortApi) client.start() val user = ExperimentUser( userId = "12345", @@ -198,13 +198,13 @@ class LocalEvaluationClientTest { val cohortConfig = LocalEvaluationConfig( cohortSyncConfiguration = CohortSyncConfiguration("api", "secret") ) - val cohortDownloadApi = mockk().apply { + val cohortApi = mockk().apply { every { getCohort(eq("52gz3yi7"), allAny()) } returns Cohort("52gz3yi7", "User", 2, 1722363790000, setOf("1", "2")) every { getCohort(eq("mv7fn2bp"), allAny()) } returns Cohort("mv7fn2bp", "User", 1, 1719350216000, setOf("67890", "12345")) every { getCohort(eq("s4t57y32"), allAny()) } returns Cohort("s4t57y32", "org name", 1, 1722368285000, setOf("Amplitude Website (Portfolio)")) every { getCohort(eq("k1lklnnb"), allAny()) } returns Cohort("k1lklnnb", "org id", 1, 1722466388000, setOf("1")) } - val client = LocalEvaluationClient(API_KEY, cohortConfig, cohortDownloadApi = cohortDownloadApi) + val client = LocalEvaluationClient(API_KEY, cohortConfig, cohortApi = cohortApi) client.start() val user = ExperimentUser( userId = "1", @@ -220,13 +220,13 @@ class LocalEvaluationClientTest { val cohortConfig = LocalEvaluationConfig( cohortSyncConfiguration = CohortSyncConfiguration("api", "secret") ) - val cohortDownloadApi = mockk().apply { + val cohortApi = mockk().apply { every { getCohort(eq("52gz3yi7"), allAny()) } returns Cohort("52gz3yi7", "User", 2, 1722363790000, setOf("1", "2")) every { getCohort(eq("mv7fn2bp"), allAny()) } returns Cohort("mv7fn2bp", "User", 1, 1719350216000, setOf("67890", "12345")) every { getCohort(eq("s4t57y32"), allAny()) } returns Cohort("s4t57y32", "org name", 1, 1722368285000, setOf("Amplitude Website (Portfolio)")) every { getCohort(eq("k1lklnnb"), allAny()) } returns Cohort("k1lklnnb", "org id", 1, 1722466388000, setOf("1")) } - val client = LocalEvaluationClient(API_KEY, cohortConfig, cohortDownloadApi = cohortDownloadApi) + val client = LocalEvaluationClient(API_KEY, cohortConfig, cohortApi = cohortApi) client.start() val user = ExperimentUser( userId = "2333", @@ -243,13 +243,13 @@ class LocalEvaluationClientTest { val cohortConfig = LocalEvaluationConfig( cohortSyncConfiguration = CohortSyncConfiguration("api", "secret") ) - val cohortDownloadApi = mockk().apply { + val cohortApi = mockk().apply { every { getCohort(eq("52gz3yi7"), allAny()) } returns Cohort("52gz3yi7", "User", 2, 1722363790000, setOf("1", "2")) every { getCohort(eq("mv7fn2bp"), allAny()) } returns Cohort("mv7fn2bp", "User", 1, 1719350216000, setOf("67890", "12345")) every { getCohort(eq("s4t57y32"), allAny()) } returns Cohort("s4t57y32", "org name", 1, 1722368285000, setOf("Amplitude Website (Portfolio)")) every { getCohort(eq("k1lklnnb"), allAny()) } returns Cohort("k1lklnnb", "org id", 1, 1722466388000, setOf("1")) } - val client = LocalEvaluationClient(API_KEY, cohortConfig, cohortDownloadApi = cohortDownloadApi) + val client = LocalEvaluationClient(API_KEY, cohortConfig, cohortApi = cohortApi) client.start() val user = ExperimentUser( userId = "2333", diff --git a/src/test/kotlin/cohort/CohortDownloadApiTest.kt b/src/test/kotlin/cohort/CohortDownloadApiTest.kt index bbcf801..800464f 100644 --- a/src/test/kotlin/cohort/CohortDownloadApiTest.kt +++ b/src/test/kotlin/cohort/CohortDownloadApiTest.kt @@ -20,7 +20,7 @@ class CohortDownloadApiTest { private val httpClient = OkHttpClient() private val server = MockWebServer() private val url = server.url("/") - private val api = DirectCohortDownloadApi(apiKey,secretKey, maxCohortSize, url, httpClient) + private val api = DirectCohortApi(apiKey,secretKey, maxCohortSize, url, httpClient) @Test fun `cohort download, success`() { val response = cohortResponse("a", setOf("1")) diff --git a/src/test/kotlin/cohort/CohortLoaderTest.kt b/src/test/kotlin/cohort/CohortLoaderTest.kt index 9a71577..4664a70 100644 --- a/src/test/kotlin/cohort/CohortLoaderTest.kt +++ b/src/test/kotlin/cohort/CohortLoaderTest.kt @@ -14,7 +14,7 @@ class CohortLoaderTest { fun `test load, success`() { val cohortA = cohort("a", setOf("1")) val cohortB = cohort("b", setOf("1", "2")) - val api = mock(CohortDownloadApi::class.java) + val api = mock(CohortApi::class.java) `when`(api.getCohort("a", null)).thenReturn(cohortA) `when`(api.getCohort("b", null)).thenReturn(cohortB) val storage = InMemoryCohortStorage() @@ -30,7 +30,7 @@ class CohortLoaderTest { @Test fun `test load, cohorts greater than max cohort size are not downloaded`() { val cohortB = cohort("b", setOf("1", "2")) - val api = mock(CohortDownloadApi::class.java) + val api = mock(CohortApi::class.java) `when`(api.getCohort("a", null)).thenThrow(CohortTooLargeException("a", 15000)) `when`(api.getCohort("b", null)).thenReturn(cohortB) val storage = InMemoryCohortStorage() @@ -51,7 +51,7 @@ class CohortLoaderTest { storage.putCohort(storageCohortA) storage.putCohort(storageCohortB) val networkCohortB = cohort("b", setOf("1", "2")) - val api = mock(CohortDownloadApi::class.java) + val api = mock(CohortApi::class.java) `when`(api.getCohort("a", storageCohortA)).thenThrow(CohortNotModifiedException("a")) `when`(api.getCohort("b", storageCohortB)).thenReturn(networkCohortB) val loader = CohortLoader(api, storage) @@ -67,7 +67,7 @@ class CohortLoaderTest { fun `test load, download failure throws`() { val cohortA = cohort("a", setOf("1")) val cohortC = cohort("c", setOf("1", "2", "3")) - val api = mock(CohortDownloadApi::class.java) + val api = mock(CohortApi::class.java) val storage = InMemoryCohortStorage() val loader = CohortLoader(api, storage) `when`(api.getCohort("a", null)).thenReturn(cohortA) diff --git a/src/test/kotlin/cohort/CohortStorageTest.kt b/src/test/kotlin/cohort/CohortStorageTest.kt new file mode 100644 index 0000000..43c756c --- /dev/null +++ b/src/test/kotlin/cohort/CohortStorageTest.kt @@ -0,0 +1,119 @@ +@file:OptIn(ExperimentalApi::class) + +package com.amplitude.experiment.cohort + +import com.amplitude.experiment.EvaluationProxyConfiguration +import com.amplitude.experiment.ExperimentalApi +import io.mockk.every +import io.mockk.mockk +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import kotlin.test.Test + +class CohortStorageTest { + + @Test + fun `test in memory`() { + val storage = InMemoryCohortStorage() + val cohortA = Cohort("a", "User", 1, 100, setOf("u1")) + val cohortB = Cohort("b", "group", 1, 100, setOf("g1")) + // test get, null + var cohort = storage.getCohort(cohortA.id) + assertNull(cohort) + // test get all, empty + var cohorts = storage.getCohorts() + assertEquals(0, cohorts.size) + // test get memberships for user, empty + var memberships = storage.getCohortsForUser("u1", setOf(cohortA.id, cohortB.id)) + assertEquals(0, memberships.size) + // test get memberships for group, empty + memberships = storage.getCohortsForUser("g1", setOf(cohortA.id, cohortB.id)) + assertEquals(0, memberships.size) + // test put, get, cohort + storage.putCohort(cohortA) + storage.putCohort(cohortB) + cohort = storage.getCohort(cohortA.id) + assertEquals(cohortA, cohort) + // test put, get all, cohorts + cohorts = storage.getCohorts() + assertEquals(mapOf(cohortA.id to cohortA, cohortB.id to cohortB), cohorts) + // test get memberships for user, cohort + memberships = storage.getCohortsForUser("u1", setOf(cohortA.id, cohortB.id)) + assertEquals(setOf(cohortA.id), memberships) + // test get memberships for group, cohort + memberships = storage.getCohortsForGroup("group", "g1", setOf(cohortA.id, cohortB.id)) + assertEquals(setOf(cohortB.id), memberships) + // test delete, get removed, null + storage.deleteCohort(cohortA.id) + cohort = storage.getCohort(cohortA.id) + assertNull(cohort) + // get existing, cohort + cohort = storage.getCohort(cohortB.id) + assertEquals(cohortB, cohort) + // test get memberships for removed, empty + memberships = storage.getCohortsForUser("u1", setOf(cohortA.id, cohortB.id)) + assertEquals(0, memberships.size) + // test get memberships for existing, cohorts + memberships = storage.getCohortsForGroup("group", "g1", setOf(cohortA.id, cohortB.id)) + assertEquals(setOf(cohortB.id), memberships) + // test get all, cohort + cohorts = storage.getCohorts() + assertEquals(mapOf(cohortB.id to cohortB), cohorts) + } + + @Test + fun `test proxy storage`() { + val cohortMembershipApi = mockk() + every { cohortMembershipApi.getCohortMemberships(eq("User"), eq("u1")) } returns setOf("a") + every { cohortMembershipApi.getCohortMemberships(eq("group"), eq("g1")) } returns setOf("b") + val storage = ProxyCohortStorage( + EvaluationProxyConfiguration(""), + cohortMembershipApi + ) + val cohortA = Cohort("a", "User", 1, 100, setOf("u1")) + val cohortB = Cohort("b", "group", 1, 100, setOf("g1")) + + // test get, null + var cohort = storage.getCohort(cohortA.id) + assertNull(cohort) + // test get all, empty + var cohorts = storage.getCohorts() + assertEquals(0, cohorts.size) + // test get memberships for user, cohort from proxy + var memberships = storage.getCohortsForUser("u1", setOf(cohortA.id, cohortB.id)) + assertEquals(setOf(cohortA.id), memberships) + // test get memberships for group, cohort from proxy + memberships = storage.getCohortsForGroup("group", "g1", setOf(cohortA.id, cohortB.id)) + assertEquals(setOf(cohortB.id), memberships) + // test put, get, cohort + storage.putCohort(cohortA) + storage.putCohort(cohortB) + cohort = storage.getCohort(cohortA.id) + assertEquals(cohortA, cohort) + // test put, get all, cohorts + cohorts = storage.getCohorts() + assertEquals(mapOf(cohortA.id to cohortA, cohortB.id to cohortB), cohorts) + // test get memberships for user, cohort + memberships = storage.getCohortsForUser("u1", setOf(cohortA.id, cohortB.id)) + assertEquals(setOf(cohortA.id), memberships) + // test get memberships for group, cohort + memberships = storage.getCohortsForGroup("group", "g1", setOf(cohortA.id, cohortB.id)) + assertEquals(setOf(cohortB.id), memberships) + // test delete, get removed, null + storage.deleteCohort(cohortA.id) + cohort = storage.getCohort(cohortA.id) + assertNull(cohort) + // get existing, cohort + cohort = storage.getCohort(cohortB.id) + assertEquals(cohortB, cohort) + // test get memberships for removed, cohort from proxy + memberships = storage.getCohortsForUser("u1", setOf(cohortA.id, cohortB.id)) + assertEquals(setOf(cohortA.id), memberships) + // test get memberships for existing, cohorts + memberships = storage.getCohortsForGroup("group", "g1", setOf(cohortA.id, cohortB.id)) + assertEquals(setOf(cohortB.id), memberships) + // test get all, cohort + cohorts = storage.getCohorts() + assertEquals(mapOf(cohortB.id to cohortB), cohorts) + } +} diff --git a/src/test/kotlin/deployment/DeploymentRunnerTest.kt b/src/test/kotlin/deployment/DeploymentRunnerTest.kt index ba76dd7..8e58a79 100644 --- a/src/test/kotlin/deployment/DeploymentRunnerTest.kt +++ b/src/test/kotlin/deployment/DeploymentRunnerTest.kt @@ -1,7 +1,7 @@ package com.amplitude.experiment.deployment import com.amplitude.experiment.LocalEvaluationConfig -import com.amplitude.experiment.cohort.CohortDownloadApi +import com.amplitude.experiment.cohort.CohortApi import com.amplitude.experiment.cohort.CohortStorage import com.amplitude.experiment.evaluation.EvaluationCondition import com.amplitude.experiment.evaluation.EvaluationFlag @@ -38,14 +38,14 @@ class DeploymentRunnerTest { @Test fun `test start throws if first flag config load fails`() { val flagApi = Mockito.mock(FlagConfigApi::class.java) - val cohortDownloadApi = Mockito.mock(CohortDownloadApi::class.java) + val cohortApi = Mockito.mock(CohortApi::class.java) val flagConfigStorage = Mockito.mock(FlagConfigStorage::class.java) val cohortStorage = Mockito.mock(CohortStorage::class.java) val runner = DeploymentRunner( LocalEvaluationConfig(), flagApi, flagConfigStorage, - cohortDownloadApi, + cohortApi, cohortStorage, ) Mockito.`when`(flagApi.getFlagConfigs()).thenThrow(RuntimeException("test")) @@ -67,17 +67,17 @@ class DeploymentRunnerTest { @Test fun `test start does not throw if first cohort load fails`() { val flagApi = Mockito.mock(FlagConfigApi::class.java) - val cohortDownloadApi = Mockito.mock(CohortDownloadApi::class.java) + val cohortApi = Mockito.mock(CohortApi::class.java) val flagConfigStorage = Mockito.mock(FlagConfigStorage::class.java) val cohortStorage = Mockito.mock(CohortStorage::class.java) val runner = DeploymentRunner( LocalEvaluationConfig(), flagApi, flagConfigStorage, - cohortDownloadApi, + cohortApi, cohortStorage, ) Mockito.`when`(flagApi.getFlagConfigs()).thenReturn(listOf(flag)) - Mockito.`when`(cohortDownloadApi.getCohort(COHORT_ID, null)).thenThrow(RuntimeException("test")) + Mockito.`when`(cohortApi.getCohort(COHORT_ID, null)).thenThrow(RuntimeException("test")) runner.start() } } From 4ab3c0b5456c23dcf2854715de0243e71edba162 Mon Sep 17 00:00:00 2001 From: Brian Giori Date: Thu, 1 Aug 2024 18:34:07 -0700 Subject: [PATCH 05/15] feat: add proxy apis --- src/main/kotlin/LocalEvaluationClient.kt | 38 ++++++++------ src/main/kotlin/LocalEvaluationConfig.kt | 2 + src/main/kotlin/cohort/CohortApi.kt | 20 ++++++- src/main/kotlin/flag/FlagConfigApi.kt | 52 +++++++++++++++---- src/main/kotlin/util/Metrics.kt | 10 ++++ src/main/kotlin/util/Request.kt | 6 ++- src/test/kotlin/LocalEvaluationClientTest.kt | 2 + .../kotlin/cohort/CohortDownloadApiTest.kt | 3 +- 8 files changed, 104 insertions(+), 29 deletions(-) diff --git a/src/main/kotlin/LocalEvaluationClient.kt b/src/main/kotlin/LocalEvaluationClient.kt index a2f07a3..0963493 100644 --- a/src/main/kotlin/LocalEvaluationClient.kt +++ b/src/main/kotlin/LocalEvaluationClient.kt @@ -1,3 +1,5 @@ +@file:OptIn(ExperimentalApi::class) + package com.amplitude.experiment import com.amplitude.Amplitude @@ -7,18 +9,17 @@ import com.amplitude.experiment.assignment.Assignment import com.amplitude.experiment.assignment.AssignmentService import com.amplitude.experiment.assignment.InMemoryAssignmentFilter import com.amplitude.experiment.cohort.CohortApi -import com.amplitude.experiment.cohort.DirectCohortApi +import com.amplitude.experiment.cohort.DynamicCohortApi import com.amplitude.experiment.cohort.InMemoryCohortStorage import com.amplitude.experiment.deployment.DeploymentRunner import com.amplitude.experiment.evaluation.EvaluationEngine import com.amplitude.experiment.evaluation.EvaluationEngineImpl import com.amplitude.experiment.evaluation.EvaluationFlag import com.amplitude.experiment.evaluation.topologicalSort -import com.amplitude.experiment.flag.DirectFlagConfigApi +import com.amplitude.experiment.flag.DynamicFlagConfigApi import com.amplitude.experiment.flag.InMemoryFlagConfigStorage import com.amplitude.experiment.util.LocalEvaluationMetricsWrapper import com.amplitude.experiment.util.Logger -import com.amplitude.experiment.util.Once import com.amplitude.experiment.util.USER_GROUP_TYPE import com.amplitude.experiment.util.filterDefaultVariants import com.amplitude.experiment.util.getGroupedCohortIds @@ -35,12 +36,12 @@ class LocalEvaluationClient internal constructor( private val httpClient: OkHttpClient = OkHttpClient(), cohortApi: CohortApi? = getCohortDownloadApi(config, httpClient) ) { - private val startLock = Once() + private val assignmentService: AssignmentService? = createAssignmentService(apiKey) private val serverUrl: HttpUrl = getServerUrl(config) private val evaluation: EvaluationEngine = EvaluationEngineImpl() private val metrics: LocalEvaluationMetrics = LocalEvaluationMetricsWrapper(config.metrics) - private val flagConfigApi = DirectFlagConfigApi(apiKey, serverUrl, httpClient) + private val flagConfigApi = DynamicFlagConfigApi(apiKey, serverUrl, getProxyUrl(config), httpClient) private val flagConfigStorage = InMemoryFlagConfigStorage() private val cohortStorage = if (config.cohortSyncConfiguration != null) { InMemoryCohortStorage() @@ -138,17 +139,16 @@ class LocalEvaluationClient internal constructor( } }.build() } - - } private fun getCohortDownloadApi(config: LocalEvaluationConfig, httpClient: OkHttpClient): CohortApi? { return if (config.cohortSyncConfiguration != null) { - DirectCohortApi( + DynamicCohortApi( apiKey = config.cohortSyncConfiguration.apiKey, secretKey = config.cohortSyncConfiguration.secretKey, maxCohortSize = config.cohortSyncConfiguration.maxCohortSize, serverUrl = getCohortServerUrl(config), + proxyUrl = getProxyUrl(config), httpClient = httpClient, ) } else { @@ -157,21 +157,25 @@ private fun getCohortDownloadApi(config: LocalEvaluationConfig, httpClient: OkHt } private fun getServerUrl(config: LocalEvaluationConfig): HttpUrl { - if (config.serverZone == LocalEvaluationConfig.Defaults.SERVER_ZONE) { - return config.serverUrl.toHttpUrl() + return if (config.serverZone == LocalEvaluationConfig.Defaults.SERVER_ZONE) { + config.serverUrl.toHttpUrl() } else { - return when (config.serverZone) { + when (config.serverZone) { ServerZone.US -> US_SERVER_URL.toHttpUrl() ServerZone.EU -> EU_SERVER_URL.toHttpUrl() } } } +private fun getProxyUrl(config: LocalEvaluationConfig): HttpUrl? { + return config.evaluationProxyConfiguration?.proxyUrl?.toHttpUrl() +} + private fun getCohortServerUrl(config: LocalEvaluationConfig): HttpUrl { - if (config.serverZone == LocalEvaluationConfig.Defaults.SERVER_ZONE) { - return config.cohortServerUrl.toHttpUrl() + return if (config.serverZone == LocalEvaluationConfig.Defaults.SERVER_ZONE) { + config.cohortServerUrl.toHttpUrl() } else { - return when (config.serverZone) { + when (config.serverZone) { ServerZone.US -> US_SERVER_URL.toHttpUrl() ServerZone.EU -> EU_SERVER_URL.toHttpUrl() } @@ -182,10 +186,10 @@ private fun getEventServerUrl( config: LocalEvaluationConfig, assignmentConfiguration: AssignmentConfiguration ): String { - if (config.serverZone == LocalEvaluationConfig.Defaults.SERVER_ZONE) { - return assignmentConfiguration.serverUrl + return if (config.serverZone == LocalEvaluationConfig.Defaults.SERVER_ZONE) { + assignmentConfiguration.serverUrl } else { - return when (config.serverZone) { + when (config.serverZone) { ServerZone.US -> US_EVENT_SERVER_URL ServerZone.EU -> EU_EVENT_SERVER_URL } diff --git a/src/main/kotlin/LocalEvaluationConfig.kt b/src/main/kotlin/LocalEvaluationConfig.kt index bf45fb9..c3f5d93 100644 --- a/src/main/kotlin/LocalEvaluationConfig.kt +++ b/src/main/kotlin/LocalEvaluationConfig.kt @@ -203,8 +203,10 @@ interface LocalEvaluationMetrics { fun onAssignmentEventFailure(exception: Exception) fun onFlagConfigFetch() fun onFlagConfigFetchFailure(exception: Exception) + fun onFlagConfigFetchOriginFallback(exception: Exception) fun onCohortDownload() fun onCohortDownloadFailure(exception: Exception) + fun onCohortDownloadOriginFallback(exception: Exception) fun onProxyCohortMembership() fun onProxyCohortMembershipFailure(exception: Exception) } diff --git a/src/main/kotlin/cohort/CohortApi.kt b/src/main/kotlin/cohort/CohortApi.kt index 5e9dedd..6c09b49 100644 --- a/src/main/kotlin/cohort/CohortApi.kt +++ b/src/main/kotlin/cohort/CohortApi.kt @@ -1,8 +1,10 @@ package com.amplitude.experiment.cohort import com.amplitude.experiment.LIBRARY_VERSION +import com.amplitude.experiment.LocalEvaluationMetrics import com.amplitude.experiment.util.BackoffConfig import com.amplitude.experiment.util.HttpErrorResponseException +import com.amplitude.experiment.util.LocalEvaluationMetricsWrapper import com.amplitude.experiment.util.Logger import com.amplitude.experiment.util.backoff import com.amplitude.experiment.util.get @@ -41,12 +43,14 @@ internal interface CohortApi { fun getCohort(cohortId: String, cohort: Cohort?): Cohort } -internal class DirectCohortApi( +internal class DynamicCohortApi( apiKey: String, secretKey: String, private val maxCohortSize: Int, private val serverUrl: HttpUrl, + private val proxyUrl: HttpUrl?, private val httpClient: OkHttpClient, + private val metrics: LocalEvaluationMetrics = LocalEvaluationMetricsWrapper() ) : CohortApi { private val token = Base64.getEncoder().encodeToString("$apiKey:$secretKey".toByteArray()) @@ -58,6 +62,19 @@ internal class DirectCohortApi( ) override fun getCohort(cohortId: String, cohort: Cohort?): Cohort { + return if (proxyUrl != null) { + try { + getCohort(proxyUrl, cohortId, cohort) + } catch (e: Exception) { + metrics.onCohortDownloadOriginFallback(e) + getCohort(serverUrl, cohortId, cohort) + } + } else { + getCohort(serverUrl, cohortId, cohort) + } + } + + private fun getCohort(url: HttpUrl, cohortId: String, cohort: Cohort?): Cohort { Logger.d("getCohortMembers($cohortId): start") val future = backoff(backoffConfig, { val headers = mapOf( @@ -98,4 +115,5 @@ internal class DirectCohortApi( throw e.cause ?: e } } + } diff --git a/src/main/kotlin/flag/FlagConfigApi.kt b/src/main/kotlin/flag/FlagConfigApi.kt index 9829393..01a5780 100644 --- a/src/main/kotlin/flag/FlagConfigApi.kt +++ b/src/main/kotlin/flag/FlagConfigApi.kt @@ -1,28 +1,62 @@ package com.amplitude.experiment.flag +import com.amplitude.experiment.LocalEvaluationMetrics import com.amplitude.experiment.evaluation.EvaluationFlag +import com.amplitude.experiment.util.BackoffConfig +import com.amplitude.experiment.util.LocalEvaluationMetricsWrapper +import com.amplitude.experiment.util.backoff import com.amplitude.experiment.util.get import okhttp3.HttpUrl import okhttp3.OkHttpClient +import java.util.concurrent.ExecutionException internal interface FlagConfigApi { fun getFlagConfigs(): List } -internal class DirectFlagConfigApi( +internal class DynamicFlagConfigApi( private val deploymentKey: String, private val serverUrl: HttpUrl, + private val proxyUrl: HttpUrl?, private val httpClient: OkHttpClient, + private val metrics: LocalEvaluationMetrics = LocalEvaluationMetricsWrapper() ) : FlagConfigApi { + private val backoffConfig = BackoffConfig( + attempts = 3, + min = 500, + max = 2000, + scalar = 2.0, + ) + override fun getFlagConfigs(): List { - return httpClient.get>( - serverUrl = serverUrl, - path = "sdk/v2/flags", - headers = mapOf( - "Authorization" to "Api-Key $deploymentKey", - ), - queries = mapOf("v" to "0") - ).get() + return if (proxyUrl != null) { + try { + getFlagConfigs(proxyUrl) + } catch (e: Exception) { + metrics.onFlagConfigFetchOriginFallback(e) + getFlagConfigs(serverUrl) + } + } else { + getFlagConfigs(serverUrl) + } + } + + private fun getFlagConfigs(url: HttpUrl): List { + val future = backoff(backoffConfig) { + httpClient.get>( + serverUrl = url, + path = "sdk/v2/flags", + headers = mapOf( + "Authorization" to "Api-Key $deploymentKey", + ), + queries = mapOf("v" to "0") + ) + } + try { + return future.get() + } catch(e: ExecutionException) { + throw e.cause ?: e + } } } diff --git a/src/main/kotlin/util/Metrics.kt b/src/main/kotlin/util/Metrics.kt index cd11d5e..2748652 100644 --- a/src/main/kotlin/util/Metrics.kt +++ b/src/main/kotlin/util/Metrics.kt @@ -66,6 +66,11 @@ internal class LocalEvaluationMetricsWrapper( executor?.execute { metrics.onFlagConfigFetchFailure(exception) } } + override fun onFlagConfigFetchOriginFallback(exception: Exception) { + val metrics = metrics ?: return + executor?.execute { metrics.onFlagConfigFetchOriginFallback(exception) } + } + override fun onCohortDownload() { val metrics = metrics ?: return executor?.execute { metrics.onCohortDownload() } @@ -76,6 +81,11 @@ internal class LocalEvaluationMetricsWrapper( executor?.execute { metrics.onCohortDownloadFailure(exception) } } + override fun onCohortDownloadOriginFallback(exception: Exception) { + val metrics = metrics ?: return + executor?.execute { metrics.onCohortDownloadOriginFallback(exception) } + } + override fun onProxyCohortMembership() { val metrics = metrics ?: return executor?.execute { metrics.onProxyCohortMembership() } diff --git a/src/main/kotlin/util/Request.kt b/src/main/kotlin/util/Request.kt index 5627bdb..2bd09fb 100644 --- a/src/main/kotlin/util/Request.kt +++ b/src/main/kotlin/util/Request.kt @@ -105,5 +105,9 @@ internal inline fun OkHttpClient.get( headers: Map? = null, queries: Map? = null, ): CompletableFuture { - return this.get(serverUrl, path, headers, queries) {} + return this.get(serverUrl, path, headers, queries) { response -> + if (!response.isSuccessful) { + throw HttpErrorResponseException(response.code) + } + } } diff --git a/src/test/kotlin/LocalEvaluationClientTest.kt b/src/test/kotlin/LocalEvaluationClientTest.kt index 6aeb642..29865c0 100644 --- a/src/test/kotlin/LocalEvaluationClientTest.kt +++ b/src/test/kotlin/LocalEvaluationClientTest.kt @@ -2,6 +2,8 @@ package com.amplitude.experiment import com.amplitude.experiment.cohort.Cohort import com.amplitude.experiment.cohort.CohortApi +import com.amplitude.experiment.util.Logger +import com.amplitude.experiment.util.SystemLogger import io.mockk.every import io.mockk.mockk import org.junit.Assert diff --git a/src/test/kotlin/cohort/CohortDownloadApiTest.kt b/src/test/kotlin/cohort/CohortDownloadApiTest.kt index 800464f..ffc931c 100644 --- a/src/test/kotlin/cohort/CohortDownloadApiTest.kt +++ b/src/test/kotlin/cohort/CohortDownloadApiTest.kt @@ -20,7 +20,8 @@ class CohortDownloadApiTest { private val httpClient = OkHttpClient() private val server = MockWebServer() private val url = server.url("/") - private val api = DirectCohortApi(apiKey,secretKey, maxCohortSize, url, httpClient) + private val proxyUrl = server.url("/") + private val api = DynamicCohortApi(apiKey,secretKey, maxCohortSize, url, proxyUrl, httpClient) @Test fun `cohort download, success`() { val response = cohortResponse("a", setOf("1")) From adae933e3396d5d29e4cdcd1231417ec17cc9d6d Mon Sep 17 00:00:00 2001 From: Brian Giori Date: Fri, 2 Aug 2024 09:49:54 -0700 Subject: [PATCH 06/15] fix: use cohort server url --- src/main/kotlin/LocalEvaluationClient.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/kotlin/LocalEvaluationClient.kt b/src/main/kotlin/LocalEvaluationClient.kt index 0963493..6bd6574 100644 --- a/src/main/kotlin/LocalEvaluationClient.kt +++ b/src/main/kotlin/LocalEvaluationClient.kt @@ -176,8 +176,8 @@ private fun getCohortServerUrl(config: LocalEvaluationConfig): HttpUrl { config.cohortServerUrl.toHttpUrl() } else { when (config.serverZone) { - ServerZone.US -> US_SERVER_URL.toHttpUrl() - ServerZone.EU -> EU_SERVER_URL.toHttpUrl() + ServerZone.US -> US_COHORT_SERVER_URL.toHttpUrl() + ServerZone.EU -> EU_COHORT_SERVER_URL.toHttpUrl() } } } From 4a2d1c7926cd060e2568a6ced4aeccbb77dec4f8 Mon Sep 17 00:00:00 2001 From: Brian Giori Date: Fri, 2 Aug 2024 09:52:48 -0700 Subject: [PATCH 07/15] fix: add log line if cohorts are targeted but not configured --- src/main/kotlin/LocalEvaluationClient.kt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/kotlin/LocalEvaluationClient.kt b/src/main/kotlin/LocalEvaluationClient.kt index 6bd6574..4e85e19 100644 --- a/src/main/kotlin/LocalEvaluationClient.kt +++ b/src/main/kotlin/LocalEvaluationClient.kt @@ -115,6 +115,9 @@ class LocalEvaluationClient internal constructor( private fun enrichUser(user: ExperimentUser, flagConfigs: List): ExperimentUser { val groupedCohortIds = flagConfigs.getGroupedCohortIds() if (cohortStorage == null) { + if (groupedCohortIds.isNotEmpty()) { + Logger.e("Local evaluation flags target cohorts but cohort targeting is not configured.") + } return user } return user.copyToBuilder().apply { From 684a99f8865950ca28021b0ebbcf3d2bf36f499f Mon Sep 17 00:00:00 2001 From: Brian Giori Date: Fri, 2 Aug 2024 12:28:14 -0700 Subject: [PATCH 08/15] fix: tests --- src/main/kotlin/cohort/CohortApi.kt | 4 ++++ src/test/kotlin/cohort/CohortDownloadApiTest.kt | 3 +-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/kotlin/cohort/CohortApi.kt b/src/main/kotlin/cohort/CohortApi.kt index 6c09b49..3d8a4ea 100644 --- a/src/main/kotlin/cohort/CohortApi.kt +++ b/src/main/kotlin/cohort/CohortApi.kt @@ -65,6 +65,10 @@ internal class DynamicCohortApi( return if (proxyUrl != null) { try { getCohort(proxyUrl, cohortId, cohort) + } catch (e: CohortNotModifiedException) { + throw e + } catch (e: CohortTooLargeException) { + throw e } catch (e: Exception) { metrics.onCohortDownloadOriginFallback(e) getCohort(serverUrl, cohortId, cohort) diff --git a/src/test/kotlin/cohort/CohortDownloadApiTest.kt b/src/test/kotlin/cohort/CohortDownloadApiTest.kt index ffc931c..dc8a375 100644 --- a/src/test/kotlin/cohort/CohortDownloadApiTest.kt +++ b/src/test/kotlin/cohort/CohortDownloadApiTest.kt @@ -20,8 +20,7 @@ class CohortDownloadApiTest { private val httpClient = OkHttpClient() private val server = MockWebServer() private val url = server.url("/") - private val proxyUrl = server.url("/") - private val api = DynamicCohortApi(apiKey,secretKey, maxCohortSize, url, proxyUrl, httpClient) + private val api = DynamicCohortApi(apiKey,secretKey, maxCohortSize, url, null, httpClient) @Test fun `cohort download, success`() { val response = cohortResponse("a", setOf("1")) From 310066b2683615772014385e5f11d021eef72e26 Mon Sep 17 00:00:00 2001 From: Brian Giori Date: Mon, 5 Aug 2024 16:36:13 -0700 Subject: [PATCH 09/15] fix: add jvm overloads to subconfig objects --- src/main/kotlin/LocalEvaluationConfig.kt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/kotlin/LocalEvaluationConfig.kt b/src/main/kotlin/LocalEvaluationConfig.kt index c3f5d93..cdd0f5a 100644 --- a/src/main/kotlin/LocalEvaluationConfig.kt +++ b/src/main/kotlin/LocalEvaluationConfig.kt @@ -170,7 +170,7 @@ class LocalEvaluationConfig internal constructor( } } -data class AssignmentConfiguration( +data class AssignmentConfiguration @JvmOverloads constructor( val apiKey: String, val cacheCapacity: Int = 65536, val eventUploadThreshold: Int = 10, @@ -180,14 +180,15 @@ data class AssignmentConfiguration( val middleware: List = listOf(), ) -data class CohortSyncConfiguration( + +data class CohortSyncConfiguration @JvmOverloads constructor( val apiKey: String, val secretKey: String, val maxCohortSize: Int = Int.MAX_VALUE, ) @ExperimentalApi -data class EvaluationProxyConfiguration( +data class EvaluationProxyConfiguration @JvmOverloads constructor( val proxyUrl: String, val cohortCacheCapacity: Int = 1000000, val cohortCacheTtlMillis: Long = 60000L, From 03ac301e53a66a723e6b54ea6171348346cd8948 Mon Sep 17 00:00:00 2001 From: Brian Giori Date: Tue, 6 Aug 2024 18:58:06 -0700 Subject: [PATCH 10/15] fix: update config --- src/main/kotlin/LocalEvaluationClient.kt | 15 +++---- src/main/kotlin/LocalEvaluationConfig.kt | 39 ++++++++++--------- .../kotlin/assignment/AssignmentService.kt | 1 - src/main/kotlin/cohort/CohortApi.kt | 7 ++-- src/main/kotlin/cohort/CohortStorage.kt | 5 +-- .../kotlin/deployment/DeploymentRunner.kt | 20 +++++++--- src/main/kotlin/flag/FlagConfigApi.kt | 2 +- src/main/kotlin/util/Backoff.kt | 4 +- src/test/kotlin/LocalEvaluationClientTest.kt | 12 +++--- .../kotlin/cohort/CohortDownloadApiTest.kt | 4 +- src/test/kotlin/cohort/CohortStorageTest.kt | 4 +- 11 files changed, 61 insertions(+), 52 deletions(-) diff --git a/src/main/kotlin/LocalEvaluationClient.kt b/src/main/kotlin/LocalEvaluationClient.kt index 4e85e19..817bb5b 100644 --- a/src/main/kotlin/LocalEvaluationClient.kt +++ b/src/main/kotlin/LocalEvaluationClient.kt @@ -43,7 +43,7 @@ class LocalEvaluationClient internal constructor( private val metrics: LocalEvaluationMetrics = LocalEvaluationMetricsWrapper(config.metrics) private val flagConfigApi = DynamicFlagConfigApi(apiKey, serverUrl, getProxyUrl(config), httpClient) private val flagConfigStorage = InMemoryFlagConfigStorage() - private val cohortStorage = if (config.cohortSyncConfiguration != null) { + private val cohortStorage = if (config.cohortSyncConfig != null) { InMemoryCohortStorage() } else { null @@ -145,11 +145,11 @@ class LocalEvaluationClient internal constructor( } private fun getCohortDownloadApi(config: LocalEvaluationConfig, httpClient: OkHttpClient): CohortApi? { - return if (config.cohortSyncConfiguration != null) { + return if (config.cohortSyncConfig != null) { DynamicCohortApi( - apiKey = config.cohortSyncConfiguration.apiKey, - secretKey = config.cohortSyncConfiguration.secretKey, - maxCohortSize = config.cohortSyncConfiguration.maxCohortSize, + apiKey = config.cohortSyncConfig.apiKey, + secretKey = config.cohortSyncConfig.secretKey, + maxCohortSize = config.cohortSyncConfig.maxCohortSize, serverUrl = getCohortServerUrl(config), proxyUrl = getProxyUrl(config), httpClient = httpClient, @@ -171,12 +171,13 @@ private fun getServerUrl(config: LocalEvaluationConfig): HttpUrl { } private fun getProxyUrl(config: LocalEvaluationConfig): HttpUrl? { - return config.evaluationProxyConfiguration?.proxyUrl?.toHttpUrl() + return config.evaluationProxyConfig?.proxyUrl?.toHttpUrl() } private fun getCohortServerUrl(config: LocalEvaluationConfig): HttpUrl { return if (config.serverZone == LocalEvaluationConfig.Defaults.SERVER_ZONE) { - config.cohortServerUrl.toHttpUrl() + config.cohortSyncConfig?.cohortServerUrl?.toHttpUrl() + ?: US_COHORT_SERVER_URL.toHttpUrl() } else { when (config.serverZone) { ServerZone.US -> US_COHORT_SERVER_URL.toHttpUrl() diff --git a/src/main/kotlin/LocalEvaluationConfig.kt b/src/main/kotlin/LocalEvaluationConfig.kt index cdd0f5a..d5da088 100644 --- a/src/main/kotlin/LocalEvaluationConfig.kt +++ b/src/main/kotlin/LocalEvaluationConfig.kt @@ -1,8 +1,7 @@ -@file:OptIn(ExperimentalApi::class) - package com.amplitude.experiment import com.amplitude.Middleware +import com.amplitude.experiment.LocalEvaluationConfig.Defaults /** * Configuration options. This is an immutable object that can be created using @@ -10,14 +9,13 @@ import com.amplitude.Middleware * * `LocalEvaluationConfig.builder().serverUrl("https://api.lab.amplitude.com/").build()` */ +@OptIn(ExperimentalApi::class) class LocalEvaluationConfig internal constructor( @JvmField val debug: Boolean = Defaults.DEBUG, @JvmField val serverUrl: String = Defaults.SERVER_URL, @JvmField - val cohortServerUrl: String = Defaults.COHORT_SERVER_URL, - @JvmField val serverZone: ServerZone = Defaults.SERVER_ZONE, @JvmField val flagConfigPollerIntervalMillis: Long = Defaults.FLAG_CONFIG_POLLER_INTERVAL_MILLIS, @@ -26,9 +24,9 @@ class LocalEvaluationConfig internal constructor( @JvmField val assignmentConfiguration: AssignmentConfiguration? = Defaults.ASSIGNMENT_CONFIGURATION, @JvmField - val cohortSyncConfiguration: CohortSyncConfiguration? = Defaults.COHORT_SYNC_CONFIGURATION, + val cohortSyncConfig: CohortSyncConfig? = Defaults.COHORT_SYNC_CONFIGURATION, @JvmField - val evaluationProxyConfiguration: EvaluationProxyConfiguration? = Defaults.EVALUATION_PROXY_CONFIGURATION, + val evaluationProxyConfig: EvaluationProxyConfig? = Defaults.EVALUATION_PROXY_CONFIGURATION, @JvmField val metrics: LocalEvaluationMetrics? = Defaults.LOCAL_EVALUATION_METRICS, ) { @@ -81,12 +79,12 @@ class LocalEvaluationConfig internal constructor( /** * null */ - val COHORT_SYNC_CONFIGURATION: CohortSyncConfiguration? = null + val COHORT_SYNC_CONFIGURATION: CohortSyncConfig? = null /** * null */ - val EVALUATION_PROXY_CONFIGURATION: EvaluationProxyConfiguration? = null + val EVALUATION_PROXY_CONFIGURATION: EvaluationProxyConfig? = null /** * null @@ -132,12 +130,13 @@ class LocalEvaluationConfig internal constructor( this.assignmentConfiguration = assignmentConfiguration } - fun enableCohortSync(cohortSyncConfiguration: CohortSyncConfiguration) = apply { - this.cohortSyncConfiguration = cohortSyncConfiguration + fun cohortSyncConfig(cohortSyncConfig: CohortSyncConfig) = apply { + this.cohortSyncConfiguration = cohortSyncConfig } - fun enableEvaluationProxy(evaluationProxyConfiguration: EvaluationProxyConfiguration) = apply { - this.evaluationProxyConfiguration = evaluationProxyConfiguration + @ExperimentalApi + fun evaluationProxyConfig(evaluationProxyConfig: EvaluationProxyConfig) = apply { + this.evaluationProxyConfiguration = evaluationProxyConfig } @ExperimentalApi @@ -152,8 +151,8 @@ class LocalEvaluationConfig internal constructor( flagConfigPollerIntervalMillis = flagConfigPollerIntervalMillis, flagConfigPollerRequestTimeoutMillis = flagConfigPollerRequestTimeoutMillis, assignmentConfiguration = assignmentConfiguration, - cohortSyncConfiguration = cohortSyncConfiguration, - evaluationProxyConfiguration = evaluationProxyConfiguration, + cohortSyncConfig = cohortSyncConfiguration, + evaluationProxyConfig = evaluationProxyConfiguration, metrics = metrics, ) } @@ -164,8 +163,8 @@ class LocalEvaluationConfig internal constructor( "flagConfigPollerIntervalMillis=$flagConfigPollerIntervalMillis, " + "flagConfigPollerRequestTimeoutMillis=$flagConfigPollerRequestTimeoutMillis, " + "assignmentConfiguration=$assignmentConfiguration, " + - "cohortSyncConfiguration=$cohortSyncConfiguration, " + - "evaluationProxyConfiguration=$evaluationProxyConfiguration, " + + "cohortSyncConfiguration=$cohortSyncConfig, " + + "evaluationProxyConfiguration=$evaluationProxyConfig, " + "metrics=$metrics)" } } @@ -180,15 +179,17 @@ data class AssignmentConfiguration @JvmOverloads constructor( val middleware: List = listOf(), ) - -data class CohortSyncConfiguration @JvmOverloads constructor( +@ExperimentalApi +data class CohortSyncConfig @JvmOverloads constructor( val apiKey: String, val secretKey: String, val maxCohortSize: Int = Int.MAX_VALUE, + val cohortServerUrl: String = Defaults.COHORT_SERVER_URL, + val cohortPollingIntervalMillis: Long = 60000L ) @ExperimentalApi -data class EvaluationProxyConfiguration @JvmOverloads constructor( +data class EvaluationProxyConfig @JvmOverloads constructor( val proxyUrl: String, val cohortCacheCapacity: Int = 1000000, val cohortCacheTtlMillis: Long = 60000L, diff --git a/src/main/kotlin/assignment/AssignmentService.kt b/src/main/kotlin/assignment/AssignmentService.kt index 322dd47..e126514 100644 --- a/src/main/kotlin/assignment/AssignmentService.kt +++ b/src/main/kotlin/assignment/AssignmentService.kt @@ -3,7 +3,6 @@ package com.amplitude.experiment.assignment import com.amplitude.Amplitude import com.amplitude.AmplitudeCallbacks import com.amplitude.Event -import com.amplitude.experiment.ExperimentalApi import com.amplitude.experiment.LocalEvaluationMetrics import com.amplitude.experiment.util.LocalEvaluationMetricsWrapper import com.amplitude.experiment.util.wrapMetrics diff --git a/src/main/kotlin/cohort/CohortApi.kt b/src/main/kotlin/cohort/CohortApi.kt index 3d8a4ea..fe689b7 100644 --- a/src/main/kotlin/cohort/CohortApi.kt +++ b/src/main/kotlin/cohort/CohortApi.kt @@ -55,8 +55,8 @@ internal class DynamicCohortApi( private val token = Base64.getEncoder().encodeToString("$apiKey:$secretKey".toByteArray()) private val backoffConfig = BackoffConfig( - attempts = 3, - min = 500, + attempts = 5, + min = 100, max = 2000, scalar = 2.0, ) @@ -115,9 +115,8 @@ internal class DynamicCohortApi( }) try { return future.get().toCohort() - } catch(e: ExecutionException) { + } catch (e: ExecutionException) { throw e.cause ?: e } } - } diff --git a/src/main/kotlin/cohort/CohortStorage.kt b/src/main/kotlin/cohort/CohortStorage.kt index 706ef28..0250362 100644 --- a/src/main/kotlin/cohort/CohortStorage.kt +++ b/src/main/kotlin/cohort/CohortStorage.kt @@ -2,7 +2,7 @@ package com.amplitude.experiment.cohort -import com.amplitude.experiment.EvaluationProxyConfiguration +import com.amplitude.experiment.EvaluationProxyConfig import com.amplitude.experiment.ExperimentalApi import com.amplitude.experiment.LocalEvaluationMetrics import com.amplitude.experiment.util.Cache @@ -26,7 +26,7 @@ internal interface CohortStorage { } internal class ProxyCohortStorage( - private val proxyConfig: EvaluationProxyConfiguration, + private val proxyConfig: EvaluationProxyConfig, private val membershipApi: CohortMembershipApi, private val metrics: LocalEvaluationMetrics = LocalEvaluationMetricsWrapper() ) : CohortStorage { @@ -126,7 +126,6 @@ internal class InMemoryCohortStorage : CohortStorage { return result } - override fun putCohort(cohort: Cohort) { lock.write { cohortStore[cohort.id] = cohort diff --git a/src/main/kotlin/deployment/DeploymentRunner.kt b/src/main/kotlin/deployment/DeploymentRunner.kt index a55578a..fe52ebb 100644 --- a/src/main/kotlin/deployment/DeploymentRunner.kt +++ b/src/main/kotlin/deployment/DeploymentRunner.kt @@ -1,5 +1,8 @@ +@file:OptIn(ExperimentalApi::class) + package com.amplitude.experiment.deployment +import com.amplitude.experiment.ExperimentalApi import com.amplitude.experiment.LocalEvaluationConfig import com.amplitude.experiment.LocalEvaluationMetrics import com.amplitude.experiment.cohort.CohortApi @@ -33,6 +36,7 @@ internal class DeploymentRunner( } else { null } + private val cohortPollingInterval: Long = getCohortPollingInterval() fun start() = lock.once { refresh() @@ -59,10 +63,9 @@ internal class DeploymentRunner( } catch (t: Throwable) { Logger.e("Refresh cohorts failed.", t) } - }, - 60, - 60, - TimeUnit.SECONDS + }, cohortPollingInterval, + cohortPollingInterval, + TimeUnit.MILLISECONDS ) } } @@ -122,9 +125,16 @@ internal class DeploymentRunner( val storageCohortIds = cohortStorage.getCohorts().keys val deletedCohortIds = storageCohortIds - flagCohortIds for (deletedCohortId in deletedCohortIds) { - cohortStorage.deleteCohort(deletedCohortId) + cohortStorage.deleteCohort(deletedCohortId) } } Logger.d("Refreshed ${flagConfigs.size} flag configs.") } + + private fun getCohortPollingInterval(): Long { + if (config.cohortSyncConfig == null || config.cohortSyncConfig.cohortPollingIntervalMillis < 60000) { + return 60000 + } + return config.cohortSyncConfig.cohortPollingIntervalMillis + } } diff --git a/src/main/kotlin/flag/FlagConfigApi.kt b/src/main/kotlin/flag/FlagConfigApi.kt index 01a5780..0845641 100644 --- a/src/main/kotlin/flag/FlagConfigApi.kt +++ b/src/main/kotlin/flag/FlagConfigApi.kt @@ -55,7 +55,7 @@ internal class DynamicFlagConfigApi( } try { return future.get() - } catch(e: ExecutionException) { + } catch (e: ExecutionException) { throw e.cause ?: e } } diff --git a/src/main/kotlin/util/Backoff.kt b/src/main/kotlin/util/Backoff.kt index 2ef13aa..46f7231 100644 --- a/src/main/kotlin/util/Backoff.kt +++ b/src/main/kotlin/util/Backoff.kt @@ -64,8 +64,8 @@ private class Backoff( if (shouldRetry && nextAttempt < config.attempts) { val nextDelay = min(delay * config.scalar, config.max.toDouble()).toLong() val jitter = Random.nextLong( - (nextDelay - (nextDelay*0.1).toLong()) * -1, - nextDelay + (nextDelay+0.1).toLong() + (nextDelay - (nextDelay * 0.1).toLong()) * -1, + nextDelay + (nextDelay + 0.1).toLong() ) backoff(nextAttempt, nextDelay + jitter, function, retry) } else { diff --git a/src/test/kotlin/LocalEvaluationClientTest.kt b/src/test/kotlin/LocalEvaluationClientTest.kt index 29865c0..7ad24a0 100644 --- a/src/test/kotlin/LocalEvaluationClientTest.kt +++ b/src/test/kotlin/LocalEvaluationClientTest.kt @@ -2,8 +2,6 @@ package com.amplitude.experiment import com.amplitude.experiment.cohort.Cohort import com.amplitude.experiment.cohort.CohortApi -import com.amplitude.experiment.util.Logger -import com.amplitude.experiment.util.SystemLogger import io.mockk.every import io.mockk.mockk import org.junit.Assert @@ -151,7 +149,7 @@ class LocalEvaluationClientTest { @Test fun `evaluate with user and group, not targeted`() { val cohortConfig = LocalEvaluationConfig( - cohortSyncConfiguration = CohortSyncConfiguration("api", "secret") + cohortSyncConfig = CohortSyncConfig("api", "secret") ) val cohortApi = mockk().apply { every { getCohort(eq("52gz3yi7"), allAny()) } returns Cohort("52gz3yi7", "User", 2, 1722363790000, setOf("1", "2")) @@ -177,7 +175,7 @@ class LocalEvaluationClientTest { @Test fun `evaluate with user, cohort segment targeted`() { val cohortConfig = LocalEvaluationConfig( - cohortSyncConfiguration = CohortSyncConfiguration("api", "secret") + cohortSyncConfig = CohortSyncConfig("api", "secret") ) val cohortApi = mockk().apply { every { getCohort(eq("52gz3yi7"), allAny()) } returns Cohort("52gz3yi7", "User", 2, 1722363790000, setOf("1", "2")) @@ -198,7 +196,7 @@ class LocalEvaluationClientTest { @Test fun `evaluate with user, cohort tester targeted`() { val cohortConfig = LocalEvaluationConfig( - cohortSyncConfiguration = CohortSyncConfiguration("api", "secret") + cohortSyncConfig = CohortSyncConfig("api", "secret") ) val cohortApi = mockk().apply { every { getCohort(eq("52gz3yi7"), allAny()) } returns Cohort("52gz3yi7", "User", 2, 1722363790000, setOf("1", "2")) @@ -220,7 +218,7 @@ class LocalEvaluationClientTest { @Test fun `evaluate with group, cohort segment targeted`() { val cohortConfig = LocalEvaluationConfig( - cohortSyncConfiguration = CohortSyncConfiguration("api", "secret") + cohortSyncConfig = CohortSyncConfig("api", "secret") ) val cohortApi = mockk().apply { every { getCohort(eq("52gz3yi7"), allAny()) } returns Cohort("52gz3yi7", "User", 2, 1722363790000, setOf("1", "2")) @@ -243,7 +241,7 @@ class LocalEvaluationClientTest { @Test fun `evaluate with group, cohort tester targeted`() { val cohortConfig = LocalEvaluationConfig( - cohortSyncConfiguration = CohortSyncConfiguration("api", "secret") + cohortSyncConfig = CohortSyncConfig("api", "secret") ) val cohortApi = mockk().apply { every { getCohort(eq("52gz3yi7"), allAny()) } returns Cohort("52gz3yi7", "User", 2, 1722363790000, setOf("1", "2")) diff --git a/src/test/kotlin/cohort/CohortDownloadApiTest.kt b/src/test/kotlin/cohort/CohortDownloadApiTest.kt index dc8a375..59b2374 100644 --- a/src/test/kotlin/cohort/CohortDownloadApiTest.kt +++ b/src/test/kotlin/cohort/CohortDownloadApiTest.kt @@ -20,7 +20,7 @@ class CohortDownloadApiTest { private val httpClient = OkHttpClient() private val server = MockWebServer() private val url = server.url("/") - private val api = DynamicCohortApi(apiKey,secretKey, maxCohortSize, url, null, httpClient) + private val api = DynamicCohortApi(apiKey, secretKey, maxCohortSize, url, null, httpClient) @Test fun `cohort download, success`() { val response = cohortResponse("a", setOf("1")) @@ -87,6 +87,8 @@ class CohortDownloadApiTest { server.enqueue(MockResponse().setResponseCode(501)) server.enqueue(MockResponse().setResponseCode(502)) server.enqueue(MockResponse().setResponseCode(503)) + server.enqueue(MockResponse().setResponseCode(503)) + server.enqueue(MockResponse().setResponseCode(503)) // Should not be sent in response server.enqueue(MockResponse().setResponseCode(204)) try { diff --git a/src/test/kotlin/cohort/CohortStorageTest.kt b/src/test/kotlin/cohort/CohortStorageTest.kt index 43c756c..64000f8 100644 --- a/src/test/kotlin/cohort/CohortStorageTest.kt +++ b/src/test/kotlin/cohort/CohortStorageTest.kt @@ -2,7 +2,7 @@ package com.amplitude.experiment.cohort -import com.amplitude.experiment.EvaluationProxyConfiguration +import com.amplitude.experiment.EvaluationProxyConfig import com.amplitude.experiment.ExperimentalApi import io.mockk.every import io.mockk.mockk @@ -67,7 +67,7 @@ class CohortStorageTest { every { cohortMembershipApi.getCohortMemberships(eq("User"), eq("u1")) } returns setOf("a") every { cohortMembershipApi.getCohortMemberships(eq("group"), eq("g1")) } returns setOf("b") val storage = ProxyCohortStorage( - EvaluationProxyConfiguration(""), + EvaluationProxyConfig(""), cohortMembershipApi ) val cohortA = Cohort("a", "User", 1, 100, setOf("u1")) From 4720f488867f1c84d5f9d86578b91058ce5b3071 Mon Sep 17 00:00:00 2001 From: Brian Giori Date: Wed, 7 Aug 2024 13:30:35 -0700 Subject: [PATCH 11/15] fix: proxy url and fallback, logging --- src/main/kotlin/LocalEvaluationClient.kt | 1 - src/main/kotlin/cohort/CohortApi.kt | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/kotlin/LocalEvaluationClient.kt b/src/main/kotlin/LocalEvaluationClient.kt index 817bb5b..14fcb14 100644 --- a/src/main/kotlin/LocalEvaluationClient.kt +++ b/src/main/kotlin/LocalEvaluationClient.kt @@ -106,7 +106,6 @@ class LocalEvaluationClient internal constructor( ) { evaluation.evaluate(enrichedUser.toEvaluationContext(), sortedFlagConfigs) } - Logger.d("evaluate - user=$enrichedUser, result=$evaluationResults") val variants = evaluationResults.toVariants() assignmentService?.track(Assignment(user, variants)) return variants diff --git a/src/main/kotlin/cohort/CohortApi.kt b/src/main/kotlin/cohort/CohortApi.kt index fe689b7..b2be50e 100644 --- a/src/main/kotlin/cohort/CohortApi.kt +++ b/src/main/kotlin/cohort/CohortApi.kt @@ -70,6 +70,7 @@ internal class DynamicCohortApi( } catch (e: CohortTooLargeException) { throw e } catch (e: Exception) { + Logger.e("Downloading cohort $cohortId from proxy failed. Falling back to Amplitude.", e) metrics.onCohortDownloadOriginFallback(e) getCohort(serverUrl, cohortId, cohort) } @@ -92,7 +93,7 @@ internal class DynamicCohortApi( queries["lastModified"] = "${cohort.lastModified}" } httpClient.get( - serverUrl = serverUrl, + serverUrl = url, path = "sdk/v1/cohort/$cohortId", headers = headers, queries = queries, From 01ec403a6b7150363a972799b556e0bc8ca749fd Mon Sep 17 00:00:00 2001 From: Brian Giori Date: Wed, 7 Aug 2024 16:06:26 -0700 Subject: [PATCH 12/15] fix: use proxycohortstorage --- src/main/kotlin/LocalEvaluationClient.kt | 33 +++++++++++-------- src/main/kotlin/LocalEvaluationConfig.kt | 29 ++++++++-------- src/main/kotlin/RemoteEvaluationClient.kt | 8 ++--- src/main/kotlin/RemoteEvaluationConfig.kt | 19 +++-------- .../kotlin/deployment/DeploymentRunner.kt | 8 +++-- 5 files changed, 49 insertions(+), 48 deletions(-) diff --git a/src/main/kotlin/LocalEvaluationClient.kt b/src/main/kotlin/LocalEvaluationClient.kt index 14fcb14..ca09b0c 100644 --- a/src/main/kotlin/LocalEvaluationClient.kt +++ b/src/main/kotlin/LocalEvaluationClient.kt @@ -11,6 +11,8 @@ import com.amplitude.experiment.assignment.InMemoryAssignmentFilter import com.amplitude.experiment.cohort.CohortApi import com.amplitude.experiment.cohort.DynamicCohortApi import com.amplitude.experiment.cohort.InMemoryCohortStorage +import com.amplitude.experiment.cohort.ProxyCohortMembershipApi +import com.amplitude.experiment.cohort.ProxyCohortStorage import com.amplitude.experiment.deployment.DeploymentRunner import com.amplitude.experiment.evaluation.EvaluationEngine import com.amplitude.experiment.evaluation.EvaluationEngineImpl @@ -36,17 +38,22 @@ class LocalEvaluationClient internal constructor( private val httpClient: OkHttpClient = OkHttpClient(), cohortApi: CohortApi? = getCohortDownloadApi(config, httpClient) ) { - private val assignmentService: AssignmentService? = createAssignmentService(apiKey) private val serverUrl: HttpUrl = getServerUrl(config) private val evaluation: EvaluationEngine = EvaluationEngineImpl() private val metrics: LocalEvaluationMetrics = LocalEvaluationMetricsWrapper(config.metrics) private val flagConfigApi = DynamicFlagConfigApi(apiKey, serverUrl, getProxyUrl(config), httpClient) private val flagConfigStorage = InMemoryFlagConfigStorage() - private val cohortStorage = if (config.cohortSyncConfig != null) { + private val cohortStorage = if (config.cohortSyncConfig == null) { + null + } else if (config.evaluationProxyConfig == null) { InMemoryCohortStorage() } else { - null + ProxyCohortStorage( + proxyConfig = config.evaluationProxyConfig, + membershipApi = ProxyCohortMembershipApi(apiKey, config.evaluationProxyConfig.proxyUrl.toHttpUrl(), httpClient), + metrics = metrics, + ) } private val deploymentRunner = DeploymentRunner( @@ -159,13 +166,13 @@ private fun getCohortDownloadApi(config: LocalEvaluationConfig, httpClient: OkHt } private fun getServerUrl(config: LocalEvaluationConfig): HttpUrl { - return if (config.serverZone == LocalEvaluationConfig.Defaults.SERVER_ZONE) { - config.serverUrl.toHttpUrl() - } else { + return if (config.serverUrl == LocalEvaluationConfig.Defaults.SERVER_URL) { when (config.serverZone) { ServerZone.US -> US_SERVER_URL.toHttpUrl() ServerZone.EU -> EU_SERVER_URL.toHttpUrl() } + } else { + config.serverUrl.toHttpUrl() } } @@ -174,14 +181,14 @@ private fun getProxyUrl(config: LocalEvaluationConfig): HttpUrl? { } private fun getCohortServerUrl(config: LocalEvaluationConfig): HttpUrl { - return if (config.serverZone == LocalEvaluationConfig.Defaults.SERVER_ZONE) { - config.cohortSyncConfig?.cohortServerUrl?.toHttpUrl() - ?: US_COHORT_SERVER_URL.toHttpUrl() - } else { + return if (config.cohortSyncConfig?.cohortServerUrl == LocalEvaluationConfig.Defaults.COHORT_SERVER_URL) { when (config.serverZone) { ServerZone.US -> US_COHORT_SERVER_URL.toHttpUrl() ServerZone.EU -> EU_COHORT_SERVER_URL.toHttpUrl() } + } else { + config.cohortSyncConfig?.cohortServerUrl?.toHttpUrl() + ?: LocalEvaluationConfig.Defaults.COHORT_SERVER_URL.toHttpUrl() } } @@ -189,12 +196,12 @@ private fun getEventServerUrl( config: LocalEvaluationConfig, assignmentConfiguration: AssignmentConfiguration ): String { - return if (config.serverZone == LocalEvaluationConfig.Defaults.SERVER_ZONE) { - assignmentConfiguration.serverUrl - } else { + return if (assignmentConfiguration.serverUrl == LocalEvaluationConfig.Defaults.EVENT_SERVER_URL) { when (config.serverZone) { ServerZone.US -> US_EVENT_SERVER_URL ServerZone.EU -> EU_EVENT_SERVER_URL } + } else { + assignmentConfiguration.serverUrl } } diff --git a/src/main/kotlin/LocalEvaluationConfig.kt b/src/main/kotlin/LocalEvaluationConfig.kt index d5da088..67a5a72 100644 --- a/src/main/kotlin/LocalEvaluationConfig.kt +++ b/src/main/kotlin/LocalEvaluationConfig.kt @@ -49,12 +49,17 @@ class LocalEvaluationConfig internal constructor( /** * "https://api.lab.amplitude.com/" */ - const val SERVER_URL = "https://api.lab.amplitude.com/" + const val SERVER_URL = US_SERVER_URL /** - * "https://api.lab.amplitude.com/" + * "https://cohort-v2.lab.amplitude.com/" + */ + const val COHORT_SERVER_URL = US_COHORT_SERVER_URL + + /** + * "https://api2.amplitude.com/2/httpapi" */ - const val COHORT_SERVER_URL = "https://cohort-v2.lab.amplitude.com/" + const val EVENT_SERVER_URL = US_EVENT_SERVER_URL /** * ServerZone.US @@ -102,6 +107,7 @@ class LocalEvaluationConfig internal constructor( class Builder { private var debug = Defaults.DEBUG + private var serverZone = Defaults.SERVER_ZONE private var serverUrl = Defaults.SERVER_URL private var flagConfigPollerIntervalMillis = Defaults.FLAG_CONFIG_POLLER_INTERVAL_MILLIS private var flagConfigPollerRequestTimeoutMillis = Defaults.FLAG_CONFIG_POLLER_REQUEST_TIMEOUT_MILLIS @@ -118,6 +124,10 @@ class LocalEvaluationConfig internal constructor( this.serverUrl = serverUrl } + fun serverZone(serverZone: ServerZone) = apply { + this.serverZone = serverZone + } + fun flagConfigPollerIntervalMillis(flagConfigPollerIntervalMillis: Long) = apply { this.flagConfigPollerIntervalMillis = flagConfigPollerIntervalMillis } @@ -148,6 +158,7 @@ class LocalEvaluationConfig internal constructor( return LocalEvaluationConfig( debug = debug, serverUrl = serverUrl, + serverZone = serverZone, flagConfigPollerIntervalMillis = flagConfigPollerIntervalMillis, flagConfigPollerRequestTimeoutMillis = flagConfigPollerRequestTimeoutMillis, assignmentConfiguration = assignmentConfiguration, @@ -157,16 +168,6 @@ class LocalEvaluationConfig internal constructor( ) } } - - override fun toString(): String { - return "LocalEvaluationConfig(debug=$debug, serverUrl='$serverUrl', " + - "flagConfigPollerIntervalMillis=$flagConfigPollerIntervalMillis, " + - "flagConfigPollerRequestTimeoutMillis=$flagConfigPollerRequestTimeoutMillis, " + - "assignmentConfiguration=$assignmentConfiguration, " + - "cohortSyncConfiguration=$cohortSyncConfig, " + - "evaluationProxyConfiguration=$evaluationProxyConfig, " + - "metrics=$metrics)" - } } data class AssignmentConfiguration @JvmOverloads constructor( @@ -175,7 +176,7 @@ data class AssignmentConfiguration @JvmOverloads constructor( val eventUploadThreshold: Int = 10, val eventUploadPeriodMillis: Int = 10000, val useBatchMode: Boolean = true, - val serverUrl: String = "https://api2.amplitude.com/2/httpapi", + val serverUrl: String = Defaults.EVENT_SERVER_URL, val middleware: List = listOf(), ) diff --git a/src/main/kotlin/RemoteEvaluationClient.kt b/src/main/kotlin/RemoteEvaluationClient.kt index 8e3517a..e693457 100644 --- a/src/main/kotlin/RemoteEvaluationClient.kt +++ b/src/main/kotlin/RemoteEvaluationClient.kt @@ -118,12 +118,12 @@ private fun shouldRetryFetch(t: Throwable): Boolean { } private fun getServerUrl(config: RemoteEvaluationConfig): HttpUrl { - if (config.serverZone == RemoteEvaluationConfig.Defaults.SERVER_ZONE) { - return config.serverUrl.toHttpUrl() - } else { - return when (config.serverZone) { + return if (config.serverUrl == RemoteEvaluationConfig.Defaults.SERVER_URL) { + when (config.serverZone) { ServerZone.US -> US_SERVER_URL.toHttpUrl() ServerZone.EU -> EU_SERVER_URL.toHttpUrl() } + } else { + config.serverUrl.toHttpUrl() } } diff --git a/src/main/kotlin/RemoteEvaluationConfig.kt b/src/main/kotlin/RemoteEvaluationConfig.kt index 486cc7e..041ea7d 100644 --- a/src/main/kotlin/RemoteEvaluationConfig.kt +++ b/src/main/kotlin/RemoteEvaluationConfig.kt @@ -14,8 +14,6 @@ class RemoteEvaluationConfig internal constructor( @JvmField val serverUrl: String = Defaults.SERVER_URL, @JvmField - val cohortServerUrl: String = Defaults.COHORT_SERVER_URL, - @JvmField val serverZone: ServerZone = Defaults.SERVER_ZONE, @JvmField val fetchTimeoutMillis: Long = Defaults.FETCH_TIMEOUT_MILLIS, @@ -49,12 +47,7 @@ class RemoteEvaluationConfig internal constructor( /** * "https://api.lab.amplitude.com/" */ - const val SERVER_URL = "https://api.lab.amplitude.com/" - - /** - * "https://api.lab.amplitude.com/" - */ - const val COHORT_SERVER_URL = "https://cohorts-v2.lab.amplitude.com/" + const val SERVER_URL = US_SERVER_URL /** * ServerZone.US @@ -103,7 +96,6 @@ class RemoteEvaluationConfig internal constructor( private var debug = Defaults.DEBUG private var serverUrl = Defaults.SERVER_URL - private var cohortServerUrl = Defaults.COHORT_SERVER_URL private var serverZone = Defaults.SERVER_ZONE private var fetchTimeoutMillis = Defaults.FETCH_TIMEOUT_MILLIS private var fetchRetries = Defaults.FETCH_RETRIES @@ -120,10 +112,6 @@ class RemoteEvaluationConfig internal constructor( this.serverUrl = serverUrl } - fun cohortServerUrl(cohortServerUrl: String) = apply { - this.cohortServerUrl = cohortServerUrl - } - fun serverZone(serverZone: ServerZone) = apply { this.serverZone = serverZone } @@ -156,6 +144,7 @@ class RemoteEvaluationConfig internal constructor( return RemoteEvaluationConfig( debug = debug, serverUrl = serverUrl, + serverZone = serverZone, fetchTimeoutMillis = fetchTimeoutMillis, fetchRetries = fetchRetries, fetchRetryBackoffMinMillis = fetchRetryBackoffMinMillis, @@ -167,8 +156,8 @@ class RemoteEvaluationConfig internal constructor( } override fun toString(): String { - return "RemoteEvaluationConfig(debug=$debug, serverUrl='$serverUrl', cohortServerUrl='$cohortServerUrl', " + - "serverZone=$serverZone, fetchTimeoutMillis=$fetchTimeoutMillis, fetchRetries=$fetchRetries, " + + return "RemoteEvaluationConfig(debug=$debug, serverUrl='$serverUrl', serverZone=$serverZone, " + + "fetchTimeoutMillis=$fetchTimeoutMillis, fetchRetries=$fetchRetries, " + "fetchRetryBackoffMinMillis=$fetchRetryBackoffMinMillis, " + "fetchRetryBackoffMaxMillis=$fetchRetryBackoffMaxMillis, " + "fetchRetryBackoffScalar=$fetchRetryBackoffScalar, httpProxy=$httpProxy)" diff --git a/src/main/kotlin/deployment/DeploymentRunner.kt b/src/main/kotlin/deployment/DeploymentRunner.kt index fe52ebb..0001278 100644 --- a/src/main/kotlin/deployment/DeploymentRunner.kt +++ b/src/main/kotlin/deployment/DeploymentRunner.kt @@ -21,6 +21,8 @@ import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.Executors import java.util.concurrent.TimeUnit +private const val MIN_COHORT_POLLING_INTERVAL = 60000L + internal class DeploymentRunner( private val config: LocalEvaluationConfig, private val flagConfigApi: FlagConfigApi, @@ -132,8 +134,10 @@ internal class DeploymentRunner( } private fun getCohortPollingInterval(): Long { - if (config.cohortSyncConfig == null || config.cohortSyncConfig.cohortPollingIntervalMillis < 60000) { - return 60000 + if (config.cohortSyncConfig == null || + config.cohortSyncConfig.cohortPollingIntervalMillis < MIN_COHORT_POLLING_INTERVAL + ) { + return MIN_COHORT_POLLING_INTERVAL } return config.cohortSyncConfig.cohortPollingIntervalMillis } From 91027d4d715603734884912d79b1d312ad9bb6e9 Mon Sep 17 00:00:00 2001 From: Brian Giori Date: Wed, 7 Aug 2024 16:17:33 -0700 Subject: [PATCH 13/15] fix: update to warning log --- src/main/kotlin/cohort/CohortApi.kt | 2 +- src/main/kotlin/flag/FlagConfigApi.kt | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/kotlin/cohort/CohortApi.kt b/src/main/kotlin/cohort/CohortApi.kt index b2be50e..35af45d 100644 --- a/src/main/kotlin/cohort/CohortApi.kt +++ b/src/main/kotlin/cohort/CohortApi.kt @@ -70,7 +70,7 @@ internal class DynamicCohortApi( } catch (e: CohortTooLargeException) { throw e } catch (e: Exception) { - Logger.e("Downloading cohort $cohortId from proxy failed. Falling back to Amplitude.", e) + Logger.w("Downloading cohort $cohortId from proxy failed. Falling back to Amplitude.", e) metrics.onCohortDownloadOriginFallback(e) getCohort(serverUrl, cohortId, cohort) } diff --git a/src/main/kotlin/flag/FlagConfigApi.kt b/src/main/kotlin/flag/FlagConfigApi.kt index 0845641..69f1f4e 100644 --- a/src/main/kotlin/flag/FlagConfigApi.kt +++ b/src/main/kotlin/flag/FlagConfigApi.kt @@ -4,6 +4,7 @@ import com.amplitude.experiment.LocalEvaluationMetrics import com.amplitude.experiment.evaluation.EvaluationFlag import com.amplitude.experiment.util.BackoffConfig import com.amplitude.experiment.util.LocalEvaluationMetricsWrapper +import com.amplitude.experiment.util.Logger import com.amplitude.experiment.util.backoff import com.amplitude.experiment.util.get import okhttp3.HttpUrl @@ -34,6 +35,7 @@ internal class DynamicFlagConfigApi( try { getFlagConfigs(proxyUrl) } catch (e: Exception) { + Logger.w("Downloading flags from proxy failed. Falling back to Amplitude.", e) metrics.onFlagConfigFetchOriginFallback(e) getFlagConfigs(serverUrl) } From e53a46cc5179ec2687910406207ac9860f8a9f70 Mon Sep 17 00:00:00 2001 From: Brian Giori Date: Wed, 7 Aug 2024 16:20:57 -0700 Subject: [PATCH 14/15] fix: update log line with flag keys --- src/main/kotlin/LocalEvaluationClient.kt | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/main/kotlin/LocalEvaluationClient.kt b/src/main/kotlin/LocalEvaluationClient.kt index ca09b0c..57dc4ae 100644 --- a/src/main/kotlin/LocalEvaluationClient.kt +++ b/src/main/kotlin/LocalEvaluationClient.kt @@ -24,6 +24,7 @@ import com.amplitude.experiment.util.LocalEvaluationMetricsWrapper import com.amplitude.experiment.util.Logger import com.amplitude.experiment.util.USER_GROUP_TYPE import com.amplitude.experiment.util.filterDefaultVariants +import com.amplitude.experiment.util.getAllCohortIds import com.amplitude.experiment.util.getGroupedCohortIds import com.amplitude.experiment.util.toEvaluationContext import com.amplitude.experiment.util.toVariants @@ -122,7 +123,15 @@ class LocalEvaluationClient internal constructor( val groupedCohortIds = flagConfigs.getGroupedCohortIds() if (cohortStorage == null) { if (groupedCohortIds.isNotEmpty()) { - Logger.e("Local evaluation flags target cohorts but cohort targeting is not configured.") + val flagKeys = flagConfigs.mapNotNull { flag -> + val cohortIds = flag.getAllCohortIds() + if (cohortIds.isEmpty()) { + null + } else { + flag.key + } + } + Logger.e("Local evaluation flags $flagKeys target cohorts but cohort targeting is not configured.") } return user } From d468d771b60a7b4f5f76ac15f45a98b374eb64da Mon Sep 17 00:00:00 2001 From: Brian Giori Date: Thu, 8 Aug 2024 11:40:27 -0700 Subject: [PATCH 15/15] fix: add warning log for targeted cohort missing --- src/main/kotlin/cohort/CohortStorage.kt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/kotlin/cohort/CohortStorage.kt b/src/main/kotlin/cohort/CohortStorage.kt index 0250362..aa17925 100644 --- a/src/main/kotlin/cohort/CohortStorage.kt +++ b/src/main/kotlin/cohort/CohortStorage.kt @@ -114,7 +114,11 @@ internal class InMemoryCohortStorage : CohortStorage { val result = mutableSetOf() lock.read { for (cohortId in cohortIds) { - val cohort = cohortStore[cohortId] ?: continue + val cohort = cohortStore[cohortId] + if (cohort == null) { + Logger.w("Targeted $groupType cohort $cohortId not found in storage.") + continue + } if (cohort.groupType != groupType) { continue }