From 377bce14a142bc1647c5a52b69ed7b1c9eeabbf2 Mon Sep 17 00:00:00 2001 From: Martti T Date: Thu, 28 Jul 2022 08:08:09 +0300 Subject: [PATCH] Revert "feat(prometheus): Configurable Registerer" This reverts commit 18f2b6677a06424ef87e30e619c6ad5a37d25c1a. --- prometheus/prometheus.go | 4 +--- prometheus/prometheus_test.go | 27 +++++++++++++++++---------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/prometheus/prometheus.go b/prometheus/prometheus.go index df64296..673aeff 100644 --- a/prometheus/prometheus.go +++ b/prometheus/prometheus.go @@ -144,7 +144,6 @@ type Prometheus struct { MetricsPath string Subsystem string Skipper middleware.Skipper - Registerer prometheus.Registerer RequestCounterURLLabelMappingFunc RequestCounterLabelMappingFunc RequestCounterHostLabelMappingFunc RequestCounterLabelMappingFunc @@ -193,7 +192,6 @@ func NewPrometheus(subsystem string, skipper middleware.Skipper, customMetricsLi RequestCounterHostLabelMappingFunc: func(c echo.Context) string { return c.Request().Host }, - Registerer: prometheus.DefaultRegisterer, } p.registerMetrics(subsystem) @@ -369,7 +367,7 @@ func (p *Prometheus) registerMetrics(subsystem string) { for _, metricDef := range p.MetricsList { metric := NewMetric(metricDef, subsystem) - if err := p.Registerer.Register(metric); err != nil { + if err := prometheus.Register(metric); err != nil { log.Errorf("%s could not be registered in Prometheus: %v", metricDef.Name, err) } switch metricDef { diff --git a/prometheus/prometheus_test.go b/prometheus/prometheus_test.go index fedca62..caed17b 100644 --- a/prometheus/prometheus_test.go +++ b/prometheus/prometheus_test.go @@ -12,21 +12,27 @@ import ( "github.com/stretchr/testify/assert" ) +func unregister(p *Prometheus) { + prometheus.Unregister(p.reqCnt) + prometheus.Unregister(p.reqDur) + prometheus.Unregister(p.reqSz) + prometheus.Unregister(p.resSz) +} + func TestPrometheus_Use(t *testing.T) { e := echo.New() p := NewPrometheus("echo", nil) - p.Registerer = prometheus.NewRegistry() p.Use(e) assert.Equal(t, 1, len(e.Routes()), "only one route should be added") assert.NotNil(t, e, "the engine should not be empty") assert.Equal(t, e.Routes()[0].Path, p.MetricsPath, "the path should match the metrics path") + unregister(p) } func TestPrometheus_Buckets(t *testing.T) { e := echo.New() p := NewPrometheus("echo", nil) - p.Registerer = prometheus.NewRegistry() p.Use(e) path := "/ping" @@ -45,24 +51,24 @@ func TestPrometheus_Buckets(t *testing.T) { assert.NotRegexp(t, "request_size_bytes.*le=\"0.005\"", r.Body.String(), "request should NOT have time bucket (like, 0.005s)") }) + unregister(p) } func TestPath(t *testing.T) { p := NewPrometheus("echo", nil) - p.Registerer = prometheus.NewRegistry() assert.Equal(t, p.MetricsPath, defaultMetricPath, "no usage of path should yield default path") + unregister(p) } func TestSubsystem(t *testing.T) { p := NewPrometheus("echo", nil) - p.Registerer = prometheus.NewRegistry() assert.Equal(t, p.Subsystem, "echo", "subsystem should be default") + unregister(p) } func TestUse(t *testing.T) { e := echo.New() p := NewPrometheus("echo", nil) - p.Registerer = prometheus.NewRegistry() g := gofight.New() g.GET(p.MetricsPath).Run(e, func(r gofight.HTTPResponse, rq gofight.HTTPRequest) { @@ -74,6 +80,7 @@ func TestUse(t *testing.T) { g.GET(p.MetricsPath).Run(e, func(r gofight.HTTPResponse, rq gofight.HTTPRequest) { assert.Equal(t, http.StatusOK, r.Code) }) + unregister(p) } func TestIgnore(t *testing.T) { @@ -88,7 +95,6 @@ func TestIgnore(t *testing.T) { return false } p := NewPrometheus("echo", ignore) - p.Registerer = prometheus.NewRegistry() p.Use(e) g := gofight.New() @@ -105,12 +111,12 @@ func TestIgnore(t *testing.T) { assert.NotContains(t, r.Body.String(), fmt.Sprintf("%s_requests_total", p.Subsystem)) assert.NotContains(t, r.Body.String(), lipath, "ignored path must not be present") }) + unregister(p) } func TestMetricsGenerated(t *testing.T) { e := echo.New() p := NewPrometheus("echo", nil) - p.Registerer = prometheus.NewRegistry() p.Use(e) path := "/ping" @@ -124,12 +130,12 @@ func TestMetricsGenerated(t *testing.T) { assert.Contains(t, r.Body.String(), fmt.Sprintf("%s_requests_total", p.Subsystem)) assert.Contains(t, r.Body.String(), lpath, "path must be present") }) + unregister(p) } func TestMetricsPathIgnored(t *testing.T) { e := echo.New() p := NewPrometheus("echo", nil) - p.Registerer = prometheus.NewRegistry() p.Use(e) g := gofight.New() @@ -137,12 +143,12 @@ func TestMetricsPathIgnored(t *testing.T) { assert.Equal(t, http.StatusOK, r.Code) assert.NotContains(t, r.Body.String(), fmt.Sprintf("%s_requests_total", p.Subsystem)) }) + unregister(p) } func TestMetricsPushGateway(t *testing.T) { e := echo.New() p := NewPrometheus("echo", nil) - p.Registerer = prometheus.NewRegistry() p.Use(e) g := gofight.New() @@ -150,12 +156,12 @@ func TestMetricsPushGateway(t *testing.T) { assert.Equal(t, http.StatusOK, r.Code) assert.NotContains(t, r.Body.String(), fmt.Sprintf("%s_request_duration", p.Subsystem)) }) + unregister(p) } func TestMetricsForErrors(t *testing.T) { e := echo.New() p := NewPrometheus("echo", nil) - p.Registerer = prometheus.NewRegistry() p.Use(e) e.GET("/handler_for_ok", func(c echo.Context) error { @@ -184,4 +190,5 @@ func TestMetricsForErrors(t *testing.T) { assert.Contains(t, body, `echo_requests_total{code="409",host="",method="GET",url="/handler_for_nok"} 2`) assert.Contains(t, body, `echo_requests_total{code="502",host="",method="GET",url="/handler_for_error"} 1`) }) + unregister(p) }