From f25e45882336f99ee0e134497374fd8f76c9a25a Mon Sep 17 00:00:00 2001 From: Brian Giori Date: Thu, 1 Jun 2023 16:23:04 -0700 Subject: [PATCH] feat: add exp key to variant and exposure (#26) --- .../ConnectorExposureTrackingProvider.kt | 1 + .../experiment/DefaultExperimentClient.kt | 4 +-- .../com/amplitude/experiment/Experiment.kt | 10 ++++--- .../java/com/amplitude/experiment/Exposure.kt | 1 + .../experiment/ExposureTrackingProvider.kt | 4 ++- .../java/com/amplitude/experiment/Variant.kt | 10 +++++++ .../com/amplitude/experiment/util/Variant.kt | 9 +++++- .../ConnectorExposureTrackingProviderTest.kt | 10 +++---- .../experiment/ExperimentClientTest.kt | 27 +++++++++++++++++ .../UserSessionExposureTrackerTest.kt | 30 +++++++++---------- .../com/amplitude/experiment/VariantTest.kt | 5 +++- 11 files changed, 82 insertions(+), 29 deletions(-) diff --git a/sdk/src/main/java/com/amplitude/experiment/ConnectorExposureTrackingProvider.kt b/sdk/src/main/java/com/amplitude/experiment/ConnectorExposureTrackingProvider.kt index 0eaec50..edac7db 100644 --- a/sdk/src/main/java/com/amplitude/experiment/ConnectorExposureTrackingProvider.kt +++ b/sdk/src/main/java/com/amplitude/experiment/ConnectorExposureTrackingProvider.kt @@ -14,6 +14,7 @@ internal class ConnectorExposureTrackingProvider( eventProperties = mapOf( "flag_key" to exposure.flagKey, "variant" to exposure.variant, + "experiment_key" to exposure.experimentKey, ).filterNull() ) ) diff --git a/sdk/src/main/java/com/amplitude/experiment/DefaultExperimentClient.kt b/sdk/src/main/java/com/amplitude/experiment/DefaultExperimentClient.kt index 8fa2188..eb5bfc1 100644 --- a/sdk/src/main/java/com/amplitude/experiment/DefaultExperimentClient.kt +++ b/sdk/src/main/java/com/amplitude/experiment/DefaultExperimentClient.kt @@ -101,10 +101,10 @@ internal class DefaultExperimentClient internal constructor( val event = OldExposureEvent(exposedUser, key, variant, source) // Track the exposure event if an analytics provider is set if (source.isFallback() || variant.value == null) { - userSessionExposureTracker?.track(Exposure(key, null), exposedUser) + userSessionExposureTracker?.track(Exposure(key, null, variant.expKey), exposedUser) analyticsProvider?.unsetUserProperty(event) } else { - userSessionExposureTracker?.track(Exposure(key, variant.value), exposedUser) + userSessionExposureTracker?.track(Exposure(key, variant.value, variant.expKey), exposedUser) analyticsProvider?.setUserProperty(event) analyticsProvider?.track(event) } diff --git a/sdk/src/main/java/com/amplitude/experiment/Experiment.kt b/sdk/src/main/java/com/amplitude/experiment/Experiment.kt index 84421dc..3163347 100644 --- a/sdk/src/main/java/com/amplitude/experiment/Experiment.kt +++ b/sdk/src/main/java/com/amplitude/experiment/Experiment.kt @@ -28,7 +28,8 @@ object Experiment { * instance name. * * @param application The Android Application context - * @param apiKey The API key. This can be found in the Experiment settings and should not be null or empty. + * @param apiKey The API key. This can be found in the Experiment settings + * and should not be null or empty. * @param config see [ExperimentConfig] for configuration options */ @JvmStatic @@ -67,11 +68,12 @@ object Experiment { * integrates with the installed and initialized instance of the amplitude * analytics SDK. * - * You must be using Amplitude-Android SDK version 2.36.0+ for this - * integration to work. + * You must be using Amplitude-Android SDK version 2.36.0+ or + * Amplitude-Kotlin 1.5.0+ for this integration to work. * * @param application The Android Application context - * @param apiKey The API key. This can be found in the Experiment settings and should not be null or empty. + * @param apiKey The API key. This can be found in the Experiment settings + * and should not be null or empty. * @param config see [ExperimentConfig] for configuration options */ @JvmStatic diff --git a/sdk/src/main/java/com/amplitude/experiment/Exposure.kt b/sdk/src/main/java/com/amplitude/experiment/Exposure.kt index d9ebf87..0e8ee90 100644 --- a/sdk/src/main/java/com/amplitude/experiment/Exposure.kt +++ b/sdk/src/main/java/com/amplitude/experiment/Exposure.kt @@ -33,4 +33,5 @@ package com.amplitude.experiment data class Exposure internal constructor( val flagKey: String, val variant: String?, + val experimentKey: String?, ) diff --git a/sdk/src/main/java/com/amplitude/experiment/ExposureTrackingProvider.kt b/sdk/src/main/java/com/amplitude/experiment/ExposureTrackingProvider.kt index eefb1fa..54649d2 100644 --- a/sdk/src/main/java/com/amplitude/experiment/ExposureTrackingProvider.kt +++ b/sdk/src/main/java/com/amplitude/experiment/ExposureTrackingProvider.kt @@ -49,7 +49,8 @@ interface ExposureTrackingProvider { * "event_type": "$exposure", * "event_properties": { * "flag_key": "", - * "variant": "" + * "variant": "", + * "experiment_key": "", * } * } * ``` @@ -62,6 +63,7 @@ interface ExposureTrackingProvider { * new Properties() * .putValue("flag_key", exposureEvent.flagKey) * .putValue("variant", exposureEvent.variant) + * .putValue("experiment_key", exposureEvent.experimentKey) * ); * ``` */ diff --git a/sdk/src/main/java/com/amplitude/experiment/Variant.kt b/sdk/src/main/java/com/amplitude/experiment/Variant.kt index 90bf74c..124db0b 100644 --- a/sdk/src/main/java/com/amplitude/experiment/Variant.kt +++ b/sdk/src/main/java/com/amplitude/experiment/Variant.kt @@ -1,8 +1,18 @@ package com.amplitude.experiment data class Variant @JvmOverloads constructor( + /** + * The value of the variant. + */ @JvmField val value: String? = null, + /** + * The attached payload, if any. + */ @JvmField val payload: Any? = null, + /** + * The experiment key. Used to distinguish two experiments associated with the same flag. + */ + @JvmField val expKey: String? = null, ) { /** diff --git a/sdk/src/main/java/com/amplitude/experiment/util/Variant.kt b/sdk/src/main/java/com/amplitude/experiment/util/Variant.kt index 24ee66d..ab7ecd9 100644 --- a/sdk/src/main/java/com/amplitude/experiment/util/Variant.kt +++ b/sdk/src/main/java/com/amplitude/experiment/util/Variant.kt @@ -11,6 +11,9 @@ internal fun Variant.toJson(): String { if (payload != null) { jsonObject.put("payload", payload) } + if (expKey != null) { + jsonObject.put("expKey", expKey) + } } catch (e: JSONException) { Logger.w("Error converting Variant to json string", e) } @@ -38,7 +41,11 @@ internal fun JSONObject?.toVariant(): Variant? { has("payload") -> get("payload") else -> null } - Variant(value, payload) + val expKey = when { + has("expKey") -> getString("expKey") + else -> null + } + Variant(value, payload, expKey) } catch (e: JSONException) { Logger.w("Error parsing Variant from json string $this") return null diff --git a/sdk/src/test/java/com/amplitude/experiment/ConnectorExposureTrackingProviderTest.kt b/sdk/src/test/java/com/amplitude/experiment/ConnectorExposureTrackingProviderTest.kt index 2ead522..ae62ba1 100644 --- a/sdk/src/test/java/com/amplitude/experiment/ConnectorExposureTrackingProviderTest.kt +++ b/sdk/src/test/java/com/amplitude/experiment/ConnectorExposureTrackingProviderTest.kt @@ -26,7 +26,7 @@ class ConnectorExposureTrackingProviderTest { // Track event with variant - val exposureEvent1 = Exposure("test-key-1", "test") + val exposureEvent1 = Exposure("test-key-1", "test", null) val expectedTrack1 = AnalyticsEvent("\$exposure", mapOf("flag_key" to "test-key-1", "variant" to "test")) connectorExposureTrackingProvider.track(exposureEvent1) @@ -41,7 +41,7 @@ class ConnectorExposureTrackingProviderTest { // Track new flag key event with same variant - val exposureEvent2 = Exposure("test-key-2", "test") + val exposureEvent2 = Exposure("test-key-2", "test", null) val expectedTrack2 = AnalyticsEvent("\$exposure", mapOf("flag_key" to "test-key-2", "variant" to "test")) eventBridge.recentEvent = null @@ -63,7 +63,7 @@ class ConnectorExposureTrackingProviderTest { // Track event with variant - val exposureEvent1 = Exposure("test-key", "test") + val exposureEvent1 = Exposure("test-key", "test", null) val expectedTrack1 = AnalyticsEvent("\$exposure", mapOf("flag_key" to "test-key", "variant" to "test")) connectorExposureTrackingProvider.track(exposureEvent1) @@ -78,7 +78,7 @@ class ConnectorExposureTrackingProviderTest { // Track same flag key event with new variant - val exposureEvent2 = Exposure("test-key", "test2") + val exposureEvent2 = Exposure("test-key", "test2", null) val expectedTrack2 = AnalyticsEvent("\$exposure", mapOf("flag_key" to "test-key", "variant" to "test2")) eventBridge.recentEvent = null @@ -94,7 +94,7 @@ class ConnectorExposureTrackingProviderTest { // Track event with no variant - val exposureEvent3 = Exposure("test-key", null) + val exposureEvent3 = Exposure("test-key", null, null) val expectedTrack3 = AnalyticsEvent("\$exposure", mapOf("flag_key" to "test-key")) eventBridge.recentEvent = null diff --git a/sdk/src/test/java/com/amplitude/experiment/ExperimentClientTest.kt b/sdk/src/test/java/com/amplitude/experiment/ExperimentClientTest.kt index 3154961..571faf5 100644 --- a/sdk/src/test/java/com/amplitude/experiment/ExperimentClientTest.kt +++ b/sdk/src/test/java/com/amplitude/experiment/ExperimentClientTest.kt @@ -380,4 +380,31 @@ class ExperimentClientTest { Assert.assertTrue(didExposureGetTracked) Assert.assertTrue(didUserPropertyGetSet) } + + @Test + fun `test exposure through exposure tracking provider has experiment key from variant`() { + var didTrack = false + val exposureTrackingProvider = object : ExposureTrackingProvider { + override fun track(exposure: Exposure) { + Assert.assertEquals("flagKey", exposure.flagKey) + Assert.assertEquals("variant", exposure.variant) + Assert.assertEquals("experimentKey", exposure.experimentKey) + didTrack = true + } + } + val client = DefaultExperimentClient( + API_KEY, + ExperimentConfig( + debug = true, + exposureTrackingProvider = exposureTrackingProvider, + source = Source.INITIAL_VARIANTS, + initialVariants = mapOf("flagKey" to Variant("variant", null, "experimentKey")) + ), + OkHttpClient(), + InMemoryStorage(), + Experiment.executorService, + ) + client.variant("flagKey") + Assert.assertTrue(didTrack) + } } diff --git a/sdk/src/test/java/com/amplitude/experiment/UserSessionExposureTrackerTest.kt b/sdk/src/test/java/com/amplitude/experiment/UserSessionExposureTrackerTest.kt index 1ba2586..c82e964 100644 --- a/sdk/src/test/java/com/amplitude/experiment/UserSessionExposureTrackerTest.kt +++ b/sdk/src/test/java/com/amplitude/experiment/UserSessionExposureTrackerTest.kt @@ -12,14 +12,14 @@ class UserSessionExposureTrackerTest { val provider = TestExposureTrackingProvider() val tracker = UserSessionExposureTracker(provider) - val exposure = Exposure("flag", "variant") + val exposure = Exposure("flag", "variant", null) repeat(10) { tracker.track(exposure) } Assert.assertEquals(exposure, provider.lastExposure) Assert.assertEquals(1, provider.trackCount) - val exposure2 = Exposure("flag2", "variant") + val exposure2 = Exposure("flag2", "variant", null) repeat(10) { tracker.track(exposure2) } @@ -32,11 +32,11 @@ class UserSessionExposureTrackerTest { val provider = TestExposureTrackingProvider() val tracker = UserSessionExposureTracker(provider) - val exposure = Exposure("flag", "variant") + val exposure = Exposure("flag", "variant", null) repeat(10) { tracker.track(exposure) } - val exposure2 = Exposure("flag", null) + val exposure2 = Exposure("flag", null, null) repeat(10) { tracker.track(exposure2) } @@ -50,11 +50,11 @@ class UserSessionExposureTrackerTest { val provider = TestExposureTrackingProvider() val tracker = UserSessionExposureTracker(provider) - val exposure = Exposure("flag", "variant") + val exposure = Exposure("flag", "variant", null) repeat(10) { tracker.track(exposure) } - val exposure2 = Exposure("flag", "variant2") + val exposure2 = Exposure("flag", "variant2", null) repeat(10) { tracker.track(exposure2) } @@ -67,7 +67,7 @@ class UserSessionExposureTrackerTest { fun `test track called again on user id change, null to value`() { val provider = TestExposureTrackingProvider() val tracker = UserSessionExposureTracker(provider) - val exposure = Exposure("flag", "variant") + val exposure = Exposure("flag", "variant", null) repeat(10) { tracker.track(exposure) } @@ -82,7 +82,7 @@ class UserSessionExposureTrackerTest { fun `test track called again on device id change, null to value`() { val provider = TestExposureTrackingProvider() val tracker = UserSessionExposureTracker(provider) - val exposure = Exposure("flag", "variant") + val exposure = Exposure("flag", "variant", null) repeat(10) { tracker.track(exposure) } @@ -97,7 +97,7 @@ class UserSessionExposureTrackerTest { fun `test track called again on user id change, value to null`() { val provider = TestExposureTrackingProvider() val tracker = UserSessionExposureTracker(provider) - val exposure = Exposure("flag", "variant") + val exposure = Exposure("flag", "variant", null) repeat(10) { tracker.track(exposure, ExperimentUser(userId = "uid")) } @@ -112,7 +112,7 @@ class UserSessionExposureTrackerTest { fun `test track called again on device id change, value to null`() { val provider = TestExposureTrackingProvider() val tracker = UserSessionExposureTracker(provider) - val exposure = Exposure("flag", "variant") + val exposure = Exposure("flag", "variant", null) repeat(10) { tracker.track(exposure, ExperimentUser(deviceId = "did")) } @@ -127,7 +127,7 @@ class UserSessionExposureTrackerTest { fun `test track called again on user id change, value to different value`() { val provider = TestExposureTrackingProvider() val tracker = UserSessionExposureTracker(provider) - val exposure = Exposure("flag", "variant") + val exposure = Exposure("flag", "variant", null) repeat(10) { tracker.track(exposure, ExperimentUser(userId = "uid")) } @@ -142,7 +142,7 @@ class UserSessionExposureTrackerTest { fun `test track called again on device id change, value to different value`() { val provider = TestExposureTrackingProvider() val tracker = UserSessionExposureTracker(provider) - val exposure = Exposure("flag", "variant") + val exposure = Exposure("flag", "variant", null) repeat(10) { tracker.track(exposure, ExperimentUser(deviceId = "did")) } @@ -157,7 +157,7 @@ class UserSessionExposureTrackerTest { fun `test track called again on user id and device id change, null to value`() { val provider = TestExposureTrackingProvider() val tracker = UserSessionExposureTracker(provider) - val exposure = Exposure("flag", "variant") + val exposure = Exposure("flag", "variant", null) repeat(10) { tracker.track(exposure) } @@ -172,7 +172,7 @@ class UserSessionExposureTrackerTest { fun `test track called again on user id and device id change, value to null`() { val provider = TestExposureTrackingProvider() val tracker = UserSessionExposureTracker(provider) - val exposure = Exposure("flag", "variant") + val exposure = Exposure("flag", "variant", null) repeat(10) { tracker.track(exposure, ExperimentUser(userId = "uid", deviceId = "did")) } @@ -187,7 +187,7 @@ class UserSessionExposureTrackerTest { fun `test track called again on user id and device id change, value to different value`() { val provider = TestExposureTrackingProvider() val tracker = UserSessionExposureTracker(provider) - val exposure = Exposure("flag", "variant") + val exposure = Exposure("flag", "variant", null) repeat(10) { tracker.track(exposure, ExperimentUser(userId = "uid", deviceId = "did")) } diff --git a/sdk/src/test/java/com/amplitude/experiment/VariantTest.kt b/sdk/src/test/java/com/amplitude/experiment/VariantTest.kt index cf945d7..f6efcc4 100644 --- a/sdk/src/test/java/com/amplitude/experiment/VariantTest.kt +++ b/sdk/src/test/java/com/amplitude/experiment/VariantTest.kt @@ -26,10 +26,12 @@ class VariantTest { val jsonObject = JSONObject() jsonObject.put("value", "value") jsonObject.put("payload", "payload") + jsonObject.put("expKey", "expKey") val variant = jsonObject.toVariant() Assert.assertNotNull(variant) Assert.assertEquals("value", variant!!.value) Assert.assertEquals("payload", variant.payload) + Assert.assertEquals("expKey", variant.expKey) } @Test @@ -45,9 +47,10 @@ class VariantTest { @Test fun `variant to json object`() { run { - val variant = Variant("value", null) + val variant = Variant("value", null, "expKey") val jsonObject = JSONObject() jsonObject.put("value", "value") + jsonObject.put("expKey", "expKey") Assert.assertEquals(jsonObject.toString(), variant.toJson()) } }