Prebid Core: Batch Video Cache Requests feature

[jlquaccia]

Type of change

• [x ] Feature

Description of change

This PR provides integration support for videos to optionally be cached in batch requests to the Prebid cache endpoint instead of all video cache requests getting sent individually. This new feature can be enabled by simply including the "batchSize" key within the object passed to the "setConfig" function. Additionally, the "batchTimeout" key can also optionally be set, stating the time in milliseconds that Prebid.js will wait to gather all batch requests before sending them out (default is 0 milliseconds).

Example:

pbjs.setConfig({
  cache: {
    url: 'https://prebid.adnxs.com/pbc/v1/cache',
    batchSize: 2,
    batchTimeout: 20
  }
})

Other information

Related Issue: https://github.com/prebid/Prebid.js/issues/7873 @bretg, could you code review when you have time? Note: I left the "video.html" in the PR in the meantime for visibility around how I was testing out the new integration. I can remove that after CR

[lgtm-com[bot]]

This pull request introduces 3 alerts when merging 4f25515d4da88e4c84144ed383e4a4aeac6feb06 into a231b0da5935b474f5341a9070f7951222f2aeed - view on LGTM.com

new alerts:

• 3 for Superfluous trailing arguments

[patmmccann]

@jlquaccia please note the 3 lgtm alerts

[jlquaccia]

Thanks @patmmccann, I still had a test HTML file that I was using to test out my changes for this issue on the PR. Just removed that file. Looks like that was what had caused the 3 lgtm alerts

[jlquaccia]

Thanks @dgirardi, addressed what you mentioned above and just pushed a few more commits just now. I created a batchRequests.init function that now gets invoked after all batch requests have been processed by the Prebid /cache endpoint which I think should cover your first and last comment up above.

Could you review again when you have time?

[jlquaccia]

hey @dgirardi, thanks for the review! always appreciate your feedback 👍 💯

i went back and updated my PR and tested things locally in the browser and the logic around batch requests appear to look good in the network tab. however, i have one question. even though the new logic seems to be working fine, there are a handful of tests that look to be failing and i can't seem to tell why.

this test breaks and causes others to break after: ✖ should run auction after video bids have been cached AssertionError [ERR_ASSERTION]: 0 == 2

(for a full list of the tests that failed, see this gist i made: https://gist.github.com/jlquaccia/ced04d2f4ead17207d7c822c21053efa)

i reviewed/debugged the new code in my PR several times but wasn't able to find the root cause of the issue. i think i might just need an extra set of eyes haha. do you have any idea what might be causing the tests to break? Well anyway, know it's Friday night, have a great weekend!

[jlquaccia]

@dgirardi thank you for all the feedback! Left a few comments above

[spormeon]

this seems to work well for us, only thing i've spotted is if you set batchTimeout: 3000 as a test, then nothing gets sent on the "video modules" params. Is targeting being sent before its waited 3000? no idea what/ if there is a "cut off" time?

[spormeon]

having just tested this some more and got more bids coming in, its actually creating a cache item for each bidder in effect, so if there are 5 bidders, making 3 bids each, its creating 5 cache batches, with 5 UUID's in each. Is this expected behaviour? as this could still go over the 3 secs total, if got e.g 20 bidders and each cache item takes 200ms = 4000ms, over the allowed 3secs, so some will "error" out