Avoid a panic during Prometheus registering

metrics/prometheus.go

@@ -7,6 +7,7 @@ import (
 	"sync"
 
 	"github.com/containous/mux"
+	"github.com/containous/traefik/log"
 	"github.com/containous/traefik/safe"
 	"github.com/containous/traefik/types"
 	"github.com/go-kit/kit/metrics"
@@ -15,25 +16,31 @@ import (
 )
 
 const (
-	metricNamePrefix = "traefik_"
+	// MetricNamePrefix prefix of all metric names
+	MetricNamePrefix = "traefik_"
 
 	// server meta information
-	configReloadsTotalName         = metricNamePrefix + "config_reloads_total"
-	configReloadsFailuresTotalName = metricNamePrefix + "config_reloads_failure_total"
-	configLastReloadSuccessName    = metricNamePrefix + "config_last_reload_success"
-	configLastReloadFailureName    = metricNamePrefix + "config_last_reload_failure"
+	metricConfigPrefix             = MetricNamePrefix + "config_"
+	configReloadsTotalName         = metricConfigPrefix + "reloads_total"
+	configReloadsFailuresTotalName = metricConfigPrefix + "reloads_failure_total"
+	configLastReloadSuccessName    = metricConfigPrefix + "last_reload_success"
+	configLastReloadFailureName    = metricConfigPrefix + "last_reload_failure"
 
 	// entrypoint
-	entrypointReqsTotalName   = metricNamePrefix + "entrypoint_requests_total"
-	entrypointReqDurationName = metricNamePrefix + "entrypoint_request_duration_seconds"
-	entrypointOpenConnsName   = metricNamePrefix + "entrypoint_open_connections"
+	metricEntryPointPrefix    = MetricNamePrefix + "entrypoint_"
+	entrypointReqsTotalName   = metricEntryPointPrefix + "requests_total"
+	entrypointReqDurationName = metricEntryPointPrefix + "request_duration_seconds"
+	entrypointOpenConnsName   = metricEntryPointPrefix + "open_connections"
 
-	// backend level
-	backendReqsTotalName    = metricNamePrefix + "backend_requests_total"
-	backendReqDurationName  = metricNamePrefix + "backend_request_duration_seconds"
-	backendOpenConnsName    = metricNamePrefix + "backend_open_connections"
-	backendRetriesTotalName = metricNamePrefix + "backend_retries_total"
-	backendServerUpName     = metricNamePrefix + "backend_server_up"
+	// backend level.
+
+	// MetricBackendPrefix prefix of all backend metric names
+	MetricBackendPrefix     = MetricNamePrefix + "backend_"
+	backendReqsTotalName    = MetricBackendPrefix + "requests_total"
+	backendReqDurationName  = MetricBackendPrefix + "request_duration_seconds"
+	backendOpenConnsName    = MetricBackendPrefix + "open_connections"
+	backendRetriesTotalName = MetricBackendPrefix + "retries_total"
+	backendServerUpName     = MetricBackendPrefix + "server_up"
 )
 
 // promState holds all metric state internally and acts as the only Collector we register for Prometheus.
@@ -61,6 +68,16 @@ func (h PrometheusHandler) AddRoutes(router *mux.Router) {
 // RegisterPrometheus registers all Prometheus metrics.
 // It must be called only once and failing to register the metrics will lead to a panic.
 func RegisterPrometheus(config *types.Prometheus) Registry {
+	standardRegistry := initStandardRegistry(config)
+
+	if !registerPromState() {
+		return nil
+	}
+
+	return standardRegistry
+}
+
+func initStandardRegistry(config *types.Prometheus) Registry {
 	buckets := []float64{0.1, 0.3, 1.2, 5.0}
 	if config.Buckets != nil {
 		buckets = config.Buckets
@@ -137,7 +154,6 @@ func RegisterPrometheus(config *types.Prometheus) Registry {
 		backendRetries.cv.Describe,
 		backendServerUp.gv.Describe,
 	}
-	stdprometheus.MustRegister(promState)
 
 	return &standardRegistry{
 		enabled:                        true,
@@ -156,6 +172,17 @@ func RegisterPrometheus(config *types.Prometheus) Registry {
 	}
 }
 
+func registerPromState() bool {
+	if err := stdprometheus.Register(promState); err != nil {
+		if _, ok := err.(stdprometheus.AlreadyRegisteredError); !ok {
+			log.Errorf("Unable to register Traefik to Prometheus: %v", err)
+			return false
+		}
+		log.Debug("Prometheus collector already registered.")
+	}
+	return true
+}
+
 // OnConfigurationUpdate receives the current configuration from Traefik.
 // It then converts the configuration to the optimized package internal format
 // and sets it to the promState.

metrics/prometheus_test.go

@@ -11,8 +11,82 @@ import (
 	"github.com/containous/traefik/types"
 	"github.com/prometheus/client_golang/prometheus"
 	dto "github.com/prometheus/client_model/go"
+	"github.com/stretchr/testify/assert"
 )
 
+func TestRegisterPromState(t *testing.T) {
+	// Reset state of global promState.
+	defer promState.reset()
+
+	testCases := []struct {
+		desc                 string
+		prometheusSlice      []*types.Prometheus
+		initPromState        bool
+		unregisterPromState  bool
+		expectedNbRegistries int
+	}{
+		{
+			desc:                 "Register once",
+			prometheusSlice:      []*types.Prometheus{{}},
+			expectedNbRegistries: 1,
+			initPromState:        true,
+		},
+		{
+			desc:                 "Register once with no promState init",
+			prometheusSlice:      []*types.Prometheus{{}},
+			expectedNbRegistries: 0,
+		},
+		{
+			desc:                 "Register twice",
+			prometheusSlice:      []*types.Prometheus{{}, {}},
+			expectedNbRegistries: 2,
+			initPromState:        true,
+		},
+		{
+			desc:                 "Register twice with no promstate init",
+			prometheusSlice:      []*types.Prometheus{{}, {}},
+			expectedNbRegistries: 0,
+		},
+		{
+			desc:                 "Register twice with unregister",
+			prometheusSlice:      []*types.Prometheus{{}, {}},
+			unregisterPromState:  true,
+			expectedNbRegistries: 2,
+			initPromState:        true,
+		},
+		{
+			desc:                 "Register twice with unregister but no promstate init",
+			prometheusSlice:      []*types.Prometheus{{}, {}},
+			unregisterPromState:  true,
+			expectedNbRegistries: 0,
+		},
+	}
+
+	for _, test := range testCases {
+		actualNbRegistries := 0
+		for _, prom := range test.prometheusSlice {
+			if test.initPromState {
+				initStandardRegistry(prom)
+			}
+
+			promReg := registerPromState()
+			if promReg != false {
+				actualNbRegistries++
+			}
+
+			if test.unregisterPromState {
+				prometheus.Unregister(promState)
+			}
+
+			promState.reset()
+		}
+
+		prometheus.Unregister(promState)
+
+		assert.Equal(t, test.expectedNbRegistries, actualNbRegistries)
+	}
+}
+
 func TestPrometheus(t *testing.T) {
 	// Reset state of global promState.
 	defer promState.reset()

server/server.go

@@ -652,8 +652,11 @@ func registerMetricClients(metricsConfig *types.Metrics) metrics.Registry {
 
 	var registries []metrics.Registry
 	if metricsConfig.Prometheus != nil {
-		registries = append(registries, metrics.RegisterPrometheus(metricsConfig.Prometheus))
-		log.Debug("Configured Prometheus metrics")
+		prometheusRegister := metrics.RegisterPrometheus(metricsConfig.Prometheus)
+		if prometheusRegister != nil {
+			registries = append(registries, prometheusRegister)
+			log.Debug("Configured Prometheus metrics")
+		}
 	}
 	if metricsConfig.Datadog != nil {
 		registries = append(registries, metrics.RegisterDatadog(metricsConfig.Datadog))