-
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
Prebid Server adapter: fledge support #9342
Conversation
…fig} to {bidId, config: fledgeAuctionConfig}
@@ -35,3 +42,166 @@ describe('fledgeForGpt module', function() { | |||
}); |
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.
prob does not matter but this is still calling bidRequest
instead of adUnitCode
for addComponentAuctionHook
Line 40 in fledge_spec
@laurb9 - mind giving this a review? |
@dgirardi I tested this PR with openx adapter, both direct and via prebid-server (both varieties), and it is working for me. Setup: Result: |
I've also tested on a stable Chrome without fledge support, and I see that
But the fledge configs coming from adapters still attempt to register the component auction, as the original code made no attempt to filter them either. I see it as more of a consistency issue now, as nothing will happen either way - or we could disable the module altogether so no fledge processing takes place. |
@@ -31,12 +32,10 @@ export function init(cfg) { | |||
} | |||
} | |||
|
|||
export function addComponentAuctionHook(next, bidRequest, componentAuctionConfig) { | |||
export function addComponentAuctionHook(next, adUnitCode, componentAuctionConfig) { |
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.
Would a hook not benefit more from seeing the original bidRequest ?
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.
possibly, but we don't always have it. In prebid server we may have multiple requests (the fledge config references an imp, but there can be any number of bidders (requests) for that imp). With allowUnknownBidderCodes
we may also have zero.
Co-authored-by: Laurentiu Badea <laurb9@users.noreply.github.com>
@laurb9 how do things look now? |
modules/fledgeForGpt.js
Outdated
} | ||
|
||
export function markBidsForFledge(next, bidderRequests) { |
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.
This function enables FLEDGE for bidderRequests, should the name be markBidderRequestsForFledge
or just markForFledge
?
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.
Looks good to me and it works. After playing with it I noticed a couple new and inherited behaviors that don't impact functionality but can be misinterpreted, so I added a few comments.
Also, the log message fledgeForGpt isEnabled: true
is emitted on a non-FLEDGE supporting browser, with no indication that we know it will not work and that imp.ext.ae
is removed from explicit pub-supplied configuration. I've come to think it is more helpful to see the final computed state of the enablement that considers the browser support, perhaps with some detail about how it was derived.
(ortbResponse.ext?.prebid?.fledge?.auctionconfigs || []).forEach((cfg) => { | ||
const impCtx = context.impContext[cfg.impid]; | ||
if (!impCtx?.imp?.ext?.ae) { | ||
logWarn('Received fledge auction configuration for an impression that was not in the request or did not ask for it', cfg, impCtx?.imp); |
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.
validation behavior is different between PBS and regular adapter. If adapters adopt this method it may become a non-issue but it may be confusing as is.
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 am not sure this validation makes sense outside of here. There are two things we are checking here -
- the request should have set
imp.ext.ae
; does this check make sense for an adapter that (in general) may not be using ORTB at all? - the fledge config should refer to an imp that existed in the request. this is necessary for PBS because the protocol pairs a fledge config with
impid
. But, for client-side adapters, they must provide a validbidId
instead - the check is here, although there's no warning.
I can add a warning for 2, I'm not sure about 1.
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.
Should be fine to leave it as it is now and update in a later iteration if it becomes a problem with usage.
@@ -48,5 +47,50 @@ export function addComponentAuctionHook(next, bidRequest, componentAuctionConfig | |||
logWarn(MODULE, `unable to register component auction config for: ${adUnitCode} x ${seller}.`); |
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.
Should addComponentAuction
be skipped altogether if FLEDGE is not supported ?
On such a browser, receiving unsolicited auction configs from an adapter will print this misleading under the circumstances warning.
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.
why is the warning misleading? to me it makes perfect sense: if your browser does not support flegde, bidders should not be trying to run fledge auctions and I'd want to be warned if they do.
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.
The message "unable to do something" tells me the expectation was that it should have been able to, so something went wrong.
If it said "ignored unsolicited fledge configs", that would be more accurate, but I understand we may not want to have a log message for every specific condition
* upstream/master: Prebid Server adapter: fledge support (prebid#9342) Taboola Bid Adapter: resolve AUCTION_PRICE macro (prebid#9735) Revert "IntentIQ Analytics Module : initial release (prebid#9322)" (prebid#9734) IntentIQ Analytics Module : initial release (prebid#9322) Increment version to 7.44.0-pre Prebid 7.43.0 release Core: allow restriction of cookies / localStorage through `bidderSettings.*.storageAllowed` (prebid#9660) Add 9 Dots Media alias (prebid#9699) Smaato: Adapters that accept geolocation data from bid parameters should also accept it from ortb2.(device|user).geo (prebid#9676) (prebid#9725) Adloox AdServer Video : lengthen test timeouts (prebid#9728) RTBHouse Bid Adapter: change `source.tid` to contain `auctionId` and populate imp-level `ext.tid` (prebid#9726) chore: update `getAudiencesAsBidderOrtb2` implementation and test (prebid#9720) CORE: allow to disable setting the pbjs global variable (prebid#9568) Display.io Bid Adapter: ad request parameters renaming, user session saving (prebid#9553) CORE: add bid to winningBids when marking as used (prebid#9612) Concert Bid Adapter: enable support for GPP consent and remove user sync (prebid#9700) Sonobi Bid Adapter: add IntentIq Id (prebid#9649) added fix and support for multibid module (prebid#9602) Update Permutive RTD documentation (prebid#9697) Adkernel Bid Adapter: add adlive.io alias (prebid#9714) FPD Enrichment: use low entropy method by default to fetch user agent data (prebid#9711) GumGum Bid Adapter : send gpp data in bidrequest (prebid#9707) Adagio Bid Adapter : add new params `splitKeyword` and `dl` to bidRequest payload (prebid#9694) fix lint and test failures Increment version to 7.43.0-pre Prebid 7.42.0 release Fluct Bid Adapter: add user sync support (prebid#9651) VIS.X fix onTimeout data (prebid#9657) ShowHeroes Bid Adapter: added support for USP (prebid#9681) Core: improve FPD enrichment (prebid#9659) changed the URL (prebid#9698) Permutive RTD Module: migrate appnexus to ortb2 (prebid#9630) Disable describe.only and it.only (prebid#9693) DistroScale bidder enhancement (prebid#9641) minutemediaplus Bid Adapter - pass gpp, sua and bid data to server. (prebid#9637) LiveIntent UserId module: update LiveConnect dependency (prebid#9672) Criteo Bid Adapter: reinforce adomain type in case of missmatch (prebid#9687) SmartyTech Bid Adapter : add size parameters (prebid#9692) Revert "Nativo Bid Adapter: adding UserId support (prebid#9583)" (prebid#9691) GDPR consent management: accept static config without `getTCData` (prebid#9664) Kulturemedia Bid Adapter: New Adapter (prebid#9613) Nativo Bid Adapter: adding UserId support (prebid#9583) Underdog Media Bid Adapter: Switch request to method to POST (prebid#9547) ZetaGlobalSsp: provide schain (prebid#9678) Next Millennium Bid Adapter : added `imp[].id` required parameter for openrtb 2.5 request. (prebid#9675) Increment version to 7.42.0-pre Prebid 7.41.0 release DFP video adserver module: set `description_url` to pub's URL by default; do not skip setting it if `cache.url` is set (prebid#9665) support the timeout parameter in the conversant adapter (prebid#9673) Browsi Bid Adapter: initial release (prebid#9562) feat(permutiveRtd): add `ix` and custom cohort `ortb2.user.data` (prebid#9631) Freewheel SSP Adapter: add prebid version in ad request (prebid#9667) Yandex Bid Adapter: (prebid#9604) Bump webpack from 5.74.0 to 5.76.0 (prebid#9668) Deleted the default empty string from userConsent argument in the module's init-function. (prebid#9663) AdUp Technology bid adapter: avoid modification of bid request (prebid#9656) NoBid Bid Adapter: support for Floors (prebid#9635) Freewheel SSP Bid Adapter : support userIdAsEids (prebid#9655) Missena: add userEids, add network and cpm to tracking (prebid#9645) IX Bid Adapter: update additional consent field (prebid#9650) Aduptech Bid Adapter : add GVLID (prebid#9658) Core: make video cache timeout configurable (prebid#9578) Delete flocIdSystem.js (prebid#9652) TheMediaGrid Bid Adapter : support gpp (prebid#9629) Stv Bid Adapter : initial adapter release (prebid#9533) CleanMediaNet Bid Adapter : add userid support and update testing (prebid#9608) Update creative.html (prebid#9648) PubWise Bid Adapter: support video and improve tests (prebid#9576) Increment version to 7.41.0-pre Prebid 7.40.0 release Rubicon Bid Adapter: add size 1x2 (prebid#9644) main>modules\neuwoRtdProvider.js > apiUrl format handling improved, removed unnecessary parameter integrationExamples\gpt\neuwoRtdProvider_example.html > fixed render-step handling on warning (prebid#9646) Core: fix native render when adUnits defines `mediaTypes.native.ortb` but adapter replies with "legacy" native bid (prebid#9638) kueezRtb Bid Adapter: pass sua data to server. (prebid#9643) update Mediago & Discovery BidAdapter:remove size filter (prebid#9585) Permutive RTD Module: migrate magnite to ortb2 (prebid#9555) Vidazoo Bid Adapter: pass sua params. (prebid#9636) ADJS-1271-send-envelope-param-for-lexicon (prebid#9634) Nexx360 Bid Adapter: native support added and ortbConverter usage (prebid#9626) PBjs Core: do not rely on an extendable `window.Promise` (prebid#9558) TheMediaGrid Bid Adapters : do not use jwp segments from bid.rtd field (prebid#9627) read video size from playerSize (prebid#9625) Lemma Digital Bid Adapter : initial adapter release (prebid#9532) smallfix on response validation (prebid#9623) Updated adf adapter to support native with type; use ortb request for natives (prebid#9616) kargo adapter - adding prebid version to requests (prebid#9620) Rubicon bid adapter: remove pchain support (prebid#9621) trigger build revert tag out markWinningBidAsUsed entirely remove unnecessary feature tag tag out ORTB video conversion utils tag out video-related code in native.js tag out video-related code in renderAd replace callPrebidCache with getHook result tag out adpod-related logic Define "VIDEO" compile time feature flag
@dgirardi Can you please include the fledge example in integrationExamples/gpt folder for reference ? |
* Move fledge logic to fledge module * Change fledge interpretResponse API: from {bidId, ...fledgeAuctionConfig} to {bidId, config: fledgeAuctionConfig} * Fledge ORTB processors * PBS adapter fledge impl * Update modules/fledgeForGpt.js Co-authored-by: Laurentiu Badea <laurb9@users.noreply.github.com> * fix fledge tests to pass adUnitCode * Update text * Fix test case to reflect check on `navigator.joinAdInterestGroup` * Adjust warnings * Name change --------- Co-authored-by: Laurentiu Badea <laurb9@users.noreply.github.com>
Type of change
Description of change
This adds untested support for fledge in the PBS adapter, following the spec described here.
In addition this also refactors some of the other logic around fledge:
interpretResponse
return value API has been changed from{bids: [...], fledgeAuctionConfigs: [{bidId, ...config}] }
to{bids: [...], fledgeAuctionConfigs: [{bidId, config}]}
addComponentAuction
signature has been changed from(bidRequest, fledgeAuctionConfig)
to(adUnitCode, fledgeAuctionConfig)
bidderRequest.fledgeEnabled
has been moved to the fledge moduleortbConverter
now automatically setsimp.ext.ae
for all adapters; and parsesext.prebid.fledge
for those that choose to use PBS extensions.Other information
Closes #9080
Documentation TBD