Skip to content

Update protos to include bloom filter #4564

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jan 17, 2023
Merged

Update protos to include bloom filter #4564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
df79e47
Report updated experiments if either experiment id or variant id chan…
danasilver Jan 14, 2023
5f94d87
Update protos
milaGGL Jan 16, 2023
a2cc36f
Merge branch 'master' into mila/BloomFilter-add-bloomFilter-to-proto
milaGGL Jan 16, 2023
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
2 changes: 2 additions & 0 deletions firebase-abt/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Unreleased

* [changed] Internal changes to improve experiment reporting.

# 21.1.0
* [changed] Internal changes to ensure functionality alignment with other
SDK releases.
Expand Down
63 changes: 35 additions & 28 deletions firebase-abt/src/main/java/com/google/firebase/abt/FirebaseABTesting.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,8 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.Deque;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

/**
* Manages Firebase A/B Testing Experiments.
Expand Down Expand Up @@ -144,11 +142,11 @@ public void removeAllExperiments() throws AbtException {
}

/**
* Gets the origin service's list of experiments in the app.
* Gets the origin service's list of experiments in the app via the Analytics SDK.
*
* <p>Note: This is a blocking call and therefore should be called from a worker thread.
*
* @return the origin service's list of experiments in the app.
* @return the origin service's list of experiments in the app as {@link AbtExperimentInfo}s.
* @throws AbtException If there is no Analytics SDK.
*/
@WorkerThread
Expand Down Expand Up @@ -204,12 +202,10 @@ public void reportActiveExperiment(AbtExperimentInfo activeExperiment) throws Ab
public void validateRunningExperiments(List<AbtExperimentInfo> runningExperiments)
throws AbtException {
throwAbtExceptionIfAnalyticsIsNull();
Set<String> runningExperimentIds = new HashSet<>();
for (AbtExperimentInfo runningExperiment : runningExperiments) {
runningExperimentIds.add(runningExperiment.getExperimentId());
}

// Get all experiments in Analytics and remove the ones that aren't running.
List<ConditionalUserProperty> experimentsToRemove =
getExperimentsToRemove(getAllExperimentsInAnalytics(), runningExperimentIds);
getExperimentsToRemove(getAllExperiments(), runningExperiments);
removeExperiments(experimentsToRemove);
}

Expand Down Expand Up @@ -245,34 +241,29 @@ private void replaceAllExperimentsWith(List<AbtExperimentInfo> replacementExperi
return;
}

Set<String> replacementExperimentIds = new HashSet<>();
for (AbtExperimentInfo replacementExperiment : replacementExperiments) {
replacementExperimentIds.add(replacementExperiment.getExperimentId());
}

List<ConditionalUserProperty> experimentsInAnalytics = getAllExperimentsInAnalytics();
Set<String> idsOfExperimentsInAnalytics = new HashSet<>();
for (ConditionalUserProperty experimentInAnalytics : experimentsInAnalytics) {
idsOfExperimentsInAnalytics.add(experimentInAnalytics.name);
}
// Get all experiments in Analytics.
List<AbtExperimentInfo> experimentsInAnalytics = getAllExperiments();

// Remove experiments no longer assigned.
List<ConditionalUserProperty> experimentsToRemove =
getExperimentsToRemove(experimentsInAnalytics, replacementExperimentIds);
getExperimentsToRemove(experimentsInAnalytics, replacementExperiments);
removeExperiments(experimentsToRemove);

// Add newly assigned or updated (changed variant id).
List<AbtExperimentInfo> experimentsToAdd =
getExperimentsToAdd(replacementExperiments, idsOfExperimentsInAnalytics);
getExperimentsToAdd(replacementExperiments, experimentsInAnalytics);
addExperiments(experimentsToAdd);
}

