fix: deduplicate prom labels in metric generator registry again (#5819) (#5822)

* fix: deduplicate prom labels in metric generator registry again

* handle nil case

Author: javiermolinar

modules/generator/registry/counter.go

@@ -102,15 +102,8 @@ func (c *counter) Inc(labelValueCombo *LabelValueCombo, value float64) {
 }
 
 func (c *counter) newSeries(labelValueCombo *LabelValueCombo, value float64) *counterSeries {
-	lb := labels.NewBuilder(getLabelsFromValueCombo(labelValueCombo))
-	for name, value := range c.externalLabels {
-		lb.Set(name, value)
-	}
-
-	lb.Set(labels.MetricName, c.metricName)
-
 	return &counterSeries{
-		labels:      lb.Labels(),
+		labels:      getSeriesLabels(c.metricName, labelValueCombo, c.externalLabels),
 		value:       atomic.NewFloat64(value),
 		lastUpdated: atomic.NewInt64(time.Now().UnixMilli()),
 		firstSeries: atomic.NewBool(true),

modules/generator/registry/gauge.go

@@ -114,15 +114,8 @@ func (g *gauge) updateSeries(labelValueCombo *LabelValueCombo, value float64, op
 }
 
 func (g *gauge) newSeries(labelValueCombo *LabelValueCombo, value float64) *gaugeSeries {
-	lb := labels.NewBuilder(getLabelsFromValueCombo(labelValueCombo))
-	for name, value := range g.externalLabels {
-		lb.Set(name, value)
-	}
-
-	lb.Set(labels.MetricName, g.metricName)
-
 	return &gaugeSeries{
-		labels:      lb.Labels(),
+		labels:      getSeriesLabels(g.metricName, labelValueCombo, g.externalLabels),
 		value:       atomic.NewFloat64(value),
 		lastUpdated: atomic.NewInt64(time.Now().UnixMilli()),
 	}

modules/generator/registry/histogram.go

@@ -146,10 +146,7 @@ func (h *histogram) newSeries(labelValueCombo *LabelValueCombo, value float64, t
 	// Precompute all labels for all sub-metrics upfront
 
 	// Create and populate label builder
-	lb := labels.NewBuilder(getLabelsFromValueCombo(labelValueCombo))
-	for name, value := range h.externalLabels {
-		lb.Set(name, value)
-	}
+	lb := newSeriesLabelsBuilder(labelValueCombo, h.externalLabels)
 
 	// _count
 	lb.Set(labels.MetricName, h.nameCount)

modules/generator/registry/native_histogram.go

@@ -188,11 +188,7 @@ func (h *nativeHistogram) newSeries(labelValueCombo *LabelValueCombo, value floa
 
 	h.updateSeries(newSeries, value, traceID, multiplier)
 
-	lb := labels.NewBuilder(getLabelsFromValueCombo(labelValueCombo))
-	// set external labels
-	for name, value := range h.externalLabels {
-		lb.Set(name, value)
-	}
+	lb := newSeriesLabelsBuilder(labelValueCombo, h.externalLabels)
 
 	lb.Set(labels.MetricName, h.metricName)
 

modules/generator/registry/registry_test.go

@@ -66,7 +66,7 @@ func TestManagedRegistry_counter(t *testing.T) {
 
 	counter := registry.NewCounter("my_counter")
 
-	counter.Inc(newLabelValueCombo([]string{"label"}, []string{"value-1"}), 1.0)
+	counter.Inc(newLabelValueCombo([]string{"label", "label"}, []string{"repeated-value", "value-1"}), 1.0)
 
 	expectedSamples := []sample{
 		newSample(map[string]string{"__name__": "my_counter", "label": "value-1", "__metrics_gen_instance": mustGetHostname()}, 0, 0.0),
@@ -304,6 +304,14 @@ func collectRegistryMetricsAndAssert(t *testing.T, r *ManagedRegistry, appender
 	collectionTimeMs := time.Now().UnixMilli()
 	r.CollectMetrics(context.Background())
 
+	// Validate that there are no duplicate label names in any sample
+	for i, sample := range appender.samples {
+		if duplicateLabel, hasDuplicate := sample.l.HasDuplicateLabelNames(); hasDuplicate {
+			t.Errorf("Sample %d has duplicate label name %q. Full labels: %v",
+				i, duplicateLabel, sample.l)
+		}
+	}
+
 	// Ignore the collection time on expected samples, since we won't know when the collection will actually take place.
 	for i := range expectedSamples {
 		expectedSamples[i].t = collectionTimeMs

modules/generator/registry/test.go

@@ -215,3 +215,12 @@ func (t *testHistogram) countActiveSeries() int {
 // countSeriesDemand is a stub to satisfy optional estimator usage in registry.
 // Test registry does not track estimates, so return 0.
 func (t *testHistogram) countSeriesDemand() int { return 0 }
+
+func getLabelsFromValueCombo(labelValueCombo *LabelValueCombo) labels.Labels {
+	lbls := labelValueCombo.getLabelPair()
+	lb := make([]labels.Label, len(lbls.names))
+	for i := range lbls.names {
+		lb[i] = labels.Label{Name: lbls.names[i], Value: lbls.values[i]}
+	}
+	return labels.New(lb...)
+}

modules/generator/registry/util.go

@@ -2,11 +2,23 @@ package registry
 
 import "github.com/prometheus/prometheus/model/labels"
 
-func getLabelsFromValueCombo(labelValueCombo *LabelValueCombo) labels.Labels {
-	lbls := labelValueCombo.getLabelPair()
-	lb := make([]labels.Label, len(lbls.names))
-	for i := range lbls.names {
-		lb[i] = labels.Label{Name: lbls.names[i], Value: lbls.values[i]}
+// newSeriesLabelsBuilder creates a labels builder with user labels and external labels pre-populated.
+func newSeriesLabelsBuilder(labelValueCombo *LabelValueCombo, externalLabels map[string]string) *labels.Builder {
+	builder := labels.NewBuilder(labels.Labels{})
+	if labelValueCombo != nil {
+		for i := range labelValueCombo.labels.names {
+			builder.Set(labelValueCombo.labels.names[i], labelValueCombo.labels.values[i])
+		}
 	}
-	return labels.New(lb...)
+	for name, value := range externalLabels {
+		builder.Set(name, value)
+	}
+	return builder
+}
+
+// Returns the labels for the metric series including external labels
+func getSeriesLabels(metricName string, labelValueCombo *LabelValueCombo, externalLabels map[string]string) labels.Labels {
+	builder := newSeriesLabelsBuilder(labelValueCombo, externalLabels)
+	builder.Set(labels.MetricName, metricName)
+	return builder.Labels()
 }