Skip to content

Use attributes for sticky bucketing #179

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.

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 9 commits into from
Nov 8, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
60 changes: 38 additions & 22 deletions packages/optimizely-sdk/lib/core/decision_service/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ var LOG_MESSAGES = enums.LOG_MESSAGES;
var DECISION_SOURCES = enums.DECISION_SOURCES;



/**
* Optimizely's decision service that determines which variation of an experiment the user will be allocated to.
*
Expand Down Expand Up @@ -79,8 +80,8 @@ DecisionService.prototype.getVariation = function(experimentKey, userId, attribu
}

// check for sticky bucketing
var userProfile = this.__getUserProfile(userId);
variation = this.__getStoredVariation(experiment, userProfile);
var experimentBucketMap = this.__resolveExperimentBucketMap(userId, attributes);
variation = this.__getStoredVariation(experiment, userId, experimentBucketMap);
if (!!variation) {
this.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.RETURNING_STORED_VARIATION, MODULE_NAME, variation.key, experimentKey, userId));
return variation.key;
Expand All @@ -99,11 +100,24 @@ DecisionService.prototype.getVariation = function(experimentKey, userId, attribu
}

// persist bucketing
this.__saveUserProfile(userProfile, experiment, variation);
this.__saveUserProfile(experiment, variation, userId, experimentBucketMap);

return variation.key;
};

/**
* Merges attributes from attributes[STICKY_BUCKETING_KEY] and userProfileService
* @param {Object} attributes
* @return {Object} finalized copy of experiment_bucket_map
*/
DecisionService.prototype.__resolveExperimentBucketMap = function(userId, attributes) {
attributes = attributes || {}
var userProfile = this.__getUserProfile(userId) || {};
var attributeExperimentBucketMap = attributes[enums.CONTROL_ATTRIBUTES.STICKY_BUCKETING_KEY];
return fns.assignIn({}, userProfile.experiment_bucket_map, attributeExperimentBucketMap);
};


/**
* Checks whether the experiment is running or launched
* @param {string} experimentKey Key of experiment being validated
Expand Down Expand Up @@ -184,23 +198,20 @@ DecisionService.prototype.__buildBucketerParams = function(experimentKey, bucket
};

/**
* Get the stored variation from the user profile for the given experiment
* Pull the stored variation out of the experimentBucketMap for an experiment/userId
* @param {Object} experiment
* @param {Object} userProfile
* @param {String} userId
* @param {Object} experimentBucketMap mapping experiment => { variation_id: <variationId> }
* @return {Object} the stored variation or null if the user profile does not have one for the given experiment
*/
DecisionService.prototype.__getStoredVariation = function(experiment, userProfile) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description of the function is no longer accurate. Not sure it was very accurate in the first place either because it wasn't really "stored"

if (!userProfile || !userProfile.experiment_bucket_map) {
return null;
}

if (userProfile.experiment_bucket_map.hasOwnProperty(experiment.id)) {
var decision = userProfile.experiment_bucket_map[experiment.id];
DecisionService.prototype.__getStoredVariation = function(experiment, userId, experimentBucketMap) {
if (experimentBucketMap.hasOwnProperty(experiment.id)) {
var decision = experimentBucketMap[experiment.id];
var variationId = decision.variation_id;
if (this.configObj.variationIdMap.hasOwnProperty(variationId)) {
return this.configObj.variationIdMap[decision.variation_id];
} else {
this.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.SAVED_VARIATION_NOT_FOUND, MODULE_NAME, userProfile.user_id, variationId, experiment.key));
this.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.SAVED_VARIATION_NOT_FOUND, MODULE_NAME, userId, variationId, experiment.key));
}
}

