getAdserverTargeting sometimes returns targets with adid from previous auctions

[drdmitry]

Type of issue

Question

Description

pbjs.getAdserverTargeting() sometimes getting targetings with AdIds which were used in previous auctions (and Ad was rendered). How to use AdIds returned in the latest requestBids call ?

Steps to reproduce

When requesting bids multiple times using pbjs.requestBids, sometimes pbjs.getAdserverTargeting() call inside bidsBackHandler returns targeting with AdIds from the previous requestBids calls.

Test page

https://jsbin.com/xaruyupeku/edit?html,output
Open page, open console, click refresh button and check that targeting.adid is equals bid.adid

Expected results

pbjs.getAdserverTargeting() should return targetings with AdIds from the last requestBids call.

Actual results

pbjs.getAdserverTargeting() sometimes returns targetings with AdIds from the previous calls.

Platform details

Prebid.js 1.12.0

Other information

enableSendAllBids: true

[drdmitry]

Possible problem might be, that for targeting Prebid considering all received bids in all auctions, even those for which ads already rendered.

Prebid.js 1.12.0

src/prebid.js:109

$$PREBID_GLOBAL$$.getAdserverTargeting = function (adUnitCode) {
  utils.logInfo('Invoking $$PREBID_GLOBAL$$.getAdserverTargeting', arguments);
  let bidsReceived = auctionManager.getBidsReceived(); // this line
  return targeting.getAllTargeting(adUnitCode, bidsReceived);
};

and

src/targeting.js:72

  targeting.getAllTargeting = function(adUnitCode, bidsReceived = getBidsReceived()) {
    const adUnitCodes = getAdUnitCodes(adUnitCode);

    // Get targeting for the winning bid. Add targeting for any bids that have
    // `alwaysUseBid=true`. If sending all bids is enabled, add targeting for losing bids.
    var targeting = getWinningBidTargeting(adUnitCodes, bidsReceived)
      .concat(getCustomBidTargeting(adUnitCodes, bidsReceived))
      .concat(config.getConfig('enableSendAllBids') ? getBidLandscapeTargeting(adUnitCodes, bidsReceived) : []);
...

and inside getBidLandscapeTargeting filtering is only done by the Highest CPM, applying this logic to already rendered ads also. So, in addition, it might be good to check the status = rendered.

  function getBidLandscapeTargeting(adUnitCodes, bidsReceived) {
...
      Object.keys(bidsByBidder).forEach(key => bids.push(bidsByBidder[key].reduce(getHighestCpm, getEmptyBid()))); // getHighestCpm here
...

another probably better idea might be - to have filtered bidsReceived, before applying the targetings.
To implement this idea - targeting.bidsReceived() can be used, which is the default value if we do not pass bidsReceived parameter to targeting.getAllTargeting, like:

$$PREBID_GLOBAL$$.getAdserverTargeting = function (adUnitCode) {
  utils.logInfo('Invoking $$PREBID_GLOBAL$$.getAdserverTargeting', arguments);
  return targeting.getAllTargeting(adUnitCode); // here we do not pass bidsReceived
};

in this case targeting.getBidsReceived will be used, which has a filter for unused bids: isUnusedBid

src/targeting.js:180

  function getBidsReceived() {
    return auctionManager.getBidsReceived()
      .filter(isUnusedBid)
      .filter(exports.isBidExpired)
      .filter(getOldestBid)
    ;
  }

[jaiminpanchal27]

@drdmitry I had a local PR with the fix. I think this might be the reason for #2648
I will be pushing my branch now and ask everyone to test with that prebid.js build if it solves the impression problem.

Thanks.

[jaiminpanchal27]

@drdmitry Have pushed a branch with that change here https://github.com/prebid/Prebid.js/tree/getAdservertargeting-bugfix

[derdeka]

Thanks @drdmitry, @jaiminpanchal27, this solves the "negative yield" bug for us, as an already rendered ad is served multiple times, but paid only once.

Additionally we still have a drop in impressions as described in #2648.

[mercuryyy]

@jaiminpanchal27 ran the "getAdservertargeting-bugfix" build seems to fix the problem but not the issue in #2648

Perhaps should consider changing core behavior for action functions?

[johngeorgewright]

I opened this pull request a little while ago.

#2697

To me it looks like Prebid is retrieving bids that have expired only. But obviously the tests then went berserk, and I thought... "this can't be right, I must be misunderstanding the logic here". So I closed it.

But the more I test this version with prod data, the more it seems that this might be the solution. Does it work for anyone else?

[rmertens]

We also see this issue of rendering ads from previous auctions (on both 1.13 and 1.14).

[mercuryyy]

@jaiminpanchal27 will you include the branch as a regular pull for next release?

[mercuryyy]

@jaiminpanchal27 any plans to include an official pull?

[jaiminpanchal27]

@mercuryyy It will be included in today's release

[jaiminpanchal27]

#2685 related issue.

[jaiminpanchal27]

@drdmitry Can you check if Prebid 1.15 has resolved your issue or not

[drdmitry]

@jaiminpanchal27, Yes, issue is resolved. Thanks.