Skip to content

Commit

Permalink
OTT-71: (CI) Add stats collecting information around the actual Ad du…
Browse files Browse the repository at this point in the history
…ration (#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 <shriprasad.marathe@pubmatic.com>
  • Loading branch information
ShriprasadM and pm-shriprasad-marathe authored Dec 18, 2020
1 parent 3892ad9 commit 9e23a9b
Show file tree
Hide file tree
Showing 7 changed files with 108 additions and 1 deletion.
3 changes: 3 additions & 0 deletions exchange/exchange.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 11 additions & 1 deletion pbsmetrics/config/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}

Expand Down Expand Up @@ -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) {
}
Expand All @@ -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) {
}
4 changes: 4 additions & 0 deletions pbsmetrics/go_metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions pbsmetrics/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
5 changes: 5 additions & 0 deletions pbsmetrics/metrics_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
12 changes: 12 additions & 0 deletions pbsmetrics/prometheus/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ type Metrics struct {
adapterRequestsTimer *prometheus.HistogramVec
adapterUserSync *prometheus.CounterVec
adapterDuplicateBidIDCounter *prometheus.CounterVec
adapterVideoBidDuration *prometheus.HistogramVec

// Account Metrics
accountRequests *prometheus.CounterVec
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))
}
}
70 changes: 70 additions & 0 deletions pbsmetrics/prometheus/prometheus_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 9e23a9b

Please sign in to comment.