Merge pull request #759 from breed808/textfile

Fix textfile crashes with duplicate metrics

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 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 []*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
@@ -223,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") {
@@ -271,12 +301,20 @@ fileLoop:
 		// a failure does not appear fresh.
 		mtimes[f.Name()] = f.ModTime()
 
-		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(

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,108 @@ 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 := []*dto.MetricFamily{}
+	duplicateFamily = append(duplicateFamily, &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 = []*dto.MetricFamily{}
+	duplicateFamily = append(duplicateFamily, &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 = []*dto.MetricFamily{}
+	duplicateFamily = append(duplicateFamily, &differentValues)
+
+	// Additional label with different values metric should not be cause for duplicate detection
+	if duplicateMetricEntry(duplicateFamily) {
+		t.Errorf("Unexpected duplicate found in differentValues")
+	}
+}