From f3249839466180902659ee45540e4e7fd0f77229 Mon Sep 17 00:00:00 2001
From: Matevz Mihalic <matevz.mihalic@3fs.si>
Date: Wed, 8 Mar 2017 15:17:07 +0100
Subject: [PATCH] Fix metrics registering

---
 middlewares/prometheus.go      | 28 ++++++++++-
 middlewares/prometheus_test.go | 85 +++++++++++++++++++++++++++++++++-
 2 files changed, 110 insertions(+), 3 deletions(-)

diff --git a/middlewares/prometheus.go b/middlewares/prometheus.go
index 366b96727..f41a51ffb 100644
--- a/middlewares/prometheus.go
+++ b/middlewares/prometheus.go
@@ -32,7 +32,8 @@ func (p *Prometheus) getLatencyHistogram() metrics.Histogram {
 // NewPrometheus returns a new prometheus Metrics implementation.
 func NewPrometheus(name string, config *types.Prometheus) *Prometheus {
 	var m Prometheus
-	m.reqsCounter = prometheus.NewCounterFrom(
+
+	cv := stdprometheus.NewCounterVec(
 		stdprometheus.CounterOpts{
 			Name:        reqsName,
 			Help:        "How many HTTP requests processed, partitioned by status code and method.",
@@ -41,6 +42,17 @@ func NewPrometheus(name string, config *types.Prometheus) *Prometheus {
 		[]string{"code", "method"},
 	)
 
+	err := stdprometheus.Register(cv)
+	if err != nil {
+		e, ok := err.(stdprometheus.AlreadyRegisteredError)
+		if !ok {
+			panic(err)
+		}
+		m.reqsCounter = prometheus.NewCounter(e.ExistingCollector.(*stdprometheus.CounterVec))
+	} else {
+		m.reqsCounter = prometheus.NewCounter(cv)
+	}
+
 	var buckets []float64
 	if config.Buckets != nil {
 		buckets = config.Buckets
@@ -48,7 +60,7 @@ func NewPrometheus(name string, config *types.Prometheus) *Prometheus {
 		buckets = []float64{0.1, 0.3, 1.2, 5}
 	}
 
-	m.latencyHistogram = prometheus.NewHistogramFrom(
+	hv := stdprometheus.NewHistogramVec(
 		stdprometheus.HistogramOpts{
 			Name:        latencyName,
 			Help:        "How long it took to process the request.",
@@ -57,6 +69,18 @@ func NewPrometheus(name string, config *types.Prometheus) *Prometheus {
 		},
 		[]string{},
 	)
+
+	err = stdprometheus.Register(hv)
+	if err != nil {
+		e, ok := err.(stdprometheus.AlreadyRegisteredError)
+		if !ok {
+			panic(err)
+		}
+		m.latencyHistogram = prometheus.NewHistogram(e.ExistingCollector.(*stdprometheus.HistogramVec))
+	} else {
+		m.latencyHistogram = prometheus.NewHistogram(hv)
+	}
+
 	return &m
 }
 
diff --git a/middlewares/prometheus_test.go b/middlewares/prometheus_test.go
index 8315b1756..08931ac8b 100644
--- a/middlewares/prometheus_test.go
+++ b/middlewares/prometheus_test.go
@@ -9,10 +9,19 @@ import (
 
 	"github.com/codegangsta/negroni"
 	"github.com/containous/traefik/types"
+	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
+	dto "github.com/prometheus/client_model/go"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestPrometheus(t *testing.T) {
+	metricsFamily, err := prometheus.DefaultGatherer.Gather()
+	if err != nil {
+		t.Fatalf("could not gather metrics family: %s", err)
+	}
+	initialMetricsFamilyCount := len(metricsFamily)
+
 	recorder := httptest.NewRecorder()
 
 	n := negroni.New()
@@ -42,6 +51,80 @@ func TestPrometheus(t *testing.T) {
 		t.Errorf("body does not contain request total entry '%s'", reqsName)
 	}
 	if !strings.Contains(body, latencyName) {
-		t.Errorf("body does not contain request duration entry '%s'", reqsName)
+		t.Errorf("body does not contain request duration entry '%s'", latencyName)
+	}
+
+	// Register the same metrics again
+	metricsMiddlewareBackend = NewMetricsWrapper(NewPrometheus("test", &types.Prometheus{}))
+	n = negroni.New()
+	n.Use(metricsMiddlewareBackend)
+	n.UseHandler(r)
+
+	n.ServeHTTP(recorder, req2)
+
+	metricsFamily, err = prometheus.DefaultGatherer.Gather()
+	if err != nil {
+		t.Fatalf("could not gather metrics family: %s", err)
+	}
+
+	tests := []struct {
+		name   string
+		labels map[string]string
+		assert func(*dto.MetricFamily)
+	}{
+		{
+			name: reqsName,
+			labels: map[string]string{
+				"code":    "200",
+				"method":  "GET",
+				"service": "test",
+			},
+			assert: func(family *dto.MetricFamily) {
+				cv := uint(family.Metric[0].Counter.GetValue())
+				if cv != 3 {
+					t.Errorf("gathered metrics do not contain correct value for total requests, got %d", cv)
+				}
+			},
+		},
+		{
+			name: latencyName,
+			labels: map[string]string{
+				"service": "test",
+			},
+			assert: func(family *dto.MetricFamily) {
+				sc := family.Metric[0].Histogram.GetSampleCount()
+				if sc != 3 {
+					t.Errorf("gathered metrics do not contain correct sample count for request duration, got %d", sc)
+				}
+			},
+		},
+	}
+
+	assert.Equal(t, len(tests), len(metricsFamily)-initialMetricsFamilyCount, "gathered traefic metrics count does not match tests count")
+
+	for _, test := range tests {
+		family := findMetricFamily(test.name, metricsFamily)
+		if family == nil {
+			t.Errorf("gathered metrics do not contain '%s'", test.name)
+			continue
+		}
+		for _, label := range family.Metric[0].Label {
+			val, ok := test.labels[*label.Name]
+			if !ok {
+				t.Errorf("'%s' metric contains unexpected label '%s'", test.name, label)
+			} else if val != *label.Value {
+				t.Errorf("label '%s' in metric '%s' has wrong value '%s'", label, test.name, *label.Value)
+			}
+		}
+		test.assert(family)
 	}
 }
+
+func findMetricFamily(name string, families []*dto.MetricFamily) *dto.MetricFamily {
+	for _, family := range families {
+		if family.GetName() == name {
+			return family
+		}
+	}
+	return nil
+}