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.

Sign up for GitHub

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

Adding bid rejection messages to debug response #1181

Merged
merged 4 commits into from
Feb 6, 2020
Merged

Adding bid rejection messages to debug response #1181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
940586a
Adding bid rejection messages to debug response
Jan 30, 2020
75ed3ac
Updates from code review comments
Feb 3, 2020
13d63bf
Cleaning up regex in unit test
Feb 4, 2020
7991215
Updating error creation
Feb 5, 2020
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
43 changes: 27 additions & 16 deletions exchange/exchange.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"math/rand"
"net/http"
Expand Down Expand Up @@ -146,10 +147,14 @@ func (e *exchange) HoldAuction(ctx context.Context, bidRequest *openrtb.BidReque
//If includebrandcategory is present in ext then CE feature is on.
if requestExt.Prebid.Targeting != nil && requestExt.Prebid.Targeting.IncludeBrandCategory != nil {
var err error
bidCategory, adapterBids, err = applyCategoryMapping(ctx, requestExt, adapterBids, *categoriesFetcher, targData)
var rejections []string
bidCategory, adapterBids, rejections, err = applyCategoryMapping(ctx, requestExt, adapterBids, *categoriesFetcher, targData)
if err != nil {
return nil, fmt.Errorf("Error in category mapping : %s", err.Error())
}
for _, message := range rejections {
errs = append(errs, errors.New(message))
}
}

auc = newAuction(adapterBids, len(bidRequest.Imp))
Expand Down Expand Up @@ -340,7 +345,7 @@ func (e *exchange) buildBidResponse(ctx context.Context, liveAdapters []openrtb_
return bidResponse, err
}

func applyCategoryMapping(ctx context.Context, requestExt openrtb_ext.ExtRequest, seatBids map[openrtb_ext.BidderName]*pbsOrtbSeatBid, categoriesFetcher stored_requests.CategoryFetcher, targData *targetData) (map[string]string, map[openrtb_ext.BidderName]*pbsOrtbSeatBid, error) {
func applyCategoryMapping(ctx context.Context, requestExt openrtb_ext.ExtRequest, seatBids map[openrtb_ext.BidderName]*pbsOrtbSeatBid, categoriesFetcher stored_requests.CategoryFetcher, targData *targetData) (map[string]string, map[openrtb_ext.BidderName]*pbsOrtbSeatBid, []string, error) {
res := make(map[string]string)

type bidDedupe struct {
Expand All @@ -359,6 +364,7 @@ func applyCategoryMapping(ctx context.Context, requestExt openrtb_ext.ExtRequest
var primaryAdServer string
var publisher string
var err error
var rejections []string
var translateCategories = true

if includeBrandCategory && brandCatExt.WithCategory {
Expand All @@ -370,7 +376,7 @@ func applyCategoryMapping(ctx context.Context, requestExt openrtb_ext.ExtRequest
//if ext.prebid.targeting.includebrandcategory present but primaryadserver/publisher not present then error out the request right away.
primaryAdServer, err = getPrimaryAdServer(brandCatExt.PrimaryAdServer) //1-Freewheel 2-DFP
if err != nil {
return res, seatBids, err
return res, seatBids, rejections, err
}
publisher = brandCatExt.Publisher
}
Expand All @@ -382,6 +388,7 @@ func applyCategoryMapping(ctx context.Context, requestExt openrtb_ext.ExtRequest
bidsToRemove := make([]int, 0)
for bidInd := range seatBid.bids {
bid := seatBid.bids[bidInd]
bidID := bid.bid.ID
var duration int
var category string
var pb string
Expand All @@ -396,6 +403,7 @@ func applyCategoryMapping(ctx context.Context, requestExt openrtb_ext.ExtRequest
//TODO: add metrics
//on receiving bids from adapters if no unique IAB category is returned or if no ad server category is returned discard the bid
bidsToRemove = append(bidsToRemove, bidInd)
rejections = updateRejections(rejections, bidID, "Bid did not contain a category")
continue
}
if translateCategories {
Expand All @@ -405,6 +413,8 @@ func applyCategoryMapping(ctx context.Context, requestExt openrtb_ext.ExtRequest
//TODO: add metrics
//if mapping required but no mapping file is found then discard the bid
bidsToRemove = append(bidsToRemove, bidInd)
reason := fmt.Sprintf("Category mapping file for primary ad server: '%s', publisher: '%s' not found", primaryAdServer, publisher)
rejections = updateRejections(rejections, bidID, reason)
continue
}
} else {
Expand All @@ -424,6 +434,7 @@ func applyCategoryMapping(ctx context.Context, requestExt openrtb_ext.ExtRequest
//if the bid is above the range of the listed durations (and outside the buffer), reject the bid
if duration > durationRange[len(durationRange)-1] {
bidsToRemove = append(bidsToRemove, bidInd)
rejections = updateRejections(rejections, bidID, "Bid duration exceeds maximum allowed")
continue
}
for _, dur := range durationRange {
Expand All @@ -447,11 +458,13 @@ func applyCategoryMapping(ctx context.Context, requestExt openrtb_ext.ExtRequest
if dupe.bidderName == bidderName {
// An older bid from the current bidder
bidsToRemove = append(bidsToRemove, dupe.bidIndex)
rejections = updateRejections(rejections, dupe.bidID, "Bid was deduplicated")
camrice marked this conversation as resolved.
Show resolved Hide resolved
} else {
// An older bid from a different seatBid we've already finished with
oldSeatBid := (seatBids)[dupe.bidderName]
if len(oldSeatBid.bids) == 1 {
seatBidsToRemove = append(seatBidsToRemove, bidderName)
rejections = updateRejections(rejections, dupe.bidID, "Bid was deduplicated")
} else {
oldSeatBid.bids = append(oldSeatBid.bids[:dupe.bidIndex], oldSeatBid.bids[dupe.bidIndex+1:]...)
}
Expand All @@ -460,11 +473,12 @@ func applyCategoryMapping(ctx context.Context, requestExt openrtb_ext.ExtRequest
} else {
// Remove this bid
bidsToRemove = append(bidsToRemove, bidInd)
rejections = updateRejections(rejections, bidID, "Bid was deduplicated")
continue
}
}
res[bid.bid.ID] = categoryDuration
dedupe[categoryDuration] = bidDedupe{bidderName: bidderName, bidIndex: bidInd, bidID: bid.bid.ID}
res[bidID] = categoryDuration
dedupe[categoryDuration] = bidDedupe{bidderName: bidderName, bidIndex: bidInd, bidID: bidID}
}

if len(bidsToRemove) > 0 {
Expand All @@ -483,19 +497,16 @@ func applyCategoryMapping(ctx context.Context, requestExt openrtb_ext.ExtRequest
}

}
if len(seatBidsToRemove) > 0 {
if len(seatBidsToRemove) == len(seatBids) {
//delete all seat bids
seatBids = nil
} else {
for _, seatBidInd := range seatBidsToRemove {
delete(seatBids, seatBidInd)
}

}
for _, seatBidInd := range seatBidsToRemove {
seatBids[seatBidInd].bids = nil
}

return res, seatBids, nil
return res, seatBids, rejections, nil
}

func updateRejections(rejections []string, bidID string, reason string) []string {
message := fmt.Sprintf("bid rejected [bid ID: %s] reason: %s", bidID, reason)
return append(rejections, message)
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
}

func getPrimaryAdServer(adServerId int) (string, error) {
Expand Down
139 changes: 134 additions & 5 deletions exchange/exchange_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"io/ioutil"
"net/http"
"net/http/httptest"
"regexp"
"strconv"
"strings"
"testing"
Expand Down Expand Up @@ -930,9 +931,11 @@ func TestCategoryMapping(t *testing.T) {

adapterBids[bidderName1] = &seatBid

bidCategory, adapterBids, err := applyCategoryMapping(nil, requestExt, adapterBids, categoriesFetcher, targData)
bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, requestExt, adapterBids, categoriesFetcher, targData)

assert.Equal(t, nil, err, "Category mapping error should be empty")
assert.Equal(t, 1, len(rejections), "There should be 1 bid rejection message")
camrice marked this conversation as resolved.
Show resolved Hide resolved
assert.Equal(t, "bid rejected [bid ID: bid_id4] reason: Category mapping file for primary ad server: 'freewheel', publisher: '' not found", rejections[0], "Rejection message did not match expected")
assert.Equal(t, "10.00_Electronics_30s", bidCategory["bid_id1"], "Category mapping doesn't match")
assert.Equal(t, "20.00_Sports_50s", bidCategory["bid_id2"], "Category mapping doesn't match")
assert.Equal(t, "20.00_AdapterOverride_30s", bidCategory["bid_id3"], "Category mapping override from adapter didn't take")
Expand Down Expand Up @@ -983,9 +986,10 @@ func TestCategoryMappingNoIncludeBrandCategory(t *testing.T) {

adapterBids[bidderName1] = &seatBid

bidCategory, adapterBids, err := applyCategoryMapping(nil, requestExt, adapterBids, categoriesFetcher, targData)
bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, requestExt, adapterBids, categoriesFetcher, targData)

assert.Equal(t, nil, err, "Category mapping error should be empty")
assert.Empty(t, rejections, "There should be no bid rejection messages")
assert.Equal(t, "10.00_30s", bidCategory["bid_id1"], "Category mapping doesn't match")
assert.Equal(t, "20.00_40s", bidCategory["bid_id2"], "Category mapping doesn't match")
assert.Equal(t, "20.00_30s", bidCategory["bid_id3"], "Category mapping doesn't match")
Expand Down Expand Up @@ -1034,9 +1038,11 @@ func TestCategoryMappingTranslateCategoriesNil(t *testing.T) {

adapterBids[bidderName1] = &seatBid

bidCategory, adapterBids, err := applyCategoryMapping(nil, requestExt, adapterBids, categoriesFetcher, targData)
bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, requestExt, adapterBids, categoriesFetcher, targData)

assert.Equal(t, nil, err, "Category mapping error should be empty")
assert.Equal(t, 1, len(rejections), "There should be 1 bid rejection message")
assert.Equal(t, "bid rejected [bid ID: bid_id3] reason: Category mapping file for primary ad server: 'freewheel', publisher: '' not found", rejections[0], "Rejection message did not match expected")
assert.Equal(t, "10.00_Electronics_30s", bidCategory["bid_id1"], "Category mapping doesn't match")
assert.Equal(t, "20.00_Sports_50s", bidCategory["bid_id2"], "Category mapping doesn't match")
assert.Equal(t, 2, len(adapterBids[bidderName1].bids), "Bidders number doesn't match")
Expand Down Expand Up @@ -1114,9 +1120,10 @@ func TestCategoryMappingTranslateCategoriesFalse(t *testing.T) {

adapterBids[bidderName1] = &seatBid

bidCategory, adapterBids, err := applyCategoryMapping(nil, requestExt, adapterBids, categoriesFetcher, targData)
bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, requestExt, adapterBids, categoriesFetcher, targData)

assert.Equal(t, nil, err, "Category mapping error should be empty")
assert.Empty(t, rejections, "There should be no bid rejection messages")
assert.Equal(t, "10.00_IAB1-3_30s", bidCategory["bid_id1"], "Category should not be translated")
assert.Equal(t, "20.00_IAB1-4_50s", bidCategory["bid_id2"], "Category should not be translated")
assert.Equal(t, "20.00_IAB1-1000_30s", bidCategory["bid_id3"], "Bid should not be rejected")
Expand Down Expand Up @@ -1179,9 +1186,12 @@ func TestCategoryDedupe(t *testing.T) {

adapterBids[bidderName1] = &seatBid

bidCategory, adapterBids, err := applyCategoryMapping(nil, requestExt, adapterBids, categoriesFetcher, targData)
bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, requestExt, adapterBids, categoriesFetcher, targData)

assert.Equal(t, nil, err, "Category mapping error should be empty")
assert.Equal(t, 2, len(rejections), "There should be 2 bid rejection messages")
assert.Regexpf(t, regexp.MustCompile(`bid rejected \[bid ID: bid_id(1|3)\] reason: Bid was deduplicated`), rejections[0], "Rejection message did not match expected")
assert.Equal(t, "bid rejected [bid ID: bid_id4] reason: Category mapping file for primary ad server: 'freewheel', publisher: '' not found", rejections[1], "Rejection message did not match expected")
assert.Equal(t, 2, len(adapterBids[bidderName1].bids), "Bidders number doesn't match")
assert.Equal(t, 2, len(bidCategory), "Bidders category mapping doesn't match")

Expand All @@ -1196,6 +1206,125 @@ func TestCategoryDedupe(t *testing.T) {
assert.NotEqual(t, numIterations, selectedBids["bid_id3"], "Bid 3 made it through every time")
}

func TestBidRejectionErrors(t *testing.T) {
categoriesFetcher, error := newCategoryFetcher("./test/category-mapping")
if error != nil {
t.Errorf("Failed to create a category Fetcher: %v", error)
}

requestExt := newExtRequest()
requestExt.Prebid.Targeting.DurationRangeSec = []int{15, 30, 50}

targData := &targetData{
priceGranularity: requestExt.Prebid.Targeting.PriceGranularity,
includeWinners: true,
}

invalidReqExt := newExtRequest()
invalidReqExt.Prebid.Targeting.DurationRangeSec = []int{15, 30, 50}
invalidReqExt.Prebid.Targeting.IncludeBrandCategory.PrimaryAdServer = 2
invalidReqExt.Prebid.Targeting.IncludeBrandCategory.Publisher = "some_publisher"

adapterBids := make(map[openrtb_ext.BidderName]*pbsOrtbSeatBid)
bidderName := openrtb_ext.BidderName("appnexus")

testCases := []struct {
description string
reqExt openrtb_ext.ExtRequest
bids []*openrtb.Bid
duration int
expectedRejections []string
expectedCatDur string
}{
{
description: "Bid should be rejected due to not containing a category",
reqExt: requestExt,
bids: []*openrtb.Bid{
{ID: "bid_id1", ImpID: "imp_id1", Price: 10.0000, Cat: []string{}, W: 1, H: 1},
},
duration: 30,
expectedRejections: []string{
"bid rejected [bid ID: bid_id1] reason: Bid did not contain a category",
},
},
{
description: "Bid should be rejected due to missing category mapping file",
reqExt: invalidReqExt,
bids: []*openrtb.Bid{
{ID: "bid_id1", ImpID: "imp_id1", Price: 10.0000, Cat: []string{"IAB1-1"}, W: 1, H: 1},
},
duration: 30,
expectedRejections: []string{
"bid rejected [bid ID: bid_id1] reason: Category mapping file for primary ad server: 'dfp', publisher: 'some_publisher' not found",
},
},
{
description: "Bid should be rejected due to duration exceeding maximum",
reqExt: requestExt,
bids: []*openrtb.Bid{
{ID: "bid_id1", ImpID: "imp_id1", Price: 10.0000, Cat: []string{"IAB1-1"}, W: 1, H: 1},
},
duration: 70,
expectedRejections: []string{
"bid rejected [bid ID: bid_id1] reason: Bid duration exceeds maximum allowed",
},
},
{
description: "Bid should be rejected due to duplicate bid",
reqExt: requestExt,
bids: []*openrtb.Bid{
{ID: "bid_id1", ImpID: "imp_id1", Price: 10.0000, Cat: []string{"IAB1-1"}, W: 1, H: 1},
{ID: "bid_id1", ImpID: "imp_id1", Price: 10.0000, Cat: []string{"IAB1-1"}, W: 1, H: 1},
},
duration: 30,
expectedRejections: []string{
"bid rejected [bid ID: bid_id1] reason: Bid was deduplicated",
},
expectedCatDur: "10.00_VideoGames_30s",
},
}

for _, test := range testCases {
innerBids := []*pbsOrtbBid{}
for _, bid := range test.bids {
currentBid := pbsOrtbBid{
bid, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: test.duration},
}
innerBids = append(innerBids, &currentBid)
}

seatBid := pbsOrtbSeatBid{innerBids, "USD", nil, nil}

adapterBids[bidderName] = &seatBid

bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, test.reqExt, adapterBids, categoriesFetcher, targData)

if len(test.expectedCatDur) > 0 {
// Bid deduplication case
assert.Equal(t, 1, len(adapterBids[bidderName].bids), "Bidders number doesn't match")
assert.Equal(t, 1, len(bidCategory), "Bidders category mapping doesn't match")
assert.Equal(t, test.expectedCatDur, bidCategory["bid_id1"], "Bid category did not contain expected hb_pb_cat_dur")
} else {
assert.Empty(t, adapterBids[bidderName].bids, "Bidders number doesn't match")
assert.Empty(t, bidCategory, "Bidders category mapping doesn't match")
}

assert.Empty(t, err, "Category mapping error should be empty")
assert.Equal(t, test.expectedRejections, rejections, test.description)
}
}

func TestUpdateRejections(t *testing.T) {
rejections := []string{}

rejections = updateRejections(rejections, "bid_id1", "some reason 1")
rejections = updateRejections(rejections, "bid_id2", "some reason 2")

assert.Equal(t, 2, len(rejections), "Rejections should contain 2 rejection messages")
camrice marked this conversation as resolved.
Show resolved Hide resolved
assert.Containsf(t, rejections, "bid rejected [bid ID: bid_id1] reason: some reason 1", "Rejection message did not match expected")
assert.Containsf(t, rejections, "bid rejected [bid ID: bid_id2] reason: some reason 2", "Rejection message did not match expected")
}

type exchangeSpec struct {
IncomingRequest exchangeRequest `json:"incomingRequest"`
OutgoingRequests map[string]*bidderSpec `json:"outgoingRequests"`
Expand Down