Option to use a configurable vendor preset for s2sConfig #2073

jsnellbaker · 2018-01-26T11:03:04Z

Type of change

Feature

Description of change

I have added support for end-users to optionally use a configurable pbs vendor preset in their setConfig({s2sConfig: {...}) page code. By using this defaultVendor option, the end-user won't have to specify a majority of the data-points that are generally needed for this object.

Below is an example usage:

pbjs.setConfig({
    "s2sConfig": {
        accountId: 'c9d412ee-3cc6-4b66-9326-9f49d528f13e',
        bidders: ['appnexus'],
        defaultVendor: 'appnexus'
    }
});

Two of the required s2sConfig attributes will still be expected to be supplied by the end-user; these are accountId and bidders (as depicted above). All of the other commonly used attributes are configured under a vendor object and are invoked by using the defaultVendor attribute.

While all these vendor settings will be automatically populated based on established defaults, end-users will still have the ability to override any individual vendor settings by simply including the attribute in their page-code (same as they would today).

An example of this would look like:

pbjs.setConfig({
    "s2sConfig": {
        accountId: 'c9d412ee-3cc6-4b66-9326-9f49d528f13e',
        bidders: ['appnexus'],
        defaultVendor: 'appnexus',
        timeout: 500,
        cookieSet: false
    }
});

Of also note - the vendor settings are configured overwrite the normally assigned system default values for the s2sConfig (assuming there is no user override). So ultimately s2sConfig settings are established in the following hierarchy:
User overrides > vendor settings > system defaults

For any changes that affect user-facing APIs or example code documented on http://prebid.org, please provide:

A link to a PR on the docs repo at https://github.com/prebid/prebid.github.io/
added information on new s2sConfig option prebid.github.io#579

bretg · 2018-01-29T16:13:29Z

modules/prebidServerBidAdapter.js

+    syncEndpoint: '//prebid.adnxs.com/pbs/v1/cookie_sync',
+    timeout: 1000
+  },
+  'rubicon': {}


Rubicon's PBS defaults:

adapter: 'prebidServer', cookieSet: true, cookieSetUrl: 'https://secure-assets.rubiconproject.com/utils/cookieset/cs.js', enabled: true, endpoint: 'https://prebid-server.rubiconproject.com/auction', syncEndpoint: 'https://prebid-server.rubiconproject.com/cookie_sync', timeout: 500 {code}

Hi @bretg, Thanks for submitting the information. Just to confirm, you do want to have hard-coded secure protocol for the URLs? For the AppNexus vendor, we were leaving them out to be dynamically populated based on the page. Thanks.

Hey @bretg, I've submitted the Rubicon config as is for now (as I had some other changes I wanted to submit). If you want to have the values changed in lieu of the above (or any other reason), please feel free to let me know and I can make the updates.

jsnellbaker · 2018-01-30T15:19:30Z

Added docs PR (see bottom of original comment).

bretg · 2018-01-30T18:29:26Z

test/spec/modules/prebidServerBidAdapter_spec.js

@@ -475,5 +475,62 @@ describe('S2S Adapter', () => {
      config.setConfig({ s2sConfig: options });
      sinon.assert.calledOnce(logErrorSpy);
    });
