Repository: prebid/prebid-server
Pull request #2181 (2022-03-11 22:06:30)

Account-Level Config -- Truncate Targeting Attribute 
This PR addresses this issue: https://github.com/prebid/prebid-server/issues/1266

Within the file `exchange/targeting.go`, there is a const int value `MaxKeyLength` that is utilized in this file's `addKeys` function to limit define the max length a bidder name can be. 

What this PR is doing, is allowing this value to be configurable at the account level, with the new variable being named `truncateTargetAttr` to match the PBS Java name. So if the `addKeys` function is called, it'll check if at the account level, `truncateTargetAttr` has been defined. If it has, then we use that value instead of the const `MaxKeyLength`, if it hasn't, then we just use `MaxKeyLength`.

The test `TestSetTargeting` has been updated to accommodate these changes. 


Patch:
diff --git a/amp/parse.go b/amp/parse.go
index 05e43aca..1ce19a76 100644
--- a/amp/parse.go
+++ b/amp/parse.go
@@ -9,7 +9,7 @@ import (
 	"github.com/mxmCherry/openrtb/v15/openrtb2"
 )
 
-// Params defines the paramters of an AMP request.
+// Params defines the parameters of an AMP request.
 type Params struct {
 	Account         string
 	CanonicalURL    string
@@ -31,7 +31,7 @@ type Size struct {
 	Width          int64
 }
 
-// ParseParams parses the AMP paramters from a HTTP request.
+// ParseParams parses the AMP parameters from a HTTP request.
 func ParseParams(httpRequest *http.Request) (Params, error) {
 	query := httpRequest.URL.Query()
 
@@ -104,7 +104,7 @@ func chooseConsent(consent, gdprConsent string) string {
 		return consent
 	}
 
-	// Fallback to 'gdpr_consent' for compatability until it's no longer used. This was our original
+	// Fallback to 'gdpr_consent' for compatibility until it's no longer used. This was our original
 	// implementation before the same AMP macro was reused for CCPA.
 	return gdprConsent
 }
diff --git a/config/accounts.go b/config/accounts.go
index 2c67a752..ae9d656e 100644
--- a/config/accounts.go
+++ b/config/accounts.go
@@ -13,16 +13,17 @@ const (
 
 // Account represents a publisher account configuration
 type Account struct {
-	ID                 string      `mapstructure:"id" json:"id"`
-	Disabled           bool        `mapstructure:"disabled" json:"disabled"`
-	CacheTTL           DefaultTTLs `mapstructure:"cache_ttl" json:"cache_ttl"`
-	EventsEnabled      bool        `mapstructure:"events_enabled" json:"events_enabled"`
-	CCPA               AccountCCPA `mapstructure:"ccpa" json:"ccpa"`
-	GDPR               AccountGDPR `mapstructure:"gdpr" json:"gdpr"`
-	DebugAllow         bool        `mapstructure:"debug_allow" json:"debug_allow"`
-	DefaultIntegration string      `mapstructure:"default_integration" json:"default_integration"`
-	CookieSync         CookieSync  `mapstructure:"cookie_sync" json:"cookie_sync"`
-	Events             Events      `mapstructure:"events" json:"events"` // Don't enable this feature. It is still under developmment - https://github.com/prebid/prebid-server/issues/1725
+	ID                      string      `mapstructure:"id" json:"id"`
+	Disabled                bool        `mapstructure:"disabled" json:"disabled"`
+	CacheTTL                DefaultTTLs `mapstructure:"cache_ttl" json:"cache_ttl"`
+	EventsEnabled           bool        `mapstructure:"events_enabled" json:"events_enabled"`
+	CCPA                    AccountCCPA `mapstructure:"ccpa" json:"ccpa"`
+	GDPR                    AccountGDPR `mapstructure:"gdpr" json:"gdpr"`
+	DebugAllow              bool        `mapstructure:"debug_allow" json:"debug_allow"`
+	DefaultIntegration      string      `mapstructure:"default_integration" json:"default_integration"`
+	CookieSync              CookieSync  `mapstructure:"cookie_sync" json:"cookie_sync"`
+	Events                  Events      `mapstructure:"events" json:"events"` // Don't enable this feature. It is still under developmment - https://github.com/prebid/prebid-server/issues/1725
+	TruncateTargetAttribute *int        `mapstructure:"truncate_target_attr" json:"truncate_target_attr"`
 }
 
 // CookieSync represents the account-level defaults for the cookie sync endpoint.
diff --git a/endpoints/openrtb2/auction.go b/endpoints/openrtb2/auction.go
index 074f6739..42caa3a7 100644
--- a/endpoints/openrtb2/auction.go
+++ b/endpoints/openrtb2/auction.go
@@ -940,7 +940,7 @@ func validateNativeContextTypes(cType native1.ContextType, cSubtype native1.Cont
 
 func validateNativePlacementType(pt native1.PlacementType, impIndex int) error {
 	if pt == 0 {
-		// Placement Type is only reccomended, not required.
+		// Placement Type is only recommended, not required.
 		return nil
 	}
 	if pt < native1.PlacementTypeFeed || (pt > native1.PlacementTypeRecommendationWidget && pt < openrtb_ext.NativeExchangeSpecificLowerBound) {
diff --git a/exchange/exchange.go b/exchange/exchange.go
index 08fd3117..80820ac1 100644
--- a/exchange/exchange.go
+++ b/exchange/exchange.go
@@ -158,7 +158,7 @@ type AuctionRequest struct {
 	GlobalPrivacyControlHeader string
 	ImpExtInfoMap              map[string]ImpExtInfo
 
-	// LegacyLabels is included here for temporary compatability with cleanOpenRTBRequests
+	// LegacyLabels is included here for temporary compatibility with cleanOpenRTBRequests
 	// in HoldAuction until we get to factoring it away. Do not use for anything new.
 	LegacyLabels   metrics.Labels
 	FirstPartyData map[openrtb_ext.BidderName]*firstpartydata.ResolvedFirstPartyData
@@ -287,7 +287,7 @@ func (e *exchange) HoldAuction(ctx context.Context, r AuctionRequest, debugLog *
 				errs = append(errs, cacheErrs...)
 			}
 
-			targData.setTargeting(auc, r.BidRequest.App != nil, bidCategory)
+			targData.setTargeting(auc, r.BidRequest.App != nil, bidCategory, r.Account.TruncateTargetAttribute)
 
 		}
 		bidResponseExt = e.makeExtBidResponse(adapterBids, adapterExtra, r, responseDebugAllow, errs)
diff --git a/exchange/targeting.go b/exchange/targeting.go
index f00e43d2..00008d29 100644
--- a/exchange/targeting.go
+++ b/exchange/targeting.go
@@ -35,7 +35,7 @@ type targetData struct {
 // The one exception is the `hb_cache_id` key. Since our APIs explicitly document cache keys to be on a "best effort" basis,
 // it's ok if those stay in the auction. For now, this method implements a very naive cache strategy.
 // In the future, we should implement a more clever retry & backoff strategy to balance the success rate & performance.
-func (targData *targetData) setTargeting(auc *auction, isApp bool, categoryMapping map[string]string) {
+func (targData *targetData) setTargeting(auc *auction, isApp bool, categoryMapping map[string]string, truncateTargetAttr *int) {
 	for impId, topBidsPerImp := range auc.winningBidsByBidder {
 		overallWinner := auc.winningBids[impId]
 		for bidderName, topBidPerBidder := range topBidsPerImp {
@@ -43,38 +43,38 @@ func (targData *targetData) setTargeting(auc *auction, isApp bool, categoryMappi
 
 			targets := make(map[string]string, 10)
 			if cpm, ok := auc.roundedPrices[topBidPerBidder]; ok {
-				targData.addKeys(targets, openrtb_ext.HbpbConstantKey, cpm, bidderName, isOverallWinner)
+				targData.addKeys(targets, openrtb_ext.HbpbConstantKey, cpm, bidderName, isOverallWinner, truncateTargetAttr)
 			}
-			targData.addKeys(targets, openrtb_ext.HbBidderConstantKey, string(bidderName), bidderName, isOverallWinner)
+			targData.addKeys(targets, openrtb_ext.HbBidderConstantKey, string(bidderName), bidderName, isOverallWinner, truncateTargetAttr)
 			if hbSize := makeHbSize(topBidPerBidder.bid); hbSize != "" {
-				targData.addKeys(targets, openrtb_ext.HbSizeConstantKey, hbSize, bidderName, isOverallWinner)
+				targData.addKeys(targets, openrtb_ext.HbSizeConstantKey, hbSize, bidderName, isOverallWinner, truncateTargetAttr)
 			}
 			if cacheID, ok := auc.cacheIds[topBidPerBidder.bid]; ok {
-				targData.addKeys(targets, openrtb_ext.HbCacheKey, cacheID, bidderName, isOverallWinner)
+				targData.addKeys(targets, openrtb_ext.HbCacheKey, cacheID, bidderName, isOverallWinner, truncateTargetAttr)
 			}
 			if vastID, ok := auc.vastCacheIds[topBidPerBidder.bid]; ok {
-				targData.addKeys(targets, openrtb_ext.HbVastCacheKey, vastID, bidderName, isOverallWinner)
+				targData.addKeys(targets, openrtb_ext.HbVastCacheKey, vastID, bidderName, isOverallWinner, truncateTargetAttr)
 			}
 			if targData.includeFormat {
-				targData.addKeys(targets, openrtb_ext.HbFormatKey, string(topBidPerBidder.bidType), bidderName, isOverallWinner)
+				targData.addKeys(targets, openrtb_ext.HbFormatKey, string(topBidPerBidder.bidType), bidderName, isOverallWinner, truncateTargetAttr)
 			}
 
 			if targData.cacheHost != "" {
-				targData.addKeys(targets, openrtb_ext.HbConstantCacheHostKey, targData.cacheHost, bidderName, isOverallWinner)
+				targData.addKeys(targets, openrtb_ext.HbConstantCacheHostKey, targData.cacheHost, bidderName, isOverallWinner, truncateTargetAttr)
 			}
 			if targData.cachePath != "" {
-				targData.addKeys(targets, openrtb_ext.HbConstantCachePathKey, targData.cachePath, bidderName, isOverallWinner)
+				targData.addKeys(targets, openrtb_ext.HbConstantCachePathKey, targData.cachePath, bidderName, isOverallWinner, truncateTargetAttr)
 			}
 
 			if deal := topBidPerBidder.bid.DealID; len(deal) > 0 {
-				targData.addKeys(targets, openrtb_ext.HbDealIDConstantKey, deal, bidderName, isOverallWinner)
+				targData.addKeys(targets, openrtb_ext.HbDealIDConstantKey, deal, bidderName, isOverallWinner, truncateTargetAttr)
 			}
 
 			if isApp {
-				targData.addKeys(targets, openrtb_ext.HbEnvKey, openrtb_ext.HbEnvKeyApp, bidderName, isOverallWinner)
+				targData.addKeys(targets, openrtb_ext.HbEnvKey, openrtb_ext.HbEnvKeyApp, bidderName, isOverallWinner, truncateTargetAttr)
 			}
 			if len(categoryMapping) > 0 {
-				targData.addKeys(targets, openrtb_ext.HbCategoryDurationKey, categoryMapping[topBidPerBidder.bid.ID], bidderName, isOverallWinner)
+				targData.addKeys(targets, openrtb_ext.HbCategoryDurationKey, categoryMapping[topBidPerBidder.bid.ID], bidderName, isOverallWinner, truncateTargetAttr)
 			}
 
 			topBidPerBidder.bidTargets = targets
@@ -82,9 +82,18 @@ func (targData *targetData) setTargeting(auc *auction, isApp bool, categoryMappi
 	}
 }
 
-func (targData *targetData) addKeys(keys map[string]string, key openrtb_ext.TargetingKey, value string, bidderName openrtb_ext.BidderName, overallWinner bool) {
+func (targData *targetData) addKeys(keys map[string]string, key openrtb_ext.TargetingKey, value string, bidderName openrtb_ext.BidderName, overallWinner bool, truncateTargetAttr *int) {
+	var maxLength int
+	if truncateTargetAttr != nil {
+		maxLength = *truncateTargetAttr
+		if maxLength < 0 {
+			maxLength = MaxKeyLength
+		}
+	} else {
+		maxLength = MaxKeyLength
+	}
 	if targData.includeBidderKeys {
-		keys[key.BidderKey(bidderName, MaxKeyLength)] = value
+		keys[key.BidderKey(bidderName, maxLength)] = value
 	}
 	if targData.includeWinners && overallWinner {
 		keys[string(key)] = value
diff --git a/metrics/metrics.go b/metrics/metrics.go
index 7ba54675..b70907ea 100644
--- a/metrics/metrics.go
+++ b/metrics/metrics.go
@@ -406,7 +406,7 @@ type MetricsEngine interface {
 	RecordStoredDataError(labels StoredDataLabels)
 	RecordPrebidCacheRequestTime(success bool, length time.Duration)
 	RecordRequestQueueTime(success bool, requestType RequestType, length time.Duration)
-	RecordTimeoutNotice(sucess bool)
+	RecordTimeoutNotice(success bool)
 	RecordRequestPrivacy(privacy PrivacyLabels)
 	RecordAdapterGDPRRequestBlocked(adapterName openrtb_ext.BidderName)
 }


Test patch:
diff --git a/adapters/infoawarebidder_test.go b/adapters/infoawarebidder_test.go
index 62bcc08d..7bd476b3 100644
--- a/adapters/infoawarebidder_test.go
+++ b/adapters/infoawarebidder_test.go
@@ -173,7 +173,7 @@ func TestImpFiltering(t *testing.T) {
 		}
 
 		// Extra MakeRequests() call check: our mockBidder returns an adapter request for every imp
-		assert.Len(t, actualAdapterRequests, test.expectedImpLen, "Test failed. Incorrect lenght of filtered imps: %s", test.description)
+		assert.Len(t, actualAdapterRequests, test.expectedImpLen, "Test failed. Incorrect length of filtered imps: %s", test.description)
 	}
 }
 
diff --git a/endpoints/info/bidders_test.go b/endpoints/info/bidders_test.go
index 7110e188..9be25f44 100644
--- a/endpoints/info/bidders_test.go
+++ b/endpoints/info/bidders_test.go
@@ -179,7 +179,7 @@ func TestBiddersHandler(t *testing.T) {
 		expectedHeaders http.Header
 	}{
 		{
-			description:     "No Query Paramters - Backwards Compatability",
+			description:     "No Query Parameters - Backwards Compatibility",
 			givenURL:        "/info/bidders",
 			expectedStatus:  http.StatusOK,
 			expectedBody:    `["a","b","x","y"]`,
diff --git a/exchange/targeting_test.go b/exchange/targeting_test.go
index 48e054c7..f2726de0 100644
--- a/exchange/targeting_test.go
+++ b/exchange/targeting_test.go
@@ -247,6 +247,7 @@ type TargetingTestData struct {
 	IsApp                      bool
 	CategoryMapping            map[string]string
 	ExpectedBidTargetsByBidder map[string]map[openrtb_ext.BidderName]map[string]string
+	TruncateTargetAttr         *int
 }
 
 var bid123 *openrtb2.Bid = &openrtb2.Bid{
@@ -261,6 +262,9 @@ var bid084 *openrtb2.Bid = &openrtb2.Bid{
 	Price: 0.84,
 }
 
+var truncateTargetAttrValueLess int = 10
+var truncateTargetAttrValueGreater int = 25
+var truncateTargetAttrValueNegative int = -1
 var TargetingTests []TargetingTestData = []TargetingTestData{
 	{
 		Description: "Targeting winners only (most basic targeting example)",
@@ -291,6 +295,7 @@ var TargetingTests []TargetingTestData = []TargetingTestData{
 				openrtb_ext.BidderRubicon: {},
 			},
 		},
+		TruncateTargetAttr: nil,
 	},
 	{
 		Description: "Targeting on bidders only",
@@ -324,6 +329,7 @@ var TargetingTests []TargetingTestData = []TargetingTestData{
 				},
 			},
 		},
+		TruncateTargetAttr: nil,
 	},
 	{
 		Description: "Full basic targeting with hd_format",
@@ -364,6 +370,7 @@ var TargetingTests []TargetingTestData = []TargetingTestData{
 				},
 			},
 		},
+		TruncateTargetAttr: nil,
 	},
 	{
 		Description: "Cache and deal targeting test",
@@ -410,6 +417,109 @@ var TargetingTests []TargetingTestData = []TargetingTestData{
 				},
 			},
 		},
+		TruncateTargetAttr: nil,
+	},
+	{
+		Description: "Truncate Targeting Attribute value is given and is less than const MaxKeyLength",
+		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:     bid123,
+						bidType: openrtb_ext.BidTypeBanner,
+					},
+					openrtb_ext.BidderRubicon: {
+						bid:     bid084,
+						bidType: openrtb_ext.BidTypeBanner,
+					},
+				},
+			},
+		},
+		ExpectedBidTargetsByBidder: map[string]map[openrtb_ext.BidderName]map[string]string{
+			"ImpId-1": {
+				openrtb_ext.BidderAppnexus: {
+					"hb_bidder_": "appnexus",
+					"hb_pb_appn": "1.20",
+				},
+				openrtb_ext.BidderRubicon: {
+					"hb_bidder_": "rubicon",
+					"hb_pb_rubi": "0.80",
+				},
+			},
+		},
+		TruncateTargetAttr: &truncateTargetAttrValueLess,
+	},
+	{
+		Description: "Truncate Targeting Attribute value is given and is greater than const MaxKeyLength",
+		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:     bid123,
+						bidType: openrtb_ext.BidTypeBanner,
+					},
+					openrtb_ext.BidderRubicon: {
+						bid:     bid084,
+						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",
+				},
+			},
+		},
+		TruncateTargetAttr: &truncateTargetAttrValueGreater,
+	},
+	{
+		Description: "Truncate Targeting Attribute value is given and is negative",
+		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:     bid123,
+						bidType: openrtb_ext.BidTypeBanner,
+					},
+					openrtb_ext.BidderRubicon: {
+						bid:     bid084,
+						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",
+				},
+			},
+		},
+		TruncateTargetAttr: &truncateTargetAttrValueNegative,
 	},
 }
 
