From b3bf8b6211ae54023262749f9b5277e92f642680 Mon Sep 17 00:00:00 2001 From: Veronika Solovei Date: Wed, 8 Apr 2020 15:28:13 -0700 Subject: [PATCH 1/6] Bugfix: no bids from bidder handling --- exchange/exchange.go | 21 ++- .../request-multi-bidders-one-no-resp.json | 169 ++++++++++++++++++ 2 files changed, 183 insertions(+), 7 deletions(-) create mode 100644 exchange/exchangetest/request-multi-bidders-one-no-resp.json diff --git a/exchange/exchange.go b/exchange/exchange.go index 3cab1880456..2c82157aff8 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -344,7 +344,12 @@ func (e *exchange) getAllBids(ctx context.Context, cleanRequests map[openrtb_ext // Wait for the bidders to do their thing for i := 0; i < len(cleanRequests); i++ { brw := <-chBids - adapterBids[brw.bidder] = brw.adapterBids + + //if bidder returned no bids back - remove bidder from further processing + if brw.adapterBids != nil && len(brw.adapterBids.bids) != 0 { + adapterBids[brw.bidder] = brw.adapterBids + } + //but we need to add all bidders data to adapterExtra to have metrics and other metadata adapterExtra[brw.bidder] = brw.adapterExtra if !bidsFound && adapterBids[brw.bidder] != nil && len(adapterBids[brw.bidder].bids) > 0 { @@ -637,19 +642,21 @@ func (e *exchange) makeExtBidResponse(adapterBids map[openrtb_ext.BidderName]*pb } } - for a, b := range adapterBids { - if b != nil && req.Test == 1 { + for a, b := range adapterExtra { + + bid := adapterBids[a] + if bid != nil && req.Test == 1 { // Fill debug info - bidResponseExt.Debug.HttpCalls[a] = b.httpCalls + bidResponseExt.Debug.HttpCalls[a] = bid.httpCalls } // Only make an entry for bidder errors if the bidder reported any. - if len(adapterExtra[a].Errors) > 0 { - bidResponseExt.Errors[a] = adapterExtra[a].Errors + if len(b.Errors) > 0 { + bidResponseExt.Errors[a] = b.Errors } if len(errList) > 0 { bidResponseExt.Errors[openrtb_ext.PrebidExtKey] = errsToBidderErrors(errList) } - bidResponseExt.ResponseTimeMillis[a] = adapterExtra[a].ResponseTimeMillis + bidResponseExt.ResponseTimeMillis[a] = b.ResponseTimeMillis // Defering the filling of bidResponseExt.Usersync[a] until later } diff --git a/exchange/exchangetest/request-multi-bidders-one-no-resp.json b/exchange/exchangetest/request-multi-bidders-one-no-resp.json new file mode 100644 index 00000000000..f184d139e09 --- /dev/null +++ b/exchange/exchangetest/request-multi-bidders-one-no-resp.json @@ -0,0 +1,169 @@ +{ + "incomingRequest": { + "ortbRequest": { + "id": "some-request-id", + "site": { + "page": "test.somepage.com" + }, + "imp": [ + { + "id": "my-imp-id", + "video": { + "mimes": ["video/mp4"] + }, + "ext": { + "appnexus": { + "placementId": 1 + }, + "audienceNetwork": { + "placementId": "some-placement" + } + } + }, + { + "id": "imp-id-2", + "video": { + "mimes": ["video/mp4"] + }, + "ext": { + "appnexus": { + "placementId": 2 + }, + "audienceNetwork": { + "placementId": "some-other-placement" + } + } + } + ], + "ext": { + "prebid": { + "targeting": {} + } + } + } + }, + "outgoingRequests": { + "appnexus": { + "mockResponse": { + "pbsSeatBid": { + "pbsBids": [ + { + "ortbBid": { + "id": "winning-bid", + "impid": "my-imp-id", + "price": 0.71, + "w": 200, + "h": 250, + "crid": "creative-1" + }, + "bidType": "video" + }, + { + "ortbBid": { + "id": "losing-bid", + "impid": "my-imp-id", + "price": 0.21, + "w": 200, + "h": 250, + "crid": "creative-2" + }, + "bidType": "video" + }, + { + "ortbBid": { + "id": "other-bid", + "impid": "imp-id-2", + "price": 0.61, + "w": 300, + "h": 500, + "crid": "creative-3" + }, + "bidType": "video" + } + ] + } + } + }, + "audienceNetwork": { + "mockResponse": { + "pbsSeatBid": { + + } + } + } + }, + "response": { + "bids": { + "id": "some-request-id", + "seatbid": [ + { + "seat": "appnexus", + "bid": [{ + "id": "winning-bid", + "impid": "my-imp-id", + "price": 0.71, + "w": 200, + "h": 250, + "crid": "creative-1", + "ext": { + "prebid": { + "type": "video", + "targeting": { + "hb_bidder": "appnexus", + "hb_bidder_appnexus": "appnexus", + "hb_cache_host": "www.pbcserver.com", + "hb_cache_host_appnex": "www.pbcserver.com", + "hb_cache_path": "/pbcache/endpoint", + "hb_cache_path_appnex": "/pbcache/endpoint", + "hb_pb": "0.70", + "hb_pb_appnexus": "0.70", + "hb_size": "200x250", + "hb_size_appnexus": "200x250" + } + } + } + }, + { + "id": "losing-bid", + "impid": "my-imp-id", + "price": 0.21, + "w": 200, + "h": 250, + "crid": "creative-2", + "ext": { + "prebid": { + "type": "video" + } + } + }, + { + "id": "other-bid", + "impid": "imp-id-2", + "price": 0.61, + "w": 300, + "h": 500, + "crid": "creative-3", + "ext": { + "prebid": { + "type": "video", + "targeting": { + "hb_bidder": "appnexus", + "hb_bidder_appnexus": "appnexus", + "hb_cache_host": "www.pbcserver.com", + "hb_cache_host_appnex": "www.pbcserver.com", + "hb_cache_path": "/pbcache/endpoint", + "hb_cache_path_appnex": "/pbcache/endpoint", + "hb_pb": "0.60", + "hb_pb_appnexus": "0.60", + "hb_size": "300x500", + "hb_size_appnexus": "300x500" + } + } + } + } + ] + } + ] + } + } +} From 3b513cfd369ea7e21f3564fcbe2dd1626c7cdbca Mon Sep 17 00:00:00 2001 From: Veronika Solovei Date: Wed, 8 Apr 2020 23:52:12 -0700 Subject: [PATCH 2/6] Fixed unit test json file: 2 bids where one bid returns nil --- .../request-multi-bidders-one-no-resp.json | 103 +++++------------- 1 file changed, 28 insertions(+), 75 deletions(-) diff --git a/exchange/exchangetest/request-multi-bidders-one-no-resp.json b/exchange/exchangetest/request-multi-bidders-one-no-resp.json index f184d139e09..b7179ccb02e 100644 --- a/exchange/exchangetest/request-multi-bidders-one-no-resp.json +++ b/exchange/exchangetest/request-multi-bidders-one-no-resp.json @@ -37,7 +37,15 @@ ], "ext": { "prebid": { - "targeting": {} + "targeting": { + "durationRangeSec": [15,30], + "includebrandcategory": { + "primaryadserver": 1, + "publisher": "", + "withCategory": true, + "translateCategories": true + } + } } } } @@ -51,34 +59,14 @@ "ortbBid": { "id": "winning-bid", "impid": "my-imp-id", - "price": 0.71, - "w": 200, - "h": 250, - "crid": "creative-1" - }, - "bidType": "video" - }, - { - "ortbBid": { - "id": "losing-bid", - "impid": "my-imp-id", - "price": 0.21, + "price": 12.00, "w": 200, "h": 250, - "crid": "creative-2" - }, - "bidType": "video" - }, - { - "ortbBid": { - "id": "other-bid", - "impid": "imp-id-2", - "price": 0.61, - "w": 300, - "h": 500, - "crid": "creative-3" - }, - "bidType": "video" + "crid": "creative-1", + "cat": [ + "IAB1-1" + ] + } } ] } @@ -86,9 +74,7 @@ }, "audienceNetwork": { "mockResponse": { - "pbsSeatBid": { - } } } }, @@ -98,16 +84,18 @@ "seatbid": [ { "seat": "appnexus", - "bid": [{ + "bid": [ + { "id": "winning-bid", "impid": "my-imp-id", - "price": 0.71, + "price": 12.00, "w": 200, "h": 250, "crid": "creative-1", + "cat": ["IAB1-1"], "ext": { "prebid": { - "type": "video", + "type": "", "targeting": { "hb_bidder": "appnexus", "hb_bidder_appnexus": "appnexus", @@ -115,55 +103,20 @@ "hb_cache_host_appnex": "www.pbcserver.com", "hb_cache_path": "/pbcache/endpoint", "hb_cache_path_appnex": "/pbcache/endpoint", - "hb_pb": "0.70", - "hb_pb_appnexus": "0.70", "hb_size": "200x250", - "hb_size_appnexus": "200x250" + "hb_size_appnexus": "200x250", + "hb_pb": "12.00", + "hb_pb_appnexus": "12.00", + "hb_pb_cat_dur": "12.00_VideoGames_15s", + "hb_pb_cat_dur_appnex": "12.00_VideoGames_15s" } } } - }, - { - "id": "losing-bid", - "impid": "my-imp-id", - "price": 0.21, - "w": 200, - "h": 250, - "crid": "creative-2", - "ext": { - "prebid": { - "type": "video" - } - } - }, - { - "id": "other-bid", - "impid": "imp-id-2", - "price": 0.61, - "w": 300, - "h": 500, - "crid": "creative-3", - "ext": { - "prebid": { - "type": "video", - "targeting": { - "hb_bidder": "appnexus", - "hb_bidder_appnexus": "appnexus", - "hb_cache_host": "www.pbcserver.com", - "hb_cache_host_appnex": "www.pbcserver.com", - "hb_cache_path": "/pbcache/endpoint", - "hb_cache_path_appnex": "/pbcache/endpoint", - "hb_pb": "0.60", - "hb_pb_appnexus": "0.60", - "hb_size": "300x500", - "hb_size_appnexus": "300x500" - } - } - } - } - ] + }] } ] } } } + + From 6e420cb27b5f424bd63f475f09087710e421df0e Mon Sep 17 00:00:00 2001 From: Veronika Solovei Date: Thu, 9 Apr 2020 02:01:55 -0700 Subject: [PATCH 3/6] Fixed the case where we have http calls, but lose it it there are no bids back --- exchange/exchange.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/exchange/exchange.go b/exchange/exchange.go index 2c82157aff8..992c245caa7 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -63,6 +63,9 @@ type exchange struct { type seatResponseExtra struct { ResponseTimeMillis int Errors []openrtb_ext.ExtBidderError + // httpCalls is the list of debugging info. It should only be populated if the request.test == 1. + // This will become response.ext.debug.httpcalls.{bidder} on the final Response. + httpCalls []*openrtb_ext.ExtHttpCall } type bidResponseWrapper struct { @@ -322,6 +325,7 @@ func (e *exchange) getAllBids(ctx context.Context, cleanRequests map[openrtb_ext // Structure to record extra tracking data generated during bidding ae := new(seatResponseExtra) ae.ResponseTimeMillis = int(elapsed / time.Millisecond) + ae.httpCalls = bids.httpCalls // Timing statistics e.me.RecordAdapterTime(*bidlabels, time.Since(start)) serr := errsToBidderErrors(err) @@ -644,10 +648,9 @@ func (e *exchange) makeExtBidResponse(adapterBids map[openrtb_ext.BidderName]*pb for a, b := range adapterExtra { - bid := adapterBids[a] - if bid != nil && req.Test == 1 { + if req.Test == 1 { // Fill debug info - bidResponseExt.Debug.HttpCalls[a] = bid.httpCalls + bidResponseExt.Debug.HttpCalls[a] = b.httpCalls } // Only make an entry for bidder errors if the bidder reported any. if len(b.Errors) > 0 { From b544eeaf7bbf14cc3882db43905b7ebfdd57c89d Mon Sep 17 00:00:00 2001 From: Veronika Solovei Date: Thu, 9 Apr 2020 14:28:08 -0700 Subject: [PATCH 4/6] Added support of httpCalls to exchange test, added unit test to test there is still debug info for case where in multi-bidder call one bidder returns no data --- exchange/exchange.go | 5 +- exchange/exchange_test.go | 19 +- ...quest-multi-bidders-one-no-resp-test1.json | 227 ++++++++++++++++++ 3 files changed, 247 insertions(+), 4 deletions(-) create mode 100644 exchange/exchangetest/request-multi-bidders-one-no-resp-test1.json diff --git a/exchange/exchange.go b/exchange/exchange.go index 992c245caa7..78f7cf51720 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -325,7 +325,10 @@ func (e *exchange) getAllBids(ctx context.Context, cleanRequests map[openrtb_ext // Structure to record extra tracking data generated during bidding ae := new(seatResponseExtra) ae.ResponseTimeMillis = int(elapsed / time.Millisecond) - ae.httpCalls = bids.httpCalls + if request.Test == 1 && bids != nil { + ae.httpCalls = bids.httpCalls + } + // Timing statistics e.me.RecordAdapterTime(*bidlabels, time.Since(start)) serr := errsToBidderErrors(err) diff --git a/exchange/exchange_test.go b/exchange/exchange_test.go index 2f115ca4f93..53e03ecbb16 100644 --- a/exchange/exchange_test.go +++ b/exchange/exchange_test.go @@ -766,6 +766,11 @@ func runSpec(t *testing.T, filename string, spec *exchangeSpec) { } } } + if spec.IncomingRequest.OrtbRequest.Test == 1 { + //compare debug info + diffJson(t, "Debug info modified", bid.Ext, spec.Response.Ext) + + } } func findBiddersInAuction(t *testing.T, context string, req *openrtb.BidRequest) []string { @@ -1614,6 +1619,7 @@ type exchangeRequest struct { type exchangeResponse struct { Bids *openrtb.BidResponse `json:"bids"` Error string `json:"error,omitempty"` + Ext json.RawMessage `json:"ext,omitempty"` } type bidderSpec struct { @@ -1627,8 +1633,9 @@ type bidderRequest struct { } type bidderResponse struct { - SeatBid *bidderSeatBid `json:"pbsSeatBid,omitempty"` - Errors []string `json:"errors,omitempty"` + SeatBid *bidderSeatBid `json:"pbsSeatBid,omitempty"` + Errors []string `json:"errors,omitempty"` + HttpCalls []*openrtb_ext.ExtHttpCall `json:"httpCalls,omitempty"` } // bidderSeatBid is basically a subset of pbsOrtbSeatBid from exchange/bidder.go. @@ -1685,7 +1692,13 @@ func (b *validatingBidder) requestBid(ctx context.Context, request *openrtb.BidR } seatBid = &pbsOrtbSeatBid{ - bids: bids, + bids: bids, + httpCalls: mockResponse.HttpCalls, + } + } else { + seatBid = &pbsOrtbSeatBid{ + bids: nil, + httpCalls: mockResponse.HttpCalls, } } diff --git a/exchange/exchangetest/request-multi-bidders-one-no-resp-test1.json b/exchange/exchangetest/request-multi-bidders-one-no-resp-test1.json new file mode 100644 index 00000000000..ec174f75b36 --- /dev/null +++ b/exchange/exchangetest/request-multi-bidders-one-no-resp-test1.json @@ -0,0 +1,227 @@ +{ + "incomingRequest": { + "ortbRequest": { + "test": 1, + "id": "some-request-id", + "site": { + "page": "test.somepage.com" + }, + "imp": [ + { + "id": "my-imp-id", + "video": { + "mimes": [ + "video/mp4" + ] + }, + "ext": { + "appnexus": { + "placementId": 1 + }, + "audienceNetwork": { + "placementId": "some-placement" + } + } + }, + { + "id": "imp-id-2", + "video": { + "mimes": [ + "video/mp4" + ] + }, + "ext": { + "appnexus": { + "placementId": 2 + }, + "audienceNetwork": { + "placementId": "some-other-placement" + } + } + } + ], + "ext": { + "prebid": { + "targeting": { + "durationRangeSec": [ + 15, + 30 + ], + "includebrandcategory": { + "primaryadserver": 1, + "publisher": "", + "withCategory": true, + "translateCategories": true + } + } + } + } + } + }, + "outgoingRequests": { + "appnexus": { + "mockResponse": { + "httpCalls": [ + { + "uri": "appnexusTest.com", + "requestbody": "appnexusTestRequestBody", + "responsebody": "appnexusTestResponseBody", + "status": 200 + } + ], + "pbsSeatBid": { + "pbsBids": [ + { + "ortbBid": { + "id": "winning-bid", + "impid": "my-imp-id", + "price": 12.00, + "w": 200, + "h": 250, + "crid": "creative-1", + "cat": [ + "IAB1-1" + ] + } + } + ] + } + } + }, + "audienceNetwork": { + "mockResponse": { + "httpCalls": [ + { + "uri": "audienceNetworkTest.com", + "requestbody": "audienceNetworkTestRequestBody", + "responsebody": "audienceNetworkTestResponseBody", + "status": 200 + } + ] + } + } + }, + "response": { + "bids": { + "id": "some-request-id", + "seatbid": [ + { + "seat": "appnexus", + "bid": [ + { + "id": "winning-bid", + "impid": "my-imp-id", + "price": 12.00, + "w": 200, + "h": 250, + "crid": "creative-1", + "cat": [ + "IAB1-1" + ], + "ext": { + "prebid": { + "type": "", + "targeting": { + "hb_bidder": "appnexus", + "hb_bidder_appnexus": "appnexus", + "hb_cache_host": "www.pbcserver.com", + "hb_cache_host_appnex": "www.pbcserver.com", + "hb_cache_path": "/pbcache/endpoint", + "hb_cache_path_appnex": "/pbcache/endpoint", + "hb_size": "200x250", + "hb_size_appnexus": "200x250", + "hb_pb": "12.00", + "hb_pb_appnexus": "12.00", + "hb_pb_cat_dur": "12.00_VideoGames_15s", + "hb_pb_cat_dur_appnex": "12.00_VideoGames_15s" + } + } + } + } + ] + } + ] + }, + "ext": { + "debug": { + "httpcalls": { + "appnexus": [ + { + "uri": "appnexusTest.com", + "requestbody": "appnexusTestRequestBody", + "responsebody": "appnexusTestResponseBody", + "status": 200 + } + ], + "audienceNetwork": [ + { + "uri": "audienceNetworkTest.com", + "requestbody": "audienceNetworkTestRequestBody", + "responsebody": "audienceNetworkTestResponseBody", + "status": 200 + } + ] + }, + "resolvedrequest": { + "id": "some-request-id", + "imp": [ + { + "id": "my-imp-id", + "video": { + "mimes": [ + "video/mp4" + ] + }, + "ext": { + "appnexus": { + "placementId": 1 + }, + "audienceNetwork": { + "placementId": "some-placement" + } + } + }, + { + "id": "imp-id-2", + "video": { + "mimes": [ + "video/mp4" + ] + }, + "ext": { + "appnexus": { + "placementId": 2 + }, + "audienceNetwork": { + "placementId": "some-other-placement" + } + } + } + ], + "site": { + "page": "test.somepage.com" + }, + "test": 1, + "ext": { + "prebid": { + "targeting": { + "durationRangeSec": [ + 15, + 30 + ], + "includebrandcategory": { + "primaryadserver": 1, + "publisher": "", + "withCategory": true, + "translateCategories": true + } + } + } + } + } + } + } + } +} + + From 319d53012c5babaea3bcfbb8748922ba3ae1c6a6 Mon Sep 17 00:00:00 2001 From: Veronika Solovei Date: Fri, 10 Apr 2020 11:07:42 -0700 Subject: [PATCH 5/6] Changed test file name --- ...e-no-resp-test1.json => request-multi-bidders-debug-info.json} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename exchange/exchangetest/{request-multi-bidders-one-no-resp-test1.json => request-multi-bidders-debug-info.json} (100%) diff --git a/exchange/exchangetest/request-multi-bidders-one-no-resp-test1.json b/exchange/exchangetest/request-multi-bidders-debug-info.json similarity index 100% rename from exchange/exchangetest/request-multi-bidders-one-no-resp-test1.json rename to exchange/exchangetest/request-multi-bidders-debug-info.json From e070505c8095dff9283d0a8254a04245c3bf19ed Mon Sep 17 00:00:00 2001 From: Veronika Solovei Date: Wed, 15 Apr 2020 15:28:13 -0700 Subject: [PATCH 6/6] Minor refactoring --- exchange/exchange.go | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/exchange/exchange.go b/exchange/exchange.go index 78f7cf51720..9ee9d8339e7 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -65,7 +65,7 @@ type seatResponseExtra struct { Errors []openrtb_ext.ExtBidderError // httpCalls is the list of debugging info. It should only be populated if the request.test == 1. // This will become response.ext.debug.httpcalls.{bidder} on the final Response. - httpCalls []*openrtb_ext.ExtHttpCall + HttpCalls []*openrtb_ext.ExtHttpCall } type bidResponseWrapper struct { @@ -325,8 +325,8 @@ func (e *exchange) getAllBids(ctx context.Context, cleanRequests map[openrtb_ext // Structure to record extra tracking data generated during bidding ae := new(seatResponseExtra) ae.ResponseTimeMillis = int(elapsed / time.Millisecond) - if request.Test == 1 && bids != nil { - ae.httpCalls = bids.httpCalls + if bids != nil { + ae.HttpCalls = bids.httpCalls } // Timing statistics @@ -649,21 +649,20 @@ func (e *exchange) makeExtBidResponse(adapterBids map[openrtb_ext.BidderName]*pb } } - for a, b := range adapterExtra { + for bidderName, responseExtra := range adapterExtra { if req.Test == 1 { - // Fill debug info - bidResponseExt.Debug.HttpCalls[a] = b.httpCalls + bidResponseExt.Debug.HttpCalls[bidderName] = responseExtra.HttpCalls } // Only make an entry for bidder errors if the bidder reported any. - if len(b.Errors) > 0 { - bidResponseExt.Errors[a] = b.Errors + if len(responseExtra.Errors) > 0 { + bidResponseExt.Errors[bidderName] = responseExtra.Errors } if len(errList) > 0 { bidResponseExt.Errors[openrtb_ext.PrebidExtKey] = errsToBidderErrors(errList) } - bidResponseExt.ResponseTimeMillis[a] = b.ResponseTimeMillis - // Defering the filling of bidResponseExt.Usersync[a] until later + bidResponseExt.ResponseTimeMillis[bidderName] = responseExtra.ResponseTimeMillis + // Defering the filling of bidResponseExt.Usersync[bidderName] until later } return bidResponseExt