Merge pull request #474 from prometheus/beorn7/testing

Add check for duplicated label name

prometheus/registry.go

@@ -787,6 +787,8 @@ func checkMetricConsistency(
 	dtoMetric *dto.Metric,
 	metricHashes map[uint64]struct{},
 ) error {
+	name := metricFamily.GetName()
+
 	// Type consistency with metric family.
 	if metricFamily.GetType() == dto.MetricType_GAUGE && dtoMetric.Gauge == nil ||
 		metricFamily.GetType() == dto.MetricType_COUNTER && dtoMetric.Counter == nil ||
@@ -795,33 +797,42 @@ func checkMetricConsistency(
 		metricFamily.GetType() == dto.MetricType_UNTYPED && dtoMetric.Untyped == nil {
 		return fmt.Errorf(
 			"collected metric %q { %s} is not a %s",
-			metricFamily.GetName(), dtoMetric, metricFamily.GetType(),
+			name, dtoMetric, metricFamily.GetType(),
 		)
 	}
 
+	previousLabelName := ""
 	for _, labelPair := range dtoMetric.GetLabel() {
-		if !checkLabelName(labelPair.GetName()) {
+		labelName := labelPair.GetName()
+		if labelName == previousLabelName {
 			return fmt.Errorf(
-				"collected metric %q { %s} has a label with an invalid name: %s",
-				metricFamily.GetName(), dtoMetric, labelPair.GetName(),
+				"collected metric %q { %s} has two or more labels with the same name: %s",
+				name, dtoMetric, labelName,
 			)
 		}
-		if dtoMetric.Summary != nil && labelPair.GetName() == quantileLabel {
+		if !checkLabelName(labelName) {
+			return fmt.Errorf(
+				"collected metric %q { %s} has a label with an invalid name: %s",
+				name, dtoMetric, labelName,
+			)
+		}
+		if dtoMetric.Summary != nil && labelName == quantileLabel {
 			return fmt.Errorf(
 				"collected metric %q { %s} must not have an explicit %q label",
-				metricFamily.GetName(), dtoMetric, quantileLabel,
+				name, dtoMetric, quantileLabel,
 			)
 		}
 		if !utf8.ValidString(labelPair.GetValue()) {
 			return fmt.Errorf(
 				"collected metric %q { %s} has a label named %q whose value is not utf8: %#v",
-				metricFamily.GetName(), dtoMetric, labelPair.GetName(), labelPair.GetValue())
+				name, dtoMetric, labelName, labelPair.GetValue())
 		}
+		previousLabelName = labelName
 	}
 
 	// Is the metric unique (i.e. no other metric with the same name and the same labels)?
 	h := hashNew()
-	h = hashAdd(h, metricFamily.GetName())
+	h = hashAdd(h, name)
 	h = hashAddByte(h, separatorByte)
 	// Make sure label pairs are sorted. We depend on it for the consistency
 	// check.
@@ -835,7 +846,7 @@ func checkMetricConsistency(
 	if _, exists := metricHashes[h]; exists {
 		return fmt.Errorf(
 			"collected metric %q { %s} was collected before with the same name and label values",
-			metricFamily.GetName(), dtoMetric,
+			name, dtoMetric,
 		)
 	}
 	metricHashes[h] = struct{}{}

prometheus/registry_test.go

@@ -307,6 +307,32 @@ collected metric named "complex_count" collides with previously collected summar
 	histogramCountCollisionMsg := []byte(`An error has occurred during metrics gathering:
 
 collected metric named "complex_count" collides with previously collected histogram named "complex"
+`)
+	externalMetricFamilyWithDuplicateLabel := &dto.MetricFamily{
+		Name: proto.String("broken_metric"),
+		Help: proto.String("The registry should detect the duplicate label."),
+		Type: dto.MetricType_COUNTER.Enum(),
+		Metric: []*dto.Metric{
+			{
+				Label: []*dto.LabelPair{
+					{
+						Name:  proto.String("foo"),
+						Value: proto.String("bar"),
+					},
+					{
+						Name:  proto.String("foo"),
+						Value: proto.String("baz"),
+					},
+				},
+				Counter: &dto.Counter{
+					Value: proto.Float64(2.7),
+				},
+			},
+		},
+	}
+	duplicateLabelMsg := []byte(`An error has occurred during metrics gathering:
+
+collected metric "broken_metric" { label:<name:"foo" value:"bar" > label:<name:"foo" value:"baz" > counter:<value:2.7 > } has two or more labels with the same name: foo
 `)
 
 	type output struct {
@@ -646,6 +672,20 @@ collected metric named "complex_count" collides with previously collected histog
 				externalMetricFamilyWithBucketSuffix,
 			},
 		},
+		{ // 22
+			headers: map[string]string{
+				"Accept": "text/plain",
+			},
+			out: output{
+				headers: map[string]string{
+					"Content-Type": `text/plain; charset=utf-8`,
+				},
+				body: duplicateLabelMsg,
+			},
+			externalMF: []*dto.MetricFamily{
+				externalMetricFamilyWithDuplicateLabel,
+			},
+		},
 	}
 	for i, scenario := range scenarios {
 		registry := prometheus.NewPedanticRegistry()