Skip to content
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

Price floors new schema support AB Test #5390

Merged
merged 4 commits into from
Jun 25, 2020
Merged

Price floors new schema support AB Test #5390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9e52650
Price floors new schema support AB Test
robertrmartinez Jun 18, 2020
a0e27c0
Add new serve-fast command + lint fix
robertrmartinez Jun 18, 2020
455a6ea
update comment
robertrmartinez Jun 18, 2020
0f60a35
Only sum up modelWeights once and set as prop!
robertrmartinez Jun 25, 2020
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
13 changes: 7 additions & 6 deletions gulpfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,12 +341,12 @@ function injectFakeServerEndpointDev() {

function startFakeServer() {
const fakeServer = spawn('node', ['./test/fake-server/index.js', `--port=${FAKE_SERVER_PORT}`]);
fakeServer.stdout.on('data', (data) => {
console.log(`stdout: ${data}`);
});
fakeServer.stderr.on('data', (data) => {
console.log(`stderr: ${data}`);
});
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

my editor keeps yelling at me about this so I just fixed it.

fakeServer.stdout.on('data', (data) => {
console.log(`stdout: ${data}`);
});
fakeServer.stderr.on('data', (data) => {
console.log(`stderr: ${data}`);
});
}

// support tasks
Expand All @@ -372,6 +372,7 @@ gulp.task('build', gulp.series(clean, 'build-bundle-prod'));
gulp.task('build-postbid', gulp.series(escapePostbidConfig, buildPostbid));

gulp.task('serve', gulp.series(clean, lint, gulp.parallel('build-bundle-dev', watch, test)));
gulp.task('serve-fast', gulp.series(clean, gulp.parallel('build-bundle-dev', watch)));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When I am just trying to test via my E2E test pages having to wait several minutes for lint + test is a little much.

Thought others might use a similar local edit, so why not merge it in.

Copy link
Contributor

Choose a reason for hiding this comment

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

This will come in handy!

gulp.task('serve-fake', gulp.series(clean, gulp.parallel('build-bundle-dev', watch), injectFakeServerEndpointDev, test, startFakeServer));

gulp.task('default', gulp.series(clean, makeWebpackPkg));
Expand Down
63 changes: 59 additions & 4 deletions modules/priceFloors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,11 +295,29 @@ export function updateAdUnitsForAuction(adUnits, floorData, auctionId) {
});
}

export function pickRandomModel(modelGroups, weightSum) {
// we loop through the models subtracting the current model weight from our random number
// once we are at or below zero, we return the associated model
let random = Math.floor(Math.random() * weightSum + 1)
for (let i = 0; i < modelGroups.length; i++) {
random -= modelGroups[i].modelWeight;
if (random <= 0) {
return modelGroups[i];
}
}
};
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here is a jsbin to test this function.
https://jsbin.com/qihiqojovu/edit?js,console

Weights do not need to add up to 100. Only shown in jsbin for clarity.


