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

Bugfix: no bids from bidder handling #1252

Merged
merged 6 commits into from
Apr 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 21 additions & 9 deletions exchange/exchange.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -322,6 +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)
if bids != nil {
ae.HttpCalls = bids.httpCalls
}

// Timing statistics
e.me.RecordAdapterTime(*bidlabels, time.Since(start))
serr := errsToBidderErrors(err)
Expand All @@ -344,7 +351,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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the consequences of removing the adapter bids entirely (do the metrics still record properly?) versus setting the bids to an empty slice?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to carry bidder data if there are no bids back. For loops will skip empty array iterating and nothing bad will happen. This is just memory and processor time optimization. Hope this makes sense.

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 {
Expand Down Expand Up @@ -637,20 +649,20 @@ func (e *exchange) makeExtBidResponse(adapterBids map[openrtb_ext.BidderName]*pb
}
}

for a, b := range adapterBids {
if b != nil && req.Test == 1 {
// Fill debug info
bidResponseExt.Debug.HttpCalls[a] = b.httpCalls
for bidderName, responseExtra := range adapterExtra {

if req.Test == 1 {
bidResponseExt.Debug.HttpCalls[bidderName] = responseExtra.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(responseExtra.Errors) > 0 {
bidResponseExt.Errors[bidderName] = responseExtra.Errors
}
if len(errList) > 0 {
bidResponseExt.Errors[openrtb_ext.PrebidExtKey] = errsToBidderErrors(errList)
}
bidResponseExt.ResponseTimeMillis[a] = adapterExtra[a].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
Expand Down
19 changes: 16 additions & 3 deletions exchange/exchange_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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.
Expand Down Expand Up @@ -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,
}
}

Expand Down
227 changes: 227 additions & 0 deletions exchange/exchangetest/request-multi-bidders-debug-info.json
Original file line number Diff line number Diff line change
@@ -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
}
}
}
}
}
}
}
}
}


Loading