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.

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

Adpod module #3511

Merged
merged 24 commits into from
Feb 27, 2019
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
2896f90
initial version of adpod module
jsnellbaker Jan 30, 2019
01369b5
fix hooks, clean-up and uncomment unit tests
jsnellbaker Jan 31, 2019
99fdaae
correct issues in unit tests
jsnellbaker Jan 31, 2019
0d7d2af
add missing custom key fields in fake bids for unit tests
jsnellbaker Jan 31, 2019
dcd4056
several updates to module and unit tests
jsnellbaker Feb 4, 2019
c661a7a
update logic to only add hooks once
jsnellbaker Feb 4, 2019
9c7d7e0
update targeting keys to new name
jsnellbaker Feb 5, 2019
1a6cd39
add module description
jsnellbaker Feb 5, 2019
2b7158f
fix typo
jsnellbaker Feb 5, 2019
44eb4ec
add adunit check in case of multi-format
jsnellbaker Feb 5, 2019
7380f08
add exports for adpod targeting keys
jsnellbaker Feb 5, 2019
db14fc6
move ADPOD constant to mediaTypes.js
jsnellbaker Feb 6, 2019
69bc093
undo mediaTypes change
jsnellbaker Feb 6, 2019
9860a56
Merge branch 'master' into adpod_module
jsnellbaker Feb 6, 2019
9131554
Merge branch 'master' into adpod_module
jsnellbaker Feb 6, 2019
f655c5c
fix unit tests
jsnellbaker Feb 6, 2019
92823cc
updates based on initial feedback
jsnellbaker Feb 6, 2019
b74d7a3
add msg when PBC rejects bid due to duplicate key
jsnellbaker Feb 7, 2019
de58a2b
fix issue in videoBidCheck hook
jsnellbaker Feb 8, 2019
ad1c678
add buffer logic when rounding bidDuration
jsnellbaker Feb 20, 2019
1e1e49b
add logic for deferCaching
jsnellbaker Feb 25, 2019
b60cea7
update logic around brandCategoryExclusion
jsnellbaker Feb 26, 2019
9728f2f
update support to new hook api and update logic when checking iab cat…
jsnellbaker Feb 27, 2019
6dfc923
merge branch 'master' to branch adpod_module + resolve conflicts
jsnellbaker Feb 27, 2019
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
364 changes: 364 additions & 0 deletions modules/adpod.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,364 @@
/**
* This module houses the functionality to evaluate and process adpod adunits/bids. Specifically there are several hooked functions,
* that either supplement the base function (ie to check something additional unique to adpod objects) or to replace the base funtion
* entirely when appropriate.
*
* Brief outline of each hook:
* - `callPrebidCacheHook` - for any adpod bids, this function will temporarily hold them in a queue in order to send the bids to Prebid Cache in bulk
* - `checkAdUnitSetupHook` - evaluates the adUnits to ensure that required fields for adpod adUnits are present. Invalid adpod adUntis are removed from the array.
* - `checkVideoBidSetupHook` - evaluates the adpod bid returned from an adaptor/bidder to ensure required fields are populated; also initializes duration bucket field.
*
* To initialize the module, there is an `initAdpodHooks()` function that should be imported and executed by a corresponding `...AdServerVideo`
* module that designed to support adpod video type ads. This import process allows this module to effectively act as a sub-module.
*/

import * as utils from '../src/utils';
import { addBidToAuction, doCallbacksIfTimedout, AUCTION_IN_PROGRESS } from '../src/auction';
import { store } from '../src/videoCache';
import { hooks } from '../src/hook';
import { config } from '../src/config';
import Set from 'core-js/library/fn/set';
import find from 'core-js/library/fn/array/find';
const from = require('core-js/library/fn/array/from');

export const ADPOD = 'adpod';
export const TARGETING_KEY_PB_CAT_DUR = 'hb_pb_cat_dur';
Copy link
Member

Choose a reason for hiding this comment

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

kind of late but are we happy with this name? :)

export const TARGETING_KEY_CACHE_ID = 'hb_cache_id'

// NOTE - are these good defaults?
let queueTimeDelay = 50;
Copy link
Member

Choose a reason for hiding this comment

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

