From 9e23a9be391081103fb64ce466ccb0cc1d6bcbbe Mon Sep 17 00:00:00 2001 From: ShriprasadM Date: Fri, 18 Dec 2020 09:28:57 +0530 Subject: [PATCH] OTT-71: (CI) Add stats collecting information around the actual Ad duration (#93) * OTT-71: added new metrics adapter_vidbid_dur * OTT-71: Added new metric * OTT-71: Reverted with master changes * OTT-71: Added missing method in mock. Refatored testcase data structure * OTT-71: Corrected unit tests and addressed code review comments * OTT-71: Removed unwanted method and comment * OTT-71: Corrected the comment Co-authored-by: Shriprasad --- exchange/exchange.go | 3 + pbsmetrics/config/metrics.go | 12 +++- pbsmetrics/go_metrics.go | 4 ++ pbsmetrics/metrics.go | 3 + pbsmetrics/metrics_mock.go | 5 ++ pbsmetrics/prometheus/prometheus.go | 12 ++++ pbsmetrics/prometheus/prometheus_test.go | 70 ++++++++++++++++++++++++ 7 files changed, 108 insertions(+), 1 deletion(-) diff --git a/exchange/exchange.go b/exchange/exchange.go index bc53de5ab5e..a898e608355 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -365,6 +365,9 @@ func (e *exchange) getAllBids(ctx context.Context, cleanRequests map[openrtb_ext var cpm = float64(bid.bid.Price * 1000) e.me.RecordAdapterPrice(*bidlabels, cpm) e.me.RecordAdapterBidReceived(*bidlabels, bid.bidType, bid.bid.AdM != "") + if bid.bidType == openrtb_ext.BidTypeVideo && bid.bidVideo != nil && bid.bidVideo.Duration > 0 { + e.me.RecordAdapterVideoBidDuration(*bidlabels, bid.bidVideo.Duration) + } } } chBids <- brw diff --git a/pbsmetrics/config/metrics.go b/pbsmetrics/config/metrics.go index 88b49a3428f..e275910fa6e 100644 --- a/pbsmetrics/config/metrics.go +++ b/pbsmetrics/config/metrics.go @@ -230,6 +230,13 @@ func (me *MultiMetricsEngine) RecordPodCompititveExclusionTime(labels pbsmetrics } } +// RecordAdapterVideoBidDuration as a noop +func (me *MultiMetricsEngine) RecordAdapterVideoBidDuration(labels pbsmetrics.AdapterLabels, videoBidDuration int) { + for _, thisME := range *me { + thisME.RecordAdapterVideoBidDuration(labels, videoBidDuration) + } +} + // DummyMetricsEngine is a Noop metrics engine in case no metrics are configured. (may also be useful for tests) type DummyMetricsEngine struct{} @@ -309,7 +316,6 @@ func (me *DummyMetricsEngine) RecordRequestQueueTime(success bool, requestType p func (me *DummyMetricsEngine) RecordTimeoutNotice(success bool) { } - // RecordAdapterDuplicateBidID as a noop func (me *DummyMetricsEngine) RecordAdapterDuplicateBidID(adaptor string, collisions int) { } @@ -329,3 +335,7 @@ func (me *DummyMetricsEngine) RecordPodCombGenTime(labels pbsmetrics.PodLabels, // RecordPodCompititveExclusionTime as a noop func (me *DummyMetricsEngine) RecordPodCompititveExclusionTime(labels pbsmetrics.PodLabels, elapsedTime time.Duration) { } + +// RecordAdapterVideoBidDuration as a noop +func (me *DummyMetricsEngine) RecordAdapterVideoBidDuration(labels pbsmetrics.AdapterLabels, videoBidDuration int) { +} diff --git a/pbsmetrics/go_metrics.go b/pbsmetrics/go_metrics.go index fd151e87136..621639683ee 100644 --- a/pbsmetrics/go_metrics.go +++ b/pbsmetrics/go_metrics.go @@ -582,6 +582,10 @@ func (me *Metrics) RecordPodCombGenTime(labels PodLabels, elapsedTime time.Durat func (me *Metrics) RecordPodCompititveExclusionTime(labels PodLabels, elapsedTime time.Duration) { } +// RecordAdapterVideoBidDuration as a noop +func (me *Metrics) RecordAdapterVideoBidDuration(labels AdapterLabels, videoBidDuration int) { +} + 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 33bc1f61a99..a00e24bc7a6 100644 --- a/pbsmetrics/metrics.go +++ b/pbsmetrics/metrics.go @@ -315,4 +315,7 @@ type MetricsEngine interface { // 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) + + //RecordAdapterVideoBidDuration records actual ad duration returned by the bidder + RecordAdapterVideoBidDuration(labels AdapterLabels, videoBidDuration int) } diff --git a/pbsmetrics/metrics_mock.go b/pbsmetrics/metrics_mock.go index 8f8fe2e0151..dac5c88e7ac 100644 --- a/pbsmetrics/metrics_mock.go +++ b/pbsmetrics/metrics_mock.go @@ -131,3 +131,8 @@ func (me *MetricsEngineMock) RecordPodCombGenTime(labels PodLabels, elapsedTime func (me *MetricsEngineMock) RecordPodCompititveExclusionTime(labels PodLabels, elapsedTime time.Duration) { me.Called(labels, elapsedTime) } + +//RecordAdapterVideoBidDuration mock +func (me *MetricsEngineMock) RecordAdapterVideoBidDuration(labels AdapterLabels, videoBidDuration int) { + me.Called(labels, videoBidDuration) +} diff --git a/pbsmetrics/prometheus/prometheus.go b/pbsmetrics/prometheus/prometheus.go index c328cdb2d37..fe621e779d7 100644 --- a/pbsmetrics/prometheus/prometheus.go +++ b/pbsmetrics/prometheus/prometheus.go @@ -41,6 +41,7 @@ type Metrics struct { adapterRequestsTimer *prometheus.HistogramVec adapterUserSync *prometheus.CounterVec adapterDuplicateBidIDCounter *prometheus.CounterVec + adapterVideoBidDuration *prometheus.HistogramVec // Account Metrics accountRequests *prometheus.CounterVec @@ -267,6 +268,10 @@ func NewMetrics(cfg config.PrometheusMetrics) *Metrics { //[]float64{0.000200000, 0.000250000, 0.000275000, 0.000300000}) []float64{0.000100000, 0.000200000, 0.000300000, 0.000400000, 0.000500000, 0.000600000}) + metrics.adapterVideoBidDuration = newHistogram(cfg, metrics.Registry, + "adapter_vidbid_dur", + "Video Ad durations returned by the bidder", []string{adapterLabel}, + []float64{4, 5, 10, 15, 20, 25, 30, 35, 40, 45, 50, 55, 60}) preloadLabelValues(&metrics) return &metrics @@ -534,3 +539,10 @@ func (m *Metrics) RecordPodCombGenTime(labels pbsmetrics.PodLabels, elapsedTime func (m *Metrics) RecordPodCompititveExclusionTime(labels pbsmetrics.PodLabels, elapsedTime time.Duration) { recordAlgoTime(m.podCompExclTimer, labels, elapsedTime) } + +//RecordAdapterVideoBidDuration records actual ad duration (>0) returned by the bidder +func (m *Metrics) RecordAdapterVideoBidDuration(labels pbsmetrics.AdapterLabels, videoBidDuration int) { + if videoBidDuration > 0 { + m.adapterVideoBidDuration.With(prometheus.Labels{adapterLabel: string(labels.Adapter)}).Observe(float64(videoBidDuration)) + } +} diff --git a/pbsmetrics/prometheus/prometheus_test.go b/pbsmetrics/prometheus/prometheus_test.go index fe810d26393..d3e42ff636c 100644 --- a/pbsmetrics/prometheus/prometheus_test.go +++ b/pbsmetrics/prometheus/prometheus_test.go @@ -1011,6 +1011,76 @@ func TestRecordPodCompetitiveExclusionTime(t *testing.T) { }) } +func TestRecordAdapterVideoBidDuration(t *testing.T) { + + testCases := []struct { + description string + bidderAdDurations map[string][]int + expectedSum map[string]int + expectedCount map[string]int + expectedBuckets map[string]map[int]int // cumulative + }{ + { + description: "single bidder multiple ad durations", + bidderAdDurations: map[string][]int{ + "bidder_1": {5, 10, 11, 32}, + }, + expectedSum: map[string]int{"bidder_1": 58}, + expectedCount: map[string]int{"bidder_1": 4}, + expectedBuckets: map[string]map[int]int{ + "bidder_1": {5: 1, 10: 2, 15: 3, 35: 4}, // Upper bound : cumulative number + }, + }, + { + description: "multiple bidders multiple ad durations", + bidderAdDurations: map[string][]int{ + "bidder_1": {5, 10, 11, 32, 39}, + "bidder_2": {25, 30}, + }, + expectedSum: map[string]int{"bidder_1": 97, "bidder_2": 55}, + expectedCount: map[string]int{"bidder_1": 5, "bidder_2": 2}, + expectedBuckets: map[string]map[int]int{ + "bidder_1": {5: 1, 10: 2, 15: 3, 35: 4, 40: 5, 41: 5}, + "bidder_2": {25: 1, 30: 2}, + }, + }, + { + description: "bidder with 0 ad durations", + bidderAdDurations: map[string][]int{ + "bidder_1": {5, 0, 0, 27}, + }, + expectedSum: map[string]int{"bidder_1": 32}, + expectedCount: map[string]int{"bidder_1": 2}, // must exclude 2 observations having 0 durations + expectedBuckets: map[string]map[int]int{ + "bidder_1": {5: 1, 30: 2}, + }, + }, + } + + for _, test := range testCases { + t.Run(test.description, func(t *testing.T) { + m := createMetricsForTesting() + for adapterName, adDurations := range test.bidderAdDurations { + for _, adDuration := range adDurations { + m.RecordAdapterVideoBidDuration(pbsmetrics.AdapterLabels{ + Adapter: openrtb_ext.BidderName(adapterName), + }, adDuration) + } + result := getHistogramFromHistogramVec(m.adapterVideoBidDuration, adapterLabel, adapterName) + for _, bucket := range result.GetBucket() { + cnt, ok := test.expectedBuckets[adapterName][int(bucket.GetUpperBound())] + if ok { + assert.Equal(t, uint64(cnt), bucket.GetCumulativeCount()) + } + } + expectedCount := test.expectedCount[adapterName] + expectedSum := test.expectedSum[adapterName] + assertHistogram(t, "adapter_vidbid_dur", result, uint64(expectedCount), float64(expectedSum)) + } + }) + } +} + func testAlgorithmMetrics(t *testing.T, input int, f func(m *Metrics) dto.Histogram) { // test input adRequests := 2