Filter Multi-Format Requests #4162
Conversation
bsardo
changed the title
| @@ -398,8 +398,11 @@ func (e *exchange) HoldAuction(ctx context.Context, r *AuctionRequest, debugLog | |||
| } else if r.Account.AlternateBidderCodes != nil { | |||
| alternateBidderCodes = *r.Account.AlternateBidderCodes | |||
| } | |||
|
|
|||
| preferredMediaType := getBidderPreferredMediaTypeMap(requestExtPrebid, &r.Account) | |||
There was a problem hiding this comment.
Under this logic, we would create a openrtb_ext.PreferedMediaType map every time a request comes in, even if a particular instance of Prebid Server has no adapters configured with openrtb2.multiformat-supported field set to false. Can we modify so we check request.ext.prebid.BidderControls[BidderName] and account.PreferredMediaType[biddername] only when needed? Maybe we can implement a singleFormatAdapters map so we can check in constant time O(1) whether or not there's a prefered type:
722 func (e *exchange) getAllBids(
723 ctx context.Context,
724 *-- 13 lines: bidderRequests []BidderRequest,-------------------------------------------------------------------------
737 preferredMediaType openrtb_ext.PreferedMediaType) (
738 map[openrtb_ext.BidderName]*entities.PbsOrtbSeatBid,
739 map[openrtb_ext.BidderName]*seatResponseExtra,
740 extraAuctionResponseInfo) {
741 // Set up pointers to the bid results
742 *-- 6 lines: adapterBids := make(map[openrtb_ext.BidderName]*entities.PbsOrtbSeatBid, len(bidderRequests))-----------
748
749 for _, bidder := range bidderRequests {
750 // Here we actually call the adapters and collect the bids.
751 bidderRunner := e.recoverSafely(bidderRequests, func(bidderRequest BidderRequest, conversions currency.Conversions) {
752 // Passing in aName so a doesn't change out from under the go routine
753 *-- 12 lines: if bidderRequest.BidderLabels.Adapter == "" {-----------------------------------------------------------
765
766 reqInfo := adapters.NewExtraRequestInfo(conversions)
767 reqInfo.PbsEntryPoint = bidderRequest.BidderLabels.RType
768 reqInfo.GlobalPrivacyControlHeader = globalPrivacyControlHeader
769 - reqInfo.PreferredMediaType = preferredMediaType[bidder.BidderName]
+ if len(singleFormatBidders) > 0 { // O(1)
+ if _, found := singleFormatBidders[bidder.BidderName]; found { // check in O(1)
+ // check in request.ext.prebid.BidderControls[BidderName]` and `account.PreferredMediaType[biddername]`
+ if _, found := extRequest.Prebid.BidderControls[bidder.BidderName]; found { // O(1)
+ reqInfo.PreferredMediaType = preferredMediaType[bidder.BidderName]
+ } else if _, found := account.PreferredMediaType[bidder.BidderName]; found { // O(1)
+ reqInfo.PreferredMediaType = preferredMediaType[bidder.BidderName]
+ }
+ }
+ }
770
exchange/exchange.go
request.ext.prebid.BidderControls[BidderName] and account.PreferredMediaType[biddername] are already defined as maps and creating the singleFormatBidders map shouldn't add time complexity either:
31 func buildBidders(infos config.BidderInfos, builders map[openrtb_ext.BidderName]adapters.Builder, server config.Server) (map[openrtb_ext.BidderName]adapters.Bidder, []error) {
32 bidders := make(map[openrtb_ext.BidderName]adapters.Bidder)
+ singleFormatBidders := make(map[openrtb_ext.BidderName]bool)
33 var errs []error
34
35 for bidder, info := range infos {
36 bidderName, bidderNameFound := openrtb_ext.NormalizeBidderName(bidder)
37 *-- 17 lines: if !bidderNameFound {-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
54
55 if info.IsEnabled() {
56 adapterInfo := buildAdapterInfo(info)
57 bidderInstance, builderErr := builder(bidderName, adapterInfo, server)
58
59 if builderErr != nil {
60 errs = append(errs, fmt.Errorf("%v: %v", bidder, builderErr))
61 continue
62 }
63 bidders[bidderName] = adapters.BuildInfoAwareBidder(bidderInstance, info)
+
+ if !bidders[bidderName].info.multiformat {
+ singleFormatBidders[bidderName] = true
+ }
64 }
65 }
66 - return bidders, errs
+ return bidders, ingleFormatBidders, errs
67 }
exchange/adapter_util.go
We could probably pass it into the exchange:
220 - adapters, adaptersErrs := exchange.BuildAdapters(generalHttpClient, cfg, cfg.BidderInfos, r.MetricsEngine)
+ adapters, singleFormatAdapters, adaptersErrs := exchange.BuildAdapters(generalHttpClient, cfg, cfg.BidderInfos, r.MetricsEngine)
221 if len(adaptersErrs) > 0 {
222 *-- 12 lines: errs := errortypes.NewAggregateError("Failed to initialize adapters", adaptersErrs) ---------------------------------
235 macroReplacer := macros.NewStringIndexBasedReplacer()
236 - theExchange := exchange.NewExchange(adapters, cacheClient, cfg, requestValidator, syncersByBidder, r.MetricsEngine, cfg.BidderInfos, gdprPermsBuilder, rateConvertor, categoriesFetcher, adsCertSigner, macroReplacer, priceFloorFetcher)
+ theExchange := exchange.NewExchange(adapters, singleFormatAdapters, cacheClient, cfg, requestValidator, syncersByBidder, r.MetricsEngine, cfg.BidderInfos, gdprPermsBuilder, rateConvertor, categoriesFetcher, adsCertSigner, macroReplacer, priceFloorFetcher)
router/router.go
Thoughts?
There was a problem hiding this comment.
I found your suggestions really thoughtful as they avoid creating the PreferredMediaType map every time a request comes in. However, I was wondering how I can access the account config and request.ext.prebid inside the getAllBids method? Do you have any suggestions on how to efficiently access this data?
There was a problem hiding this comment.
As an alternative, we can also update the getBidderPreferredMediaTypeMap to accept both singleFormatBidders and liveBidder list. [only if above approach is not possible]
The function will first check if singleFormatBidders is empty and return immediately to avoid unnecessary processing.
Otherwise, it will iterate only through the liveBidders list and check for the preferred media type, ensuring efficiency by focusing only on bidders present in the request that do not support multi-format.
func getBidderPreferredMediaTypeMap(prebid *openrtb_ext.ExtRequestPrebid, account *config.Account, liveBidders []openrtb_ext.BidderName, singleFormatBidders map[openrtb_ext.BidderName]bool) openrtb_ext.PreferredMediaType {
preferredMediaType := make(openrtb_ext.PreferredMediaType)
if len(singleFormatBidders) == 0 {
return preferredMediaType
}
for _, liveBidder := range liveBidders {
// Skip if the bidder is not present in singleFormatBidders
if !singleFormatBidders[liveBidder] {
continue
}
//read preferred media type from request, if not present read from account
if prebid != nil && prebid.BidderControls != nil && prebid.BidderControls[liveBidder].PreferredMediaType != "" {
preferredMediaType[liveBidder] = prebid.BidderControls[liveBidder].PreferredMediaType
} else if account != nil && account.PreferredMediaType != nil && account.PreferredMediaType[liveBidder] != "" {
preferredMediaType[liveBidder] = account.PreferredMediaType[liveBidder]
}
}
return preferredMediaType
}
There was a problem hiding this comment.
Yes @pm-isha-bharti I agree. In case len(singleFormatBidders) equals zero, can we return nil? Let's make sure we nil check the preferredMediaType map downstream.
There was a problem hiding this comment.
@pm-isha-bharti I agree with the modification to getBidderPreferredMediaTypeMap you suggest above and, in case len(singleFormatBidders) equals zero, can we return nil? Let's also make sure we nil check the preferredMediaType map downstream. In Go, len(myMap) checks for both nil and lenght.
A couple more asks in order to test this feature more comprenhensively:
-
In directory
endpoints/openrtb2/sample-requestswe have end-to-end JSON tests. Can you add a test insideendpoints/openrtb2/sample-requests/valid-whole/exemplaryand another inendpoints/openrtb2/sample-requests/invalid-wholethat deal with the newly added field? We can configure the mock bidders with a prefered media type inendpoints/openrtb2/test_utils.go:946 // mockBidderHandler carries mock bidder server information that will be read from JSON test files 947 // and defines a handle function of a a mock bidder service. 948 type mockBidderHandler struct { 949 BidderName string `json:"bidderName"` 950 Currency string `json:"currency"` 951 Price float64 `json:"price"` 952 DealID string `json:"dealid"` 953 Seat string `json:"seat"` + PreferedMediaType string `json:"preferedmtype"` 954 } endpoints/openrtb2/test_utils.goThis is where the end-to-end tests exchange gets configured:
1222 // buildTestExchange returns an exchange with mock bidder servers and mock currency conversion server 1223 func buildTestExchange(testCfg *testConfigValues, adapterMap map[openrtb_ext.BidderName]exchange.AdaptedBidder, mockBidServersArray []*httptest.Server, mockCurrencyRatesServer *httptest.Server, bidderInfos config.BidderInfos, cfg *config.Configuration, met metrics.MetricsEngine, mockFetcher stored_requests.CategoryFetcher, requestValidator ortb.RequestValidator) (exchange.Exchange, []*httptest.Server) { 1224 if len(testCfg.MockBidders) == 0 { 1225 testCfg.MockBidders = append(testCfg.MockBidders, mockBidderHandler{BidderName: "appnexus", Currency: "USD", Price: 0.00}) 1226 } + singleFormatBidders := make(map[openrtb_ext.BidderName]struct{}) 1227 for _, mockBidder := range testCfg.MockBidders { 1228 bidServer := httptest.NewServer(http.HandlerFunc(mockBidder.bid)) 1229 bidderAdapter := mockAdapter{mockServerURL: bidServer.URL, seat: mockBidder.Seat} 1230 bidderName := openrtb_ext.BidderName(mockBidder.BidderName) 1231 1232 adapterMap[bidderName] = exchange.AdaptBidder(&bidderAdapter, bidServer.Client(), &config.Configuration{}, &metricsConfig.NilMetricsEngine{}, bidderName, nil, "") 1233 mockBidServersArray = append(mockBidServersArray, bidServer) + if len(testCfg.MockBidders.PreferedMediaType) > 0 { + singleFormatBidders[bidderName] = testCfg.MockBidders.PreferedMediaType + } 1234 } 1235 1236 *-- 6 lines: mockCurrencyConverter := currency.NewRateConverter(mockCurrencyRatesServer.Client(), mockCurrencyRatesServer.URL, time.Second)-------------------------------------------------------------------------------------------------- 1242 1243 testExchange := exchange.NewExchange(adapterMap, 1244 1245 &wellBehavedCache{}, 1246 cfg, 1247 requestValidator, 1248 nil, 1249 met, 1250 bidderInfos, 1251 gdprPermsBuilder, 1252 mockCurrencyConverter, 1253 mockFetcher, 1254 &adscert.NilSigner{}, 1255 macros.NewStringIndexBasedReplacer(), 1256 nil, + singleFormatBidders, 1257 ) endpoints/openrtb2/test_utils.goSo our new JSON test cases, say a hypothetical new JSON test called
endpoints/openrtb2/sample-requests/valid-whole/exemplary/prefered-media-type.json, can make use of this field and assert this feature:1 { 2 "description": "A new JSON test to assert a bidder's prefered media type", 3 "config": { 4 "mockBidders": [ 5 { 6 "bidderName": "appnexus", 7 "currency": "USD", + "preferedmediatype": "video", 8 "price": 0.00 9 } 10 ] 11 }, -
On top of these tests, can you also add
biddercontrolstoendpoints/openrtb2/sample-requests/valid-whole/exemplary/all-ext.json?endpoints/openrtb2/sample-requests/valid-whole/exemplary/all-ext.jsonis a text case meant to exemplify a request with all of the fields a requestextcan come with
|
|
||
| //if bidder doesnt support multiformat, send only preferred media type in the request | ||
| if !i.info.multiformat { | ||
| var newErrs []error |
There was a problem hiding this comment.
Nitpick: can we rename newErrs to formatErrs or something more descriptive?
| //if bidder doesnt support multiformat, send only preferred media type in the request | ||
| if !i.info.multiformat { | ||
| var newErrs []error | ||
| request.Imp, newErrs = FilterMultiformatImps(request, reqInfo.PreferredMediaType) |
There was a problem hiding this comment.
Just wondering: do we need a new function to filter these imps altogether? There's already imp filtering happening a couple of lines below. Would it be hard to add that functionality above?
57 // Filtering imps is quite expensive (array filter with large, non-pointer elements)... but should be rare,
58 // because it only happens if the publisher makes a really bad request.
59 //
60 // To avoid allocating new arrays and copying in the normal case, we'll make one pass to
61 // see if any imps need to be removed, and another to do the removing if necessary.
62 -> numToFilter, errs := pruneImps(request.Imp, allowedMediaTypes) <-- CAN WE DECIDE WHICH ONES TO FILTER HERE?
63
64 // If all imps in bid request come with unsupported media types, exit
65 if numToFilter == len(request.Imp) {
66 return nil, append(errs, &errortypes.Warning{Message: "Bid request didn't contain media types supported by the bidder"})
67 }
68
69 if numToFilter != 0 {
70 // Filter out imps with unsupported media types
71 -> filteredImps, newErrs := filterImps(request.Imp, numToFilter) <-- AND FILTER HERE SO WE DON'T NEED FilterMultiformatImps() below?
72 request.Imp = filteredImps
73 errs = append(errs, newErrs...)
74 }
75
76 - //if bidder doesnt support multiformat, send only preferred media type in the request
77 - if !i.info.multiformat {
78 - var newErrs []error
79 - request.Imp, newErrs = FilterMultiformatImps(request, reqInfo.PreferredMediaType)
80 - if newErrs != nil {
81 - errs = append(errs, newErrs...)
82 - }
83 - }
84
adapters/infoawarebidder.go
Implements 2711, 3342
If a multi-format request is received for a bidder in which
openrtb2.multiformat-supportedisfalse, the following algorithm will be implemented for each impression:ext.prebid.biddercontrols.BIDDER.prefmtype):account.preferredmediatype.BIDDERcontains a preferred type:If
openrtb2.multiformat-supportedis not defined for the bidder, then default value will betruealways i.e each bidder explicitly supports multi-format requests unless definedfalsein the bidder info yaml.request-level overrides
Request level field
ext.prebid.bidder.BIDDER.prefmtypeaccount-level overrides
bidder multiformat support flag
In bidder static YAML/config: