From e1fb14a7761bc3a1df8fd9cdb691610008adffeb Mon Sep 17 00:00:00 2001 From: beorn7 Date: Wed, 10 Oct 2018 15:41:21 +0200 Subject: [PATCH 1/2] Adjust TestRegisterWithOrGet to the present Signed-off-by: beorn7 --- prometheus/registry_test.go | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/prometheus/registry_test.go b/prometheus/registry_test.go index b037931..ecd46bd 100644 --- a/prometheus/registry_test.go +++ b/prometheus/registry_test.go @@ -738,14 +738,8 @@ func BenchmarkHandler(b *testing.B) { } } -func TestRegisterWithOrGet(t *testing.T) { - // Replace the default registerer just to be sure. This is bad, but this - // whole test will go away once RegisterOrGet is removed. - oldRegisterer := prometheus.DefaultRegisterer - defer func() { - prometheus.DefaultRegisterer = oldRegisterer - }() - prometheus.DefaultRegisterer = prometheus.NewRegistry() +func TestAlreadyRegistered(t *testing.T) { + reg := prometheus.NewRegistry() original := prometheus.NewCounterVec( prometheus.CounterOpts{ Name: "test", @@ -761,11 +755,11 @@ func TestRegisterWithOrGet(t *testing.T) { []string{"foo", "bar"}, ) var err error - if err = prometheus.Register(original); err != nil { + if err = reg.Register(original); err != nil { t.Fatal(err) } - if err = prometheus.Register(equalButNotSame); err == nil { - t.Fatal("expected error when registringe equal collector") + if err = reg.Register(equalButNotSame); err == nil { + t.Fatal("expected error when registering equal collector") } if are, ok := err.(prometheus.AlreadyRegisteredError); ok { if are.ExistingCollector != original { From 5e8ac3cd5850a9dedb80f701938f36397d04fcec Mon Sep 17 00:00:00 2001 From: beorn7 Date: Wed, 10 Oct 2018 16:59:24 +0200 Subject: [PATCH 2/2] Add a concurrent end-to-end test for observe-register-gather This is an attempt to expose https://github.com/istio/istio/issues/8906 . The failure to do so makes me believe the error is either already fixed in current client_golang, or something weird I haven't spotted yet is happening in the istio code. Signed-off-by: beorn7 --- prometheus/registry_test.go | 99 +++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/prometheus/registry_test.go b/prometheus/registry_test.go index ecd46bd..d05ac9a 100644 --- a/prometheus/registry_test.go +++ b/prometheus/registry_test.go @@ -21,9 +21,12 @@ package prometheus_test import ( "bytes" + "math/rand" "net/http" "net/http/httptest" + "sync" "testing" + "time" dto "github.com/prometheus/client_model/go" @@ -772,3 +775,99 @@ func TestAlreadyRegistered(t *testing.T) { t.Error("unexpected error:", err) } } + +// TestHistogramVecRegisterGatherConcurrency is an end-to-end test that +// concurrently calls Observe on random elements of a HistogramVec while the +// same HistogramVec is registered concurrently and the Gather method of the +// registry is called concurrently. +func TestHistogramVecRegisterGatherConcurrency(t *testing.T) { + var ( + reg = prometheus.NewPedanticRegistry() + hv = prometheus.NewHistogramVec( + prometheus.HistogramOpts{ + Name: "test_histogram", + Help: "This helps testing.", + ConstLabels: prometheus.Labels{"foo": "bar"}, + }, + []string{"one", "two", "three"}, + ) + labelValues = []string{"a", "b", "c", "alpha", "beta", "gamma", "aleph", "beth", "gimel"} + quit = make(chan struct{}) + wg sync.WaitGroup + ) + + observe := func() { + defer wg.Done() + for { + select { + case <-quit: + return + default: + obs := rand.NormFloat64()*.1 + .2 + hv.WithLabelValues( + labelValues[rand.Intn(len(labelValues))], + labelValues[rand.Intn(len(labelValues))], + labelValues[rand.Intn(len(labelValues))], + ).Observe(obs) + } + } + } + + register := func() { + defer wg.Done() + for { + select { + case <-quit: + return + default: + if err := reg.Register(hv); err != nil { + if _, ok := err.(prometheus.AlreadyRegisteredError); !ok { + t.Error("Registering failed:", err) + } + } + time.Sleep(7 * time.Millisecond) + } + } + } + + gather := func() { + defer wg.Done() + for { + select { + case <-quit: + return + default: + if g, err := reg.Gather(); err != nil { + t.Error("Gathering failed:", err) + } else { + if len(g) == 0 { + continue + } + if len(g) != 1 { + t.Error("Gathered unexpected number of metric families:", len(g)) + } + if len(g[0].Metric[0].Label) != 4 { + t.Error("Gathered unexpected number of label pairs:", len(g[0].Metric[0].Label)) + } + } + time.Sleep(4 * time.Millisecond) + } + } + } + + wg.Add(10) + go observe() + go observe() + go register() + go observe() + go gather() + go observe() + go register() + go observe() + go gather() + go observe() + + time.Sleep(time.Second) + close(quit) + wg.Wait() +}