From 1dc03a72f62495f53a581b94d1236a2f1c56134f Mon Sep 17 00:00:00 2001 From: beorn7 Date: Thu, 4 Aug 2016 16:03:06 +0200 Subject: [PATCH] Replace hashicorp/go-multierror by own implementation The own implementation is much easier as it only has to serve our one use case. --- prometheus/promhttp/http.go | 6 +++--- prometheus/promhttp/http_test.go | 6 ++---- prometheus/registry.go | 33 ++++++++++++++++++++++++++------ 3 files changed, 32 insertions(+), 13 deletions(-) diff --git a/prometheus/promhttp/http.go b/prometheus/promhttp/http.go index 91b9a2c..3116d2f 100644 --- a/prometheus/promhttp/http.go +++ b/prometheus/promhttp/http.go @@ -82,18 +82,18 @@ func HandlerFor(reg prometheus.Gatherer, opts HandlerOpts) http.Handler { mfs, err := reg.Gather() if err != nil { if opts.ErrorLog != nil { - opts.ErrorLog.Println("error collecting metrics:", err) + opts.ErrorLog.Println("error gathering metrics:", err) } switch opts.ErrorHandling { case PanicOnError: panic(err) case ContinueOnError: if len(mfs) == 0 { - http.Error(w, "No metrics collected, last error:\n\n"+err.Error(), http.StatusInternalServerError) + http.Error(w, "No metrics gathered, last error:\n\n"+err.Error(), http.StatusInternalServerError) return } case HTTPErrorOnError: - http.Error(w, "An error has occurred during metrics collection:\n\n"+err.Error(), http.StatusInternalServerError) + http.Error(w, "An error has occurred during metrics gathering:\n\n"+err.Error(), http.StatusInternalServerError) return } } diff --git a/prometheus/promhttp/http_test.go b/prometheus/promhttp/http_test.go index 4d0370e..f3a26f8 100644 --- a/prometheus/promhttp/http_test.go +++ b/prometheus/promhttp/http_test.go @@ -88,14 +88,12 @@ func TestHandlerErrorHandling(t *testing.T) { ErrorLog: logger, ErrorHandling: PanicOnError, }) - wantMsg := `error collecting metrics: 1 error(s) occurred: - + wantMsg := `error gathering metrics: 1 error(s) occurred: * error collecting metric Desc{fqName: "invalid_metric", help: "not helpful", constLabels: {}, variableLabels: []}: collect error ` - wantErrorBody := `An error has occurred during metrics collection: + wantErrorBody := `An error has occurred during metrics gathering: 1 error(s) occurred: - * error collecting metric Desc{fqName: "invalid_metric", help: "not helpful", constLabels: {}, variableLabels: []}: collect error ` wantOKBody := `# HELP name docstring diff --git a/prometheus/registry.go b/prometheus/registry.go index 7b19b43..20b73f1 100644 --- a/prometheus/registry.go +++ b/prometheus/registry.go @@ -14,6 +14,7 @@ package prometheus import ( + "bytes" "errors" "fmt" "os" @@ -21,7 +22,6 @@ import ( "sync" "github.com/golang/protobuf/proto" - "github.com/hashicorp/go-multierror" dto "github.com/prometheus/client_model/go" ) @@ -220,6 +220,22 @@ func (err AlreadyRegisteredError) Error() string { return "duplicate metrics collector registration attempted" } +// MultiError is a slice of errors implementing the error interface. It is used +// by a Gatherer to report multiple errors during MetricFamily gathering. +type MultiError []error + +func (errs MultiError) Error() string { + if len(errs) == 0 { + return "" + } + buf := &bytes.Buffer{} + fmt.Fprintf(buf, "%d error(s) occurred:", len(errs)) + for _, err := range errs { + fmt.Fprintf(buf, "\n* %s", err) + } + return buf.String() +} + // Registry registers Prometheus collectors, collects their metrics, and gathers // them into MetricFamilies for exposition. It implements Registerer and // Gatherer. The zero value is not usable. Create instances with NewRegistry or @@ -366,7 +382,7 @@ func (r *Registry) Gather() ([]*dto.MetricFamily, error) { metricChan = make(chan Metric, capMetricChan) metricHashes = map[uint64]struct{}{} wg sync.WaitGroup - errs error // The collected errors to return in the end. + errs MultiError // The collected errors to return in the end. registeredDescIDs map[uint64]struct{} // Only used for pedantic checks ) @@ -419,7 +435,7 @@ func (r *Registry) Gather() ([]*dto.MetricFamily, error) { } dtoMetric := &dto.Metric{} if err := metric.Write(dtoMetric); err != nil { - errs = multierror.Append(errs, fmt.Errorf( + errs = append(errs, fmt.Errorf( "error collecting metric %v: %s", desc, err, )) continue @@ -438,13 +454,13 @@ func (r *Registry) Gather() ([]*dto.MetricFamily, error) { case dtoMetric.Histogram != nil: metricFamily.Type = dto.MetricType_HISTOGRAM.Enum() default: - errs = multierror.Append(errs, fmt.Errorf( + errs = append(errs, fmt.Errorf( "empty metric collected: %s", dtoMetric, )) continue } if err := r.checkConsistency(metricFamily, dtoMetric, desc, metricHashes, registeredDescIDs); err != nil { - errs = multierror.Append(errs, err) + errs = append(errs, err) continue } metricFamily.Metric = append(metricFamily.Metric, dtoMetric) @@ -464,7 +480,7 @@ func (r *Registry) Gather() ([]*dto.MetricFamily, error) { } for _, m := range mf.Metric { if err := r.checkConsistency(existingMF, m, nil, metricHashes, nil); err != nil { - errs = multierror.Append(errs, err) + errs = append(errs, err) continue } existingMF.Metric = append(existingMF.Metric, m) @@ -493,6 +509,11 @@ func (r *Registry) Gather() ([]*dto.MetricFamily, error) { for _, name := range names { result = append(result, metricFamiliesByName[name]) } + // We cannot just `return result, errs`. Even if errs == nil, it will + // not be seen as nil through the error interface. + if len(errs) == 0 { + return result, nil + } return result, errs }