Skip to content

Commit

Permalink
Handle duplicate prometheus metrics registrations (#192)
Browse files Browse the repository at this point in the history
Handle adding prometheus metrics in the same way that honeycomb metrics class works.

Surprised that no one else has run into this!

Another thing we could do here in addition to this: we could append the dataset that the sampler is for to the metrics so that they should all be unique
  • Loading branch information
martin308 authored Nov 13, 2020
1 parent b30d601 commit 442e269
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 3 deletions.
13 changes: 10 additions & 3 deletions metrics/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,16 @@ func (p *PromMetrics) Start() error {
// Register takes a name and a metric type. The type should be one of "counter",
// "gauge", or "histogram"
func (p *PromMetrics) Register(name string, metricType string) {
var newmet interface{}
p.lock.Lock()
defer p.lock.Unlock()

newmet, exists := p.metrics[name]

// don't attempt to add the metric again as this will cause a panic
if exists {
return
}

switch metricType {
case "counter":
newmet = promauto.NewCounter(prometheus.CounterOpts{
Expand All @@ -61,9 +70,7 @@ func (p *PromMetrics) Register(name string, metricType string) {
})
}

p.lock.Lock()
p.metrics[name] = newmet
p.lock.Unlock()
}

func (p *PromMetrics) IncrementCounter(name string) {
Expand Down
26 changes: 26 additions & 0 deletions metrics/prometheus_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// +build all race

package metrics

import (
"testing"

"github.com/honeycombio/refinery/config"
"github.com/honeycombio/refinery/logger"
"github.com/stretchr/testify/assert"
)

func TestMultipleRegistrations(t *testing.T) {
p := &PromMetrics{
Logger: &logger.MockLogger{},
Config: &config.MockConfig{},
}

err := p.Start()

assert.NoError(t, err)

p.Register("test", "counter")

p.Register("test", "counter")
}

0 comments on commit 442e269

Please sign in to comment.