Skip to content
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 does not select highest bid when more than one bid per bidder/adapter in "send all bids" mode. #1246

Closed
kizzard opened this issue May 30, 2017 · 11 comments · Fixed by #1427
Assignees
Labels

Comments

@kizzard
Copy link
Contributor

kizzard commented May 30, 2017

Type of Issue

Bug (Possibly feature request if working as intended)

Description

When in "Send all Bids" mode, if two bidder IDs are configured for the same adapter against the same ad unit, the bidder specific key value is not necessarily the highest bid of the two responses.

Steps to reproduce

  1. Run demo page: https://jsfiddle.net/ry8h1hg2/
  2. Monitor calls to DFP. Compare the hb_pb key with hb_pb_aol. note that sometimes, the hb_pb_aol key is lower than hb_pb even though both bids are from the AOL adapter.

Expected results

The bid submitted by the bidder specific key (hb_pb_aol) should be the highest of all bids submitted by that adapter for that ad unit.

Actual results

The bid submitted by the bidder specific key (hb_pb_aol) is not always the highest of all bids submitted by that adapter for that ad unit (it may be the last received bid although I am having trouble confirming this)

Here is an example of the prev_scp query parameter in the DFP ad call showing this behavior. Note that this is for a single multi-size 300x250/728x90 ad unit:

hb_size_aol=728x90&hb_pb_aol=0.16&hb_adid_aol=3b23281593c0488&hb_bidder_aol=aol&hb_size=300x250&hb_pb=0.21&hb_adid=23fe003c94653a&hb_bidder=aol

Platform details

All browsers, Prebid 0.24.1

Other information

Using multiple IDs for the same bidder for the same ad unit is often used for multi-size units.

@aneuway2
Copy link
Contributor

aneuway2 commented Jun 6, 2017

Hey @kizzard have you by chance checked out http://prebid.org/dev-docs/prebid-troubleshooting-guide.html I'd run through the List your Bids and Bidders section to Check the Ad Server’s Auction (up to Are the key-values being set in the ad server?)

Maybe its possible that the second bid was received after the timeout, and thus never was sent to the ad server? pbjs.getBidResponses(); should be able to tell you time to respond on the bid response.

@kizzard
Copy link
Contributor Author

kizzard commented Jun 6, 2017

Hi @aneuway2 - Thanks for your reply! It's not possible that the second bid was received after the timeout because I'm diagnosing this by actually checking the GPT ad requests and teasing out the key values (you can see them in my "Actual Results" section, which shows that both results are indeed sent to DFP, but the generic hb_pb and hb_pb_aol values don't match.

You should be able to reproduce this behavior pretty easily with my demo page (let me know if you're not getting bids)

@aneuway2
Copy link
Contributor

aneuway2 commented Jun 7, 2017

okay, I was able to reproduce on that JS fiddle. I suspect that hb_pb is getting the highest bid while hb_pb_aol is getting the last returned bid for that bidder that occurs before the timeout. (not sure what the fix would be for this though, or if my hunch checks out)

screen shot 2017-06-07 at 9 46 50 am
screen shot 2017-06-07 at 9 47 29 am

@kizzard
Copy link
Contributor Author

kizzard commented Jun 7, 2017

Hi @aneuway2, yes I agree that your hunch is the most likely explanation.

The problem here is most likely an assumption that there would only ever be a single bid per bidder, per ad unit (therefore we don't do any work to select the highest), which is not necessarily the case. An example of this use-case is for bidders that supply separate bidder IDs for different sizes, which are then used together on a multi-size ad unit. AOL and Sovrn are examples of bidders that operate like this (there could be others).

A suggested fix would be to modify the code that sets the bidder specific key values to check to see if there already exists a bidder specific key value, and if so, only override it if the price is higher.

@protonate protonate self-assigned this Jun 7, 2017
@kizzard
Copy link
Contributor Author

kizzard commented Jun 16, 2017

Hi @aneuway2 and @protonate , any thoughts here?

@gramorris
Copy link

Just a note to say this also affects bidders that return multiple bids for multiple sized slots e.g. IndexExchange. In the case attached this was a slot where one request was made to Index for 300x250 and 300x600 sizes and Index returned two bids. Although the highest bid is in hb_pb the lowest is in [hb_pb_indexExchange

screen shot 2017-06-29 at 13 50 04
screen shot 2017-06-29 at 13 53 01

@jnsprngs
Copy link

Adding that this bug severely reduces the data quality coming into DFP and therefore makes it hard to monitor and analyze prebid. Discrepancies between DFP and bidder reporting are inconsistent and unpredictable. It should also be investigated whether this issue extends into prebid analytics adapters. I think this bug is a big deal.

@gramorris
Copy link

The big deal is that revenue (possibly significant) is silently lost as a result of the highest bid not being selected. I certainly wouldn't recommend anyone use the send all bids functionality until it's resolved.

@mkendall07
Copy link
Member

thanks for the nudge guys. Will look into this ASAP.

@bretg
Copy link
Collaborator

bretg commented Jul 27, 2017

I suspect this is related to #1267

@harpere, please confirm relatedness as part of your research?

@ghost ghost assigned mkendall07 Jul 27, 2017
@ghost ghost added the in progress label Jul 27, 2017
@mkendall07
Copy link
Member

@gramorris @jnsprngs @kizzard
I opened #1427 to fix this issue. Thanks for the report.

@ghost ghost removed the in progress label Aug 17, 2017
matthewlane pushed a commit that referenced this issue Aug 17, 2017
* fixed #1267 and add unit test

* remove markup

* move unit tests

* remove comment

* interim fix

* fix unit tests. Not sure about having to import config directly in the spec file
philipwatson pushed a commit to mbrtargeting/Prebid.js that referenced this issue Sep 18, 2017
* fixed prebid#1267 and add unit test

* remove markup

* move unit tests

* remove comment

* interim fix

* fix unit tests. Not sure about having to import config directly in the spec file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants