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

Need to do cookie sync for all bidders when passing empty bidders #341

Closed
jaiminpanchal27 opened this issue Feb 12, 2018 · 5 comments
Closed
Assignees

Comments

@jaiminpanchal27
Copy link
Contributor

In Amp RTC config example we will be using Amp-iframe to do the pixel syncs. But we don't know the bidders so we would like to have cookie sync status for all the bidders.

Request like this should return status for all bidders

{"uuid":"<uuid>","bidders":[]}
@dbemiller
Copy link
Contributor

We chatted offline about this... I think it makes sense, with a few caveats.

Motivation

AMP pages include one element for Auctions, and another totally separate element for Usersyncs.

The /openrtb2/amp endpoint takes a Stored Request ID, and resolves it as an entire OpenRTB Request. Because of this, the AMP page has no idea which Bidders are involved.

Options

There are a few implementation options and tradeoffs we can consider.

Sync all bidders

As @jaiminpanchal27 suggested, we could expose an API which returns Usersyncs for every bidder.
This would be fine now, but will become inefficient as the number of Bidders grows in PBS.

Require AMP pages to list Bidders

The existing endpoint supports this already. A major drawback is that publishers need to remember to update the bidder list on their pages whenever they add or remove one from their Stored Request.

Resolve usersyncs from Stored Request ID(s)

We could allow Pubs to send in their Stored Request ID when generating usersyncs. We would need to support multiple IDs, since the openrtb2/amp endpoint only supports a single Imp, and any given page may have multiple Imps.

If we added this, we could parse the Stored Requests to figure out which bidders are currently needed, and return the syncs for just those Bidders.

This has the minor drawback that pubs need to put the Stored Request ID on their AMP pages in two places (auction & usersync), which is vulnerable to typos. @mkendall07 also points out that Pubs may want to "seed" their page with syncs from a new Bidder before they add that bidder to the auctions.

Use case summary

In the long term, the cookie sync endpoint should support:

  1. Specifying Bidders through Stored Request IDs
  2. Specifying Bidders manually
  3. Specifying all bidders

Next steps

I think we should first work out an API which satisfies all of the known use-cases, and work backwards from there to make a feature we can implement now. It's the best way (that I know of) to minimize the chance of breaking changes later.

@hhhjort
Copy link
Collaborator

hhhjort commented Feb 16, 2018

My thoughts on the use case summary above:
2. is already handled
3. is trivial if we say if no bidders field plus no stored request supplied equals sync all bidders.
So 1. is where all the action is.

The current motivation for expanding the cookie sync is to support AMP. The list of bidders is done on a per imp basis, but luckily AMP is restricted to serving just one imp at a time. However in a general case (assuming that use may expand beyond AMP) there might be multiple imps in a referenced stored request. I think the behavior should be include all bidders found in at least one imp.

It was also noted that @mkendal07 can see a use case for a bidder list that is not identical to the stored request being used on the page. I see two solutions to this problem.

  1. Create a "seed" stored request to use for cookie sync that contains the extended list of bidders. The downside is now we are intentionally using two different tag IDs for one ad unit, which may add confusion to deployments. Add to that the "seed" stored request is likely to be a temporary tag, that adds to the maintenance cost of managing the tags on the page.
  2. Create another fetcher for cookie sync bidder list. This would push the mapping of tag ID to bidder list off of PBS to the stored request management solution. We have a second query that either pulls the bidders from the tag id, or something more sophisticated to handle Kendal's use case.

For 2. we need to choose if the second fetcher returns the bidder list in an imp object (meaning the original stored request is a valid return for the second fetcher) or returns a simplified object that gives just the bidders. The first option favors simple stored request solutions (can simply use the same query twice if seeding is not supported), but adds a little work to PBS. The second pushes more processing off to the stored procedure database, saving a couple of CPU cycles on prebid server.

@dbemiller
Copy link
Contributor

dbemiller commented Feb 16, 2018

I think the behavior should be include all bidders found in at least one imp.

Agreed.

It was also noted that @mkendal07 can see a use case for a bidder list that is not identical to the stored request being used on the page.

Suggesting a third option here: Could we just support ids and bidders at the same time? If someone hits /cookie_sync with both, we would sync the explicit list and any from the stored requests.

Seems to me much simpler than the other two.

@hhhjort
Copy link
Collaborator

hhhjort commented Feb 16, 2018

That is a possibility too. The down side is again we need to edit code on page to update the preload bidders.

Also from an optimization perspective, the more work for this that we can push off of PBS the better. But perhaps we should determine what is the maximum level of effort we will want to support here, and then aim for that. We can save a few cycles with either/or lists, but it shouldn't be a big hit to merge lists.

@hhhjort hhhjort self-assigned this Feb 20, 2018
hhhjort added a commit that referenced this issue Feb 20, 2018
Adds support for missing "bidders" == all bidders.
Empty bidders still return no syncs.
Does not implement any stored request loading discussed in #341
Fixes an error message typo in the AMP code.
dbemiller pushed a commit that referenced this issue Feb 23, 2018
* First commit for cookie sync all bidders (Issue #341)
Adds support for missing "bidders" == all bidders.
Empty bidders still return no syncs.
Does not implement any stored request loading discussed in #341
Fixes an error message typo in the AMP code.

* adds a documentation page

* removes "uuid" from cookie syncs (immediate deprecation)

* splits new test into TestCookieSyncEmptyBidders() and TestCookieSyncNoBidders()
@dbemiller
Copy link
Contributor

The basic functionality here was implemented in #354. Since the bidders list is generally a whitelist, it's inconsistent for an empty array to sync all the bidders. An empty whitelist would sync no bidders.

To sync all the bidders, you can omit the bidders key completely. Full API docs can be found here: https://github.com/prebid/prebid-server/blob/master/docs/endpoints/cookieSync.md

katsuo5 pushed a commit to flux-dev-team/prebid-server-1 that referenced this issue Dec 1, 2020
* First commit for cookie sync all bidders (Issue prebid#341)
Adds support for missing "bidders" == all bidders.
Empty bidders still return no syncs.
Does not implement any stored request loading discussed in prebid#341
Fixes an error message typo in the AMP code.

* adds a documentation page

* removes "uuid" from cookie syncs (immediate deprecation)

* splits new test into TestCookieSyncEmptyBidders() and TestCookieSyncNoBidders()
katsuo5 pushed a commit to flux-dev-team/prebid-server-1 that referenced this issue Dec 2, 2020
* First commit for cookie sync all bidders (Issue prebid#341)
Adds support for missing "bidders" == all bidders.
Empty bidders still return no syncs.
Does not implement any stored request loading discussed in prebid#341
Fixes an error message typo in the AMP code.

* adds a documentation page

* removes "uuid" from cookie syncs (immediate deprecation)

* splits new test into TestCookieSyncEmptyBidders() and TestCookieSyncNoBidders()
StarWindMoonCloud pushed a commit to ParticleMedia/prebid-server that referenced this issue Jun 5, 2024
* move config

* update helm config

* delete eks specific config

* move stored requests

* disable hooks

* set up mspai config + test plan

* disable execution plan

* add new dockerfile for mspai

* only keep one imp config for testing

* update domain

* rename account id

* add workflow

* update ECR path

* disable cross platform

* checkout submodules = true

* rename workflowe

* update image repo address

* update checkout action

* update aws account id

* update account id

* resolve comments

* setup kafka topic and update schema id

* adjust resource and k8s config
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants