prebid / prebid-server

Open-source solution for running real-time advertising auctions in the cloud.
https://prebid.org/product-suite/prebid-server/
Apache License 2.0
425 stars 730 forks source link

FPD: Merge all bidderconfig ORTB fields into root object while ignoring non-ORTB fields #2317

Closed bsardo closed 11 months ago

bsardo commented 2 years ago

Issue #2182 focused on how FPD specified in the bidderconfig.config.ortb2.user.data and bidderconfig.config.ortb2.{app/site}.content.data locations were merged into the root object.

The investigation detailed on this issue resulted in a broader spec change. As a result, all ORTB fields specified in bidderconfig.config.ortb2.{app/site/user} should be merged into the corresponding root object and all non-ORTB fields specified in the bidderconfig should be ignored during the merge. Furthermore, as part of this change, we should ensure that all arrays are merged by replacement instead of concatenation.

PBS-Go and PBS-Java currently only merge a handful of ORTB fields specified in the bidderconfig into the root object with all other ORTB fields and all non-ORTB fields merging into the root object's ext.data field.

We should also be sure to consider ORTB 2.6-specific fields.

bretg commented 2 years ago

Thanks for the analysis and issue @bsardo

bretg commented 2 years ago

PBS-Go and PBS-Java currently only merge a handful of ORTB fields specified in the bidderconfig into the root object with all other ORTB fields and all non-ORTB fields merging into the root object's ext.data field.

Sorry, should have noticed this statement before. It's not correct... we don't move bidder-specific fields to ext.data. Not sure where that came from other than a very old transition period where we moved {site,app,user}.data to {site,app,user}.ext.data.

Anyhow, I've updated the PBS FPD doc with a couple of changes:

  1. Trying to simplify this thing. If you feel something is too complex, you should ask. This is supposed to be simple - in most cases it's just merge All The Things.
  2. I was asked to expand bidder-specific config to more ORTB fields than just site/app/user. I had thought it was simpler just supporting those fields, but others disagreed. The issue is that only valid ORTB fields should be merged, or alternately, after merge, another validation check needs to be done.

Here's the updated doc. Feedback/questions welcome.

https://docs.google.com/document/d/1I2eS40yMYJCgz8XvFljf4f59bEPsbWTrUpnD7a7uJEg/edit#

SyntaxNode commented 1 year ago

I've been working in the FPD area of PBS-Go while working on support for DOOH. We can simplify code and improve performance by removing the "non-ORTB fields merging into the root object's ext.data field" requirement.

@bretg Based on your response, that requirement seems like it was a misunderstanding and never existed. To clarify, the expected behavior is any non-ORTB field in the bidder config will be ignored and removed the request. No warnings or errors will be emitted. I believe the spec supports this approach, but doesn't come right out and say it directly.

bretg commented 1 year ago

Agreed - made a few updates to the text hoping to clarify. e.g.

Bidderconfig.config.ortb2 entries may contain any valid ORTB field.
Any value in bidderconfig.config outside or ortb2 should be ignored.
bretg commented 11 months ago

Confirmed for PBS-Java