Add support for instream video ads bidsWon tracking

[monis0395]

Type of change

• Feature
• Bug Fix

Description of change

• Add support for optional instream video ads tracking module

Problem:

• Inability to track BID WON or rendered for events of in-stream inventories by client analytics adapters

Possible Solution for #4768, #5308 :

Since we know for sure every type of bid response will have videoCacheKey and will be loaded by VideoPlayer. We can use the Performance API to keep track of any resource fired with a video cache key.

[monis0395]

@bretg @Fawke @jaiminpanchal27 can you please review this PR and let me know your thoughts ?

[pm-harshad-mane]

@mike-chowla what do you think about this approach?
As mentioned in code.

/**
 * Here the idea is
 * find all network entries via performance.getEntriesByType()
 * filter it by video cache key in the url
 * and exclude the ad server urls so that we dont match twice
 * eg:
 * dfp ads call: https://securepubads.g.doubleclick.net/gampad/ads?...hb_cache_id%3D55e85cd3-6ea4-4469-b890-84241816b131%26...
 * prebid cache url: https://prebid.adnxs.com/pbc/v1/cache?uuid=55e85cd3-6ea4-4469-b890-84241816b131
 *
 * if the entry exists, emit the BID_WON

[pm-harshad-mane]

@monisq can you make the changes to core for the (suggested in review) so that BID_WON event will be available for all for instream video bids.

[pm-harshad-mane]

Hello @monisq ,
Can you please work on the above comments? This will help the community.

[monis0395]

Hello @monisq ,
Can you please work on the above comments? This will help the community.

Yes, I am already working on it! would be pushing the changes by 2mrow!

[pm-harshad-mane]

Thank you @monis0395 👍

[monis0395]

@pm-harshad-mane I have done the changes in the core, can please review them?

Till then, I am trying figuring out, how to write unit tests for instream tracking (facing some issues while mocking and stubing), will push those changes as soon as done

[pm-harshad-mane]

Thank you @monis0395 !!

[monis0395]

@pm-harshad-mane I have added unit test cases for instream tracking.

Do you think we should change the title or PR as its a little different from initial PR ?

[pm-harshad-mane]

@monis0395
If the PR does not have medianetAnalyticsAdater related change, can we rename it?

@jsnellbaker
This approach looks good to me.
Can you please review it as well?

[monis0395]

@monis0395
If the PR does not have medianetAnalyticsAdater related change, can we rename it?

Done!

[pm-harshad-mane]

LGTM.
Requesting second review from @jsnellbaker .

[jsnellbaker]

Hi @monis0395

I had a few questions. When you have the chance, can you please take a look at them?

Additionally, I'm going to tag some others to review this PR as well. There was something similar to this effort (properly tracking winning instream bids) before and there were some concerns raised at the time. I don't recall the specifics currently, I will try to dig up the PR in question.

Thanks.

[bretg]

Pass entire creative cache url as a targeting key to the ad server

1. ad servers like GAM don't support taking a URL as the VAST URL param
2. We get flak for how many chars we send to the ad server already

Enforce 100% predid server caching

We're not in a position to 'enforce' much. This feature has to be considered optional.

should we rely on the publisher to give the correct pattern

It could be documented. e.g. "if you use spotx, add ____ to the regex."

Can we get on call/sync outside this PR to discuss this further ?

Are you with a Prebid.org member company? The Prebid.js or Video Taskforce committee meetings would be good forums.

At a high level I view this as a fine optional feature, but bidsWon for video is a metric of questionable value IMO -- kind of a "poor-man's" metric since advertisers pay for video on impression and unlike banner, there could be a large time difference in video between ad call and actual display.

Rubicon has already built a video impression-tracking feature that's working well. And tracking for App and AMP. See these references:

• https://docs.google.com/document/d/1ry0X4C2EV-R0pMrm1IQk9BstxaT395UCl3KKqTGa5c8/edit#heading=h.7w5yevygp2gz
• Prebid Server Event Updates prebid-server#1202
• Prebid.js support for tracking Prebid Server events #5180

The issue has been that PBS-Go doesn't support events (for video specifically, the /vtrack endpoint). The good news here is that we may have some community support for porting that feature from PBS-Java to PBS-Go.

[pm-harshad-mane]

@monis0395 , can you please move this code to a separate module and provide some documentation?

[bretg]

If this is broken out into a separate module I'll approve it. But the documentation has to mention the scenario where this method of counting could break down, basically unexpected URLs carrying the cache ID.

[monis0395]

Hi @bretg @pm-harshad-mane,

Thanks for waiting, I have done the suggested code changes!

I have started working on prebid.github.io doc changes for this PR, would be pushing those in a day or two.

Thanks!

[pm-harshad-mane]

Great @monis0395 !! Please mention the documentation PR here when you are ready.
We are excited to have this feature available!

[monis0395]

Hey @bretg @pm-harshad-mane ,

Here is the documentation PR : prebid/prebid.github.io#2277

[patmmccann]

qq: should this affect https://github.com/prebid/Prebid.js/blob/master/src/prebid.js#L764 ; which seems to mark winning bids as used too aggressively ?

[monis0395]

qq: should this affect https://github.com/prebid/Prebid.js/blob/master/src/prebid.js#L764 ; which seems to mark winning bids as used too aggressively ?

As such this PR wont affect $$PREBID_GLOBAL$$.markWinningBidAsUsed, but let the reviewers confirm.

[pm-harshad-mane]

@jsnellbaker @idettman could you please review the changes?

[monis0395]

Hi @jsnellbaker @bretg,

Did you get a chance to look at it?

[monis0395]

If this is broken out into a separate module I'll approve it. But the documentation has to mention the scenario where this method of counting could break down, basically unexpected URLs carrying the cache ID.

Hi @bretg, as you have suggested. I have already broken down in a separate module.. The docs PR is also approved by you prebid/prebid.github.io#2277

Let me know if anything else needs to be done!

Thanks!

[pm-harshad-mane]

Hello @bretg ,
We are waiting for you.

[bretg]

My review is merely for positioning and user-facing concerns. Since we have two code reviews done already, I've removed Isaac from the list.

My one question about merging is whether this will be in 4.8 or if that train has already left.

[pm-harshad-mane]

Thank you @bretg , I have merged it to master, so it will be available in 4.8, doc PR need to be merged post 4.8 release.

[pm-harshad-mane]

Thank you @monisq for your efforts and patience. 👍

[monis0395]

Thanks @pm-harshad-mane 👍