Skip to content

Commit

Permalink
Merge branch 'main' into main
Browse files Browse the repository at this point in the history
  • Loading branch information
msaf1980 authored Aug 17, 2023
2 parents d05178f + 3d57703 commit a23ef54
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 70 deletions.
24 changes: 11 additions & 13 deletions cmd/carbonapi/http/find_handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ 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 @@ -176,7 +175,6 @@ 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 @@ -199,7 +197,7 @@ func findHandler(w http.ResponseWriter, r *http.Request) {
var accessLogDetails = carbonapipb.AccessLogDetails{
Handler: "find",
Username: username,
CarbonapiUUID: carbonapiUUID,
CarbonapiUUID: uid.String(),
URL: r.URL.RequestURI(),
PeerIP: srcIP,
PeerPort: srcPort,
Expand All @@ -216,9 +214,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
writeErrorResponse(w, int(accessLogDetails.HTTPCode), accessLogDetails.Reason, carbonapiUUID)
logAsError = true
return
}

Expand All @@ -241,16 +240,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()
writeErrorResponse(w, http.StatusBadRequest, accessLogDetails.Reason, carbonapiUUID)
http.Error(w, "bad request (failed to parse format): "+err.Error(), http.StatusBadRequest)
return
}

err = pv3Request.Unmarshal(body)
if err != nil {
w.Header().Set(ctxHeaderUUID, carbonapiUUID)
accessLogDetails.HTTPCode = http.StatusBadRequest
accessLogDetails.Reason = "failed to parse message body: " + err.Error()
writeErrorResponse(w, http.StatusBadRequest, accessLogDetails.Reason, carbonapiUUID)
http.Error(w, "bad request (failed to parse format): "+err.Error(), http.StatusBadRequest)
return
}
} else {
Expand All @@ -260,9 +258,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`"
writeErrorResponse(w, http.StatusBadRequest, accessLogDetails.Reason, carbonapiUUID)
logAsError = true
return
}

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

logAsError = true
return
}

writeResponse(w, http.StatusOK, b, format, jsonp, carbonapiUUID)
writeResponse(w, http.StatusOK, b, format, jsonp, uid.String())
}
5 changes: 0 additions & 5 deletions cmd/carbonapi/http/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,11 +158,6 @@ 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
24 changes: 11 additions & 13 deletions cmd/carbonapi/http/tags_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,15 @@ import (

func tagHandler(w http.ResponseWriter, r *http.Request) {
t0 := time.Now()
uid := uuid.NewV4()
carbonapiUUID := uid.String()
uuid := uuid.NewV4()

// TODO: Migrate to context.WithTimeout
ctx := utilctx.SetUUID(r.Context(), uid.String())
ctx := utilctx.SetUUID(r.Context(), uuid.String())
requestHeaders := utilctx.GetLogHeaders(ctx)
username, _, _ := r.BasicAuth()

logger := zapwriter.Logger("tag").With(
zap.String("carbonapi_uuid", uid.String()),
zap.String("carbonapi_uuid", uuid.String()),
zap.String("username", username),
zap.Any("request_headers", requestHeaders),
)
Expand All @@ -40,7 +39,7 @@ func tagHandler(w http.ResponseWriter, r *http.Request) {
var accessLogDetails = &carbonapipb.AccessLogDetails{
Handler: "tags",
Username: username,
CarbonapiUUID: carbonapiUUID,
CarbonapiUUID: uuid.String(),
URL: r.URL.Path,
PeerIP: srcIP,
PeerPort: srcPort,
Expand All @@ -58,8 +57,8 @@ func tagHandler(w http.ResponseWriter, r *http.Request) {
err := r.ParseForm()
if err != nil {
logAsError = true
accessLogDetails.HTTPCode = int32(http.StatusBadRequest)
writeErrorResponse(w, http.StatusBadRequest, accessLogDetails.Reason, carbonapiUUID)
w.Header().Set("Content-Type", contentTypeJSON)
_, _ = w.Write([]byte{'[', ']'})
return
}

Expand Down Expand Up @@ -88,16 +87,15 @@ func tagHandler(w http.ResponseWriter, r *http.Request) {
} else if strings.HasSuffix(r.URL.Path, "values") || strings.HasSuffix(r.URL.Path, "values/") {
res, err = config.Config.ZipperInstance.TagValues(ctx, rawQuery, limit)
} else {
writeErrorResponse(w, http.StatusNotFound, http.StatusText(http.StatusNotFound), carbonapiUUID)
http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound)
accessLogDetails.HTTPCode = http.StatusNotFound
return
}

// TODO(civil): Implement stats
if err != nil && !merry.Is(err, types.ErrNoMetricsFetched) && !merry.Is(err, types.ErrNonFatalErrors) {
code := merry.HTTPCode(err)
writeErrorResponse(w, code, http.StatusText(code), carbonapiUUID)
accessLogDetails.HTTPCode = int32(code)
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
accessLogDetails.HTTPCode = http.StatusInternalServerError
accessLogDetails.Reason = err.Error()
logAsError = true
return
Expand All @@ -111,15 +109,15 @@ func tagHandler(w http.ResponseWriter, r *http.Request) {
}

if err != nil {
writeErrorResponse(w, http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError), carbonapiUUID)
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
accessLogDetails.HTTPCode = http.StatusInternalServerError
accessLogDetails.Reason = err.Error()
logAsError = true
return
}

w.Header().Set("Content-Type", contentTypeJSON)
w.Header().Set(ctxHeaderUUID, uid.String())
w.Header().Set(ctxHeaderUUID, uuid.String())
_, _ = w.Write(b)
accessLogDetails.Runtime = time.Since(t0).Seconds()
accessLogDetails.HTTPCode = http.StatusOK
Expand Down
9 changes: 5 additions & 4 deletions zipper/broadcast/broadcast_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -607,8 +607,7 @@ func (bg *BroadcastGroup) Find(ctx context.Context, request *protov3.MultiGlobRe
if len(result.Response.Metrics) == 0 {
nonNotFoundErrors := types.ReturnNonNotFoundError(result.Err)
if nonNotFoundErrors != nil {
code := helper.MergeHttpErrorsCode(result.Err)
err := types.ErrFailedToFetch.WithHTTPCode(code)
err := types.ErrFailedToFetch.WithHTTPCode(500)
for _, e := range nonNotFoundErrors {
err = err.WithCause(e)
}
Expand Down Expand Up @@ -819,8 +818,10 @@ func (bg *BroadcastGroup) tagEverything(ctx context.Context, isTagName bool, que

var err merry.Error
if result.Err != nil {
code := helper.MergeHttpErrorsCode(result.Err)
err = types.ErrFailedToFetch.WithHTTPCode(code)
err = types.ErrNonFatalErrors
for _, e := range result.Err {
err = err.WithCause(e)
}
}

return result.Response, err
Expand Down
35 changes: 0 additions & 35 deletions zipper/helper/requests.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,41 +98,6 @@ func requestError(err error, server string) merry.Error {
return types.ErrResponceError.WithValue("server", server)
}

func MergeHttpErrorsCode(errors []merry.Error) (returnCode int) {
returnCode = http.StatusNotFound
for _, err := range errors {
c := merry.RootCause(err)
if c == nil {
c = err
}

code := merry.HTTPCode(err)
if code == http.StatusNotFound {
continue
} else if code == http.StatusInternalServerError && merry.Is(c, parser.ErrInvalidArg) {
// check for invalid args, see applyByNode rewrite function
code = http.StatusBadRequest
}

if code == http.StatusGatewayTimeout || code == http.StatusBadGateway {
// simplify code, one error type for communications errors, all we can retry
code = http.StatusServiceUnavailable
}

if code == http.StatusBadRequest {
// The 400 is returned on wrong requests, e.g. non-existent functions
returnCode = code
} else if returnCode == http.StatusNotFound || code == http.StatusForbidden {
// First error or access denied (may be limits or other)
returnCode = code
} else if code != http.StatusServiceUnavailable {
returnCode = code
}
}

return returnCode
}

func MergeHttpErrors(errors []merry.Error) (int, []string) {
returnCode := http.StatusNotFound
errMsgs := make([]string, 0)
Expand Down
1 change: 1 addition & 0 deletions zipper/zipper.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ func NewZipper(sender func(*types.Stats), cfg *config.Config, logger *zap.Logger
)
}


logger.Error("DEBUG ERROR LOGGGGG", zap.Any("cfg", cfg))
broadcastGroup, err := broadcast.New(
broadcast.WithLogger(logger),
Expand Down

0 comments on commit a23ef54

Please sign in to comment.