any experimentation with different default values here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did test using different values (went to 500) and with the timers I setup at different points in this module - it was executing the call to Prebid Cache at the set time value when there was a sufficient gap in bids.

let queueSizeLimit = 5;
let bidCacheRegistry = createBidCacheRegistry();

/**
* Create a registry object that stores/manages bids while be held in queue for Prebid Cache.
* @returns registry object with defined accessor functions
*/
function createBidCacheRegistry() {
let registry = {};
return {
addBid: function (bid) {
// create parent level object based on auction ID (in case there are concurrent auctions running) to store objects for that auction
if (!registry[bid.auctionId]) {
registry[bid.auctionId] = {};
registry[bid.auctionId].bidStorage = new Set();
Copy link
Member

Choose a reason for hiding this comment

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

nice use of a new data type!

registry[bid.auctionId].queueDispatcher = createDispatcher(queueTimeDelay);
registry[bid.auctionId].initialCacheKey = utils.generateUUID();
}
registry[bid.auctionId].bidStorage.add(bid);
},
removeBid: function (bid) {
registry[bid.auctionId].bidStorage.delete(bid);
},
getBids: function (bid) {
return registry[bid.auctionId] && registry[bid.auctionId].bidStorage.values();
},
getQueueDispatcher: function(bid) {
return registry[bid.auctionId] && registry[bid.auctionId].queueDispatcher;
},
getInitialCacheKey: function(bid) {
return registry[bid.auctionId] && registry[bid.auctionId].initialCacheKey;
}
}
}

/**
* Creates a function that when called updates the bid queue and extends the running timer (when called subsequently).
* Once the threshold for the queue (defined by queueSizeLimit), the queue will be flushed by calling the `firePrebidCacheCall` function.
* If there is a long enough time between calls (based on timeoutDration), the queue will automatically flush itself.
* @param {Number} timeoutDuration number of milliseconds to pass before timer expires and current bid queue is flushed
* @returns {Function}
*/
function createDispatcher(timeoutDuration) {
let timeout;
let counter = 1;

return function(auctionInstance, bidListArr, afterBidAdded, killQueue) {
const context = this;

var callbackFn = function() {
firePrebidCacheCall.call(context, auctionInstance, bidListArr, afterBidAdded);
};

clearTimeout(timeout);

if (!killQueue) {
// want to fire off the queue if either: size limit is reached or time has passed since last call to dispatcher
if (counter === queueSizeLimit) {
counter = 1;
callbackFn();
} else {
counter++;
timeout = setTimeout(callbackFn, timeoutDuration);
}
} else {
counter = 1;
}
};
}

/**
* This function reads certain fields from the bid to generate a specific key used for caching the bid in Prebid Cache
* @param {Object} bid bid object to update
*/
function attachPriceIndustryDurationKeyToBid(bid) {
let category = bid.meta && bid.meta.adServerCatId;
let duration = bid.video && bid.video.durationBucket;
let cpmFixed = bid.cpm.toFixed(2);
let initialCacheKey = bidCacheRegistry.getInitialCacheKey(bid);
// TODO? - add check to verify all above values are populated and throw error if not?
let pcd = `${cpmFixed}_${category}_${duration}s`;

if (!bid.adserverTargeting) {
bid.adserverTargeting = {};
}
bid.adserverTargeting[TARGETING_KEY_PB_CAT_DUR] = pcd;
bid.adserverTargeting[TARGETING_KEY_CACHE_ID] = initialCacheKey;
bid.customCacheKey = `${pcd}_${initialCacheKey}`;
}

/**
* Updates the running queue for the associated auction.
* Does a check to ensure the auction is still running; if it's not - the previously running queue is killed.
* @param {*} auctionInstance running context of the auction
* @param {Object} bidResponse bid object being added to queue
* @param {Function} afterBidAdded callback function used when Prebid Cache responds
*/
function updateBidQueue(auctionInstance, bidResponse, afterBidAdded) {
let bidListIter = bidCacheRegistry.getBids(bidResponse);

if (bidListIter) {
let bidListArr = from(bidListIter);
let callDispatcher = bidCacheRegistry.getQueueDispatcher(bidResponse);
let killQueue = !!(auctionInstance.getAuctionStatus() !== AUCTION_IN_PROGRESS);
callDispatcher(auctionInstance, bidListArr, afterBidAdded, killQueue);
} else {
utils.logWarn('Attempted to cache a bid from an unknown auction. Bid:', bidResponse);
}
}

