Add support for OpenRTB protocol and endpoint #2102

matthewlane · 2018-02-03T00:52:16Z

Type of change

Feature

Description of change

Adds support to prebidServer adapter for working with the OpenRTB prebid-server endpoint. Enable support by configuring s2s to use an endpoint with the pattern openrtb2/auction:

pbjs.setConfig({
  debug: true,
  s2sConfig: {
    accountId: '123',
    enabled: true,
    bidders: ['appnexus'],
    endpoint: 'https://prebid.adnxs.com/pbs/v1/openrtb2/auction',
  },
});

When s2sConfig.endpoint is not configured to this type of endpoint, prebidServer communicates with the configured endpoint using the existing s2s protocol.

Other information

Fixes #2126

bretg · 2018-02-12T21:23:13Z

What's the status on this -- is it ready for review @matthewlane ?

matthewlane · 2018-02-12T21:30:14Z

@bretg Yes, status is ready for review

bretg · 2018-02-12T22:24:14Z

Cool. Added the Needs Docs label as well. Ought to have page on prebid.org somewhere describing the different endpoints.

Will the legacy endpoint be phased out at some point?

matthewlane · 2018-02-12T22:42:31Z

I'll work on a docs draft. Not sure on legacy endpoint being phased out or timing, ccing @mkendall07 @dbemiller for comment

dbemiller · 2018-02-13T14:17:10Z

Legacy endpoint will definitely be phased out... but no specific timeline yet. I'd planned to get rid of it once the metrics said that the traffic was low enough (and Rubicon agreed, and we left the PR deleting them open for a week or so in case anyone else wanted to complain).

I haven't been using prebid.org at all, because I find jekyll to be annoying and I think there's a lot of value in keeping the docs in the same repo as the code... but there's documentation for some of the endpoints here. The folder structure & filenames of .md files mirror the endpoints on PBS. /cookie_sync is not documented yet, but should be. We should probably include that during work on prebid/prebid-server#341.

We should definitely update these two pages, though:

Once prebid.js uses /openrtb2/auction, there will be no value in a buyer implementing a legacy adapter anymore.

snapwich

Overall looking pretty good. I have some questions and nitpicks that I left.

snapwich · 2018-02-13T22:37:32Z

modules/prebidServerBidAdapter.js

