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

Multi-Bid Support #1715

Closed
bretg opened this issue Feb 18, 2021 · 13 comments
Closed

Multi-Bid Support #1715

bretg opened this issue Feb 18, 2021 · 13 comments

Comments

@bretg
Copy link
Contributor

bretg commented Feb 18, 2021

Copying the Prebid Server-related requirements from prebid/Prebid.js#6317 -- go there for the use cases.

Multi-Bid Requirements

  1. PBS must be able to parse the ext.prebid.multibid config and allow the defined number of additional bids on seatbid.bid. If the number of bids allowed is less than 1, treat it as 1. If greater than 9, treat it as 9.
  2. The extra bids must have different bidids in ext.prebid.bidid
  3. The bid responses should support ad server targeting on the extra bids in the same way as PBJS: the biddercode used for the targeting of the extra bids should be definable as a targetbiddercodeprefix that incorporates a dynamic 'index' starting from 2. e.g. "bidA2, bidA3". Practically speaking, the base of the dynamic aliases will need to be 5 characters or less because the longest base attribute name is 14 chars (hb_cache_host_) and attributes are truncated at 20 chars.
    a) The extra bids receive the 'aliased' targets only if ext.prebid.targeting.includebidderkeys is specified and true
  4. The bidresponse 'seat' should be associated with the primary biddercode, but store the resolved targeting biddercode in seatbid.bid.ext.prebid.targetbiddercode.
  5. If the multibid config doesn't define a targetbiddercodepattern, no ad targeting should be generated for the extra bids. Just send them as additional seatbid.bid entries under the appropriate imp.
  6. PBS analytics adapters may choose how to represent the extra bids: either with the primary biddercode, the targetbiddercode, or both.
  7. In the future, we may want to allow account-level multi-bid config, but for now the config is only part of the incoming request or the stored request.
  8. A multi-bid response still counts as one response for the purpose of metrics.
  9. There are no additional logging or tracing requirements.
  10. Extra bids must be cached in PBC when appropriate.
  11. The system should support specifying a single bidder, in which case targetbiddercodeprefix is allowed. Or the user can specify multiple bidders, in which case targetbiddercodeprefix is not allowed. This is to support the scenario where multiple bids are allowed but without targeting variables.
  12. If targetbiddercodeprefix is not specified, Prebid Server should still emit any additional bids but without setting targeting variables.
  13. Validations:
    • If a given biddercode is specified multiple times in bidder and/or bidders, use the first bidder and add warning when in debug mode
    • If maxbids is not specified, ignore whole block and add warning when in debug mode
    • if bidders and targetbiddercodeprefix are both present, ignore targetbiddercodeprefix and add warning when in debug mode
    • if both 'bidder' and 'bidders' are both present in a single block, ignore 'bidders' and add warning when in debug mode
  14. Bid adapters should know how many bids they're allowed to submit. i.e. they should have access to ext.prebid.multibid, but of course only the entry that involves them.

The proposed PBS OpenRTB Request enhancement is essentially the same as the PBJS config:

{
  ...
  ext: {
    prebid: {
      multibid: [{
        bidder: "bidderA",
        maxbids: 2,
        targetbiddercodeprefix: "bidA"
      },{
        bidder: "bidderB",
        maxbids: 3,
        targetbiddercodeprefix: "bidB"
      },{
        bidders: ["bidderC", "bidderD"]
        maxbids: 2
      }]
    }
  }
}

So for example, if a request came in with the above ext.prebid.multibid, and bidderA supplies 3 bids, the system would limit them to the first 2 and the output would look like:

