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

Adagio Rtd Provider: initial release #11509

Merged
merged 3 commits into from
May 29, 2024

Conversation

osazos
Copy link
Collaborator

@osazos osazos commented May 17, 2024

Type of change

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

Description of change

New Rtd Provider for Adagio. This is the following of #11485.

Context for reviewers:
We computes and collects data required to leverage Adagio viewability and attention prediction engine.
To do so we need an external script. Historically this script was loaded from the adagioBidAdapter this was before the RTD modules were implemented. This behavior becomes not accepted with #10653.

This provider essentially mirrors what we did in our bidAdapter, minus the "cache" system that previously required us to use Function() and the encryption verification.

In Prebid.js 9, this module will be mandatory to work with Adagio and our bidder adapter will be "cleaned" to remove duplicated code. But, rather than waiting for v.9 to be released, some slight modifications have been done to our bidAdapter to ensure compatibility and to streamline the transition for our clients. See #11485.

Contact email of the maintainer: dev@adagio.io, osazos@adagio.io

Other information

@patmmccann, please find here the Rtd Provider for Adagio. The next step is to clean the Adagio Bid Adapter and open PR on the 9.0 branch.

@osazos osazos force-pushed the adagio-feature/adagioRtdProvider branch from b32357a to 70bfded Compare May 22, 2024 09:08
},

/**
* Ensure that the bidder is Adagio.
Copy link
Collaborator

Choose a reason for hiding this comment

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

RTD modules cannot favor a particular hardcoded bidder. They can favor bidders if the list is taken from publisher configuration; but they cannot 'compute features only for the adagio bidder'

Copy link
Collaborator Author

@osazos osazos May 22, 2024

Choose a reason for hiding this comment

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

Ok, I'll do the changes, it was already planed anyway.
Beside this required change, is there anything that is not compliant? Thx

Copy link
Collaborator

Choose a reason for hiding this comment

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

no other than this looks good!

@ChrisHuie ChrisHuie removed the request for review from mkomorski May 22, 2024 16:32
@patmmccann patmmccann self-assigned this May 23, 2024
@osazos osazos force-pushed the adagio-feature/adagioRtdProvider branch from 68e081d to d8bed88 Compare May 24, 2024 19:14
@osazos
Copy link
Collaborator Author

osazos commented May 27, 2024

@patmmccann the changes have been done in d8bed88. Here the summary:

  • the dependency to adagio bidder is removed
  • the FPD computed by the rtd module are dispatched at the "global" level: ortb2.site.ext.data and ortb2Imp.ext.data
  • to avoid collapse naming, some of these FPD are wrapped in an adg_rtd object.

@osazos osazos force-pushed the adagio-feature/adagioRtdProvider branch from d8bed88 to 8b3d8a2 Compare May 28, 2024 14:19
// `placement` is expected in FPD `adUnits[].ortb2Imp.ext.data` object. (Please note that this `placement` is not related to the oRTB video property.)
// Btw, we have to ensure compatibility with publishers that use the "legacy" adagio params at the adUnit.params level.
const adagioBid = adUnit.bids.find(bid => _internal.isAdagioBidder(bid.bidder));
if (adagioBid) {
Copy link
Collaborator

@patmmccann patmmccann May 28, 2024

Choose a reason for hiding this comment

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

hI @osazos ! I noticed a new commit, and also this special handling for the adagio bidder in this block remains. in #8596 for Prebid 8, all special handling of rtd information was moved from the rtd module into the bid module. If your bidder needs to look at the injected rtd info differently than a typical bidder, why not put that in your bidder js?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @patmmccann,

Let me know if I misunderstand your comment, because the fallback is to handle the opposite: we set for "all" what was set for the adagio adapter.

The case is, when a client has its ad-units already configured at the adUnits[].adagio.params level and switch to the rtd Provider, he must move some of these params at the FPD level. In the real life, our clients won't have the flexibility to easily do the switch. Applying this fallback allow us to progressively onboard them.

@patmmccann patmmccann merged commit 6b7c86e into prebid:master May 29, 2024
4 checks passed
@osazos osazos deleted the adagio-feature/adagioRtdProvider branch June 21, 2024 12:01
DecayConstant pushed a commit to mediavine/Prebid.js that referenced this pull request Jul 18, 2024
* AdagioRtdProvider: add Adagio Rtd Provider

* AdagioRtdProvider: missing callback() call

* AdagioRtdProvider: set signals for all
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 this pull request may close these issues.

4 participants