From dee2ca502fbd8573a46380df461159368d4dba84 Mon Sep 17 00:00:00 2001 From: ShriprasadM Date: Fri, 14 Aug 2020 17:15:19 +0530 Subject: [PATCH 01/22] UOE-5440: Changes for capturing Pod algorithm execution time using pbmetrics (#65) * Added function getPrometheusRegistry() * Exported function GetPrometheusRegistry * UOE-5440: Capturing execution time in nanoseconds for algorithms * UOE-5440: Changes for prometheus algorithem metrics for pod using pbsmetrics * UOE-5440: Test cases for prometheus * UOE-5440: Added test cases * UOE-5440: Changing buckets * UOE-5440: changes in pbsmetrics for newly added metrics Co-authored-by: Sachin Survase Co-authored-by: PubMatic-OpenWrap Co-authored-by: Shriprasad --- endpoints/openrtb2/ctv/adpod_generator.go | 57 ++++++++++--- endpoints/openrtb2/ctv/constant.go | 10 +++ .../openrtb2/ctv/impressions/impressions.go | 6 ++ endpoints/openrtb2/ctv_auction.go | 16 +++- pbsmetrics/config/metrics.go | 33 +++++++ pbsmetrics/go_metrics.go | 12 +++ pbsmetrics/metrics.go | 31 +++++++ pbsmetrics/prometheus/prometheus.go | 85 +++++++++++++++++++ pbsmetrics/prometheus/prometheus_test.go | 41 +++++++++ router/router.go | 10 +++ 10 files changed, 285 insertions(+), 16 deletions(-) diff --git a/endpoints/openrtb2/ctv/adpod_generator.go b/endpoints/openrtb2/ctv/adpod_generator.go index 10f4cc4427d..bec01ee069a 100644 --- a/endpoints/openrtb2/ctv/adpod_generator.go +++ b/endpoints/openrtb2/ctv/adpod_generator.go @@ -7,6 +7,7 @@ import ( "github.com/PubMatic-OpenWrap/openrtb" "github.com/PubMatic-OpenWrap/prebid-server/openrtb_ext" + "github.com/PubMatic-OpenWrap/prebid-server/pbsmetrics" ) /********************* AdPodGenerator Functions *********************/ @@ -20,14 +21,15 @@ type filteredBid struct { reasonCode FilterReasonCode } type highestCombination struct { - bids []*Bid - bidIDs []string - durations []int - price float64 - categoryScore map[string]int - domainScore map[string]int - filteredBids map[string]*filteredBid - timeTaken time.Duration + bids []*Bid + bidIDs []string + durations []int + price float64 + categoryScore map[string]int + domainScore map[string]int + filteredBids map[string]*filteredBid + timeTakenCompExcl time.Duration // time taken by comp excl + timeTakenCombGen time.Duration // time taken by combination generator } //AdPodGenerator AdPodGenerator @@ -38,16 +40,18 @@ type AdPodGenerator struct { buckets BidsBuckets comb ICombination adpod *openrtb_ext.VideoAdPod + met pbsmetrics.MetricsEngine } //NewAdPodGenerator will generate adpod based on configuration -func NewAdPodGenerator(request *openrtb.BidRequest, impIndex int, buckets BidsBuckets, comb ICombination, adpod *openrtb_ext.VideoAdPod) *AdPodGenerator { +func NewAdPodGenerator(request *openrtb.BidRequest, impIndex int, buckets BidsBuckets, comb ICombination, adpod *openrtb_ext.VideoAdPod, met pbsmetrics.MetricsEngine) *AdPodGenerator { return &AdPodGenerator{ request: request, impIndex: impIndex, buckets: buckets, comb: comb, adpod: adpod, + met: met, } } @@ -74,7 +78,8 @@ func (o *AdPodGenerator) cleanup(wg *sync.WaitGroup, responseCh chan *highestCom } func (o *AdPodGenerator) getAdPodBids(timeout time.Duration) []*highestCombination { - defer TimeTrack(time.Now(), fmt.Sprintf("Tid:%v ImpId:%v getAdPodBids", o.request.ID, o.request.Imp[o.impIndex].ID)) + start := time.Now() + defer TimeTrack(start, fmt.Sprintf("Tid:%v ImpId:%v getAdPodBids", o.request.ID, o.request.Imp[o.impIndex].ID)) maxRoutines := 3 isTimedOutORReceivedAllResponses := false @@ -84,20 +89,24 @@ func (o *AdPodGenerator) getAdPodBids(timeout time.Duration) []*highestCombinati lock := sync.Mutex{} ticker := time.NewTicker(timeout) + combinationCount := 0 for i := 0; i < maxRoutines; i++ { wg.Add(1) go func() { for !isTimedOutORReceivedAllResponses { + combGenStartTime := time.Now() lock.Lock() durations := o.comb.Get() lock.Unlock() + combGenElapsedTime := time.Since(combGenStartTime) if len(durations) == 0 { break } hbc := o.getUniqueBids(durations) + hbc.timeTakenCombGen = combGenElapsedTime responseCh <- hbc - Logf("Tid:%v GetUniqueBids Durations:%v Price:%v Time:%v Bids:%v", o.request.ID, hbc.durations[:], hbc.price, hbc.timeTaken, hbc.bidIDs[:]) + Logf("Tid:%v GetUniqueBids Durations:%v Price:%v Time:%v Bids:%v", o.request.ID, hbc.durations[:], hbc.price, hbc.timeTakenCompExcl, hbc.bidIDs[:]) } wg.Done() }() @@ -107,14 +116,20 @@ func (o *AdPodGenerator) getAdPodBids(timeout time.Duration) []*highestCombinati // when all go routines are executed go o.cleanup(wg, responseCh) + totalTimeByCombGen := int64(0) + totalTimeByCompExcl := int64(0) for !isTimedOutORReceivedAllResponses { select { case hbc, ok := <-responseCh: + if false == ok { isTimedOutORReceivedAllResponses = true break } if nil != hbc { + combinationCount++ + totalTimeByCombGen += int64(hbc.timeTakenCombGen) + totalTimeByCompExcl += int64(hbc.timeTakenCompExcl) results = append(results, hbc) } case <-ticker.C: @@ -124,6 +139,24 @@ func (o *AdPodGenerator) getAdPodBids(timeout time.Duration) []*highestCombinati } defer ticker.Stop() + + labels := pbsmetrics.PodLabels{ + AlgorithmName: string(CombinationGeneratorV1), + NoOfCombinations: new(int), + } + *labels.NoOfCombinations = combinationCount + o.met.RecordPodCombGenTime(labels, time.Duration(totalTimeByCombGen)) + + compExclLabels := pbsmetrics.PodLabels{ + AlgorithmName: string(CompetitiveExclusionV1), + NoOfResponseBids: new(int), + } + *compExclLabels.NoOfResponseBids = 0 + for _, ads := range o.buckets { + *compExclLabels.NoOfResponseBids += len(ads) + } + o.met.RecordPodCompititveExclusionTime(compExclLabels, time.Duration(totalTimeByCompExcl)) + return results[:] } @@ -193,7 +226,7 @@ func (o *AdPodGenerator) getUniqueBids(durationSequence []int) *highestCombinati } hbc := findUniqueCombinations(data[:], combinations[:], *o.adpod.IABCategoryExclusionPercent, *o.adpod.AdvertiserExclusionPercent) hbc.durations = durationSequence[:] - hbc.timeTaken = time.Since(startTime) + hbc.timeTakenCompExcl = time.Since(startTime) return hbc } diff --git a/endpoints/openrtb2/ctv/constant.go b/endpoints/openrtb2/ctv/constant.go index fd7beebc6fc..e3b7af8ad3e 100644 --- a/endpoints/openrtb2/ctv/constant.go +++ b/endpoints/openrtb2/ctv/constant.go @@ -39,3 +39,13 @@ const ( CTVRCCategoryExclusion FilterReasonCode = 2 CTVRCDomainExclusion FilterReasonCode = 3 ) + +// MonitorKey provides the unique key for moniroting the algorithms +type MonitorKey string + +const ( + // CombinationGeneratorV1 ... + CombinationGeneratorV1 MonitorKey = "comp_exclusion_v1" + // CompetitiveExclusionV1 ... + CompetitiveExclusionV1 MonitorKey = "comp_exclusion_v1" +) diff --git a/endpoints/openrtb2/ctv/impressions/impressions.go b/endpoints/openrtb2/ctv/impressions/impressions.go index 1131e05f927..9338e6ece94 100644 --- a/endpoints/openrtb2/ctv/impressions/impressions.go +++ b/endpoints/openrtb2/ctv/impressions/impressions.go @@ -29,6 +29,12 @@ const ( MinMaxAlgorithm ) +// MonitorKey provides the unique key for moniroting the impressions algorithm +var MonitorKey = map[Algorithm]string{ + MaximizeForDuration: `a1_max`, + MinMaxAlgorithm: `a2_min_max`, +} + // Value use to compute Ad Slot Durations and Pod Durations for internal computation // Right now this value is set to 5, based on passed data observations // Observed that typically video impression contains contains minimum and maximum duration in multiples of 5 diff --git a/endpoints/openrtb2/ctv_auction.go b/endpoints/openrtb2/ctv_auction.go index df8835af10d..4754ad06c40 100644 --- a/endpoints/openrtb2/ctv_auction.go +++ b/endpoints/openrtb2/ctv_auction.go @@ -407,7 +407,7 @@ func (deps *ctvEndpointDeps) getAllAdPodImpsConfigs() { if nil == imp.Video || nil == deps.impData[index].VideoExt || nil == deps.impData[index].VideoExt.AdPod { continue } - deps.impData[index].Config = getAdPodImpsConfigs(&imp, deps.impData[index].VideoExt.AdPod) + deps.impData[index].Config = deps.getAdPodImpsConfigs(&imp, deps.impData[index].VideoExt.AdPod) if 0 == len(deps.impData[index].Config) { errorCode := new(int) *errorCode = 101 @@ -417,9 +417,17 @@ func (deps *ctvEndpointDeps) getAllAdPodImpsConfigs() { } //getAdPodImpsConfigs will return number of impressions configurations within adpod -func getAdPodImpsConfigs(imp *openrtb.Imp, adpod *openrtb_ext.VideoAdPod) []*ctv.ImpAdPodConfig { - impGen := impressions.NewImpressions(imp.Video.MinDuration, imp.Video.MaxDuration, adpod, impressions.MinMaxAlgorithm) +func (deps *ctvEndpointDeps) getAdPodImpsConfigs(imp *openrtb.Imp, adpod *openrtb_ext.VideoAdPod) []*ctv.ImpAdPodConfig { + selectedAlgorithm := impressions.MinMaxAlgorithm + labels := pbsmetrics.PodLabels{AlgorithmName: impressions.MonitorKey[selectedAlgorithm], NoOfImpressions: new(int)} + + // monitor + start := time.Now() + impGen := impressions.NewImpressions(imp.Video.MinDuration, imp.Video.MaxDuration, adpod, selectedAlgorithm) impRanges := impGen.Get() + *labels.NoOfImpressions = len(impRanges) + deps.metricsEngine.RecordPodImpGenTime(labels, start) + config := make([]*ctv.ImpAdPodConfig, len(impRanges)) for i, value := range impRanges { config[i] = &ctv.ImpAdPodConfig{ @@ -610,7 +618,7 @@ func (deps *ctvEndpointDeps) doAdPodExclusions() ctv.AdPodBids { deps.impData[index].VideoExt.AdPod) //adpod generator - adpodGenerator := ctv.NewAdPodGenerator(deps.request, index, buckets, comb, deps.impData[index].VideoExt.AdPod) + adpodGenerator := ctv.NewAdPodGenerator(deps.request, index, buckets, comb, deps.impData[index].VideoExt.AdPod, deps.metricsEngine) adpodBids := adpodGenerator.GetAdPodBids() if adpodBids != nil { diff --git a/pbsmetrics/config/metrics.go b/pbsmetrics/config/metrics.go index ce6c0f5a707..edc0d1c1192 100644 --- a/pbsmetrics/config/metrics.go +++ b/pbsmetrics/config/metrics.go @@ -195,6 +195,27 @@ func (me *MultiMetricsEngine) RecordTimeoutNotice(success bool) { } } +// RecordPodImpGenTime across all engines +func (me *MultiMetricsEngine) RecordPodImpGenTime(labels pbsmetrics.PodLabels, startTime time.Time) { + for _, thisME := range *me { + thisME.RecordPodImpGenTime(labels, startTime) + } +} + +// RecordPodCombGenTime as a noop +func (me *MultiMetricsEngine) RecordPodCombGenTime(labels pbsmetrics.PodLabels, elapsedTime time.Duration) { + for _, thisME := range *me { + thisME.RecordPodCombGenTime(labels, elapsedTime) + } +} + +// RecordPodCompititveExclusionTime as a noop +func (me *MultiMetricsEngine) RecordPodCompititveExclusionTime(labels pbsmetrics.PodLabels, elapsedTime time.Duration) { + for _, thisME := range *me { + thisME.RecordPodCompititveExclusionTime(labels, elapsedTime) + } +} + // DummyMetricsEngine is a Noop metrics engine in case no metrics are configured. (may also be useful for tests) type DummyMetricsEngine struct{} @@ -273,3 +294,15 @@ func (me *DummyMetricsEngine) RecordRequestQueueTime(success bool, requestType p // RecordTimeoutNotice as a noop func (me *DummyMetricsEngine) RecordTimeoutNotice(success bool) { } + +// RecordPodImpGenTime as a noop +func (me *DummyMetricsEngine) RecordPodImpGenTime(labels pbsmetrics.PodLabels, start time.Time) { +} + +// RecordPodCombGenTime as a noop +func (me *DummyMetricsEngine) RecordPodCombGenTime(labels pbsmetrics.PodLabels, elapsedTime time.Duration) { +} + +// RecordPodCompititveExclusionTime as a noop +func (me *DummyMetricsEngine) RecordPodCompititveExclusionTime(labels pbsmetrics.PodLabels, elapsedTime time.Duration) { +} diff --git a/pbsmetrics/go_metrics.go b/pbsmetrics/go_metrics.go index cf634cc5ae1..01305fb46f3 100644 --- a/pbsmetrics/go_metrics.go +++ b/pbsmetrics/go_metrics.go @@ -562,6 +562,18 @@ func (me *Metrics) RecordTimeoutNotice(success bool) { return } +// RecordPodImpGenTime as a noop +func (me *Metrics) RecordPodImpGenTime(labels PodLabels, startTime time.Time) { +} + +// RecordPodCombGenTime as a noop +func (me *Metrics) RecordPodCombGenTime(labels PodLabels, elapsedTime time.Duration) { +} + +// RecordPodCompititveExclusionTime as a noop +func (me *Metrics) RecordPodCompititveExclusionTime(labels PodLabels, elapsedTime time.Duration) { +} + func doMark(bidder openrtb_ext.BidderName, meters map[openrtb_ext.BidderName]metrics.Meter) { met, ok := meters[bidder] if ok { diff --git a/pbsmetrics/metrics.go b/pbsmetrics/metrics.go index 770f5750335..430849deca0 100644 --- a/pbsmetrics/metrics.go +++ b/pbsmetrics/metrics.go @@ -36,6 +36,15 @@ type ImpLabels struct { NativeImps bool } +// PodLabels defines metric labels describing algorithm type +// and other labels as per scenario +type PodLabels struct { + AlgorithmName string // AlgorithmName which is used for generating impressions + NoOfImpressions *int // NoOfImpressions represents number of impressions generated + NoOfCombinations *int // NoOfCombinations represents number of combinations generated + NoOfResponseBids *int // NoOfResponseBids represents number of bids responded (including bids with similar duration) +} + // RequestLabels defines metric labels describing the result of a network request. type RequestLabels struct { RequestStatus RequestStatus @@ -276,4 +285,26 @@ type MetricsEngine interface { RecordPrebidCacheRequestTime(success bool, length time.Duration) RecordRequestQueueTime(success bool, requestType RequestType, length time.Duration) RecordTimeoutNotice(sucess bool) + // ad pod specific metrics + + // RecordPodImpGenTime records number of impressions generated and time taken + // by underneath algorithm to generate them + // labels accept name of the algorithm and no of impressions generated + // startTime indicates the time at which algorithm started + // This function will take care of computing the elpased time + RecordPodImpGenTime(labels PodLabels, startTime time.Time) + + // RecordPodCombGenTime records number of combinations generated and time taken + // by underneath algorithm to generate them + // labels accept name of the algorithm and no of combinations generated + // elapsedTime indicates the time taken by combination generator to compute all requested combinations + // This function will take care of computing the elpased time + RecordPodCombGenTime(labels PodLabels, elapsedTime time.Duration) + + // RecordPodCompititveExclusionTime records time take by competitive exclusion + // to compute the final Ad pod Response. + // labels accept name of the algorithm and no of combinations evaluated, total bids + // elapsedTime indicates the time taken by competitive exclusion to form final ad pod response using combinations and exclusion algorithm + // This function will take care of computing the elpased time + RecordPodCompititveExclusionTime(labels PodLabels, elapsedTime time.Duration) } diff --git a/pbsmetrics/prometheus/prometheus.go b/pbsmetrics/prometheus/prometheus.go index 9c06d6032f4..cde0cab0283 100644 --- a/pbsmetrics/prometheus/prometheus.go +++ b/pbsmetrics/prometheus/prometheus.go @@ -42,6 +42,20 @@ type Metrics struct { // Account Metrics accountRequests *prometheus.CounterVec + + // Ad Pod Metrics + + // podImpGenTimer indicates time taken by impression generator + // algorithm to generate impressions for given ad pod request + podImpGenTimer *prometheus.HistogramVec + + // podImpGenTimer indicates time taken by combination generator + // algorithm to generate combination based on bid response and ad pod request + podCombGenTimer *prometheus.HistogramVec + + // podCompExclTimer indicates time taken by compititve exclusion + // algorithm to generate final pod response based on bid response and ad pod request + podCompExclTimer *prometheus.HistogramVec } const ( @@ -85,6 +99,14 @@ const ( requestFailed = "failed" ) +// pod specific constants +const ( + podAlgorithm = "algorithm" + podNoOfImpressions = "no_of_impressions" + podTotalCombinations = "total_combinations" + podNoOfResponseBids = "no_of_response_bids" +) + // NewMetrics initializes a new Prometheus metrics instance with preloaded label values. func NewMetrics(cfg config.PrometheusMetrics) *Metrics { requestTimeBuckets := []float64{0.05, 0.1, 0.15, 0.20, 0.25, 0.3, 0.4, 0.5, 0.75, 1} @@ -211,6 +233,28 @@ func NewMetrics(cfg config.PrometheusMetrics) *Metrics { []string{requestTypeLabel, requestStatusLabel}, queuedRequestTimeBuckets) + // adpod specific metrics + metrics.podImpGenTimer = newHistogram(cfg, metrics.Registry, + "impr_gen", + "Time taken by Ad Pod Impression Generator in seconds", []string{podAlgorithm, podNoOfImpressions}, + // 200 µS, 250 µS, 275 µS, 300 µS + //[]float64{0.000200000, 0.000250000, 0.000275000, 0.000300000}) + // 100 µS, 200 µS, 300 µS, 400 µS, 500 µS, 600 µS, + []float64{0.000100000, 0.000200000, 0.000300000, 0.000400000, 0.000500000, 0.000600000}) + + metrics.podCombGenTimer = newHistogram(cfg, metrics.Registry, + "comb_gen", + "Time taken by Ad Pod Combination Generator in seconds", []string{podAlgorithm, podTotalCombinations}, + // 200 µS, 250 µS, 275 µS, 300 µS + //[]float64{0.000200000, 0.000250000, 0.000275000, 0.000300000}) + []float64{0.000100000, 0.000200000, 0.000300000, 0.000400000, 0.000500000, 0.000600000}) + + metrics.podCompExclTimer = newHistogram(cfg, metrics.Registry, + "comp_excl", + "Time taken by Ad Pod Compititve Exclusion in seconds", []string{podAlgorithm, podNoOfResponseBids}, + // 200 µS, 250 µS, 275 µS, 300 µS + //[]float64{0.000200000, 0.000250000, 0.000275000, 0.000300000}) + []float64{0.000100000, 0.000200000, 0.000300000, 0.000400000, 0.000500000, 0.000600000}) preloadLabelValues(&metrics) return &metrics @@ -421,3 +465,44 @@ func (m *Metrics) RecordTimeoutNotice(success bool) { }).Inc() } } + +// pod specific metrics + +// recordAlgoTime is common method which handles algorithm time performance +func recordAlgoTime(timer *prometheus.HistogramVec, labels pbsmetrics.PodLabels, elapsedTime time.Duration) { + + pmLabels := prometheus.Labels{ + podAlgorithm: labels.AlgorithmName, + } + + if labels.NoOfImpressions != nil { + pmLabels[podNoOfImpressions] = strconv.Itoa(*labels.NoOfImpressions) + } + if labels.NoOfCombinations != nil { + pmLabels[podTotalCombinations] = strconv.Itoa(*labels.NoOfCombinations) + } + if labels.NoOfResponseBids != nil { + pmLabels[podNoOfResponseBids] = strconv.Itoa(*labels.NoOfResponseBids) + } + + timer.With(pmLabels).Observe(elapsedTime.Seconds()) +} + +// RecordPodImpGenTime records number of impressions generated and time taken +// by underneath algorithm to generate them +func (m *Metrics) RecordPodImpGenTime(labels pbsmetrics.PodLabels, start time.Time) { + elapsedTime := time.Since(start) + recordAlgoTime(m.podImpGenTimer, labels, elapsedTime) +} + +// RecordPodCombGenTime records number of combinations generated and time taken +// by underneath algorithm to generate them +func (m *Metrics) RecordPodCombGenTime(labels pbsmetrics.PodLabels, elapsedTime time.Duration) { + recordAlgoTime(m.podCombGenTimer, labels, elapsedTime) +} + +// RecordPodCompititveExclusionTime records number of combinations comsumed for forming +// final ad pod response and time taken by underneath algorithm to generate them +func (m *Metrics) RecordPodCompititveExclusionTime(labels pbsmetrics.PodLabels, elapsedTime time.Duration) { + recordAlgoTime(m.podCompExclTimer, labels, elapsedTime) +} diff --git a/pbsmetrics/prometheus/prometheus_test.go b/pbsmetrics/prometheus/prometheus_test.go index 21f182e2094..ba187603b60 100644 --- a/pbsmetrics/prometheus/prometheus_test.go +++ b/pbsmetrics/prometheus/prometheus_test.go @@ -1,6 +1,7 @@ package prometheusmetrics import ( + "strconv" "testing" "time" @@ -944,6 +945,46 @@ func TestTimeoutNotifications(t *testing.T) { } +func TestRecordPodImpGenTime(t *testing.T) { + impressions := 4 + testAlgorithmMetrics(t, impressions, func(m *Metrics) dto.Histogram { + m.RecordPodImpGenTime(pbsmetrics.PodLabels{AlgorithmName: "sample_imp_algo", NoOfImpressions: &impressions}, time.Now()) + return getHistogramFromHistogramVec(m.podImpGenTimer, podNoOfImpressions, strconv.Itoa(impressions)) + }) +} + +func TestRecordPodCombGenTime(t *testing.T) { + combinations := 5 + testAlgorithmMetrics(t, combinations, func(m *Metrics) dto.Histogram { + m.RecordPodCombGenTime(pbsmetrics.PodLabels{AlgorithmName: "sample_comb_algo", NoOfCombinations: &combinations}, time.Now()) + return getHistogramFromHistogramVec(m.podCombGenTimer, podTotalCombinations, strconv.Itoa(combinations)) + }) +} + +func TestRecordPodCompetitiveExclusionTime(t *testing.T) { + totalBids := 8 + testAlgorithmMetrics(t, totalBids, func(m *Metrics) dto.Histogram { + m.RecordPodCompititveExclusionTime(pbsmetrics.PodLabels{AlgorithmName: "sample_comt_excl_algo", NoOfResponseBids: &totalBids}, time.Now()) + return getHistogramFromHistogramVec(m.podCompExclTimer, podNoOfResponseBids, strconv.Itoa(totalBids)) + }) +} + +func testAlgorithmMetrics(t *testing.T, input int, f func(m *Metrics) dto.Histogram) { + // test input + adRequests := 2 + m := createMetricsForTesting() + var result dto.Histogram + for req := 1; req <= adRequests; req++ { + result = f(m) + } + + // assert observations + assert.Equal(t, uint64(adRequests), result.GetSampleCount(), "ad requests : count") + for _, bucket := range result.Bucket { + assert.Equal(t, uint64(adRequests), bucket.GetCumulativeCount(), "total observations") + } +} + func assertCounterValue(t *testing.T, description, name string, counter prometheus.Counter, expected float64) { m := dto.Metric{} counter.Write(&m) diff --git a/router/router.go b/router/router.go index 6da9800ba43..843fda9ab25 100644 --- a/router/router.go +++ b/router/router.go @@ -6,6 +6,7 @@ import ( "database/sql" "encoding/json" "fmt" + "github.com/prometheus/client_golang/prometheus" "io/ioutil" "net/http" "path/filepath" @@ -422,3 +423,12 @@ func readDefaultRequest(defReqConfig config.DefReqConfig) (map[string]string, [] } return aliases, []byte{} } + +func GetPrometheusRegistry() *prometheus.Registry { + mEngine, ok := g_metrics.(*metricsConf.DetailedMetricsEngine) + if !ok || mEngine == nil || mEngine.PrometheusMetrics == nil { + return nil + } + + return mEngine.PrometheusMetrics.Registry +} From db9a64382c715fe11005af49f5aada690c904e4b Mon Sep 17 00:00:00 2001 From: ShriprasadM Date: Fri, 14 Aug 2020 18:28:00 +0530 Subject: [PATCH 02/22] UOE-5440: Fixed the Unit test issues (#72) Fixed unit test issues Co-authored-by: Sachin Survase Co-authored-by: PubMatic-OpenWrap Co-authored-by: Shriprasad --- pbsmetrics/metrics_mock.go | 15 +++++++++++++++ pbsmetrics/prometheus/prometheus_test.go | 4 ++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/pbsmetrics/metrics_mock.go b/pbsmetrics/metrics_mock.go index d5661f4bfe4..946900ee202 100644 --- a/pbsmetrics/metrics_mock.go +++ b/pbsmetrics/metrics_mock.go @@ -106,3 +106,18 @@ func (me *MetricsEngineMock) RecordRequestQueueTime(success bool, requestType Re func (me *MetricsEngineMock) RecordTimeoutNotice(success bool) { me.Called(success) } + +// RecordPodImpGenTime mock +func (me *MetricsEngineMock) RecordPodImpGenTime(labels PodLabels, startTime time.Time) { + me.Called(labels, startTime) +} + +// RecordPodCombGenTime mock +func (me *MetricsEngineMock) RecordPodCombGenTime(labels PodLabels, elapsedTime time.Duration) { + me.Called(labels, elapsedTime) +} + +// RecordPodCompititveExclusionTime mock +func (me *MetricsEngineMock) RecordPodCompititveExclusionTime(labels PodLabels, elapsedTime time.Duration) { + me.Called(labels, elapsedTime) +} diff --git a/pbsmetrics/prometheus/prometheus_test.go b/pbsmetrics/prometheus/prometheus_test.go index ba187603b60..ed9a81caef6 100644 --- a/pbsmetrics/prometheus/prometheus_test.go +++ b/pbsmetrics/prometheus/prometheus_test.go @@ -956,7 +956,7 @@ func TestRecordPodImpGenTime(t *testing.T) { func TestRecordPodCombGenTime(t *testing.T) { combinations := 5 testAlgorithmMetrics(t, combinations, func(m *Metrics) dto.Histogram { - m.RecordPodCombGenTime(pbsmetrics.PodLabels{AlgorithmName: "sample_comb_algo", NoOfCombinations: &combinations}, time.Now()) + m.RecordPodCombGenTime(pbsmetrics.PodLabels{AlgorithmName: "sample_comb_algo", NoOfCombinations: &combinations}, time.Since(time.Now())) return getHistogramFromHistogramVec(m.podCombGenTimer, podTotalCombinations, strconv.Itoa(combinations)) }) } @@ -964,7 +964,7 @@ func TestRecordPodCombGenTime(t *testing.T) { func TestRecordPodCompetitiveExclusionTime(t *testing.T) { totalBids := 8 testAlgorithmMetrics(t, totalBids, func(m *Metrics) dto.Histogram { - m.RecordPodCompititveExclusionTime(pbsmetrics.PodLabels{AlgorithmName: "sample_comt_excl_algo", NoOfResponseBids: &totalBids}, time.Now()) + m.RecordPodCompititveExclusionTime(pbsmetrics.PodLabels{AlgorithmName: "sample_comt_excl_algo", NoOfResponseBids: &totalBids}, time.Since(time.Now())) return getHistogramFromHistogramVec(m.podCompExclTimer, podNoOfResponseBids, strconv.Itoa(totalBids)) }) } From 501927a632f2ed3e8bdc9e9db16b0541e3113e89 Mon Sep 17 00:00:00 2001 From: PubMatic-OpenWrap Date: Thu, 20 Aug 2020 15:28:32 +0530 Subject: [PATCH 03/22] UOE-5511 Support for skadnetwork in pubmatic (#73) Co-authored-by: Isha Bharti --- adapters/pubmatic/pubmatic.go | 30 ++++++++---- .../pubmatictest/supplemental/app.json | 46 +++++++++++++++++-- openrtb_ext/imp.go | 2 + 3 files changed, 65 insertions(+), 13 deletions(-) diff --git a/adapters/pubmatic/pubmatic.go b/adapters/pubmatic/pubmatic.go index 394e4189dfe..7c7063e07b1 100644 --- a/adapters/pubmatic/pubmatic.go +++ b/adapters/pubmatic/pubmatic.go @@ -20,10 +20,13 @@ import ( "golang.org/x/net/context/ctxhttp" ) -const MAX_IMPRESSIONS_PUBMATIC = 30 -const PUBMATIC = "[PUBMATIC]" -const buyId = "buyid" -const buyIdTargetingKey = "hb_buyid_pubmatic" +const ( + MAX_IMPRESSIONS_PUBMATIC = 30 + PUBMATIC = "[PUBMATIC]" + buyId = "buyid" + buyIdTargetingKey = "hb_buyid_pubmatic" + skAdnetworkKey = "skadn" +) type PubmaticAdapter struct { http *adapters.HTTPAdapter @@ -613,13 +616,22 @@ func parseImpressionObject(imp *openrtb.Imp, wrapExt *string, pubID *string) err } } + imp.Ext = nil + impExt := "" if pubmaticExt.Keywords != nil && len(pubmaticExt.Keywords) != 0 { - kvstr := makeKeywordStr(pubmaticExt.Keywords) - imp.Ext = json.RawMessage([]byte(kvstr)) - } else { - imp.Ext = nil + impExt = makeKeywordStr(pubmaticExt.Keywords) } + if bidderExt.Prebid != nil && bidderExt.Prebid.SKAdnetwork != nil { + if impExt == "" { + impExt = fmt.Sprintf(`"%s":%s`, skAdnetworkKey, string(bidderExt.Prebid.SKAdnetwork)) + } else { + impExt = fmt.Sprintf(`%s,"%s":%s`, impExt, skAdnetworkKey, string(bidderExt.Prebid.SKAdnetwork)) + } + } + if len(impExt) != 0 { + imp.Ext = json.RawMessage([]byte(fmt.Sprintf(`{%s}`, impExt))) + } return nil } @@ -635,7 +647,7 @@ func makeKeywordStr(keywords []*openrtb_ext.ExtImpPubmaticKeyVal) string { } } - kvStr := "{" + strings.Join(eachKv, ",") + "}" + kvStr := strings.Join(eachKv, ",") return kvStr } diff --git a/adapters/pubmatic/pubmatictest/supplemental/app.json b/adapters/pubmatic/pubmatictest/supplemental/app.json index 636433ca1f5..3aabe54a7dd 100644 --- a/adapters/pubmatic/pubmatictest/supplemental/app.json +++ b/adapters/pubmatic/pubmatictest/supplemental/app.json @@ -26,6 +26,12 @@ "version": 1, "profile": 5123 } + }, + "prebid": { + "skadn": { + "skadnetids": ["k674qkevps.skadnetwork"], + "version": "2.0" + } } } }], @@ -62,7 +68,11 @@ }, "ext": { "pmZoneID": "Zone1,Zone2", - "preference": "sports,movies" + "preference": "sports,movies", + "skadn": { + "skadnetids": ["k674qkevps.skadnetwork"], + "version": "2.0" + } } } ], @@ -100,7 +110,21 @@ "crid": "29681110", "h": 250, "w": 300, - "dealid":"test deal" + "dealid":"test deal", + "ext": { + "dspid": 6, + "deal_channel": 1, + "skadn": { + "signature": "MDUCGQDreBN5/xBN547tJeUdqcMSBtBA+Lk06b8CGFkjR1V56rh/H9osF8iripkuZApeDsZ+lQ==", + "campaign": "4", + "network": "k674qkevps.skadnetwork", + "nonce": "D0EC0F04-A4BF-445B-ADF1-E010430C29FD", + "timestamp": "1596695461984", + "sourceapp": "525463029", + "itunesitem": "1499436635", + "version": "2.0" + } + } }] } ], @@ -126,11 +150,25 @@ "crid": "29681110", "w": 300, "h": 250, - "dealid":"test deal" + "dealid":"test deal", + "ext": { + "dspid": 6, + "deal_channel": 1, + "skadn": { + "signature": "MDUCGQDreBN5/xBN547tJeUdqcMSBtBA+Lk06b8CGFkjR1V56rh/H9osF8iripkuZApeDsZ+lQ==", + "campaign": "4", + "network": "k674qkevps.skadnetwork", + "nonce": "D0EC0F04-A4BF-445B-ADF1-E010430C29FD", + "timestamp": "1596695461984", + "sourceapp": "525463029", + "itunesitem": "1499436635", + "version": "2.0" + } + } }, "type": "banner" } ] } ] - } \ No newline at end of file + } diff --git a/openrtb_ext/imp.go b/openrtb_ext/imp.go index ed3a88d62eb..d8ebecb3dc6 100644 --- a/openrtb_ext/imp.go +++ b/openrtb_ext/imp.go @@ -28,6 +28,8 @@ type ExtImpPrebid struct { // at this time // https://github.com/PubMatic-OpenWrap/prebid-server/pull/846#issuecomment-476352224 Bidder map[string]json.RawMessage `json:"bidder"` + + SKAdnetwork json.RawMessage `json:"skadn"` } // ExtStoredRequest defines the contract for bidrequest.imp[i].ext.prebid.storedrequest From ebe98c795ed936bf8c3cf6784acdd0a2e9da8796 Mon Sep 17 00:00:00 2001 From: Shriprasad Date: Tue, 1 Sep 2020 18:06:41 +0530 Subject: [PATCH 04/22] Resolved merge issues --- endpoints/openrtb2/ctv/response/adpod_generator.go | 10 +++++----- endpoints/openrtb2/ctv_auction.go | 5 ++--- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/endpoints/openrtb2/ctv/response/adpod_generator.go b/endpoints/openrtb2/ctv/response/adpod_generator.go index 5c69063bd55..059b2774bee 100644 --- a/endpoints/openrtb2/ctv/response/adpod_generator.go +++ b/endpoints/openrtb2/ctv/response/adpod_generator.go @@ -25,7 +25,7 @@ type filteredBid struct { reasonCode constant.FilterReasonCode } type highestCombination struct { - bids []*Bid + bids []*types.Bid bidIDs []string durations []int price float64 @@ -48,7 +48,7 @@ type AdPodGenerator struct { } //NewAdPodGenerator will generate adpod based on configuration -func NewAdPodGenerator(request *openrtb.BidRequest, impIndex int, buckets BidsBuckets, comb ICombination, adpod *openrtb_ext.VideoAdPod, met pbsmetrics.MetricsEngine) *AdPodGenerator { +func NewAdPodGenerator(request *openrtb.BidRequest, impIndex int, buckets types.BidsBuckets, comb combination.ICombination, adpod *openrtb_ext.VideoAdPod, met pbsmetrics.MetricsEngine) *AdPodGenerator { return &AdPodGenerator{ request: request, impIndex: impIndex, @@ -83,7 +83,7 @@ func (o *AdPodGenerator) cleanup(wg *sync.WaitGroup, responseCh chan *highestCom func (o *AdPodGenerator) getAdPodBids(timeout time.Duration) []*highestCombination { start := time.Now() - defer TimeTrack(start, fmt.Sprintf("Tid:%v ImpId:%v getAdPodBids", o.request.ID, o.request.Imp[o.impIndex].ID)) + defer util.TimeTrack(start, fmt.Sprintf("Tid:%v ImpId:%v getAdPodBids", o.request.ID, o.request.Imp[o.impIndex].ID)) maxRoutines := 3 isTimedOutORReceivedAllResponses := false @@ -145,14 +145,14 @@ func (o *AdPodGenerator) getAdPodBids(timeout time.Duration) []*highestCombinati defer ticker.Stop() labels := pbsmetrics.PodLabels{ - AlgorithmName: string(CombinationGeneratorV1), + AlgorithmName: string(constant.CombinationGeneratorV1), NoOfCombinations: new(int), } *labels.NoOfCombinations = combinationCount o.met.RecordPodCombGenTime(labels, time.Duration(totalTimeByCombGen)) compExclLabels := pbsmetrics.PodLabels{ - AlgorithmName: string(CompetitiveExclusionV1), + AlgorithmName: string(constant.CompetitiveExclusionV1), NoOfResponseBids: new(int), } *compExclLabels.NoOfResponseBids = 0 diff --git a/endpoints/openrtb2/ctv_auction.go b/endpoints/openrtb2/ctv_auction.go index 1f460fcc3dc..ec313a575bd 100644 --- a/endpoints/openrtb2/ctv_auction.go +++ b/endpoints/openrtb2/ctv_auction.go @@ -16,7 +16,6 @@ import ( "github.com/PubMatic-OpenWrap/openrtb" "github.com/PubMatic-OpenWrap/prebid-server/analytics" "github.com/PubMatic-OpenWrap/prebid-server/config" - "github.com/PubMatic-OpenWrap/prebid-server/endpoints/openrtb2/ctv" "github.com/PubMatic-OpenWrap/prebid-server/endpoints/openrtb2/ctv/combination" "github.com/PubMatic-OpenWrap/prebid-server/endpoints/openrtb2/ctv/constant" "github.com/PubMatic-OpenWrap/prebid-server/endpoints/openrtb2/ctv/impressions" @@ -422,7 +421,7 @@ func (deps *ctvEndpointDeps) getAllAdPodImpsConfigs() { } //getAdPodImpsConfigs will return number of impressions configurations within adpod -func (deps *ctvEndpointDeps) getAdPodImpsConfigs(imp *openrtb.Imp, adpod *openrtb_ext.VideoAdPod) []*ctv.ImpAdPodConfig { +func (deps *ctvEndpointDeps) getAdPodImpsConfigs(imp *openrtb.Imp, adpod *openrtb_ext.VideoAdPod) []*types.ImpAdPodConfig { selectedAlgorithm := impressions.MinMaxAlgorithm labels := pbsmetrics.PodLabels{AlgorithmName: impressions.MonitorKey[selectedAlgorithm], NoOfImpressions: new(int)} @@ -433,7 +432,7 @@ func (deps *ctvEndpointDeps) getAdPodImpsConfigs(imp *openrtb.Imp, adpod *openrt *labels.NoOfImpressions = len(impRanges) deps.metricsEngine.RecordPodImpGenTime(labels, start) - config := make([]*ctv.ImpAdPodConfig, len(impRanges)) + config := make([]*types.ImpAdPodConfig, len(impRanges)) for i, value := range impRanges { config[i] = &types.ImpAdPodConfig{ ImpID: util.GetCTVImpressionID(imp.ID, i+1), From dc4d192c177b2220d9a41fa8146ddef57216f3db Mon Sep 17 00:00:00 2001 From: Viral Vala Date: Wed, 9 Sep 2020 16:04:20 +0530 Subject: [PATCH 05/22] BugID:OTT-17 First Commit --- endpoints/openrtb2/ctv_auction.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/endpoints/openrtb2/ctv_auction.go b/endpoints/openrtb2/ctv_auction.go index 00fe0c4a5e2..c8d8c07b100 100644 --- a/endpoints/openrtb2/ctv_auction.go +++ b/endpoints/openrtb2/ctv_auction.go @@ -692,7 +692,7 @@ func (deps *ctvEndpointDeps) getBidResponseExt(resp *openrtb.BidResponse) (data if nil != imp.Bid && len(imp.Bid.Bids) > 0 { for _, bid := range imp.Bid.Bids { //update adm - bid.AdM = constant.VASTDefaultTag + //bid.AdM = constant.VASTDefaultTag //add duration value raw, err := jsonparser.Set(bid.Ext, []byte(strconv.Itoa(int(bid.Duration))), "prebid", "video", "duration") From 58b356d5ad53b9eee1a40086eccb55ae00e36d1e Mon Sep 17 00:00:00 2001 From: ShriprasadM Date: Tue, 15 Sep 2020 12:37:55 +0530 Subject: [PATCH 06/22] OTT-18: moved VideoAuction to selector pattern. This required for mocking PBS response (#76) Co-authored-by: Shriprasad --- main.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/main.go b/main.go index 0fa4454026b..802714590e0 100644 --- a/main.go +++ b/main.go @@ -48,9 +48,9 @@ func main() { */ func InitPrebidServer(configFile string) { - //init contents + //init contents rand.Seed(time.Now().UnixNano()) - + //main contents cfg, err := loadConfig(configFile) if err != nil { @@ -96,7 +96,7 @@ func OrtbAuction(w http.ResponseWriter, r *http.Request) error { return router.OrtbAuctionEndpointWrapper(w, r) } -func VideoAuction(w http.ResponseWriter, r *http.Request) error { +var VideoAuction = func(w http.ResponseWriter, r *http.Request) error { return router.VideoAuctionEndpointWrapper(w, r) } From e5648b4750b830930eb7763f6023722c79e70b37 Mon Sep 17 00:00:00 2001 From: Shriprasad Date: Mon, 21 Sep 2020 20:03:25 +0530 Subject: [PATCH 07/22] OTT-24: Basic support for sorting the deal bids in forming the final ad pod response --- endpoints/openrtb2/ctv/types/adpod_types.go | 5 +- endpoints/openrtb2/ctv/util/util.go | 24 +++- endpoints/openrtb2/ctv/util/util_test.go | 126 ++++++++++++++++++++ endpoints/openrtb2/ctv_auction.go | 8 +- openrtb_ext/bid.go | 11 +- 5 files changed, 163 insertions(+), 11 deletions(-) diff --git a/endpoints/openrtb2/ctv/types/adpod_types.go b/endpoints/openrtb2/ctv/types/adpod_types.go index 0a906d970cf..a3348d7d816 100644 --- a/endpoints/openrtb2/ctv/types/adpod_types.go +++ b/endpoints/openrtb2/ctv/types/adpod_types.go @@ -9,8 +9,9 @@ import ( //Bid openrtb bid object with extra parameters type Bid struct { *openrtb.Bid - Duration int - FilterReasonCode constant.FilterReasonCode + Duration int + FilterReasonCode constant.FilterReasonCode + DealTierSatisfied bool } //ExtCTVBidResponse object for ctv bid resposne object diff --git a/endpoints/openrtb2/ctv/util/util.go b/endpoints/openrtb2/ctv/util/util.go index 11f78e02f24..9b045c9e9db 100644 --- a/endpoints/openrtb2/ctv/util/util.go +++ b/endpoints/openrtb2/ctv/util/util.go @@ -10,6 +10,7 @@ import ( "github.com/PubMatic-OpenWrap/prebid-server/endpoints/openrtb2/ctv/constant" "github.com/PubMatic-OpenWrap/prebid-server/endpoints/openrtb2/ctv/types" + "github.com/PubMatic-OpenWrap/prebid-server/openrtb_ext" "github.com/golang/glog" ) @@ -21,13 +22,34 @@ func GetDurationWiseBidsBucket(bids []*types.Bid) types.BidsBuckets { } for k, v := range result { - sort.Slice(v[:], func(i, j int) bool { return v[i].Price > v[j].Price }) + //sort.Slice(v[:], func(i, j int) bool { return v[i].Price > v[j].Price }) + sortBids(v[:]) result[k] = v } return result } +func sortBids(bids []*types.Bid) { + sort.Slice(bids, func(i, j int) bool { + if bids[i].DealTierSatisfied == bids[j].DealTierSatisfied { + return bids[i].Price > bids[j].Price + } + return bids[i].DealTierSatisfied + }) +} + +// GetDealTierSatisfied ... +func GetDealTierSatisfied(bid types.Bid) (bool, error) { + ext := openrtb_ext.ExtBid{} + err := json.Unmarshal(bid.Ext, &ext) + if nil != err { + //TODO warn can not determine deal tier is satisfied + return false, err + } + return ext.Prebid.DealTierSatisfied, nil +} + func DecodeImpressionID(id string) (string, int) { index := strings.LastIndex(id, constant.CTVImpressionIDSeparator) if index == -1 { diff --git a/endpoints/openrtb2/ctv/util/util_test.go b/endpoints/openrtb2/ctv/util/util_test.go index 3c84d93dd4e..fd1f8d1d68a 100644 --- a/endpoints/openrtb2/ctv/util/util_test.go +++ b/endpoints/openrtb2/ctv/util/util_test.go @@ -1,8 +1,12 @@ package util import ( + "fmt" "testing" + "github.com/PubMatic-OpenWrap/openrtb" + + "github.com/PubMatic-OpenWrap/prebid-server/endpoints/openrtb2/ctv/types" "github.com/stretchr/testify/assert" ) @@ -48,3 +52,125 @@ func TestDecodeImpressionID(t *testing.T) { }) } } + +func TestSortByDealPriority(t *testing.T) { + + type testbid struct { + id string + price float64 + isDealBid bool + } + + testcases := []struct { + scenario string + bids []testbid + expectedBidIDOrdering []string + }{ + /* tests based on truth table */ + { + scenario: "all_deal_bids_do_price_based_sort", + bids: []testbid{ + {id: "DB_$5", price: 5.0, isDealBid: true}, // Deal bid with low price + {id: "DB_$10", price: 10.0, isDealBid: true}, // Deal bid with high price + }, + expectedBidIDOrdering: []string{"DB_$10", "DB_$5"}, // sort by price among deal bids + }, + { + scenario: "normal_and_deal_bid_mix_case_1", + bids: []testbid{ + {id: "DB_$15", price: 15.0, isDealBid: true}, // Deal bid with low price + {id: "B_$30", price: 30.0, isDealBid: false}, // Normal bid with high price + }, + expectedBidIDOrdering: []string{"DB_$15", "B_$30"}, // no sort expected. Deal bid is already 1st in order + }, + { + scenario: "normal_and_deal_bid_mix_case_2", // deal bids are not at start position in order + bids: []testbid{ + {id: "B_$30", price: 30.0, isDealBid: false}, // Normal bid with high price + {id: "DB_$15", price: 15.0, isDealBid: true}, // Deal bid with low price + }, + expectedBidIDOrdering: []string{"DB_$15", "B_$30"}, // sort based on deal bid + }, + { + scenario: "all_normal_bids_sort_by_price_case_1", + bids: []testbid{ + {id: "B_$5", price: 5.0, isDealBid: false}, + {id: "B_$10", price: 10.0, isDealBid: false}, + }, + expectedBidIDOrdering: []string{"B_$10", "B_$5"}, // sort by price + }, + { + scenario: "all_normal_bids_sort_by_price_case_2", // already sorted by highest price + bids: []testbid{ + {id: "B_$10", price: 10.0, isDealBid: false}, + {id: "B_$5", price: 5.0, isDealBid: false}, + }, + expectedBidIDOrdering: []string{"B_$10", "B_$5"}, // no sort required as already sorted + }, + /* use cases */ + { + scenario: "deal_bids_with_same_price", + bids: []testbid{ + {id: "DB2_$10", price: 10.0, isDealBid: true}, + {id: "DB1_$10", price: 10.0, isDealBid: true}, + }, + expectedBidIDOrdering: []string{"DB2_$10", "DB1_$10"}, // no sort expected + }, + /* more than 2 Bids testcases */ + { + scenario: "4_bids_with_first_and_last_are_deal_bids", + bids: []testbid{ + {id: "DB_$15", price: 15.0, isDealBid: true}, // deal bid with low CPM than another bid + {id: "B_$40", price: 40.0, isDealBid: false}, // normal bid with highest CPM + {id: "B_$3", price: 3.0, isDealBid: false}, + {id: "DB_$20", price: 20.0, isDealBid: true}, // deal bid with high cpm than another deal bid + }, + expectedBidIDOrdering: []string{"DB_$20", "DB_$15", "B_$40", "B_$3"}, + }, + { + scenario: "deal_bids_and_normal_bids_with_same_price", + bids: []testbid{ + {id: "B1_$7", price: 7.0, isDealBid: false}, + {id: "DB2_$7", price: 7.0, isDealBid: true}, + {id: "B3_$7", price: 7.0, isDealBid: false}, + {id: "DB1_$7", price: 7.0, isDealBid: true}, + {id: "B2_$7", price: 7.0, isDealBid: false}, + }, + expectedBidIDOrdering: []string{"DB2_$7", "DB1_$7", "B1_$7", "B3_$7", "B2_$7"}, // no sort expected + }, + } + + newBid := func(bid testbid) *types.Bid { + return &types.Bid{ + Bid: &openrtb.Bid{ + ID: bid.id, + Price: bid.price, + //Ext: json.RawMessage(`{"prebid":{ "dealTierSatisfied" : ` + bid.isDealBid + ` }}`), + }, + DealTierSatisfied: bid.isDealBid, + } + } + + for _, test := range testcases { + // if test.scenario != "deal_bids_and_normal_bids_with_same_price" { + // continue + // } + fmt.Println("Scenario : ", test.scenario) + bids := []*types.Bid{} + for _, bid := range test.bids { + bids = append(bids, newBid(bid)) + } + for _, bid := range bids { + fmt.Println(bid.ID, ",", bid.Price, ",", bid.DealTierSatisfied) + } + sortBids(bids[:]) + fmt.Println("After sort") + actual := []string{} + for _, bid := range bids { + fmt.Println(bid.ID, ",", bid.Price, ", ", bid.DealTierSatisfied) + actual = append(actual, bid.ID) + } + assert.Equal(t, test.expectedBidIDOrdering, actual, test.scenario+" failed") + fmt.Println("") + } +} diff --git a/endpoints/openrtb2/ctv_auction.go b/endpoints/openrtb2/ctv_auction.go index c8d8c07b100..ec907585dcb 100644 --- a/endpoints/openrtb2/ctv_auction.go +++ b/endpoints/openrtb2/ctv_auction.go @@ -186,7 +186,6 @@ func (deps *ctvEndpointDeps) CTVAuctionEndpoint(w http.ResponseWriter, r *http.R } response, err = deps.holdAuction(request, usersyncs) - ao.Request = request ao.Response = response if err != nil || nil == response { @@ -552,9 +551,10 @@ func (deps *ctvEndpointDeps) getBids(resp *openrtb.BidResponse) { bid.ID = util.GetUniqueBidID(bid.ID, len(impBids.Bids)+1) impBids.Bids = append(impBids.Bids, &types.Bid{ - Bid: bid, - FilterReasonCode: constant.CTVRCDidNotGetChance, - Duration: int(deps.impData[index].Config[sequenceNumber-1].MaxDuration), + Bid: bid, + FilterReasonCode: constant.CTVRCDidNotGetChance, + Duration: int(deps.impData[index].Config[sequenceNumber-1].MaxDuration), + DealTierSatisfied: util.GetDealTierSatisfied(bid), }) } } diff --git a/openrtb_ext/bid.go b/openrtb_ext/bid.go index 3b297c7ab5d..c1ba5bdbadb 100644 --- a/openrtb_ext/bid.go +++ b/openrtb_ext/bid.go @@ -12,11 +12,14 @@ type ExtBid struct { } // ExtBidPrebid defines the contract for bidresponse.seatbid.bid[i].ext.prebid +// DealPriority represents priority of deal bid. If its non deal bid then value will be 0 type ExtBidPrebid struct { - Cache *ExtBidPrebidCache `json:"cache,omitempty"` - Targeting map[string]string `json:"targeting,omitempty"` - Type BidType `json:"type"` - Video *ExtBidPrebidVideo `json:"video,omitempty"` + Cache *ExtBidPrebidCache `json:"cache,omitempty"` + Targeting map[string]string `json:"targeting,omitempty"` + Type BidType `json:"type"` + Video *ExtBidPrebidVideo `json:"video,omitempty"` + DealPriority int `json:"dealpriority,omitempty"` + DealTierSatisfied bool `json:"dealtiersatisfied,omitempty"` } // ExtBidPrebidCache defines the contract for bidresponse.seatbid.bid[i].ext.prebid.cache From 0f8f81e818cc81586f226d0af9e84d3915c0f77c Mon Sep 17 00:00:00 2001 From: Shriprasad Date: Thu, 24 Sep 2020 13:14:44 +0530 Subject: [PATCH 08/22] OTT-24: Added changes around https://github.com/prebid/prebid-server/issues/1503 (Proposal for Prebid Server) --- endpoints/openrtb2/ctv/util/util.go | 7 ++++--- exchange/bidder.go | 11 ++++++----- exchange/exchange.go | 11 ++++++++--- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/endpoints/openrtb2/ctv/util/util.go b/endpoints/openrtb2/ctv/util/util.go index 9b045c9e9db..cad3d1bd7e7 100644 --- a/endpoints/openrtb2/ctv/util/util.go +++ b/endpoints/openrtb2/ctv/util/util.go @@ -12,6 +12,7 @@ import ( "github.com/PubMatic-OpenWrap/prebid-server/endpoints/openrtb2/ctv/types" "github.com/PubMatic-OpenWrap/prebid-server/openrtb_ext" "github.com/golang/glog" + "github.com/PubMatic-OpenWrap/openrtb" ) func GetDurationWiseBidsBucket(bids []*types.Bid) types.BidsBuckets { @@ -40,14 +41,14 @@ func sortBids(bids []*types.Bid) { } // GetDealTierSatisfied ... -func GetDealTierSatisfied(bid types.Bid) (bool, error) { +func GetDealTierSatisfied(bid *openrtb.Bid) bool { ext := openrtb_ext.ExtBid{} err := json.Unmarshal(bid.Ext, &ext) if nil != err { //TODO warn can not determine deal tier is satisfied - return false, err + return false } - return ext.Prebid.DealTierSatisfied, nil + return ext.Prebid.DealTierSatisfied } func DecodeImpressionID(id string) (string, int) { diff --git a/exchange/bidder.go b/exchange/bidder.go index f3d8e794b60..7e28214890a 100644 --- a/exchange/bidder.go +++ b/exchange/bidder.go @@ -58,11 +58,12 @@ type adaptedBidder interface { // pbsOrtbBid.bidVideo is optional but should be filled out by the Bidder if bidType is video. // pbsOrtbBid.dealPriority will become "response.seatbid[i].bid.dealPriority" in the final OpenRTB response. type pbsOrtbBid struct { - bid *openrtb.Bid - bidType openrtb_ext.BidType - bidTargets map[string]string - bidVideo *openrtb_ext.ExtBidPrebidVideo - dealPriority int + bid *openrtb.Bid + bidType openrtb_ext.BidType + bidTargets map[string]string + bidVideo *openrtb_ext.ExtBidPrebidVideo + dealPriority int + dealTierSatisfied bool } // pbsOrtbSeatBid is a SeatBid returned by an adaptedBidder. diff --git a/exchange/exchange.go b/exchange/exchange.go index fe57255f457..6ac36071c2d 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -7,6 +7,8 @@ import ( "errors" "fmt" "math/rand" + + // "math/rand" "net/http" "runtime/debug" "sort" @@ -290,6 +292,7 @@ func updateHbPbCatDur(bid *pbsOrtbBid, dealTierInfo *DealTierInfo, bidCategory m newCatDur := strings.Join(oldCatDurSplit, "") bidCategory[bid.bid.ID] = newCatDur + bid.dealTierSatisfied = true } } } @@ -741,9 +744,11 @@ func (e *exchange) makeBid(Bids []*pbsOrtbBid, adapter openrtb_ext.BidderName, a bidExt := &openrtb_ext.ExtBid{ Bidder: thisBid.bid.Ext, Prebid: &openrtb_ext.ExtBidPrebid{ - Targeting: thisBid.bidTargets, - Type: thisBid.bidType, - Video: thisBid.bidVideo, + Targeting: thisBid.bidTargets, + Type: thisBid.bidType, + Video: thisBid.bidVideo, + DealPriority: thisBid.dealPriority, + DealTierSatisfied: thisBid.dealTierSatisfied, }, } if cacheInfo, found := e.getBidCacheInfo(thisBid, auc); found { From 766a92a3e592eb7a42db393c51cdfbe39b5d5ed8 Mon Sep 17 00:00:00 2001 From: Viral Vala Date: Mon, 28 Sep 2020 20:31:35 +0530 Subject: [PATCH 09/22] OTT-27, OTT-32 Supporting Deal Prioritization for CTV --- .../openrtb2/ctv/response/adpod_generator.go | 11 ++++- endpoints/openrtb2/ctv/util/util.go | 11 +---- endpoints/openrtb2/ctv_auction.go | 8 +++- exchange/exchange.go | 41 ++++++++++--------- openrtb_ext/request.go | 1 + 5 files changed, 41 insertions(+), 31 deletions(-) diff --git a/endpoints/openrtb2/ctv/response/adpod_generator.go b/endpoints/openrtb2/ctv/response/adpod_generator.go index dc54f845513..d6fcfb4ccd9 100644 --- a/endpoints/openrtb2/ctv/response/adpod_generator.go +++ b/endpoints/openrtb2/ctv/response/adpod_generator.go @@ -32,6 +32,7 @@ type highestCombination struct { domainScore map[string]int filteredBids map[string]*filteredBid timeTaken time.Duration + nDealBids int } //AdPodGenerator AdPodGenerator @@ -146,7 +147,7 @@ func (o *AdPodGenerator) getMaxAdPodBid(results []*highestCombination) *types.Ad } } - if len(result.bidIDs) > 0 && (nil == maxResult || maxResult.price < result.price) { + if len(result.bidIDs) > 0 && (nil == maxResult || maxResult.nDealBids < result.nDealBids || maxResult.price < result.price) { maxResult = result } } @@ -228,7 +229,7 @@ func findUniqueCombinations(data [][]*types.Bid, combination []int, maxCategoryS ehc, inext, jnext, rc = evaluate(data[:], indices[:], totalBids, maxCategoryScore, maxDomainScore) if nil != ehc { - if nil == hc || hc.price < ehc.price { + if nil == hc || hc.nDealBids < ehc.nDealBids || hc.price < ehc.price { hc = ehc } else { // if you see current combination price lower than the highest one then break the loop @@ -305,6 +306,7 @@ func evaluate(bids [][]*types.Bid, indices [][]int, totalBids int, maxCategorySc price: 0, categoryScore: make(map[string]int), domainScore: make(map[string]int), + nDealBids: 0, } pos := 0 @@ -316,6 +318,11 @@ func evaluate(bids [][]*types.Bid, indices [][]int, totalBids int, maxCategorySc hbc.bidIDs[pos] = bid.ID pos++ + //nDealBids + if bid.DealTierSatisfied { + hbc.nDealBids++ + } + //Price hbc.price = hbc.price + bid.Price diff --git a/endpoints/openrtb2/ctv/util/util.go b/endpoints/openrtb2/ctv/util/util.go index cad3d1bd7e7..32cb8d29e27 100644 --- a/endpoints/openrtb2/ctv/util/util.go +++ b/endpoints/openrtb2/ctv/util/util.go @@ -12,7 +12,6 @@ import ( "github.com/PubMatic-OpenWrap/prebid-server/endpoints/openrtb2/ctv/types" "github.com/PubMatic-OpenWrap/prebid-server/openrtb_ext" "github.com/golang/glog" - "github.com/PubMatic-OpenWrap/openrtb" ) func GetDurationWiseBidsBucket(bids []*types.Bid) types.BidsBuckets { @@ -41,14 +40,8 @@ func sortBids(bids []*types.Bid) { } // GetDealTierSatisfied ... -func GetDealTierSatisfied(bid *openrtb.Bid) bool { - ext := openrtb_ext.ExtBid{} - err := json.Unmarshal(bid.Ext, &ext) - if nil != err { - //TODO warn can not determine deal tier is satisfied - return false - } - return ext.Prebid.DealTierSatisfied +func GetDealTierSatisfied(ext *openrtb_ext.ExtBid) bool { + return ext != nil && ext.Prebid != nil && ext.Prebid.DealTierSatisfied } func DecodeImpressionID(id string) (string, int) { diff --git a/endpoints/openrtb2/ctv_auction.go b/endpoints/openrtb2/ctv_auction.go index ec907585dcb..1e8a8c1c16b 100644 --- a/endpoints/openrtb2/ctv_auction.go +++ b/endpoints/openrtb2/ctv_auction.go @@ -537,6 +537,12 @@ func (deps *ctvEndpointDeps) getBids(resp *openrtb.BidResponse) { } vseat.Bid = append(vseat.Bid, *bid) } else { + //reading extension, ingorning parsing error + ext := openrtb_ext.ExtBid{} + if nil != bid.Ext { + json.Unmarshal(bid.Ext, &ext) + } + //Adding adpod bids impBids, ok := result[originalImpID] if !ok { @@ -554,7 +560,7 @@ func (deps *ctvEndpointDeps) getBids(resp *openrtb.BidResponse) { Bid: bid, FilterReasonCode: constant.CTVRCDidNotGetChance, Duration: int(deps.impData[index].Config[sequenceNumber-1].MaxDuration), - DealTierSatisfied: util.GetDealTierSatisfied(bid), + DealTierSatisfied: util.GetDealTierSatisfied(&ext), }) } } diff --git a/exchange/exchange.go b/exchange/exchange.go index 6ac36071c2d..ea3cb36b4fe 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -600,31 +600,34 @@ func applyCategoryMapping(ctx context.Context, requestExt openrtb_ext.ExtRequest categoryDuration = fmt.Sprintf("%s_%ds", pb, newDur) } - if dupe, ok := dedupe[categoryDuration]; ok { - // 50% chance for either bid with duplicate categoryDuration values to be kept - if rand.Intn(100) < 50 { - if dupe.bidderName == bidderName { - // An older bid from the current bidder - bidsToRemove = append(bidsToRemove, dupe.bidIndex) - rejections = updateRejections(rejections, dupe.bidID, "Bid was deduplicated") - } 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) + if false == brandCatExt.SkipDedup { + if dupe, ok := dedupe[categoryDuration]; ok { + // 50% chance for either bid with duplicate categoryDuration values to be kept + if rand.Intn(100) < 50 { + if dupe.bidderName == bidderName { + // An older bid from the current bidder + bidsToRemove = append(bidsToRemove, dupe.bidIndex) rejections = updateRejections(rejections, dupe.bidID, "Bid was deduplicated") } else { - oldSeatBid.bids = append(oldSeatBid.bids[:dupe.bidIndex], oldSeatBid.bids[dupe.bidIndex+1:]...) + // 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:]...) + } } + delete(res, dupe.bidID) + } else { + // Remove this bid + bidsToRemove = append(bidsToRemove, bidInd) + rejections = updateRejections(rejections, bidID, "Bid was deduplicated") + continue } - delete(res, dupe.bidID) - } else { - // Remove this bid - bidsToRemove = append(bidsToRemove, bidInd) - rejections = updateRejections(rejections, bidID, "Bid was deduplicated") - continue } } + res[bidID] = categoryDuration dedupe[categoryDuration] = bidDedupe{bidderName: bidderName, bidIndex: bidInd, bidID: bidID} } diff --git a/openrtb_ext/request.go b/openrtb_ext/request.go index a0c74af6891..ca7c0b40c17 100644 --- a/openrtb_ext/request.go +++ b/openrtb_ext/request.go @@ -64,6 +64,7 @@ type ExtIncludeBrandCategory struct { Publisher string `json:"publisher"` WithCategory bool `json:"withcategory"` TranslateCategories *bool `json:"translatecategories,omitempty"` + SkipDedup bool `json:"skipdedup"` } // Make an unmarshaller that will set a default PriceGranularity From bc974f5f070203f13a8ad5e7a47ad165ba5e1e1b Mon Sep 17 00:00:00 2001 From: PubMatic-OpenWrap Date: Wed, 7 Oct 2020 17:18:18 +0530 Subject: [PATCH 10/22] UOE-5616: Support wiid in pubmatic (#77) Co-authored-by: Isha Bharti --- adapters/pubmatic/pubmatic.go | 30 +++++++---- .../pubmatictest/exemplary/simple-banner.json | 14 +++-- exchange/utils.go | 51 +++++++++++++++++++ 3 files changed, 83 insertions(+), 12 deletions(-) diff --git a/adapters/pubmatic/pubmatic.go b/adapters/pubmatic/pubmatic.go index 7c7063e07b1..9dc4a4aaaa2 100644 --- a/adapters/pubmatic/pubmatic.go +++ b/adapters/pubmatic/pubmatic.go @@ -53,6 +53,12 @@ type pubmaticParams struct { Keywords map[string]string `json:"keywords,omitempty"` } +type pubmaticWrapperExt struct { + ProfileID int `json:"profile,omitempty"` + VersionID int `json:"version,omitempty"` + WrapperImpID string `json:"wiid,omitempty"` +} + type pubmaticBidExtVideo struct { Duration *int `json:"duration,omitempty"` } @@ -392,8 +398,9 @@ func (a *PubmaticAdapter) MakeRequests(request *openrtb.BidRequest, reqInfo *ada errs := make([]error, 0, len(request.Imp)) var err error - wrapExt := "" + wrapperExt := new(pubmaticWrapperExt) pubID := "" + wrapperExtSet := false cookies, err := getCookiesFromRequest(request) if err != nil { @@ -401,7 +408,7 @@ func (a *PubmaticAdapter) MakeRequests(request *openrtb.BidRequest, reqInfo *ada } for i := 0; i < len(request.Imp); i++ { - err = parseImpressionObject(&request.Imp[i], &wrapExt, &pubID) + err = parseImpressionObject(&request.Imp[i], wrapperExt, &pubID, &wrapperExtSet) // If the parsing is failed, remove imp and add the error. if err != nil { errs = append(errs, err) @@ -410,8 +417,14 @@ func (a *PubmaticAdapter) MakeRequests(request *openrtb.BidRequest, reqInfo *ada } } - if wrapExt != "" { - rawExt := fmt.Sprintf("{\"wrapper\": %s}", wrapExt) + //if wrapper ext is set, then add it in request.ext + if wrapperExtSet { + //get wrapper impression ID from bidder params + if wbytes, err := getBidderParam(request, "wiid"); err == nil && len(wbytes) != 0 { + wrapperExt.WrapperImpID, _ = strconv.Unquote(string(wbytes)) + } + jsonData, _ := json.Marshal(wrapperExt) + rawExt := fmt.Sprintf("{\"wrapper\": %s}", string(jsonData)) request.Ext = json.RawMessage(rawExt) } @@ -572,7 +585,7 @@ func assignBannerSize(banner *openrtb.Banner) error { } // parseImpressionObject parse the imp to get it ready to send to pubmatic -func parseImpressionObject(imp *openrtb.Imp, wrapExt *string, pubID *string) error { +func parseImpressionObject(imp *openrtb.Imp, wrapExt *pubmaticWrapperExt, pubID *string, wrapperExtSet *bool) error { // PubMatic supports banner and video impressions. if imp.Banner == nil && imp.Video == nil { return fmt.Errorf("Invalid MediaType. PubMatic only supports Banner and Video. Ignoring ImpID=%s", imp.ID) @@ -597,13 +610,12 @@ func parseImpressionObject(imp *openrtb.Imp, wrapExt *string, pubID *string) err } // Parse Wrapper Extension only once per request - if *wrapExt == "" && len(pubmaticExt.WrapExt) != 0 { - var wrapExtMap map[string]int - err := json.Unmarshal([]byte(pubmaticExt.WrapExt), &wrapExtMap) + if !*wrapperExtSet && len(pubmaticExt.WrapExt) != 0 { + err := json.Unmarshal([]byte(pubmaticExt.WrapExt), &wrapExt) if err != nil { return fmt.Errorf("Error in Wrapper Parameters = %v for ImpID = %v WrapperExt = %v", err.Error(), imp.ID, string(pubmaticExt.WrapExt)) } - *wrapExt = string(pubmaticExt.WrapExt) + *wrapperExtSet = true } if err := validateAdSlot(strings.TrimSpace(pubmaticExt.AdSlot), imp); err != nil { diff --git a/adapters/pubmatic/pubmatictest/exemplary/simple-banner.json b/adapters/pubmatic/pubmatictest/exemplary/simple-banner.json index 1eb8a212bff..0cb2739b2d0 100644 --- a/adapters/pubmatic/pubmatictest/exemplary/simple-banner.json +++ b/adapters/pubmatic/pubmatictest/exemplary/simple-banner.json @@ -37,7 +37,14 @@ "publisher": { "id": "1234" } - } + }, + "ext":{ + "prebid" :{ + "bidderparams": { + "wiid": "dwzafakjflan-tygannnvlla-mlljvj" + } + } + } }, "httpCalls": [ @@ -78,7 +85,8 @@ "ext": { "wrapper": { "profile": 5123, - "version":1 + "version":1, + "wiid" : "dwzafakjflan-tygannnvlla-mlljvj" } } } @@ -141,4 +149,4 @@ ] } ] - } \ No newline at end of file + } diff --git a/exchange/utils.go b/exchange/utils.go index ada0edbae05..969471b04c3 100644 --- a/exchange/utils.go +++ b/exchange/utils.go @@ -85,12 +85,50 @@ func cleanOpenRTBRequests(ctx context.Context, return } +func getBidderExts(reqExt *openrtb_ext.ExtRequest) (map[string]map[string]interface{}, error) { + if reqExt == nil { + return nil, nil + } + + if reqExt.Prebid.BidderParams == nil { + return nil, nil + } + + pbytes, err := json.Marshal(reqExt.Prebid.BidderParams) + if err != nil { + return nil, err + } + + var bidderParams map[string]map[string]interface{} + err = json.Unmarshal(pbytes, &bidderParams) + if err != nil { + return nil, err + } + + return bidderParams, nil +} + func splitBidRequest(req *openrtb.BidRequest, impsByBidder map[string][]openrtb.Imp, aliases map[string]string, usersyncs IdFetcher, blabels map[openrtb_ext.BidderName]*pbsmetrics.AdapterLabels, labels pbsmetrics.Labels) (map[openrtb_ext.BidderName]*openrtb.BidRequest, []error) { requestsByBidder := make(map[openrtb_ext.BidderName]*openrtb.BidRequest, len(impsByBidder)) explicitBuyerUIDs, err := extractBuyerUIDs(req.User) if err != nil { return nil, []error{err} } + + var bidderExt map[string]map[string]interface{} + var reqExt openrtb_ext.ExtRequest + if req.Ext != nil { + err = json.Unmarshal(req.Ext, &reqExt) + if err != nil { + return nil, []error{err} + } + + bidderExt, err = getBidderExts(&reqExt) + if err != nil { + return nil, []error{err} + } + } + for bidder, imps := range impsByBidder { reqCopy := *req coreBidder := resolveBidder(bidder, aliases) @@ -110,6 +148,19 @@ func splitBidRequest(req *openrtb.BidRequest, impsByBidder map[string][]openrtb. blabels[coreBidder].CookieFlag = pbsmetrics.CookieFlagYes } reqCopy.Imp = imps + if len(bidderExt) != 0 { + bidderName := openrtb_ext.BidderName(bidder) + if bidderParams, ok := bidderExt[string(bidderName)]; ok { + reqExt.Prebid.BidderParams = bidderParams + } else { + reqExt.Prebid.BidderParams = nil + } + + if reqCopy.Ext, err = json.Marshal(&reqExt); err != nil { + return nil, []error{err} + } + } + requestsByBidder[openrtb_ext.BidderName(bidder)] = &reqCopy } return requestsByBidder, nil From d3c969ee845469df48e7a5dc5a53b6978fdf3d25 Mon Sep 17 00:00:00 2001 From: Viral Vala Date: Wed, 21 Oct 2020 15:28:00 +0530 Subject: [PATCH 11/22] OTT-29 Fixing Skip Dedup Map Issue --- exchange/exchange.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exchange/exchange.go b/exchange/exchange.go index ea3cb36b4fe..5396f5a97bb 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -626,10 +626,10 @@ func applyCategoryMapping(ctx context.Context, requestExt openrtb_ext.ExtRequest continue } } + dedupe[categoryDuration] = bidDedupe{bidderName: bidderName, bidIndex: bidInd, bidID: bidID} } res[bidID] = categoryDuration - dedupe[categoryDuration] = bidDedupe{bidderName: bidderName, bidIndex: bidInd, bidID: bidID} } if len(bidsToRemove) > 0 { From de219fb212a877e66cfb9f20a3b2fa43bc749d65 Mon Sep 17 00:00:00 2001 From: Viral Vala Date: Wed, 21 Oct 2020 15:28:31 +0530 Subject: [PATCH 12/22] OTT-29 Fixing Skip Dedup Map Issue --- exchange/exchange.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exchange/exchange.go b/exchange/exchange.go index ea3cb36b4fe..5396f5a97bb 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -626,10 +626,10 @@ func applyCategoryMapping(ctx context.Context, requestExt openrtb_ext.ExtRequest continue } } + dedupe[categoryDuration] = bidDedupe{bidderName: bidderName, bidIndex: bidInd, bidID: bidID} } res[bidID] = categoryDuration - dedupe[categoryDuration] = bidDedupe{bidderName: bidderName, bidIndex: bidInd, bidID: bidID} } if len(bidsToRemove) > 0 { From 44b526e4920dec942f99d120a6c5fd871b2e2890 Mon Sep 17 00:00:00 2001 From: Viral Vala Date: Mon, 26 Oct 2020 16:02:53 +0530 Subject: [PATCH 13/22] OTT-29 Adding Video Duration in hb_pb_cat_dur key --- exchange/exchange.go | 21 ++++++++++---- exchange/exchange_test.go | 60 +++++++++++++++++++++------------------ 2 files changed, 49 insertions(+), 32 deletions(-) diff --git a/exchange/exchange.go b/exchange/exchange.go index 5396f5a97bb..fb56d91f69d 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -172,7 +172,7 @@ func (e *exchange) HoldAuction(ctx context.Context, bidRequest *openrtb.BidReque if requestExt.Prebid.Targeting != nil && requestExt.Prebid.Targeting.IncludeBrandCategory != nil { var err error var rejections []string - bidCategory, adapterBids, rejections, err = applyCategoryMapping(ctx, requestExt, adapterBids, *categoriesFetcher, targData) + bidCategory, adapterBids, rejections, err = applyCategoryMapping(ctx, bidRequest, requestExt, adapterBids, *categoriesFetcher, targData) if err != nil { return nil, fmt.Errorf("Error in category mapping : %s", err.Error()) } @@ -284,15 +284,15 @@ func validateAndNormalizeDealTier(impDeal *DealTier) bool { func updateHbPbCatDur(bid *pbsOrtbBid, dealTierInfo *DealTierInfo, bidCategory map[string]string) { if bid.dealPriority >= dealTierInfo.MinDealTier { - prefixTier := fmt.Sprintf("%s%d_", dealTierInfo.Prefix, bid.dealPriority) + bid.dealTierSatisfied = true + prefixTier := fmt.Sprintf("%s%d_", dealTierInfo.Prefix, bid.dealPriority) if oldCatDur, ok := bidCategory[bid.bid.ID]; ok { oldCatDurSplit := strings.SplitAfterN(oldCatDur, "_", 2) oldCatDurSplit[0] = prefixTier newCatDur := strings.Join(oldCatDurSplit, "") bidCategory[bid.bid.ID] = newCatDur - bid.dealTierSatisfied = true } } } @@ -493,7 +493,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, []string, error) { +func applyCategoryMapping(ctx context.Context, bidRequest *openrtb.BidRequest, 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 { @@ -503,7 +503,7 @@ func applyCategoryMapping(ctx context.Context, requestExt openrtb_ext.ExtRequest } dedupe := make(map[string]bidDedupe) - + impMap := make(map[string]*openrtb.Imp) brandCatExt := requestExt.Prebid.Targeting.IncludeBrandCategory //If ext.prebid.targeting.includebrandcategory is present in ext then competitive exclusion feature is on. @@ -515,6 +515,11 @@ func applyCategoryMapping(ctx context.Context, requestExt openrtb_ext.ExtRequest var rejections []string var translateCategories = true + //Maintaining BidRequest Impression Map + for i := range bidRequest.Imp { + impMap[bidRequest.Imp[i].ID] = &bidRequest.Imp[i] + } + if includeBrandCategory && brandCatExt.WithCategory { if brandCatExt.TranslateCategories != nil { translateCategories = *brandCatExt.TranslateCategories @@ -591,6 +596,12 @@ func applyCategoryMapping(ctx context.Context, requestExt openrtb_ext.ExtRequest break } } + } else if newDur == 0 { + if imp, ok := impMap[bid.bid.ID]; ok { + if nil != imp.Video && imp.Video.MaxDuration > 0 { + newDur = int(imp.Video.MaxDuration) + } + } } var categoryDuration string diff --git a/exchange/exchange_test.go b/exchange/exchange_test.go index 350438b1be6..ce58b5b0590 100644 --- a/exchange/exchange_test.go +++ b/exchange/exchange_test.go @@ -938,6 +938,7 @@ func TestCategoryMapping(t *testing.T) { t.Errorf("Failed to create a category Fetcher: %v", error) } + bidRequest := &openrtb.BidRequest{} requestExt := newExtRequest() targData := &targetData{ @@ -958,10 +959,10 @@ func TestCategoryMapping(t *testing.T) { bid3 := openrtb.Bid{ID: "bid_id3", ImpID: "imp_id3", Price: 30.0000, Cat: cats3, W: 1, H: 1} bid4 := openrtb.Bid{ID: "bid_id4", ImpID: "imp_id4", Price: 40.0000, Cat: cats4, W: 1, H: 1} - bid1_1 := pbsOrtbBid{&bid1, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0} - bid1_2 := pbsOrtbBid{&bid2, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 40}, 0} - bid1_3 := pbsOrtbBid{&bid3, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30, PrimaryCategory: "AdapterOverride"}, 0} - bid1_4 := pbsOrtbBid{&bid4, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0} + bid1_1 := pbsOrtbBid{&bid1, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0, false} + bid1_2 := pbsOrtbBid{&bid2, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 40}, 0, false} + bid1_3 := pbsOrtbBid{&bid3, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30, PrimaryCategory: "AdapterOverride"}, 0, false} + bid1_4 := pbsOrtbBid{&bid4, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0, false} innerBids := []*pbsOrtbBid{ &bid1_1, @@ -975,7 +976,7 @@ func TestCategoryMapping(t *testing.T) { adapterBids[bidderName1] = &seatBid - bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, requestExt, adapterBids, categoriesFetcher, targData) + bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, bidRequest, 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") @@ -994,6 +995,7 @@ func TestCategoryMappingNoIncludeBrandCategory(t *testing.T) { t.Errorf("Failed to create a category Fetcher: %v", error) } + bidRequest := &openrtb.BidRequest{} requestExt := newExtRequestNoBrandCat() targData := &targetData{ @@ -1013,10 +1015,10 @@ func TestCategoryMappingNoIncludeBrandCategory(t *testing.T) { bid3 := openrtb.Bid{ID: "bid_id3", ImpID: "imp_id3", Price: 30.0000, Cat: cats3, W: 1, H: 1} bid4 := openrtb.Bid{ID: "bid_id4", ImpID: "imp_id4", Price: 40.0000, Cat: cats4, W: 1, H: 1} - bid1_1 := pbsOrtbBid{&bid1, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0} - bid1_2 := pbsOrtbBid{&bid2, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 40}, 0} - bid1_3 := pbsOrtbBid{&bid3, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30, PrimaryCategory: "AdapterOverride"}, 0} - bid1_4 := pbsOrtbBid{&bid4, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 50}, 0} + bid1_1 := pbsOrtbBid{&bid1, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0, false} + bid1_2 := pbsOrtbBid{&bid2, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 40}, 0, false} + bid1_3 := pbsOrtbBid{&bid3, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30, PrimaryCategory: "AdapterOverride"}, 0, false} + bid1_4 := pbsOrtbBid{&bid4, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 50}, 0, false} innerBids := []*pbsOrtbBid{ &bid1_1, @@ -1030,7 +1032,7 @@ func TestCategoryMappingNoIncludeBrandCategory(t *testing.T) { adapterBids[bidderName1] = &seatBid - bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, requestExt, adapterBids, categoriesFetcher, targData) + bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, bidRequest, 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") @@ -1049,6 +1051,7 @@ func TestCategoryMappingTranslateCategoriesNil(t *testing.T) { t.Errorf("Failed to create a category Fetcher: %v", error) } + bidRequest := &openrtb.BidRequest{} requestExt := newExtRequestTranslateCategories(nil) targData := &targetData{ @@ -1067,9 +1070,9 @@ func TestCategoryMappingTranslateCategoriesNil(t *testing.T) { bid2 := openrtb.Bid{ID: "bid_id2", ImpID: "imp_id2", Price: 20.0000, Cat: cats2, W: 1, H: 1} bid3 := openrtb.Bid{ID: "bid_id3", ImpID: "imp_id3", Price: 30.0000, Cat: cats3, W: 1, H: 1} - bid1_1 := pbsOrtbBid{&bid1, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0} - bid1_2 := pbsOrtbBid{&bid2, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 40}, 0} - bid1_3 := pbsOrtbBid{&bid3, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0} + bid1_1 := pbsOrtbBid{&bid1, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0, false} + bid1_2 := pbsOrtbBid{&bid2, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 40}, 0, false} + bid1_3 := pbsOrtbBid{&bid3, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0, false} innerBids := []*pbsOrtbBid{ &bid1_1, @@ -1082,7 +1085,7 @@ func TestCategoryMappingTranslateCategoriesNil(t *testing.T) { adapterBids[bidderName1] = &seatBid - bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, requestExt, adapterBids, categoriesFetcher, targData) + bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, bidRequest, 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") @@ -1131,6 +1134,7 @@ func TestCategoryMappingTranslateCategoriesFalse(t *testing.T) { } translateCategories := false + bidRequest := &openrtb.BidRequest{} requestExt := newExtRequestTranslateCategories(&translateCategories) targData := &targetData{ @@ -1149,9 +1153,9 @@ func TestCategoryMappingTranslateCategoriesFalse(t *testing.T) { bid2 := openrtb.Bid{ID: "bid_id2", ImpID: "imp_id2", Price: 20.0000, Cat: cats2, W: 1, H: 1} bid3 := openrtb.Bid{ID: "bid_id3", ImpID: "imp_id3", Price: 30.0000, Cat: cats3, W: 1, H: 1} - bid1_1 := pbsOrtbBid{&bid1, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0} - bid1_2 := pbsOrtbBid{&bid2, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 40}, 0} - bid1_3 := pbsOrtbBid{&bid3, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0} + bid1_1 := pbsOrtbBid{&bid1, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0, false} + bid1_2 := pbsOrtbBid{&bid2, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 40}, 0, false} + bid1_3 := pbsOrtbBid{&bid3, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0, false} innerBids := []*pbsOrtbBid{ &bid1_1, @@ -1164,7 +1168,7 @@ func TestCategoryMappingTranslateCategoriesFalse(t *testing.T) { adapterBids[bidderName1] = &seatBid - bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, requestExt, adapterBids, categoriesFetcher, targData) + bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, bidRequest, 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") @@ -1182,6 +1186,7 @@ func TestCategoryDedupe(t *testing.T) { t.Errorf("Failed to create a category Fetcher: %v", error) } + bidRequest := &openrtb.BidRequest{} requestExt := newExtRequest() targData := &targetData{ @@ -1200,10 +1205,10 @@ func TestCategoryDedupe(t *testing.T) { bid3 := openrtb.Bid{ID: "bid_id3", ImpID: "imp_id3", Price: 10.0000, Cat: cats1, W: 1, H: 1} bid4 := openrtb.Bid{ID: "bid_id4", ImpID: "imp_id4", Price: 20.0000, Cat: cats4, W: 1, H: 1} - bid1_1 := pbsOrtbBid{&bid1, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0} - bid1_2 := pbsOrtbBid{&bid2, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 50}, 0} - bid1_3 := pbsOrtbBid{&bid3, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0} - bid1_4 := pbsOrtbBid{&bid4, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0} + bid1_1 := pbsOrtbBid{&bid1, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0, false} + bid1_2 := pbsOrtbBid{&bid2, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 50}, 0, false} + bid1_3 := pbsOrtbBid{&bid3, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0, false} + bid1_4 := pbsOrtbBid{&bid4, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0, false} selectedBids := make(map[string]int) expectedCategories := map[string]string{ @@ -1230,7 +1235,7 @@ func TestCategoryDedupe(t *testing.T) { adapterBids[bidderName1] = &seatBid - bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, requestExt, adapterBids, categoriesFetcher, targData) + bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, bidRequest, 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") @@ -1332,16 +1337,17 @@ func TestBidRejectionErrors(t *testing.T) { innerBids := []*pbsOrtbBid{} for _, bid := range test.bids { currentBid := pbsOrtbBid{ - bid, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: test.duration}, 0, + bid, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: test.duration}, 0, false, } innerBids = append(innerBids, ¤tBid) } + bidRequest := &openrtb.BidRequest{} seatBid := pbsOrtbSeatBid{innerBids, "USD", nil, nil} adapterBids[bidderName] = &seatBid - bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, test.reqExt, adapterBids, categoriesFetcher, targData) + bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, bidRequest, test.reqExt, adapterBids, categoriesFetcher, targData) if len(test.expectedCatDur) > 0 { // Bid deduplication case @@ -1432,7 +1438,7 @@ func TestApplyDealSupport(t *testing.T) { }, } - bid := pbsOrtbBid{&openrtb.Bid{ID: "123456"}, "video", map[string]string{}, &openrtb_ext.ExtBidPrebidVideo{}, test.dealPriority} + bid := pbsOrtbBid{&openrtb.Bid{ID: "123456"}, "video", map[string]string{}, &openrtb_ext.ExtBidPrebidVideo{}, test.dealPriority, false} bidCategory := map[string]string{ bid.bid.ID: test.targ["hb_pb_cat_dur"], } @@ -1622,7 +1628,7 @@ func TestUpdateHbPbCatDur(t *testing.T) { } for _, test := range testCases { - bid := pbsOrtbBid{&openrtb.Bid{ID: "123456"}, "video", map[string]string{}, &openrtb_ext.ExtBidPrebidVideo{}, test.dealPriority} + bid := pbsOrtbBid{&openrtb.Bid{ID: "123456"}, "video", map[string]string{}, &openrtb_ext.ExtBidPrebidVideo{}, test.dealPriority, false} bidCategory := map[string]string{ bid.bid.ID: test.targ["hb_pb_cat_dur"], } From ebce2fbdba902c2d2a98963e9a042541654df307 Mon Sep 17 00:00:00 2001 From: Viral Vala Date: Mon, 26 Oct 2020 16:05:42 +0530 Subject: [PATCH 14/22] OTT-29 Adding Video Duration in hb_pb_cat_dur key OTT-29 Fixing Skip Dedup Map Issue --- exchange/exchange.go | 21 ++++++++++---- exchange/exchange_test.go | 60 +++++++++++++++++++++------------------ 2 files changed, 49 insertions(+), 32 deletions(-) diff --git a/exchange/exchange.go b/exchange/exchange.go index 5396f5a97bb..fb56d91f69d 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -172,7 +172,7 @@ func (e *exchange) HoldAuction(ctx context.Context, bidRequest *openrtb.BidReque if requestExt.Prebid.Targeting != nil && requestExt.Prebid.Targeting.IncludeBrandCategory != nil { var err error var rejections []string - bidCategory, adapterBids, rejections, err = applyCategoryMapping(ctx, requestExt, adapterBids, *categoriesFetcher, targData) + bidCategory, adapterBids, rejections, err = applyCategoryMapping(ctx, bidRequest, requestExt, adapterBids, *categoriesFetcher, targData) if err != nil { return nil, fmt.Errorf("Error in category mapping : %s", err.Error()) } @@ -284,15 +284,15 @@ func validateAndNormalizeDealTier(impDeal *DealTier) bool { func updateHbPbCatDur(bid *pbsOrtbBid, dealTierInfo *DealTierInfo, bidCategory map[string]string) { if bid.dealPriority >= dealTierInfo.MinDealTier { - prefixTier := fmt.Sprintf("%s%d_", dealTierInfo.Prefix, bid.dealPriority) + bid.dealTierSatisfied = true + prefixTier := fmt.Sprintf("%s%d_", dealTierInfo.Prefix, bid.dealPriority) if oldCatDur, ok := bidCategory[bid.bid.ID]; ok { oldCatDurSplit := strings.SplitAfterN(oldCatDur, "_", 2) oldCatDurSplit[0] = prefixTier newCatDur := strings.Join(oldCatDurSplit, "") bidCategory[bid.bid.ID] = newCatDur - bid.dealTierSatisfied = true } } } @@ -493,7 +493,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, []string, error) { +func applyCategoryMapping(ctx context.Context, bidRequest *openrtb.BidRequest, 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 { @@ -503,7 +503,7 @@ func applyCategoryMapping(ctx context.Context, requestExt openrtb_ext.ExtRequest } dedupe := make(map[string]bidDedupe) - + impMap := make(map[string]*openrtb.Imp) brandCatExt := requestExt.Prebid.Targeting.IncludeBrandCategory //If ext.prebid.targeting.includebrandcategory is present in ext then competitive exclusion feature is on. @@ -515,6 +515,11 @@ func applyCategoryMapping(ctx context.Context, requestExt openrtb_ext.ExtRequest var rejections []string var translateCategories = true + //Maintaining BidRequest Impression Map + for i := range bidRequest.Imp { + impMap[bidRequest.Imp[i].ID] = &bidRequest.Imp[i] + } + if includeBrandCategory && brandCatExt.WithCategory { if brandCatExt.TranslateCategories != nil { translateCategories = *brandCatExt.TranslateCategories @@ -591,6 +596,12 @@ func applyCategoryMapping(ctx context.Context, requestExt openrtb_ext.ExtRequest break } } + } else if newDur == 0 { + if imp, ok := impMap[bid.bid.ID]; ok { + if nil != imp.Video && imp.Video.MaxDuration > 0 { + newDur = int(imp.Video.MaxDuration) + } + } } var categoryDuration string diff --git a/exchange/exchange_test.go b/exchange/exchange_test.go index 350438b1be6..ce58b5b0590 100644 --- a/exchange/exchange_test.go +++ b/exchange/exchange_test.go @@ -938,6 +938,7 @@ func TestCategoryMapping(t *testing.T) { t.Errorf("Failed to create a category Fetcher: %v", error) } + bidRequest := &openrtb.BidRequest{} requestExt := newExtRequest() targData := &targetData{ @@ -958,10 +959,10 @@ func TestCategoryMapping(t *testing.T) { bid3 := openrtb.Bid{ID: "bid_id3", ImpID: "imp_id3", Price: 30.0000, Cat: cats3, W: 1, H: 1} bid4 := openrtb.Bid{ID: "bid_id4", ImpID: "imp_id4", Price: 40.0000, Cat: cats4, W: 1, H: 1} - bid1_1 := pbsOrtbBid{&bid1, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0} - bid1_2 := pbsOrtbBid{&bid2, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 40}, 0} - bid1_3 := pbsOrtbBid{&bid3, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30, PrimaryCategory: "AdapterOverride"}, 0} - bid1_4 := pbsOrtbBid{&bid4, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0} + bid1_1 := pbsOrtbBid{&bid1, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0, false} + bid1_2 := pbsOrtbBid{&bid2, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 40}, 0, false} + bid1_3 := pbsOrtbBid{&bid3, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30, PrimaryCategory: "AdapterOverride"}, 0, false} + bid1_4 := pbsOrtbBid{&bid4, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0, false} innerBids := []*pbsOrtbBid{ &bid1_1, @@ -975,7 +976,7 @@ func TestCategoryMapping(t *testing.T) { adapterBids[bidderName1] = &seatBid - bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, requestExt, adapterBids, categoriesFetcher, targData) + bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, bidRequest, 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") @@ -994,6 +995,7 @@ func TestCategoryMappingNoIncludeBrandCategory(t *testing.T) { t.Errorf("Failed to create a category Fetcher: %v", error) } + bidRequest := &openrtb.BidRequest{} requestExt := newExtRequestNoBrandCat() targData := &targetData{ @@ -1013,10 +1015,10 @@ func TestCategoryMappingNoIncludeBrandCategory(t *testing.T) { bid3 := openrtb.Bid{ID: "bid_id3", ImpID: "imp_id3", Price: 30.0000, Cat: cats3, W: 1, H: 1} bid4 := openrtb.Bid{ID: "bid_id4", ImpID: "imp_id4", Price: 40.0000, Cat: cats4, W: 1, H: 1} - bid1_1 := pbsOrtbBid{&bid1, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0} - bid1_2 := pbsOrtbBid{&bid2, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 40}, 0} - bid1_3 := pbsOrtbBid{&bid3, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30, PrimaryCategory: "AdapterOverride"}, 0} - bid1_4 := pbsOrtbBid{&bid4, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 50}, 0} + bid1_1 := pbsOrtbBid{&bid1, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0, false} + bid1_2 := pbsOrtbBid{&bid2, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 40}, 0, false} + bid1_3 := pbsOrtbBid{&bid3, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30, PrimaryCategory: "AdapterOverride"}, 0, false} + bid1_4 := pbsOrtbBid{&bid4, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 50}, 0, false} innerBids := []*pbsOrtbBid{ &bid1_1, @@ -1030,7 +1032,7 @@ func TestCategoryMappingNoIncludeBrandCategory(t *testing.T) { adapterBids[bidderName1] = &seatBid - bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, requestExt, adapterBids, categoriesFetcher, targData) + bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, bidRequest, 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") @@ -1049,6 +1051,7 @@ func TestCategoryMappingTranslateCategoriesNil(t *testing.T) { t.Errorf("Failed to create a category Fetcher: %v", error) } + bidRequest := &openrtb.BidRequest{} requestExt := newExtRequestTranslateCategories(nil) targData := &targetData{ @@ -1067,9 +1070,9 @@ func TestCategoryMappingTranslateCategoriesNil(t *testing.T) { bid2 := openrtb.Bid{ID: "bid_id2", ImpID: "imp_id2", Price: 20.0000, Cat: cats2, W: 1, H: 1} bid3 := openrtb.Bid{ID: "bid_id3", ImpID: "imp_id3", Price: 30.0000, Cat: cats3, W: 1, H: 1} - bid1_1 := pbsOrtbBid{&bid1, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0} - bid1_2 := pbsOrtbBid{&bid2, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 40}, 0} - bid1_3 := pbsOrtbBid{&bid3, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0} + bid1_1 := pbsOrtbBid{&bid1, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0, false} + bid1_2 := pbsOrtbBid{&bid2, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 40}, 0, false} + bid1_3 := pbsOrtbBid{&bid3, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0, false} innerBids := []*pbsOrtbBid{ &bid1_1, @@ -1082,7 +1085,7 @@ func TestCategoryMappingTranslateCategoriesNil(t *testing.T) { adapterBids[bidderName1] = &seatBid - bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, requestExt, adapterBids, categoriesFetcher, targData) + bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, bidRequest, 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") @@ -1131,6 +1134,7 @@ func TestCategoryMappingTranslateCategoriesFalse(t *testing.T) { } translateCategories := false + bidRequest := &openrtb.BidRequest{} requestExt := newExtRequestTranslateCategories(&translateCategories) targData := &targetData{ @@ -1149,9 +1153,9 @@ func TestCategoryMappingTranslateCategoriesFalse(t *testing.T) { bid2 := openrtb.Bid{ID: "bid_id2", ImpID: "imp_id2", Price: 20.0000, Cat: cats2, W: 1, H: 1} bid3 := openrtb.Bid{ID: "bid_id3", ImpID: "imp_id3", Price: 30.0000, Cat: cats3, W: 1, H: 1} - bid1_1 := pbsOrtbBid{&bid1, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0} - bid1_2 := pbsOrtbBid{&bid2, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 40}, 0} - bid1_3 := pbsOrtbBid{&bid3, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0} + bid1_1 := pbsOrtbBid{&bid1, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0, false} + bid1_2 := pbsOrtbBid{&bid2, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 40}, 0, false} + bid1_3 := pbsOrtbBid{&bid3, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0, false} innerBids := []*pbsOrtbBid{ &bid1_1, @@ -1164,7 +1168,7 @@ func TestCategoryMappingTranslateCategoriesFalse(t *testing.T) { adapterBids[bidderName1] = &seatBid - bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, requestExt, adapterBids, categoriesFetcher, targData) + bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, bidRequest, 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") @@ -1182,6 +1186,7 @@ func TestCategoryDedupe(t *testing.T) { t.Errorf("Failed to create a category Fetcher: %v", error) } + bidRequest := &openrtb.BidRequest{} requestExt := newExtRequest() targData := &targetData{ @@ -1200,10 +1205,10 @@ func TestCategoryDedupe(t *testing.T) { bid3 := openrtb.Bid{ID: "bid_id3", ImpID: "imp_id3", Price: 10.0000, Cat: cats1, W: 1, H: 1} bid4 := openrtb.Bid{ID: "bid_id4", ImpID: "imp_id4", Price: 20.0000, Cat: cats4, W: 1, H: 1} - bid1_1 := pbsOrtbBid{&bid1, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0} - bid1_2 := pbsOrtbBid{&bid2, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 50}, 0} - bid1_3 := pbsOrtbBid{&bid3, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0} - bid1_4 := pbsOrtbBid{&bid4, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0} + bid1_1 := pbsOrtbBid{&bid1, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0, false} + bid1_2 := pbsOrtbBid{&bid2, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 50}, 0, false} + bid1_3 := pbsOrtbBid{&bid3, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0, false} + bid1_4 := pbsOrtbBid{&bid4, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, 0, false} selectedBids := make(map[string]int) expectedCategories := map[string]string{ @@ -1230,7 +1235,7 @@ func TestCategoryDedupe(t *testing.T) { adapterBids[bidderName1] = &seatBid - bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, requestExt, adapterBids, categoriesFetcher, targData) + bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, bidRequest, 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") @@ -1332,16 +1337,17 @@ func TestBidRejectionErrors(t *testing.T) { innerBids := []*pbsOrtbBid{} for _, bid := range test.bids { currentBid := pbsOrtbBid{ - bid, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: test.duration}, 0, + bid, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: test.duration}, 0, false, } innerBids = append(innerBids, ¤tBid) } + bidRequest := &openrtb.BidRequest{} seatBid := pbsOrtbSeatBid{innerBids, "USD", nil, nil} adapterBids[bidderName] = &seatBid - bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, test.reqExt, adapterBids, categoriesFetcher, targData) + bidCategory, adapterBids, rejections, err := applyCategoryMapping(nil, bidRequest, test.reqExt, adapterBids, categoriesFetcher, targData) if len(test.expectedCatDur) > 0 { // Bid deduplication case @@ -1432,7 +1438,7 @@ func TestApplyDealSupport(t *testing.T) { }, } - bid := pbsOrtbBid{&openrtb.Bid{ID: "123456"}, "video", map[string]string{}, &openrtb_ext.ExtBidPrebidVideo{}, test.dealPriority} + bid := pbsOrtbBid{&openrtb.Bid{ID: "123456"}, "video", map[string]string{}, &openrtb_ext.ExtBidPrebidVideo{}, test.dealPriority, false} bidCategory := map[string]string{ bid.bid.ID: test.targ["hb_pb_cat_dur"], } @@ -1622,7 +1628,7 @@ func TestUpdateHbPbCatDur(t *testing.T) { } for _, test := range testCases { - bid := pbsOrtbBid{&openrtb.Bid{ID: "123456"}, "video", map[string]string{}, &openrtb_ext.ExtBidPrebidVideo{}, test.dealPriority} + bid := pbsOrtbBid{&openrtb.Bid{ID: "123456"}, "video", map[string]string{}, &openrtb_ext.ExtBidPrebidVideo{}, test.dealPriority, false} bidCategory := map[string]string{ bid.bid.ID: test.targ["hb_pb_cat_dur"], } From 5f32d6d5040db8c655ef93d0a15cf8b5e8356af2 Mon Sep 17 00:00:00 2001 From: shalmali-patil Date: Wed, 28 Oct 2020 11:34:29 +0530 Subject: [PATCH 15/22] UOE-5741: adding omitempty for ExtImpPrebid fields --- openrtb_ext/imp.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/openrtb_ext/imp.go b/openrtb_ext/imp.go index d8ebecb3dc6..b84af25212c 100644 --- a/openrtb_ext/imp.go +++ b/openrtb_ext/imp.go @@ -18,18 +18,18 @@ type ExtImp struct { // ExtImpPrebid defines the contract for bidrequest.imp[i].ext.prebid type ExtImpPrebid struct { - StoredRequest *ExtStoredRequest `json:"storedrequest"` + StoredRequest *ExtStoredRequest `json:"storedrequest,omitempty"` // Rewarded inventory signal, can be 0 or 1 - IsRewardedInventory int8 `json:"is_rewarded_inventory"` + IsRewardedInventory int8 `json:"is_rewarded_inventory,omitempty"` // NOTE: This is not part of the official API, we are not expecting clients // migrate from imp[...].ext.${BIDDER} to imp[...].ext.prebid.bidder.${BIDDER} // at this time // https://github.com/PubMatic-OpenWrap/prebid-server/pull/846#issuecomment-476352224 - Bidder map[string]json.RawMessage `json:"bidder"` + Bidder map[string]json.RawMessage `json:"bidder,omitempty"` - SKAdnetwork json.RawMessage `json:"skadn"` + SKAdnetwork json.RawMessage `json:"skadn,omitempty"` } // ExtStoredRequest defines the contract for bidrequest.imp[i].ext.prebid.storedrequest From 76af1f0f5e2386f4021f89e306005acbad9f1382 Mon Sep 17 00:00:00 2001 From: shalmali-patil Date: Wed, 28 Oct 2020 11:43:10 +0530 Subject: [PATCH 16/22] UOE-5741: adding omitempty for ExtImpPrebid fields --- openrtb_ext/imp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openrtb_ext/imp.go b/openrtb_ext/imp.go index b84af25212c..0d5e1f655cb 100644 --- a/openrtb_ext/imp.go +++ b/openrtb_ext/imp.go @@ -6,7 +6,7 @@ import ( // ExtImp defines the contract for bidrequest.imp[i].ext type ExtImp struct { - Prebid *ExtImpPrebid `json:"prebid"` + Prebid *ExtImpPrebid `json:"prebid,omitempty"` Appnexus *ExtImpAppnexus `json:"appnexus"` Consumable *ExtImpConsumable `json:"consumable"` Rubicon *ExtImpRubicon `json:"rubicon"` From 2e3e1a70acb507bfc362327ee0b8bd6f4ca431eb Mon Sep 17 00:00:00 2001 From: Viral Vala Date: Wed, 28 Oct 2020 13:19:40 +0530 Subject: [PATCH 17/22] OTT-9 Adding Duration in hb_pb_cat_dur field --- exchange/exchange.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exchange/exchange.go b/exchange/exchange.go index fb56d91f69d..4beb238fe67 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -597,7 +597,7 @@ func applyCategoryMapping(ctx context.Context, bidRequest *openrtb.BidRequest, r } } } else if newDur == 0 { - if imp, ok := impMap[bid.bid.ID]; ok { + if imp, ok := impMap[bid.bid.ImpID]; ok { if nil != imp.Video && imp.Video.MaxDuration > 0 { newDur = int(imp.Video.MaxDuration) } From 344c96559a1830fe059e804e68f822acc3d56411 Mon Sep 17 00:00:00 2001 From: Viral Vala Date: Wed, 28 Oct 2020 13:22:54 +0530 Subject: [PATCH 18/22] OTT-9 Adding Duration in hb_pb_cat_dur field --- exchange/exchange.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exchange/exchange.go b/exchange/exchange.go index fb56d91f69d..4beb238fe67 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -597,7 +597,7 @@ func applyCategoryMapping(ctx context.Context, bidRequest *openrtb.BidRequest, r } } } else if newDur == 0 { - if imp, ok := impMap[bid.bid.ID]; ok { + if imp, ok := impMap[bid.bid.ImpID]; ok { if nil != imp.Video && imp.Video.MaxDuration > 0 { newDur = int(imp.Video.MaxDuration) } From 4f1e44a34a2f24ebb49f993dce30a6d6348d00da Mon Sep 17 00:00:00 2001 From: ShriprasadM Date: Tue, 3 Nov 2020 08:33:42 +0530 Subject: [PATCH 19/22] OTT-45: Added logger and Prometheus metrics to capture bid.id collisions (#84) * OTT-45: Added logger and Prometheus metrics to capture bid.id collisions Co-authored-by: Shriprasad --- exchange/exchange.go | 30 +++++++++- exchange/exchange_test.go | 44 +++++++++++++++ pbsmetrics/config/metrics.go | 22 ++++++++ pbsmetrics/go_metrics.go | 8 +++ pbsmetrics/metrics.go | 7 +++ pbsmetrics/metrics_mock.go | 10 ++++ pbsmetrics/prometheus/prometheus.go | 71 ++++++++++++++++-------- pbsmetrics/prometheus/prometheus_test.go | 42 ++++++++++++++ router/router.go | 10 ++++ 9 files changed, 221 insertions(+), 23 deletions(-) diff --git a/exchange/exchange.go b/exchange/exchange.go index 4beb238fe67..bc53de5ab5e 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -315,6 +315,7 @@ func (e *exchange) getAllBids(ctx context.Context, cleanRequests map[openrtb_ext adapterExtra := make(map[openrtb_ext.BidderName]*seatResponseExtra, len(cleanRequests)) chBids := make(chan *bidResponseWrapper, len(cleanRequests)) bidsFound := false + bidIDsCollision := false for bidderName, req := range cleanRequests { // Here we actually call the adapters and collect the bids. @@ -383,9 +384,13 @@ func (e *exchange) getAllBids(ctx context.Context, cleanRequests map[openrtb_ext if !bidsFound && adapterBids[brw.bidder] != nil && len(adapterBids[brw.bidder].bids) > 0 { bidsFound = true + bidIDsCollision = recordAdaptorDuplicateBidIDs(e.me, adapterBids) } } - + if bidIDsCollision { + // record this request count this request if bid collision is detected + e.me.RecordRequestHavingDuplicateBidID() + } return adapterBids, adapterExtra, bidsFound } @@ -823,3 +828,26 @@ func listBiddersWithRequests(cleanRequests map[openrtb_ext.BidderName]*openrtb.B return liveAdapters } + +// recordAdaptorDuplicateBidIDs finds the bid.id collisions for each bidder and records them with metrics engine +// it returns true if collosion(s) is/are detected in any of the bidder's bids +func recordAdaptorDuplicateBidIDs(metricsEngine pbsmetrics.MetricsEngine, adapterBids map[openrtb_ext.BidderName]*pbsOrtbSeatBid) bool { + bidIDCollisionFound := false + if nil == adapterBids { + return false + } + for bidder, bid := range adapterBids { + bidIDColisionMap := make(map[string]int, len(adapterBids[bidder].bids)) + for _, thisBid := range bid.bids { + if collisions, ok := bidIDColisionMap[thisBid.bid.ID]; ok { + bidIDCollisionFound = true + bidIDColisionMap[thisBid.bid.ID]++ + glog.Warningf("Bid.id %v :: %v collision(s) [imp.id = %v] for bidder '%v'", thisBid.bid.ID, collisions, thisBid.bid.ImpID, string(bidder)) + metricsEngine.RecordAdapterDuplicateBidID(string(bidder), 1) + } else { + bidIDColisionMap[thisBid.bid.ID] = 1 + } + } + } + return bidIDCollisionFound +} diff --git a/exchange/exchange_test.go b/exchange/exchange_test.go index ce58b5b0590..aa48b9b71cc 100644 --- a/exchange/exchange_test.go +++ b/exchange/exchange_test.go @@ -1639,6 +1639,50 @@ func TestUpdateHbPbCatDur(t *testing.T) { } } +func TestRecordAdaptorDuplicateBidIDs(t *testing.T) { + type bidderCollisions = map[string]int + testCases := []struct { + scenario string + bidderCollisions *bidderCollisions // represents no of collisions detected for bid.id at bidder level for given request + hasCollision bool + }{ + {scenario: "invalid collision value", bidderCollisions: &map[string]int{"bidder-1": -1}, hasCollision: false}, + {scenario: "no collision", bidderCollisions: &map[string]int{"bidder-1": 0}, hasCollision: false}, + {scenario: "one collision", bidderCollisions: &map[string]int{"bidder-1": 1}, hasCollision: false}, + {scenario: "multiple collisions", bidderCollisions: &map[string]int{"bidder-1": 2}, hasCollision: true}, // when 2 collisions it counter will be 1 + {scenario: "multiple bidders", bidderCollisions: &map[string]int{"bidder-1": 2, "bidder-2": 4}, hasCollision: true}, + {scenario: "multiple bidders with bidder-1 no collision", bidderCollisions: &map[string]int{"bidder-1": 1, "bidder-2": 4}, hasCollision: true}, + {scenario: "no bidders", bidderCollisions: nil, hasCollision: false}, + } + testEngine := metricsConf.NewMetricsEngine(&config.Configuration{}, nil) + + for _, testcase := range testCases { + var adapterBids map[openrtb_ext.BidderName]*pbsOrtbSeatBid + if nil == testcase.bidderCollisions { + break + } + adapterBids = make(map[openrtb_ext.BidderName]*pbsOrtbSeatBid) + for bidder, collisions := range *testcase.bidderCollisions { + bids := make([]*pbsOrtbBid, 0) + testBidID := "bid_id_for_bidder_" + bidder + // add bids as per collisions value + bidCount := 0 + for ; bidCount < collisions; bidCount++ { + bids = append(bids, &pbsOrtbBid{ + bid: &openrtb.Bid{ + ID: testBidID, + }, + }) + } + if nil == adapterBids[openrtb_ext.BidderName(bidder)] { + adapterBids[openrtb_ext.BidderName(bidder)] = new(pbsOrtbSeatBid) + } + adapterBids[openrtb_ext.BidderName(bidder)].bids = bids + } + assert.Equal(t, testcase.hasCollision, recordAdaptorDuplicateBidIDs(testEngine, adapterBids)) + } +} + type exchangeSpec struct { IncomingRequest exchangeRequest `json:"incomingRequest"` OutgoingRequests map[string]*bidderSpec `json:"outgoingRequests"` diff --git a/pbsmetrics/config/metrics.go b/pbsmetrics/config/metrics.go index ce6c0f5a707..bc237313652 100644 --- a/pbsmetrics/config/metrics.go +++ b/pbsmetrics/config/metrics.go @@ -195,6 +195,20 @@ func (me *MultiMetricsEngine) RecordTimeoutNotice(success bool) { } } +// RecordAdapterDuplicateBidID across all engines +func (me *MultiMetricsEngine) RecordAdapterDuplicateBidID(adaptor string, collisions int) { + for _, thisME := range *me { + thisME.RecordAdapterDuplicateBidID(adaptor, collisions) + } +} + +// RecordRequestHavingDuplicateBidID across all engines +func (me *MultiMetricsEngine) RecordRequestHavingDuplicateBidID() { + for _, thisME := range *me { + thisME.RecordRequestHavingDuplicateBidID() + } +} + // DummyMetricsEngine is a Noop metrics engine in case no metrics are configured. (may also be useful for tests) type DummyMetricsEngine struct{} @@ -273,3 +287,11 @@ func (me *DummyMetricsEngine) RecordRequestQueueTime(success bool, requestType p // RecordTimeoutNotice as a noop func (me *DummyMetricsEngine) RecordTimeoutNotice(success bool) { } + +// RecordAdapterDuplicateBidID as a noop +func (me *DummyMetricsEngine) RecordAdapterDuplicateBidID(adaptor string, collisions int) { +} + +// RecordRequestHavingDuplicateBidID as a noop +func (me *DummyMetricsEngine) RecordRequestHavingDuplicateBidID() { +} diff --git a/pbsmetrics/go_metrics.go b/pbsmetrics/go_metrics.go index cf634cc5ae1..607b792be9c 100644 --- a/pbsmetrics/go_metrics.go +++ b/pbsmetrics/go_metrics.go @@ -562,6 +562,14 @@ func (me *Metrics) RecordTimeoutNotice(success bool) { return } +// RecordAdapterDuplicateBidID as noop +func (me *Metrics) RecordAdapterDuplicateBidID(adaptor string, collisions int) { +} + +// RecordRequestHavingDuplicateBidID as noop +func (me *Metrics) RecordRequestHavingDuplicateBidID() { +} + func doMark(bidder openrtb_ext.BidderName, meters map[openrtb_ext.BidderName]metrics.Meter) { met, ok := meters[bidder] if ok { diff --git a/pbsmetrics/metrics.go b/pbsmetrics/metrics.go index 770f5750335..376334e205e 100644 --- a/pbsmetrics/metrics.go +++ b/pbsmetrics/metrics.go @@ -276,4 +276,11 @@ type MetricsEngine interface { RecordPrebidCacheRequestTime(success bool, length time.Duration) RecordRequestQueueTime(success bool, requestType RequestType, length time.Duration) RecordTimeoutNotice(sucess bool) + // RecordAdapterDuplicateBidID captures the bid.ID collisions when adaptor + // gives the bid response with multiple bids containing same bid.ID + RecordAdapterDuplicateBidID(adaptor string, collisions int) + + // RecordRequestHavingDuplicateBidID keeps track off how many request got bid.id collision + // detected + RecordRequestHavingDuplicateBidID() } diff --git a/pbsmetrics/metrics_mock.go b/pbsmetrics/metrics_mock.go index d5661f4bfe4..33ce3c83cc2 100644 --- a/pbsmetrics/metrics_mock.go +++ b/pbsmetrics/metrics_mock.go @@ -106,3 +106,13 @@ func (me *MetricsEngineMock) RecordRequestQueueTime(success bool, requestType Re func (me *MetricsEngineMock) RecordTimeoutNotice(success bool) { me.Called(success) } + +// RecordAdapterDuplicateBidID mock +func (me *MetricsEngineMock) RecordAdapterDuplicateBidID(adaptor string, collisions int) { + me.Called(adaptor, collisions) +} + +// RecordRequestHavingDuplicateBidID mock +func (me *MetricsEngineMock) RecordRequestHavingDuplicateBidID() { + me.Called() +} diff --git a/pbsmetrics/prometheus/prometheus.go b/pbsmetrics/prometheus/prometheus.go index 9c06d6032f4..fa6fb67ba99 100644 --- a/pbsmetrics/prometheus/prometheus.go +++ b/pbsmetrics/prometheus/prometheus.go @@ -15,30 +15,32 @@ type Metrics struct { Registry *prometheus.Registry // General Metrics - connectionsClosed prometheus.Counter - connectionsError *prometheus.CounterVec - connectionsOpened prometheus.Counter - cookieSync prometheus.Counter - impressions *prometheus.CounterVec - impressionsLegacy prometheus.Counter - prebidCacheWriteTimer *prometheus.HistogramVec - requests *prometheus.CounterVec - requestsTimer *prometheus.HistogramVec - requestsQueueTimer *prometheus.HistogramVec - requestsWithoutCookie *prometheus.CounterVec - storedImpressionsCacheResult *prometheus.CounterVec - storedRequestCacheResult *prometheus.CounterVec - timeout_notifications *prometheus.CounterVec + connectionsClosed prometheus.Counter + connectionsError *prometheus.CounterVec + connectionsOpened prometheus.Counter + cookieSync prometheus.Counter + impressions *prometheus.CounterVec + impressionsLegacy prometheus.Counter + prebidCacheWriteTimer *prometheus.HistogramVec + requests *prometheus.CounterVec + requestsTimer *prometheus.HistogramVec + requestsQueueTimer *prometheus.HistogramVec + requestsWithoutCookie *prometheus.CounterVec + storedImpressionsCacheResult *prometheus.CounterVec + storedRequestCacheResult *prometheus.CounterVec + timeout_notifications *prometheus.CounterVec + requestsDuplicateBidIDCounter prometheus.Counter // sum of total bid collisions at bidder level for given request // Adapter Metrics - adapterBids *prometheus.CounterVec - adapterCookieSync *prometheus.CounterVec - adapterErrors *prometheus.CounterVec - adapterPanics *prometheus.CounterVec - adapterPrices *prometheus.HistogramVec - adapterRequests *prometheus.CounterVec - adapterRequestsTimer *prometheus.HistogramVec - adapterUserSync *prometheus.CounterVec + adapterBids *prometheus.CounterVec + adapterCookieSync *prometheus.CounterVec + adapterErrors *prometheus.CounterVec + adapterPanics *prometheus.CounterVec + adapterPrices *prometheus.HistogramVec + adapterRequests *prometheus.CounterVec + adapterRequestsTimer *prometheus.HistogramVec + adapterUserSync *prometheus.CounterVec + adapterDuplicateBidIDCounter *prometheus.CounterVec // Account Metrics accountRequests *prometheus.CounterVec @@ -211,6 +213,15 @@ func NewMetrics(cfg config.PrometheusMetrics) *Metrics { []string{requestTypeLabel, requestStatusLabel}, queuedRequestTimeBuckets) + metrics.adapterDuplicateBidIDCounter = newCounter(cfg, metrics.Registry, + "duplicate_bid_ids", + "Number of collisions observed for given adaptor", + []string{adapterLabel}) + + metrics.requestsDuplicateBidIDCounter = newCounterWithoutLabels(cfg, metrics.Registry, + "requests_having_duplicate_bid_ids", + "Count of number of request where bid collision is detected.") + preloadLabelValues(&metrics) return &metrics @@ -421,3 +432,19 @@ func (m *Metrics) RecordTimeoutNotice(success bool) { }).Inc() } } + +// RecordAdapterDuplicateBidID captures the bid.ID collisions when adaptor +// gives the bid response with multiple bids containing same bid.ID +// ensure collisions value is greater than 1. This function will not give any error +// if collisions = 1 is passed +func (m *Metrics) RecordAdapterDuplicateBidID(adaptor string, collisions int) { + m.adapterDuplicateBidIDCounter.With(prometheus.Labels{ + adapterLabel: adaptor, + }).Add(float64(collisions)) +} + +// RecordRequestHavingDuplicateBidID keeps count of request when duplicate bid.id is +// detected in partner's response +func (m *Metrics) RecordRequestHavingDuplicateBidID() { + m.requestsDuplicateBidIDCounter.Inc() +} diff --git a/pbsmetrics/prometheus/prometheus_test.go b/pbsmetrics/prometheus/prometheus_test.go index 21f182e2094..38588ac3b89 100644 --- a/pbsmetrics/prometheus/prometheus_test.go +++ b/pbsmetrics/prometheus/prometheus_test.go @@ -944,6 +944,48 @@ func TestTimeoutNotifications(t *testing.T) { } +// TestRecordRequestDuplicateBidID checks RecordRequestDuplicateBidID +func TestRecordRequestDuplicateBidID(t *testing.T) { + m := createMetricsForTesting() + m.RecordRequestHavingDuplicateBidID() + // verify total no of requests which detected collision + assertCounterValue(t, "request cnt having duplicate bid.id", "request cnt having duplicate bid.id", m.requestsDuplicateBidIDCounter, float64(1)) +} + +// TestRecordAdapterDuplicateBidID checks RecordAdapterDuplicateBidID +func TestRecordAdapterDuplicateBidID(t *testing.T) { + type collisions struct { + simulate int // no of bids to be simulate with same bid.id + expect int // no of collisions expected to be recorded by metrics engine for given bidder + } + type bidderCollisions = map[string]collisions + testCases := []struct { + scenario string + bidderCollisions bidderCollisions // represents no of collisions detected for bid.id at bidder level for given request + expectCollisions int + }{ + {scenario: "invalid collision value", bidderCollisions: map[string]collisions{"bidder-1": {simulate: -1, expect: 0}}}, + {scenario: "no collision", bidderCollisions: map[string]collisions{"bidder-1": {simulate: 0, expect: 0}}}, + {scenario: "one collision", bidderCollisions: map[string]collisions{"bidder-1": {simulate: 1, expect: 1}}}, + {scenario: "multiple collisions", bidderCollisions: map[string]collisions{"bidder-1": {simulate: 2, expect: 2}}}, + {scenario: "multiple bidders", bidderCollisions: map[string]collisions{"bidder-1": {simulate: 2, expect: 2}, "bidder-2": {simulate: 4, expect: 4}}}, + {scenario: "multiple bidders with bidder-1 no collision", bidderCollisions: map[string]collisions{"bidder-1": {simulate: 0, expect: 0}, + "bidder-2": {simulate: 4, expect: 4}}}, + } + + for _, testcase := range testCases { + m := createMetricsForTesting() + for bidder, collisions := range testcase.bidderCollisions { + for collision := 1; collision <= collisions.simulate; collision++ { + m.RecordAdapterDuplicateBidID(bidder, 1) + } + assertCounterVecValue(t, testcase.scenario, testcase.scenario, m.adapterDuplicateBidIDCounter, float64(collisions.expect), prometheus.Labels{ + adapterLabel: bidder, + }) + } + } +} + func assertCounterValue(t *testing.T, description, name string, counter prometheus.Counter, expected float64) { m := dto.Metric{} counter.Write(&m) diff --git a/router/router.go b/router/router.go index 6da9800ba43..4d71c244095 100644 --- a/router/router.go +++ b/router/router.go @@ -13,6 +13,7 @@ import ( "time" "github.com/PubMatic-OpenWrap/prebid-server/pbsmetrics" + "github.com/prometheus/client_golang/prometheus" "github.com/PubMatic-OpenWrap/prebid-server/adapters" "github.com/PubMatic-OpenWrap/prebid-server/adapters/adform" @@ -422,3 +423,12 @@ func readDefaultRequest(defReqConfig config.DefReqConfig) (map[string]string, [] } return aliases, []byte{} } + +func GetPrometheusRegistry() *prometheus.Registry { + mEngine, ok := g_metrics.(*metricsConf.DetailedMetricsEngine) + if !ok || mEngine == nil || mEngine.PrometheusMetrics == nil { + return nil + } + + return mEngine.PrometheusMetrics.Registry +} From 5eb4968cd45fd7f1621f5cdd0a75ee08d412699d Mon Sep 17 00:00:00 2001 From: Shriprasad Date: Tue, 3 Nov 2020 09:24:33 +0530 Subject: [PATCH 20/22] OTT-9 Removed duplicate import --- router/router.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/router/router.go b/router/router.go index 8175dc7908a..755c18a452b 100644 --- a/router/router.go +++ b/router/router.go @@ -6,16 +6,16 @@ import ( "database/sql" "encoding/json" "fmt" - "github.com/prometheus/client_golang/prometheus" "io/ioutil" "net/http" "path/filepath" "strings" "time" - "github.com/PubMatic-OpenWrap/prebid-server/pbsmetrics" "github.com/prometheus/client_golang/prometheus" + "github.com/PubMatic-OpenWrap/prebid-server/pbsmetrics" + "github.com/PubMatic-OpenWrap/prebid-server/adapters" "github.com/PubMatic-OpenWrap/prebid-server/adapters/adform" "github.com/PubMatic-OpenWrap/prebid-server/adapters/appnexus" From a1dfea5f0dc584c02bf5fcd0110c6cd0a619095d Mon Sep 17 00:00:00 2001 From: Shriprasad Date: Tue, 3 Nov 2020 09:28:11 +0530 Subject: [PATCH 21/22] OTT-45: corrected comment --- pbsmetrics/prometheus/prometheus.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pbsmetrics/prometheus/prometheus.go b/pbsmetrics/prometheus/prometheus.go index 05aa8ca2673..c328cdb2d37 100644 --- a/pbsmetrics/prometheus/prometheus.go +++ b/pbsmetrics/prometheus/prometheus.go @@ -29,7 +29,7 @@ type Metrics struct { storedImpressionsCacheResult *prometheus.CounterVec storedRequestCacheResult *prometheus.CounterVec timeout_notifications *prometheus.CounterVec - requestsDuplicateBidIDCounter prometheus.Counter // sum of total bid collisions at bidder level for given request + requestsDuplicateBidIDCounter prometheus.Counter // total request having duplicate bid.id for given bidder // Adapter Metrics adapterBids *prometheus.CounterVec From fe90a71a5d34a1d9c618900a00934e86f28a067e Mon Sep 17 00:00:00 2001 From: Shriprasad Date: Wed, 4 Nov 2020 13:30:59 +0530 Subject: [PATCH 22/22] OTT-9: Reverted with master changes. This changes are not required for OTT-9 --- endpoints/openrtb2/ctv/constant/constant.go | 2 +- openrtb_ext/imp.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/endpoints/openrtb2/ctv/constant/constant.go b/endpoints/openrtb2/ctv/constant/constant.go index 997ea87d9b6..50dcc447805 100644 --- a/endpoints/openrtb2/ctv/constant/constant.go +++ b/endpoints/openrtb2/ctv/constant/constant.go @@ -45,7 +45,7 @@ type MonitorKey string const ( // CombinationGeneratorV1 ... - CombinationGeneratorV1 MonitorKey = "comp_exclusion_v1" + CombinationGeneratorV1 MonitorKey = "comb_gen_v1" // CompetitiveExclusionV1 ... CompetitiveExclusionV1 MonitorKey = "comp_exclusion_v1" ) diff --git a/openrtb_ext/imp.go b/openrtb_ext/imp.go index 0d5e1f655cb..d8ebecb3dc6 100644 --- a/openrtb_ext/imp.go +++ b/openrtb_ext/imp.go @@ -6,7 +6,7 @@ import ( // ExtImp defines the contract for bidrequest.imp[i].ext type ExtImp struct { - Prebid *ExtImpPrebid `json:"prebid,omitempty"` + Prebid *ExtImpPrebid `json:"prebid"` Appnexus *ExtImpAppnexus `json:"appnexus"` Consumable *ExtImpConsumable `json:"consumable"` Rubicon *ExtImpRubicon `json:"rubicon"` @@ -18,18 +18,18 @@ type ExtImp struct { // ExtImpPrebid defines the contract for bidrequest.imp[i].ext.prebid type ExtImpPrebid struct { - StoredRequest *ExtStoredRequest `json:"storedrequest,omitempty"` + StoredRequest *ExtStoredRequest `json:"storedrequest"` // Rewarded inventory signal, can be 0 or 1 - IsRewardedInventory int8 `json:"is_rewarded_inventory,omitempty"` + IsRewardedInventory int8 `json:"is_rewarded_inventory"` // NOTE: This is not part of the official API, we are not expecting clients // migrate from imp[...].ext.${BIDDER} to imp[...].ext.prebid.bidder.${BIDDER} // at this time // https://github.com/PubMatic-OpenWrap/prebid-server/pull/846#issuecomment-476352224 - Bidder map[string]json.RawMessage `json:"bidder,omitempty"` + Bidder map[string]json.RawMessage `json:"bidder"` - SKAdnetwork json.RawMessage `json:"skadn,omitempty"` + SKAdnetwork json.RawMessage `json:"skadn"` } // ExtStoredRequest defines the contract for bidrequest.imp[i].ext.prebid.storedrequest