Skip to content

Commit

Permalink
Incremental feature flags rollout (#3355)
Browse files Browse the repository at this point in the history 
Task/Issue URL: https://app.asana.com/0/72649045549333/1204971805879942/f

### Description
Add incremental feature rollouts

### Steps to test this PR
Follow test steps in [asana](https://app.asana.com/0/0/1205103489376199/f) task
  • Loading branch information
@aitorvs
aitorvs authored Jul 24, 2023
1 parent 4e1221a commit 11b2c9a
Show file tree
Hide file tree
Showing 9 changed files with 1,264 additions and 39 deletions.
107 changes: 78 additions & 29 deletions ...iler/src/main/java/com/duckduckgo/anvil/compiler/ContributesRemoteFeatureCodeGenerator.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,8 @@ class ContributesRemoteFeatureCodeGenerator : CodeGenerator {
.addFunction(createParseJsonFun(module))
.addFunction(createParseExceptions(module))
.addFunction(createInvokeMethod(boundType))
.addType(createJsonRolloutDataClass(generatedPackage, module))
.addType(createJsonRolloutStepDataClass(generatedPackage, module))
.addType(createJsonToggleDataClass(generatedPackage, module))
.addType(createJsonFeatureDataClass(generatedPackage, module))
.addType(createJsonExceptionDataClass(generatedPackage, module))
Expand Down Expand Up @@ -408,16 +410,31 @@ class ContributesRemoteFeatureCodeGenerator : CodeGenerator {
exceptionStore.insertAll(exceptions)
val isEnabled = (feature.state == "enabled") || (appBuildConfig.flavor == %T && feature.state == "internal")
this.feature.get().invokeMethod("self").setEnabled(Toggle.State(isEnabled, feature.minSupportedVersion))
this.feature.get().invokeMethod("self").setEnabled(
Toggle.State(
remoteEnableState = isEnabled,
enable = isEnabled,
minSupportedVersion = feature.minSupportedVersion,
)
)
// Handle sub-features
feature.features?.forEach { subfeature ->
subfeature.value.let { jsonObject ->
val jsonToggle = JsonToggle(jsonObject)
subfeature.value.let { jsonToggle ->
val previousState = this.feature.get().invokeMethod(subfeature.key).getRawStoredState()
// we try to honour the previous state
// else we resort to compute it using isEnabled()
val previousStateValue = previousState?.enable ?: this.feature.get().invokeMethod(subfeature.key).isEnabled()
val previousRolloutStep = previousState?.rolloutStep
val newStateValue = (jsonToggle.state == "enabled" || (appBuildConfig.flavor == %T && jsonToggle.state == "internal"))
this.feature.get().invokeMethod(subfeature.key).setEnabled(
Toggle.State(
enable = jsonToggle.state == "enabled" || (appBuildConfig.flavor == %T && jsonToggle.state == "internal"),
remoteEnableState = newStateValue,
enable = previousStateValue,
minSupportedVersion = jsonToggle.minSupportedVersion?.toInt(),
rollout = jsonToggle?.rollout?.steps?.map { it.percent },
rolloutStep = previousRolloutStep,
),
)
}
Expand Down Expand Up @@ -548,51 +565,86 @@ class ContributesRemoteFeatureCodeGenerator : CodeGenerator {
.build()
}

private fun createJsonToggleDataClass(
private fun createJsonRolloutDataClass(
generatedPackage: String,
module: ModuleDescriptor,
): TypeSpec {
return TypeSpec.classBuilder(FqName("$generatedPackage.JsonToggle").asClassName(module))
return TypeSpec.classBuilder(FqName("$generatedPackage.JsonToggleRollout").asClassName(module))
.addModifiers(KModifier.PRIVATE)
.addModifiers(KModifier.DATA)
.primaryConstructor(
FunSpec.constructorBuilder()
.addParameter(
"map",
Map::class.asClassName().parameterizedBy(String::class.asClassName(), Any::class.asClassName().copy(nullable = true)),
"steps",
List::class.asClassName().parameterizedBy(FqName("JsonToggleRolloutStep").asClassName(module)),
)
.build(),
)
// Property map that matches params to generate data class
.addProperty(
PropertySpec
.builder(
"map",
Map::class.asClassName().parameterizedBy(String::class.asClassName(), Any::class.asClassName().copy(nullable = true)),
)
.initializer("map")
PropertySpec.builder(
"steps",
List::class.asClassName().parameterizedBy(FqName("JsonToggleRolloutStep").asClassName(module)),
).initializer("steps")
.build(),
)
.addProperty(
PropertySpec
.builder(
"attributes",
Map::class.asClassName().parameterizedBy(String::class.asClassName(), Any::class.asClassName().copy(nullable = true)),
.build()
}

private fun createJsonRolloutStepDataClass(
generatedPackage: String,
module: ModuleDescriptor,
): TypeSpec {
return TypeSpec.classBuilder(FqName("$generatedPackage.JsonToggleRolloutStep").asClassName(module))
.addModifiers(KModifier.PRIVATE)
.addModifiers(KModifier.DATA)
.primaryConstructor(
FunSpec.constructorBuilder()
.addParameter("percent", Double::class.asClassName())
.build(),
)
.addProperty(PropertySpec.builder("percent", Double::class.asClassName()).initializer("percent").build())
.build()
}

private fun createJsonToggleDataClass(
generatedPackage: String,
module: ModuleDescriptor,
): TypeSpec {
return TypeSpec.classBuilder(FqName("$generatedPackage.JsonToggle").asClassName(module))
.addModifiers(KModifier.PRIVATE)
.addModifiers(KModifier.DATA)
.primaryConstructor(
FunSpec.constructorBuilder()
.addParameter(
"state",
String::class.asClassName().copy(nullable = true),
)
.addParameter(
"minSupportedVersion",
Double::class.asClassName().copy(nullable = true),
)
.addParameter(
"rollout",
FqName("JsonToggleRollout").asClassName(module).copy(nullable = true),
)
.addModifiers(KModifier.PRIVATE)
.initializer(CodeBlock.of("map.withDefault { null }"))
.build(),
)
.addProperty(
PropertySpec
.builder("state", String::class.asClassName().copy(nullable = true))
.delegate("attributes")
.initializer("state")
.build(),
)
.addProperty(
PropertySpec
.builder("minSupportedVersion", Double::class.asClassName().copy(nullable = true))
.delegate("attributes")
.initializer("minSupportedVersion")
.build(),
)
.addProperty(
PropertySpec
.builder("rollout", FqName("JsonToggleRollout").asClassName(module).copy(nullable = true))
.initializer("rollout")
.build(),
)
.build()
Expand Down Expand Up @@ -625,10 +677,7 @@ class ContributesRemoteFeatureCodeGenerator : CodeGenerator {
"features",
Map::class.asClassName().parameterizedBy(
String::class.asClassName(),
Map::class.asClassName().parameterizedBy(
String::class.asClassName(),
Any::class.asClassName().copy(nullable = true),
),
FqName("JsonToggle").asClassName(module),
).copy(nullable = true),
)
.build(),
Expand Down Expand Up @@ -661,7 +710,7 @@ class ContributesRemoteFeatureCodeGenerator : CodeGenerator {
"features",
Map::class.asClassName().parameterizedBy(
String::class.asClassName(),
Map::class.asClassName().parameterizedBy(String::class.asClassName(), Any::class.asClassName().copy(nullable = true)),
FqName("JsonToggle").asClassName(module),
).copy(nullable = true),
)
.initializer("features")
Expand Down
3 changes: 3 additions & 0 deletions app/src/androidTest/java/com/duckduckgo/app/email/EmailInjectorJsTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import com.duckduckgo.app.global.DispatcherProvider
import com.duckduckgo.autofill.api.Autofill
import com.duckduckgo.autofill.api.AutofillFeature
import com.duckduckgo.feature.toggles.api.Toggle
import com.duckduckgo.feature.toggles.api.Toggle.State
import java.io.BufferedReader
import org.junit.Before
import org.junit.Test
Expand Down Expand Up @@ -64,6 +65,8 @@ class EmailInjectorJsTest {
override fun setEnabled(state: Toggle.State) {
this.state = state
}

override fun getRawStoredState(): State? = this.state
},
)
whenever(mockAutofill.isAnException(any())).thenReturn(false)
Expand Down
15 changes: 15 additions & 0 deletions ...fill-impl/src/test/java/com/duckduckgo/autofill/impl/AutofillCapabilityCheckerImplTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,34 +168,49 @@ class AutofillCapabilityCheckerImplTest {
return object : Toggle {
override fun isEnabled(): Boolean = topLevelFeatureEnabled
override fun setEnabled(state: State) {}
override fun getRawStoredState(): State? {
TODO("Not yet implemented")
}
}
}

override fun canInjectCredentials(): Toggle {
return object : Toggle {
override fun isEnabled(): Boolean = canInjectCredentials
override fun setEnabled(state: State) {}
override fun getRawStoredState(): State? {
TODO("Not yet implemented")
}
}
}

override fun canSaveCredentials(): Toggle {
return object : Toggle {
override fun isEnabled(): Boolean = canSaveCredentials
override fun setEnabled(state: State) {}
override fun getRawStoredState(): State? {
TODO("Not yet implemented")
}
}
}

override fun canGeneratePasswords(): Toggle {
return object : Toggle {
override fun isEnabled(): Boolean = canGeneratePassword
override fun setEnabled(state: State) {}
override fun getRawStoredState(): State? {
TODO("Not yet implemented")
}
}
}

override fun canAccessCredentialManagement(): Toggle {
return object : Toggle {
override fun isEnabled(): Boolean = canAccessCredentialManagement
override fun setEnabled(state: State) {}
override fun getRawStoredState(): State? {
TODO("Not yet implemented")
}
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions ...impl/src/test/java/com/duckduckgo/contentscopescripts/impl/RealContentScopeScriptsTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,8 @@ class RealContentScopeScriptsTest {
override fun setEnabled(state: State) {
// not implemented
}

override fun getRawStoredState(): State? = null
}

class DisabledToggle : Toggle {
Expand All @@ -224,6 +226,8 @@ class RealContentScopeScriptsTest {
override fun setEnabled(state: State) {
// not implemented
}

override fun getRawStoredState(): State? = null
}

companion object {
Expand Down
101 changes: 99 additions & 2 deletions ...es/feature-toggles-api/src/main/java/com/duckduckgo/feature/toggles/api/FeatureToggles.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@

package com.duckduckgo.feature.toggles.api

import com.duckduckgo.feature.toggles.api.Toggle.State
import java.lang.IllegalArgumentException
import java.lang.IllegalStateException
import java.lang.reflect.Method
import java.lang.reflect.Proxy
import kotlin.random.Random

class FeatureToggles private constructor(
private val store: Toggle.Store,
Expand Down Expand Up @@ -108,14 +110,35 @@ class FeatureToggles private constructor(
}

interface Toggle {
/**
* This is the method that SHALL be called to get whether a feature is enabled or not. DO NOT USE [getRawStoredState] for that
* @return `true` if the feature should be enabled, `false` otherwise
*/
fun isEnabled(): Boolean

/**
* The usage of this API is only useful for internal/dev settings/features
* If you find yourself having to call this method in production code, then YOUR DOING SOMETHING WRONG
*
* @param state update the stored [State] of the feature flag
*/
fun setEnabled(state: State)

/**
* The usage of this API is only useful for internal/dev settings/features
* If you find yourself having to call this method in production code, then YOUR DOING SOMETHING WRONG
*
* @return the raw [State] store for this feature flag.
*/
fun getRawStoredState(): State?

data class State(
val remoteEnableState: Boolean? = null,
val enable: Boolean = false,
val minSupportedVersion: Int? = null,
val enabledOverrideValue: Boolean? = null,
val rollout: List<Double>? = null,
val rolloutStep: Int? = null,
)

interface Store {
Expand All @@ -138,12 +161,86 @@ internal class ToggleImpl constructor(
private val appVersionProvider: () -> Int,
) : Toggle {
override fun isEnabled(): Boolean {
store.get(key)?.let { state ->
return state.enable && appVersionProvider.invoke() >= (state.minSupportedVersion ?: 0)
return store.get(key)?.let { state ->
state.remoteEnableState?.let { remoteState ->
remoteState && state.enable && appVersionProvider.invoke() >= (state.minSupportedVersion ?: 0)
} ?: defaultValue
} ?: return defaultValue
}

@Suppress("NAME_SHADOWING")
override fun setEnabled(state: Toggle.State) {
var state = state

// remote is disabled, store and skip everything
if (state.remoteEnableState == false) {
store.set(key, state)
return
}

// local state is false (and remote state is enabled) try incremental rollout
if (!state.enable) {
state = calculateRolloutState(state)
}

// remote state is null, means app update. Propagate the local state to remote state
if (state.remoteEnableState == null) {
state = state.copy(remoteEnableState = state.enable)
}

// finally store the state
store.set(key, state)
}

override fun getRawStoredState(): State? {
return store.get(key)
}

private fun calculateRolloutState(
state: State,
): State {
fun sample(probability: Double): Boolean {
val random = Random.nextDouble(100.0)
return random < probability
}
val rolloutStep = state.rolloutStep

// there is no rollout, return whatever the previous state was
if (state.rollout.isNullOrEmpty()) return state

val sortedRollout = state.rollout.sorted().filter { it in 0.0..100.0 }
if (sortedRollout.isEmpty()) return state

when (rolloutStep) {
// first time we see the rollout, pick the last step
null -> {
val step = sortedRollout.last()
val isEnabled = sample(step.toDouble())
return state.copy(
enable = isEnabled,
rolloutStep = sortedRollout.size,
)
}
// this is an error and should not happen, don't change state
0 -> {
return state
}
else -> {
val steps = sortedRollout.size
val lastStep = state.rolloutStep

for (s in lastStep until steps) {
// determine effective probability
val probability = (sortedRollout[s] - sortedRollout[s - 1]) / (100.0 - sortedRollout[s - 1])
if (sample(probability * 100.0)) {
return state.copy(
enable = true,
rolloutStep = s + 1,
)
}
}
return state.copy(rolloutStep = sortedRollout.size)
}
}
}
}
Loading

0 comments on commit 11b2c9a

Please sign in to comment.