Skip to content

Commit 6bb9969

Browse files
Feature/bucketing (#154)
* bucketing id implementation * add a few more unit tests * change bucket name * added event builder code and tests after looking at php master bucketing_id changes * cleanup from alda's comments * update bucketingId javadoc and tests * remove userId from bucketer method parameters since it is only used for logging * update to fix FindBug warnings
1 parent f0ca996 commit 6bb9969

File tree

8 files changed

+449
-144
lines changed

8 files changed

+449
-144
lines changed

core-api/src/main/java/com/optimizely/ab/Optimizely.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -773,7 +773,8 @@ private EventType getEventTypeOrThrow(ProjectConfig projectConfig, String eventN
773773
* {@link ProjectConfig}.
774774
*
775775
* @param projectConfig the current project config
776-
* @param attributes the attributes map to validate and potentially filter
776+
* @param attributes the attributes map to validate and potentially filter. The reserved key for bucketing id
777+
* {@link DecisionService#BUCKETING_ATTRIBUTE} is kept.
777778
* @return the filtered attributes map (containing only attributes that are present in the project config) or an
778779
* empty map if a null attributes object is passed in
779780
*/
@@ -788,7 +789,8 @@ private Map<String, String> filterAttributes(@Nonnull ProjectConfig projectConfi
788789

789790
Map<String, Attribute> attributeKeyMapping = projectConfig.getAttributeKeyMapping();
790791
for (Map.Entry<String, String> attribute : attributes.entrySet()) {
791-
if (!attributeKeyMapping.containsKey(attribute.getKey())) {
792+
if (!attributeKeyMapping.containsKey(attribute.getKey()) &&
793+
attribute.getKey() != com.optimizely.ab.bucketing.DecisionService.BUCKETING_ATTRIBUTE) {
792794
if (unknownAttributes == null) {
793795
unknownAttributes = new ArrayList<String>();
794796
}

core-api/src/main/java/com/optimizely/ab/bucketing/Bucketer.java

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -76,87 +76,90 @@ private String bucketToEntity(int bucketValue, List<TrafficAllocation> trafficAl
7676
}
7777

7878
private Experiment bucketToExperiment(@Nonnull Group group,
79-
@Nonnull String userId) {
79+
@Nonnull String bucketingId) {
8080
// "salt" the bucket id using the group id
81-
String bucketId = userId + group.getId();
81+
String bucketKey = bucketingId + group.getId();
8282

8383
List<TrafficAllocation> trafficAllocations = group.getTrafficAllocation();
8484

85-
int hashCode = MurmurHash3.murmurhash3_x86_32(bucketId, 0, bucketId.length(), MURMUR_HASH_SEED);
85+
int hashCode = MurmurHash3.murmurhash3_x86_32(bucketKey, 0, bucketKey.length(), MURMUR_HASH_SEED);
8686
int bucketValue = generateBucketValue(hashCode);
87-
logger.debug("Assigned bucket {} to user \"{}\" during experiment bucketing.", bucketValue, userId);
87+
logger.debug("Assigned bucket {} to user with bucketingId \"{}\" during experiment bucketing.", bucketValue, bucketingId);
8888

8989
String bucketedExperimentId = bucketToEntity(bucketValue, trafficAllocations);
9090
if (bucketedExperimentId != null) {
9191
return projectConfig.getExperimentIdMapping().get(bucketedExperimentId);
9292
}
9393

9494
// user was not bucketed to an experiment in the group
95-
logger.info("User \"{}\" is not in any experiment of group {}.", userId, group.getId());
9695
return null;
9796
}
9897

9998
private Variation bucketToVariation(@Nonnull Experiment experiment,
100-
@Nonnull String userId) {
99+
@Nonnull String bucketingId) {
101100
// "salt" the bucket id using the experiment id
102101
String experimentId = experiment.getId();
103102
String experimentKey = experiment.getKey();
104-
String combinedBucketId = userId + experimentId;
103+
String combinedBucketId = bucketingId + experimentId;
105104

106105
List<TrafficAllocation> trafficAllocations = experiment.getTrafficAllocation();
107106

108107
int hashCode = MurmurHash3.murmurhash3_x86_32(combinedBucketId, 0, combinedBucketId.length(), MURMUR_HASH_SEED);
109108
int bucketValue = generateBucketValue(hashCode);
110-
logger.debug("Assigned bucket {} to user \"{}\" during variation bucketing.", bucketValue, userId);
109+
logger.debug("Assigned bucket {} to user with bucketingId \"{}\" when bucketing to a variation.", bucketValue, bucketingId);
111110

112111
String bucketedVariationId = bucketToEntity(bucketValue, trafficAllocations);
113112
if (bucketedVariationId != null) {
114113
Variation bucketedVariation = experiment.getVariationIdToVariationMap().get(bucketedVariationId);
115114
String variationKey = bucketedVariation.getKey();
116-
logger.info("User \"{}\" is in variation \"{}\" of experiment \"{}\".", userId, variationKey,
117-
experimentKey);
115+
logger.info("User with bucketingId \"{}\" is in variation \"{}\" of experiment \"{}\".", bucketingId, variationKey,
116+
experimentKey);
118117

119118
return bucketedVariation;
120119
}
121120

122121
// user was not bucketed to a variation
123-
logger.info("User \"{}\" is not in any variation of experiment \"{}\".", userId, experimentKey);
122+
logger.info("User with bucketingId \"{}\" is not in any variation of experiment \"{}\".", bucketingId, experimentKey);
124123
return null;
125124
}
126125

127126
/**
128127
* Assign a {@link Variation} of an {@link Experiment} to a user based on hashed value from murmurhash3.
129128
* @param experiment The Experiment in which the user is to be bucketed.
130-
* @param userId User Identifier
129+
* @param bucketingId string A customer-assigned value used to create the key for the murmur hash.
131130
* @return Variation the user is bucketed into or null.
132131
*/
133132
public @Nullable Variation bucket(@Nonnull Experiment experiment,
134-
@Nonnull String userId) {
133+
@Nonnull String bucketingId) {
135134
// ---------- Bucket User ----------
136135
String groupId = experiment.getGroupId();
137136
// check whether the experiment belongs to a group
138137
if (!groupId.isEmpty()) {
139138
Group experimentGroup = projectConfig.getGroupIdMapping().get(groupId);
140139
// bucket to an experiment only if group entities are to be mutually exclusive
141140
if (experimentGroup.getPolicy().equals(Group.RANDOM_POLICY)) {
142-
Experiment bucketedExperiment = bucketToExperiment(experimentGroup, userId);
141+
Experiment bucketedExperiment = bucketToExperiment(experimentGroup, bucketingId);
143142
if (bucketedExperiment == null) {
143+
logger.info("User with bucketingId \"{}\" is not in any experiment of group {}.", bucketingId, experimentGroup.getId());
144144
return null;
145+
}
146+
else {
147+
145148
}
146149
// if the experiment a user is bucketed in within a group isn't the same as the experiment provided,
147150
// don't perform further bucketing within the experiment
148151
if (!bucketedExperiment.getId().equals(experiment.getId())) {
149-
logger.info("User \"{}\" is not in experiment \"{}\" of group {}.", userId, experiment.getKey(),
152+
logger.info("User with bucketingId \"{}\" is not in experiment \"{}\" of group {}.", bucketingId, experiment.getKey(),
150153
experimentGroup.getId());
151154
return null;
152155
}
153156

154-
logger.info("User \"{}\" is in experiment \"{}\" of group {}.", userId, experiment.getKey(),
157+
logger.info("User with bucketingId \"{}\" is in experiment \"{}\" of group {}.", bucketingId, experiment.getKey(),
155158
experimentGroup.getId());
156159
}
157160
}
158161

159-
return bucketToVariation(experiment, userId);
162+
return bucketToVariation(experiment, bucketingId);
160163
}
161164

162165

core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
*/
4747
public class DecisionService {
4848

49+
public static final String BUCKETING_ATTRIBUTE = "$opt_bucketing_id";
4950
private final Bucketer bucketer;
5051
private final ErrorHandler errorHandler;
5152
private final ProjectConfig projectConfig;
@@ -99,6 +100,7 @@ public DecisionService(@Nonnull Bucketer bucketer,
99100

100101
// fetch the user profile map from the user profile service
101102
UserProfile userProfile = null;
103+
102104
if (userProfileService != null) {
103105
try {
104106
Map<String, Object> userProfileMap = userProfileService.lookup(userId);
@@ -127,7 +129,11 @@ public DecisionService(@Nonnull Bucketer bucketer,
127129
}
128130

129131
if (ExperimentUtils.isUserInExperiment(projectConfig, experiment, filteredAttributes)) {
130-
variation = bucketer.bucket(experiment, userId);
132+
String bucketingId = userId;
133+
if (filteredAttributes.containsKey(BUCKETING_ATTRIBUTE)) {
134+
bucketingId = filteredAttributes.get(BUCKETING_ATTRIBUTE);
135+
}
136+
variation = bucketer.bucket(experiment, bucketingId);
131137

132138
if (variation != null) {
133139
if (userProfileService != null) {
@@ -204,14 +210,19 @@ public DecisionService(@Nonnull Bucketer bucketer,
204210

205211
// for all rules before the everyone else rule
206212
int rolloutRulesLength = rollout.getExperiments().size();
213+
String bucketingId = userId;
214+
if (filteredAttributes.containsKey(BUCKETING_ATTRIBUTE)) {
215+
bucketingId = filteredAttributes.get(BUCKETING_ATTRIBUTE);
216+
}
207217
Variation variation;
208218
for (int i = 0; i < rolloutRulesLength - 1; i++) {
209219
Experiment rolloutRule = rollout.getExperiments().get(i);
210220
Audience audience = projectConfig.getAudienceIdMapping().get(rolloutRule.getAudienceIds().get(0));
211221
if (ExperimentUtils.isUserInExperiment(projectConfig, rolloutRule, filteredAttributes)) {
212-
logger.debug("Attempting to bucket user \"{}\" into rollout rule for audience \"{}\".",
213-
userId, audience.getName());
214-
variation = bucketer.bucket(rolloutRule, userId);
222+
logger.debug("Attempting to bucket user \"" + userId +
223+
"\" into rollout rule for audience \"" + audience.getName() +
224+
"\".");
225+
variation = bucketer.bucket(rolloutRule, bucketingId);
215226
if (variation == null) {
216227
logger.debug("User \"{}\" was excluded due to traffic allocation.", userId);
217228
break;
@@ -226,7 +237,7 @@ public DecisionService(@Nonnull Bucketer bucketer,
226237

227238
// get last rule which is the everyone else rule
228239
Experiment everyoneElseRule = rollout.getExperiments().get(rolloutRulesLength - 1);
229-
variation = bucketer.bucket(everyoneElseRule, userId); // ignore audience
240+
variation = bucketer.bucket(everyoneElseRule, bucketingId); // ignore audience
230241
if (variation == null) {
231242
logger.debug("User \"{}\" was excluded from the \"Everyone Else\" rule for feature flag \"{}\".",
232243
userId, featureFlag.getKey());

core-api/src/main/java/com/optimizely/ab/event/internal/EventBuilderV2.java

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.optimizely.ab.event.internal;
1818

1919
import com.optimizely.ab.annotations.VisibleForTesting;
20+
import com.optimizely.ab.bucketing.DecisionService;
2021
import com.optimizely.ab.config.Attribute;
2122
import com.optimizely.ab.config.Experiment;
2223
import com.optimizely.ab.config.ProjectConfig;
@@ -49,6 +50,7 @@ public class EventBuilderV2 extends EventBuilder {
4950

5051
static final String IMPRESSION_ENDPOINT = "https://logx.optimizely.com/log/decision";
5152
static final String CONVERSION_ENDPOINT = "https://logx.optimizely.com/log/event";
53+
static final String ATTRIBUTE_KEY_FOR_BUCKETING_ATTRIBUTE = "optimizely_bucketing_id";
5254

5355
@VisibleForTesting
5456
public final ClientEngine clientEngine;
@@ -159,13 +161,22 @@ private List<Feature> createUserFeatures(Map<String, String> attributes, Project
159161
String attributeKey = attributeEntry.getKey();
160162
Attribute attribute = attributeKeyMapping.get(attributeKey);
161163

164+
if (attributeEntry.getKey() == DecisionService.BUCKETING_ATTRIBUTE) {
165+
features.add(new Feature(com.optimizely.ab.bucketing.DecisionService.BUCKETING_ATTRIBUTE,
166+
ATTRIBUTE_KEY_FOR_BUCKETING_ATTRIBUTE,
167+
Feature.CUSTOM_ATTRIBUTE_FEATURE_TYPE,
168+
attributeEntry.getValue(), true));
169+
continue;
170+
}
171+
162172
if (attribute == null) {
163173
logger.warn("Attempting to use unknown attribute key: {}. Attribute will be ignored", attributeKey);
164174
continue;
165175
}
166176

167177
features.add(new Feature(attribute.getId(), attributeKey, Feature.CUSTOM_ATTRIBUTE_FEATURE_TYPE,
168-
attributeEntry.getValue(), true));
178+
attributeEntry.getValue(), true));
179+
169180
}
170181

171182
return features;

0 commit comments

Comments
 (0)