@@ -433,7 +543,7 @@ func TestSetTargeting(t *testing.T) {
 		}
 		auc.winningBids = winningBids
 		targData := test.TargetData
-		targData.setTargeting(auc, test.IsApp, test.CategoryMapping)
+		targData.setTargeting(auc, test.IsApp, test.CategoryMapping, test.TruncateTargetAttr)
 		for imp, targetsByBidder := range test.ExpectedBidTargetsByBidder {
 			for bidder, expected := range targetsByBidder {
 				assert.Equal(t,
diff --git a/privacy/scrubber_test.go b/privacy/scrubber_test.go
index 1198d0bb..13ac1f5c 100644
--- a/privacy/scrubber_test.go
+++ b/privacy/scrubber_test.go
@@ -37,7 +37,7 @@ func TestScrubDevice(t *testing.T) {
 		geo         ScrubStrategyGeo
 	}{
 		{
-			description: "All Strageties - None",
+			description: "All Strategies - None",
 			expected:    device,
 			id:          ScrubStrategyDeviceIDNone,
 			ipv4:        ScrubStrategyIPV4None,
@@ -45,7 +45,7 @@ func TestScrubDevice(t *testing.T) {
 			geo:         ScrubStrategyGeoNone,
 		},
 		{
-			description: "All Strageties - Strictest",
+			description: "All Strategies - Strictest",
 			expected: &openrtb2.Device{
 				DIDMD5:   "",
 				DIDSHA1:  "",