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

s2sConfig requires at least one bidder when allowUnknownBidderCodes is true #8668

Closed
SSaboFS opened this issue Jul 11, 2022 · 12 comments · Fixed by #9470
Closed

s2sConfig requires at least one bidder when allowUnknownBidderCodes is true #8668

SSaboFS opened this issue Jul 11, 2022 · 12 comments · Fixed by #9470
Assignees
Labels

Comments

@SSaboFS
Copy link

SSaboFS commented Jul 11, 2022

Type of issue

Question/Feature request

Description

When defining the s2sConfig a bidders array is required before a request to Prebid Server is sent despite allowUnknownBidderCodes being true. How can we use the ortb2Imp property on adUnits were we have defined client side & unknown server side bidders without having to include a bidder in s2sConfig?

We would like to use something like the example below to define client side bidders & a stored request ID on the same adUnit, however no request to Prebid Server is sent as no bidders array is included in s2sConfig

pbjs.addAdUnits([{
  code: 'example-stored-request',
  bids: [
    {
      bidder: someClientSideBidder,
      params: {someClientSideParams}
    }
  ]
  ortb2Imp: {
    ext: {
      prebid: {
        storedrequest: {
          id: 'stored-request-id'
        }
      }
    }
  }
}])

pbjs.setConfig({
    s2sConfig: [{
        accountId: '1001',
        endpoint: 'https://mypbs.example.com/path',
        syncEndpoint: 'https://mypbs.example.com/path',
        timeout: 300,
        allowUnkownBidderCodes: true
    }]
})

We can get the above working by using a 'dummy' bidder with no params to trigger the request to Prebid Server. Using a dummy bidder here feels redundant if the ortb2Imp & allowUnkownBidderCodes: true properties are both defined -

pbjs.addAdUnits([{
  code: 'example-stored-request',
  bids: [
    {
      bidder: 'someClientSideBidder',
      params: {someClientSideParams...}
    },
   {
      bidder: 'dummyS2SBidder'
   }
  ]
  ortb2Imp: {
    ext: {
      prebid: {
        storedrequest: {
          id: 'stored-request-id'
        }
      }
    }
  }
}])

pbjs.setConfig({
    s2sConfig: [{
        accountId: '1001',
        endpoint: 'https://mypbs.example.com/path',
        syncEndpoint: 'https://mypbs.example.com/path',
        timeout: 300,
        bidders: ['dummyS2SBidder']
        allowUnkownBidderCodes: true
    }]
})

Is there currently a way around this or is this expected behaviour?

@dgirardi
Copy link
Collaborator

the ext.prebid.storedrequest is forwarded to all bidders, not just Prebid Server. It happens to be ignored by most client adapters now, but not all, and more are likely to pick it up in the future, so I'm not sure that it should be used to mean "run both a client-side auction, and a PBS stored request".

Maybe we need a way to specify ortb2Imp by adUnit and bidder? so that your example might look like:

pbjs.addAdUnits([{
    code: 'example-stored-request',
    bids: [
        {
            bidder: 'someClientSideBidder',
            params: {someClientSideParams...}
        },
        {
            bidder: null, // or other placeholder for S2S
            ortb2Imp: {
                ext: {
                    prebid: {
                        storedrequest: {
                            id: 'pbs-request-id'
                        }
                    }
                },
            }
        },
        {
            bidder: 'someOtherBidderThatDoesStoredRequests',
            ortb2Imp: {
                ext: {
                    prebid: {
                        storedrequest: {
                            id: 'other-request-id'
                        }
                    }
                },
            }
        }
    ]
}])

@muuki88
Copy link
Collaborator

muuki88 commented Jul 11, 2022

I really like your suggestion @dgirardi . We have similar requirements in the future.

The prebidServerAdapter is a valid bidder, so why not give it a proper bidderCode after all ( null is so java in the ninetees 😅 )

This could open the doors for a lot of potential improvements and customizations.

@dgirardi
Copy link
Collaborator

The PBS adapter is not really a bidder - it only looks like one when using stored requests, but it general, it can't do anything unless you give it other bidders to funnel to the server. Internally it does not make sense to give it a bidder code (what would it mean to do pbjs.bidderSettings.prebidServer = ..., or pbjs.setBidderConfig({bidders: ['prebidServer'], config: ...})?)

But I won't argue about the aesthetics - we could give it a special code that means "this is a stored request/response", but is not valid as a bidder code elsewhere; or we could ask for something like s2sOnly: true instead of bidder: null

@bretg bretg self-assigned this Jul 12, 2022
@bretg
Copy link
Collaborator

bretg commented Jul 13, 2022

