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

bidrequest.id is not always unique in PBS requests #1507

Closed
epouradier opened this issue Sep 21, 2020 · 9 comments
Closed

bidrequest.id is not always unique in PBS requests #1507

epouradier opened this issue Sep 21, 2020 · 9 comments

Comments

@epouradier
Copy link

epouradier commented Sep 21, 2020

A publisher partner who is using Prebid Server for their AMP inventory is sending us bid requests with a static value of BidRequest.id for all ad opportunities, while this field should be populated with a unique ID as per openRTB specifications.

Key takeaway from the initial discussion, from @bretg:

  1. Add a configuration to overwrite any bidRequest.id set in a storedrequest or add one.
settings:
  generate-storedrequest-bidrequest-id: true
  1. if the config is on and the stored request is for AMP or called from ext.prebid.storedrequest
    2a. If the storedrequest contains an "id" field, generate a UUID and replace the value
    2b. If the storedrequest doesn't contain an "id" field, generate a UUID and add it
  2. Configuration should be at the host-company level.
  3. Default should be to not overwrite ids
@bretg
Copy link
Contributor

bretg commented Sep 22, 2020

  • Refined the proposed algorithm
  • Removed the per-account config. I don't see this as something a publisher is going to care to control.
  • The default is still for the feature to be turned off so we don't risk a breaking change. But overall this seems like something not likely to break PBS functionality.

@hhhjort
Copy link
Collaborator

hhhjort commented Sep 23, 2020

So to be clear, this is only for requests coming into the AMP endpoint. Do we want to extend this functionality to request.imp[n].id, since these also won't be unique? Also if we ever want to extend this to non-AMP, I think we should never overwrite an ID that appears in the incoming request.

@epouradier
Copy link
Author

epouradier commented Sep 25, 2020

Per openRTB specs, for request.imp.id is

A unique identifier for this impression within the context of
the bid request (typically, starts with 1 and increments).

So as soon as it is unique in the context of the bid request, it should be ok.

@bretg
Copy link
Contributor

bretg commented Oct 2, 2020

Discussed in committee.

Another option is to support a macro in the ID field. e.g. { id: "{{UUID}}" }. If Prebid Server sees {{UUID}} then resolve to a UUID.

We generally agreed that this is a cleaner approach, but it could require host companies to update thousands of database entries and the code that manages those entries.

So we agreed to move forward with the "override" setting noted above as an initial solution but to also support {{UUID}} in the storedrequest top level ID so we can someday deprecate the override.

Specfically, the override will:

  • replace bidrequest.id with a 16-char random string
  • confirm that all bidrequest.imp[].id fields in the request are different. If not different, re-number them starting from "1".

@bretg
Copy link
Contributor

bretg commented Dec 7, 2020

So to be clear, this is only for requests coming into the AMP endpoint

Not quite true -- this is for AMP and App. Point number 2 says "if the config is on and the stored request is for AMP or called from ext.prebid.storedrequest" -- the latter clause covers mobile app.

@bretg
Copy link
Contributor

bretg commented Mar 19, 2021

Discussed again in the PBS committee. The priority of this has been raised due to a finding that AMP responses are significantly better when the bidrequest.id is random.

@bretg
Copy link
Contributor

bretg commented May 28, 2021

This was released with PBS-Java 1.61.

@SyntaxNode
Copy link
Contributor

Implemented in PBS-Go 0.179.0.

@AlexBVolcy
Copy link
Contributor

Just want to update this thread:

There are some new updates that Bret suggested to be made to the UUID Generation feature that was initially implemented in these PRs:

#1938

#2000

Another conversation will be had to Bret to confirm these updates, but essentially he wants the UUID to only be generated and updated for the request for Amp Auctions. The UUID should still be generated for other types of auctions, but not actually updated for the request.

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

5 participants