implement review feedback

prometheus/desc.go

@@ -25,19 +25,6 @@ import (
 	dto "github.com/prometheus/client_model/go"
 )
 
-// reservedLabelPrefix is a prefix which is not legal in user-supplied
-// label names.
-const reservedLabelPrefix = "__"
-
-// Labels represents a collection of label name -> value mappings. This type is
-// commonly used with the With(Labels) and GetMetricWith(Labels) methods of
-// metric vector Collectors, e.g.:
-//     myVec.With(Labels{"code": "404", "method": "GET"}).Add(42)
-//
-// The other use-case is the specification of constant label pairs in Opts or to
-// create a Desc.
-type Labels map[string]string
-
 // Desc is the descriptor used by every Prometheus Metric. It is essentially
 // the immutable meta-data of a Metric. The normal Metric implementations
 // included in this package manage their Desc under the hood. Users only have to
@@ -124,7 +111,7 @@ func NewDesc(fqName, help string, variableLabels []string, constLabels Labels) *
 	}
 	// Validate the const label values. They can't have a wrong cardinality, so
 	// use in len(labelValues) as expectedNumberOfValues.
-	if err := validateLabelValues(labelValues, len(labelValues)); err != nil {
+	if err := validateValuesInLabels(labelValues, len(labelValues)); err != nil {
 		d.err = err
 		return d
 	}
@@ -200,8 +187,3 @@ func (d *Desc) String() string {
 		d.variableLabels,
 	)
 }
-
-func checkLabelName(l string) bool {
-	return model.LabelName(l).IsValid() &&
-		!strings.HasPrefix(l, reservedLabelPrefix)
-}

prometheus/histogram.go

@@ -430,7 +430,7 @@ func NewConstHistogram(
 	buckets map[float64]uint64,
 	labelValues ...string,
 ) (Metric, error) {
-	if err := validateLabelValues(labelValues, len(desc.variableLabels)); err != nil {
+	if err := validateValuesInLabels(labelValues, len(desc.variableLabels)); err != nil {
 		return nil, err
 	}
 	return &constHistogram{

prometheus/label_values.go

@@ -1,35 +0,0 @@
-package prometheus
-
-import (
-	"errors"
-	"fmt"
-	"unicode/utf8"
-)
-
-func validateLabelValues(vals []string, expectedNumberOfValues int) error {
-	if len(vals) != expectedNumberOfValues {
-		return errInconsistentCardinality
-	}
-
-	for _, val := range vals {
-		if !utf8.ValidString(val) {
-			return errors.New(fmt.Sprintf("label value %#v is not valid utf8", val))
-		}
-	}
-
-	return nil
-}
-
-func validateLabels(labels Labels, expectedNumberOfValues int) error {
-	if len(labels) != expectedNumberOfValues {
-		return errInconsistentCardinality
-	}
-
-	for name, val := range labels {
-		if !utf8.ValidString(val) {
-			return errors.New(fmt.Sprintf("label %s: %#v is not valid utf8", name, val))
-		}
-	}
-
-	return nil
-}

prometheus/labels.go

@@ -0,0 +1,57 @@
+package prometheus
+
+import (
+	"errors"
+	"fmt"
+	"strings"
+	"unicode/utf8"
+
+	"github.com/prometheus/common/model"
+)
+
+// Labels represents a collection of label name -> value mappings. This type is
+// commonly used with the With(Labels) and GetMetricWith(Labels) methods of
+// metric vector Collectors, e.g.:
+//     myVec.With(Labels{"code": "404", "method": "GET"}).Add(42)
+//
+// The other use-case is the specification of constant label pairs in Opts or to
+// create a Desc.
+type Labels map[string]string
+
+// reservedLabelPrefix is a prefix which is not legal in user-supplied
+// label names.
+const reservedLabelPrefix = "__"
+
+var errInconsistentCardinality = errors.New("inconsistent label cardinality")
+
+func validateLabels(labels Labels, expectedNumberOfValues int) error {
+	if len(labels) != expectedNumberOfValues {
+		return errInconsistentCardinality
+	}
+
+	for name, val := range labels {
+		if !utf8.ValidString(val) {
+			return fmt.Errorf("label %s: value %q is not valid UTF-8", name, val)
+		}
+	}
+
+	return nil
+}
+
+func validateValuesInLabels(vals []string, expectedNumberOfValues int) error {
+	if len(vals) != expectedNumberOfValues {
+		return errInconsistentCardinality
+	}
+
+	for _, val := range vals {
+		if !utf8.ValidString(val) {
+			return fmt.Errorf("label value %q is not valid UTF-8", val)
+		}
+	}
+
+	return nil
+}
+
+func checkLabelName(l string) bool {
+	return model.LabelName(l).IsValid() && !strings.HasPrefix(l, reservedLabelPrefix)
+}

prometheus/registry.go

@@ -20,11 +20,10 @@ import (
 	"os"
 	"sort"
 	"sync"
+	"unicode/utf8"
 
 	"github.com/golang/protobuf/proto"
 
-	"unicode/utf8"
-
 	dto "github.com/prometheus/client_model/go"
 )
 

prometheus/summary.go

@@ -543,7 +543,7 @@ func NewConstSummary(
 	quantiles map[float64]float64,
 	labelValues ...string,
 ) (Metric, error) {
-	if err := validateLabelValues(labelValues, len(desc.variableLabels)); err != nil {
+	if err := validateValuesInLabels(labelValues, len(desc.variableLabels)); err != nil {
 		return nil, err
 	}
 	return &constSummary{

prometheus/value.go

@@ -14,7 +14,6 @@
 package prometheus
 
 import (
-	"errors"
 	"fmt"
 	"math"
 	"sort"
@@ -37,8 +36,6 @@ const (
 	UntypedValue
 )
 
-var errInconsistentCardinality = errors.New("inconsistent label cardinality")
-
 // value is a generic metric for simple values. It implements Metric, Collector,
 // Counter, Gauge, and Untyped. Its effective type is determined by
 // ValueType. This is a low-level building block used by the library to back the
@@ -158,7 +155,7 @@ func (v *valueFunc) Write(out *dto.Metric) error {
 // the Collect method. NewConstMetric returns an error if the length of
 // labelValues is not consistent with the variable labels in Desc.
 func NewConstMetric(desc *Desc, valueType ValueType, value float64, labelValues ...string) (Metric, error) {
-	if err := validateLabelValues(labelValues, len(desc.variableLabels)); err != nil {
+	if err := validateValuesInLabels(labelValues, len(desc.variableLabels)); err != nil {
 		return nil, err
 	}
 	return &constMetric{

prometheus/vec.go

@@ -207,7 +207,7 @@ func (m *metricVec) Reset() {
 }
 
 func (m *metricVec) hashLabelValues(vals []string) (uint64, error) {
-	if err := validateLabelValues(vals, len(m.desc.variableLabels)); err != nil {
+	if err := validateValuesInLabels(vals, len(m.desc.variableLabels)); err != nil {
 		return 0, err
 	}