Skip to content

Commit

Permalink
Store full enrollment date on a/b/n and truncate before sending pixels
Browse files Browse the repository at this point in the history
  • Loading branch information
marcosholgado committed Jan 24, 2025
1 parent 00dee76 commit 109d4af
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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(),
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -45,7 +46,7 @@ data class MetricsPixel(
fun getPixelDefinitions(): List<PixelDefinition> {
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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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("", "")
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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(
Expand All @@ -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(
Expand Down

0 comments on commit 109d4af

Please sign in to comment.