/**
* @summary Updates the adUnits accordingly and returns the necessary floorsData for the current auction
*/
export function createFloorsDataForAuction(adUnits, auctionId) {
let resolvedFloorsData = utils.deepClone(_floorsConfig);
// if using schema 2 pick a model here:
if (utils.deepAccess(resolvedFloorsData, 'data.floorsSchemaVersion') === 2) {
// merge the models specific stuff into the top level data settings (now it looks like floorsSchemaVersion 1!)
let { modelGroups, ...rest } = resolvedFloorsData.data;
resolvedFloorsData.data = Object.assign(rest, pickRandomModel(modelGroups, rest.modelWeightSum));
}

// if we do not have a floors data set, we will try to use data set on adUnits
let useAdUnitData = Object.keys(utils.deepAccess(resolvedFloorsData, 'data.values') || {}).length === 0;
Expand Down Expand Up @@ -372,6 +390,36 @@ function validateRules(floorsData, numFields, delimiter) {
return Object.keys(floorsData.values).length > 0;
}

function modelIsValid(model) {
// schema.fields has only allowed attributes
if (!validateSchemaFields(utils.deepAccess(model, 'schema.fields'))) {
return false;
}
return validateRules(model, model.schema.fields.length, model.schema.delimiter || '|')
}

/**
* @summary Mapping of floor schema version to it's corresponding validation
*/
const floorsSchemaValidation = {
1: data => modelIsValid(data),
2: data => {
// model groups should be an array with at least one element
if (!Array.isArray(data.modelGroups) || data.modelGroups.length === 0) {
return false;
}
// every model should have valid schema, as well as an accompanying modelWeight
data.modelWeightSum = 0;
return data.modelGroups.every(model => {
if (typeof model.modelWeight === 'number' && modelIsValid(model)) {
data.modelWeightSum += model.modelWeight;
return true;
}
return false;
});
}
};

/**
* @summary Fields array should have at least one entry and all should match allowed fields
* Each rule in the values array should have a 'key' and 'floor' param
Expand All @@ -382,11 +430,12 @@ export function isFloorsDataValid(floorsData) {
if (typeof floorsData !== 'object') {
return false;
}
// schema.fields has only allowed attributes
if (!validateSchemaFields(utils.deepAccess(floorsData, 'schema.fields'))) {
floorsData.floorsSchemaVersion = floorsData.floorsSchemaVersion || 1;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

default to version 1 for backwards compat

if (typeof floorsSchemaValidation[floorsData.floorsSchemaVersion] !== 'function') {
utils.logError(`${MODULE_NAME}: Unknown floorsSchemaVersion: `, floorsData.floorsSchemaVersion);
return false;
}
return validateRules(floorsData, floorsData.schema.fields.length, floorsData.schema.delimiter || '|')
return floorsSchemaValidation[floorsData.floorsSchemaVersion](floorsData);
}

/**
Expand Down Expand Up @@ -458,7 +507,13 @@ export function handleFetchResponse(fetchResponse) {
floorResponse = fetchResponse;
}
// Update the global floors object according to the fetched data
_floorsConfig.data = parseFloorData(floorResponse, 'fetch') || _floorsConfig.data;
const fetchData = parseFloorData(floorResponse, 'fetch');
if (fetchData) {
// set .data to it
_floorsConfig.data = fetchData;
// set skipRate override if necessary
_floorsConfig.skipRate = utils.isNumber(fetchData.skipRate) ? fetchData.skipRate : _floorsConfig.skipRate;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We were not honoring fetch level skip rates, which is a bug. Need to get this out asap.

Added a test to confirm.

}

// if any auctions are waiting for fetch to finish, we need to continue them!
resumeDelayedAuctions();
Expand Down
186 changes: 186 additions & 0 deletions test/spec/modules/priceFloors_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,89 @@ describe('the price floors module', function () {
fetchStatus: undefined
});
});
it('should randomly pick a model if floorsSchemaVersion is 2', function () {
let inputFloors = {
...basicFloorConfig,
data: {
floorsSchemaVersion: 2,
currency: 'USD',
modelGroups: [
{
modelVersion: 'model-1',
modelWeight: 10,
schema: {
delimiter: '|',
fields: ['mediaType']
},
values: {
'banner': 1.0,
'*': 2.5
}
}, {
modelVersion: 'model-2',
modelWeight: 40,
schema: {
delimiter: '|',
fields: ['size']
},
values: {
'300x250': 1.0,
'*': 2.5
}
}, {
modelVersion: 'model-3',
modelWeight: 50,
schema: {
delimiter: '|',
fields: ['domain']
},
values: {
'www.prebid.org': 1.0,
'*': 2.5
}
}
]
}
};
handleSetFloorsConfig(inputFloors);

// stub random to give us wanted vals
let randValue;
sandbox.stub(Math, 'random').callsFake(() => randValue);

// 0 - 10 should use first model
randValue = 0.05;
runStandardAuction();
validateBidRequests(true, {
skipped: false,
modelVersion: 'model-1',
location: 'setConfig',
skipRate: 0,
fetchStatus: undefined
});

// 11 - 50 should use second model
randValue = 0.40;
runStandardAuction();
validateBidRequests(true, {
skipped: false,
modelVersion: 'model-2',
location: 'setConfig',
skipRate: 0,
fetchStatus: undefined
});

// 51 - 100 should use third model
randValue = 0.75;
runStandardAuction();
validateBidRequests(true, {
skipped: false,
modelVersion: 'model-3',
location: 'setConfig',
skipRate: 0,
fetchStatus: undefined
});
});
it('should not overwrite previous data object if the new one is bad', function () {
handleSetFloorsConfig({...basicFloorConfig});
handleSetFloorsConfig({
Expand Down Expand Up @@ -547,6 +630,44 @@ describe('the price floors module', function () {
fetchStatus: 'success'
});
});
it('it should correctly overwrite skipRate with fetch skipRate', function () {
// so floors does not skip
sandbox.stub(Math, 'random').callsFake(() => 0.99);
// init the fake server with response stuff
let fetchFloorData = {
...basicFloorData,
modelVersion: 'fetch model name', // change the model name
};
fetchFloorData.skipRate = 95;
fakeFloorProvider.respondWith(JSON.stringify(fetchFloorData));

// run setConfig indicating fetch
handleSetFloorsConfig({...basicFloorConfig, auctionDelay: 250, endpoint: {url: 'http://www.fakeFloorProvider.json'}});

// floor provider should be called
expect(fakeFloorProvider.requests.length).to.equal(1);
expect(fakeFloorProvider.requests[0].url).to.equal('http://www.fakeFloorProvider.json');

// start the auction it should delay and not immediately call `continueAuction`
runStandardAuction();

// exposedAdUnits should be undefined if the auction has not continued
expect(exposedAdUnits).to.be.undefined;

// make the fetch respond
fakeFloorProvider.respond();
expect(exposedAdUnits).to.not.be.undefined;

// the exposedAdUnits should be from the fetch not setConfig level data
// and fetchStatus is success since fetch worked
validateBidRequests(true, {
skipped: false,
modelVersion: 'fetch model name',
location: 'fetch',
skipRate: 95,
fetchStatus: 'success'
});
});
it('Should not break if floor provider returns 404', function () {
// run setConfig indicating fetch
handleSetFloorsConfig({...basicFloorConfig, auctionDelay: 250, endpoint: {url: 'http://www.fakeFloorProvider.json'}});
Expand Down Expand Up @@ -615,6 +736,11 @@ describe('the price floors module', function () {
expect(logErrorSpy.calledOnce).to.equal(true);
});
describe('isFloorsDataValid', function () {
it('should return false if unknown floorsSchemaVersion', function () {
let inputFloorData = utils.deepClone(basicFloorData);
inputFloorData.floorsSchemaVersion = 3;
expect(isFloorsDataValid(inputFloorData)).to.to.equal(false);
});
it('should work correctly for fields array', function () {
let inputFloorData = utils.deepClone(basicFloorData);
expect(isFloorsDataValid(inputFloorData)).to.to.equal(true);
Expand Down Expand Up @@ -670,6 +796,66 @@ describe('the price floors module', function () {
expect(isFloorsDataValid(inputFloorData)).to.to.equal(true);
expect(inputFloorData.values).to.deep.equal({ 'test-div-1|native': 1.0 });
});
it('should work correctly for floorsSchemaVersion 2', function () {
let inputFloorData = {
floorsSchemaVersion: 2,
currency: 'USD',
modelGroups: [
{
modelVersion: 'model-1',
modelWeight: 10,
schema: {
delimiter: '|',
fields: ['mediaType']
},
values: {
'banner': 1.0,
'*': 2.5
}
}, {
modelVersion: 'model-2',
modelWeight: 40,
schema: {
delimiter: '|',
fields: ['size']
},
values: {
'300x250': 1.0,
'*': 2.5
}
}, {
modelVersion: 'model-3',
modelWeight: 50,
schema: {
delimiter: '|',
fields: ['domain']
},
values: {
'www.prebid.org': 1.0,
'*': 2.5
}
}
]
};
expect(isFloorsDataValid(inputFloorData)).to.to.equal(true);

// remove one of the modelWeight's and it should be false
delete inputFloorData.modelGroups[1].modelWeight;
expect(isFloorsDataValid(inputFloorData)).to.to.equal(false);
inputFloorData.modelGroups[1].modelWeight = 40;

// remove values from a model and it should not validate
const tempValues = {...inputFloorData.modelGroups[0].values};
delete inputFloorData.modelGroups[0].values;
expect(isFloorsDataValid(inputFloorData)).to.to.equal(false);
inputFloorData.modelGroups[0].values = tempValues;

// modelGroups should be an array and have at least one entry
delete inputFloorData.modelGroups;
expect(isFloorsDataValid(inputFloorData)).to.to.equal(false);
inputFloorData.modelGroups = [];
expect(isFloorsDataValid(inputFloorData)).to.to.equal(false);
});
});
describe('getFloor', function () {
let bidRequest = {
Expand Down