Skip to content

Commit

Permalink
Id5 ID System: handle A/B testing server side in prebid (prebid#7086)
Browse files Browse the repository at this point in the history 
* #22 Removing a/b logic and using result from server side

* #22 Better docs

* #22 Added a check in a test case

* fix typo

Co-authored-by: Marco Cosentino <mcosentino@id5.io>
Co-authored-by: Scott Menzer <scott@id5.io>
  • Loading branch information
@cosenmarco @smenzer
3 people authored Jun 23, 2021
1 parent ba5f404 commit 5ad0df0
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 209 deletions.
77 changes: 25 additions & 52 deletions modules/id5IdSystem.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ const NB_EXP_DAYS = 30;
export const ID5_STORAGE_NAME = 'id5id';
export const ID5_PRIVACY_STORAGE_NAME = `${ID5_STORAGE_NAME}_privacy`;
const LOCAL_STORAGE = 'html5';
const ABTEST_RESOLUTION = 10000;
const LOG_PREFIX = 'User ID - ID5 submodule: ';

// order the legacy cookie names in reverse priority order so the last
Expand Down Expand Up @@ -59,22 +58,6 @@ export const id5IdSubmodule = {
return undefined;
}

// check for A/B testing configuration and hide ID if in Control Group
const abConfig = getAbTestingConfig(config);
const controlGroup = isInControlGroup(universalUid, abConfig.controlGroupPct);
if (abConfig.enabled === true && typeof controlGroup === 'undefined') {
// A/B Testing is enabled, but configured improperly, so skip A/B testing
utils.logError(LOG_PREFIX + 'A/B Testing controlGroupPct must be a number >= 0 and <= 1! Skipping A/B Testing');
} else if (abConfig.enabled === true && controlGroup === true) {
// A/B Testing is enabled and user is in the Control Group, so do not share the ID5 ID
utils.logInfo(LOG_PREFIX + 'A/B Testing Enabled - user is in the Control Group, so the ID5 ID is NOT exposed');
universalUid = '';
linkType = 0;
} else if (abConfig.enabled === true) {
// A/B Testing is enabled but user is not in the Control Group, so ID5 ID is shared
utils.logInfo(LOG_PREFIX + 'A/B Testing Enabled - user is NOT in the Control Group, so the ID5 ID is exposed');
}

let responseObj = {
id5id: {
uid: universalUid,
Expand All @@ -84,8 +67,22 @@ export const id5IdSubmodule = {
}
};

if (abConfig.enabled === true) {
utils.deepSetValue(responseObj, 'id5id.ext.abTestingControlGroup', (typeof controlGroup === 'undefined' ? false : controlGroup));
const abTestingResult = utils.deepAccess(value, 'ab_testing.result');
switch (abTestingResult) {
case 'control':
// A/B Testing is enabled and user is in the Control Group
utils.logInfo(LOG_PREFIX + 'A/B Testing - user is in the Control Group: ID5 ID is NOT exposed');
utils.deepSetValue(responseObj, 'id5id.ext.abTestingControlGroup', true);
break;
case 'error':
// A/B Testing is enabled, but configured improperly, so skip A/B testing
utils.logError(LOG_PREFIX + 'A/B Testing ERROR! controlGroupPct must be a number >= 0 and <= 1');
break;
case 'normal':
// A/B Testing is enabled but user is not in the Control Group, so ID5 ID is shared
utils.logInfo(LOG_PREFIX + 'A/B Testing - user is NOT in the Control Group');
utils.deepSetValue(responseObj, 'id5id.ext.abTestingControlGroup', false);
break;
}

utils.logInfo(LOG_PREFIX + 'Decoded ID', responseObj);
Expand Down Expand Up @@ -139,9 +136,12 @@ export const id5IdSubmodule = {
data.provider = config.params.provider;
}

// pass in feature flags, if applicable
if (getAbTestingConfig(config).enabled === true) {
utils.deepSetValue(data, 'features.ab', 1);
const abTestingConfig = getAbTestingConfig(config);
if (abTestingConfig.enabled === true) {
data.ab_testing = {
enabled: true,
control_group_pct: abTestingConfig.controlGroupPct // The server validates
};
}

const resp = function (callback) {
Expand Down Expand Up @@ -178,7 +178,7 @@ export const id5IdSubmodule = {
utils.logInfo(LOG_PREFIX + 'requesting an ID from the server', data);
ajax(url, callbacks, JSON.stringify(data), { method: 'POST', withCredentials: true });
};
return {callback: resp};
return { callback: resp };
},

/**
Expand Down Expand Up @@ -310,37 +310,10 @@ export function storeInLocalStorage(key, value, expDays) {
* gets the existing abTesting config or generates a default config with abTesting off
*
* @param {SubmoduleConfig|undefined} config
* @returns {(Object|undefined)}
* @returns {Object} an object which always contains at least the property "enabled"
*/
function getAbTestingConfig(config) {
return (config && config.params && config.params.abTesting) || { enabled: false };
}

/**
* Return a consistant random number between 0 and ABTEST_RESOLUTION-1 for this user
* Falls back to plain random if no user provided
* @param {string} userId
* @returns {number}
*/
function abTestBucket(userId) {
if (userId) {
return ((utils.cyrb53Hash(userId) % ABTEST_RESOLUTION) + ABTEST_RESOLUTION) % ABTEST_RESOLUTION;
} else {
return Math.floor(Math.random() * ABTEST_RESOLUTION);
}
}

/**
* Return a consistant boolean if this user is within the control group ratio provided
* @param {string} userId
* @param {number} controlGroupPct - Ratio [0,1] of users expected to be in the control group
* @returns {boolean}
*/
export function isInControlGroup(userId, controlGroupPct) {
if (!utils.isNumber(controlGroupPct) || controlGroupPct < 0 || controlGroupPct > 1) {
return undefined;
}
return abTestBucket(userId) < controlGroupPct * ABTEST_RESOLUTION;
return utils.deepAccess(config, 'params.abTesting', { enabled: false });
}

submodule('userId', id5IdSubmodule);
190 changes: 33 additions & 157 deletions test/spec/modules/id5IdSystem_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,6 @@ describe('ID5 ID System', function() {
}
}
}
function getFetchCookieConfig() {
return getUserSyncConfig([getId5FetchConfig(ID5_STORAGE_NAME, 'cookie')]);
}
function getFetchLocalStorageConfig() {
return getUserSyncConfig([getId5FetchConfig(ID5_STORAGE_NAME, 'html5')]);
}
Expand Down Expand Up @@ -239,43 +236,44 @@ describe('ID5 ID System', function() {
expect(getNbFromCache(ID5_TEST_PARTNER_ID)).to.be.eq(0);
});

it('should call the ID5 server with ab feature = 1 when abTesting is turned on', function () {
it('should call the ID5 server with ab_testing object when abTesting is turned on', function () {
let id5Config = getId5FetchConfig();
id5Config.params.abTesting = { enabled: true, controlGroupPct: 10 }
id5Config.params.abTesting = { enabled: true, controlGroupPct: 0.234 }

let submoduleCallback = id5IdSubmodule.getId(id5Config, undefined, ID5_STORED_OBJ).callback;
submoduleCallback(callbackSpy);

let request = server.requests[0];
let requestBody = JSON.parse(request.requestBody);
expect(requestBody.features.ab).to.eq(1);
expect(requestBody.ab_testing.enabled).to.eq(true);
expect(requestBody.ab_testing.control_group_pct).to.eq(0.234);

request.respond(200, responseHeader, JSON.stringify(ID5_JSON_RESPONSE));
});

it('should call the ID5 server without ab feature when abTesting is turned off', function () {
it('should call the ID5 server without ab_testing object when abTesting is turned off', function () {
let id5Config = getId5FetchConfig();
id5Config.params.abTesting = { enabled: false, controlGroupPct: 10 }
id5Config.params.abTesting = { enabled: false, controlGroupPct: 0.55 }

let submoduleCallback = id5IdSubmodule.getId(id5Config, undefined, ID5_STORED_OBJ).callback;
submoduleCallback(callbackSpy);

let request = server.requests[0];
let requestBody = JSON.parse(request.requestBody);
expect(requestBody.features).to.be.undefined;
expect(requestBody.ab_testing).to.be.undefined;

request.respond(200, responseHeader, JSON.stringify(ID5_JSON_RESPONSE));
});

it('should call the ID5 server without ab feature when when abTesting is not set', function () {
it('should call the ID5 server without ab_testing when when abTesting is not set', function () {
let id5Config = getId5FetchConfig();

let submoduleCallback = id5IdSubmodule.getId(id5Config, undefined, ID5_STORED_OBJ).callback;
submoduleCallback(callbackSpy);

let request = server.requests[0];
let requestBody = JSON.parse(request.requestBody);
expect(requestBody.features).to.be.undefined;
expect(requestBody.ab_testing).to.be.undefined;

request.respond(200, responseHeader, JSON.stringify(ID5_JSON_RESPONSE));
});
Expand Down Expand Up @@ -450,105 +448,11 @@ describe('ID5 ID System', function() {
const expectedDecodedObjectWithIdAbOff = { id5id: { uid: ID5_STORED_ID, ext: { linkType: ID5_STORED_LINK_TYPE } } };
const expectedDecodedObjectWithIdAbOn = { id5id: { uid: ID5_STORED_ID, ext: { linkType: ID5_STORED_LINK_TYPE, abTestingControlGroup: false } } };
const expectedDecodedObjectWithoutIdAbOn = { id5id: { uid: '', ext: { linkType: 0, abTestingControlGroup: true } } };
let testConfig;
let testConfig, storedObject;

beforeEach(function() {
testConfig = getId5FetchConfig();
});

describe('Configuration Validation', function() {
let logErrorSpy;
let logInfoSpy;

beforeEach(function() {
logErrorSpy = sinon.spy(utils, 'logError');
logInfoSpy = sinon.spy(utils, 'logInfo');
});
afterEach(function() {
logErrorSpy.restore();
logInfoSpy.restore();
});

// A/B Testing ON, but invalid config
let testInvalidAbTestingConfigsWithError = [
{ enabled: true },
{ enabled: true, controlGroupPct: 2 },
{ enabled: true, controlGroupPct: -1 },
{ enabled: true, controlGroupPct: 'a' },
{ enabled: true, controlGroupPct: true }
];
testInvalidAbTestingConfigsWithError.forEach((testAbTestingConfig) => {
it('should be undefined if ratio is invalid', () => {
expect(isInControlGroup('userId', testAbTestingConfig.controlGroupPct)).to.be.undefined;
});
it('should error if config is invalid, and always return an ID', function () {
testConfig.params.abTesting = testAbTestingConfig;
let decoded = id5IdSubmodule.decode(ID5_STORED_OBJ, testConfig);
expect(decoded).to.deep.equal(expectedDecodedObjectWithIdAbOn);
sinon.assert.calledOnce(logErrorSpy);
});
});

// A/B Testing OFF, with invalid config (ignore)
let testInvalidAbTestingConfigsWithoutError = [
{ enabled: false, controlGroupPct: -1 },
{ enabled: false, controlGroupPct: 2 },
{ enabled: false, controlGroupPct: 'a' },
{ enabled: false, controlGroupPct: true }
];
testInvalidAbTestingConfigsWithoutError.forEach((testAbTestingConfig) => {
it('should be undefined if ratio is invalid', () => {
expect(isInControlGroup('userId', testAbTestingConfig.controlGroupPct)).to.be.undefined;
});
it('should not error if config is invalid but A/B testing is off, and always return an ID', function () {
testConfig.params.abTesting = testAbTestingConfig;
let decoded = id5IdSubmodule.decode(ID5_STORED_OBJ, testConfig);
expect(decoded).to.deep.equal(expectedDecodedObjectWithIdAbOff);
sinon.assert.notCalled(logErrorSpy);
});
});

// A/B Testing ON, with valid config
let testValidConfigs = [
{ enabled: true, controlGroupPct: 0 },
{ enabled: true, controlGroupPct: 0.5 },
{ enabled: true, controlGroupPct: 1 }
];
testValidConfigs.forEach((testAbTestingConfig) => {
it('should not be undefined if ratio is valid', () => {
expect(isInControlGroup('userId', testAbTestingConfig.controlGroupPct)).to.not.be.undefined;
});
it('should not error if config is valid', function () {
testConfig.params.abTesting = testAbTestingConfig;
id5IdSubmodule.decode(ID5_STORED_OBJ, testConfig);
sinon.assert.notCalled(logErrorSpy);
});
});
});

describe('A/B Testing Config is not Set', function() {
let randStub;

beforeEach(function() {
randStub = sinon.stub(Math, 'random').callsFake(function() {
return 0;
});
});
afterEach(function () {
randStub.restore();
});

it('should expose ID when A/B config is not set', function () {
let decoded = id5IdSubmodule.decode(ID5_STORED_OBJ, testConfig);
expect(decoded).to.deep.equal(expectedDecodedObjectWithIdAbOff);
});

it('should expose ID when A/B config is empty', function () {
testConfig.params.abTesting = { };

let decoded = id5IdSubmodule.decode(ID5_STORED_OBJ, testConfig);
expect(decoded).to.deep.equal(expectedDecodedObjectWithIdAbOff);
});
storedObject = utils.deepClone(ID5_STORED_OBJ);
});

describe('A/B Testing Config is Set', function() {
Expand All @@ -563,69 +467,41 @@ describe('ID5 ID System', function() {
randStub.restore();
});

describe('IsInControlGroup', function () {
it('Nobody is in a 0% control group', function () {
expect(isInControlGroup('dsdndskhsdks', 0)).to.be.false;
expect(isInControlGroup('3erfghyuijkm', 0)).to.be.false;
expect(isInControlGroup('', 0)).to.be.false;
expect(isInControlGroup(undefined, 0)).to.be.false;
});

it('Everybody is in a 100% control group', function () {
expect(isInControlGroup('dsdndskhsdks', 1)).to.be.true;
expect(isInControlGroup('3erfghyuijkm', 1)).to.be.true;
expect(isInControlGroup('', 1)).to.be.true;
expect(isInControlGroup(undefined, 1)).to.be.true;
});
describe('Decode', function() {
let logErrorSpy;

it('Being in the control group must be consistant', function () {
const inControlGroup = isInControlGroup('dsdndskhsdks', 0.5);
expect(inControlGroup === isInControlGroup('dsdndskhsdks', 0.5)).to.be.true;
expect(inControlGroup === isInControlGroup('dsdndskhsdks', 0.5)).to.be.true;
expect(inControlGroup === isInControlGroup('dsdndskhsdks', 0.5)).to.be.true;
beforeEach(function() {
logErrorSpy = sinon.spy(utils, 'logError');
});

it('Control group ratio must be within a 10% error on a large sample', function () {
let nbInControlGroup = 0;
const sampleSize = 100;
for (let i = 0; i < sampleSize; i++) {
nbInControlGroup = nbInControlGroup + (isInControlGroup('R$*df' + i, 0.5) ? 1 : 0);
}
expect(nbInControlGroup).to.be.greaterThan(sampleSize / 2 - sampleSize / 10);
expect(nbInControlGroup).to.be.lessThan(sampleSize / 2 + sampleSize / 10);
afterEach(function() {
logErrorSpy.restore();
});
});

describe('Decode', function() {
it('should expose ID when A/B testing is off', function () {
testConfig.params.abTesting = {
enabled: false,
controlGroupPct: 0.5
};

let decoded = id5IdSubmodule.decode(ID5_STORED_OBJ, testConfig);
it('should not set abTestingControlGroup extension when A/B testing is off', function () {
let decoded = id5IdSubmodule.decode(storedObject, testConfig);
expect(decoded).to.deep.equal(expectedDecodedObjectWithIdAbOff);
});

it('should expose ID when no one is in control group', function () {
testConfig.params.abTesting = {
enabled: true,
controlGroupPct: 0
};

let decoded = id5IdSubmodule.decode(ID5_STORED_OBJ, testConfig);
it('should set abTestingControlGroup to false when A/B testing is on but in normal group', function () {
storedObject.ab_testing = { result: 'normal' };
let decoded = id5IdSubmodule.decode(storedObject, testConfig);
expect(decoded).to.deep.equal(expectedDecodedObjectWithIdAbOn);
});

it('should not expose ID when everyone is in control group', function () {
testConfig.params.abTesting = {
enabled: true,
controlGroupPct: 1
};

let decoded = id5IdSubmodule.decode(ID5_STORED_OBJ, testConfig);
storedObject.ab_testing = { result: 'control' };
storedObject.universal_uid = '';
storedObject.link_type = 0;
let decoded = id5IdSubmodule.decode(storedObject, testConfig);
expect(decoded).to.deep.equal(expectedDecodedObjectWithoutIdAbOn);
});

it('should log A/B testing errors', function () {
storedObject.ab_testing = { result: 'error' };
let decoded = id5IdSubmodule.decode(storedObject, testConfig);
expect(decoded).to.deep.equal(expectedDecodedObjectWithIdAbOff);
sinon.assert.calledOnce(logErrorSpy);
});
});
});
});
Expand Down

0 comments on commit 5ad0df0

Please sign in to comment.