From 87076b673153c45409bf2fe45b9f446c55908204 Mon Sep 17 00:00:00 2001
From: Mike Goldsmth <goldsmith.mike@gmail.com>
Date: Mon, 4 Oct 2021 16:08:58 +0100
Subject: [PATCH 1/2] use rwlock in metrics prometheus

---
 metrics/prometheus.go | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/metrics/prometheus.go b/metrics/prometheus.go
index 0f799328b3..5809da919d 100644
--- a/metrics/prometheus.go
+++ b/metrics/prometheus.go
@@ -19,7 +19,7 @@ type PromMetrics struct {
 	// metrics keeps a record of all the registered metrics so we can increment
 	// them by name
 	metrics map[string]interface{}
-	lock    sync.Mutex
+	lock    sync.RWMutex
 
 	prefix string
 }
@@ -57,21 +57,21 @@ func (p *PromMetrics) Register(name string, metricType string) {
 	switch metricType {
 	case "counter":
 		newmet = promauto.NewCounter(prometheus.CounterOpts{
-			Name: name,
+			Name:      name,
 			Namespace: p.prefix,
-			Help: name,
+			Help:      name,
 		})
 	case "gauge":
 		newmet = promauto.NewGauge(prometheus.GaugeOpts{
-			Name: name,
+			Name:      name,
 			Namespace: p.prefix,
-			Help: name,
+			Help:      name,
 		})
 	case "histogram":
 		newmet = promauto.NewHistogram(prometheus.HistogramOpts{
-			Name: name,
+			Name:      name,
 			Namespace: p.prefix,
-			Help: name,
+			Help:      name,
 		})
 	}
 
@@ -79,8 +79,8 @@ func (p *PromMetrics) Register(name string, metricType string) {
 }
 
 func (p *PromMetrics) Increment(name string) {
-	p.lock.Lock()
-	defer p.lock.Unlock()
+	p.lock.RLock()
+	defer p.lock.RUnlock()
 
 	if counterIface, ok := p.metrics[name]; ok {
 		if counter, ok := counterIface.(prometheus.Counter); ok {
@@ -89,8 +89,8 @@ func (p *PromMetrics) Increment(name string) {
 	}
 }
 func (p *PromMetrics) Count(name string, n interface{}) {
-	p.lock.Lock()
-	defer p.lock.Unlock()
+	p.lock.RLock()
+	defer p.lock.RUnlock()
 
 	if counterIface, ok := p.metrics[name]; ok {
 		if counter, ok := counterIface.(prometheus.Counter); ok {
@@ -99,8 +99,8 @@ func (p *PromMetrics) Count(name string, n interface{}) {
 	}
 }
 func (p *PromMetrics) Gauge(name string, val interface{}) {
-	p.lock.Lock()
-	defer p.lock.Unlock()
+	p.lock.RLock()
+	defer p.lock.RUnlock()
 
 	if gaugeIface, ok := p.metrics[name]; ok {
 		if gauge, ok := gaugeIface.(prometheus.Gauge); ok {
@@ -109,8 +109,8 @@ func (p *PromMetrics) Gauge(name string, val interface{}) {
 	}
 }
 func (p *PromMetrics) Histogram(name string, obs interface{}) {
-	p.lock.Lock()
-	defer p.lock.Unlock()
+	p.lock.RLock()
+	defer p.lock.RUnlock()
 
 	if histIface, ok := p.metrics[name]; ok {
 		if hist, ok := histIface.(prometheus.Histogram); ok {

From 192e088ef8edf54253e51eb33d559b99b332e113 Mon Sep 17 00:00:00 2001
From: Robb Kidd <robbkidd@honeycomb.io>
Date: Tue, 5 Oct 2021 16:37:23 -0400
Subject: [PATCH 2/2] check for racing in metrics

---
 metrics/prometheus_test.go | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/metrics/prometheus_test.go b/metrics/prometheus_test.go
index b0c093caf5..539f926a41 100644
--- a/metrics/prometheus_test.go
+++ b/metrics/prometheus_test.go
@@ -3,6 +3,7 @@
 package metrics
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/honeycombio/refinery/config"
@@ -24,3 +25,29 @@ func TestMultipleRegistrations(t *testing.T) {
 
 	p.Register("test", "counter")
 }
+
+func TestRaciness(t *testing.T) {
+	p := &PromMetrics{
+		Logger: &logger.MockLogger{},
+		Config: &config.MockConfig{},
+	}
+
+	err := p.Start()
+
+	assert.NoError(t, err)
+
+	p.Register("race", "counter")
+
+	// this loop modifying the metric registry and reading it to increment
+	// a counter should not trigger a race condition
+	for i := 0; i < 50; i++ {
+		go func(j int) {
+			metricName := fmt.Sprintf("metric%d", j)
+			p.Register(metricName, "counter")
+		}(i)
+
+		go func(j int) {
+			p.Increment("race")
+		}(i)
+	}
+}