Add check for duplicated label names

Fixes #471

Signed-off-by: beorn7 <beorn@soundcloud.com>
This commit is contained in:
beorn7 2018-09-30 15:37:40 +02:00
parent dd9e125455
commit edb489a1ef
2 changed files with 25 additions and 11 deletions

View File

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

View File

@ -330,7 +330,10 @@ collected metric named "complex_count" collides with previously collected histog
}, },
}, },
} }
duplicateLabelMsg := []byte(`duplicate label mumble mumble`) 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 { type output struct {
headers map[string]string headers map[string]string
@ -675,7 +678,7 @@ collected metric named "complex_count" collides with previously collected histog
}, },
out: output{ out: output{
headers: map[string]string{ headers: map[string]string{
"Content-Type": `text/plain; version=0.0.4; charset=utf-8`, "Content-Type": `text/plain; charset=utf-8`,
}, },
body: duplicateLabelMsg, body: duplicateLabelMsg,
}, },