From 0b8aef084ef71aa52c2a2618658e804cfc330249 Mon Sep 17 00:00:00 2001 From: Marco Jantke Date: Fri, 25 Aug 2017 14:50:43 +0200 Subject: [PATCH] implement review feedback --- prometheus/desc.go | 20 +------------ prometheus/histogram.go | 2 +- prometheus/label_values.go | 35 ----------------------- prometheus/labels.go | 57 ++++++++++++++++++++++++++++++++++++++ prometheus/registry.go | 3 +- prometheus/summary.go | 2 +- prometheus/value.go | 5 +--- prometheus/vec.go | 2 +- 8 files changed, 63 insertions(+), 63 deletions(-) delete mode 100644 prometheus/label_values.go create mode 100644 prometheus/labels.go diff --git a/prometheus/desc.go b/prometheus/desc.go index 4c67346..a6c37e2 100644 --- a/prometheus/desc.go +++ b/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) -} diff --git a/prometheus/histogram.go b/prometheus/histogram.go index 6cc6e68..530811d 100644 --- a/prometheus/histogram.go +++ b/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{ diff --git a/prometheus/label_values.go b/prometheus/label_values.go deleted file mode 100644 index 519290e..0000000 --- a/prometheus/label_values.go +++ /dev/null @@ -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 -} diff --git a/prometheus/labels.go b/prometheus/labels.go new file mode 100644 index 0000000..3755a9f --- /dev/null +++ b/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) +} diff --git a/prometheus/registry.go b/prometheus/registry.go index 1e84531..8f2094c 100644 --- a/prometheus/registry.go +++ b/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" ) diff --git a/prometheus/summary.go b/prometheus/summary.go index 56b0663..bf69a50 100644 --- a/prometheus/summary.go +++ b/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{ diff --git a/prometheus/value.go b/prometheus/value.go index c445f67..b0d7e60 100644 --- a/prometheus/value.go +++ b/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{ diff --git a/prometheus/vec.go b/prometheus/vec.go index 4e19995..6c483b6 100644 --- a/prometheus/vec.go +++ b/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 }