Scott's review

Author: guscarreon

gdpr/impl.go

@@ -20,6 +20,7 @@ type permissionsImpl struct {
 	fetchVendorList        VendorListFetcher
 	gdprDefaultValue       string
 	hostVendorID           int
+	metrics                metrics.MetricsEngine
 	nonStandardPublishers  map[string]struct{}
 	purposeEnforcerBuilder PurposeEnforcerBuilder
 	vendorIDs              map[openrtb_ext.BidderName]uint16
@@ -29,8 +30,6 @@ type permissionsImpl struct {
 	consent     string
 	gdprSignal  Signal
 	publisherID string
-
-	metrics metrics.MetricsEngine
 }
 
 // HostCookiesAllowed determines whether the host is allowed to set cookies on the user's device

gdpr/vendorlist-fetching.go

@@ -116,7 +116,7 @@ func newOccasionalSaver(timeout time.Duration) func(ctx context.Context, client
 	}
 }
 
-func saveOne(ctx context.Context, client *http.Client, url string, saver saveVendors, metricsEngine metrics.MetricsEngine) uint16 {
+func saveOne(ctx context.Context, client *http.Client, url string, saver saveVendors, me metrics.MetricsEngine) uint16 {
 	req, err := http.NewRequest("GET", url, nil)
 	if err != nil {
 		glog.Errorf("Failed to build GET %s request. Cookie syncs may be affected: %v", url, err)
@@ -137,11 +137,10 @@ func saveOne(ctx context.Context, client *http.Client, url string, saver saveVen
 	}
 	if resp.StatusCode != http.StatusOK {
 		glog.Errorf("GET %s returned %d. Cookie syncs may be affected.", url, resp.StatusCode)
-		//metricsEngine.RecordVendorListFetch(fail)
+		me.RecordGvlListRequest()
 		return 0
 	}
-	// log GVL fetch metric here
-	//metricsEngine.RecordVendorListFetch(success)
+	me.RecordGvlListRequest()
 
 	var newList api.VendorList
 	newList, err = vendorlist2.ParseEagerly(respBody)

metrics/go_metrics.go

@@ -303,7 +303,7 @@ func NewMetrics(registry metrics.Registry, exchanges []openrtb_ext.BidderName, d
 	newMetrics.PrebidCacheRequestTimerSuccess = metrics.GetOrRegisterTimer("prebid_cache_request_time.ok", registry)
 	newMetrics.PrebidCacheRequestTimerError = metrics.GetOrRegisterTimer("prebid_cache_request_time.err", registry)
 	newMetrics.StoredResponsesMeter = metrics.GetOrRegisterMeter("stored_responses", registry)
-	newMetrics.GvlListRequestsMeter = metrics.GetOrRegisterMeter("gvl_list_requests", registry)
+	newMetrics.GvlListRequestsMeter = metrics.GetOrRegisterMeter("gvl_requests", registry)
 	newMetrics.OverheadTimer = makeOverheadTimerMetrics(registry)
 	newMetrics.BidderServerResponseTimer = metrics.GetOrRegisterTimer("bidder_server_response_time_seconds", registry)
 

metrics/go_metrics_test.go

@@ -36,7 +36,7 @@ func TestNewMetrics(t *testing.T) {
 	ensureContains(t, registry, "setuid_requests.gdpr_blocked_host_cookie", m.SetUidStatusMeter[SetUidGDPRHostCookieBlocked])
 	ensureContains(t, registry, "setuid_requests.syncer_unknown", m.SetUidStatusMeter[SetUidSyncerUnknown])
 	ensureContains(t, registry, "stored_responses", m.StoredResponsesMeter)
-	ensureContains(t, registry, "gvl_list_requests", m.GvlListRequestsMeter)
+	ensureContains(t, registry, "gvl_requests", m.GvlListRequestsMeter)
 
 	ensureContains(t, registry, "prebid_cache_request_time.ok", m.PrebidCacheRequestTimerSuccess)
 	ensureContains(t, registry, "prebid_cache_request_time.err", m.PrebidCacheRequestTimerError)

metrics/prometheus/prometheus.go

@@ -366,7 +366,7 @@ func NewMetrics(cfg config.PrometheusMetrics, disabledMetrics config.DisabledMet
 		"Count of total requests to Prebid Server that have stored responses")
 
 	metrics.gvlListRequests = newCounterWithoutLabels(cfg, reg,
-		"gvl_list_requests",
+		"gvl_requests",
 		"Count number of times GVL list is fetched")
 
 	metrics.adapterBids = newCounter(cfg, reg,