lib/httpserver: log remote address in error message from httpserver.Errorf

This should improve detection of the root cause of errors.
Thanks to Anant for the idea.

app/vmagent/main.go

@@ -136,7 +136,7 @@ func requestHandler(w http.ResponseWriter, r *http.Request) bool {
 		prometheusWriteRequests.Inc()
 		if err := promremotewrite.InsertHandler(r); err != nil {
 			prometheusWriteErrors.Inc()
-			httpserver.Errorf(w, "error in %q: %s", r.URL.Path, err)
+			httpserver.Errorf(w, r, "error in %q: %s", r.URL.Path, err)
 			return true
 		}
 		w.WriteHeader(http.StatusNoContent)
@@ -145,7 +145,7 @@ func requestHandler(w http.ResponseWriter, r *http.Request) bool {
 		vmimportRequests.Inc()
 		if err := vmimport.InsertHandler(r); err != nil {
 			vmimportErrors.Inc()
-			httpserver.Errorf(w, "error in %q: %s", r.URL.Path, err)
+			httpserver.Errorf(w, r, "error in %q: %s", r.URL.Path, err)
 			return true
 		}
 		w.WriteHeader(http.StatusNoContent)
@@ -154,7 +154,7 @@ func requestHandler(w http.ResponseWriter, r *http.Request) bool {
 		csvimportRequests.Inc()
 		if err := csvimport.InsertHandler(r); err != nil {
 			csvimportErrors.Inc()
-			httpserver.Errorf(w, "error in %q: %s", r.URL.Path, err)
+			httpserver.Errorf(w, r, "error in %q: %s", r.URL.Path, err)
 			return true
 		}
 		w.WriteHeader(http.StatusNoContent)
@@ -163,7 +163,7 @@ func requestHandler(w http.ResponseWriter, r *http.Request) bool {
 		prometheusimportRequests.Inc()
 		if err := prometheusimport.InsertHandler(r); err != nil {
 			prometheusimportErrors.Inc()
-			httpserver.Errorf(w, "error in %q: %s", r.URL.Path, err)
+			httpserver.Errorf(w, r, "error in %q: %s", r.URL.Path, err)
 			return true
 		}
 		w.WriteHeader(http.StatusNoContent)
@@ -172,7 +172,7 @@ func requestHandler(w http.ResponseWriter, r *http.Request) bool {
 		influxWriteRequests.Inc()
 		if err := influx.InsertHandlerForHTTP(r); err != nil {
 			influxWriteErrors.Inc()
-			httpserver.Errorf(w, "error in %q: %s", r.URL.Path, err)
+			httpserver.Errorf(w, r, "error in %q: %s", r.URL.Path, err)
 			return true
 		}
 		w.WriteHeader(http.StatusNoContent)

app/vmalert/web.go

@@ -27,7 +27,6 @@ var pathList = [][]string{
 }
 
 func (rh *requestHandler) handler(w http.ResponseWriter, r *http.Request) bool {
-	resph := responseHandler{w}
 	switch r.URL.Path {
 	case "/":
 		for _, path := range pathList {
@@ -36,10 +35,22 @@ func (rh *requestHandler) handler(w http.ResponseWriter, r *http.Request) bool {
 		}
 		return true
 	case "/api/v1/groups":
-		resph.handle(rh.listGroups())
+		data, err := rh.listGroups()
+		if err != nil {
+			httpserver.Errorf(w, r, "error in %q: %s", r.URL.Path, err)
+			return true
+		}
+		w.Header().Set("Content-Type", "application/json")
+		w.Write(data)
 		return true
 	case "/api/v1/alerts":
-		resph.handle(rh.listAlerts())
+		data, err := rh.listAlerts()
+		if err != nil {
+			httpserver.Errorf(w, r, "error in %q: %s", r.URL.Path, err)
+			return true
+		}
+		w.Header().Set("Content-Type", "application/json")
+		w.Write(data)
 		return true
 	case "/-/reload":
 		logger.Infof("api config reload was called, sending sighup")
@@ -47,12 +58,18 @@ func (rh *requestHandler) handler(w http.ResponseWriter, r *http.Request) bool {
 		w.WriteHeader(http.StatusOK)
 		return true
 	default:
+		if !strings.HasSuffix(r.URL.Path, "/status") {
+			return false
+		}
 		// /api/v1/<groupName>/<alertID>/status
-		if strings.HasSuffix(r.URL.Path, "/status") {
+		data, err := rh.alert(r.URL.Path)
-			resph.handle(rh.alert(r.URL.Path))
+		if err != nil {
+			httpserver.Errorf(w, r, "error in %q: %s", r.URL.Path, err)
 			return true
 		}
-		return false
+		w.Header().Set("Content-Type", "application/json")
+		w.Write(data)
+		return true
 	}
 }
 
@@ -151,18 +168,6 @@ func (rh *requestHandler) alert(path string) ([]byte, error) {
 	return json.Marshal(resp)
 }
 
-// responseHandler wrapper on http.ResponseWriter with sugar
-type responseHandler struct{ http.ResponseWriter }
-
-func (w responseHandler) handle(b []byte, err error) {
-	if err != nil {
-		httpserver.Errorf(w, "%s", err)
-		return
-	}
-	w.Header().Set("Content-Type", "application/json")
-	w.Write(b)
-}
-
 func uint64FromPath(path string) (uint64, error) {
 	s := strings.TrimRight(path, "/")
 	return strconv.ParseUint(s, 10, 0)

app/vmauth/main.go

@@ -49,20 +49,20 @@ func main() {
 func requestHandler(w http.ResponseWriter, r *http.Request) bool {
 	username, password, ok := r.BasicAuth()
 	if !ok {
-		httpserver.Errorf(w, "Missing `Authorization: Basic *` header")
+		httpserver.Errorf(w, r, "missing `Authorization: Basic *` header")
 		return true
 	}
 	ac := authConfig.Load().(map[string]*UserInfo)
 	info := ac[username]
 	if info == nil || info.Password != password {
-		httpserver.Errorf(w, "Cannot find the provided username %q or password in config", username)
+		httpserver.Errorf(w, r, "cannot find the provided username %q or password in config", username)
 		return true
 	}
 	info.requests.Inc()
 
 	targetURL := createTargetURL(info.URLPrefix, r.URL)
 	if _, err := url.Parse(targetURL); err != nil {
-		httpserver.Errorf(w, "Invalid targetURL=%q: %s", targetURL, err)
+		httpserver.Errorf(w, r, "invalid targetURL=%q: %s", targetURL, err)
 		return true
 	}
 	r.Header.Set("vm-target-url", targetURL)

app/vminsert/main.go

@@ -136,7 +136,7 @@ func main() {
 func requestHandler(w http.ResponseWriter, r *http.Request) bool {
 	p, err := httpserver.ParsePath(r.URL.Path)
 	if err != nil {
-		httpserver.Errorf(w, "cannot parse path %q: %s", r.URL.Path, err)
+		httpserver.Errorf(w, r, "cannot parse path %q: %s", r.URL.Path, err)
 		return true
 	}
 	if p.Prefix != "insert" {
@@ -145,7 +145,7 @@ func requestHandler(w http.ResponseWriter, r *http.Request) bool {
 	}
 	at, err := auth.NewToken(p.AuthToken)
 	if err != nil {
-		httpserver.Errorf(w, "auth error: %s", err)
+		httpserver.Errorf(w, r, "auth error: %s", err)
 		return true
 	}
 
@@ -154,7 +154,7 @@ func requestHandler(w http.ResponseWriter, r *http.Request) bool {
 		prometheusWriteRequests.Inc()
 		if err := promremotewrite.InsertHandler(at, r); err != nil {
 			prometheusWriteErrors.Inc()
-			httpserver.Errorf(w, "error in %q: %s", r.URL.Path, err)
+			httpserver.Errorf(w, r, "error in %q: %s", r.URL.Path, err)
 			return true
 		}
 		w.WriteHeader(http.StatusNoContent)
@@ -163,7 +163,7 @@ func requestHandler(w http.ResponseWriter, r *http.Request) bool {
 		vmimportRequests.Inc()
 		if err := vmimport.InsertHandler(at, r); err != nil {
 			vmimportErrors.Inc()
-			httpserver.Errorf(w, "error in %q: %s", r.URL.Path, err)
+			httpserver.Errorf(w, r, "error in %q: %s", r.URL.Path, err)
 			return true
 		}
 		w.WriteHeader(http.StatusNoContent)
@@ -172,7 +172,7 @@ func requestHandler(w http.ResponseWriter, r *http.Request) bool {
 		csvimportRequests.Inc()
 		if err := csvimport.InsertHandler(at, r); err != nil {
 			csvimportErrors.Inc()
-			httpserver.Errorf(w, "error in %q: %s", r.URL.Path, err)
+			httpserver.Errorf(w, r, "error in %q: %s", r.URL.Path, err)
 			return true
 		}
 		w.WriteHeader(http.StatusNoContent)
@@ -181,7 +181,7 @@ func requestHandler(w http.ResponseWriter, r *http.Request) bool {
 		prometheusimportRequests.Inc()
 		if err := prometheusimport.InsertHandler(at, r); err != nil {
 			prometheusimportErrors.Inc()
-			httpserver.Errorf(w, "error in %q: %s", r.URL.Path, err)
+			httpserver.Errorf(w, r, "error in %q: %s", r.URL.Path, err)
 			return true
 		}
 		w.WriteHeader(http.StatusNoContent)
@@ -190,7 +190,7 @@ func requestHandler(w http.ResponseWriter, r *http.Request) bool {
 		influxWriteRequests.Inc()
 		if err := influx.InsertHandlerForHTTP(at, r); err != nil {
 			influxWriteErrors.Inc()
-			httpserver.Errorf(w, "error in %q: %s", r.URL.Path, err)
+			httpserver.Errorf(w, r, "error in %q: %s", r.URL.Path, err)
 			return true
 		}
 		w.WriteHeader(http.StatusNoContent)

app/vmselect/main.go

@@ -144,7 +144,7 @@ func requestHandler(w http.ResponseWriter, r *http.Request) bool {
 					*maxConcurrentRequests, *maxQueueDuration),
 				StatusCode: http.StatusServiceUnavailable,
 			}
-			httpserver.Errorf(w, "%s", err)
+			httpserver.Errorf(w, r, "%s", err)
 			return true
 		}
 	}
@@ -161,12 +161,12 @@ func requestHandler(w http.ResponseWriter, r *http.Request) bool {
 
 	p, err := httpserver.ParsePath(path)
 	if err != nil {
-		httpserver.Errorf(w, "cannot parse path %q: %s", path, err)
+		httpserver.Errorf(w, r, "cannot parse path %q: %s", path, err)
 		return true
 	}
 	at, err := auth.NewToken(p.AuthToken)
 	if err != nil {
-		httpserver.Errorf(w, "auth error: %s", err)
+		httpserver.Errorf(w, r, "auth error: %s", err)
 		return true
 	}
 	switch p.Prefix {
@@ -267,7 +267,7 @@ func selectHandler(startTime time.Time, w http.ResponseWriter, r *http.Request,
 		exportRequests.Inc()
 		if err := prometheus.ExportHandler(startTime, at, w, r); err != nil {
 			exportErrors.Inc()
-			httpserver.Errorf(w, "error in %q: %s", r.URL.Path, err)
+			httpserver.Errorf(w, r, "error in %q: %s", r.URL.Path, err)
 			return true
 		}
 		return true
@@ -275,7 +275,7 @@ func selectHandler(startTime time.Time, w http.ResponseWriter, r *http.Request,
 		federateRequests.Inc()
 		if err := prometheus.FederateHandler(startTime, at, w, r); err != nil {
 			federateErrors.Inc()
-			httpserver.Errorf(w, "error in %q: %s", r.URL.Path, err)
+			httpserver.Errorf(w, r, "error in %q: %s", r.URL.Path, err)
 			return true
 		}
 		return true
@@ -308,7 +308,7 @@ func deleteHandler(startTime time.Time, w http.ResponseWriter, r *http.Request,
 		deleteRequests.Inc()
 		if err := prometheus.DeleteHandler(startTime, at, r); err != nil {
 			deleteErrors.Inc()
-			httpserver.Errorf(w, "error in %q: %s", r.URL.Path, err)
+			httpserver.Errorf(w, r, "error in %q: %s", r.URL.Path, err)
 			return true
 		}
 		w.WriteHeader(http.StatusNoContent)

app/vmstorage/main.go

@@ -116,7 +116,7 @@ func requestHandler(w http.ResponseWriter, r *http.Request, strg *storage.Storag
 	}
 	authKey := r.FormValue("authKey")
 	if authKey != *snapshotAuthKey {
-		httpserver.Errorf(w, "invalid authKey %q. It must match the value from -snapshotAuthKey command line flag", authKey)
+		httpserver.Errorf(w, r, "invalid authKey %q. It must match the value from -snapshotAuthKey command line flag", authKey)
 		return true
 	}
 	path = path[len("/snapshot"):]

lib/httpserver/httpserver.go

@@ -147,7 +147,7 @@ func handlerWrapper(s *server, w http.ResponseWriter, r *http.Request, rh Reques
 	requestsTotal.Inc()
 	path, err := getCanonicalPath(r.URL.Path)
 	if err != nil {
-		Errorf(w, "cannot get canonical path: %s", err)
+		Errorf(w, r, "cannot get canonical path: %s", err)
 		unsupportedRequestErrors.Inc()
 		return
 	}
@@ -201,7 +201,7 @@ func handlerWrapper(s *server, w http.ResponseWriter, r *http.Request, rh Reques
 			return
 		}
 
-		Errorf(w, "unsupported path requested: %q", r.URL.Path)
+		Errorf(w, r, "unsupported path requested: %q", r.URL.Path)
 		unsupportedRequestErrors.Inc()
 		return
 	}
@@ -431,8 +431,9 @@ var (
 )
 
 // Errorf writes formatted error message to w and to logger.
-func Errorf(w http.ResponseWriter, format string, args ...interface{}) {
+func Errorf(w http.ResponseWriter, r *http.Request, format string, args ...interface{}) {
 	errStr := fmt.Sprintf(format, args...)
+	errStr = fmt.Sprintf("remoteAddr: %s; %s", r.RemoteAddr, errStr)
 	logger.WarnfSkipframes(1, "%s", errStr)
 
 	// Extract statusCode from args

lib/ingestserver/opentsdbhttp/server.go

@@ -28,7 +28,7 @@ type Server struct {
 // MustStart starts HTTP OpenTSDB server on the given addr.
 //
 // MustStop must be called on the returned server when it is no longer needed.
-func MustStart(addr string, insertHandler func(req *http.Request) error) *Server {
+func MustStart(addr string, insertHandler func(r *http.Request) error) *Server {
 	logger.Infof("starting HTTP OpenTSDB server at %q", addr)
 	lnTCP, err := netutil.NewTCPListener("opentsdbhttp", addr)
 	if err != nil {
@@ -40,7 +40,7 @@ func MustStart(addr string, insertHandler func(req *http.Request) error) *Server
 // MustServe serves OpenTSDB HTTP put requests from ln.
 //
 // MustStop must be called on the returned server when it is no longer needed.
-func MustServe(ln net.Listener, insertHandler func(req *http.Request) error) *Server {
+func MustServe(ln net.Listener, insertHandler func(r *http.Request) error) *Server {
 	h := newRequestHandler(insertHandler)
 	hs := &http.Server{
 		Handler:           h,
@@ -84,12 +84,12 @@ func (s *Server) MustStop() {
 	logger.Infof("OpenTSDB HTTP server at %q has been stopped", s.ln.Addr())
 }
 
-func newRequestHandler(insertHandler func(req *http.Request) error) http.Handler {
+func newRequestHandler(insertHandler func(r *http.Request) error) http.Handler {
-	rh := func(w http.ResponseWriter, req *http.Request) {
+	rh := func(w http.ResponseWriter, r *http.Request) {
 		writeRequests.Inc()
-		if err := insertHandler(req); err != nil {
+		if err := insertHandler(r); err != nil {
 			writeErrors.Inc()
-			httpserver.Errorf(w, "error in %q: %s", req.URL.Path, err)
+			httpserver.Errorf(w, r, "error in %q: %s", r.URL.Path, err)
 			return
 		}
 		w.WriteHeader(http.StatusNoContent)