[tor-commits] [snowflake/main] Make promMetrics not a global

[arlo at torproject.org]

commit 160ae2dd71879ab83226a3d4eb2b15cefdb570f4
Author: Arlo Breault <abreault at wikimedia.org>
Date:   Tue May 18 20:06:28 2021 -0400

    Make promMetrics not a global
    
    Doesn't seem like it needs to exist outside of the metrics struct.
    
    Also, the call to logMetrics is moved to the constructor.  A metrics
    instance is only created when a BrokerContext is created, which only
    happens at startup.  The sync of only doing that once is left for
    documentation purposes, since it doesn't hurt, but also seems redundant.
---
 broker/broker.go  | 16 ++++++++--------
 broker/metrics.go | 25 ++++++++++++-------------
 2 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/broker/broker.go b/broker/broker.go
index 8d7a314..8c1159e 100644
--- a/broker/broker.go
+++ b/broker/broker.go
@@ -151,7 +151,7 @@ func (ctx *BrokerContext) Broker() {
 					} else {
 						heap.Remove(ctx.restrictedSnowflakes, snowflake.index)
 					}
-					promMetrics.AvailableProxies.With(prometheus.Labels{"nat": request.natType, "type": request.proxyType}).Dec()
+					ctx.metrics.promMetrics.AvailableProxies.With(prometheus.Labels{"nat": request.natType, "type": request.proxyType}).Dec()
 					delete(ctx.idToSnowflake, snowflake.id)
 					close(request.offerChannel)
 				}
@@ -177,7 +177,7 @@ func (ctx *BrokerContext) AddSnowflake(id string, proxyType string, natType stri
 	} else {
 		heap.Push(ctx.restrictedSnowflakes, snowflake)
 	}
-	promMetrics.AvailableProxies.With(prometheus.Labels{"nat": natType, "type": proxyType}).Inc()
+	ctx.metrics.promMetrics.AvailableProxies.With(prometheus.Labels{"nat": natType, "type": proxyType}).Inc()
 	ctx.snowflakeLock.Unlock()
 	ctx.idToSnowflake[id] = snowflake
 	return snowflake