/**
* Small helper function to remove bids from internal storage; normally b/c they're about to sent to Prebid Cache for processing.
* @param {Array[Object]} bidResponses list of bids to remove
*/
function removeBidsFromStorage(bidResponses) {
for (let i = 0; i < bidResponses.length; i++) {
bidCacheRegistry.removeBid(bidResponses[i]);
}
}

/**
*
* @param {*} auctionInstance running context of the auction
* @param {Array[Object]} bidList list of bid objects that need to be sent to Prebid Cache
* @param {Function} afterBidAdded callback function used when Prebid Cache responds
*/
function firePrebidCacheCall(auctionInstance, bidList, afterBidAdded) {
// note to reviewers - was doing this to ensure the variable wouldn't be malformed by the additional incoming bids
// while the current list is being processed by PBC and the store callback
// ...but is this copy really needed?
let bidListCopy = bidList.slice();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can be removed. No need to copy


// remove entries now so other incoming bids won't accidentally have a stale version of the list while PBC is processing the current submitted list
removeBidsFromStorage(bidListCopy);

store(bidListCopy, function (error, cacheIds) {
if (error) {
utils.logWarn(`Failed to save to the video cache: ${error}. Video bid(s) must be discarded.`);
for (let i = 0; i < bidList.length; i++) {
doCallbacksIfTimedout(auctionInstance, bidList[i]);
}
} else {
for (let i = 0; i < cacheIds.length; i++) {
// when uuid in response is empty string then the key already existed, so this bid wasn't cached
// TODO? - should we throw warning here? should we call the afterBidAdded() anyway to decrement the internal bid counter?
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should call afterBidAdded to decrement the counter because there can be a case where auction is not timedout and one of the bid is back with empty uuid. In this case we would like to close the auction and not wait for timer to expire

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To confirm on this point - should we throw any type of message that the bid wasn't cached b/c it had a duplicate key? Or just let it go silently?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yep logMessage is good

// TODO? - verify the cacheKey is one of the expected values (eg in case PBC returns generic UUID)?
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can ignore this check

if (cacheIds[i].uuid !== '') {
addBidToAuction(auctionInstance, bidListCopy[i]);
afterBidAdded();
}
}
}
});
}

/**
* This is the main hook function to handle adpod bids; maintains the logic to temporarily hold bids in a queue in order to send bulk requests to Prebid Cache.
* @param {Function} fn reference to original function (used by hook logic)
* @param {*} auctionInstance running context of the auction
* @param {Object} bidResponse incoming bid; if adpod, will be processed through hook function. If not adpod, returns to original function.
* @param {Function} afterBidAdded callback function used when Prebid Cache responds
* @param {Object} bidderRequest copy of bid's associated bidderRequest object
*/
export function callPrebidCacheHook(fn, auctionInstance, bidResponse, afterBidAdded, bidderRequest) {
let videoConfig = bidderRequest.mediaTypes && bidderRequest.mediaTypes.video;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion: utils.deepAccess can be used here. deepAccess(bidderRequest, 'mediaTypes.video.context')

if (videoConfig && videoConfig.context === ADPOD) {
bidCacheRegistry.addBid(bidResponse);
attachPriceIndustryDurationKeyToBid(bidResponse);

updateBidQueue(auctionInstance, bidResponse, afterBidAdded);
} else {
fn.call(this, auctionInstance, bidResponse, afterBidAdded, bidderRequest);
}
}

