Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(render): return error on partial targets fetch #814

Merged
merged 2 commits into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 9 additions & 18 deletions cmd/carbonapi/http/find_handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -214,9 +215,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
}
Expand Down Expand Up @@ -244,17 +243,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 {
Expand All @@ -264,9 +261,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
}
Expand All @@ -289,9 +284,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, 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
Expand Down Expand Up @@ -371,9 +364,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
}
Expand Down
71 changes: 70 additions & 1 deletion cmd/carbonapi/http/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package http

import (
"fmt"
"html"
"net/http"
"strings"
"time"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
26 changes: 14 additions & 12 deletions cmd/carbonapi/http/render_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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...)
Expand All @@ -347,20 +349,20 @@ 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
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 == 404 {
if returnCode == http.StatusNotFound {
returnCode = config.Config.NotFoundStatusCode
}

if returnCode == 400 || returnCode == http.StatusForbidden || returnCode >= 500 {
setError(w, accessLogDetails, strings.Join(errMsgs, ","), returnCode, uid.String())
if returnCode == http.StatusBadRequest || returnCode == http.StatusNotFound || returnCode == http.StatusForbidden || returnCode >= 500 {
msaf1980 marked this conversation as resolved.
Show resolved Hide resolved
setErrors(w, accessLogDetails, errMsgs, returnCode, uid.String())
logAsError = true
return
}
Expand Down
9 changes: 5 additions & 4 deletions cmd/carbonapi/http/tags_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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,
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions cmd/mockbackend/carbonapi_singlebackend.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: ""
Expand Down
Loading
Loading