Allegro Bid Adapter: initial release #14111
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new bid adapter for Allegro, an advertising platform. The adapter integrates with Allegro's demand sources using the OpenRTB 2.5 protocol with optional conversion of extension fields to Google DoubleClick proto format.
Key Changes:
- New bid adapter implementation using ortbConverter for ORTB 2.5 request/response handling
- Configurable extension field conversion to Google DoubleClick format (enabled by default)
- Optional impression tracking pixel firing on bid won events
- Support for banner, video, and native media types
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| modules/allegroBidAdapter.js | Main adapter implementation with ORTB conversion, extension field transformations, and bid lifecycle handlers |
| modules/allegroBidAdapter.md | Documentation covering configuration options, ad unit examples, and technical specifications |
| test/spec/modules/allegroBidAdapter_spec.js | Comprehensive test suite validating adapter behavior including request building, response interpretation, and config-driven features |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Hi everyone. Could someone from the core team take a look at this PR or let me know if anything else is needed? Thanks! |
|
Hi @patmmccann, sorry for pinging you directly, but it's been a while since I've created this PR. Could you please take a look, or help me assign the appropriate reviewer who can handle it? Thanks for your assistance! |
|
hello hello, our automated assignment is fixed, i repulled master to activate it |
|
Hi @ncolletti, could you please take a look at this PR? My team needs the adapter to be accepted as soon as possible. Your review would be appreciated :) |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Hi @ncolletti, @patmmccann, I wanted to ask are we blocked on something here with the review or it's just you have a huge queue of PRs? Mostly asking because I see some of the CI jobs skipped and there was problem with auto assignment of a reviewer. |
|
sorry for pinging you once again but I'm a bit worried we landed in some PR review limbo and our PR goes unnoticed. I hope this messages reaches you :) |
|
@dgirardi @robertrmartinez excuse me gentleman calling you out in this thread explicitly but I saw your activity in other prebid adapters. |
| * @returns Request details for Prebid to send. | ||
| */ | ||
| buildRequests: function (bidRequests, bidderRequest) { | ||
| const url = config.getConfig('allegro.bidderUrl') || BIDDER_URL; |
There was a problem hiding this comment.
What's the intent behind the configurable endpoint? is it also the reason why you have convertExtensionFields? (it does not look like something a publisher would care about).
We have a rule against "fully variable endpoints", which is to avoid burdening publishers with complicated logic to calculate them at run time (e.g. routing users to a regional endpoint).
There was a problem hiding this comment.
Hi @dgirardi, thank you for your message.
The purpose of the configurable endpoint is for testing and to support a potential endpoint change in the future. And yes, the same reasoning applies to convertExtensionFields. As you might have noticed, the fields are converted to a very specific format to support deserialization into structures based on DoubleClick's protobuf messages. Still, we wanted to keep the option to handle traffic without conversion in the future. To keep the configuration effortless, we provided default values for both variables.
There was a problem hiding this comment.
I'd like to clarify one thing, because I'm not entirely sure I understand the second part of your comment. Does using URL that is read from config violates the "fully variable endpoints" rule? We use exact value of the config's variable, so there is no any calculation or that's my misunderstanding.
I've noticed other adapters take the bidder url from ad unit params. Is that the way to go?
There was a problem hiding this comment.
Does using URL that is read from config violates the "fully variable endpoints" rule?
Technically yes, but the rule is not intended against your use case (and we likely need to make it clearer). The concern is asking publishers to do something like set this endpoint if the user is in NA, and this other endpoint if they're in EU which we feel is too much for some.
Bid params are preferable as they're friendlier to the publisher, but it's not a rule and since yours all appear to be optional I think it's OK to leave them as config.
4 participants
Type of change
Description of change
This PR introduces an Allegro bid adapter for Prebid.js.
Contact e-mail: the-bidders@allegro.com
Test params:
Other information
Documentation PR: prebid/prebid.github.io#6329