Seems to me there are two issues here:

  1. When should the pbsBidAdapter call Prebid Server in the "less" scenario?
  2. stored-request-IDs are not currently supported per-bidder and there are scenarios where that could cause a problem

When should the pbsBidAdapter call Prebid Server in the "less" scenario?

Dug up the history on this. This comment seems to be the most relevant:

#7027 (comment)

Discussed with @dgirardi - we agreed that the signal for the pbsBidAdapter to emit a stored-imp-only imp object should be implicit: if an AdUnit contains no bids array and does contain ortb2Imp, it will add the imp object.

We considered having an explicit signal (e.g. s2sConfig.bidders: ["*"]) but realized that could get confusing in a scenario where some adunits are server-only and some are a mix of client and server.

What's exposed by this issue is that our first answer didn't address the "mix of client- and server-adapters" scenario... it required absolutely no bids array. Ok. Here's a cut at evolving the model -- how about we generalize the bids[] object with a destination 'module'? This would be similar to the idea behind s2sConfig.adapter -- that there could be a competing (slimmed down?) pbsBidAdapter someday.

pbjs.addAdUnits([{
  code: 'example-stored-request',
  ortb2Imp: { ... cross-bidder params ... }, 
  bids: [
    {
      module: "pbsBidAdapter",    // instead of 'bidder'
      params: {
          ortb2Imp: {                      // or we could just put this in s2sConfig?
            ext: {
              prebid: {
                storedrequest: {
                  id: 'stored-request-id'
                }
             }
          }
        }
    },
    { ... other actual bidders ... }
  ]
  }
}])

Then the pbsBidAdapter would have several sources of data for each imp:

  1. imp.ext.prebid.bidder.BIDDER as defined by the intersection of s2sConfig.bidders and bidders in the adunit
  2. defined by adunit-level ortb2Imp when the adunit.bids array is empty
  3. if the adunit.bids array exists and adunit.bids.module=pbsBidAdapter.params.ortb2Imp exists, use that

stored-request-IDs are not currently supported per-bidder

We have an issue where some bidders would consume any globally-defined SRIDs and might choke on them. e.g. Rubicon. While it's not intended that a global ortb2Imp.ext.prebid.storedrequest would be forwarded to the Rubicon video endpoint, it would, and would likely cause problems.

Options:

  1. standardize that storedrequest data should be defined mainly in the new 'module: pbsBidAdapter' as proposed above. And if a regular bid adapter wants to support a storedrequest, they should take it as a bidder-specific param.
  2. require that adapters sensitive to this config deal with it. e.g. the rubicon adapter could be modified to ignore a global ext.prebid.storedrequest.
  3. both. (I think this is best for the short-term.)

It's possible we might someday need per-bidder ortb2Imp, but IMO, storedrequests are not a strong use case for the syntactic headache.

@bretg
Copy link
Collaborator

bretg commented Jul 14, 2022

Discussed with @dgirardi and @patmmccann. Revised proposal:

  1. changing the default JSON would be a breaking change for 7.x. This can be done in 8.x but not before.
  2. In the meantime, supporting an override would be good.
  3. The override needs to support multiple s2sConfigs across multiple impressions.
  4. We propose adding an optional 'configName' field to s2sConfig.
  5. This 'configName' can be referred to in the adunit
  6. s2sConfig.bidders becomes optional
  7. If PBS bid adapter can't find anything to add an imp, it just skips the imp. If there aren't any imps for PBS in the auction, it won't send the request.
pbjs.setConfig({
  s2sConfig: [{
    configName: "pbs1",
    ...
  },{
    configName: "pbs2",
    ...
  }]
});

pbjs.addAdUnits([{
  code: 'example-stored-request',
  ortb2Imp: { ... cross-bidder params ... }, 
  bids: [
    {
      module: "pbsBidAdapter",    // instead of 'bidder'
      params: {
          configName: "pbs1",
          ortb2Imp: {
            ext: {
              prebid: {
                storedrequest: {
                  id: 'stored-request-id'
                }
             }
          }
        }
    },
    { ... other actual bidders ... }
  ]
  }
}])

Here's how PBS tries to figure out what to put in the ORTB:

1) loop through s2sConfig array entries
2) loop through all AdUnits in the auction
    A) if there aren't any bids[] in the adunit but there is an ortb2Imp, then merge that into the imp
    B) else loop through all bids[] in the adunit
        i) If s2sConfig.bidders contains that bidder, add it to imp.ext.prebid.bidder.BIDDER
        ii) if adunit.bids.module=pbsBidAdapter
            a) if params.configName exists, then make sure it matches the current s2sConfig.configName
            b) if params.ortb2Imp exists, merge that into the imp
    C) If there's no imp defined for this s2sConfig entry, don't sent that request

