Prebid Core: Functionality to Optionally Defer Billing for an Ad #9640

jlquaccia · 2023-03-08T23:16:52Z

Type of change

Feature

Description of change

Added the functionality for a Publisher to optionally defer billing on a specific ad unit if desired

Logic follows the proposal outlined here:

bid adapters can provide a new method onBidBillable(bid) which will be invoked by core when it deems a bid to be billable;
when a bid wins, it is by default also billable; that is, by default, core will invoke onBidWon and onBidBillable one after the other;
publisher can flag adUnits as being separately billable: pbjs.addAdUnits({deferBilling: true, ...})
winning bids for those adUnits trigger onBidWon but not onBidBillable;
when appropriate (e.g. interstitial is displayed), the publisher calls a new public API, pbjs.triggerBilling({adId}), which would trigger onBidBillable.

Note: For this feature to fully be enabled, the following is also required:

The bid adapter a Publisher utilizes must support the onBidBillable method
A Publisher would have to manually invoke the pbjs.triggerBilling when desired

Other information

Related Issue: #4494

dgirardi · 2023-03-09T15:09:40Z

src/adapterManager.js

@@ -616,6 +616,12 @@ adapterManager.callBidWonBidder = function(bidder, bid, adUnits) {
  tryCallBidderMethod(bidder, 'onBidWon', bid);
 };

