From 109d4afbf73ffa46c87e618a27f1016445346714 Mon Sep 17 00:00:00 2001 From: Marcos Date: Fri, 24 Jan 2025 16:43:55 +0000 Subject: [PATCH] Store full enrollment date on a/b/n and truncate before sending pixels --- .../feature/toggles/api/FeatureToggles.kt | 3 +-- .../feature/toggles/api/MetricsPixelPlugin.kt | 3 ++- .../impl/RealFeatureTogglesCallback.kt | 3 ++- .../RetentionMetricsAtbLifecyclePlugin.kt | 15 +++++++-------- .../impl/MetricPixelInterceptorTest.kt | 19 +++++++++---------- .../impl/RealFeatureTogglesCallbackTest.kt | 2 +- .../RetentionMetricsAtbLifecyclePluginTest.kt | 17 ++++++++--------- 7 files changed, 30 insertions(+), 32 deletions(-) diff --git a/feature-toggles/feature-toggles-api/src/main/java/com/duckduckgo/feature/toggles/api/FeatureToggles.kt b/feature-toggles/feature-toggles-api/src/main/java/com/duckduckgo/feature/toggles/api/FeatureToggles.kt index 70d7fb646269..95af5c4704a0 100644 --- a/feature-toggles/feature-toggles-api/src/main/java/com/duckduckgo/feature/toggles/api/FeatureToggles.kt +++ b/feature-toggles/feature-toggles-api/src/main/java/com/duckduckgo/feature/toggles/api/FeatureToggles.kt @@ -28,7 +28,6 @@ import java.lang.reflect.Method import java.lang.reflect.Proxy import java.time.ZoneId import java.time.ZonedDateTime -import java.time.temporal.ChronoUnit import kotlin.random.Random import org.apache.commons.math3.distribution.EnumeratedIntegerDistribution @@ -506,7 +505,7 @@ internal class ToggleImpl constructor( } return getRandomCohort(cohorts)?.copy( - enrollmentDateET = ZonedDateTime.now(ZoneId.of("America/New_York")).truncatedTo(ChronoUnit.DAYS).toString(), + enrollmentDateET = ZonedDateTime.now(ZoneId.of("America/New_York")).toString(), ) } diff --git a/feature-toggles/feature-toggles-api/src/main/java/com/duckduckgo/feature/toggles/api/MetricsPixelPlugin.kt b/feature-toggles/feature-toggles-api/src/main/java/com/duckduckgo/feature/toggles/api/MetricsPixelPlugin.kt index a88279b80f64..37b3fc23c4f0 100644 --- a/feature-toggles/feature-toggles-api/src/main/java/com/duckduckgo/feature/toggles/api/MetricsPixelPlugin.kt +++ b/feature-toggles/feature-toggles-api/src/main/java/com/duckduckgo/feature/toggles/api/MetricsPixelPlugin.kt @@ -18,6 +18,7 @@ package com.duckduckgo.feature.toggles.api import java.time.ZonedDateTime import java.time.format.DateTimeFormatter +import java.time.temporal.ChronoUnit /** * Experiment pixels that want to be fired should implement this plugin. The associated plugin point @@ -45,7 +46,7 @@ data class MetricsPixel( fun getPixelDefinitions(): List { val cohort = toggle.getRawStoredState()?.assignedCohort?.name.orEmpty() val enrollmentDateET = toggle.getRawStoredState()?.assignedCohort?.enrollmentDateET?.let { - ZonedDateTime.parse(it).format(DateTimeFormatter.ISO_LOCAL_DATE) + ZonedDateTime.parse(it).truncatedTo(ChronoUnit.DAYS).format(DateTimeFormatter.ISO_LOCAL_DATE) }.orEmpty() if (cohort.isEmpty() || enrollmentDateET.isEmpty()) { return emptyList() diff --git a/feature-toggles/feature-toggles-impl/src/main/java/com/duckduckgo/feature/toggles/impl/RealFeatureTogglesCallback.kt b/feature-toggles/feature-toggles-impl/src/main/java/com/duckduckgo/feature/toggles/impl/RealFeatureTogglesCallback.kt index c26b89e38424..31c3b0db843d 100644 --- a/feature-toggles/feature-toggles-impl/src/main/java/com/duckduckgo/feature/toggles/impl/RealFeatureTogglesCallback.kt +++ b/feature-toggles/feature-toggles-impl/src/main/java/com/duckduckgo/feature/toggles/impl/RealFeatureTogglesCallback.kt @@ -29,6 +29,7 @@ import com.squareup.anvil.annotations.ContributesBinding import com.squareup.anvil.annotations.ContributesMultibinding import java.time.ZonedDateTime import java.time.format.DateTimeFormatter +import java.time.temporal.ChronoUnit import javax.inject.Inject import okio.ByteString.Companion.encode @@ -43,7 +44,7 @@ class RealFeatureTogglesCallback @Inject constructor( cohortName: String, enrollmentDate: String, ) { - val parsedDate = ZonedDateTime.parse(enrollmentDate).format(DateTimeFormatter.ISO_LOCAL_DATE) + val parsedDate = ZonedDateTime.parse(enrollmentDate).truncatedTo(ChronoUnit.DAYS).format(DateTimeFormatter.ISO_LOCAL_DATE) val params = mapOf("enrollmentDate" to parsedDate) val pixelName = getPixelName(experimentName, cohortName) val tag = "${pixelName}_$params".encode().md5().hex() diff --git a/feature-toggles/feature-toggles-impl/src/main/java/com/duckduckgo/feature/toggles/impl/metrics/RetentionMetricsAtbLifecyclePlugin.kt b/feature-toggles/feature-toggles-impl/src/main/java/com/duckduckgo/feature/toggles/impl/metrics/RetentionMetricsAtbLifecyclePlugin.kt index 9f5b9eeecb65..503684b5a750 100644 --- a/feature-toggles/feature-toggles-impl/src/main/java/com/duckduckgo/feature/toggles/impl/metrics/RetentionMetricsAtbLifecyclePlugin.kt +++ b/feature-toggles/feature-toggles-impl/src/main/java/com/duckduckgo/feature/toggles/impl/metrics/RetentionMetricsAtbLifecyclePlugin.kt @@ -20,12 +20,12 @@ import com.duckduckgo.app.di.AppCoroutineScope import com.duckduckgo.app.statistics.api.AtbLifecyclePlugin import com.duckduckgo.app.statistics.pixels.Pixel import com.duckduckgo.di.scopes.AppScope +import com.duckduckgo.feature.toggles.api.MetricsPixel import com.duckduckgo.feature.toggles.api.PixelDefinition import com.duckduckgo.feature.toggles.impl.MetricsPixelStore import com.duckduckgo.feature.toggles.impl.RetentionMetric.APP_USE import com.duckduckgo.feature.toggles.impl.RetentionMetric.SEARCH import com.squareup.anvil.annotations.ContributesMultibinding -import java.time.LocalDate import java.time.ZoneId import java.time.ZonedDateTime import java.time.temporal.ChronoUnit @@ -46,7 +46,7 @@ class RetentionMetricsAtbLifecyclePlugin @Inject constructor( appCoroutineScope.launch { searchMetricPixelsPlugin.getMetrics().forEach { metric -> metric.getPixelDefinitions().forEach { definition -> - if (isInConversionWindow(definition)) { + if (isInConversionWindow(definition, metric)) { store.getMetricForPixelDefinition(definition, SEARCH).takeIf { it < metric.value.toInt() }?.let { store.increaseMetricForPixelDefinition(definition, SEARCH).takeIf { it == metric.value.toInt() }?.apply { pixel.fire(definition.pixelName, definition.params) @@ -62,7 +62,7 @@ class RetentionMetricsAtbLifecyclePlugin @Inject constructor( appCoroutineScope.launch { appUseMetricPixelsPlugin.getMetrics().forEach { metric -> metric.getPixelDefinitions().forEach { definition -> - if (isInConversionWindow(definition)) { + if (isInConversionWindow(definition, metric)) { store.getMetricForPixelDefinition(definition, APP_USE).takeIf { it < metric.value.toInt() }?.let { store.increaseMetricForPixelDefinition(definition, APP_USE).takeIf { it == metric.value.toInt() }?.apply { pixel.fire(definition.pixelName, definition.params) @@ -74,8 +74,8 @@ class RetentionMetricsAtbLifecyclePlugin @Inject constructor( } } - private fun isInConversionWindow(definition: PixelDefinition): Boolean { - val enrollmentDate = definition.params["enrollmentDate"] ?: return false + private fun isInConversionWindow(definition: PixelDefinition, metric: MetricsPixel): Boolean { + val enrollmentDate = metric.toggle.getCohort()?.enrollmentDateET ?: return false val lowerWindow = definition.params["conversionWindowDays"]?.split("-")?.first()?.toInt() ?: return false val upperWindow = definition.params["conversionWindowDays"]?.split("-")?.last()?.toInt() ?: return false val daysDiff = daysBetweenTodayAnd(enrollmentDate) @@ -85,8 +85,7 @@ class RetentionMetricsAtbLifecyclePlugin @Inject constructor( private fun daysBetweenTodayAnd(date: String): Long { val today = ZonedDateTime.now(ZoneId.of("America/New_York")) - val localDate = LocalDate.parse(date) - val zoneDateTime: ZonedDateTime = localDate.atStartOfDay(ZoneId.of("America/New_York")) - return ChronoUnit.DAYS.between(zoneDateTime, today) + val localDate = ZonedDateTime.parse(date) + return ChronoUnit.DAYS.between(localDate, today) } } diff --git a/feature-toggles/feature-toggles-impl/src/test/java/com/duckduckgo/feature/toggles/impl/MetricPixelInterceptorTest.kt b/feature-toggles/feature-toggles-impl/src/test/java/com/duckduckgo/feature/toggles/impl/MetricPixelInterceptorTest.kt index f655063d4505..87273bf20701 100644 --- a/feature-toggles/feature-toggles-impl/src/test/java/com/duckduckgo/feature/toggles/impl/MetricPixelInterceptorTest.kt +++ b/feature-toggles/feature-toggles-impl/src/test/java/com/duckduckgo/feature/toggles/impl/MetricPixelInterceptorTest.kt @@ -15,7 +15,6 @@ import com.duckduckgo.feature.toggles.codegen.TestTriggerFeature import java.time.ZoneId import java.time.ZonedDateTime import java.time.format.DateTimeFormatter -import java.time.temporal.ChronoUnit import org.junit.Assert.* import org.junit.Before import org.junit.Rule @@ -43,7 +42,7 @@ class MetricPixelInterceptorTest { @Test fun `test metric exist but is not in conversion window drops pixel`() { - val enrollmentDateET = ZonedDateTime.now(ZoneId.of("America/New_York")).truncatedTo(ChronoUnit.DAYS).toString() + val enrollmentDateET = ZonedDateTime.now(ZoneId.of("America/New_York")).toString() val enrollmentDateParsedET: String = ZonedDateTime.parse(enrollmentDateET).format(DateTimeFormatter.ISO_LOCAL_DATE).toString() testFeature.experimentFooFeature().setRawStoredState( @@ -69,7 +68,7 @@ class MetricPixelInterceptorTest { @Test fun `test metric exist and is in conversion window sends pixel`() { - val enrollmentDateET = ZonedDateTime.now(ZoneId.of("America/New_York")).truncatedTo(ChronoUnit.DAYS).toString() + val enrollmentDateET = ZonedDateTime.now(ZoneId.of("America/New_York")).toString() val enrollmentDateParsedET: String = ZonedDateTime.parse(enrollmentDateET).format(DateTimeFormatter.ISO_LOCAL_DATE).toString() testFeature.experimentFooFeature().setRawStoredState( @@ -95,7 +94,7 @@ class MetricPixelInterceptorTest { @Test fun `test metric allows conversion window of one day`() { - val enrollmentDateET = ZonedDateTime.now(ZoneId.of("America/New_York")).truncatedTo(ChronoUnit.DAYS).toString() + val enrollmentDateET = ZonedDateTime.now(ZoneId.of("America/New_York")).toString() val enrollmentDateParsedET: String = ZonedDateTime.parse(enrollmentDateET).format(DateTimeFormatter.ISO_LOCAL_DATE).toString() testFeature.experimentFooFeature().setRawStoredState( @@ -121,7 +120,7 @@ class MetricPixelInterceptorTest { @Test fun `test metric exist and is in conversion window from previous days sends pixel`() { - val enrollmentDateET = ZonedDateTime.now(ZoneId.of("America/New_York")).minusDays(4).truncatedTo(ChronoUnit.DAYS).toString() + val enrollmentDateET = ZonedDateTime.now(ZoneId.of("America/New_York")).minusDays(4).toString() val enrollmentDateParsedET: String = ZonedDateTime.parse(enrollmentDateET).format(DateTimeFormatter.ISO_LOCAL_DATE).toString() testFeature.experimentFooFeature().setRawStoredState( @@ -147,7 +146,7 @@ class MetricPixelInterceptorTest { @Test fun `test pixel metric cannot be sent twice`() { - val enrollmentDateET = ZonedDateTime.now(ZoneId.of("America/New_York")).truncatedTo(ChronoUnit.DAYS).toString() + val enrollmentDateET = ZonedDateTime.now(ZoneId.of("America/New_York")).toString() val enrollmentDateParsedET: String = ZonedDateTime.parse(enrollmentDateET).format(DateTimeFormatter.ISO_LOCAL_DATE).toString() testFeature.experimentFooFeature().setRawStoredState( @@ -177,7 +176,7 @@ class MetricPixelInterceptorTest { @Test fun `test metric doesnt exist drops pixels`() { - val enrollmentDateET = ZonedDateTime.now(ZoneId.of("America/New_York")).truncatedTo(ChronoUnit.DAYS).toString() + val enrollmentDateET = ZonedDateTime.now(ZoneId.of("America/New_York")).toString() val enrollmentDateParsedET: String = ZonedDateTime.parse(enrollmentDateET).format(DateTimeFormatter.ISO_LOCAL_DATE).toString() testFeature.experimentFooFeature().setRawStoredState( @@ -203,7 +202,7 @@ class MetricPixelInterceptorTest { @Test fun `test value doesnt exist drops pixels`() { - val enrollmentDateET = ZonedDateTime.now(ZoneId.of("America/New_York")).truncatedTo(ChronoUnit.DAYS).toString() + val enrollmentDateET = ZonedDateTime.now(ZoneId.of("America/New_York")).toString() val enrollmentDateParsedET: String = ZonedDateTime.parse(enrollmentDateET).format(DateTimeFormatter.ISO_LOCAL_DATE).toString() testFeature.experimentFooFeature().setRawStoredState( @@ -229,7 +228,7 @@ class MetricPixelInterceptorTest { @Test fun `test cohort name doesnt exist drops pixels`() { - val enrollmentDateET = ZonedDateTime.now(ZoneId.of("America/New_York")).truncatedTo(ChronoUnit.DAYS).toString() + val enrollmentDateET = ZonedDateTime.now(ZoneId.of("America/New_York")).toString() val enrollmentDateParsedET: String = ZonedDateTime.parse(enrollmentDateET).format(DateTimeFormatter.ISO_LOCAL_DATE).toString() testFeature.experimentFooFeature().setRawStoredState( @@ -255,7 +254,7 @@ class MetricPixelInterceptorTest { @Test fun `test experiment name doesnt exist drops pixels`() { - val enrollmentDateET = ZonedDateTime.now(ZoneId.of("America/New_York")).truncatedTo(ChronoUnit.DAYS).toString() + val enrollmentDateET = ZonedDateTime.now(ZoneId.of("America/New_York")).toString() val enrollmentDateParsedET: String = ZonedDateTime.parse(enrollmentDateET).format(DateTimeFormatter.ISO_LOCAL_DATE).toString() testFeature.experimentFooFeature().setRawStoredState( diff --git a/feature-toggles/feature-toggles-impl/src/test/java/com/duckduckgo/feature/toggles/impl/RealFeatureTogglesCallbackTest.kt b/feature-toggles/feature-toggles-impl/src/test/java/com/duckduckgo/feature/toggles/impl/RealFeatureTogglesCallbackTest.kt index 8c00615f3ab4..b0e094a8bdba 100644 --- a/feature-toggles/feature-toggles-impl/src/test/java/com/duckduckgo/feature/toggles/impl/RealFeatureTogglesCallbackTest.kt +++ b/feature-toggles/feature-toggles-impl/src/test/java/com/duckduckgo/feature/toggles/impl/RealFeatureTogglesCallbackTest.kt @@ -26,7 +26,7 @@ class RealFeatureTogglesCallbackTest { callback.onCohortAssigned( experimentName = "experimentName", cohortName = "cohortName", - enrollmentDate = "2024-10-15T00:00-04:00[America/New_York]", + enrollmentDate = "2024-10-15T08:50:17.467-05:00[America/New_York]", ) verify(pixel).fire(pixelName = pixelName, parameters = params, type = Unique(tag)) } diff --git a/feature-toggles/feature-toggles-impl/src/test/java/com/duckduckgo/feature/toggles/impl/metrics/RetentionMetricsAtbLifecyclePluginTest.kt b/feature-toggles/feature-toggles-impl/src/test/java/com/duckduckgo/feature/toggles/impl/metrics/RetentionMetricsAtbLifecyclePluginTest.kt index 4b3fc51e46cd..fe00dab9889d 100644 --- a/feature-toggles/feature-toggles-impl/src/test/java/com/duckduckgo/feature/toggles/impl/metrics/RetentionMetricsAtbLifecyclePluginTest.kt +++ b/feature-toggles/feature-toggles-impl/src/test/java/com/duckduckgo/feature/toggles/impl/metrics/RetentionMetricsAtbLifecyclePluginTest.kt @@ -33,7 +33,6 @@ import com.duckduckgo.feature.toggles.impl.RetentionMetric import java.time.ZoneId import java.time.ZonedDateTime import java.time.format.DateTimeFormatter -import java.time.temporal.ChronoUnit import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse @@ -90,7 +89,7 @@ class RetentionMetricsAtbLifecyclePluginTest { @Test fun `when search atb refreshed and matches metric and conversion window, pixel sent to all active experiments`() = runTest { - val today = ZonedDateTime.now(ZoneId.of("America/New_York")).truncatedTo(ChronoUnit.DAYS).toString() + val today = ZonedDateTime.now(ZoneId.of("America/New_York")).toString() val parsedDate: String = ZonedDateTime.parse(today).format(DateTimeFormatter.ISO_LOCAL_DATE).toString() setCohorts(today) @@ -107,7 +106,7 @@ class RetentionMetricsAtbLifecyclePluginTest { @Test fun `when app use atb refreshed and no metric match conversion window, do not send pixels`() = runTest { - val today = ZonedDateTime.now(ZoneId.of("America/New_York")).truncatedTo(ChronoUnit.DAYS).toString() + val today = ZonedDateTime.now(ZoneId.of("America/New_York")).toString() setCohorts(today) atbLifecyclePlugin.onAppRetentionAtbRefreshed("", "") @@ -116,7 +115,7 @@ class RetentionMetricsAtbLifecyclePluginTest { @Test fun `when search atb refreshed, fire all pixels which metric matches the number of searches done and conversion window`() = runTest { - val today = ZonedDateTime.now(ZoneId.of("America/New_York")).minusDays(6).truncatedTo(ChronoUnit.DAYS).toString() + val today = ZonedDateTime.now(ZoneId.of("America/New_York")).minusDays(6).toString() val parsedDate: String = ZonedDateTime.parse(today).format(DateTimeFormatter.ISO_LOCAL_DATE).toString() setCohorts(today) @@ -140,7 +139,7 @@ class RetentionMetricsAtbLifecyclePluginTest { @Test fun `when app use atb refreshed, fire all pixels which metric matches the number of app use and conversion window`() = runTest { - val today = ZonedDateTime.now(ZoneId.of("America/New_York")).minusDays(6).truncatedTo(ChronoUnit.DAYS).toString() + val today = ZonedDateTime.now(ZoneId.of("America/New_York")).minusDays(6).toString() val parsedDate: String = ZonedDateTime.parse(today).format(DateTimeFormatter.ISO_LOCAL_DATE).toString() setCohorts(today) @@ -164,7 +163,7 @@ class RetentionMetricsAtbLifecyclePluginTest { @Test fun `when search atb refreshed, only fire pixels with active experiments`() = runTest { - val today = ZonedDateTime.now(ZoneId.of("America/New_York")).truncatedTo(ChronoUnit.DAYS).toString() + val today = ZonedDateTime.now(ZoneId.of("America/New_York")).toString() testFeature.experimentFooFeature().setRawStoredState( State( remoteEnableState = false, @@ -188,7 +187,7 @@ class RetentionMetricsAtbLifecyclePluginTest { @Test fun `when search atb refreshed, only fire pixels from experiments with cohorts assigned`() = runTest { - val today = ZonedDateTime.now(ZoneId.of("America/New_York")).truncatedTo(ChronoUnit.DAYS).toString() + val today = ZonedDateTime.now(ZoneId.of("America/New_York")).toString() testFeature.experimentFooFeature().setRawStoredState( State( remoteEnableState = true, @@ -212,7 +211,7 @@ class RetentionMetricsAtbLifecyclePluginTest { @Test fun `when app use refreshed, only fire pixels with active experiments`() = runTest { - val today = ZonedDateTime.now(ZoneId.of("America/New_York")).minusDays(1).truncatedTo(ChronoUnit.DAYS).toString() + val today = ZonedDateTime.now(ZoneId.of("America/New_York")).minusDays(1).toString() testFeature.experimentFooFeature().setRawStoredState( State( @@ -237,7 +236,7 @@ class RetentionMetricsAtbLifecyclePluginTest { @Test fun `when app use refreshed, only fire pixels from experiments with cohorts assigned`() = runTest { - val today = ZonedDateTime.now(ZoneId.of("America/New_York")).minusDays(1).truncatedTo(ChronoUnit.DAYS).toString() + val today = ZonedDateTime.now(ZoneId.of("America/New_York")).minusDays(1).toString() testFeature.experimentFooFeature().setRawStoredState( State(