Skip to content

Commit

Permalink
fix(find): return error on partial failure of metrics find
Browse files Browse the repository at this point in the history
  • Loading branch information
msaf1980 committed Dec 28, 2023
1 parent 0f7d9fd commit e997608
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 9 deletions.
20 changes: 11 additions & 9 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 @@ -175,6 +176,7 @@ func findList(multiGlobs *pbv3.MultiGlobResponse) ([]byte, error) {
func findHandler(w http.ResponseWriter, r *http.Request) {
t0 := time.Now()
uid := uuid.NewV4()
carbonapiUUID := uid.String()
// TODO: Migrate to context.WithTimeout
// ctx, _ := context.WithTimeout(context.TODO(), config.Config.ZipperTimeout)
ctx := utilctx.SetUUID(r.Context(), uid.String())
Expand All @@ -197,7 +199,7 @@ func findHandler(w http.ResponseWriter, r *http.Request) {
var accessLogDetails = carbonapipb.AccessLogDetails{
Handler: "find",
Username: username,
CarbonapiUUID: uid.String(),
CarbonapiUUID: carbonapiUUID,
URL: r.URL.RequestURI(),
PeerIP: srcIP,
PeerPort: srcPort,
Expand All @@ -214,10 +216,10 @@ 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
logAsError = true
writeErrorResponse(w, int(accessLogDetails.HTTPCode), accessLogDetails.Reason, carbonapiUUID)
return
}

Expand All @@ -240,15 +242,15 @@ func findHandler(w http.ResponseWriter, r *http.Request) {
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)
writeErrorResponse(w, http.StatusBadRequest, accessLogDetails.Reason, carbonapiUUID)
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)
writeErrorResponse(w, http.StatusBadRequest, accessLogDetails.Reason, carbonapiUUID)
return
}
} else {
Expand All @@ -258,10 +260,10 @@ 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`"
logAsError = true
writeErrorResponse(w, http.StatusBadRequest, accessLogDetails.Reason, carbonapiUUID)
return
}

Expand All @@ -273,7 +275,7 @@ func findHandler(w http.ResponseWriter, r *http.Request) {
accessLogDetails.TotalMetricsCount += stats.TotalMetricsCount
}
if err != nil {
returnCode := merry.HTTPCode(err)
returnCode := merry.HTTPCode(helper.HttpErrorByCode(err))
if returnCode != http.StatusOK || multiGlobs == nil {
// Allow override status code for 404-not-found replies.
if returnCode == http.StatusNotFound {
Expand All @@ -283,9 +285,9 @@ 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()
writeErrorResponse(w, returnCode, accessLogDetails.Reason, carbonapiUUID)
// 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 @@ -365,12 +367,12 @@ 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()
logAsError = true
writeErrorResponse(w, http.StatusInternalServerError, accessLogDetails.Reason, carbonapiUUID)
return
}

writeResponse(w, http.StatusOK, b, format, jsonp, uid.String())
writeResponse(w, http.StatusOK, b, format, jsonp, carbonapiUUID)
}
5 changes: 5 additions & 0 deletions cmd/carbonapi/http/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,11 @@ func getFormat(r *http.Request, defaultFormat responseFormat) (responseFormat, b
return f, ok, format
}

func writeErrorResponse(w http.ResponseWriter, returnCode int, err, carbonapiUUID string) {
w.Header().Set(ctxHeaderUUID, carbonapiUUID)
http.Error(w, err, returnCode)
}

func writeResponse(w http.ResponseWriter, returnCode int, b []byte, format responseFormat, jsonp, carbonapiUUID string) {
//TODO: Simplify that switch
w.Header().Set(ctxHeaderUUID, carbonapiUUID)
Expand Down

0 comments on commit e997608

Please sign in to comment.