Expand All @@ -210,7 +221,7 @@ DecisionService.prototype.__getStoredVariation = function(experiment, userProfil
/**
* Get the user profile with the given user ID
* @param {string} userId
* @return {Object} the stored user profile or an empty one if not found
* @return {Object|undefined} the stored user profile or undefined if one isn't found
*/
DecisionService.prototype.__getUserProfile = function(userId) {
var userProfile = {
Expand All @@ -223,33 +234,38 @@ DecisionService.prototype.__getUserProfile = function(userId) {
}

try {
userProfile = this.userProfileService.lookup(userId) || userProfile; // only assign if the lookup is successful
return this.userProfileService.lookup(userId);
} catch (ex) {
this.logger.log(LOG_LEVEL.ERROR, sprintf(ERROR_MESSAGES.USER_PROFILE_LOOKUP_ERROR, MODULE_NAME, userId, ex.message));
}
return userProfile;
};

/**
* Saves the bucketing decision to the user profile
* @param {Object} userProfile
* @param {Object} experiment
* @param {Object} variation
* @param {Object} experimentBucketMap
*/
DecisionService.prototype.__saveUserProfile = function(userProfile, experiment, variation) {
DecisionService.prototype.__saveUserProfile = function(experiment, variation, userId, experimentBucketMap) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: another doc comment changed

if (!this.userProfileService) {
return;
}

try {
userProfile.experiment_bucket_map[experiment.id] = {
variation_id: variation.id,
var newBucketMap = fns.cloneDeep(experimentBucketMap);
newBucketMap[experiment.id] = {
variation_id: variation.id
};

this.userProfileService.save(userProfile);
this.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.SAVED_VARIATION, MODULE_NAME, variation.key, experiment.key, userProfile.user_id));
this.userProfileService.save({
user_id: userId,
experiment_bucket_map: newBucketMap,
});

this.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.SAVED_VARIATION, MODULE_NAME, variation.key, experiment.key, userId));
} catch (ex) {
this.logger.log(LOG_LEVEL.ERROR, sprintf(ERROR_MESSAGES.USER_PROFILE_SAVE_ERROR, MODULE_NAME, userProfile.user_id, ex.message));
this.logger.log(LOG_LEVEL.ERROR, sprintf(ERROR_MESSAGES.USER_PROFILE_SAVE_ERROR, MODULE_NAME, userId, ex.message));
}
};

Expand Down
115 changes: 114 additions & 1 deletion packages/optimizely-sdk/lib/core/decision_service/index.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,22 @@ describe('lib/core/decision_service', function() {
assert.strictEqual(mockLogger.log.args[0][1], 'DECISION_SERVICE: Experiment testExperimentNotRunning is not running.');
});

describe('when attributes.$opt_experiment_bucket_map is supplied', function() {
it('should respect the sticky bucketing information for attributes', function() {
bucketerStub.returns('111128'); // ID of the 'control' variation from `test_data`
var attributes = {
$opt_experiment_bucket_map: {
'111127': {
'variation_id': '111129' // ID of the 'variation' variation
},
},
};

assert.strictEqual('variation', decisionServiceInstance.getVariation('testExperiment', 'decision_service_user', attributes));
sinon.assert.notCalled(bucketerStub);
});
});

describe('when a user profile service is provided', function () {
var userProfileServiceInstance = null;
var userProfileLookupStub;
Expand Down Expand Up @@ -252,6 +268,102 @@ describe('lib/core/decision_service', function() {
},
});
});