/**
* This hook function will review the adUnit setup and verify certain required are present in any adpod adUnits.
* If the fields are missing or incorrectly setup, the adUnit is removed from the list.
* @param {Function} fn reference to original function (used by hook logic)
* @param {Array[Object]} adUnits list of adUnits to be evaluated
* @returns {Array[Object]} list of adUnits that passed the check
*/
export function checkAdUnitSetupHook(fn, adUnits) {
let goodAdUnits = adUnits.filter(adUnit => {
let mediaTypes = adUnit.mediaTypes;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion: utils.deepAccess can be used here. deepAccess(adUnit, 'mediaTypes.video.context')

let videoConfig = mediaTypes && mediaTypes.video;
if (videoConfig && videoConfig.context === ADPOD) {
// run check to see if other mediaTypes are defined (ie multi-format); reject adUnit if so
if (Object.keys(mediaTypes).length > 1) {
utils.logWarn(`Detected more than one mediaType in adUnitCode: ${adUnit.code} while attempting to define an 'adpod' video adUnit. 'adpod' adUnits cannot be mixed with other mediaTypes. This adUnit will be removed from the auction.`);
return false;
}

let errMsg = `Detected missing or incorrectly setup fields for an adpod adUnit. Please review the following fields of adUnitCode: ${adUnit.code}. This adUnit will be removed from the auction.`;

let playerSize = !!(videoConfig.playerSize && utils.isArrayOfNums(videoConfig.playerSize));
let adPodDurationSec = !!(videoConfig.adPodDurationSec && utils.isNumber(videoConfig.adPodDurationSec));
let durationRangeSec = !!(videoConfig.durationRangeSec && utils.isArrayOfNums(videoConfig.durationRangeSec));

if (!playerSize || !adPodDurationSec || !durationRangeSec) {
errMsg += (!playerSize) ? '\nmediaTypes.video.playerSize' : '';
errMsg += (!adPodDurationSec) ? '\nmediaTypes.video.adPodDurationSec' : '';
errMsg += (!durationRangeSec) ? '\nmediaTypes.video.durationRangeSec' : '';
utils.logWarn(errMsg);
return false;
}
}
return true;
});
adUnits = goodAdUnits;
fn.call(this, adUnits);
}

/**
* This check evaluates the incoming bid's `video.durationSeconds` field and tests it against specific logic depending on adUnit config. Summary of logic below:
* when adUnit.mediaTypes.video.requireExactDuration is true
* - only bids that exactly match those listed values are accepted (don't round at all).
* - populate the `bid.video.durationBucket` field with the matching duration value
* when adUnit.mediaTypes.video.requireExactDuration is false
* - round the duration to the next highest specified duration value based on adunit (eg if range was [5, 15, 30] -> 3s is rounded to 5s; 16s is rounded to 30s)
Copy link
Member

Choose a reason for hiding this comment

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

we wanted to add a 2 second buffer here. That is if a bid is 16s it should be rounded down to the the nearest bucket (15s in this case) because it is within 2s of a bucket.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll make an update here to incorporate this logic. Should this buffer apply to the highest range? ie if the ranges were [15, 30, 45, 60] and the bid was 61 - we would round it down to 60 and accept it?

Copy link
Member

Choose a reason for hiding this comment

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

yeah I think we can apply it to the highest value as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I updated the logic to account for this buffer; please take another look when you have the chance.

* - if the bid is above the range of thje listed durations, reject the bid
* - set the rounded duration value in the `bid.video.durationBucket` field for accepted bids
* @param {Object} bidderRequest copy of the bidderRequest object associated to bidResponse
* @param {Object} bidResponse incoming bidResponse being evaluated by bidderFactory
* @returns {boolean} return false if bid duration is deemed invalid as per adUnit configuration; return true if fine
*/
function checkBidDuration(bidderRequest, bidResponse) {
let bidDuration = bidResponse.video && bidResponse.video.durationSeconds;
let videoConfig = bidderRequest.mediaTypes && bidderRequest.mediaTypes.video;
let adUnitRanges = videoConfig.durationRangeSec;
adUnitRanges.sort((a, b) => a - b); // ensure the ranges are sorted in numeric order

if (!videoConfig.requireExactDuration) {
let max = Math.max(...adUnitRanges);
if (bidDuration <= max) {
let nextHighestRange = find(adUnitRanges, range => range >= bidDuration);
bidResponse.video.durationBucket = nextHighestRange;
} else {
utils.logWarn(`Detected a bid with a duration value outside the accepted ranges specified in adUnit.mediaTypes.video.durationRangeSec. Rejecting bid: `, bidResponse);
return false;
}
} else {
if (find(adUnitRanges, range => range === bidDuration)) {
bidResponse.video.durationBucket = bidDuration;
} else {
utils.logWarn(`Detected a bid with a duration value not part of the list of accepted ranges specified in adUnit.mediaTypes.video.durationRangeSec. Exact match durations must be used for this adUnit. Rejecting bid: `, bidResponse);
return false;
}
}
return true;
}