@muuki88
Copy link
Collaborator

muuki88 commented Jul 15, 2022

Sorry I missed the discussion in the PMC 😞 In general I agree with the outcome. However I'm a little nervous about the multiple additional parameters when from my point of view is everything in place.

I want to pick up @dgirardi statement

The PBS adapter is not really a bidder - it only looks like one when using stored requests, but it general, it can't do anything unless you give it other bidders to funnel to the server

I argue that the PBS Adapter is a real bidder. The main difference IMHO is that instead of returning everything with the same bidder code, it transparently returns the bidder that won the auction. There are couple of SSPs that trade bid requests to other SSPs, but hide the fact. The Xandr client adpater also works with prebid server (if I got this right) and then returns the winning bidder.

With #8154 (adding allowUnknownBidderCodes) this behaviour is now officially available. And yes, somethings don't work as easy as before (#8629).

What I'm aiming at. The s2sConfig.defaultVendor already allows configuring a bidder code. So a config may look like

pbjs.setConfig({
  s2sConfig: [{
    defaultVendor: "my-bidder",
    ...
  },{
    defaultVendor: "rubicon",
    ...
  }]
});

and the ad unit would reference this vendor (fair enough, bidderCode would be a better property name)

pbjs.addAdUnits([{
  code: 'example-stored-request',
  ortb2Imp: { ... cross-bidder params ... }, 
  bids: [
    {
      bidder: "my-bidder", 
      params: {

          ortb2Imp: {
            ext: {
              prebid: {
                storedrequest: {
                  id: 'stored-request-id'
                }
             }
          }
        }
    },
    { ... other actual bidders ... }
  ]
  }
}])

And from an internal implementation point of view, the pbsBidAdapter registers itself for every config with the specified defaultVendor (bidderCode).

Yes, this treats the pbsBidAdapter as a regular bidder. It's a generic one that can be configured at will. I couldn't find the issue where it states that openRTB logic should be refactored and reused by adapters as more and more adapters simply call an OpenRTB endpoint.

So a generic, pbs server adapter that speaks OpenRTB would replace some adapters entirely. You could either configure an SSP like in the example above or the client side adapter does nothing more than registering a new instance of the pbsServerAdapter (we may re-implement a new one, calling it OpenRtbAdapter) with the necessary config parameters.

This would be a huge step towards #5001

@bretg
Copy link
Collaborator

bretg commented Jul 15, 2022

@muuki88 - I have some reservations about some details in your suggestion, but in the end I think we're close to saying the same thing: it should be possible to name a particular s2sConfig entry from an adunit.

My concerns:

  1. terminology: calling PBS a 'bidder' is potentially confusing. Yes, it shares some of the same interfaces, but it is a trusted module that: (A) has access to bidder-specific config (B) has config way more complicated than a bidder (C) has a separate cookie_sync implementation (D) sets up win-url events.
  2. s2sConfig.defaultVendor is optional and without hacking, only has values for appnexus, rubicon, and openx.

FWIW, the "defaultVendor" feature has been bothering me for some time. It's not ideal that these 3 SSPs are enshrined in the pbsBidAdapter module when there are many dozens of PBS host companies. The PBJS package carries around these extra URLs whether the pub uses them or not. If 20+ PBS host companies insisted on being in https://github.com/prebid/Prebid.js/blob/master/modules/prebidServerBidAdapter/config.js, that would add up to a bunch of unnecessary bytes. Maybe we could utilize the new FEATURES thing? Something like this:

if (FEATURES.pbs-rubicon) {
    mergeConfig({
        S2S_VENDORS: {
             'rubicon': { ... }
        }
    })
}

But anyhow, I get that it's kinda ugly to have both "defaultVendor" and "configName". They overlap for sure. How about if configName were optional in s2sConfig and the AdUnit configName would look for either it or defaultVendor? This would support the scenario where a PBS vendor isn't configured as one of the enshrined defaults.

@patmmccann patmmccann moved this from Triage to Needs Req in Prebid.js Tactical Issues table Jul 18, 2022
@patmmccann patmmccann moved this from Needs Req to Ready for Dev in Prebid.js Tactical Issues table Jul 20, 2022
@bretg
Copy link
Collaborator

bretg commented Jul 20, 2022

Here's another, perhaps quicker solution to the specific use case brought up by FreeStar:

if s2sConfig.bidders is non-existent or empty AND a global ortb2.ext.prebid.storedrequest exists, then pbsBidAdapter will ignore the AdUnit.bids and send just the ext.prebid.storedrequest

This is probably easier from a publisher perspective than having to modify the AdUnit.bids to have a specific PBS entry.

@dgirardi
Copy link
Collaborator

The stored request would be "global" only to the adUnit - from ortb2Imp. That's quicker to implement, and easier for the publisher.

But it's still a workaround - essentially just syntactic sugar for the one in the OP, and I think in the long run it will create confusion. As soon as there's more than one party interested in stored requests for the same adUnit, it will not make sense to treat it that way, but potentially breaking to stop doing it.

Because the existing workaround is not that onerous, my sense is that it'd be better to ride on it for now and aim for the larger proposal - what do you think @SSaboFS ?

@bretg
Copy link
Collaborator

bretg commented Jul 20, 2022

Spoke with @nicgallardo -- seems like perhaps the 'explicit signal add PBS as a bidder' might work for them. One question is going to be about validations... should pbsBidAdapter allow imp to go out that are just imp.id and imp.{banner,video,native}? Freestar might be making a custom PBS-side module to support their use case, which would mean they don't need imp.ext.prebid at all. So I'd say no validations needed on the JS side.

@bretg
Copy link
Collaborator

bretg commented Oct 18, 2022

Reviewed this thread and trying to summarize the various use cases. This is an upgrade of was done before to support the harder scenarios.

Here are the assumptions:

  • we need to support multiple Prebid Servers... i.e. an array of s2sConfig entries
  • we need to support different storedrequest IDs for each PBS
  • we need to support a mix of client-side and server-side bidders
  • the 'defaultVendor' concept is dated and will go away someday
  • we need to be able to initiate a PBS request even if s2sConfig doesn't define bidders

Scenario 0: Backwards compatible. if s2sConfig specifies bidders and the AdUnit bids array doesn't specify any 'modules', everything works as it does today.

Scenario 1: Prebid.less with multiple PBS. AdUnit bidder list defines only the two Prebid Servers and PBS-specific ortb2imp.ext.prebid.storedrequest.id

(Note: for the case of 1 PBS, just imagine removing the 'pbs2' entries.)

pbjs.setConfig({
  s2sConfig: [{
    configName: "pbs1",
    endpoint: "url1",    // or defaultVendor
    allowUnknownBidderCodes: true
    // no bidders
    ...
  },{
    configName: "pbs2",
    endpoint: "url2",    // or defaultVendor
    allowUnknownBidderCodes: true
    ...
  }]
});