{
seatbid: [{
  seat: "bidderA",
  bid: [{
    id: "bid1",
    impid: "imp1",
    price: 1.04,
    ext: {
        prebid: {
            targeting: {
                hb_pb_bidderA: 1.00
            },
            targetbiddercode: "bidderA"
        }
    }
    ...
  },{
    id: "bid2",
    impid: "imp1",   // same imp as above
    price:0.8,
    ext: {
        prebid: {
            targeting: {
                hb_pb_bidA2: 0.50
            },
            targetbiddercode: "bidA2"
        }
    }
    ...
  }]
},{
  seat: "bidderC",
  bid: [{
    id: "bid1",
    impid: "imp1",
    price: 1.17,
    ext: {
        prebid: {
            targeting: {
                hb_pb_bidderC: 1.10
            },
            targetbiddercode: "bidderC"
        }
    }

    ...
  },{
    id: "bid2",
    impid: "imp1",
    // no targeting on extra bids for this bidder because
    // targetbiddercodepattern wasn't specified in ext.prebid.multibid
    ...
    },{
}]
}
@bretg bretg changed the title Multi-bid Support Multi-Bid Support Feb 18, 2021
@bretg
Copy link
Contributor Author

bretg commented Feb 22, 2021

Updated based on community feedback:

  • changed targetBiddercodePattern to targetBiddercodePrefix.
  • the index will automatically be appended and start from 2

@bretg
Copy link
Contributor Author

bretg commented Mar 8, 2021

docs PR prebid/prebid.github.io#2757

@bretg
Copy link
Contributor Author

bretg commented Mar 12, 2021

Done in PBS-Java 1.59

@bretg bretg added the PBS-Go label Mar 12, 2021
@SyntaxNode SyntaxNode added the Intent to implement An issue describing a plan for a major feature. These are intended for community feedback label Apr 19, 2021
@bretg bretg added Ready For Dev Feature specification is ready to be developed. and removed feature request Intent to implement An issue describing a plan for a major feature. These are intended for community feedback labels May 23, 2022
@bretg
Copy link
Contributor Author

bretg commented Jun 7, 2022

The removal of extra bids has been discussed in slack. Here's a proposed additional clarification/requirement:

The focus of the multibid feature is in the definition of the targeting KVPs. However, PBS-Java currently snips off extra bids and that's the way we like it. But PBS-Go's default is to let all extra bids through without KVPs. Which means we either need to debate or make it configurable.

  1. PBS-core should optionally remove extra seatbid.bid entries from the bidresponse with host- and account-level config. e.g. auction.default-bid-limit-min
    • account-level config takes precedence over host-level config
    • if neither is specified overall default is 0, which means "no limit"
    • Valid values are 0 to MAXINT
  2. The multibid feature applies at the seat level, not the bid adapter level. i.e. with the new 'multiple biddercode' feature (Conventions and controls for adapters responding with multiple biddercodes #2174), bid adapters may be allowed to bid multiple times (under different names) without running afoul of any multibid rules.

Note: the way this is implemented in PBS-Java is:

bidLimit = multiBid != null ? multiBid.getMaxBids() : DEFAULT_BID_LIMIT_MIN;

@SyntaxNode SyntaxNode added In Dev - Go and removed Ready For Dev Feature specification is ready to be developed. labels Sep 7, 2022
@bretg bretg removed the projectboard label Sep 8, 2022
pm-nilesh-chate added a commit to pm-nilesh-chate/prebid-server that referenced this issue Nov 25, 2022
pm-nilesh-chate added a commit to pm-nilesh-chate/prebid-server that referenced this issue Nov 25, 2022
@bretg bretg moved this from Triage to Ready for Dev in Prebid Server Prioritization Dec 16, 2022
@bretg bretg moved this from Ready for Dev to In Progress in Prebid Server Prioritization Dec 16, 2022
@mansinahar
Copy link
Contributor

mansinahar commented Feb 6, 2023

@bretg What's the rationale behind having both multibid.bidder and multibid.bidders ? IMHO having both multibid.bidder and multibid.bidders is strange and confusing. The only reason I see is to support targetbiddercodeprefix for a single bidder and not for multiple bidders but this can be easily accomplished in the code by validating the presence of targetbiddercodeprefix based on the length of the multibid.bidders array. Am I missing something else here?

Also, the above spec suggests that multibid.maxbids value can only be between 1 and 9. However, the host config default_bid_limit_min can have values between 0 and MAX_INT. Could you please explain your thought process behind having different ranges for these signals? To me it looks like they should support the same range as they both control the maximum number of bids allowed to passthrough for a given bidder/all bidders. It's just the source of these signals that is different.

@bretg
Copy link
Contributor Author

bretg commented Feb 6, 2023

IMHO having both multibid.bidder and multibid.bidders is strange and confusing.

It was for JSON validation

The system should support specifying a single bidder, in which case targetbiddercodeprefix is allowed. Or the user can specify multiple bidders, in which case targetbiddercodeprefix is not allowed. This is to support the scenario where multiple bids are allowed but without targeting variables.

i.e. if bidders is specified, then targetbiddercodeprefix is not allowed. This seemed cleaner than stating "bidders array contains only 1 entry, targetbiddercodeprefix is allowed."

This sort of disappointment is why folks are encouraged to comment during the review period. Yes, I get that it's a lot of work to review the specs and not everyone has time to read all of them. It's also a lot of work to write them. :-). If you strongly object, please open a new issue and we can discuss in PBS committee

@bretg
Copy link
Contributor Author

bretg commented Feb 24, 2023

@bsardo asks:

It appears from the discussion on Multi-Bid Support that the spirit of the feature is targeting, however, are there other use cases for it? The answer here might influence whether we choose to always snip extra bids or make that a config option.

Multibid was all the rage for a bit, but is low priority now. The use case was that certain DSPs wanted their bids to flow all the way through to the ad server. That's it.

What I'd personally like to see happen is that we snip off extra bids, but I've accepted that because PBS-Go always returned all bids, that the feature is about targeting.

The config option auction.default-bid-limit-min you’re describing in this comment appears to be a suggestion solely to address the possibility that PBS-Go might want to still allow passing through extra bids (no snipping), is that correct?

Yes.

If we add that config option would you like to have some influence over what it is called? I wasn’t sure if PBS-Java would have any interest in it.

Go and Java config is so divergent at this point it doesn't matter to me. If we do take on a config-consistency project, this will be one of hundreds of options to consider.

Assuming that config option is solely to turn on/off extra bid snipping, that is different from the account-level multi-bid config described in requirement 7 on the issue right? “In the future, we may want to allow account-level multi-bid config, but for now the config is only part of the incoming request or the stored request”. I assume this type of config would be to allow each account to specify maxbids and targetbiddercodeprefix for specific bidders similar to how it works at the request level.

Yes, the intention of Req 7 was to have account-level defaults for the request params. It might make sense to also have the 'snipping' config be an account config, but that wasn't mentioned in this issue, so we can save that for the future should it be needed.

@VeronikaSolovei9
Copy link
Contributor

@bretg Bret, can you please confirm what exact result we expect in next use case.

default_bid_limit: 10
in request.ext.prebid.multibid.maxbids (for bidder): 3
In response from bidder there are 5 bids back (per imp).

What is expected result?

  1. There are total 3 bids back and all of them have targeting keys according to multibid config
  2. There are total 5 bids back and only 3 of them have targeting keys according to multibid config

Another question: if req.ext.prebid.targeting is not specified but request.ext.prebid.multibid is specified: should request.ext.prebid.multibid config be ignored?

@bretg
Copy link
Contributor Author

bretg commented Feb 27, 2023

I don't know what you mean by 'default_bid_limit'. Perhaps you mean 'default_bid_limit_min'?

Here's what requirement 15 says:

PBS-core should optionally remove extra seatbid.bid entries from the bidresponse with host- and account-level config. e.g. auction.default-bid-limit-min

So in your example:

default_bid_limit_min: 10
in request.ext.prebid.multibid.maxbids (for bidder): 3
In response from bidder there are 5 bids back (per imp).

Here's what happens:

  1. There are 5 seatbid.bid entries from that seat
  2. 3 of seatbid.bid entries contain targeting, 2 do not

I admit the name default_bid_limit_min is pretty confusing. I wish people would push back harder when these things are written.

@VeronikaSolovei9
Copy link
Contributor

VeronikaSolovei9 commented Feb 27, 2023

Thank you for the quick response!

Ok, it means code works correct in the PR.

Yes, I took the config name from the PR, config/accounts.go :

DefaultBidLimit   int  `mapstructure:"default_bid_limit" json:"default_bid_limit"`

https://github.com/prebid/prebid-server/blob/d5795e202bc3adf2b2a2abcfe42765be8ed3cfca/config/accounts.go, line 39
Should it be renamed to default_bid_limit_min? It's very confusing, because it's max not min...

@bretg
Copy link
Contributor Author

bretg commented Feb 27, 2023

default_bid_limit is fine. Thanks.

@VeronikaSolovei9
Copy link
Contributor

Thank you!

@SyntaxNode
Copy link
Contributor

Implemented in PBS-Go 0.245.0.

@github-project-automation github-project-automation bot moved this from In Progress to Done in Prebid Server Prioritization Apr 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

4 participants