Prebid.js icon indicating copy to clipboard operation
Prebid.js copied to clipboard

PBJS-ORTB conversion library

Open dgirardi opened this issue 3 years ago • 8 comments

Type of change

  • [x] Feature

Description of change

This is an initial version of the pbjs-ortb conversion library proposal, an attempt to unify all ORTB-related request generation and response parsing.

The following adapters have been refactored to use the new library:

  • Prebid Server
  • openxOrtb
  • improvedigital (FYI @jbartek25)

Other information

Closes https://github.com/prebid/Prebid.js/issues/8562 Closes https://github.com/prebid/Prebid.js/issues/8665 (missing: documentation on new s2sConfig.ortbNative configuration) Library documentation is included in libraries/ortbConverter/README.md

dgirardi avatar Jul 25 '22 21:07 dgirardi

This pull request introduces 2 alerts when merging f6ed3122eb0f5f97ab9cb6317b9b9bfbc88c6b5d into 90d1a188860a8ec256a0db2f660bbe9d8a46b655 - view on LGTM.com

new alerts:

  • 2 for Unused variable, import, function or class

lgtm-com[bot] avatar Jul 26 '22 19:07 lgtm-com[bot]

@jsnellbaker looks like this is ready for reviewer assignment

patmmccann avatar Aug 04 '22 18:08 patmmccann

Hi @dgirardi,

Can you please resolve the merge conflicts. I want to clone it locally and run some tests, unable to do so till it's merged with master.

Correct me if I'm wrong, you said in your proposal here to move enrichmentModule logic into core. So, when that happens, do we get rid of that module?

Plus, do we wanna add the docs to Prebid.org? As a guide for Adapter creators, perhaps somewhere over here?

Fawke avatar Sep 16 '22 08:09 Fawke

Thank you @Fawke! Just merged master in. This at the moment does not attempt to move fpdEnrichments into core, I figured this PR is big enough already. there's this other issue to track that - I'm not sure about the strategy, probably keeping an empty module until we can remove it in v8.

Agree on making this visible in the new adapter guide, I'll come back with a PR.

dgirardi avatar Sep 16 '22 15:09 dgirardi

Created a test page that exercises a number of features that affect ORTB output: FPD, bidder-specific FPD, GDPR, COPPA, Floors, auction ID override, s2sConfig.extPrebid, sharedId, eidPermissions, global schain, bidder-specific schain, and aliases. Compared the results from the live file on jsdelivr vs this branch.

Two items of substance:

  • the library is missing tmax. This should be the value defined by s2sConfig.timeout.
  • it doesn't produce a global schain entry. Specifying a global schain only results in a bidder-specific schain going to PBS. Having both a global and bidder-specific schain

Comments:

  • topframe is new - nice
  • the library produces device.{dnt, ua, language} - nice
  • cosmetic, but I'd prefer that '$.id' be the first thing in the struct and imp[].id be the first thing in the imp

bretg avatar Sep 27 '22 21:09 bretg

Fixed tmax; source.ext.schain is now set to the schain that is used by the most bidders in the request.

Regarding id, JSON object property order is in general not defined I think. Firefox' network tab orders them alphabetically. Javascript objects do preserve insertion order, but I don't think it's worth having extra code for that.

dgirardi avatar Sep 28 '22 19:09 dgirardi

@dgirardi - @musikele found one more gap -- pricegranularity.

PBS should receive ext.prebid.targeting.pricegranularity so it quantizes the hb_pb* targeting values appropriately.

bretg avatar Oct 07 '22 15:10 bretg

@bretg, I don't know what that refers to - I don't see granularity in PBS requests generated from master. Is this a regression or a new requirement? What should be in that field exactly?

dgirardi avatar Oct 07 '22 15:10 dgirardi