-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Add 'hb_cache_host' targeting for video bids when cache is set #3654
Add 'hb_cache_host' targeting for video bids when cache is set #3654
Conversation
…ebid.js into master-rubicon-clean
# Conflicts: # modules/advangelistsBidAdapter.js # test/spec/modules/advangelistsBidAdapter_spec.js
…ter-remote-updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a question - is the hb_cache_path
currently in-use in the redirector service? Or is this an anticipated future need?
I looked through the other repo and the examples in the main README just refer to the hb_uuid
and hb_cache_host
keys.
hb_cache_path isn't needed right now because it's built into the redirector config. we added it to be orthogonal, but it can be removed. |
@bretg Thanks for the clarification; that's fine. The overall changes here LGTM. Is there anything we'd need to update in the prebid docs in lieu of this change? It seems like it'd be handled by any docs focused on the redirector service, but I wasn't sure if there would be anything else to note base-line. I was potentially thinking around the |
Good call @jsnellbaker. However, I'd like these targeting values to be set even if the user sets
Will open review comments on that. BTW - @mkendall07 and I are starting to discuss whether the system should be setting the standard targeting variables even if the page supplies |
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest moving this block out of the check for bidderSettings.standard.adServerTargeting
so that if a user does set that value, hb_cache_host is set anyhow. Probably the right approach is to check:
- when getConfig('cache.url') is set
-- ifhb_cache_host
is not already set by the user, set it
And while we're at it, let's drop hb_cache_path. I should be consistent in my desire to minimize our targeting footprint, so if we don't need hb_cache_path, let's not set it.
…into add-video-cache-targeting-host-path
…into add-video-cache-targeting-host-path
…t so it will be defined even if adserverTargeting was defined, removed hb_cache_path.
Ready for re-review, |
Ok, so what this code says is:
I like it. But I can see how others might not like the added complexity of what was formerly a simple rule. However, in talking to @mkendall07, I believe this is the direction we should be going -- other attributes will follow these in future releases. Matt, if you approve this, I'll write up a note on the Pub API page. |
Thanks for the note. I do think it's the right direction, however, I think we need a way for a pub to opt out of all these additional keys. For example, no one is using |
@mkendall07 - good point. We've already documented bidderSettings.sendStandardTargeting. It defaults to true. So this code needs to be updated to not add the variables if bidderSettings.sendStandardTargeting is false. Sorry for the back and forth here @idettman. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just putting a block on this to prevent accidental merges until it can be updated.
Will add logic to not add the variables if |
…com/rubicon-project/Prebid.js; branch 'master' of https://github.com/prebid/Prebid.js into add-video-cache-targeting-host-path
Added logic to not add |
…ve add-video-cache-targeting-host-path value
Docs PR prebid/prebid.github.io#1238 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Confirmed with Kendall that we're good to go here. Merging. |
…d#3654) * Add microadBidAdapter * Remove unnecessary encodeURIComponent from microadBidAdapter * Submit Advangelists Prebid Adapter * Submit Advangelists Prebid Adapter 1.1 * Correct procudtion endpoint for prebid * add 'hb_cache_host' and 'hb_cache_path' targeting for video bids using cache * update with requested changes from pull request, changed hb_cache_host so it will be defined even if adserverTargeting was defined, removed hb_cache_path. * update to not add hb_cache_host targeting if sendStandardTargeting is false * update condition logic to add hb_cache_host if bidderCode does not have add-video-cache-targeting-host-path value
Type of change
Motivation
DFP imposes a limit on publishers entering in a video VAST tag URL... the host cannot be a variable.
Given that macros are possible, supplying the
host
andpath
on the ad request would solve this, but this is impossible in DFP, as %% macros cannot be supplied in the host name.The solution we chose is for Prebid.org, was to host a cache redirector service. The video creative would point to this service, which then redirects the player to where the cached asset actually resides, and the publisher would be able to enter a constant VAST tag URL like:
https://cache.prebid.org/redir?uuid=%%PATTERN:hb_uuid%%&host=%%PATTERN:hb_cache_host%%
The Cache Redirector Service at
cache.prebid.org
looks at thehb_cache_host
variable and redirects the video player to the appropriate place, e.g. prebid.adnxs.com/pbc/v1/cache or prebid-server.rubiconproject.com/cache.To support separate line items for each bidder, the VAST tag URL could contain the bidder-specific key-value-pairs:
https://cache.prebid.org/redir?uuid=%%PATTERN:hb_uuid_rubicon%%&host=%%PATTERN:hb_cache_host_rubicon%%
Description of change
If
mediaType
isvideo
, and the config hascache.url
defined, everybidResponse
object will now containhb_cache_host
in theadServerTargeting
.If an adapter had previously specified
hb_cache_host
in it'sbidResponseObject
, that value is used, instead of overriding with the client sidecache.url
.This addressees the scenario where some bidders use a server side cache and others use client side cache.
Handling invalid cache url values
hb_cache_host
is parsed from the config propertycache.url
(if it's defined). If thecache.url
value is not a valid url, bothhb_cache_id
andhb_uuid
will beundefined
, and PBJS tries to cache on the random string and fails, logging the following warning:WARNING: Failed to save to the video cache: Error: Error storing video ad in the cache: : {}. Video bid must be discarded.
Example
Related