Merge pull request #460 from prometheus/beorn7/desc

Detect invalid Descs in const metrics, but also allow empty Help strings.
This commit is contained in:
Björn Rabenstein 2018-09-17 12:21:22 +02:00 committed by GitHub
commit e637cec7d9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 27 additions and 50 deletions

View File

@ -67,7 +67,7 @@ type Desc struct {
// NewDesc allocates and initializes a new Desc. Errors are recorded in the Desc // NewDesc allocates and initializes a new Desc. Errors are recorded in the Desc
// and will be reported on registration time. variableLabels and constLabels can // and will be reported on registration time. variableLabels and constLabels can
// be nil if no such labels should be set. fqName and help must not be empty. // be nil if no such labels should be set. fqName must not be empty.
// //
// variableLabels only contain the label names. Their label values are variable // variableLabels only contain the label names. Their label values are variable
// and therefore not part of the Desc. (They are managed within the Metric.) // and therefore not part of the Desc. (They are managed within the Metric.)
@ -80,10 +80,6 @@ func NewDesc(fqName, help string, variableLabels []string, constLabels Labels) *
help: help, help: help,
variableLabels: variableLabels, variableLabels: variableLabels,
} }
if help == "" {
d.err = errors.New("empty help string")
return d
}
if !model.IsValidMetricName(model.LabelValue(fqName)) { if !model.IsValidMetricName(model.LabelValue(fqName)) {
d.err = fmt.Errorf("%q is not a valid metric name", fqName) d.err = fmt.Errorf("%q is not a valid metric name", fqName)
return d return d

View File

@ -89,37 +89,6 @@ func ExampleGaugeFunc() {
// GaugeFunc 'goroutines_count' registered. // GaugeFunc 'goroutines_count' registered.
} }
func ExampleCounter() {
pushCounter := prometheus.NewCounter(prometheus.CounterOpts{
Name: "repository_pushes", // Note: No help string...
})
err := prometheus.Register(pushCounter) // ... so this will return an error.
if err != nil {
fmt.Println("Push counter couldn't be registered, no counting will happen:", err)
return
}
// Try it once more, this time with a help string.
pushCounter = prometheus.NewCounter(prometheus.CounterOpts{
Name: "repository_pushes",
Help: "Number of pushes to external repository.",
})
err = prometheus.Register(pushCounter)
if err != nil {
fmt.Println("Push counter couldn't be registered AGAIN, no counting will happen:", err)
return
}
pushComplete := make(chan struct{})
// TODO: Start a goroutine that performs repository pushes and reports
// each completion via the channel.
for range pushComplete {
pushCounter.Inc()
}
// Output:
// Push counter couldn't be registered, no counting will happen: descriptor Desc{fqName: "repository_pushes", help: "", constLabels: {}, variableLabels: []} is invalid: empty help string
}
func ExampleCounterVec() { func ExampleCounterVec() {
httpReqs := prometheus.NewCounterVec( httpReqs := prometheus.NewCounterVec(
prometheus.CounterOpts{ prometheus.CounterOpts{

View File

@ -110,8 +110,9 @@ func ExponentialBuckets(start, factor float64, count int) []float64 {
} }
// HistogramOpts bundles the options for creating a Histogram metric. It is // HistogramOpts bundles the options for creating a Histogram metric. It is
// mandatory to set Name and Help to a non-empty string. All other fields are // mandatory to set Name to a non-empty string. All other fields are optional
// optional and can safely be left at their zero value. // and can safely be left at their zero value, although it is strongly
// encouraged to set a Help string.
type HistogramOpts struct { type HistogramOpts struct {
// Namespace, Subsystem, and Name are components of the fully-qualified // Namespace, Subsystem, and Name are components of the fully-qualified
// name of the Histogram (created by joining these components with // name of the Histogram (created by joining these components with
@ -122,7 +123,7 @@ type HistogramOpts struct {
Subsystem string Subsystem string
Name string Name string
// Help provides information about this Histogram. Mandatory! // Help provides information about this Histogram.
// //
// Metrics with the same fully-qualified name must have the same Help // Metrics with the same fully-qualified name must have the same Help
// string. // string.
@ -554,7 +555,7 @@ func (h *constHistogram) Write(out *dto.Metric) error {
// bucket. // bucket.
// //
// NewConstHistogram returns an error if the length of labelValues is not // NewConstHistogram returns an error if the length of labelValues is not
// consistent with the variable labels in Desc. // consistent with the variable labels in Desc or if Desc is invalid.
func NewConstHistogram( func NewConstHistogram(
desc *Desc, desc *Desc,
count uint64, count uint64,
@ -562,6 +563,9 @@ func NewConstHistogram(
buckets map[float64]uint64, buckets map[float64]uint64,
labelValues ...string, labelValues ...string,
) (Metric, error) { ) (Metric, error) {
if desc.err != nil {
return nil, desc.err
}
if err := validateLabelValues(labelValues, len(desc.variableLabels)); err != nil { if err := validateLabelValues(labelValues, len(desc.variableLabels)); err != nil {
return nil, err return nil, err
} }

View File

@ -59,8 +59,9 @@ type Metric interface {
// implementation XXX has its own XXXOpts type, but in most cases, it is just be // implementation XXX has its own XXXOpts type, but in most cases, it is just be
// an alias of this type (which might change when the requirement arises.) // an alias of this type (which might change when the requirement arises.)
// //
// It is mandatory to set Name and Help to a non-empty string. All other fields // It is mandatory to set Name to a non-empty string. All other fields are
// are optional and can safely be left at their zero value. // optional and can safely be left at their zero value, although it is strongly
// encouraged to set a Help string.
type Opts struct { type Opts struct {
// Namespace, Subsystem, and Name are components of the fully-qualified // Namespace, Subsystem, and Name are components of the fully-qualified
// name of the Metric (created by joining these components with // name of the Metric (created by joining these components with
@ -71,7 +72,7 @@ type Opts struct {
Subsystem string Subsystem string
Name string Name string
// Help provides information about this metric. Mandatory! // Help provides information about this metric.
// //
// Metrics with the same fully-qualified name must have the same Help // Metrics with the same fully-qualified name must have the same Help
// string. // string.

View File

@ -81,10 +81,10 @@ const (
) )
// SummaryOpts bundles the options for creating a Summary metric. It is // SummaryOpts bundles the options for creating a Summary metric. It is
// mandatory to set Name and Help to a non-empty string. While all other fields // mandatory to set Name to a non-empty string. While all other fields are
// are optional and can safely be left at their zero value, it is recommended to // optional and can safely be left at their zero value, it is recommended to set
// explicitly set the Objectives field to the desired value as the default value // a help string and to explicitly set the Objectives field to the desired value
// will change in the upcoming v0.10 of the library. // as the default value will change in the upcoming v0.10 of the library.
type SummaryOpts struct { type SummaryOpts struct {
// Namespace, Subsystem, and Name are components of the fully-qualified // Namespace, Subsystem, and Name are components of the fully-qualified
// name of the Summary (created by joining these components with // name of the Summary (created by joining these components with
@ -95,7 +95,7 @@ type SummaryOpts struct {
Subsystem string Subsystem string
Name string Name string
// Help provides information about this Summary. Mandatory! // Help provides information about this Summary.
// //
// Metrics with the same fully-qualified name must have the same Help // Metrics with the same fully-qualified name must have the same Help
// string. // string.
@ -586,7 +586,7 @@ func (s *constSummary) Write(out *dto.Metric) error {
// map[float64]float64{0.5: 0.23, 0.99: 0.56} // map[float64]float64{0.5: 0.23, 0.99: 0.56}
// //
// NewConstSummary returns an error if the length of labelValues is not // NewConstSummary returns an error if the length of labelValues is not
// consistent with the variable labels in Desc. // consistent with the variable labels in Desc or if Desc is invalid.
func NewConstSummary( func NewConstSummary(
desc *Desc, desc *Desc,
count uint64, count uint64,
@ -594,6 +594,9 @@ func NewConstSummary(
quantiles map[float64]float64, quantiles map[float64]float64,
labelValues ...string, labelValues ...string,
) (Metric, error) { ) (Metric, error) {
if desc.err != nil {
return nil, desc.err
}
if err := validateLabelValues(labelValues, len(desc.variableLabels)); err != nil { if err := validateLabelValues(labelValues, len(desc.variableLabels)); err != nil {
return nil, err return nil, err
} }

View File

@ -77,8 +77,12 @@ func (v *valueFunc) Write(out *dto.Metric) error {
// operations. However, when implementing custom Collectors, it is useful as a // operations. However, when implementing custom Collectors, it is useful as a
// throw-away metric that is generated on the fly to send it to Prometheus in // throw-away metric that is generated on the fly to send it to Prometheus in
// the Collect method. NewConstMetric returns an error if the length of // the Collect method. NewConstMetric returns an error if the length of
// labelValues is not consistent with the variable labels in Desc. // labelValues is not consistent with the variable labels in Desc or if Desc is
// invalid.
func NewConstMetric(desc *Desc, valueType ValueType, value float64, labelValues ...string) (Metric, error) { func NewConstMetric(desc *Desc, valueType ValueType, value float64, labelValues ...string) (Metric, error) {
if desc.err != nil {
return nil, desc.err
}
if err := validateLabelValues(labelValues, len(desc.variableLabels)); err != nil { if err := validateLabelValues(labelValues, len(desc.variableLabels)); err != nil {
return nil, err return nil, err
} }