/** Returns this origin's experiments in Analytics that are no longer assigned to this App. */
private ArrayList<ConditionalUserProperty> getExperimentsToRemove(
List<ConditionalUserProperty> experimentsInAnalytics, Set<String> replacementExperimentIds) {
List<AbtExperimentInfo> experimentsInAnalytics,
List<AbtExperimentInfo> replacementExperiments) {

ArrayList<ConditionalUserProperty> experimentsToRemove = new ArrayList<>();
for (ConditionalUserProperty experimentInAnalytics : experimentsInAnalytics) {
if (!replacementExperimentIds.contains(experimentInAnalytics.name)) {
experimentsToRemove.add(experimentInAnalytics);
for (AbtExperimentInfo experimentInAnalytics : experimentsInAnalytics) {
if (!experimentsListContainsExperiment(replacementExperiments, experimentInAnalytics)) {
experimentsToRemove.add(experimentInAnalytics.toConditionalUserProperty(originService));
}
}
return experimentsToRemove;
Expand All @@ -283,17 +274,33 @@ private ArrayList<ConditionalUserProperty> getExperimentsToRemove(
* to this origin's list of experiments in Analytics.
*/
private ArrayList<AbtExperimentInfo> getExperimentsToAdd(
List<AbtExperimentInfo> replacementExperiments, Set<String> idsOfExperimentsInAnalytics) {
List<AbtExperimentInfo> replacementExperiments,
List<AbtExperimentInfo> experimentInfoFromAnalytics) {

ArrayList<AbtExperimentInfo> experimentsToAdd = new ArrayList<>();
for (AbtExperimentInfo replacementExperiment : replacementExperiments) {
if (!idsOfExperimentsInAnalytics.contains(replacementExperiment.getExperimentId())) {
if (!experimentsListContainsExperiment(experimentInfoFromAnalytics, replacementExperiment)) {
experimentsToAdd.add(replacementExperiment);
}
}
return experimentsToAdd;
}

private boolean experimentsListContainsExperiment(
List<AbtExperimentInfo> experiments, AbtExperimentInfo experiment) {
String experimentId = experiment.getExperimentId();
String variantId = experiment.getVariantId();

for (AbtExperimentInfo experimentInfo : experiments) {
if (experimentInfo.getExperimentId().equals(experimentId)
&& experimentInfo.getVariantId().equals(variantId)) {
return true;
}
}

return false;
}

/** Adds the given experiments to the origin's list in Analytics. */
private void addExperiments(List<AbtExperimentInfo> experimentsToAdd) {

Expand Down Expand Up @@ -369,7 +376,7 @@ private int getMaxUserPropertiesInAnalytics() {
* Returns a list of all this origin's experiments in this App's Analytics SDK.
*
* <p>The list is sorted chronologically by the experiment start time, with the oldest experiment
* at index 0.
* at index 0. Experiments are stored as {@link ConditionalUserProperty}s in Analytics.
*/
@WorkerThread
private List<ConditionalUserProperty> getAllExperimentsInAnalytics() {
Expand Down
98 changes: 79 additions & 19 deletions firebase-abt/src/test/java/com/google/firebase/abt/FirebaseABTestingTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,27 @@ public class FirebaseABTestingTest {
private static final String TEST_EXPERIMENT_1_ID = "1";
private static final String TEST_EXPERIMENT_2_ID = "2";

private static final String TEST_VARIANT_ID_A = "VARIANT_A";
private static final String TEST_VARIANT_ID_B = "VARIANT_B";

private static final AbtExperimentInfo TEST_ABT_EXPERIMENT_1 =
createExperimentInfo(
TEST_EXPERIMENT_1_ID,
TEST_VARIANT_ID_A,
/*triggerEventName=*/ "",
/*experimentStartTimeInEpochMillis=*/ 1000L);
private static final AbtExperimentInfo TEST_ABT_EXPERIMENT_2 =
private static final AbtExperimentInfo TEST_ABT_EXPERIMENT_2_VARIANT_A =
createExperimentInfo(
TEST_EXPERIMENT_2_ID,
TEST_VARIANT_ID_A,
"trigger_event_2",
/*experimentStartTimeInEpochMillis=*/ 2000L);
private static final AbtExperimentInfo TEST_ABT_EXPERIMENT_2_VARIANT_B =
createExperimentInfo(
TEST_EXPERIMENT_2_ID, "trigger_event_2", /*experimentStartTimeInEpochMillis=*/ 2000L);
TEST_EXPERIMENT_2_ID,
TEST_VARIANT_ID_B,
"trigger_event_2",
/*experimentStartTimeInEpochMillis=*/ 2000L);

private static final int MAX_ALLOWED_EXPERIMENTS_IN_ANALYTICS = 100;

Expand Down Expand Up @@ -91,7 +104,7 @@ public void replaceAllExperiments_noExperimentsInAnalytics_experimentsCorrectlyS

firebaseAbt.replaceAllExperiments(
Lists.newArrayList(
TEST_ABT_EXPERIMENT_1.toStringMap(), TEST_ABT_EXPERIMENT_2.toStringMap()));
TEST_ABT_EXPERIMENT_1.toStringMap(), TEST_ABT_EXPERIMENT_2_VARIANT_A.toStringMap()));

ArgumentCaptor<ConditionalUserProperty> analyticsExperimentArgumentCaptor =
ArgumentCaptor.forClass(ConditionalUserProperty.class);
Expand All @@ -107,7 +120,8 @@ public void replaceAllExperiments_noExperimentsInAnalytics_experimentsCorrectlyS

// Validates that TEST_ABT_EXPERIMENT_1 and TEST_ABT_EXPERIMENT_2 have been set in Analytics.
assertThat(analyticsExperiment1.toStringMap()).isEqualTo(TEST_ABT_EXPERIMENT_1.toStringMap());
assertThat(analyticsExperiment2.toStringMap()).isEqualTo(TEST_ABT_EXPERIMENT_2.toStringMap());
assertThat(analyticsExperiment2.toStringMap())
.isEqualTo(TEST_ABT_EXPERIMENT_2_VARIANT_A.toStringMap());
}

@Test
Expand All @@ -117,10 +131,11 @@ public void replaceAllExperiments_existExperimentsInAnalytics_experimentsCorrect
.thenReturn(
Lists.newArrayList(
TEST_ABT_EXPERIMENT_1.toConditionalUserProperty(ORIGIN_SERVICE),
TEST_ABT_EXPERIMENT_2.toConditionalUserProperty(ORIGIN_SERVICE)));
TEST_ABT_EXPERIMENT_2_VARIANT_A.toConditionalUserProperty(ORIGIN_SERVICE)));

AbtExperimentInfo newExperiment3 = createExperimentInfo("3", "", 1000L);
AbtExperimentInfo newExperiment4 = createExperimentInfo("4", "trigger_event_4", 1000L);
AbtExperimentInfo newExperiment3 = createExperimentInfo("3", TEST_VARIANT_ID_A, "", 1000L);
AbtExperimentInfo newExperiment4 =
createExperimentInfo("4", TEST_VARIANT_ID_A, "trigger_event_4", 1000L);

// Simulates the case where experiment 1 is assigned (as before), experiment 2 is no longer
// assigned; experiment 3 and experiment 4 are newly assigned.
Expand All @@ -146,6 +161,47 @@ public void replaceAllExperiments_existExperimentsInAnalytics_experimentsCorrect
.isEqualTo(newExperiment4.toStringMap());
}

@Test
public void
replaceAllExperiments_existExperimentsInAnalyticsWithDifferentVariants_experimentsCorrectlySetInAnalytics()
throws Exception {
when(mockAnalyticsConnector.getConditionalUserProperties(ORIGIN_SERVICE, ""))
.thenReturn(
Lists.newArrayList(
TEST_ABT_EXPERIMENT_1.toConditionalUserProperty(ORIGIN_SERVICE),
TEST_ABT_EXPERIMENT_2_VARIANT_A.toConditionalUserProperty(ORIGIN_SERVICE)));

AbtExperimentInfo newExperiment3 = createExperimentInfo("3", "b", "", 1000L);
AbtExperimentInfo newExperiment4 = createExperimentInfo("4", "a", "trigger_event_4", 1000L);

// Simulates the case where experiments 1 and 2 are removed,
// experiment 2 is re-set with a new variant, and experiments 3 and 4 are newly added.
firebaseAbt.replaceAllExperiments(
Lists.newArrayList(
TEST_ABT_EXPERIMENT_2_VARIANT_B.toStringMap(),
newExperiment3.toStringMap(),
newExperiment4.toStringMap()));

// Validates that experiment 1 is cleared, experiment 2 is updated,
// and experiment 3 and experiment 4 are set in Analytics.
ArgumentCaptor<ConditionalUserProperty> analyticsExperimentArgumentCaptor =
ArgumentCaptor.forClass(ConditionalUserProperty.class);
verify(mockAnalyticsConnector, times(1))
.clearConditionalUserProperty(TEST_EXPERIMENT_1_ID, null, null);
verify(mockAnalyticsConnector, times(1))
.clearConditionalUserProperty(TEST_EXPERIMENT_2_ID, null, null);
verify(mockAnalyticsConnector, times(3))
.setConditionalUserProperty(analyticsExperimentArgumentCaptor.capture());

List<ConditionalUserProperty> actualValues = analyticsExperimentArgumentCaptor.getAllValues();
assertThat(AbtExperimentInfo.fromConditionalUserProperty(actualValues.get(0)).toStringMap())
.isEqualTo(TEST_ABT_EXPERIMENT_2_VARIANT_B.toStringMap());
assertThat(AbtExperimentInfo.fromConditionalUserProperty(actualValues.get(1)).toStringMap())
.isEqualTo(newExperiment3.toStringMap());
assertThat(AbtExperimentInfo.fromConditionalUserProperty(actualValues.get(2)).toStringMap())
.isEqualTo(newExperiment4.toStringMap());
}

@Test
public void replaceAllExperiments_totalExperimentsExceedsAnalyticsLimit_oldExperimentsDiscarded()
throws Exception {
Expand All @@ -155,17 +211,18 @@ public void replaceAllExperiments_totalExperimentsExceedsAnalyticsLimit_oldExper
.thenReturn(
Lists.newArrayList(
TEST_ABT_EXPERIMENT_1.toConditionalUserProperty(ORIGIN_SERVICE),
TEST_ABT_EXPERIMENT_2.toConditionalUserProperty(ORIGIN_SERVICE)));
TEST_ABT_EXPERIMENT_2_VARIANT_A.toConditionalUserProperty(ORIGIN_SERVICE)));

AbtExperimentInfo newExperiment3 = createExperimentInfo("3", "", 1000L);
AbtExperimentInfo newExperiment4 = createExperimentInfo("4", "trigger_event_4", 1000L);
AbtExperimentInfo newExperiment3 = createExperimentInfo("3", TEST_VARIANT_ID_A, "", 1000L);
AbtExperimentInfo newExperiment4 =
createExperimentInfo("4", TEST_VARIANT_ID_A, "trigger_event_4", 1000L);

// Simulates the case where experiment 1 and 2 are assigned (as before), experiment 3 and
// experiment 4 are newly assigned.
firebaseAbt.replaceAllExperiments(
Lists.newArrayList(
TEST_ABT_EXPERIMENT_1.toStringMap(),
TEST_ABT_EXPERIMENT_2.toStringMap(),
TEST_ABT_EXPERIMENT_2_VARIANT_A.toStringMap(),
newExperiment3.toStringMap(),
newExperiment4.toStringMap()));

Expand Down Expand Up @@ -231,7 +288,7 @@ public void removeAllExperiments_existExperimentsInAnalytics_experimentsClearedF
.thenReturn(
Lists.newArrayList(
TEST_ABT_EXPERIMENT_1.toConditionalUserProperty(ORIGIN_SERVICE),
TEST_ABT_EXPERIMENT_2.toConditionalUserProperty(ORIGIN_SERVICE)));
TEST_ABT_EXPERIMENT_2_VARIANT_A.toConditionalUserProperty(ORIGIN_SERVICE)));