+ */
+const protocolAdapter = () => {
+  const OPEN_RTB_PATH = 'openrtb2/auction';
+  const isOpenRtb = _s2sConfig.endpoint.includes(OPEN_RTB_PATH);


Not sure if this is polyfilled. Either way, it's probably safer using a regex or something to validate that this is at the end of the endpoint.

One of the legacy test pages I found originally had an endpoint configured like https://prebid.adnxs.com/pbs/v1/auction?url_override=..., so checking for the end of the string might not work. Good point about the polyfill, updated to use old school indexOf rather than the string version of includes

snapwich · 2018-02-13T22:47:04Z

modules/prebidServerBidAdapter.js

    if (digiTrust) {
-      requestJson.digiTrust = digiTrust;
+      request.digiTrust = digiTrust;
    }


What happens to this app, device, digiTrust, etc stuff if we use the OpenRTB endpoint? Can we pass it as ext data?

Good question, will find out

Edit: yes, these are supported by the OpenRTB endpoint, pushed an update to send them

snapwich · 2018-02-13T22:53:29Z

modules/prebidServerBidAdapter.js

+      if (bannerParams && bannerParams.sizes) {
+        // get banner sizes in form [{ w: <int>, h: <int> }, ...]
+        const format = bannerParams.sizes.reduce((acc, size) =>
+          [...acc, { w: size[0], h: size[1] }], []);


This seems like a perfect candidate for Array#map, is there a reason for using reduce? Also, I think creating a new array on each iteration of the reduce is less than ideal for performance.

snapwich · 2018-02-13T23:01:25Z

modules/prebidServerBidAdapter.js

+
+      // get bidder params in form { <bidder code>: {...params} }
+      const ext = adUnit.bids.reduce((acc, bid) =>
+        Object.assign({}, acc, { [bid.bidder]: bid.params }), {});


Same performance complaint as a above.

const ext = adUnit.bids.reduce((acc, bid) => { acc[bid.bidder] = bid.params; return acc; }, {})

In my opinion this is more readable, but opinion aside it's definitely going to perform much better as it's not doing additional iteration per each loop (each arg passed to Object.assign is extra iteration) and not instantiating a bunch of throwaway object literals.

Ah yeah, totally blanked on the way of doing this, thanks, updated

… handler

mkendall07

@matthewlane
When I test this using our hello world page and the new ORTB endpoint, I receive the following error from PBS:
Invalid request format: request.imp[%d] must contain at least one of "banner", "video", "audio", or "native"

Request payload that generated the error:

{
	"id": "44cb5fb9-33d8-4767-978e-d2be5819225c",
	"site": {
		"publisher": {
			"id": "c9d412ee-3cc6-4b66-9326-9f49d528f13e"
		}
	},
	"source": {
		"tid": "44cb5fb9-33d8-4767-978e-d2be5819225c"
	},
	"tmax": 1000,
	"imp": [{
		"id": "div-gpt-ad-1460505748561-0",
		"ext": {
			"appnexus": {
				"placementId": 10433394
			}
		},
		"secure": 0
	}],
	"test": 0
}

matthewlane · 2018-02-20T17:58:47Z

@mkendall07 Thanks, updated to default to banner when mediaTypes is not defined, please test again

mkendall07 · 2018-02-20T19:33:28Z

modules/prebidServerBidAdapter.js

+  const OPEN_RTB_PATH = 'openrtb2/auction';
+
+  const endpoint = (_s2sConfig && _s2sConfig.endpoint) || '';
+  const isOpenRtb = ~endpoint.indexOf(OPEN_RTB_PATH);


nice operator usage :)

mkendall07

Looks great, nice work

matthewlane added the in progress label Feb 3, 2018

matthewlane force-pushed the openrtb-protocol branch 5 times, most recently from d207b48 to a405382 Compare February 8, 2018 04:34

dbemiller mentioned this pull request Feb 12, 2018

I am building a Bidder. Do I need to build a legacy Adapter? prebid/prebid-server#340

Closed

mkendall07 added this to the Prebid 1.4.0 milestone Feb 12, 2018

matthewlane force-pushed the openrtb-protocol branch from 3700692 to 5a02b24 Compare February 12, 2018 19:10

bretg assigned matthewlane Feb 12, 2018

matthewlane added the needs review label Feb 12, 2018

bretg assigned snapwich and unassigned matthewlane Feb 12, 2018

bretg requested a review from snapwich February 12, 2018 22:21

bretg added needs docs and removed in progress labels Feb 12, 2018

snapwich requested changes Feb 13, 2018

View reviewed changes

snapwich assigned matthewlane and unassigned snapwich Feb 14, 2018

matthewlane added needs update and removed needs review labels Feb 14, 2018

matthewlane force-pushed the openrtb-protocol branch from a8b090e to 065da41 Compare February 14, 2018 23:57

matthewlane added 3 commits February 15, 2018 11:28

Update to use OpenRTB protocol and endpoint

20460c9

Refactor request and response handlers

d98f9eb

Handle adding bids in core response handler

e049ca7

matthewlane added 6 commits February 15, 2018 11:28

Update openrtb endpoint response interpreter

a810d04

Refactor variable names, update comments, extract cookie sync to core…

437ce06

… handler

Update openrtb bid response fields

8759ed2

Match bid ids

a4fb4cd

Update based on review feedback

bcbc836

Move legacy-specific video code to legcy spec

e20ba0d

matthewlane force-pushed the openrtb-protocol branch from 065da41 to e20ba0d Compare February 16, 2018 00:45

matthewlane added 2 commits February 16, 2018 11:52

Add video support to OpenRTB spec

186c946

Add digiTrust, app, device to request

94c91b6

matthewlane removed the needs update label Feb 16, 2018

matthewlane mentioned this pull request Feb 16, 2018

Add note about OpenRTB endpoint config prebid/prebid.github.io#622

Merged

matthewlane added needs review and removed needs docs labels Feb 16, 2018

mkendall07 requested changes Feb 20, 2018

View reviewed changes

matthewlane added 2 commits February 20, 2018 09:13

Default to banner if mediaTypes not defined

7845600

Allow single or double banner.sizes array

32755bd

mkendall07 reviewed Feb 20, 2018

View reviewed changes

mkendall07 approved these changes Feb 20, 2018

View reviewed changes

mkendall07 merged commit a455757 into master Feb 20, 2018

mkendall07 deleted the openrtb-protocol branch February 20, 2018 19:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for OpenRTB protocol and endpoint #2102

Add support for OpenRTB protocol and endpoint #2102

matthewlane commented Feb 3, 2018 •

edited

Loading

bretg commented Feb 12, 2018

matthewlane commented Feb 12, 2018

bretg commented Feb 12, 2018

matthewlane commented Feb 12, 2018

dbemiller commented Feb 13, 2018 •

edited

Loading

snapwich left a comment

snapwich Feb 13, 2018

matthewlane Feb 15, 2018 •

edited

Loading

snapwich Feb 13, 2018

matthewlane Feb 15, 2018 •

edited

Loading

snapwich Feb 13, 2018

snapwich Feb 13, 2018

matthewlane Feb 14, 2018

mkendall07 left a comment •

edited

Loading

matthewlane commented Feb 20, 2018

mkendall07 Feb 20, 2018

mkendall07 left a comment

Add support for OpenRTB protocol and endpoint #2102

Add support for OpenRTB protocol and endpoint #2102

Conversation

matthewlane commented Feb 3, 2018 • edited Loading

Type of change

Description of change

Other information

bretg commented Feb 12, 2018

matthewlane commented Feb 12, 2018

bretg commented Feb 12, 2018

matthewlane commented Feb 12, 2018

dbemiller commented Feb 13, 2018 • edited Loading

snapwich left a comment

Choose a reason for hiding this comment

snapwich Feb 13, 2018

Choose a reason for hiding this comment

matthewlane Feb 15, 2018 • edited Loading

Choose a reason for hiding this comment

snapwich Feb 13, 2018

Choose a reason for hiding this comment

matthewlane Feb 15, 2018 • edited Loading

Choose a reason for hiding this comment

snapwich Feb 13, 2018

Choose a reason for hiding this comment

snapwich Feb 13, 2018

Choose a reason for hiding this comment

matthewlane Feb 14, 2018

Choose a reason for hiding this comment

mkendall07 left a comment • edited Loading

Choose a reason for hiding this comment

matthewlane commented Feb 20, 2018

mkendall07 Feb 20, 2018

Choose a reason for hiding this comment

mkendall07 left a comment

Choose a reason for hiding this comment

matthewlane commented Feb 3, 2018 •

edited

Loading

dbemiller commented Feb 13, 2018 •

edited

Loading

matthewlane Feb 15, 2018 •

edited

Loading

matthewlane Feb 15, 2018 •

edited

Loading

mkendall07 left a comment •

edited

Loading