feat(render,find): return error on partial targets fetch and return d…

…etailed error instead of 500
go-graphite · Mar 21, 2024 · b84ae8f · b84ae8f
1 parent dd8dde6
commit b84ae8f
Show file tree

Hide file tree

Showing 60 changed files with 1,275 additions and 313 deletions.
diff --git a/cmd/carbonapi/http/find_handlers.go b/cmd/carbonapi/http/find_handlers.go
@@ -214,9 +214,7 @@ func findHandler(w http.ResponseWriter, r *http.Request) {
 	}()
 
 	if !ok || !format.ValidFindFormat() {
-		http.Error(w, "unsupported format: "+formatRaw, http.StatusBadRequest)
-		accessLogDetails.HTTPCode = http.StatusBadRequest
-		accessLogDetails.Reason = "unsupported format: " + formatRaw
+		setError(w, &accessLogDetails, "unsupported format: "+formatRaw, http.StatusBadRequest, uid.String())
 		logAsError = true
 		return
 	}
@@ -244,17 +242,15 @@ func findHandler(w http.ResponseWriter, r *http.Request) {
 	if format == protoV3Format {
 		body, err := io.ReadAll(r.Body)
 		if err != nil {
-			accessLogDetails.HTTPCode = http.StatusBadRequest
-			accessLogDetails.Reason = "failed to parse message body: " + err.Error()
-			http.Error(w, "bad request (failed to parse format): "+err.Error(), http.StatusBadRequest)
+			setError(w, &accessLogDetails, "failed to parse message body: "+err.Error(), http.StatusBadRequest, uid.String())
+			logAsError = true
 			return
 		}
 
 		err = pv3Request.Unmarshal(body)
 		if err != nil {
-			accessLogDetails.HTTPCode = http.StatusBadRequest
-			accessLogDetails.Reason = "failed to parse message body: " + err.Error()
-			http.Error(w, "bad request (failed to parse format): "+err.Error(), http.StatusBadRequest)
+			setError(w, &accessLogDetails, "failed to parse message body: "+err.Error(), http.StatusBadRequest, uid.String())
+			logAsError = true
 			return
 		}
 	} else {
@@ -264,9 +260,7 @@ func findHandler(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if len(pv3Request.Metrics) == 0 {
-		http.Error(w, "missing parameter `query`", http.StatusBadRequest)
-		accessLogDetails.HTTPCode = http.StatusBadRequest
-		accessLogDetails.Reason = "missing parameter `query`"
+		setError(w, &accessLogDetails, "missing parameter `query`", http.StatusBadRequest, uid.String())
 		logAsError = true
 		return
 	}
@@ -289,9 +283,7 @@ func findHandler(w http.ResponseWriter, r *http.Request) {
 			if returnCode < 300 {
 				multiGlobs = &pbv3.MultiGlobResponse{Metrics: []pbv3.GlobResponse{}}
 			} else {
-				http.Error(w, http.StatusText(returnCode), returnCode)
-				accessLogDetails.HTTPCode = int32(returnCode)
-				accessLogDetails.Reason = err.Error()
+				setError(w, &accessLogDetails, http.StatusText(returnCode), 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
@@ -371,9 +363,7 @@ func findHandler(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if err != nil {
-		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
-		accessLogDetails.HTTPCode = http.StatusInternalServerError
-		accessLogDetails.Reason = err.Error()
+		setError(w, &accessLogDetails, err.Error(), http.StatusInternalServerError, uid.String())
 		logAsError = true
 		return
 	}

diff --git a/cmd/carbonapi/http/render_handler.go b/cmd/carbonapi/http/render_handler.go
@@ -201,19 +201,15 @@ func renderHandler(w http.ResponseWriter, r *http.Request) {
 	if format == protoV3Format {
 		body, err := io.ReadAll(r.Body)
 		if err != nil {
-			accessLogDetails.HTTPCode = http.StatusBadRequest
-			accessLogDetails.Reason = "failed to parse message body: " + err.Error()
-			http.Error(w, "bad request (failed to parse format): "+err.Error(), http.StatusBadRequest)
+			setError(w, accessLogDetails, "failed to parse message body: "+err.Error(), http.StatusBadRequest, uid.String())
 			return
 		}
 
 		var pv3Request pb.MultiFetchRequest
 		err = pv3Request.Unmarshal(body)
 
 		if err != nil {
-			accessLogDetails.HTTPCode = http.StatusBadRequest
-			accessLogDetails.Reason = "failed to parse message body: " + err.Error()
-			http.Error(w, "bad request (failed to parse format): "+err.Error(), http.StatusBadRequest)
+			setError(w, accessLogDetails, "failed to parse message body: "+err.Error(), http.StatusBadRequest, uid.String())
 			return
 		}
 
@@ -327,6 +323,12 @@ func renderHandler(w http.ResponseWriter, r *http.Request) {
 				result, err := expr.FetchAndEvalExp(ctx, config.Config.Evaluator, exp, from32, until32, values)
 				if err != nil {
 					errors[target] = merry.Wrap(err)
+					if config.Config.Upstreams.RequireSuccessAll {
+						code := merry.HTTPCode(err)
+						if code != http.StatusOK && code != http.StatusNotFound {
+							break
+						}
+					}
 				}
 
 				results = append(results, result...)
@@ -347,19 +349,19 @@ func renderHandler(w http.ResponseWriter, r *http.Request) {
 	var body []byte
 
 	returnCode := http.StatusOK
-	if len(results) == 0 {
+	if len(results) == 0 || (len(errors) > 0 && config.Config.Upstreams.RequireSuccessAll) {
 		// 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
 		returnCode, errMsgs = helper.MergeHttpErrorMap(errors)
 		logger.Debug("error response or no response", zap.Strings("error", errMsgs))
 		// Allow override status code for 404-not-found replies.
-		if returnCode == 404 {
+		if returnCode == http.StatusNotFound {
 			returnCode = config.Config.NotFoundStatusCode
 		}
 
-		if returnCode == 400 || returnCode == http.StatusForbidden || returnCode >= 500 {
+		if returnCode == http.StatusBadRequest || returnCode == http.StatusNotFound || returnCode == http.StatusForbidden || returnCode >= 500 {
 			setError(w, accessLogDetails, strings.Join(errMsgs, ","), returnCode, uid.String())
 			logAsError = true
 			return

diff --git a/cmd/mockbackend/carbonapi_singlebackend.yaml b/cmd/mockbackend/carbonapi_singlebackend.yaml
@@ -41,12 +41,12 @@ upstreams:
             forceAttemptHTTP2: true
             maxIdleConnsPerHost: 1000
             timeouts:
-                find: "15s"
-                render: "50s"
+                find: "15000s"
+                render: "5000s"
                 connect: "200ms"
             servers:
                 - "http://127.0.0.1:9070"
-    graphite09compat: false
+graphite09compat: false
 expireDelaySec: 10
 logger:
     - logger: ""