firebaseAbt.removeAllExperiments();

Expand Down Expand Up @@ -266,15 +323,15 @@ public void getAllExperiments_existExperimentsInAnalytics_returnAllExperiments()
.thenReturn(
Lists.newArrayList(
TEST_ABT_EXPERIMENT_1.toConditionalUserProperty(ORIGIN_SERVICE),
TEST_ABT_EXPERIMENT_2.toConditionalUserProperty(ORIGIN_SERVICE)));
TEST_ABT_EXPERIMENT_2_VARIANT_A.toConditionalUserProperty(ORIGIN_SERVICE)));

List<AbtExperimentInfo> abtExperimentInfoList = firebaseAbt.getAllExperiments();

assertThat(abtExperimentInfoList).hasSize(2);
assertThat(abtExperimentInfoList.get(0).toStringMap())
.isEqualTo(TEST_ABT_EXPERIMENT_1.toStringMap());
assertThat(abtExperimentInfoList.get(1).toStringMap())
.isEqualTo(TEST_ABT_EXPERIMENT_2.toStringMap());
.isEqualTo(TEST_ABT_EXPERIMENT_2_VARIANT_A.toStringMap());
}

@Test
Expand All @@ -298,7 +355,7 @@ public void getAllExperiments_analyticsSdkUnavailable_throwsAbtException() {
.thenReturn(
Lists.newArrayList(
TEST_ABT_EXPERIMENT_1.toConditionalUserProperty(ORIGIN_SERVICE),
TEST_ABT_EXPERIMENT_2.toConditionalUserProperty(ORIGIN_SERVICE)));
TEST_ABT_EXPERIMENT_2_VARIANT_A.toConditionalUserProperty(ORIGIN_SERVICE)));

