Merge pull request #41 from statsig-io/exposure-deduping

add exposure deduping logic
statsig-io · Jan 27, 2022 · 501fcd7 · 501fcd7
2 parents 30a9d32 + cbd81dd
commit 501fcd7
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 41 deletions.
diff --git a/gradle.properties b/gradle.properties
@@ -1,3 +1,3 @@
 android.useAndroidX=true
-libraryVersion=4.2.2
+libraryVersion=4.2.3
 kotlinVersion=1.5.0
diff --git a/src/main/java/com/statsig/androidsdk/StatsigClient.kt b/src/main/java/com/statsig/androidsdk/StatsigClient.kt
@@ -271,6 +271,7 @@ internal class StatsigClient() {
      */
     suspend fun updateUser(user: StatsigUser?) {
         enforceInitialized("updateUser")
+        logger.onUpdateUser()
         pollingJob?.cancel()
         this.user = normalizeUser(user)
         store.loadAndResetStickyUserValues(user?.userID)

diff --git a/src/main/java/com/statsig/androidsdk/StatsigLogger.kt b/src/main/java/com/statsig/androidsdk/StatsigLogger.kt
@@ -6,7 +6,9 @@ import android.content.SharedPreferences
 import com.google.gson.annotations.SerializedName
 import kotlinx.coroutines.*
 
-internal const val MAX_EVENTS: Int = 10
+private const val EXPOSURE_DEDUPE_INTERVAL: Long = 10 * 60 * 1000
+
+internal const val MAX_EVENTS: Int = 50
 internal const val FLUSH_TIMER_MS: Long = 60000
 
 internal const val CONFIG_EXPOSURE = "statsig::config_exposure"
@@ -37,6 +39,8 @@ internal class StatsigLogger(
     // Modify in a single thread only
     internal var events = arrayListOf<LogEvent>()
 
+    private var loggedExposures: MutableMap<String, Long> = HashMap()
+
     suspend fun log(event: LogEvent) {
         withContext(singleThreadDispatcher) {
             events.add(event)
@@ -47,6 +51,10 @@ internal class StatsigLogger(
         }
     }
 
+    fun onUpdateUser() {
+        this.loggedExposures = HashMap()
+    }
+
     suspend fun flush() {
         withContext(singleThreadDispatcher) {
             if (events.size == 0) {
@@ -60,28 +68,34 @@ internal class StatsigLogger(
 
     suspend fun logGateExposure(gateName: String, gateValue: Boolean, ruleID: String,
                                 secondaryExposures: Array<Map<String, String>>, user: StatsigUser?) {
-        withContext(singleThreadDispatcher) {
-            var event = LogEvent(GATE_EXPOSURE)
-            event.user = user
-            event.metadata =
-                mapOf(
-                    "gate" to gateName,
-                    "gateValue" to gateValue.toString(),
-                    "ruleID" to ruleID
-                )
-            event.secondaryExposures = secondaryExposures
-            log(event)
+        val dedupeKey = gateName + gateValue + ruleID
+        if (shouldLogExposure(dedupeKey)) {
+            withContext(singleThreadDispatcher) {
+                var event = LogEvent(GATE_EXPOSURE)
+                event.user = user
+                event.metadata =
+                    mapOf(
+                        "gate" to gateName,
+                        "gateValue" to gateValue.toString(),
+                        "ruleID" to ruleID
+                    )
+                event.secondaryExposures = secondaryExposures
+                log(event)
+            }
         }
     }
 
     suspend fun logConfigExposure(configName: String, ruleID: String, secondaryExposures: Array<Map<String, String>>,
                                   user: StatsigUser?) {
-        withContext(singleThreadDispatcher) {
-            var event = LogEvent(CONFIG_EXPOSURE)
-            event.user = user
-            event.metadata = mapOf("config" to configName, "ruleID" to ruleID)
-            event.secondaryExposures = secondaryExposures
-            log(event)
+        val dedupeKey = configName + ruleID
+        if (shouldLogExposure(dedupeKey)) {
+            withContext(singleThreadDispatcher) {
+                var event = LogEvent(CONFIG_EXPOSURE)
+                event.user = user
+                event.metadata = mapOf("config" to configName, "ruleID" to ruleID)
+                event.secondaryExposures = secondaryExposures
+                log(event)
+            }
         }
     }
 
@@ -90,4 +104,14 @@ internal class StatsigLogger(
         flush()
         executor.shutdown()
     }
+
+    private fun shouldLogExposure(key: String): Boolean {
+        val now = System.currentTimeMillis()
+        val lastTime = loggedExposures[key] ?: 0
+        if (lastTime >= now - EXPOSURE_DEDUPE_INTERVAL) {
+            return false
+        }
+        loggedExposures[key] = now
+        return true
+    }
 }
diff --git a/src/test/java/com/statsig/androidsdk/StatsigTest.kt b/src/test/java/com/statsig/androidsdk/StatsigTest.kt
@@ -164,7 +164,7 @@ class StatsigTest : IStatsigCallback {
             "default string instead",
             config.getString("otherNumber", "default string instead")
         )
-        assertEquals("default", client.getConfig("test_config").getRuleID())
+        assertEquals("default",config.getRuleID())
 
         val invalidConfig = client.getConfig("not_a_valid_config")
         assertEquals("", invalidConfig.getRuleID())
@@ -177,10 +177,16 @@ class StatsigTest : IStatsigCallback {
         client.logEvent("test_event1", 1.toDouble(), mapOf("key" to "value"));
         client.logEvent("test_event2", mapOf("key" to "value2"));
         client.logEvent("test_event3", "1");
+
+        // check a few previously checked gate and config; they should not result in exposure logs due to deduping logic
+        client.checkGate("always_on")
+        client.getConfig("test_config")
+        client.getExperiment("exp")
+
         client.shutdown()
 
         val parsedLogs = Gson().fromJson(flushedLogs, LogEventData::class.java)
-        assertEquals(10, parsedLogs.events.count())
+        assertEquals(9, parsedLogs.events.count())
         // first 2 are exposures pre initialize() completion
         assertEquals("custom_stable_id", parsedLogs.statsigMetadata.stableID);
         assertEquals("custom_stable_id", client.getStableID())
@@ -228,36 +234,36 @@ class StatsigTest : IStatsigCallback {
         )
 
         // validate exp exposure
-        assertEquals(parsedLogs.events[6].eventName, "statsig::config_exposure")
-        assertEquals(parsedLogs.events[6].user!!.userID, "123")
-        assertEquals(parsedLogs.events[6].metadata!!["config"], "exp")
-        assertEquals(parsedLogs.events[6].metadata!!["ruleID"], "exp_rule")
-        assertEquals(parsedLogs.events[6].secondaryExposures?.count() ?: 1, 0)
+        assertEquals(parsedLogs.events[5].eventName, "statsig::config_exposure")
+        assertEquals(parsedLogs.events[5].user!!.userID, "123")
+        assertEquals(parsedLogs.events[5].metadata!!["config"], "exp")
+        assertEquals(parsedLogs.events[5].metadata!!["ruleID"], "exp_rule")
+        assertEquals(parsedLogs.events[5].secondaryExposures?.count() ?: 1, 0)
 
         // Validate custom logs
-        assertEquals(parsedLogs.events[7].eventName, "test_event1")
-        assertEquals(parsedLogs.events[7].user!!.userID, "123")
-        assertEquals(parsedLogs.events[7].value, 1.0)
+        assertEquals(parsedLogs.events[6].eventName, "test_event1")
+        assertEquals(parsedLogs.events[6].user!!.userID, "123")
+        assertEquals(parsedLogs.events[6].value, 1.0)
         assertEquals(
-            Gson().toJson(parsedLogs.events[7].metadata),
+            Gson().toJson(parsedLogs.events[6].metadata),
             Gson().toJson(mapOf("key" to "value"))
         )
-        assertNull(parsedLogs.events[7].secondaryExposures)
+        assertNull(parsedLogs.events[6].secondaryExposures)
 
-        assertEquals(parsedLogs.events[8].eventName, "test_event2")
-        assertEquals(parsedLogs.events[8].user!!.userID, "123")
-        assertEquals(parsedLogs.events[8].value, null)
+        assertEquals(parsedLogs.events[7].eventName, "test_event2")
+        assertEquals(parsedLogs.events[7].user!!.userID, "123")
+        assertEquals(parsedLogs.events[7].value, null)
         assertEquals(
-            Gson().toJson(parsedLogs.events[8].metadata),
+            Gson().toJson(parsedLogs.events[7].metadata),
             Gson().toJson(mapOf("key" to "value2"))
         )
-        assertNull(parsedLogs.events[8].secondaryExposures)
+        assertNull(parsedLogs.events[7].secondaryExposures)
 
-        assertEquals(parsedLogs.events[9].eventName, "test_event3")
-        assertEquals(parsedLogs.events[9].user!!.userID, "123")
-        assertEquals(parsedLogs.events[9].value, "1")
-        assertNull(parsedLogs.events[9].metadata)
-        assertNull(parsedLogs.events[9].secondaryExposures)
+        assertEquals(parsedLogs.events[8].eventName, "test_event3")
+        assertEquals(parsedLogs.events[8].user!!.userID, "123")
+        assertEquals(parsedLogs.events[8].value, "1")
+        assertNull(parsedLogs.events[8].metadata)
+        assertNull(parsedLogs.events[8].secondaryExposures)
         return Unit
     }
 }