pbjs.addAdUnits([{
  code: 'example-prebid-less',
  mediaTypes: { banner: { sizes: [[300, 250]]}},
  ortb2Imp: {
    ext: { gpid: "homepage" }
  },
  bids: [{
      module: "pbsBidAdapter",    // 'module' instead of 'bidder'
      // future bid destination modules may have other params
      params: { configName: "pbs1" },
      ortb2Imp: {
            ext: { prebid: { storedrequest: { id: 'stored-request-id-pbs1' }}}
      }
    },{
      module: "pbsBidAdapter",
      params: { configName: "pbs2" },
      ortb2Imp: {
            ext: { prebid: { storedrequest: { id: 'stored-request-id-pbs2' }}}
      }
   }
}]
  1. pbsBidAdapter builds template ORTB for both pbs1 and pbs2 that includes all the usual details (page info, FPD, etc.).
  2. pbsBidAdapter merges in the 'global' AdUnit[].ortb2Imp into both ORTB blocks. This applies to both pbs1 and pbs2.
  3. pbsBidAdapter sees that AdUnit[].bids[0].module is "pbsBidAdapter", and that AdUnit[].bids[0].params.configName is "pbs1", so it merges AdUnit[].bids[0].ortb2Imp into the ORTB for pbs1 for this adunit/imp. It follows the same process AdUnit[].bids[1] for pbs2.

The imp sent to pbs1 would look like:

  imp: {
    banner: { ... },
    ext: { 
      gpid: "homepage",
      prebid: { storedrequest: {id: "stored-request-id-pbs1"}}
    }
  }

Scenario 2: Mixed client- and server-side bidders with multiple PBS. Server-side bidders are specified in the s2sConfig, but there's also PBS-specific storedrequest in the AdUnit.

pbjs.setConfig({
  s2sConfig: [{
    configName: "pbs1",
    endpoint: "url1",    // or defaultVendor
    allowUnknownBidderCodes: true,
    bidders: ["bidderA"]
    ...
  },{
    configName: "pbs2",
    endpoint: "url2",    // or defaultVendor
    allowUnknownBidderCodes: true,
    bidders: ["bidderB"]
    ...
  }]
});

pbjs.addAdUnits([{
  code: 'example-mixed',
  mediaTypes: { banner: { sizes: [[300, 250]]}},
  ortb2Imp: {
    ext: { gpid: "homepage" }
  },
  bids: [{
      module: "pbsBidAdapter",
      params: { configName: "pbs1" },
      ortb2Imp: {
            ext: { prebid: { storedrequest: { id: 'stored-request-id-pbs1' }}}
      }
    },{
      module: "pbsBidAdapter",
      params: { configName: "pbs2" },
      ortb2Imp: {
            ext: { prebid: { storedrequest: { id: 'stored-request-id-pbs2' }}}
      }
   },{
     bidder: "bidderA",
     params: { placement: "A" }
   },{
     bidder: "bidderB",
     params: { pid: "B" }
   }
}]
  1. pbsBidAdapter builds template ORTB for both pbs1 and pbs2 that includes all the usual details (page info, FPD, etc.).
  2. pbsBidAdapter merges in the 'global' AdUnit[].ortb2Imp into both ORTB blocks. This applies to both pbs1 and pbs2.
  3. pbsBidAdapter sees that AdUnit[].bids[0].module is "pbsBidAdapter", and that AdUnit[].bids[0].params.configName is "pbs1", so it merges AdUnit[].bids[0].ortb2Imp into the ORTB for pbs1 for this adunit/imp. It follows the same process AdUnit[].bids[1] for pbs2.
  4. pbsBidAdapter sees that bidderA goes to pbs1 so adds it to that ORTB. Likewise, bidderB goes into the ORTB for pbs2.

The imp sent to pbs1 would look like:

  imp: {
    banner: { ... },
    ext: { 
      gpid: "homepage",
      prebid: {
        storedrequest: {id: "stored-request-id-pbs1"},
        bidder: {
          bidderA: { placement: "A" }
        }
      }
    }
  }

I think these two scenarios and the proposed meet all of the constraints. It's true that the syntax is not as simple as we'd like it to be, but I don't think we can simplify the syntax without breaking some use cases.

@dgirardi dgirardi moved this from Ready for Dev to Blocked in Prebid.js Tactical Issues table Oct 20, 2022
@patmmccann patmmccann moved this from Blocked to Ready for Dev in Prebid.js Tactical Issues table Oct 24, 2022
@dgirardi
Copy link
Collaborator

I'd like to expand this proposal to also handle bidder-specific ortb2Imp in general (not just in relation to Prebid Server). The use case for this is imp.pmp, there's this discussion for PBS, but to me it looks like it would apply to client bidders as well.

pbjs.addAdUnits([{
  code: 'example-pmp',
  mediaTypes: { banner: { sizes: [[300, 250]]}},
  ortb2Imp: {
    ext: { gpid: "homepage" }
  },
  bids: [{
     bidder: "bidderA",
     orbt2Imp: {
          pmp: { deals: [{id: "A"}]}
     }
   },{
     bidder: "bidderB",
     orbt2Imp: {
          pmp: { deals: [{id: "B"}]}
     }
   }
}]

This would generate client-side bid requests like {bidder: "bidderA", ortb2Imp: {ext: { gpid: "homepage" }, pmp: {deals: [{id: "A"}]}}}. In the s2s general case, where it's mixed with module-level settings:

pbjs.setConfig({
   s2sConfig: {
    bidders: ["bidderA", "bidderB"]
    ...
  }
});
pbjs.addAdUnits([{
  code: 'example-pmp-s2s',
  mediaTypes: { banner: { sizes: [[300, 250]]}},
  ortb2Imp: {
    ext: { gpid: "homepage" }
  },
  bids: [{
      module: "pbsBidAdapter",
      ortb2Imp: {
            ext: { prebid: { storedrequest: { id: 'stored-request-id-pbs' }}}
      }
    }, {
     bidder: "bidderA",
     ortb2Imp: {
          pmp: { deals: [{id: "A"}]}
     }
   },{
     bidder: "bidderB",
     ortb2Imp: {
          pmp: { deals: [{id: "B"}]}
     }
   }
}]

it would merge module - level ortb2Imp with the adUnit, and keep bidder-level ortb2Imp in imp.ext.prebid.imp (as detailed in prebid/prebid-server#2335):

{
  imp: [
     {
       ...
       ext: {
           gpid: "homepage",
           prebid: {
               storedrequest: { id: 'stored-request-id-pbs' },
               imp: {
                   bidderA: { pmp: { deals: [{id: "A"}]} },
                   bidderB: { pmp: { deals: [{id: "B"}]} },
               }
           }
       }
     }
  ]
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
4 participants