@@ -216,7 +216,7 @@ func proxyPolls(ctx *BrokerContext, w http.ResponseWriter, r *http.Request) {
 	if nil == offer {
 		ctx.metrics.lock.Lock()
 		ctx.metrics.proxyIdleCount++
-		promMetrics.ProxyPollTotal.With(prometheus.Labels{"nat": natType, "status": "idle"}).Inc()
+		ctx.metrics.promMetrics.ProxyPollTotal.With(prometheus.Labels{"nat": natType, "status": "idle"}).Inc()
 		ctx.metrics.lock.Unlock()
 
 		b, err = messages.EncodePollResponse("", false, "")
@@ -228,7 +228,7 @@ func proxyPolls(ctx *BrokerContext, w http.ResponseWriter, r *http.Request) {
 		w.Write(b)
 		return
 	}
-	promMetrics.ProxyPollTotal.With(prometheus.Labels{"nat": natType, "status": "matched"}).Inc()
+	ctx.metrics.promMetrics.ProxyPollTotal.With(prometheus.Labels{"nat": natType, "status": "matched"}).Inc()
 	b, err = messages.EncodePollResponse(string(offer.sdp), true, offer.natType)
 	if err != nil {
 		w.WriteHeader(http.StatusInternalServerError)
@@ -282,7 +282,7 @@ func clientOffers(ctx *BrokerContext, w http.ResponseWriter, r *http.Request) {
 	if numSnowflakes <= 0 {
 		ctx.metrics.lock.Lock()
 		ctx.metrics.clientDeniedCount++
-		promMetrics.ClientPollTotal.With(prometheus.Labels{"nat": offer.natType, "status": "denied"}).Inc()
+		ctx.metrics.promMetrics.ClientPollTotal.With(prometheus.Labels{"nat": offer.natType, "status": "denied"}).Inc()
 		if offer.natType == NATUnrestricted {
 			ctx.metrics.clientUnrestrictedDeniedCount++
 		} else {
@@ -304,7 +304,7 @@ func clientOffers(ctx *BrokerContext, w http.ResponseWriter, r *http.Request) {
 	case answer := <-snowflake.answerChannel:
 		ctx.metrics.lock.Lock()
 		ctx.metrics.clientProxyMatchCount++
-		promMetrics.ClientPollTotal.With(prometheus.Labels{"nat": offer.natType, "status": "matched"}).Inc()
+		ctx.metrics.promMetrics.ClientPollTotal.With(prometheus.Labels{"nat": offer.natType, "status": "matched"}).Inc()
 		ctx.metrics.lock.Unlock()
 		if _, err := w.Write(answer); err != nil {
 			log.Printf("unable to write answer with error: %v", err)
@@ -321,7 +321,7 @@ func clientOffers(ctx *BrokerContext, w http.ResponseWriter, r *http.Request) {
 	}
 
 	ctx.snowflakeLock.Lock()
-	promMetrics.AvailableProxies.With(prometheus.Labels{"nat": snowflake.natType, "type": snowflake.proxyType}).Dec()
+	ctx.metrics.promMetrics.AvailableProxies.With(prometheus.Labels{"nat": snowflake.natType, "type": snowflake.proxyType}).Dec()
 	delete(ctx.idToSnowflake, snowflake.id)
 	ctx.snowflakeLock.Unlock()
 }
@@ -506,7 +506,7 @@ func main() {
 	http.Handle("/answer", SnowflakeHandler{ctx, proxyAnswers})
 	http.Handle("/debug", SnowflakeHandler{ctx, debugHandler})
 	http.Handle("/metrics", MetricsHandler{metricsFilename, metricsHandler})
-	http.Handle("/prometheus", promhttp.HandlerFor(promMetrics.registry, promhttp.HandlerOpts{}))
+	http.Handle("/prometheus", promhttp.HandlerFor(ctx.metrics.promMetrics.registry, promhttp.HandlerOpts{}))
 
 	server := http.Server{
 		Addr: addr,
diff --git a/broker/metrics.go b/broker/metrics.go
index ad55bcb..a79a1c4 100644
--- a/broker/metrics.go
+++ b/broker/metrics.go
@@ -17,11 +17,6 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 )
 
-var (
-	once        sync.Once
-	promMetrics = initPrometheus()
-)
-
 const (
 	prometheusNamespace = "snowflake"
 	metricsResolution   = 60 * 60 * 24 * time.Second //86400 seconds
@@ -54,8 +49,11 @@ type Metrics struct {
 	clientUnrestrictedDeniedCount uint
 	clientProxyMatchCount         uint
 
-	//synchronization for access to snowflake metrics
+	// synchronization for access to snowflake metrics
 	lock sync.Mutex
+
+	promMetrics *PromMetrics
+	once        sync.Once
 }
 
 type record struct {
@@ -147,7 +145,7 @@ func (m *Metrics) UpdateCountryStats(addr string, proxyType string, natType stri
 		m.countryStats.unknown[addr] = true
 	}
 
-	promMetrics.ProxyTotal.With(prometheus.Labels{
+	m.promMetrics.ProxyTotal.With(prometheus.Labels{
 		"nat":  natType,
 		"type": proxyType,
 		"cc":   country,
@@ -201,9 +199,10 @@ func NewMetrics(metricsLogger *log.Logger) (*Metrics, error) {
 	}
 
 	m.logger = metricsLogger
+	m.promMetrics = initPrometheus()
 
 	// Write to log file every hour with updated metrics
-	go once.Do(m.logMetrics)
+	go m.once.Do(m.logMetrics)
 
 	return m, nil
 }
@@ -267,9 +266,8 @@ type PromMetrics struct {
 	AvailableProxies *prometheus.GaugeVec
 }
 
-//Initialize metrics for prometheus exporter
+// Initialize metrics for prometheus exporter
 func initPrometheus() *PromMetrics {
-
 	promMetrics := &PromMetrics{}
 
 	promMetrics.registry = prometheus.NewRegistry()
@@ -311,9 +309,10 @@ func initPrometheus() *PromMetrics {
 	)
 
 	// We need to register our metrics so they can be exported.
-	promMetrics.registry.MustRegister(promMetrics.ClientPollTotal, promMetrics.ProxyPollTotal,
-		promMetrics.ProxyTotal, promMetrics.AvailableProxies)
+	promMetrics.registry.MustRegister(
+		promMetrics.ClientPollTotal, promMetrics.ProxyPollTotal,
+		promMetrics.ProxyTotal, promMetrics.AvailableProxies,
+	)
 
 	return promMetrics
-
 }