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

Add onBidWon handler to bid adapters spec #2786

Merged
merged 1 commit into from
Jul 30, 2018

Conversation

Spark-NF
Copy link
Contributor

Type of change

  • Feature
  • Does this change affect user-facing APIs or examples documented on http://prebid.org?

Description of change

Some bid adapters might want to know when their bid won the auction. Given listening to events is off-limits for bid adapters, they can now add a onBidWon handler to their spec that will be called with their winning bid.

Corresponding PR for documentation: prebid/prebid.github.io#864.

@jsnellbaker
Copy link
Collaborator

@Spark-NF Thanks for submitting this PR.

Based on some internal discussion, would it be possible to convert this new field to only handle a pixel URL that would be fired if the bidder won rather than allow a free-form callback method?

It's likely (unless there's some use-case I'm not aware of) that bidders would ultimately just want to fire off this pixel when they won. So having the field setup to handle this call directly rather than setup customized code (that would need to be reviewed each time) would be more conducive.

Please me let me know your thoughts on this. Thanks

@Spark-NF
Copy link
Contributor Author

@jsnellbaker I see where you're coming from with this. Indeed, firing pixels would be a very common use of this handler, and I see how having a way to simply provide URLs (or a method that returns them after building them) would be useful. However, I don't think bid adapters should be limited to this.

In our case for example, we prefer not to generate an additional network call (to not flood the publisher nor the user), and keep the information that would possibly be passed in the pixel (in this case "our bid won") in the localStorage. Then, in the next call, we would send this information along with our bid request, effectively only ever causing a single XHR. This sometimes causes loss of information (if the user doesn't cause another bid request), but as we only use this information for monitoring purposes (not billing), we find this to be an acceptable compromise, and a win-win for everyone.

Copy link
Collaborator

@jsnellbaker jsnellbaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Spark-NF Thanks for the additional feedback/context, those were some valid points.

LGTM.

@jsnellbaker jsnellbaker added needs 2nd review Core module updates require two approvals from the core team and removed needs update labels Jul 25, 2018
@jsnellbaker jsnellbaker requested a review from mike-chowla July 25, 2018 15:27
Copy link
Member

@mkendall07 mkendall07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@mkendall07 mkendall07 merged commit 838ccee into prebid:master Jul 30, 2018
mkendall07 added a commit that referenced this pull request Jul 31, 2018
jsnellbaker pushed a commit that referenced this pull request Jul 31, 2018
* Revert "Add onBidWon method to bid adapter spec (#2786)"

This reverts commit 838ccee.

* Revert "added display count global object to track number of times requestBids has been called per ad-unit (#2802)"

This reverts commit 54f23fa.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs docs needs 2nd review Core module updates require two approvals from the core team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants