From 5072879dca11dccc5ee3a967c162d2cca3fe238d Mon Sep 17 00:00:00 2001
From: Ben Reedy <breed808@breed808.com>
Date: Sun, 16 May 2021 12:29:09 +1000
Subject: [PATCH] Check duplicates across entire textfile set

Check all textfile metrics will be checked for duplicates. If duplicates
are detected, drop all metrics and log error.

Signed-off-by: Ben Reedy <breed808@breed808.com>
---
 collector/textfile.go      | 28 +++++++++++++++++-----------
 collector/textfile_test.go | 11 +++++++----
 2 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/collector/textfile.go b/collector/textfile.go
index 63c55c0e..7be1a016 100644
--- a/collector/textfile.go
+++ b/collector/textfile.go
@@ -66,9 +66,9 @@ func NewTextFileCollector() (Collector, error) {
 	}, nil
 }
 
-// Given a metric family, determine if any two entries are duplicates.
+// Given a slice of metric families, determine if any two entries are duplicates.
 // Duplicates will be detected where the metric name, labels and label values are identical.
-func duplicateMetricEntry(metricFamilies map[string]*dto.MetricFamily) bool {
+func duplicateMetricEntry(metricFamilies []*dto.MetricFamily) bool {
 	uniqueMetrics := make(map[string]map[string]string)
 	for _, metricFamily := range metricFamilies {
 		metric_name := *metricFamily.Name
@@ -249,6 +249,10 @@ func (c *textFileCollector) Collect(ctx *ScrapeContext, ch chan<- prometheus.Met
 		error = 1.0
 	}
 
+	// Create empty metricFamily slice here and append parsedFamilies to it inside the loop.
+	// Once loop is complete, raise error if any duplicates are present.
+	// This will ensure that duplicate metrics are correctly detected between multiple .prom files.
+	var metricFamilies = []*dto.MetricFamily{}
 fileLoop:
 	for _, f := range files {
 		if !strings.HasSuffix(f.Name(), ".prom") {
@@ -297,18 +301,20 @@ fileLoop:
 		// a failure does not appear fresh.
 		mtimes[f.Name()] = f.ModTime()
 
-		if duplicateMetricEntry(parsedFamilies) {
-			log.Errorf("Duplicate metrics detected in file: %q", path)
-			error = 1.0
-			continue
-		}
-
-		for _, mf := range parsedFamilies {
-			convertMetricFamily(mf, ch)
+		for _, metricFamily := range parsedFamilies {
+			metricFamilies = append(metricFamilies, metricFamily)
 		}
 	}
 
-	c.exportMTimes(mtimes, ch)
+	if duplicateMetricEntry(metricFamilies) {
+		log.Errorf("Duplicate metrics detected in files")
+		error = 1.0
+	} else {
+		for _, mf := range metricFamilies {
+			convertMetricFamily(mf, ch)
+			c.exportMTimes(mtimes, ch)
+		}
+	}
 
 	// Export if there were errors.
 	ch <- prometheus.MustNewConstMetric(
diff --git a/collector/textfile_test.go b/collector/textfile_test.go
index 26e470ec..9fbed55f 100644
--- a/collector/textfile_test.go
+++ b/collector/textfile_test.go
@@ -92,8 +92,8 @@ func TestDuplicateMetricEntry(t *testing.T) {
 		Metric: []*dto.Metric{&metric1, &metric2},
 	}
 
-	duplicateFamily := make(map[string]*dto.MetricFamily)
-	duplicateFamily["test"] = &duplicate
+	duplicateFamily := []*dto.MetricFamily{}
+	duplicateFamily = append(duplicateFamily, &duplicate)
 
 	// Ensure detection for duplicate metrics
 	if !duplicateMetricEntry(duplicateFamily) {
@@ -118,7 +118,9 @@ func TestDuplicateMetricEntry(t *testing.T) {
 		Type:   &metric_type,
 		Metric: []*dto.Metric{&metric1, &metric3},
 	}
-	duplicateFamily["test"] = &differentLabels
+
+	duplicateFamily = []*dto.MetricFamily{}
+	duplicateFamily = append(duplicateFamily, &differentLabels)
 
 	// Additional label on second metric should not be cause for duplicate detection
 	if duplicateMetricEntry(duplicateFamily) {
@@ -142,7 +144,8 @@ func TestDuplicateMetricEntry(t *testing.T) {
 		Type:   &metric_type,
 		Metric: []*dto.Metric{&metric3, &metric4},
 	}
-	duplicateFamily["test"] = &differentValues
+	duplicateFamily = []*dto.MetricFamily{}
+	duplicateFamily = append(duplicateFamily, &differentValues)
 
 	// Additional label with different values metric should not be cause for duplicate detection
 	if duplicateMetricEntry(duplicateFamily) {