Adds a layer of abstraction to get TCF metrics more in line with othe…

…r metrics.
prebid · Jul 1, 2020 · bfa680f · bfa680f
1 parent 3b76bde
commit bfa680f
Show file tree

Hide file tree

Showing 7 changed files with 24 additions and 30 deletions.
diff --git a/exchange/exchange.go b/exchange/exchange.go
@@ -107,7 +107,7 @@ func (e *exchange) HoldAuction(ctx context.Context, bidRequest *openrtb.BidReque
 	cleanRequests, aliases, cleanMetrics, errs := cleanOpenRTBRequests(ctx, bidRequest, usersyncs, blabels, labels, e.gDPR, e.UsersyncIfAmbiguous, e.privacyConfig)
 
 	if cleanMetrics.gdprEnforced {
-		e.me.RecordTCFReq(cleanMetrics.gdprTcfVersion)
+		e.me.RecordTCFReq(pbsmetrics.TCFVersionToValue(cleanMetrics.gdprTcfVersion))
 	}
 	// List of bidders we have requests for.
 	liveAdapters := listBiddersWithRequests(cleanRequests)

diff --git a/pbsmetrics/config/metrics.go b/pbsmetrics/config/metrics.go
@@ -196,7 +196,7 @@ func (me *MultiMetricsEngine) RecordTimeoutNotice(success bool) {
 }
 
 // RecordTCFReq across all engines
-func (me *MultiMetricsEngine) RecordTCFReq(version int) {
+func (me *MultiMetricsEngine) RecordTCFReq(version pbsmetrics.TCFVersionValue) {
 	for _, thisME := range *me {
 		thisME.RecordTCFReq(version)
 	}
@@ -282,5 +282,5 @@ func (me *DummyMetricsEngine) RecordTimeoutNotice(success bool) {
 }
 
 // RecordReq as a noop
-func (me *DummyMetricsEngine) RecordTCFReq(version int) {
+func (me *DummyMetricsEngine) RecordTCFReq(version pbsmetrics.TCFVersionValue) {
 }
diff --git a/pbsmetrics/go_metrics.go b/pbsmetrics/go_metrics.go
@@ -54,7 +54,6 @@ type Metrics struct {
 
 	// TCF adaption metrics
 	TCFReqVersion map[TCFVersionValue]metrics.Meter
-	TCFVersions   []TCFVersionValue
 
 	AdapterMetrics map[openrtb_ext.BidderName]*AdapterMetrics
 	// Don't export accountMetrics because we need helper functions here to insure its properly populated dynamically
@@ -143,7 +142,6 @@ func NewBlankMetrics(registry metrics.Registry, exchanges []openrtb_ext.BidderNa
 		TimeoutNotificationFailure: blankMeter,
 
 		TCFReqVersion: make(map[TCFVersionValue]metrics.Meter, len(TCFVersions())),
-		TCFVersions:   TCFVersions(),
 
 		AdapterMetrics:  make(map[openrtb_ext.BidderName]*AdapterMetrics, len(exchanges)),
 		accountMetrics:  make(map[string]*accountMetrics),
@@ -584,13 +582,8 @@ func (me *Metrics) RecordTimeoutNotice(success bool) {
 	return
 }
 
-func (me *Metrics) RecordTCFReq(version int) {
-	if version >= 0 && version < len(me.TCFVersions) {
-		key := me.TCFVersions[version]
-		me.TCFReqVersion[key].Mark(1)
-		return
-	}
-	me.TCFReqVersion[TCFVersionErr].Mark(1)
+func (me *Metrics) RecordTCFReq(version TCFVersionValue) {
+	me.TCFReqVersion[version].Mark(1)
 	return
 }
 

diff --git a/pbsmetrics/metrics.go b/pbsmetrics/metrics.go
@@ -266,6 +266,17 @@ func TCFVersions() []TCFVersionValue {
 	}
 }
 
+// TCFVersionToValue takes an integer TCF version and returns the corresponding TCFVersionValue
+func TCFVersionToValue(version int) TCFVersionValue {
+	switch {
+	case version == 1:
+		return TCFVersionV1
+	case version == 2:
+		return TCFVersionV2
+	}
+	return TCFVersionErr
+}
+
 // MetricsEngine is a generic interface to record PBS metrics into the desired backend
 // The first three metrics function fire off once per incoming request, so total metrics
 // will equal the total number of incoming requests. The remaining 5 fire off per outgoing
@@ -294,5 +305,5 @@ type MetricsEngine interface {
 	RecordPrebidCacheRequestTime(success bool, length time.Duration)
 	RecordRequestQueueTime(success bool, requestType RequestType, length time.Duration)
 	RecordTimeoutNotice(sucess bool)
-	RecordTCFReq(version int)
+	RecordTCFReq(version TCFVersionValue)
 }
diff --git a/pbsmetrics/metrics_mock.go b/pbsmetrics/metrics_mock.go
@@ -108,6 +108,6 @@ func (me *MetricsEngineMock) RecordTimeoutNotice(success bool) {
 }
 
 // RecordTCFReq mock
-func (me *MetricsEngineMock) RecordTCFReq(version int) {
+func (me *MetricsEngineMock) RecordTCFReq(version TCFVersionValue) {
 	me.Called(version)
 }
diff --git a/pbsmetrics/prometheus/prometheus.go b/pbsmetrics/prometheus/prometheus.go
@@ -43,9 +43,6 @@ type Metrics struct {
 
 	// Account Metrics
 	accountRequests *prometheus.CounterVec
-
-	// Lookup tables
-	tcfVersions []string
 }
 
 const (
@@ -172,7 +169,6 @@ func NewMetrics(cfg config.PrometheusMetrics) *Metrics {
 		"privacy_tcf",
 		"Count of TCF versions for requests where GDPR was enforced.",
 		[]string{versionLabel, sourceLabel})
-	metrics.tcfVersions = tcfVersionsAsString()
 
 	metrics.adapterBids = newCounter(cfg, metrics.Registry,
 		"adapter_bids",
@@ -438,15 +434,9 @@ func (m *Metrics) RecordTimeoutNotice(success bool) {
 	}
 }
 
-func (m *Metrics) RecordTCFReq(version int) {
-	var value string
-	if version >= 0 && version < len(m.tcfVersions) {
-		value = m.tcfVersions[version]
-	} else {
-		value = m.tcfVersions[0]
-	}
+func (m *Metrics) RecordTCFReq(version pbsmetrics.TCFVersionValue) {
 	m.tcfVersion.With(prometheus.Labels{
-		versionLabel: value,
+		versionLabel: string(version),
 		sourceLabel:  sourceRequest,
 	}).Inc()
 }
diff --git a/pbsmetrics/prometheus/prometheus_test.go b/pbsmetrics/prometheus/prometheus_test.go
@@ -947,10 +947,10 @@ func TestTimeoutNotifications(t *testing.T) {
 func TestTCFMetrics(t *testing.T) {
 	m := createMetricsForTesting()
 
-	m.RecordTCFReq(0)
-	m.RecordTCFReq(1)
-	m.RecordTCFReq(2)
-	m.RecordTCFReq(1)
+	m.RecordTCFReq(pbsmetrics.TCFVersionToValue(0))
+	m.RecordTCFReq(pbsmetrics.TCFVersionToValue(1))
+	m.RecordTCFReq(pbsmetrics.TCFVersionToValue(2))
+	m.RecordTCFReq(pbsmetrics.TCFVersionToValue(1))
 
 	assertCounterVecValue(t, "", "privacy_tcf:err", m.tcfVersion,
 		float64(1),