From 4293497b29a73bd5e53fe04ec0c1abf959be6af6 Mon Sep 17 00:00:00 2001
From: Ben Reedy <breed808@breed808.com>
Date: Sun, 18 Apr 2021 13:04:39 +1000
Subject: [PATCH] Fix textfile crashes with duplicate metrics

Signed-off-by: Ben Reedy <breed808@breed808.com>
---
 collector/textfile.go      |  32 ++++++++++++
 collector/textfile_test.go | 104 +++++++++++++++++++++++++++++++++++++
 2 files changed, 136 insertions(+)

diff --git a/collector/textfile.go b/collector/textfile.go
index 767f58bf..63c55c0e 100644
--- a/collector/textfile.go
+++ b/collector/textfile.go
@@ -21,6 +21,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"reflect"
 	"sort"
 	"strings"
 	"time"
@@ -65,6 +66,31 @@ func NewTextFileCollector() (Collector, error) {
 	}, nil
 }
 
+// Given a metric family, 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 {
+	uniqueMetrics := make(map[string]map[string]string)
+	for _, metricFamily := range metricFamilies {
+		metric_name := *metricFamily.Name
+		for _, metric := range metricFamily.Metric {
+			metric_labels := metric.GetLabel()
+			labels := make(map[string]string)
+			for _, label := range metric_labels {
+				labels[label.GetName()] = label.GetValue()
+			}
+			// Check if key is present before appending
+			_, mapContainsKey := uniqueMetrics[metric_name]
+
+			// Duplicate metric found with identical labels & label values
+			if mapContainsKey == true && reflect.DeepEqual(uniqueMetrics[metric_name], labels) {
+				return true
+			}
+			uniqueMetrics[metric_name] = labels
+		}
+	}
+	return false
+}
+
 func convertMetricFamily(metricFamily *dto.MetricFamily, ch chan<- prometheus.Metric) {
 	var valType prometheus.ValueType
 	var val float64
@@ -271,6 +297,12 @@ 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)
 		}
diff --git a/collector/textfile_test.go b/collector/textfile_test.go
index d3717146..26e470ec 100644
--- a/collector/textfile_test.go
+++ b/collector/textfile_test.go
@@ -5,6 +5,8 @@ import (
 	"io/ioutil"
 	"strings"
 	"testing"
+
+	dto "github.com/prometheus/client_model/go"
 )
 
 func TestCRFilter(t *testing.T) {
@@ -45,3 +47,105 @@ func TestCheckBOM(t *testing.T) {
 		}
 	}
 }
+
+func TestDuplicateMetricEntry(t *testing.T) {
+	metric_name := "windows_sometest"
+	metric_help := "This is a Test."
+	metric_type := dto.MetricType_GAUGE
+
+	gauge_value := 1.0
+
+	gauge := dto.Gauge{
+		Value: &gauge_value,
+	}
+
+	label1_name := "display_name"
+	label1_value := "foobar"
+
+	label1 := dto.LabelPair{
+		Name:  &label1_name,
+		Value: &label1_value,
+	}
+
+	label2_name := "display_version"
+	label2_value := "13.4.0"
+
+	label2 := dto.LabelPair{
+		Name:  &label2_name,
+		Value: &label2_value,
+	}
+
+	metric1 := dto.Metric{
+		Label: []*dto.LabelPair{&label1, &label2},
+		Gauge: &gauge,
+	}
+
+	metric2 := dto.Metric{
+		Label: []*dto.LabelPair{&label1, &label2},
+		Gauge: &gauge,
+	}
+
+	duplicate := dto.MetricFamily{
+		Name:   &metric_name,
+		Help:   &metric_help,
+		Type:   &metric_type,
+		Metric: []*dto.Metric{&metric1, &metric2},
+	}
+
+	duplicateFamily := make(map[string]*dto.MetricFamily)
+	duplicateFamily["test"] = &duplicate
+
+	// Ensure detection for duplicate metrics
+	if !duplicateMetricEntry(duplicateFamily) {
+		t.Errorf("Duplicate not found in duplicateFamily")
+	}
+
+	label3_name := "test"
+	label3_value := "1.0"
+
+	label3 := dto.LabelPair{
+		Name:  &label3_name,
+		Value: &label3_value,
+	}
+	metric3 := dto.Metric{
+		Label: []*dto.LabelPair{&label1, &label2, &label3},
+		Gauge: &gauge,
+	}
+
+	differentLabels := dto.MetricFamily{
+		Name:   &metric_name,
+		Help:   &metric_help,
+		Type:   &metric_type,
+		Metric: []*dto.Metric{&metric1, &metric3},
+	}
+	duplicateFamily["test"] = &differentLabels
+
+	// Additional label on second metric should not be cause for duplicate detection
+	if duplicateMetricEntry(duplicateFamily) {
+		t.Errorf("Unexpected duplicate found in differentLabels")
+	}
+
+	label4_value := "2.0"
+
+	label4 := dto.LabelPair{
+		Name:  &label3_name,
+		Value: &label4_value,
+	}
+	metric4 := dto.Metric{
+		Label: []*dto.LabelPair{&label1, &label2, &label4},
+		Gauge: &gauge,
+	}
+
+	differentValues := dto.MetricFamily{
+		Name:   &metric_name,
+		Help:   &metric_help,
+		Type:   &metric_type,
+		Metric: []*dto.Metric{&metric3, &metric4},
+	}
+	duplicateFamily["test"] = &differentValues
+
+	// Additional label with different values metric should not be cause for duplicate detection
+	if duplicateMetricEntry(duplicateFamily) {
+		t.Errorf("Unexpected duplicate found in differentValues")
+	}
+}