+
+    it('should log error when vendor does not exist', () => {


Is there a test to show there's a failure when 'defaultVendor' doesn't exist but endpoint isn't specified? This will show there's no default-defaultVendor.

Hey @bretg - I'm not sure I follow what you mean. Can you please clarify further when you get the chance?

I think he's saying to add a test case to assert an error when both endpoint & vendor default are not specified in the S2S config (or vendor default doesn't exist)

OK, thanks for that clarification. I've added a test case in the next commit.

…l cookieSet issue

mkendall07 · 2018-02-02T00:21:42Z

modules/prebidServerBidAdapter.js

+    cookieSet: true,
+    cookieSetUrl: 'https://secure-assets.rubiconproject.com/utils/cookieset/cs.js',
+    enabled: true,
+    endpoint: 'https://prebid-server.rubiconproject.com/auction',


@bretg
You want these hardcoded to HTTPS right?

We've been avoiding the protocol-relative URLs due to articles like

https://stackoverflow.com/questions/28446314/why-use-protocol-relative-urls-at-all
https://jeremywagner.me/blog/stop-using-the-protocol-relative-url/

You haven't seen issues with protocol-relative URLs?

In any case, go ahead with this for now and we'll start the conversation internally.

Talked to @mkendall07 -- the issue here is that non-secure demand is left on the table for HTTPS.

So, yes, let's switch the endpoints to protocol-relative. Thanks Matt. We'll take this discussion up internally as well.

mkendall07 · 2018-02-02T00:58:00Z

modules/prebidServerBidAdapter.js

+
+    if (availVendorDefaults.hasOwnProperty(vendor)) {
+      Object.keys(availVendorDefaults[vendor]).forEach(function(vendorKey) {
+        if (s2sDefaultConfig[vendorKey] === options[vendorKey]) {


I read somewhere that you shouldn't write code as clever as you are, because debugging code is 2x more difficult than writing it. So if you write clever code, you by definition cannot debug it... :)

Ok it's probably an exaggeration but the point remains. It took me awhile to figure out why this works - it's because its compare undefined to undefined (in the case of a missing key) and so it's actually proceeding to copy to undefined key over. It's not necessarily wrong, it's just hard to read. At minimum it needs a comment, or ideally a nicer way to understand what's going on in code.

Thanks for the feedback @mkendall07, I've just finished making a slight change to hopefully infer the logic better. I also put in a comment as a backup. Please let me know your thoughts.

bretg · 2018-02-05T14:12:58Z

modules/prebidServerBidAdapter.js

+  'rubicon': {
+    adapter: 'prebidServer',
+    cookieSet: true,
+    cookieSetUrl: 'https://secure-assets.rubiconproject.com/utils/cookieset/cs.js',


We discussed -- please remove the cookieSetUrl for Rubicon and turn cookieSet to false. Thanks. Also make "endpoint" protocol-relative. Thanks.

jsnellbaker · 2018-02-09T12:43:06Z

All requested changes should be in place at this time. Please take a look again and let me know. Thanks.

* initial commit to support pbs vendor config options * correcting a value in a test case * adding rubicon config info and updated var names * made changes to unit tests, changed cookieSet default, and fixed small cookieSet issue * tweaking logic of main check to read it better * adjusting rubicon vendor values and default config to fix issue * undoing default config change * additional changes to rubicon defaults

jsnellbaker added 2 commits January 25, 2018 14:24

initial commit to support pbs vendor config options

1832bdc

correcting a value in a test case

b1f3823

jsnellbaker requested a review from mkendall07 January 26, 2018 11:03

jsnellbaker mentioned this pull request Jan 29, 2018

Rubicon s2sConfig information #2079

Closed

bretg requested changes Jan 29, 2018

View reviewed changes

bretg assigned mkendall07 Jan 29, 2018

bretg added needs review needs docs labels Jan 29, 2018

jsnellbaker mentioned this pull request Jan 30, 2018

added information on new s2sConfig option prebid/prebid.github.io#579

Merged

adding rubicon config info and updated var names

c1c6462

ghost assigned jsnellbaker Jan 30, 2018

ghost added the in progress label Jan 30, 2018

bretg reviewed Jan 30, 2018

View reviewed changes

mkendall07 unassigned jsnellbaker Jan 30, 2018

made changes to unit tests, changed cookieSet default, and fixed smal…

ee6bdb4

…l cookieSet issue

mkendall07 reviewed Feb 2, 2018

View reviewed changes

mkendall07 approved these changes Feb 2, 2018

View reviewed changes

mkendall07 added needs update and removed needs review labels Feb 2, 2018

jsnellbaker added 3 commits February 2, 2018 09:56

tweaking logic of main check to read it better

5488050

adjusting rubicon vendor values and default config to fix issue

7e8ec2c

undoing default config change

6bc107d

bretg requested changes Feb 5, 2018

View reviewed changes

additional changes to rubicon defaults

b901fc1

bretg approved these changes Feb 9, 2018

View reviewed changes

bretg added reviewed and removed needs docs needs update labels Feb 9, 2018

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

mkendall07 merged commit b16071c into master Feb 12, 2018

mkendall07 removed the in progress label Feb 12, 2018

matthewlane deleted the pbs_vendor_config_opt branch February 15, 2018 17:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Option to use a configurable vendor preset for s2sConfig #2073

Option to use a configurable vendor preset for s2sConfig #2073

jsnellbaker commented Jan 26, 2018 •

edited

Loading

bretg Jan 29, 2018

jsnellbaker Jan 29, 2018

jsnellbaker Jan 30, 2018

jsnellbaker commented Jan 30, 2018

bretg Jan 30, 2018

jsnellbaker Jan 30, 2018

mkendall07 Jan 31, 2018

jsnellbaker Jan 31, 2018

mkendall07 Feb 2, 2018

bretg Feb 2, 2018

bretg Feb 2, 2018

mkendall07 Feb 2, 2018 •

edited

Loading

jsnellbaker Feb 2, 2018

bretg Feb 5, 2018

jsnellbaker commented Feb 9, 2018

Option to use a configurable vendor preset for s2sConfig #2073

Option to use a configurable vendor preset for s2sConfig #2073

Conversation

jsnellbaker commented Jan 26, 2018 • edited Loading

Type of change

Description of change

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsnellbaker commented Jan 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkendall07 Feb 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsnellbaker commented Feb 9, 2018

jsnellbaker commented Jan 26, 2018 •

edited

Loading

mkendall07 Feb 2, 2018 •

edited

Loading