feat(render,find): strip private info from returned errors info

cmd/carbonapi/http/find_handlers.go

@@ -23,6 +23,7 @@ import (
 	"github.com/go-graphite/carbonapi/date"
 	"github.com/go-graphite/carbonapi/intervalset"
 	utilctx "github.com/go-graphite/carbonapi/util/ctx"
+	"github.com/go-graphite/carbonapi/zipper/helper"
 )
 
 // Find handler and it's helper functions
@@ -283,7 +284,7 @@ func findHandler(w http.ResponseWriter, r *http.Request) {
 			if returnCode < 300 {
 				multiGlobs = &pbv3.MultiGlobResponse{Metrics: []pbv3.GlobResponse{}}
 			} else {
-				setError(w, &accessLogDetails, http.StatusText(returnCode), returnCode, uid.String())
+				setError(w, &accessLogDetails, helper.MerryRootError(err), returnCode, uid.String())
 				// We don't want to log this as an error if it's something normal
 				// Normal is everything that is >= 500. So if config.Config.NotFoundStatusCode is 500 - this will be
 				// logged as error

cmd/carbonapi/http/helper.go

@@ -2,6 +2,7 @@ package http
 
 import (
 	"fmt"
+	"html"
 	"net/http"
 	"strings"
 	"time"
@@ -229,6 +230,27 @@ func splitRemoteAddr(addr string) (string, string) {
 	return tmp[0], tmp[1]
 }
 
+func stripKey(key string, n int) string {
+	if len(key) > n+3 {
+		key = key[:n/2] + "..." + key[n/2+1:]
+	}
+	return key
+}
+
+// stripError for strip network errors (ip and other private info)
+func stripError(err string) string {
+	if strings.Contains(err, "connection refused") {
+		return "connection refused"
+	} else if strings.Contains(err, " lookup ") {
+		return "lookup error"
+	} else if strings.Contains(err, "broken pipe") {
+		return "broken pipe"
+	} else if strings.Contains(err, " connection reset ") {
+		return "connection reset"
+	}
+	return html.EscapeString(err)
+}
+
 func buildParseErrorString(target, e string, err error) string {
 	msg := fmt.Sprintf("%s\n\n%-20s: %s\n", http.StatusText(http.StatusBadRequest), "Target", target)
 	if err != nil {
@@ -285,9 +307,56 @@ func timestampTruncate(ts int64, duration time.Duration, durations []config.Dura
 
 func setError(w http.ResponseWriter, accessLogDetails *carbonapipb.AccessLogDetails, msg string, status int, carbonapiUUID string) {
 	w.Header().Set(ctxHeaderUUID, carbonapiUUID)
-	http.Error(w, http.StatusText(status)+": "+msg, status)
+	if msg == "" {
+		msg = http.StatusText(status)
+	}
 	accessLogDetails.Reason = msg
 	accessLogDetails.HTTPCode = int32(status)
+	msg = html.EscapeString(stripError(msg))
+	http.Error(w, msg, status)
+}
+
+func joinErrors(errMap map[string]string, sep string, status int) (msg, reason string) {
+	if len(errMap) == 0 {
+		msg = http.StatusText(status)
+	} else {
+		var buf, rBuf strings.Builder
+		buf.Grow(512)
+		rBuf.Grow(512)
+
+		// map is unsorted, can produce flapping ordered output, not critical
+		for k, err := range errMap {
+			if buf.Len() > 0 {
+				buf.WriteString(sep)
+				rBuf.WriteString(sep)
+			}
+			buf.WriteString(html.EscapeString(stripKey(k, 128)))
+			rBuf.WriteString(k)
+			buf.WriteString(": ")
+			rBuf.WriteString(": ")
+			buf.WriteString(html.EscapeString(stripError(err)))
+			rBuf.WriteString(err)
+		}
+
+		msg = buf.String()
+		reason = rBuf.String()
+	}
+	return
+}
+
+func setErrors(w http.ResponseWriter, accessLogDetails *carbonapipb.AccessLogDetails, errMamp map[string]string, status int, carbonapiUUID string) {
+	w.Header().Set(ctxHeaderUUID, carbonapiUUID)
+	var msg string
+	if status != http.StatusOK {
+		if len(errMamp) == 0 {
+			msg = http.StatusText(status)
+			accessLogDetails.Reason = msg
+		} else {
+			msg, accessLogDetails.Reason = joinErrors(errMamp, "\n", status)
+		}
+	}
+	accessLogDetails.HTTPCode = int32(status)
+	http.Error(w, msg, status)
 }
 
 func queryLengthLimitExceeded(query []string, maxLength uint64) bool {

cmd/carbonapi/http/render_handler.go

@@ -353,16 +353,16 @@ func renderHandler(w http.ResponseWriter, r *http.Request) {
 		// Obtain error code from the errors
 		// In case we have only "Not Found" errors, result should be 404
 		// Otherwise it should be 500
-		var errMsgs []string
+		var errMsgs map[string]string
 		returnCode, errMsgs = helper.MergeHttpErrorMap(errors)
-		logger.Debug("error response or no response", zap.Strings("error", errMsgs))
+		logger.Debug("error response or no response", zap.Any("error", errMsgs))
 		// Allow override status code for 404-not-found replies.
 		if returnCode == http.StatusNotFound {
 			returnCode = config.Config.NotFoundStatusCode
 		}
 
 		if returnCode == http.StatusBadRequest || returnCode == http.StatusNotFound || returnCode == http.StatusForbidden || returnCode >= 500 {
-			setError(w, accessLogDetails, strings.Join(errMsgs, ","), returnCode, uid.String())
+			setErrors(w, accessLogDetails, errMsgs, returnCode, uid.String())
 			logAsError = true
 			return
 		}

cmd/carbonapi/http/tags_handler.go

@@ -21,9 +21,10 @@ import (
 func tagHandler(w http.ResponseWriter, r *http.Request) {
 	t0 := time.Now()
 	uuid := uuid.NewV4()
+	carbonapiUUID := uuid.String()
 
 	// TODO: Migrate to context.WithTimeout
-	ctx := utilctx.SetUUID(r.Context(), uuid.String())
+	ctx := utilctx.SetUUID(r.Context(), carbonapiUUID)
 	requestHeaders := utilctx.GetLogHeaders(ctx)
 	username, _, _ := r.BasicAuth()
 
@@ -39,7 +40,7 @@ func tagHandler(w http.ResponseWriter, r *http.Request) {
 	var accessLogDetails = &carbonapipb.AccessLogDetails{
 		Handler:        "tags",
 		Username:       username,
-		CarbonapiUUID:  uuid.String(),
+		CarbonapiUUID:  carbonapiUUID,
 		URL:            r.URL.Path,
 		PeerIP:         srcIP,
 		PeerPort:       srcPort,
@@ -81,7 +82,7 @@ func tagHandler(w http.ResponseWriter, r *http.Request) {
 	rawQuery := q.Encode()
 
 	if queryLengthLimitExceeded(r.Form["query"], config.Config.MaxQueryLength) {
-		setError(w, accessLogDetails, "query length limit exceeded", http.StatusBadRequest, uuid.String())
+		setError(w, accessLogDetails, "query length limit exceeded", http.StatusBadRequest, carbonapiUUID)
 		logAsError = true
 		return
 	}
@@ -123,7 +124,7 @@ func tagHandler(w http.ResponseWriter, r *http.Request) {
 	}
 
 	w.Header().Set("Content-Type", contentTypeJSON)
-	w.Header().Set(ctxHeaderUUID, uuid.String())
+	w.Header().Set(ctxHeaderUUID, carbonapiUUID)
 	_, _ = w.Write(b)
 	accessLogDetails.Runtime = time.Since(t0).Seconds()
 	accessLogDetails.HTTPCode = http.StatusOK

cmd/mockbackend/e2etesting.go

@@ -13,6 +13,7 @@ import (
 	"net/url"
 	"os"
 	"reflect"
+	"sort"
 	"strconv"
 	"strings"
 	"sync"
@@ -45,6 +46,8 @@ type Query struct {
 type ExpectedResponse struct {
 	HttpCode        int              `yaml:"httpCode"`
 	ContentType     string           `yaml:"contentType"`
+	ErrBody         string           `yaml:"errBody"`
+	ErrSort         bool             `yaml:"errSort"`
 	ExpectedResults []ExpectedResult `yaml:"expectedResults"`
 }
 
@@ -245,8 +248,22 @@ func doTest(logger *zap.Logger, t *Query, verbose bool) []error {
 		)
 	}
 
-	// We don't need to actually check body of response if we expect any sort of error (4xx/5xx)
+	// We don't need to actually check body of response if we expect any sort of error (4xx/5xx), but for check error handling do this
 	if t.ExpectedResponse.HttpCode >= 300 {
+		if t.ExpectedResponse.ErrBody != "" {
+			errStr := string(b)
+			if t.ExpectedResponse.ErrSort && strings.Index(errStr, "\n") != len(errStr)-1 {
+				// resort error string
+				errs := sort.StringSlice(strings.Split(errStr, "\n"))
+				if errs[len(errs)-1] == "" {
+					errs = errs[:len(errs)-1]
+				}
+				errStr = strings.Join(errs, "\n") + "\n"
+			}
+			if t.ExpectedResponse.ErrBody != errStr {
+				failures = append(failures, merry2.Errorf("mismatch error body, got '%s', expected '%s'", string(b), t.ExpectedResponse.ErrBody))
+			}
+		}
 		return failures
 	}
 

cmd/mockbackend/testcases/find_error/find_error.yaml

@@ -61,6 +61,25 @@ test:
           expectedResponse:
               httpCode: 503
               contentType: "text/plain; charset=utf-8"
+              errBody: "Service Unavailable\n"
+
+        # 503
+        - endpoint: "http://127.0.0.1:8081"
+          type: "GET"
+          URL: "/metrics/find?query=c&query=d&format=json"
+          expectedResponse:
+              httpCode: 503
+              contentType: "text/plain; charset=utf-8"
+              errBody: "timeout while fetching Response\n"
+
+        # 503
+        - endpoint: "http://127.0.0.1:8081"
+          type: "GET"
+          URL: "/metrics/find?query=d&query=e&format=json"
+          expectedResponse:
+              httpCode: 503
+              contentType: "text/plain; charset=utf-8"
+              errBody: "Service Unavailable\n"
 
         # 503, partial success
         - endpoint: "http://127.0.0.1:8081"
@@ -69,6 +88,7 @@ test:
           expectedResponse:
               httpCode: 503
               contentType: "text/plain; charset=utf-8"
+              errBody: "Service Unavailable\n"
 
 listeners:
       - address: ":9070"
@@ -91,3 +111,7 @@ listeners:
             "d":
                 pathExpression: "d"
                 code: 503
+
+            "e":
+                pathExpression: "e"
+                code: 503

cmd/mockbackend/testcases/render_error_all/render_error_all.yaml

@@ -45,6 +45,7 @@ test:
           expectedResponse:
               httpCode: 503
               contentType: "text/plain; charset=utf-8"
+              errBody: "c: timeout while fetching Response\n"
 
         # 503
         - endpoint: "http://127.0.0.1:8081"
@@ -53,6 +54,7 @@ test:
           expectedResponse:
               httpCode: 503
               contentType: "text/plain; charset=utf-8"
+              errBody: "d: Service Unavailable\n"
 
         # partial success
         - endpoint: "http://127.0.0.1:8081"
@@ -61,6 +63,7 @@ test:
           expectedResponse:
               httpCode: 503
               contentType: "text/plain; charset=utf-8"
+              errBody: "d: Service Unavailable\n"
 
         # partial success, must fail, target d failed
         - endpoint: "http://127.0.0.1:8081"
@@ -69,6 +72,7 @@ test:
           expectedResponse:
               httpCode: 503
               contentType: "text/plain; charset=utf-8"
+              errBody: "divideSeries(a,d): Service Unavailable\n"
 
 listeners:
       - address: ":9070"

cmd/mockbackend/testcases/render_error_all_rr/render_error_all_rr.yaml

@@ -81,6 +81,7 @@ test:
           expectedResponse:
               httpCode: 503
               contentType: "text/plain; charset=utf-8"
+              errBody: "d: Service Unavailable\n"
 
         # partial success
         - endpoint: "http://127.0.0.1:8081"
@@ -89,6 +90,7 @@ test:
           expectedResponse:
               httpCode: 503
               contentType: "text/plain; charset=utf-8"
+              errBody: "d: Service Unavailable\n"
 
         # partial success
         # TODO: must fail, target d failed
@@ -98,6 +100,7 @@ test:
           expectedResponse:
             httpCode: 503
             contentType: "text/plain; charset=utf-8"
+            errBody: "divideSeries(a,d): Service Unavailable\n"
 
 listeners:
       - address: ":9070"

cmd/mockbackend/testcases/render_error_refused/carbonapi.yaml

@@ -0,0 +1,57 @@
+listen: "localhost:8081"
+expvar:
+  enabled: true
+  pprofEnabled: false
+  listen: ""
+concurency: 1000
+notFoundStatusCode: 200
+cache:
+   type: "mem"
+   size_mb: 0
+   defaultTimeoutSec: 60
+cpus: 0
+tz: ""
+maxBatchSize: 0
+graphite:
+    host: ""
+    interval: "60s"
+    prefix: "carbon.api"
+    pattern: "{prefix}.{fqdn}"
+idleConnections: 10
+pidFile: ""
+upstreams:
+    buckets: 10
+    timeouts:
+        find: "2s"
+        render: "5s"
+        connect: "200ms"
+    concurrencyLimitPerServer: 0
+    keepAliveInterval: "30s"
+    maxIdleConnsPerHost: 100
+    backendsv2:
+        backends:
+          -
+            groupName: "mock-001"
+            protocol: "auto"
+            lbMethod: "all"
+            maxTries: 1
+            maxBatchSize: 0
+            keepAliveInterval: "10s"
+            concurrencyLimit: 0
+            forceAttemptHTTP2: true
+            maxIdleConnsPerHost: 1000
+            timeouts:
+                find: "3s"
+                render: "5s"
+                connect: "200ms"
+            servers:
+                - "http://127.0.0.1:9071"
+graphite09compat: false
+expireDelaySec: 10
+logger:
+    - logger: ""
+      file: "stderr"
+      level: "debug"
+      encoding: "console"
+      encodingTime: "iso8601"
+      encodingDuration: "seconds"