+adapterManager.callBidBillableBidder = function(bidder, bid, adUnits) {
+  // Adding user configured params to bidWon event data
+  bid.params = getUserConfiguredParams(adUnits, bid.adUnitCode, bid.bidder);


I don't think this is necessary. It's already happened in bidWon, and even there I'd question it.

dgirardi · 2023-03-09T15:10:46Z

src/auction.js

-  function addWinningBid(winningBid) {
+  function addWinningBid(winningBid, aUnits = adUnits, am = adapterManager) {
+    const { adUnitCode } = winningBid;
+    const winningAd = aUnits.find(adUnit => adUnit.code === adUnitCode);


I think transactionId is a better ID than code. Multiple adUnits can have the same code.

dgirardi · 2023-03-09T15:13:29Z

src/prebid.js

+/**
+ * @alias module:pbjs.triggerBilling
+ */
+$$PREBID_GLOBAL$$.triggerBilling = (bidder, bid, adUnits, am = adapterManager) => {


this is a public method, so its interface should be kept clean - I don't think you need anything more than bid.

dgirardi · 2023-03-09T15:19:05Z

src/auction.js

@@ -363,9 +363,12 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels, a
    }
  }

-  function addWinningBid(winningBid) {
+  function addWinningBid(winningBid, aUnits = adUnits, am = adapterManager) {


I think this is overdoing dependency injection - adUnits is already something you can pass in directly to the constructor; adapterManger could also be if you want it injected, it doesn't make a lot of sense to do it in just one method - the next guy will need to reason about an auction with tightly coupled methods but completely disjointed depedendencies.

jlquaccia · 2023-03-14T00:02:35Z

thanks @dgirardi, went back to address your feedback and also refactored a bit. let me know if anything else is needed.

dgirardi · 2023-03-15T15:46:12Z

src/prebid.js

+/**
+ * @alias module:pbjs.triggerBilling
+ */
+$$PREBID_GLOBAL$$.triggerBilling = (bid) => {


what do you think about extracting this piece of code and reusing it here?

what I am worried about is that this call comes from the publisher and here bid could be anything at all; but the adapters will expect it to be an actual certified bid response object.

I would also improve that code a bit to look up by bidId as well. I don't like that it apparently accepts finding multiple matching bids, and just decides that the first one is the "correct" one.

hey @dgirardi, i agree, makes sense to extract that piece of code and reuse it in the triggerBilling func, especially to your point about how a pub could pass through anything.

for the second part of your comment, if/when the bids var gets defined, i added an additional check to make sure that the bid we grab from that array has a matching transactionId with the bid that was passed through the triggerBilling func. should that be sufficient?

jlquaccia · 2023-03-29T23:35:42Z

@dgirardi do you think anything else is needed for this PR?

dgirardi · 2023-03-30T22:03:23Z

src/auction.js

@@ -367,8 +367,11 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels, a
  }

  function addWinningBid(winningBid) {
+    const { transactionId } = winningBid;


I don't think this helps; by the time it gets here, it should already be the correct bid. The filtering happens in markWinningBidAsUsed.

I think for this part I was just aiming to link the winning bid with it's associated adunit to check if deferBilling is true or false. Looks like that can be done by either comparing adUnitCode & code between the two or transactionId. Or let me know if there is another way to do this that I may have missed.

dgirardi · 2023-03-30T22:04:39Z

src/prebid.js

+    bids = auctionManager.getBidsReceived()
+      .filter(bid => bid.adId === winningBid.adId && bid.adUnitCode === winningBid.adUnitCode);
+  } else if (winningBid.adUnitCode) {
+    bids = targeting.getWinningBids(winningBid.adUnitCode);
+  } else if (winningBid.adId) {
+    bids = auctionManager.getBidsReceived().filter(bid => bid.adId === winningBid.adId);


this could be extracted no? as in, re-use code instead of copy-paste.

dgirardi · 2023-03-30T22:06:38Z

src/prebid.js

+  }
+
+  if (bids.length > 0) {
+    const triggerBillingBid = bids.find(bid => bid.transactionId === winningBid.transactionId);


bidId is a better identifier than transactionId; but I also think you need to do something - a warning or so - when you're left with nothing. note that you can only do this in here, not in markWinningBidAsUsed, because that would be a breaking change.

Addressed this as well. Had one basic question though, I didn't see bidId on the winning bids but did see requestId which looks to have the same value. Is it safe to you that?

dgirardi · 2023-04-13T17:29:49Z

src/adapterManager.js

@@ -609,6 +609,10 @@ adapterManager.callBidWonBidder = function(bidder, bid, adUnits) {
  tryCallBidderMethod(bidder, 'onBidWon', bid);
 };

+adapterManager.callBidBillableBidder = function(bid) {
+  tryCallBidderMethod(bid.adapterCode || bid.bidder, 'onBidBillable', bid);


I'd leave out the use of adapterCode, because it makes this inconsistent with every other method of this type. (also, if we were to use it, it should be the fallback and not the default).

dgirardi · 2023-04-13T17:31:32Z

src/auction.js

@@ -367,8 +367,10 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels, a
  }

  function addWinningBid(winningBid) {
+    const winningAd = adUnits.find(adUnit => adUnit.code === winningBid.adUnitCode);


you probably want to use transactionId instead of code here; code is not unique and in theory I could set up deferred billing on only some adUnits that share the same code.

dgirardi · 2023-04-13T17:36:45Z

src/prebid.js

+ */
+pbjsInstance.triggerBilling = (winningBid) => {
+  const bids = fetchReceivedBids(winningBid, 'Improper use of triggerBilling. It requires a bid with at least an adUnitCode or an adId to function.');
+  const triggerBillingBid = bids.find(bid => (bid.adId === winningBid.adId && bid.adUnitCode === winningBid.adUnitCode) || bid.requestId === winningBid.requestId);


Didn't fetchReceivedBids just do the searching on adId and adUnitCode? I think those would always be true here.

Also, the match on requestId should be optional here IMO. e.g. const triggerBillingBid = bids.find(bid => bid.requestId === winningBid.requestId) || bids[0].

jlquaccia · 2023-04-18T19:05:56Z

thanks @dgirardi, the feedback you gave makes sense. just addressed the changes you pointed out with a new commit. if this pr looks good now, would it be possible to merge this one this week? Will free us up over here to work on another ticket that depends on the code in this PR.

bretg · 2023-05-05T16:51:53Z

Docs PR?

…bid#9640) * logic for billing deferrals * refactored and addressed feedback * refactored triggerBilling func * addressed feedback * rebased on top of master * reverted some unneeded changes * refactored triggerBilling and addWinningBid funcs * reverted changes to example html file * addressed changes from feedback

jlquaccia requested a review from dgirardi March 8, 2023 23:16

dgirardi reviewed Mar 9, 2023

View reviewed changes

ChrisHuie added the feature label Mar 15, 2023

ChrisHuie assigned dgirardi Mar 15, 2023

dgirardi reviewed Mar 15, 2023

View reviewed changes

patmmccann linked an issue Mar 27, 2023 that may be closed by this pull request

Feature Request: Support for loading the ad but not firing the billing notifications #4494

Closed

dgirardi reviewed Mar 30, 2023

View reviewed changes

jlquaccia added 5 commits April 4, 2023 16:31

logic for billing deferrals

2104353

refactored and addressed feedback

3287bd3

refactored triggerBilling func

6f79a31

addressed feedback

9acd07d

rebased on top of master

a1fe985

jlquaccia force-pushed the ISSUE-4494 branch from ea8ba89 to a1fe985 Compare April 4, 2023 23:44

jlquaccia added 3 commits April 4, 2023 16:50

reverted some unneeded changes

4ff8010

refactored triggerBilling and addWinningBid funcs

e57dee9

reverted changes to example html file

3b365b1

dgirardi reviewed Apr 13, 2023

View reviewed changes

addressed changes from feedback

ff4fca6

dgirardi approved these changes May 2, 2023

View reviewed changes

jlquaccia requested a review from ChrisHuie May 3, 2023 18:49

patmmccann changed the title ~~Core: Functionality to Optionally Defer Billing for an Ad~~ Prebid Core: Functionality to Optionally Defer Billing for an Ad May 4, 2023

patmmccann approved these changes May 5, 2023

View reviewed changes

patmmccann merged commit 879e5c4 into prebid:master May 5, 2023

jlquaccia mentioned this pull request May 5, 2023

Documentation for Billing Deferral prebid/prebid.github.io#4546

Merged

2 tasks

This was referenced May 22, 2023

Integration of mobile web interstitial ad with prebid #5917

Open

Native mediatype: Track Visible impression #6358

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prebid Core: Functionality to Optionally Defer Billing for an Ad #9640

Prebid Core: Functionality to Optionally Defer Billing for an Ad #9640

jlquaccia commented Mar 8, 2023

dgirardi Mar 9, 2023

dgirardi Mar 9, 2023

dgirardi Mar 9, 2023

dgirardi Mar 9, 2023

jlquaccia commented Mar 14, 2023

dgirardi Mar 15, 2023

jlquaccia Mar 16, 2023

jlquaccia commented Mar 29, 2023

dgirardi Mar 30, 2023

jlquaccia Apr 4, 2023

dgirardi Mar 30, 2023

dgirardi Mar 30, 2023

jlquaccia Apr 4, 2023

dgirardi Apr 13, 2023

dgirardi Apr 13, 2023

dgirardi Apr 13, 2023

jlquaccia commented Apr 18, 2023 •

edited

Loading

bretg commented May 5, 2023

Prebid Core: Functionality to Optionally Defer Billing for an Ad #9640

Prebid Core: Functionality to Optionally Defer Billing for an Ad #9640

Conversation

jlquaccia commented Mar 8, 2023

Type of change

Description of change

Other information

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlquaccia commented Mar 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlquaccia commented Mar 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlquaccia commented Apr 18, 2023 • edited Loading

bretg commented May 5, 2023

jlquaccia commented Apr 18, 2023 •

edited

Loading