Merge pull request #461 from pedro-stanaka/fix/histogram-and-counters-clash

Counter with histogram suffix will clash with histogram

pkg/exporter/exporter_test.go

@@ -401,6 +401,76 @@ func TestConflictingMetrics(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:     "histogram vs counter with count suffix",
+			expected: []float64{2},
+			in: event.Events{
+				&event.ObserverEvent{
+					OMetricName: "histogram_test1",
+					OValue:      2,
+				},
+				&event.CounterEvent{
+					CMetricName: "histogram_test1_count",
+					CValue:      1,
+				},
+			},
+		},
+		{
+			name:     "histogram vs counter with sum suffix",
+			expected: []float64{2},
+			in: event.Events{
+				&event.ObserverEvent{
+					OMetricName: "histogram_test1",
+					OValue:      2,
+				},
+				&event.CounterEvent{
+					CMetricName: "histogram_test1_sum",
+					CValue:      1,
+				},
+			},
+		},
+		{
+			name:     "histogram vs counter with bucket suffix",
+			expected: []float64{2},
+			in: event.Events{
+				&event.ObserverEvent{
+					OMetricName: "histogram_test1",
+					OValue:      2,
+				},
+				&event.CounterEvent{
+					CMetricName: "histogram_test1_bucket",
+					CValue:      1,
+				},
+			},
+		},
+		{
+			name:     "histogram vs gauge with sum suffix",
+			expected: []float64{2},
+			in: event.Events{
+				&event.ObserverEvent{
+					OMetricName: "histogram_test1",
+					OValue:      2,
+				},
+				&event.GaugeEvent{
+					GMetricName: "histogram_test1_sum",
+					GValue:      1,
+				},
+			},
+		},
+		{
+			name:     "histogram vs gauge with count suffix",
+			expected: []float64{2},
+			in: event.Events{
+				&event.ObserverEvent{
+					OMetricName: "histogram_test1",
+					OValue:      2,
+				},
+				&event.GaugeEvent{
+					GMetricName: "histogram_test1_count",
+					GValue:      1,
+				},
+			},
+		},
 		{
 			name:     "counter vs histogram",
 			expected: []float64{1},
@@ -520,10 +590,11 @@ mappings:
 				events <- s.in
 				close(events)
 			}()
-			ex := NewExporter(prometheus.DefaultRegisterer, testMapper, log.NewNopLogger(), eventsActions, eventsUnmapped, errorEventStats, eventStats, conflictingEventStats, metricsCount)
+			reg := prometheus.NewRegistry()
+			ex := NewExporter(reg, testMapper, log.NewNopLogger(), eventsActions, eventsUnmapped, errorEventStats, eventStats, conflictingEventStats, metricsCount)
 			ex.Listen(events)
 
-			metrics, err := prometheus.DefaultGatherer.Gather()
+			metrics, err := reg.Gather()
 			if err != nil {
 				t.Fatalf("Cannot gather from DefaultGatherer: %v", err)
 			}

pkg/registry/registry.go

@@ -19,6 +19,7 @@ import (
 	"hash"
 	"hash/fnv"
 	"sort"
+	"strings"
 	"time"
 
 	"github.com/prometheus/client_golang/prometheus"
@@ -165,6 +166,11 @@ func (r *Registry) GetCounter(metricName string, labels prometheus.Labels, help
 		return nil, fmt.Errorf("metric with name %s is already registered", metricName)
 	}
 
+	err := r.checkHistogramNameCollision(metricName)
+	if err != nil {
+		return nil, err
+	}
+
 	var counterVec *prometheus.CounterVec
 	if vh == nil {
 		metricsCount.WithLabelValues("counter").Inc()
@@ -181,7 +187,6 @@ func (r *Registry) GetCounter(metricName string, labels prometheus.Labels, help
 	}
 
 	var counter prometheus.Counter
-	var err error
 	if counter, err = counterVec.GetMetricWith(labels); err != nil {
 		return nil, err
 	}
@@ -190,6 +195,18 @@ func (r *Registry) GetCounter(metricName string, labels prometheus.Labels, help
 	return counter, nil
 }
 
+func (r *Registry) checkHistogramNameCollision(metricName string) error {
+	histogramSuffixes := []string{"_bucket", "_count", "_sum"}
+	for _, suffix := range histogramSuffixes {
+		if strings.HasSuffix(metricName, suffix) {
+			if r.MetricConflicts(strings.TrimSuffix(metricName, suffix), metrics.CounterMetricType) {
+				return fmt.Errorf("metric with name %s is already registered", metricName)
+			}
+		}
+	}
+	return nil
+}
+
 func (r *Registry) GetGauge(metricName string, labels prometheus.Labels, help string, mapping *mapper.MetricMapping, metricsCount *prometheus.GaugeVec) (prometheus.Gauge, error) {
 	hash, labelNames := r.HashLabels(labels)
 	vh, mh := r.Get(metricName, hash, metrics.GaugeMetricType)
@@ -201,6 +218,11 @@ func (r *Registry) GetGauge(metricName string, labels prometheus.Labels, help st
 		return nil, fmt.Errorf("metrics.Metric with name %s is already registered", metricName)
 	}
 
+	err := r.checkHistogramNameCollision(metricName)
+	if err != nil {
+		return nil, fmt.Errorf("metrics.Metric with name %s is already registered", metricName)
+	}
+
 	var gaugeVec *prometheus.GaugeVec
 	if vh == nil {
 		metricsCount.WithLabelValues("gauge").Inc()
@@ -217,7 +239,6 @@ func (r *Registry) GetGauge(metricName string, labels prometheus.Labels, help st
 	}
 
 	var gauge prometheus.Gauge
-	var err error
 	if gauge, err = gaugeVec.GetMetricWith(labels); err != nil {
 		return nil, err
 	}