Handle case of duplicate labels in metrics

pkg/promutil/migrate.go

@@ -290,13 +290,19 @@ func recordLabelsForMetric(metricName string, labelKeys []string, observedMetric
 // EnsureLabelConsistencyAndRemoveDuplicates ensures that every metric has the same set of labels based on the data
 // in observedMetricLabels and that there are no duplicate metrics.
 // Prometheus requires that all metrics with the same name have the same set of labels and that no duplicates are registered
-func EnsureLabelConsistencyAndRemoveDuplicates(metrics []*PrometheusMetric, observedMetricLabels map[string]model.LabelSet) []*PrometheusMetric {
+func EnsureLabelConsistencyAndRemoveDuplicates(metrics []*PrometheusMetric, observedMetricLabels map[string]model.LabelSet, logger logging.Logger) []*PrometheusMetric {
 	metricKeys := make(map[string]struct{}, len(metrics))
 	output := make([]*PrometheusMetric, 0, len(metrics))
 
 	for _, metric := range metrics {
-		for observedLabels := range observedMetricLabels[metric.Name()] {
-			metric.AddIfMissingLabelPair(observedLabels, "")
+		observedLabels := observedMetricLabels[metric.Name()]
+		for label := range observedLabels {
+			metric.AddIfMissingLabelPair(label, "")
+		}
+
+		if len(observedLabels) != metric.LabelsLen() {
+			logger.Warn("metric has duplicate labels", "metric_name", metric.Name(), "observed_labels", len(observedLabels), "labels_len", metric.LabelsLen())
+			metric.RemoveDuplicateLabels()
 		}
 
 		metricKey := metric.Name() + "-" + strconv.FormatUint(metric.LabelsSignature(), 10)

pkg/promutil/migrate_test.go

@@ -1289,13 +1289,23 @@ func Test_EnsureLabelConsistencyAndRemoveDuplicates(t *testing.T) {
 				NewPrometheusMetric("metric1", []string{"label1", "label2", "label3"}, []string{"", "", ""}, 3.0),
 			},
 		},
+		{
+			name: "removes duplicate labels",
+			metrics: []*PrometheusMetric{
+				NewPrometheusMetric("metric1", []string{"label1", "label1", "label2"}, []string{"value1", "value1", "value2"}, 1.0),
+			},
+			observedLabels: map[string]model.LabelSet{"metric1": {"label1": {}, "label2": {}}},
+			output: []*PrometheusMetric{
+				NewPrometheusMetric("metric1", []string{"label1", "label2"}, []string{"value1", "value2"}, 1.0),
+			},
+		},
 		{
 			name: "duplicate metric",
 			metrics: []*PrometheusMetric{
 				NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
 				NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
 			},
-			observedLabels: map[string]model.LabelSet{},
+			observedLabels: map[string]model.LabelSet{"metric1": {"label1": {}}},
 			output: []*PrometheusMetric{
 				NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
 			},
@@ -1306,7 +1316,7 @@ func Test_EnsureLabelConsistencyAndRemoveDuplicates(t *testing.T) {
 				NewPrometheusMetric("metric1", []string{"label1", "label2"}, []string{"value1", "value2"}, 1.0),
 				NewPrometheusMetric("metric1", []string{"label2", "label1"}, []string{"value2", "value1"}, 1.0),
 			},
-			observedLabels: map[string]model.LabelSet{},
+			observedLabels: map[string]model.LabelSet{"metric1": {"label1": {}, "label2": {}}},
 			output: []*PrometheusMetric{
 				NewPrometheusMetric("metric1", []string{"label1", "label2"}, []string{"value1", "value2"}, 1.0),
 			},
@@ -1317,10 +1327,10 @@ func Test_EnsureLabelConsistencyAndRemoveDuplicates(t *testing.T) {
 				NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
 				NewPrometheusMetric("metric1", []string{"label2"}, []string{"value2"}, 1.0),
 			},
-			observedLabels: map[string]model.LabelSet{},
+			observedLabels: map[string]model.LabelSet{"metric1": {"label1": {}, "label2": {}}},
 			output: []*PrometheusMetric{
-				NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
-				NewPrometheusMetric("metric1", []string{"label2"}, []string{"value2"}, 1.0),
+				NewPrometheusMetric("metric1", []string{"label1", "label2"}, []string{"value1", ""}, 1.0),
+				NewPrometheusMetric("metric1", []string{"label1", "label2"}, []string{"", "value2"}, 1.0),
 			},
 		},
 		{
@@ -1329,7 +1339,7 @@ func Test_EnsureLabelConsistencyAndRemoveDuplicates(t *testing.T) {
 				NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
 				NewPrometheusMetric("metric2", []string{"label1"}, []string{"value1"}, 1.0),
 			},
-			observedLabels: map[string]model.LabelSet{},
+			observedLabels: map[string]model.LabelSet{"metric1": {"label1": {}}, "metric2": {"label1": {}}},
 			output: []*PrometheusMetric{
 				NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
 				NewPrometheusMetric("metric2", []string{"label1"}, []string{"value1"}, 1.0),
@@ -1341,7 +1351,7 @@ func Test_EnsureLabelConsistencyAndRemoveDuplicates(t *testing.T) {
 				NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
 				NewPrometheusMetric("metric2", []string{"label2"}, []string{"value2"}, 1.0),
 			},
-			observedLabels: map[string]model.LabelSet{},
+			observedLabels: map[string]model.LabelSet{"metric1": {"label1": {}}, "metric2": {"label2": {}}},
 			output: []*PrometheusMetric{
 				NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
 				NewPrometheusMetric("metric2", []string{"label2"}, []string{"value2"}, 1.0),
@@ -1356,18 +1366,18 @@ func Test_EnsureLabelConsistencyAndRemoveDuplicates(t *testing.T) {
 				NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
 				NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
 			},
-			observedLabels: map[string]model.LabelSet{},
+			observedLabels: map[string]model.LabelSet{"metric1": {"label1": {}}, "metric2": {"label1": {}, "label2": {}}},
 			output: []*PrometheusMetric{
-				NewPrometheusMetric("metric2", []string{"label2"}, []string{"value2"}, 1.0),
-				NewPrometheusMetric("metric2", []string{"label1"}, []string{"value1"}, 1.0),
+				NewPrometheusMetric("metric2", []string{"label1", "label2"}, []string{"", "value2"}, 1.0),
+				NewPrometheusMetric("metric2", []string{"label1", "label2"}, []string{"value1", ""}, 1.0),
 				NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
 			},
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			actual := EnsureLabelConsistencyAndRemoveDuplicates(tc.metrics, tc.observedLabels)
+			actual := EnsureLabelConsistencyAndRemoveDuplicates(tc.metrics, tc.observedLabels, logging.NewNopLogger())
 			require.ElementsMatch(t, tc.output, actual)
 		})
 	}
@@ -1381,14 +1391,15 @@ func Benchmark_EnsureLabelConsistencyAndRemoveDuplicates(b *testing.B) {
 		NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
 	}
 	observedLabels := map[string]model.LabelSet{"metric1": {"label1": {}, "label2": {}, "label3": {}}}
+	logger := logging.NewNopLogger()
 
 	var output []*PrometheusMetric
 
 	b.ReportAllocs()
 	b.ResetTimer()
 
 	for i := 0; i < b.N; i++ {
-		output = EnsureLabelConsistencyAndRemoveDuplicates(metrics, observedLabels)
+		output = EnsureLabelConsistencyAndRemoveDuplicates(metrics, observedLabels, logger)
 	}
 
 	expectedOutput := []*PrometheusMetric{

pkg/promutil/prometheus.go

@@ -154,6 +154,10 @@ func (p *PrometheusMetric) Labels() ([]string, []string) {
 	return p.labels.keys, p.labels.vals
 }
 
+func (p *PrometheusMetric) LabelsLen() int {
+	return len(p.labels.keys)
+}
+
 func (p *PrometheusMetric) Value() float64 {
 	return p.value
 }
@@ -199,6 +203,21 @@ func (p *PrometheusMetric) AddIfMissingLabelPair(key, val string) {
 	}
 }
 
+func (p *PrometheusMetric) RemoveDuplicateLabels() {
+	seen := map[string]struct{}{}
+	idx := 0
+	for i, key := range p.labels.keys {
+		if _, ok := seen[key]; !ok {
+			seen[key] = struct{}{}
+			p.labels.keys[idx] = key
+			p.labels.vals[idx] = p.labels.vals[i]
+			idx++
+		}
+	}
+	p.labels.keys = p.labels.keys[:idx]
+	p.labels.vals = p.labels.vals[:idx]
+}
+
 type PrometheusCollector struct {
 	metrics []prometheus.Metric
 }

pkg/promutil/prometheus_test.go

@@ -121,6 +121,47 @@ func TestPromStringTag(t *testing.T) {
 	}
 }
 
+func TestPrometheusMetric(t *testing.T) {
+	t.Run("NewPrometheusMetric panics with wrong label size", func(t *testing.T) {
+		require.Panics(t, func() {
+			NewPrometheusMetric("metric1", []string{"key1"}, []string{}, 1.0)
+		})
+		require.Panics(t, func() {
+			NewPrometheusMetric("metric1", []string{}, []string{"label1"}, 1.0)
+		})
+		require.Panics(t, func() {
+			NewPrometheusMetric("metric1", []string{"key1", "key2"}, []string{"label1"}, 1.0)
+		})
+		require.Panics(t, func() {
+			NewPrometheusMetric("metric1", []string{"key1"}, []string{"label1", "label2"}, 1.0)
+		})
+	})
+
+	t.Run("NewPrometheusMetric sorts labels", func(t *testing.T) {
+		metric := NewPrometheusMetric("metric", []string{"key2", "key1"}, []string{"value2", "value1"}, 1.0)
+		keys, vals := metric.Labels()
+		require.Equal(t, []string{"key1", "key2"}, keys)
+		require.Equal(t, []string{"value1", "value2"}, vals)
+	})
+
+	t.Run("AddIfMissingLabelPair keeps labels sorted", func(t *testing.T) {
+		metric := NewPrometheusMetric("metric", []string{"key2"}, []string{"value2"}, 1.0)
+		metric.AddIfMissingLabelPair("key1", "value1")
+		keys, vals := metric.Labels()
+		require.Equal(t, []string{"key1", "key2"}, keys)
+		require.Equal(t, []string{"value1", "value2"}, vals)
+	})
+
+	t.Run("RemoveDuplicateLabels", func(t *testing.T) {
+		metric := NewPrometheusMetric("metric", []string{"key1", "key1"}, []string{"value1", "value2"}, 1.0)
+		require.Equal(t, 2, metric.LabelsLen())
+		metric.RemoveDuplicateLabels()
+		keys, vals := metric.Labels()
+		require.Equal(t, []string{"key1"}, keys)
+		require.Equal(t, []string{"value1"}, vals)
+	})
+}
+
 func TestNewPrometheusCollector_CanReportMetricsAndErrors(t *testing.T) {
 	metrics := []*PrometheusMetric{
 		NewPrometheusMetric("this*is*not*valid", []string{}, []string{}, 0),