/**
* This hooked function evaluates an adpod bid and determines if the required fields are present.
* If it's found to not be an adpod bid, it return to original function via hook logic
* @param {Function} fn reference to original function (used by hook logic)
* @param {Object} bid incoming bid object
* @param {Object} bidRequest bidRequest object of associated bid
* @param {Object} videoMediaType copy of the `bidRequest.mediaTypes.video` object; used in original function
* @param {String} context value of the `bidRequest.mediaTypes.video.context` field; used in original function
* @returns {boolean} this return is only used for adpod bids
*/
export function checkVideoBidSetupHook(fn, bid, bidRequest, videoMediaType, context) {
if (context === ADPOD) {
if (!utils.deepAccess(bid, 'meta.iabSubCatId')) {
return false;
}

if (utils.deepAccess(bid, 'video')) {
if (!utils.deepAccess(bid, 'video.context') || bid.video.context !== ADPOD) {
return false;
}

if (!utils.deepAccess(bid, 'video.durationSeconds') || bid.video.durationSeconds <= 0) {
return false;
} else {
let isBidGood = checkBidDuration(bidRequest, bid);
if (!isBidGood) return false;
}
}

if (!config.getConfig('cache.url') && bid.vastXml && !bid.vastUrl) {
utils.logError(`
This bid contains only vastXml and will not work when a prebid cache url is not specified.
Try enabling prebid cache with pbjs.setConfig({ cache: {url: "..."} });
`);
return false;
};

return true;
} else {
fn.call(this, bid, bidRequest, videoMediaType, context);
}
}

/**
* This function reads the (optional) settings for the adpod as set from the setConfig()
* @param {Object} config contains the config settings for adpod module
*/
export function adpodSetConfig(config) {
if (config.bidQueueTimeDelay !== undefined) {
if (typeof config.bidQueueTimeDelay === 'number' && config.bidQueueTimeDelay > 0) {
// add check to see if config setting is too high?
queueTimeDelay = config.bidQueueTimeDelay;
} else {
utils.logWarn(`Detected invalid value for adpod.bidQueueTimeDelay in setConfig; must be a positive number. Using default: ${queueTimeDelay}`)
}
}

if (config.bidQueueSizeLimit !== undefined) {
if (typeof config.bidQueueSizeLimit === 'number' && config.bidQueueSizeLimit > 0) {
// add check to see if config setting is too high? too low?
queueSizeLimit = config.bidQueueSizeLimit;
} else {
utils.logWarn(`Detected invalid value for adpod.bidQueueSizeLimit in setConfig; must be a positive number. Using default: ${queueSizeLimit}`)
}
}
}
config.getConfig('adpod', config => adpodSetConfig(config.adpod));

/**
* This function initializes the adpod module's hooks. This is called by the corresponding adserver video module.
*/
export function initAdpodHooks() {
function setupHookFnOnce(hookId, hookFn, priority = 15) {
let result = hooks[hookId].getHooks({hook: hookFn});
Copy link
Collaborator

Choose a reason for hiding this comment

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

What if some other module has added hook on checkAdUnitSetup and it is added first. Will these hooks be added since your are comparing the length to 0 ?

Copy link
Collaborator Author

@jsnellbaker jsnellbaker Feb 6, 2019

Choose a reason for hiding this comment

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

So they would be added to the hooks['checkAdUnitSetup'] object, but they wouldn't show when I execute the getHooks() call because I'm effectively filtering that get to only look for the function that's inside this module.

I tested this out locally and even when I name the hook function the same (ie checkAdUnitSetupHook) - the hooked function won't return with this type of call because the function signature is different.

if (result.length === 0) {
hooks[hookId].before(hookFn, priority);
}
}

setupHookFnOnce('callPrebidCache', callPrebidCacheHook);
setupHookFnOnce('checkAdUnitSetup', checkAdUnitSetupHook);
setupHookFnOnce('checkVideoBidSetup', checkVideoBidSetupHook);
}
Loading