// Update to just one experiment running
firebaseAbt.validateRunningExperiments(Lists.newArrayList(TEST_ABT_EXPERIMENT_1));
Expand All @@ -315,11 +372,11 @@ public void validateRunningExperiments_noinactiveExperimentsInAnalytics_cleansUp
.thenReturn(
Lists.newArrayList(
TEST_ABT_EXPERIMENT_1.toConditionalUserProperty(ORIGIN_SERVICE),
TEST_ABT_EXPERIMENT_2.toConditionalUserProperty(ORIGIN_SERVICE)));
TEST_ABT_EXPERIMENT_2_VARIANT_A.toConditionalUserProperty(ORIGIN_SERVICE)));

// Update still says the same two experiments are running
firebaseAbt.validateRunningExperiments(
Lists.newArrayList(TEST_ABT_EXPERIMENT_1, TEST_ABT_EXPERIMENT_2));
Lists.newArrayList(TEST_ABT_EXPERIMENT_1, TEST_ABT_EXPERIMENT_2_VARIANT_A));

// Verify nothing cleared
verify(mockAnalyticsConnector, never()).clearConditionalUserProperty(any(), any(), any());
Expand All @@ -346,11 +403,14 @@ public void reportActiveExperiment_setsNullTriggerCondition() throws Exception {
}

private static AbtExperimentInfo createExperimentInfo(
String experimentId, String triggerEventName, long experimentStartTimeInEpochMillis) {
String experimentId,
String variantId,
String triggerEventName,
long experimentStartTimeInEpochMillis) {

return new AbtExperimentInfo(
experimentId,
VARIANT_ID_VALUE,
variantId,
triggerEventName,
new Date(experimentStartTimeInEpochMillis),
TRIGGER_TIMEOUT_IN_MILLIS_VALUE,
Expand Down
Loading