Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Impression events #25

Merged
merged 3 commits into from
Jul 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ androidx-work-testing = { group = "androidx.work", name = "work-testing", versio
androidx-lifecycle-process = { group = "androidx.lifecycle", name = "lifecycle-process", version.ref = "lifecycleProcess" }
kotlinx-coroutines-test = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-test", version.ref = "kotlinxCoroutinesTest" }
robolectric-test = { group = "org.robolectric", name = "robolectric", version = "4.7.3" }
awaitility = { group = "org.awaitility", name = "awaitility", version = "4.2.1" }

okhttp = { group = "com.squareup.okhttp3", name = "okhttp", version.ref = "okhttp" }
okhttp-mockserver = { group = "com.squareup.okhttp3", name = "mockwebserver", version.ref = "okhttp" }
Expand Down
1 change: 1 addition & 0 deletions unleashandroidsdk/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ dependencies {
testImplementation(libs.mockito)
testImplementation(libs.robolectric.test)
testImplementation(libs.okhttp.mockserver)
testImplementation(libs.awaitility)
androidTestImplementation(libs.kotlinx.coroutines.test)
androidTestImplementation(libs.assertj)
androidTestImplementation(libs.mockito)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ import io.getunleash.android.cache.InMemoryToggleCache
import io.getunleash.android.cache.ObservableCache
import io.getunleash.android.cache.ObservableToggleCache
import io.getunleash.android.cache.ToggleCache
import io.getunleash.android.data.ImpressionEvent
import io.getunleash.android.data.Toggle
import io.getunleash.android.data.UnleashContext
import io.getunleash.android.data.UnleashState
import io.getunleash.android.data.Variant
import io.getunleash.android.events.UnleashEventListener
import io.getunleash.android.http.ClientBuilder
Expand All @@ -28,9 +31,12 @@ import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.cancel
import kotlinx.coroutines.channels.BufferOverflow
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.takeWhile
import kotlinx.coroutines.launch
import kotlinx.coroutines.runBlocking
Expand All @@ -53,7 +59,8 @@ class DefaultUnleash(
unleashContext: UnleashContext = UnleashContext(),
cacheImpl: ToggleCache = InMemoryToggleCache(),
eventListeners: List<UnleashEventListener> = emptyList(),
private val lifecycle: Lifecycle = getLifecycle(androidContext)
private val lifecycle: Lifecycle = getLifecycle(androidContext),
private val coroutineScope: CoroutineScope = unleashScope
) : Unleash {
companion object {
private const val TAG = "Unleash"
Expand All @@ -62,12 +69,16 @@ class DefaultUnleash(
private val unleashContextState = MutableStateFlow(unleashContext)
private val metrics: MetricsCollector
private val taskManager: LifecycleAwareTaskManager
private val cache: ObservableToggleCache = ObservableCache(cacheImpl)
private val cache: ObservableToggleCache = ObservableCache(cacheImpl, coroutineScope)
private var started = AtomicBoolean(false)
private var ready = AtomicBoolean(false)
private val readyFlow = MutableStateFlow(false)
private val fetcher: UnleashFetcher?
private val networkStatusHelper = NetworkStatusHelper(androidContext)
private val impressionEventsFlow = MutableSharedFlow<ImpressionEvent>(
extraBufferCapacity = 64,
onBufferOverflow = BufferOverflow.DROP_OLDEST
)

init {
val httpClientBuilder = ClientBuilder(unleashConfig, androidContext)
Expand All @@ -87,7 +98,8 @@ class DefaultUnleash(
) else null
taskManager = LifecycleAwareTaskManager(
dataJobs = buildDataJobs(fetcher, metricsSender),
networkAvailable = networkStatusHelper.isAvailable()
networkAvailable = networkStatusHelper.isAvailable(),
scope = coroutineScope
)
if (!unleashConfig.delayedInitialization) {
start(eventListeners)
Expand All @@ -96,7 +108,10 @@ class DefaultUnleash(
}
}

fun start(eventListeners: List<UnleashEventListener> = emptyList()) {
fun start(
eventListeners: List<UnleashEventListener> = emptyList(),
bootstrap: List<Toggle> = emptyList()
) {
if (!started.compareAndSet(false, true)) {
Log.w(TAG, "Unleash already started, ignoring start call")
return
Expand All @@ -112,6 +127,10 @@ class DefaultUnleash(
cache.subscribeTo(it.getFeaturesReceivedFlow())
}
lifecycle.addObserver(taskManager)
if (bootstrap.isNotEmpty()) {
Log.i(TAG, "Using provided bootstrap toggles")
cache.write(UnleashState(unleashContextState.value, bootstrap.associateBy { it.name }))
}
}

private fun buildDataJobs(fetcher: UnleashFetcher?, metricsSender: MetricsReporter) = buildList {
Expand Down Expand Up @@ -139,7 +158,7 @@ class DefaultUnleash(
val backupDir = CacheDirectoryProvider(unleashConfig.localStorageConfig, androidContext)
.getCacheDirectory("unleash_backup")
val localBackup = LocalBackup(backupDir)
unleashScope.launch {
coroutineScope.launch {
withContext(Dispatchers.IO) {
unleashContextState.asStateFlow().takeWhile { !ready.get() }.collect { ctx ->
Log.d(TAG, "Loading state from backup for $ctx")
Expand All @@ -158,30 +177,42 @@ class DefaultUnleash(
}

override fun isEnabled(toggleName: String, defaultValue: Boolean): Boolean {
val enabled = cache.get(toggleName)?.enabled ?: defaultValue
val toggle = cache.get(toggleName)
val enabled = toggle?.enabled ?: defaultValue
val impressionData = toggle?.impressionData ?: unleashConfig.forceImpressionData
if (impressionData) {
emit(ImpressionEvent(toggleName, enabled, unleashContextState.value))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chriswk @sighphyre there's a chance that context might change from checking isEnabled to obtaining the context in this line. Is this something we should be worried about? Maybe the js proxy SDK won't suffer from this issue because there's a single thread and I'm not sure this method can be interrupted by other thread setting the context:
https://github.com/Unleash/unleash-proxy-client-js/blob/d13ad709213cad7bedd92c8a4c1bc9ee09d4a5c7/src/index.ts#L252-L269

Server side SDKs don't suffer from this problem because they need the context to evaluate isEnabled, therefore it's guaranteed that the context used for the evaluation is the same one sent in the impression event.

There is a solution for this... the cache should also store the current context and should return it when we get a toggle. It'd be something like: val (toggle, context) = cache.get(toggleName)

}
metrics.count(toggleName, enabled)
return enabled
}

override fun getVariant(toggleName: String, defaultValue: Variant): Variant {
val variant =
if (isEnabled(toggleName)) {
cache.get(toggleName)?.variant ?: defaultValue
} else {
defaultValue
}
val toggle = cache.get(toggleName)
val enabled = isEnabled(toggleName)
val variant = if (enabled) (toggle?.variant ?: defaultValue) else defaultValue
val impressionData = toggle?.impressionData ?: unleashConfig.forceImpressionData
if (impressionData) {
emit(ImpressionEvent(toggleName, enabled, unleashContextState.value, variant.name))
}
metrics.countVariant(toggleName, variant)
return variant
}

private fun emit(impressionEvent: ImpressionEvent) {
coroutineScope.launch {
impressionEventsFlow.emit(impressionEvent)
}
}

override fun refreshTogglesNow() {
runBlocking {
fetcher?.refreshToggles()
}
}

override fun refreshTogglesNowAsync() {
unleashScope.launch {
coroutineScope.launch {
withContext(Dispatchers.IO) {
fetcher?.refreshToggles()
}
Expand All @@ -208,19 +239,24 @@ class DefaultUnleash(
}

override fun addUnleashEventListener(listener: UnleashEventListener) {
unleashScope.launch {
coroutineScope.launch {
val firstReady = readyFlow.asStateFlow().filter { it }.first()
Log.d(TAG, "Ready state changed to $firstReady, notifying $listener")
listener.onReady()
}
coroutineScope.launch {
cache.getUpdatesFlow().collect {
if (ready.compareAndSet(false, true)) {
Log.d(TAG, "Unleash is now ready")
readyFlow.value = true
}
Log.d(TAG, "Cache updated, notifying $listener that state changed")
listener.onStateChanged()
}
}
unleashScope.launch {
readyFlow.asStateFlow().filter { it }.collect {
Log.d(TAG, "Ready state changed, notifying $listener")
listener.onReady()
coroutineScope.launch {
impressionEventsFlow.collect { event ->
listener.onImpression(event)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ data class UnleashConfig(
val metricsStrategy: DataStrategy = DataStrategy(
pauseOnBackground = true,
),
val delayedInitialization: Boolean = true
val delayedInitialization: Boolean = true,
val forceImpressionData: Boolean = false
) {
companion object {
val instanceId: String = UUID.randomUUID().toString()
Expand All @@ -32,7 +33,6 @@ data class UnleashConfig(
*/
fun newBuilder(appName: String): Builder = Builder(appName)
}

val instanceId: String get() = Companion.instanceId

fun getApplicationHeaders(strategy: DataStrategy): Map<String, String> {
Expand All @@ -49,10 +49,12 @@ data class UnleashConfig(
* Builder for [io.getunleash.android.UnleashConfig]
*/
data class Builder(
var appName: String,
var proxyUrl: String? = null,
var clientKey: String? = null
private var appName: String,
private var proxyUrl: String? = null,
private var clientKey: String? = null
) {
private var delayedInitialization: Boolean = true
private var forceImpressionData: Boolean = false
val pollingStrategy: DataStrategy.Builder = DataStrategy()
.newBuilder(parent = this)
val metricsStrategy: DataStrategy.Builder = DataStrategy()
Expand All @@ -69,11 +71,19 @@ data class UnleashConfig(
appName = appName,
pollingStrategy = pollingStrategy.build(),
metricsStrategy = metricsStrategy.build(),
delayedInitialization = delayedInitialization,
forceImpressionData = forceImpressionData,
localStorageConfig = localStorageConfig.build()
)
}

fun proxyUrl(proxyUrl: String) = apply { this.proxyUrl = proxyUrl }
fun clientKey(clientKey: String) = apply { this.clientKey = clientKey }

fun delayedInitialization(delayedInitialization: Boolean) =
apply { this.delayedInitialization = delayedInitialization }

fun forceImpressionData(forceImpressionData: Boolean) =
apply { this.forceImpressionData = forceImpressionData }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@ import android.util.Log
import io.getunleash.android.data.Toggle
import io.getunleash.android.data.UnleashState
import io.getunleash.android.unleashScope
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.asSharedFlow
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext

class ObservableCache(private val cache: ToggleCache) : ObservableToggleCache {
class ObservableCache(private val cache: ToggleCache, private val coroutineScope: CoroutineScope = unleashScope) : ObservableToggleCache {
companion object {
private const val TAG = "ObservableCache"
}
Expand All @@ -27,14 +28,17 @@ class ObservableCache(private val cache: ToggleCache) : ObservableToggleCache {

override fun write(state: UnleashState) {
cache.write(state)
unleashScope.launch {
Log.d(TAG, "Done writing cache")
coroutineScope.launch {
Log.d(TAG, "Emitting new state with ${state.toggles.size} toggles for ${state.context}")
events.emit(state)
}
Log.d(TAG, "Done sending event")
}

override fun subscribeTo(featuresReceived: Flow<UnleashState>) {
Log.d(TAG, "Subscribing to observable cache")
unleashScope.launch {
coroutineScope.launch {
featuresReceived.collect { state ->
withContext(Dispatchers.IO) {
Log.d(TAG, "Storing new state with ${state.toggles.size} toggles for $state.context")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package io.getunleash.android.data

import java.util.UUID

data class ImpressionEvent(
val featureName: String,
val enabled: Boolean,
val context: UnleashContext,
val variant: String? = null,
val eventId: String = UUID.randomUUID().toString(),
)
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,9 @@ package io.getunleash.android.data
* @property enabled Did this toggle get evaluated to true
* @property variant used by [io.getunleash.UnleashClientSpec.getVariant] to get the variant data
*/
data class Toggle(val name: String, val enabled: Boolean, val variant: Variant = Variant(name = "disabled"))
data class Toggle(
val name: String,
val enabled: Boolean,
val impressionData: Boolean = false,
val variant: Variant = Variant(name = "disabled")
)
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package io.getunleash.android.events

import io.getunleash.android.data.ImpressionEvent

interface UnleashEventListener {
fun onReady() {}
fun onStateChanged() {}
fun onImpression(event: ImpressionEvent) {}
}
Loading