describe('when passing `attributes.$opt_experiment_bucket_map`', function() {
it('should respect attributes over the userProfileService for the matching experiment id', function () {
userProfileLookupStub.returns({
user_id: 'decision_service_user',
experiment_bucket_map: {
'111127': {
'variation_id': '111128' // ID of the 'control' variation
},
},
});

var attributes = {
$opt_experiment_bucket_map: {
'111127': {
'variation_id': '111129' // ID of the 'variation' variation
},
},
};


assert.strictEqual('variation', decisionServiceInstance.getVariation('testExperiment', 'decision_service_user', attributes));
sinon.assert.calledWith(userProfileLookupStub, 'decision_service_user');
sinon.assert.notCalled(bucketerStub);
assert.strictEqual(mockLogger.log.args[0][1], 'PROJECT_CONFIG: User decision_service_user is not in the forced variation map.');
assert.strictEqual(mockLogger.log.args[1][1], 'DECISION_SERVICE: Returning previously activated variation \"variation\" of experiment \"testExperiment\" for user \"decision_service_user\" from user profile.');
});

it('should ignore attributes for a different experiment id', function () {
userProfileLookupStub.returns({
user_id: 'decision_service_user',
experiment_bucket_map: {
'111127': { // 'testExperiment' ID
'variation_id': '111128' // ID of the 'control' variation
},
},
});

var attributes = {
$opt_experiment_bucket_map: {
'122227': { // other experiment ID
'variation_id': '122229' // ID of the 'variationWithAudience' variation
},
},
};

assert.strictEqual('control', decisionServiceInstance.getVariation('testExperiment', 'decision_service_user', attributes));
sinon.assert.calledWith(userProfileLookupStub, 'decision_service_user');
sinon.assert.notCalled(bucketerStub);
assert.strictEqual(mockLogger.log.args[0][1], 'PROJECT_CONFIG: User decision_service_user is not in the forced variation map.');
assert.strictEqual(mockLogger.log.args[1][1], 'DECISION_SERVICE: Returning previously activated variation \"control\" of experiment \"testExperiment\" for user \"decision_service_user\" from user profile.');
});

it('should use attributes when the userProfileLookup variations for other experiments', function () {
userProfileLookupStub.returns({
user_id: 'decision_service_user',
experiment_bucket_map: {
'122227': { // other experiment ID
'variation_id': '122229' // ID of the 'variationWithAudience' variation
},
}
});

var attributes = {
$opt_experiment_bucket_map: {
'111127': { // 'testExperiment' ID
'variation_id': '111129' // ID of the 'variation' variation
},
},
};

assert.strictEqual('variation', decisionServiceInstance.getVariation('testExperiment', 'decision_service_user', attributes));
sinon.assert.calledWith(userProfileLookupStub, 'decision_service_user');
sinon.assert.notCalled(bucketerStub);
assert.strictEqual(mockLogger.log.args[0][1], 'PROJECT_CONFIG: User decision_service_user is not in the forced variation map.');
assert.strictEqual(mockLogger.log.args[1][1], 'DECISION_SERVICE: Returning previously activated variation \"variation\" of experiment \"testExperiment\" for user \"decision_service_user\" from user profile.');
});

it('should use attributes when the userProfileLookup returns null', function () {
userProfileLookupStub.returns(null);

var attributes = {
$opt_experiment_bucket_map: {
'111127': {
'variation_id': '111129' // ID of the 'variation' variation
},
},
};

assert.strictEqual('variation', decisionServiceInstance.getVariation('testExperiment', 'decision_service_user', attributes));
sinon.assert.calledWith(userProfileLookupStub, 'decision_service_user');
sinon.assert.notCalled(bucketerStub);
assert.strictEqual(mockLogger.log.args[0][1], 'PROJECT_CONFIG: User decision_service_user is not in the forced variation map.');
assert.strictEqual(mockLogger.log.args[1][1], 'DECISION_SERVICE: Returning previously activated variation \"variation\" of experiment \"testExperiment\" for user \"decision_service_user\" from user profile.');
});
});
});
});

Expand Down Expand Up @@ -459,6 +571,7 @@ describe('lib/core/decision_service', function() {
'test_user',
userAttributesWithBucketingId
));
sinon.assert.calledWithExactly(userProfileLookupStub, 'test_user');
});
});

Expand All @@ -475,7 +588,7 @@ describe('lib/core/decision_service', function() {
'browser_type': 'safari',
'$opt_bucketing_id': 50
};

beforeEach(function() {
sinon.stub(mockLogger, 'log');
configObj = projectConfig.createProjectConfig(testData);
Expand Down
16 changes: 16 additions & 0 deletions packages/optimizely-sdk/lib/optimizely/index.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,22 @@ describe('lib/optimizely', function() {
JSON.stringify(expectedObj.params)));
});

describe('when experiment_bucket_map attribute is present', function() {
it('should call activate and respect attribute experiment_bucket_map', function() {
bucketStub.returns('111128'); // id of "control" variation
var activate = optlyInstance.activate('testExperiment', 'testUser', {
$opt_experiment_bucket_map: {
'111127': {
variation_id: '111129', // id of "variation" variation
},
},
});

assert.strictEqual(activate, 'variation');
sinon.assert.notCalled(bucketer.bucket);
});
});

it('should call bucketer and dispatchEvent with proper args and return variation key if user is in grouped experiment', function() {
bucketStub.returns('662');
var activate = optlyInstance.activate('groupExperiment2', 'testUser');
Expand Down
1 change: 1 addition & 0 deletions packages/optimizely-sdk/lib/utils/enums/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ exports.RESERVED_EVENT_KEYWORDS = {
exports.CONTROL_ATTRIBUTES = {
BOT_FILTERING: '$opt_bot_filtering',
BUCKETING_ID: '$opt_bucketing_id',
STICKY_BUCKETING_KEY: '$opt_experiment_bucket_map',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit. Alphabetize.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is really a nit

USER_AGENT: '$opt_user_agent',
};

Expand Down