From a65492818f887e5f9585c5c29fe6bf82b836dc74 Mon Sep 17 00:00:00 2001 From: Hans Hjort Date: Thu, 23 Jul 2020 13:24:11 -0400 Subject: [PATCH 1/4] Adds keyvalue hd_format support --- docs/endpoints/openrtb2/auction.md | 3 + exchange/exchange.go | 1 + exchange/targeting.go | 4 + exchange/targeting_test.go | 163 +++++++++++++++++++++++++++++ openrtb_ext/bid.go | 3 + openrtb_ext/request.go | 1 + 6 files changed, 175 insertions(+) diff --git a/docs/endpoints/openrtb2/auction.md b/docs/endpoints/openrtb2/auction.md index d09216188b8..0c02d078b47 100644 --- a/docs/endpoints/openrtb2/auction.md +++ b/docs/endpoints/openrtb2/auction.md @@ -173,6 +173,7 @@ to set these params on the response at `response.seatbid[i].bid[j].ext.prebid.ta }, "includewinners": false, // Optional param defaulting to true "includebidderkeys": false // Optional param defaulting to true + "includeformat": false //Optional param defaulting to false } } } @@ -184,6 +185,8 @@ For backwards compatibility the following strings will also be allowed as price One of "includewinners" or "includebidderkeys" must be true (both default to true if unset). If both were false, then no targeting keys would be set, which is better configured by omitting targeting altogether. +The parameter "includeformat" is for multiformat support. It will add the key `hb_format` and/or `hb_format_{bidderName}` as per "includewinners" and "includebidderkeys" above. + MediaType PriceGranularity (PBS-Java only) - when a single OpenRTB request contains multiple impressions with different mediatypes, or a single impression supports multiple formats, the different mediatypes may need different price granularities. If `mediatypepricegranularity` is present, `pricegranularity` would only be used for any mediatypes not specified. ``` diff --git a/exchange/exchange.go b/exchange/exchange.go index 3f0258dd3c1..5001e495440 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -135,6 +135,7 @@ func (e *exchange) HoldAuction(ctx context.Context, bidRequest *openrtb.BidReque includeBidderKeys: requestExt.Prebid.Targeting.IncludeBidderKeys, includeCacheBids: shouldCacheBids, includeCacheVast: shouldCacheVAST, + includeFormat: requestExt.Prebid.Targeting.IncludeFormat, } targData.cacheHost, targData.cachePath = e.cache.GetExtCacheData() } diff --git a/exchange/targeting.go b/exchange/targeting.go index 994ad9e7c81..dca57653b44 100644 --- a/exchange/targeting.go +++ b/exchange/targeting.go @@ -22,6 +22,7 @@ type targetData struct { includeBidderKeys bool includeCacheBids bool includeCacheVast bool + includeFormat bool // cacheHost and cachePath exist to supply cache host and path as targeting parameters cacheHost string cachePath string @@ -53,6 +54,9 @@ func (targData *targetData) setTargeting(auc *auction, isApp bool, categoryMappi if vastID, ok := auc.vastCacheIds[topBidPerBidder.bid]; ok { targData.addKeys(targets, openrtb_ext.HbVastCacheKey, vastID, bidderName, isOverallWinner) } + if targData.includeFormat { + targData.addKeys(targets, openrtb_ext.HbFormatKey, string(topBidPerBidder.bidType), bidderName, isOverallWinner) + } if targData.cacheHost != "" { targData.addKeys(targets, openrtb_ext.HbConstantCacheHostKey, targData.cacheHost, bidderName, isOverallWinner) diff --git a/exchange/targeting_test.go b/exchange/targeting_test.go index 72de1d4261f..cbc69963ff5 100644 --- a/exchange/targeting_test.go +++ b/exchange/targeting_test.go @@ -247,3 +247,166 @@ func (f *mockFetcher) GetId(bidder openrtb_ext.BidderName) (string, bool) { func mockServer(w http.ResponseWriter, req *http.Request) { w.Write([]byte("{}")) } + +type TargetingTestData struct { + Description string + TargetData targetData + Auction auction + IsApp bool + CategoryMapping map[string]string + ExpectedBidTargetsByBidder map[string]map[openrtb_ext.BidderName]map[string]string +} + +func TestSetTargeting(t *testing.T) { + for _, test := range TargetingTests { + auc := &test.Auction + // Set rounded prices from the auction data + auc.setRoundedPrices(test.TargetData.priceGranularity) + winningBids := make(map[string]*pbsOrtbBid) + // Set winning bids from the auction data + for imp, bidsByBidder := range auc.winningBidsByBidder { + for _, bid := range bidsByBidder { + if winningBid, ok := winningBids[imp]; ok { + if winningBid.bid.Price < bid.bid.Price { + winningBids[imp] = bid + } + } else { + winningBids[imp] = bid + } + } + } + auc.winningBids = winningBids + targData := test.TargetData + targData.setTargeting(auc, test.IsApp, test.CategoryMapping) + for imp, targetsByBidder := range test.ExpectedBidTargetsByBidder { + for bidder, expected := range targetsByBidder { + assert.Equal(t, + expected, + auc.winningBidsByBidder[imp][bidder].bidTargets, + "Test: %s\nTargeting failed for bidder %s on imp %s.", + test.Description, + string(bidder), + imp) + } + } + } + +} + +var TargetingTests []TargetingTestData = []TargetingTestData{ + { + Description: "Targeting winners only (most basic targeting example)", + TargetData: targetData{ + priceGranularity: openrtb_ext.PriceGranularityFromString("med"), + includeWinners: true, + }, + Auction: auction{ + winningBidsByBidder: map[string]map[openrtb_ext.BidderName]*pbsOrtbBid{ + "ImpId-1": { + openrtb_ext.BidderAppnexus: { + bid: &openrtb.Bid{ + Price: 1.23, + }, + bidType: openrtb_ext.BidTypeBanner, + }, + openrtb_ext.BidderRubicon: { + bid: &openrtb.Bid{ + Price: 0.84, + }, + bidType: openrtb_ext.BidTypeBanner, + }, + }, + }, + }, + ExpectedBidTargetsByBidder: map[string]map[openrtb_ext.BidderName]map[string]string{ + "ImpId-1": { + openrtb_ext.BidderAppnexus: { + "hb_bidder": "appnexus", + "hb_pb": "1.20", + }, + openrtb_ext.BidderRubicon: {}, + }, + }, + }, + { + Description: "Targeting on bidders only", + TargetData: targetData{ + priceGranularity: openrtb_ext.PriceGranularityFromString("med"), + includeBidderKeys: true, + }, + Auction: auction{ + winningBidsByBidder: map[string]map[openrtb_ext.BidderName]*pbsOrtbBid{ + "ImpId-1": { + openrtb_ext.BidderAppnexus: { + bid: &openrtb.Bid{ + Price: 1.23, + }, + bidType: openrtb_ext.BidTypeBanner, + }, + openrtb_ext.BidderRubicon: { + bid: &openrtb.Bid{ + Price: 0.84, + }, + bidType: openrtb_ext.BidTypeBanner, + }, + }, + }, + }, + ExpectedBidTargetsByBidder: map[string]map[openrtb_ext.BidderName]map[string]string{ + "ImpId-1": { + openrtb_ext.BidderAppnexus: { + "hb_bidder_appnexus": "appnexus", + "hb_pb_appnexus": "1.20", + }, + openrtb_ext.BidderRubicon: { + "hb_bidder_rubicon": "rubicon", + "hb_pb_rubicon": "0.80", + }, + }, + }, + }, + { + Description: "Full basic targeting with hd_format", + TargetData: targetData{ + priceGranularity: openrtb_ext.PriceGranularityFromString("med"), + includeWinners: true, + includeBidderKeys: true, + includeFormat: true, + }, + Auction: auction{ + winningBidsByBidder: map[string]map[openrtb_ext.BidderName]*pbsOrtbBid{ + "ImpId-1": { + openrtb_ext.BidderAppnexus: { + bid: &openrtb.Bid{ + Price: 1.23, + }, + bidType: openrtb_ext.BidTypeBanner, + }, + openrtb_ext.BidderRubicon: { + bid: &openrtb.Bid{ + Price: 0.84, + }, + bidType: openrtb_ext.BidTypeBanner, + }, + }, + }, + }, + ExpectedBidTargetsByBidder: map[string]map[openrtb_ext.BidderName]map[string]string{ + "ImpId-1": { + openrtb_ext.BidderAppnexus: { + "hb_bidder": "appnexus", + "hb_bidder_appnexus": "appnexus", + "hb_pb": "1.20", + "hb_pb_appnexus": "1.20", + "hb_format": "banner", + "hb_format_appnexus": "banner", + }, + openrtb_ext.BidderRubicon: { + "hb_bidder_rubicon": "rubicon", + "hb_pb_rubicon": "0.80", + "hb_format_rubicon": "banner", + }, + }, + }, + }, +} diff --git a/openrtb_ext/bid.go b/openrtb_ext/bid.go index 3b297c7ab5d..09ee375be82 100644 --- a/openrtb_ext/bid.go +++ b/openrtb_ext/bid.go @@ -99,6 +99,9 @@ const ( HbSizeConstantKey TargetingKey = "hb_size" HbDealIDConstantKey TargetingKey = "hb_deal" + // HbFormatKey is the format of the bid. For example, "video", "banner" + HbFormatKey TargetingKey = "hb_format" + // HbCacheKey and HbVastCacheKey store UUIDs which can be used to fetch things from prebid cache. // Callers should *never* assume that either of these exist, since the call to the cache may always fail. // diff --git a/openrtb_ext/request.go b/openrtb_ext/request.go index 86388f60cf4..acfd4a1e71f 100644 --- a/openrtb_ext/request.go +++ b/openrtb_ext/request.go @@ -85,6 +85,7 @@ type ExtRequestTargeting struct { IncludeWinners bool `json:"includewinners"` IncludeBidderKeys bool `json:"includebidderkeys"` IncludeBrandCategory *ExtIncludeBrandCategory `json:"includebrandcategory"` + IncludeFormat bool `json:"includeformat"` DurationRangeSec []int `json:"durationrangesec"` } From 7c7b63051ec1f3f9d11ef4764d864fc649de272a Mon Sep 17 00:00:00 2001 From: Hans Hjort Date: Mon, 27 Jul 2020 13:18:58 -0400 Subject: [PATCH 2/4] Tweak documentation formatting --- docs/endpoints/openrtb2/auction.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/endpoints/openrtb2/auction.md b/docs/endpoints/openrtb2/auction.md index 0c02d078b47..a8401757cf6 100644 --- a/docs/endpoints/openrtb2/auction.md +++ b/docs/endpoints/openrtb2/auction.md @@ -173,7 +173,7 @@ to set these params on the response at `response.seatbid[i].bid[j].ext.prebid.ta }, "includewinners": false, // Optional param defaulting to true "includebidderkeys": false // Optional param defaulting to true - "includeformat": false //Optional param defaulting to false + "includeformat": false // Optional param defaulting to false } } } From 9c74ff98d0303d32fb98799d48c0e11d2053c6e9 Mon Sep 17 00:00:00 2001 From: Hans Hjort Date: Tue, 28 Jul 2020 09:37:49 -0400 Subject: [PATCH 3/4] Rearranging test pieces --- exchange/targeting_test.go | 72 +++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/exchange/targeting_test.go b/exchange/targeting_test.go index cbc69963ff5..b032ea6b66f 100644 --- a/exchange/targeting_test.go +++ b/exchange/targeting_test.go @@ -257,42 +257,6 @@ type TargetingTestData struct { ExpectedBidTargetsByBidder map[string]map[openrtb_ext.BidderName]map[string]string } -func TestSetTargeting(t *testing.T) { - for _, test := range TargetingTests { - auc := &test.Auction - // Set rounded prices from the auction data - auc.setRoundedPrices(test.TargetData.priceGranularity) - winningBids := make(map[string]*pbsOrtbBid) - // Set winning bids from the auction data - for imp, bidsByBidder := range auc.winningBidsByBidder { - for _, bid := range bidsByBidder { - if winningBid, ok := winningBids[imp]; ok { - if winningBid.bid.Price < bid.bid.Price { - winningBids[imp] = bid - } - } else { - winningBids[imp] = bid - } - } - } - auc.winningBids = winningBids - targData := test.TargetData - targData.setTargeting(auc, test.IsApp, test.CategoryMapping) - for imp, targetsByBidder := range test.ExpectedBidTargetsByBidder { - for bidder, expected := range targetsByBidder { - assert.Equal(t, - expected, - auc.winningBidsByBidder[imp][bidder].bidTargets, - "Test: %s\nTargeting failed for bidder %s on imp %s.", - test.Description, - string(bidder), - imp) - } - } - } - -} - var TargetingTests []TargetingTestData = []TargetingTestData{ { Description: "Targeting winners only (most basic targeting example)", @@ -410,3 +374,39 @@ var TargetingTests []TargetingTestData = []TargetingTestData{ }, }, } + +func TestSetTargeting(t *testing.T) { + for _, test := range TargetingTests { + auc := &test.Auction + // Set rounded prices from the auction data + auc.setRoundedPrices(test.TargetData.priceGranularity) + winningBids := make(map[string]*pbsOrtbBid) + // Set winning bids from the auction data + for imp, bidsByBidder := range auc.winningBidsByBidder { + for _, bid := range bidsByBidder { + if winningBid, ok := winningBids[imp]; ok { + if winningBid.bid.Price < bid.bid.Price { + winningBids[imp] = bid + } + } else { + winningBids[imp] = bid + } + } + } + auc.winningBids = winningBids + targData := test.TargetData + targData.setTargeting(auc, test.IsApp, test.CategoryMapping) + for imp, targetsByBidder := range test.ExpectedBidTargetsByBidder { + for bidder, expected := range targetsByBidder { + assert.Equal(t, + expected, + auc.winningBidsByBidder[imp][bidder].bidTargets, + "Test: %s\nTargeting failed for bidder %s on imp %s.", + test.Description, + string(bidder), + imp) + } + } + } + +} From 250b3c99492ad5cbaed3d2bd553ee7a913960072 Mon Sep 17 00:00:00 2001 From: Hans Hjort Date: Wed, 29 Jul 2020 14:16:51 -0400 Subject: [PATCH 4/4] PR review tweaks --- docs/endpoints/openrtb2/auction.md | 2 +- exchange/targeting_test.go | 82 +++++++++++++++++++++++------- 2 files changed, 65 insertions(+), 19 deletions(-) diff --git a/docs/endpoints/openrtb2/auction.md b/docs/endpoints/openrtb2/auction.md index a8401757cf6..b532923e793 100644 --- a/docs/endpoints/openrtb2/auction.md +++ b/docs/endpoints/openrtb2/auction.md @@ -185,7 +185,7 @@ For backwards compatibility the following strings will also be allowed as price One of "includewinners" or "includebidderkeys" must be true (both default to true if unset). If both were false, then no targeting keys would be set, which is better configured by omitting targeting altogether. -The parameter "includeformat" is for multiformat support. It will add the key `hb_format` and/or `hb_format_{bidderName}` as per "includewinners" and "includebidderkeys" above. +The parameter "includeformat" indicates the type of the bid (banner, video, etc) for multiformat requests. It will add the key `hb_format` and/or `hb_format_{bidderName}` as per "includewinners" and "includebidderkeys" above. MediaType PriceGranularity (PBS-Java only) - when a single OpenRTB request contains multiple impressions with different mediatypes, or a single impression supports multiple formats, the different mediatypes may need different price granularities. If `mediatypepricegranularity` is present, `pricegranularity` would only be used for any mediatypes not specified. diff --git a/exchange/targeting_test.go b/exchange/targeting_test.go index b032ea6b66f..11ce568d2bf 100644 --- a/exchange/targeting_test.go +++ b/exchange/targeting_test.go @@ -257,6 +257,18 @@ type TargetingTestData struct { ExpectedBidTargetsByBidder map[string]map[openrtb_ext.BidderName]map[string]string } +var bid123 *openrtb.Bid = &openrtb.Bid{ + Price: 1.23, +} + +var bid111 *openrtb.Bid = &openrtb.Bid{ + Price: 1.11, + DealID: "mydeal", +} +var bid084 *openrtb.Bid = &openrtb.Bid{ + Price: 0.84, +} + var TargetingTests []TargetingTestData = []TargetingTestData{ { Description: "Targeting winners only (most basic targeting example)", @@ -268,15 +280,11 @@ var TargetingTests []TargetingTestData = []TargetingTestData{ winningBidsByBidder: map[string]map[openrtb_ext.BidderName]*pbsOrtbBid{ "ImpId-1": { openrtb_ext.BidderAppnexus: { - bid: &openrtb.Bid{ - Price: 1.23, - }, + bid: bid123, bidType: openrtb_ext.BidTypeBanner, }, openrtb_ext.BidderRubicon: { - bid: &openrtb.Bid{ - Price: 0.84, - }, + bid: bid084, bidType: openrtb_ext.BidTypeBanner, }, }, @@ -302,15 +310,11 @@ var TargetingTests []TargetingTestData = []TargetingTestData{ winningBidsByBidder: map[string]map[openrtb_ext.BidderName]*pbsOrtbBid{ "ImpId-1": { openrtb_ext.BidderAppnexus: { - bid: &openrtb.Bid{ - Price: 1.23, - }, + bid: bid123, bidType: openrtb_ext.BidTypeBanner, }, openrtb_ext.BidderRubicon: { - bid: &openrtb.Bid{ - Price: 0.84, - }, + bid: bid084, bidType: openrtb_ext.BidTypeBanner, }, }, @@ -341,15 +345,11 @@ var TargetingTests []TargetingTestData = []TargetingTestData{ winningBidsByBidder: map[string]map[openrtb_ext.BidderName]*pbsOrtbBid{ "ImpId-1": { openrtb_ext.BidderAppnexus: { - bid: &openrtb.Bid{ - Price: 1.23, - }, + bid: bid123, bidType: openrtb_ext.BidTypeBanner, }, openrtb_ext.BidderRubicon: { - bid: &openrtb.Bid{ - Price: 0.84, - }, + bid: bid084, bidType: openrtb_ext.BidTypeBanner, }, }, @@ -373,6 +373,52 @@ var TargetingTests []TargetingTestData = []TargetingTestData{ }, }, }, + { + Description: "Cache and deal targeting test", + TargetData: targetData{ + priceGranularity: openrtb_ext.PriceGranularityFromString("med"), + includeBidderKeys: true, + cacheHost: "cache.prebid.com", + cachePath: "cache", + }, + Auction: auction{ + winningBidsByBidder: map[string]map[openrtb_ext.BidderName]*pbsOrtbBid{ + "ImpId-1": { + openrtb_ext.BidderAppnexus: { + bid: bid123, + bidType: openrtb_ext.BidTypeBanner, + }, + openrtb_ext.BidderRubicon: { + bid: bid111, + bidType: openrtb_ext.BidTypeBanner, + }, + }, + }, + cacheIds: map[*openrtb.Bid]string{ + bid123: "55555", + bid111: "cacheme", + }, + }, + ExpectedBidTargetsByBidder: map[string]map[openrtb_ext.BidderName]map[string]string{ + "ImpId-1": { + openrtb_ext.BidderAppnexus: { + "hb_bidder_appnexus": "appnexus", + "hb_pb_appnexus": "1.20", + "hb_cache_id_appnexus": "55555", + "hb_cache_host_appnex": "cache.prebid.com", + "hb_cache_path_appnex": "cache", + }, + openrtb_ext.BidderRubicon: { + "hb_bidder_rubicon": "rubicon", + "hb_pb_rubicon": "1.10", + "hb_cache_id_rubicon": "cacheme", + "hb_deal_rubicon": "mydeal", + "hb_cache_host_rubico": "cache.prebid.com", + "hb_cache_path_rubico": "cache", + }, + }, + }, + }, } func TestSetTargeting(t *testing.T) {