From ef138a11c86952fb17636cd22d57951de823f942 Mon Sep 17 00:00:00 2001 From: Julien Pivotto Date: Sun, 15 Mar 2020 00:42:14 +0100 Subject: [PATCH] Update client_golang to fix promhttp error handling (#6986) Fixes #6139 Signed-off-by: Julien Pivotto --- go.mod | 2 +- go.sum | 4 +-- .../prometheus/promhttp/delegator.go | 10 ++++--- .../client_golang/prometheus/promhttp/http.go | 27 +++++++++---------- vendor/modules.txt | 2 +- 5 files changed, 24 insertions(+), 21 deletions(-) diff --git a/go.mod b/go.mod index 1d02853c3..437c7c5af 100644 --- a/go.mod +++ b/go.mod @@ -33,7 +33,7 @@ require ( github.com/opentracing/opentracing-go v1.1.0 github.com/pkg/errors v0.9.1 github.com/prometheus/alertmanager v0.20.0 - github.com/prometheus/client_golang v1.5.0 + github.com/prometheus/client_golang v1.5.1 github.com/prometheus/client_model v0.2.0 github.com/prometheus/common v0.9.1 github.com/samuel/go-zookeeper v0.0.0-20190923202752-2cc03de413da diff --git a/go.sum b/go.sum index 3412ca796..efd013974 100644 --- a/go.sum +++ b/go.sum @@ -459,8 +459,8 @@ github.com/prometheus/client_golang v0.9.3-0.20190127221311-3c4408c8b829/go.mod github.com/prometheus/client_golang v1.0.0/go.mod h1:db9x61etRT2tGnBNRi70OPL5FsnadC4Ky3P0J6CfImo= github.com/prometheus/client_golang v1.2.1/go.mod h1:XMU6Z2MjaRKVu/dC1qupJI9SiNkDYzz3xecMgSW/F+U= github.com/prometheus/client_golang v1.3.0/go.mod h1:hJaj2vgQTGQmVCsAACORcieXFeDPbaTKGT+JTgUa3og= -github.com/prometheus/client_golang v1.5.0 h1:Ctq0iGpCmr3jeP77kbF2UxgvRwzWWz+4Bh9/vJTyg1A= -github.com/prometheus/client_golang v1.5.0/go.mod h1:e9GMxYsXl05ICDXkRhurwBS4Q3OK1iX/F2sw+iXX5zU= +github.com/prometheus/client_golang v1.5.1 h1:bdHYieyGlH+6OLEk2YQha8THib30KP0/yD0YH9m6xcA= +github.com/prometheus/client_golang v1.5.1/go.mod h1:e9GMxYsXl05ICDXkRhurwBS4Q3OK1iX/F2sw+iXX5zU= github.com/prometheus/client_model v0.0.0-20180712105110-5c3871d89910/go.mod h1:MbSGuTsp3dbXC40dX6PRTWyKYBIrTGTE9sqQNg2J8bo= github.com/prometheus/client_model v0.0.0-20190115171406-56726106282f/go.mod h1:MbSGuTsp3dbXC40dX6PRTWyKYBIrTGTE9sqQNg2J8bo= github.com/prometheus/client_model v0.0.0-20190129233127-fd36f4220a90/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= diff --git a/vendor/github.com/prometheus/client_golang/prometheus/promhttp/delegator.go b/vendor/github.com/prometheus/client_golang/prometheus/promhttp/delegator.go index d1354b101..5070e72e2 100644 --- a/vendor/github.com/prometheus/client_golang/prometheus/promhttp/delegator.go +++ b/vendor/github.com/prometheus/client_golang/prometheus/promhttp/delegator.go @@ -53,12 +53,16 @@ func (r *responseWriterDelegator) Written() int64 { } func (r *responseWriterDelegator) WriteHeader(code int) { + if r.observeWriteHeader != nil && !r.wroteHeader { + // Only call observeWriteHeader for the 1st time. It's a bug if + // WriteHeader is called more than once, but we want to protect + // against it here. Note that we still delegate the WriteHeader + // to the original ResponseWriter to not mask the bug from it. + r.observeWriteHeader(code) + } r.status = code r.wroteHeader = true r.ResponseWriter.WriteHeader(code) - if r.observeWriteHeader != nil { - r.observeWriteHeader(code) - } } func (r *responseWriterDelegator) Write(b []byte) (int, error) { diff --git a/vendor/github.com/prometheus/client_golang/prometheus/promhttp/http.go b/vendor/github.com/prometheus/client_golang/prometheus/promhttp/http.go index b0ee4678e..5e1c4546c 100644 --- a/vendor/github.com/prometheus/client_golang/prometheus/promhttp/http.go +++ b/vendor/github.com/prometheus/client_golang/prometheus/promhttp/http.go @@ -167,15 +167,12 @@ func HandlerFor(reg prometheus.Gatherer, opts HandlerOpts) http.Handler { enc := expfmt.NewEncoder(w, contentType) - var lastErr error - // handleError handles the error according to opts.ErrorHandling // and returns true if we have to abort after the handling. handleError := func(err error) bool { if err == nil { return false } - lastErr = err if opts.ErrorLog != nil { opts.ErrorLog.Println("error encoding and sending metric family:", err) } @@ -184,7 +181,10 @@ func HandlerFor(reg prometheus.Gatherer, opts HandlerOpts) http.Handler { case PanicOnError: panic(err) case HTTPErrorOnError: - httpError(rsp, err) + // We cannot really send an HTTP error at this + // point because we most likely have written + // something to rsp already. But at least we can + // stop sending. return true } // Do nothing in all other cases, including ContinueOnError. @@ -202,10 +202,6 @@ func HandlerFor(reg prometheus.Gatherer, opts HandlerOpts) http.Handler { return } } - - if lastErr != nil { - httpError(rsp, lastErr) - } }) if opts.Timeout <= 0 { @@ -276,7 +272,12 @@ type HandlerErrorHandling int // errors are encountered. const ( // Serve an HTTP status code 500 upon the first error - // encountered. Report the error message in the body. + // encountered. Report the error message in the body. Note that HTTP + // errors cannot be served anymore once the beginning of a regular + // payload has been sent. Thus, in the (unlikely) case that encoding the + // payload into the negotiated wire format fails, serving the response + // will simply be aborted. Set an ErrorLog in HandlerOpts to detect + // those errors. HTTPErrorOnError HandlerErrorHandling = iota // Ignore errors and try to serve as many metrics as possible. However, // if no metrics can be served, serve an HTTP status code 500 and the @@ -365,11 +366,9 @@ func gzipAccepted(header http.Header) bool { } // httpError removes any content-encoding header and then calls http.Error with -// the provided error and http.StatusInternalServerErrer. Error contents is -// supposed to be uncompressed plain text. However, same as with a plain -// http.Error, any header settings will be void if the header has already been -// sent. The error message will still be written to the writer, but it will -// probably be of limited use. +// the provided error and http.StatusInternalServerError. Error contents is +// supposed to be uncompressed plain text. Same as with a plain http.Error, this +// must not be called if the header or any payload has already been sent. func httpError(rsp http.ResponseWriter, err error) { rsp.Header().Del(contentEncodingHeader) http.Error( diff --git a/vendor/modules.txt b/vendor/modules.txt index 4872d8522..e4ff4a4c9 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -272,7 +272,7 @@ github.com/opentracing/opentracing-go/log github.com/pkg/errors # github.com/prometheus/alertmanager v0.20.0 github.com/prometheus/alertmanager/api/v2/models -# github.com/prometheus/client_golang v1.5.0 +# github.com/prometheus/client_golang v1.5.1 github.com/prometheus/client_golang/api github.com/prometheus/client_golang/api/prometheus/v1 github.com/prometheus/client_golang/prometheus