diff --git a/metrics/generic/generic.go b/metrics/generic/generic.go index 9fc6e01..2f7ae70 100644 --- a/metrics/generic/generic.go +++ b/metrics/generic/generic.go @@ -121,7 +121,7 @@ type Histogram struct { Name string lvs lv.LabelValues - h gohistogram.Histogram + h *safeHistogram } // NewHistogram returns a numeric histogram based on VividCortex/gohistogram. A @@ -129,7 +129,7 @@ func NewHistogram(name string, buckets int) *Histogram { return &Histogram{ Name: name, - h: gohistogram.NewHistogram(buckets), + h: &safeHistogram{Histogram: gohistogram.NewHistogram(buckets)}, } } @@ -143,11 +143,15 @@ // Observe implements Histogram. func (h *Histogram) Observe(value float64) { + h.h.Lock() + defer h.h.Unlock() h.h.Add(value) } // Quantile returns the value of the quantile q, 0.0 < q < 1.0. func (h *Histogram) Quantile(q float64) float64 { + h.h.RLock() + defer h.h.RUnlock() return h.h.Quantile(q) } @@ -159,7 +163,15 @@ // Print writes a string representation of the histogram to the passed writer. // Useful for printing to a terminal. func (h *Histogram) Print(w io.Writer) { + h.h.RLock() + defer h.h.RUnlock() fmt.Fprintf(w, h.h.String()) +} + +// safeHistogram exists as gohistogram.Histogram is not goroutine-safe. +type safeHistogram struct { + sync.RWMutex + gohistogram.Histogram } // Bucket is a range in a histogram which aggregates observations. diff --git a/metrics/generic/generic_test.go b/metrics/generic/generic_test.go index de8940b..a2eb91b 100644 --- a/metrics/generic/generic_test.go +++ b/metrics/generic/generic_test.go @@ -7,6 +7,7 @@ import ( "math" "math/rand" + "sync" "testing" "github.com/go-kit/kit/metrics/generic" @@ -52,6 +53,27 @@ } } +func TestIssue424(t *testing.T) { + var ( + histogram = generic.NewHistogram("dont_panic", 50) + concurrency = 100 + operations = 1000 + wg sync.WaitGroup + ) + + wg.Add(concurrency) + for i := 0; i < concurrency; i++ { + go func() { + defer wg.Done() + for j := 0; j < operations; j++ { + histogram.Observe(float64(j)) + histogram.Observe(histogram.Quantile(0.5)) + } + }() + } + wg.Wait() +} + func TestSimpleHistogram(t *testing.T) { histogram := generic.NewSimpleHistogram().With("label", "simple_histogram").(*generic.SimpleHistogram) var (