From b84ae8f4390345cf88a1b33966013d8d6f2dd4aa Mon Sep 17 00:00:00 2001 From: Michail Safronov Date: Sat, 20 Jan 2024 00:10:16 +0500 Subject: [PATCH 1/2] feat(render,find): return error on partial targets fetch and return detailed error instead of 500 --- cmd/carbonapi/http/find_handlers.go | 26 +-- cmd/carbonapi/http/render_handler.go | 20 +- cmd/mockbackend/carbonapi_singlebackend.yaml | 6 +- cmd/mockbackend/e2etesting.go | 206 ++++++++++++++---- cmd/mockbackend/find.go | 57 +++-- cmd/mockbackend/http_common.go | 2 + cmd/mockbackend/main.go | 13 +- cmd/mockbackend/render.go | 153 ++++++------- cmd/mockbackend/tags.go | 104 +++++++++ .../testcases/bad_requests/bad_requests.yaml | 5 +- .../testcases/find_error/find_error.yaml | 93 ++++++++ cmd/mockbackend/testcases/i484/carbonapi.yaml | 2 +- cmd/mockbackend/testcases/i484/i484.yaml | 1 + cmd/mockbackend/testcases/i503/i503.yaml | 4 +- cmd/mockbackend/testcases/i506/carbonapi.yaml | 9 +- cmd/mockbackend/testcases/i506/i506.yaml | 13 +- cmd/mockbackend/testcases/i516/i516.yaml | 1 + cmd/mockbackend/testcases/i517/i517.yaml | 1 + cmd/mockbackend/testcases/i545/carbonapi.yaml | 2 +- cmd/mockbackend/testcases/i545/i545.yaml | 1 + cmd/mockbackend/testcases/i565/i565.yaml | 1 + cmd/mockbackend/testcases/i580/i580.yaml | 1 + cmd/mockbackend/testcases/i584/i584.yaml | 1 + cmd/mockbackend/testcases/i589/i589.yaml | 1 + cmd/mockbackend/testcases/i598/carbonapi.yaml | 2 +- cmd/mockbackend/testcases/i598/i598.yaml | 1 + cmd/mockbackend/testcases/i661/carbonapi.yaml | 2 +- cmd/mockbackend/testcases/i661/i661.yaml | 1 + cmd/mockbackend/testcases/pr500/pr500.yaml | 1 + cmd/mockbackend/testcases/pr529/pr529.yaml | 1 + cmd/mockbackend/testcases/pr560/pr560.yaml | 1 + .../testcases/pr594-403-504/carbonapi.yaml | 2 +- .../pr594-403-504/pr594-403-504.yaml | 1 + .../testcases/pr594-maskerror/carbonapi.yaml | 2 +- .../pr594-maskerror/pr594-maskerror.yaml | 1 + .../testcases/pr743/carbonapi.yaml | 2 +- cmd/mockbackend/testcases/pr743/pr743.yaml | 1 + .../testcases/pr817/carbonapi.yaml | 2 +- cmd/mockbackend/testcases/pr817/pr817.yaml | 16 +- .../testcases/render_error/carbonapi.yaml | 57 +++++ .../testcases/render_error/render_error.yaml | 98 +++++++++ .../testcases/render_error_all/carbonapi.yaml | 58 +++++ .../render_error_all/render_error_all.yaml | 90 ++++++++ .../render_error_all_rr/carbonapi.yaml | 59 +++++ .../render_error_all_rr.yaml | 134 ++++++++++++ .../testcases/tags_error/tags_error.yaml | 86 ++++++++ zipper/broadcast/broadcast_group.go | 87 +++++--- zipper/broadcast/broadcast_group_test.go | 15 +- zipper/config/config.go | 1 + zipper/helper/requests.go | 26 +++ zipper/metadata/metadata.go | 8 +- zipper/protocols/auto/auto_group.go | 21 +- zipper/protocols/graphite/graphite_group.go | 6 +- zipper/protocols/irondb/irondb_group.go | 6 +- .../protocols/prometheus/prometheus_group.go | 10 +- zipper/protocols/v2/protobuf_group.go | 6 +- zipper/protocols/v3/protobuf_group.go | 6 +- .../victoriametrics/victoriametrics_group.go | 8 +- zipper/types/response.go | 10 +- zipper/zipper.go | 37 +--- 60 files changed, 1275 insertions(+), 313 deletions(-) create mode 100644 cmd/mockbackend/tags.go create mode 100644 cmd/mockbackend/testcases/find_error/find_error.yaml create mode 100644 cmd/mockbackend/testcases/render_error/carbonapi.yaml create mode 100644 cmd/mockbackend/testcases/render_error/render_error.yaml create mode 100644 cmd/mockbackend/testcases/render_error_all/carbonapi.yaml create mode 100644 cmd/mockbackend/testcases/render_error_all/render_error_all.yaml create mode 100644 cmd/mockbackend/testcases/render_error_all_rr/carbonapi.yaml create mode 100644 cmd/mockbackend/testcases/render_error_all_rr/render_error_all_rr.yaml create mode 100644 cmd/mockbackend/testcases/tags_error/tags_error.yaml diff --git a/cmd/carbonapi/http/find_handlers.go b/cmd/carbonapi/http/find_handlers.go index 7b7349326..298cae2c4 100644 --- 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 index 79b8a71cd..f2938527c 100644 --- a/cmd/carbonapi/http/render_handler.go +++ b/cmd/carbonapi/http/render_handler.go @@ -201,9 +201,7 @@ 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 } @@ -211,9 +209,7 @@ func renderHandler(w http.ResponseWriter, r *http.Request) { 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,7 +349,7 @@ 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 @@ -355,11 +357,11 @@ func renderHandler(w http.ResponseWriter, r *http.Request) { 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 index 7684ef9ba..63e9762a1 100644 --- 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: "" diff --git a/cmd/mockbackend/e2etesting.go b/cmd/mockbackend/e2etesting.go index 922246364..b8ab00ef3 100644 --- a/cmd/mockbackend/e2etesting.go +++ b/cmd/mockbackend/e2etesting.go @@ -12,6 +12,7 @@ import ( "net/http" "net/url" "os" + "reflect" "strconv" "strings" "sync" @@ -48,11 +49,22 @@ type ExpectedResponse struct { } type ExpectedResult struct { - SHA256 []string `yaml:"sha256"` - Metrics []CarbonAPIResponse + SHA256 []string `yaml:"sha256"` + Metrics []RenderResponse + MetricsFind []MetricsFindResponse `json:"metricsFind" yaml:"metricsFind"` + TagsAutocompelete []string `json:"tagsAutocompelete" yaml:"tagsAutocompelete"` } -type CarbonAPIResponse struct { +type MetricsFindResponse struct { + AllowChildren int `json:"allowChildren" yaml:"allowChildren"` + Expandable int `json:"expandable" yaml:"expandable"` + Leaf int `json:"leaf" yaml:"leaf"` + Id string `json:"id" yaml:"id"` + Text string `json:"text" yaml:"text"` + Context map[string]string `json:"context" yaml:"context"` +} + +type RenderResponse struct { Target string `json:"target" yaml:"target"` Datapoints []Datapoint `json:"datapoints" yaml:"datapoints"` Tags map[string]string `json:"tags" yaml:"tags"` @@ -121,7 +133,7 @@ func (d *Datapoint) UnmarshalYAML(unmarshal func(interface{}) error) error { return nil } -func isMetricsEqual(m1, m2 CarbonAPIResponse) error { +func isRenderEqual(m1, m2 RenderResponse) error { if m1.Target != m2.Target { return fmt.Errorf("target mismatch, got '%v', expected '%v'", m1.Target, m2.Target) } @@ -158,7 +170,14 @@ func isMetricsEqual(m1, m2 CarbonAPIResponse) error { return nil } -func doTest(logger *zap.Logger, t *Query) []error { +func max(a, b int) int { + if a > b { + return a + } + return b +} + +func doTest(logger *zap.Logger, t *Query, verbose bool) []error { client := http.Client{} failures := make([]error, 0) d, err := time.ParseDuration(fmt.Sprintf("%v", t.Delay) + "s") @@ -201,19 +220,11 @@ func doTest(logger *zap.Logger, t *Query) []error { return failures } - if resp.StatusCode != t.ExpectedResponse.HttpCode { - failures = append(failures, merry2.Errorf("unexpected status code, got %v, expected %v", - resp.StatusCode, - t.ExpectedResponse.HttpCode, - ), - ) - } - contentType = resp.Header.Get("Content-Type") if t.ExpectedResponse.ContentType != contentType { failures = append(failures, - merry2.Errorf("unexpected content-type, got %v, expected %v", - contentType, + merry2.Errorf("unexpected content-type, got %v (code %d), expected %v", + contentType, resp.StatusCode, t.ExpectedResponse.ContentType, ), ) @@ -226,6 +237,14 @@ func doTest(logger *zap.Logger, t *Query) []error { return failures } + if resp.StatusCode != t.ExpectedResponse.HttpCode { + failures = append(failures, merry2.Errorf("unexpected status code, got %v, expected %v", + resp.StatusCode, + t.ExpectedResponse.HttpCode, + ), + ) + } + // We don't need to actually check body of response if we expect any sort of error (4xx/5xx) if t.ExpectedResponse.HttpCode >= 300 { return failures @@ -245,45 +264,152 @@ func doTest(logger *zap.Logger, t *Query) []error { } if !sha256matched { encodedBody := base64.StdEncoding.EncodeToString(b) - failures = append(failures, merry2.Errorf("sha256 mismatch, got '%v', expected '%v', encodedBodyy: '%v'", hashStr, t.ExpectedResponse.ExpectedResults[0].SHA256, encodedBody)) + failures = append(failures, merry2.Errorf("sha256 mismatch, got '%v', expected '%v', encodedBody: '%v'", hashStr, t.ExpectedResponse.ExpectedResults[0].SHA256, encodedBody)) return failures } case "application/json": - res := make([]CarbonAPIResponse, 0, 1) - err := json.Unmarshal(b, &res) - if err != nil { - err = merry2.Prepend(err, "failed to parse response") - failures = append(failures, err) - return failures - } + if strings.HasPrefix(t.URL, "/metrics/find") { + res := make([]MetricsFindResponse, 0, 1) + err := json.Unmarshal(b, &res) + if err != nil { + err = merry2.Prepend(err, "failed to parse response") + failures = append(failures, err) + return failures + } - if len(t.ExpectedResponse.ExpectedResults) == 0 { - return failures - } + if len(t.ExpectedResponse.ExpectedResults) == 0 { + return failures + } - if len(res) != len(t.ExpectedResponse.ExpectedResults[0].Metrics) { - failures = append(failures, merry2.Errorf("unexpected amount of results, got %v, expected %v", - len(res), - len(t.ExpectedResponse.ExpectedResults[0].Metrics))) - return failures - } + if len(res) != len(t.ExpectedResponse.ExpectedResults[0].MetricsFind) { + failures = append(failures, merry2.Errorf("unexpected amount of metrics find, got %v, expected %v", + len(res), + len(t.ExpectedResponse.ExpectedResults[0].MetricsFind))) + if verbose { + length := max(len(t.ExpectedResponse.ExpectedResults[0].MetricsFind), len(res)) + for i := 0; i < length; i++ { + if i >= len(res) { + err = fmt.Errorf("metrics find[%d] want=`%+v`", i, t.ExpectedResponse.ExpectedResults[0].MetricsFind[i]) + failures = append(failures, err) + } else if i >= len(t.ExpectedResponse.ExpectedResults[0].MetricsFind) { + err = fmt.Errorf("metrics find[%d] got unexpected=`%+v`", i, res[i]) + failures = append(failures, err) + } else if !reflect.DeepEqual(res[i], t.ExpectedResponse.ExpectedResults[0].MetricsFind[i]) { + err = fmt.Errorf("metrics find[%d] are not equal, got=`%+v`, expected=`%+v`", i, res[i], t.ExpectedResponse.ExpectedResults[0].MetricsFind[i]) + failures = append(failures, err) + } + } + } + return failures + } - for i := range res { - err := isMetricsEqual(res[i], t.ExpectedResponse.ExpectedResults[0].Metrics[i]) + for i := range res { + if !reflect.DeepEqual(res[i], t.ExpectedResponse.ExpectedResults[0].MetricsFind[i]) { + err = fmt.Errorf("metrics find[%d] are not equal, got=`%+v`, expected=`%+v`", i, res[i], t.ExpectedResponse.ExpectedResults[0].MetricsFind[i]) + failures = append(failures, err) + } + } + } else if strings.HasPrefix(t.URL, "/tags/autoComplete/") { + // tags/autoComplete + res := make([]string, 0, 1) + err := json.Unmarshal(b, &res) if err != nil { - err = merry2.Prependf(err, "metrics are not equal, got=`%+v`, expected=`%+v`", res[i], t.ExpectedResponse.ExpectedResults[0].Metrics[i]) + err = merry2.Prepend(err, "failed to parse response") failures = append(failures, err) + return failures + } + + if len(t.ExpectedResponse.ExpectedResults) == 0 { + return failures } - } + if len(res) != len(t.ExpectedResponse.ExpectedResults[0].TagsAutocompelete) { + failures = append(failures, merry2.Errorf("unexpected amount of results, got %v, expected %v", + len(res), + len(t.ExpectedResponse.ExpectedResults[0].TagsAutocompelete))) + if verbose { + length := max(len(t.ExpectedResponse.ExpectedResults[0].TagsAutocompelete), len(res)) + for i := 0; i < length; i++ { + if i >= len(res) { + err = fmt.Errorf("tags[%d] want=`%+v`", i, t.ExpectedResponse.ExpectedResults[0].TagsAutocompelete[i]) + failures = append(failures, err) + } else if i >= len(t.ExpectedResponse.ExpectedResults[0].TagsAutocompelete) { + err = fmt.Errorf("tags[%d] got unexpected=`%+v`", i, res[i]) + failures = append(failures, err) + } else if !reflect.DeepEqual(res[i], t.ExpectedResponse.ExpectedResults[0].TagsAutocompelete[i]) { + err = fmt.Errorf("tags[%d] are not equal, got=`%+v`, expected=`%+v`", i, res[i], t.ExpectedResponse.ExpectedResults[0].TagsAutocompelete[i]) + failures = append(failures, err) + } + } + } + return failures + } + + for i := range res { + if res[i] != t.ExpectedResponse.ExpectedResults[0].TagsAutocompelete[i] { + err = merry2.Prependf(err, "tags[%d] are not equal, got=`%+v`, expected=`%+v`", i, res[i], t.ExpectedResponse.ExpectedResults[0].TagsAutocompelete[i]) + failures = append(failures, err) + } + } + + } else { + // render + res := make([]RenderResponse, 0, 1) + err := json.Unmarshal(b, &res) + if err != nil { + err = merry2.Prepend(err, "failed to parse response") + failures = append(failures, err) + return failures + } + + if len(t.ExpectedResponse.ExpectedResults) == 0 { + return failures + } + + if len(res) != len(t.ExpectedResponse.ExpectedResults[0].Metrics) { + failures = append(failures, merry2.Errorf("unexpected amount of results, got %v, expected %v", + len(res), + len(t.ExpectedResponse.ExpectedResults[0].Metrics))) + if verbose { + length := max(len(t.ExpectedResponse.ExpectedResults[0].Metrics), len(res)) + for i := 0; i < length; i++ { + if i >= len(res) { + err = fmt.Errorf("metrics[%d] want=`%+v`", i, t.ExpectedResponse.ExpectedResults[0].Metrics[i]) + failures = append(failures, err) + } else if i >= len(t.ExpectedResponse.ExpectedResults[0].Metrics) { + err = fmt.Errorf("metrics[%d] got unexpected=`%+v`", i, res[i]) + failures = append(failures, err) + } else if !reflect.DeepEqual(res[i], t.ExpectedResponse.ExpectedResults[0].Metrics[i]) { + err = fmt.Errorf("metrics[%d] are not equal, got=`%+v`, expected=`%+v`", i, res[i], t.ExpectedResponse.ExpectedResults[0].Metrics[i]) + failures = append(failures, err) + } + } + } + return failures + } + + for i := range res { + err := isRenderEqual(res[i], t.ExpectedResponse.ExpectedResults[0].Metrics[i]) + if err != nil { + err = merry2.Prependf(err, "metrics are not equal, got=`%+v`, expected=`%+v`", res[i], t.ExpectedResponse.ExpectedResults[0].Metrics[i]) + failures = append(failures, err) + } + } + } default: - failures = append(failures, merry2.Errorf("unsupported content-type: got '%v'", contentType)) + if resp.StatusCode == http.StatusOK { + // if !strings.HasPrefix(t.URL, "/tags/autoComplete/") || + // (contentType == "text/plain; charset=utf-8" && + // resp.StatusCode == http.StatusNotFound && + // t.ExpectedResponse.HttpCode == http.StatusNotFound) { + failures = append(failures, merry2.Errorf("unsupported content-type: got '%v'", contentType)) + } } return failures } -func e2eTest(logger *zap.Logger, noapp, breakOnError bool) bool { +func e2eTest(logger *zap.Logger, noapp, breakOnError, verbose bool) bool { failed := false logger.Info("will run test", zap.Any("config", cfg.Test), @@ -307,12 +433,12 @@ func e2eTest(logger *zap.Logger, noapp, breakOnError bool) bool { } for _, t := range cfg.Test.Queries { - failures := doTest(logger, &t) - + failures := doTest(logger, &t, verbose) if len(failures) != 0 { failed = true logger.Error("test failed", zap.Errors("failures", failures), + zap.String("url", t.URL), zap.String("type", t.Type), zap.String("body", t.Body), ) for _, v := range runningApps { if !v.IsRunning() { diff --git a/cmd/mockbackend/find.go b/cmd/mockbackend/find.go index 2c0c14592..74ba2729a 100644 --- a/cmd/mockbackend/find.go +++ b/cmd/mockbackend/find.go @@ -46,7 +46,7 @@ func (cfg *listener) findHandler(wr http.ResponseWriter, req *http.Request) { return } - query := req.Form["query"] + var query []string if format == protoV3Format { body, err := io.ReadAll(req.Body) @@ -56,12 +56,22 @@ func (cfg *listener) findHandler(wr http.ResponseWriter, req *http.Request) { ) http.Error(wr, "Bad request (unsupported format)", http.StatusBadRequest) + return } var pv3Request carbonapi_v3_pb.MultiGlobRequest _ = pv3Request.Unmarshal(body) query = pv3Request.Metrics + } else { + query = req.Form["query"] + } + + if len(query) == 0 { + logger.Error("Bad request (no query)") + http.Error(wr, "Bad request (no query)", + http.StatusBadRequest) + return } logger.Info("request details", @@ -72,23 +82,7 @@ func (cfg *listener) findHandler(wr http.ResponseWriter, req *http.Request) { Metrics: []carbonapi_v3_pb.GlobResponse{}, } - if query[0] != "*" { - for m := range cfg.Listener.Expressions { - globMatches := []carbonapi_v3_pb.GlobMatch{} - - for _, metric := range cfg.Expressions[m].Data { - globMatches = append(globMatches, carbonapi_v3_pb.GlobMatch{ - Path: metric.MetricName, - IsLeaf: true, - }) - } - multiGlobs.Metrics = append(multiGlobs.Metrics, - carbonapi_v3_pb.GlobResponse{ - Name: cfg.Expressions[m].PathExpression, - Matches: globMatches, - }) - } - } else { + if query[0] == "*" { returnMap := make(map[string]struct{}) for m := range cfg.Listener.Expressions { response := cfg.Expressions[m] @@ -115,6 +109,33 @@ func (cfg *listener) findHandler(wr http.ResponseWriter, req *http.Request) { Name: "*", Matches: globMatches, }) + } else { + for _, m := range query { + globMatches := []carbonapi_v3_pb.GlobMatch{} + if response, ok := cfg.Expressions[m]; ok { + if response.ReplyDelayMS > 0 { + delay := time.Duration(response.ReplyDelayMS) * time.Millisecond + time.Sleep(delay) + } + if response.Code != 0 && response.Code != http.StatusOK { + // return first error + http.Error(wr, http.StatusText(response.Code), response.Code) + return + } + + for _, metric := range cfg.Expressions[m].Data { + globMatches = append(globMatches, carbonapi_v3_pb.GlobMatch{ + Path: metric.MetricName, + IsLeaf: true, + }) + } + multiGlobs.Metrics = append(multiGlobs.Metrics, + carbonapi_v3_pb.GlobResponse{ + Name: cfg.Expressions[m].PathExpression, + Matches: globMatches, + }) + } + } } if cfg.Listener.ShuffleResults { diff --git a/cmd/mockbackend/http_common.go b/cmd/mockbackend/http_common.go index 0bb75a912..b55db199d 100644 --- a/cmd/mockbackend/http_common.go +++ b/cmd/mockbackend/http_common.go @@ -7,9 +7,11 @@ import ( ) type Response struct { + Code int `yaml:"code"` ReplyDelayMS int `yaml:"replyDelayMS"` PathExpression string `yaml:"pathExpression"` Data []Metric `yaml:"data"` + Tags []string `yaml:"tags"` } type Metric struct { diff --git a/cmd/mockbackend/main.go b/cmd/mockbackend/main.go index 3692083ea..ec9bfd084 100644 --- a/cmd/mockbackend/main.go +++ b/cmd/mockbackend/main.go @@ -21,7 +21,7 @@ type MainConfig struct { type Listener struct { Address string `yaml:"address"` - Code int `yaml:"httpCode"` + Code int `yaml:"httpCode"` // global responce code ShuffleResults bool `yaml:"shuffleResults"` EmptyBody bool `yaml:"emptyBody"` Expressions map[string]Response `yaml:"expressions"` @@ -36,6 +36,7 @@ type listener struct { func main() { config := flag.String("config", "average.yaml", "yaml where it would be possible to get data") + verbose := flag.Bool("verbose", false, "verbose reporting") testonly := flag.Bool("testonly", false, "run only unit test") noapp := flag.Bool("noapp", false, "do not run application") test := flag.Bool("test", false, "run unit test if present") @@ -50,12 +51,14 @@ func main() { logger.Fatal("failed to get config, it should be non-null") } - d, err := os.ReadFile(*config) + f, err := os.Open(*config) if err != nil { logger.Fatal("failed to read config", zap.Error(err)) } - err = yaml.Unmarshal(d, &cfg) + decoder := yaml.NewDecoder(f) + decoder.SetStrict(true) + err = decoder.Decode(&cfg) if err != nil { logger.Fatal("failed to read config", zap.Error(err)) return @@ -94,6 +97,8 @@ func main() { mux.HandleFunc("/render/", listener.renderHandler) mux.HandleFunc("/metrics/find", listener.findHandler) mux.HandleFunc("/metrics/find/", listener.findHandler) + mux.HandleFunc("/tags/autoComplete/values", listener.tagsValuesHandler) + mux.HandleFunc("/tags/autoComplete/tags", listener.tagsNamesHandler) wg.Add(1) wgStart.Add(1) @@ -120,7 +125,7 @@ func main() { failed := false if cfg.Test != nil && (*test || *testonly) { - failed = e2eTest(logger, *noapp, *breakOnError) + failed = e2eTest(logger, *noapp, *breakOnError, *verbose) } if !*testonly { diff --git a/cmd/mockbackend/render.go b/cmd/mockbackend/render.go index 8d9ff469e..0b2b189a1 100644 --- a/cmd/mockbackend/render.go +++ b/cmd/mockbackend/render.go @@ -91,89 +91,96 @@ func (cfg *listener) renderHandler(wr http.ResponseWriter, req *http.Request) { Metrics: []carbonapi_v3_pb.FetchResponse{}, } - newCfg := Listener{ - Code: cfg.Code, - EmptyBody: cfg.EmptyBody, - Expressions: copyMap(cfg.Expressions), - } - + httpCode := http.StatusOK for _, target := range targets { - response, ok := newCfg.Expressions[target] - if !ok { - wr.WriteHeader(http.StatusNotFound) - _, _ = wr.Write([]byte("Not found")) - return - } - if response.ReplyDelayMS > 0 { - delay := time.Duration(response.ReplyDelayMS) * time.Millisecond - logger.Info("will add extra delay", - zap.Duration("delay", delay), - ) - time.Sleep(delay) - } - for _, m := range response.Data { - step := m.Step - if step == 0 { - step = 1 - } - startTime := m.StartTime - if startTime == 0 { - startTime = step - } - isAbsent := make([]bool, 0, len(m.Values)) - protov2Values := make([]float64, 0, len(m.Values)) - for i := range m.Values { - if math.IsNaN(m.Values[i]) { - isAbsent = append(isAbsent, true) - protov2Values = append(protov2Values, 0.0) - } else { - isAbsent = append(isAbsent, false) - protov2Values = append(protov2Values, m.Values[i]) - } + if response, ok := cfg.Expressions[target]; ok { + if response.ReplyDelayMS > 0 { + delay := time.Duration(response.ReplyDelayMS) * time.Millisecond + logger.Info("will add extra delay", + zap.Duration("delay", delay), + ) + time.Sleep(delay) } - fr2 := carbonapi_v2_pb.FetchResponse{ - Name: m.MetricName, - StartTime: int32(startTime), - StopTime: int32(startTime + step*len(protov2Values)), - StepTime: int32(step), - Values: protov2Values, - IsAbsent: isAbsent, + if response.Code > 0 && response.Code != http.StatusOK { + httpCode = response.Code } + if httpCode == http.StatusOK { + for _, m := range response.Data { + step := m.Step + if step == 0 { + step = 1 + } + startTime := m.StartTime + if startTime == 0 { + startTime = step + } + isAbsent := make([]bool, 0, len(m.Values)) + protov2Values := make([]float64, 0, len(m.Values)) + for i := range m.Values { + if math.IsNaN(m.Values[i]) { + isAbsent = append(isAbsent, true) + protov2Values = append(protov2Values, 0.0) + } else { + isAbsent = append(isAbsent, false) + protov2Values = append(protov2Values, m.Values[i]) + } + } + fr2 := carbonapi_v2_pb.FetchResponse{ + Name: m.MetricName, + StartTime: int32(startTime), + StopTime: int32(startTime + step*len(protov2Values)), + StepTime: int32(step), + Values: protov2Values, + IsAbsent: isAbsent, + } - fr3 := carbonapi_v3_pb.FetchResponse{ - Name: m.MetricName, - PathExpression: target, - ConsolidationFunc: "avg", - StartTime: int64(startTime), - StopTime: int64(startTime + step*len(m.Values)), - StepTime: int64(step), - XFilesFactor: 0, - HighPrecisionTimestamps: false, - Values: m.Values, - RequestStartTime: 1, - RequestStopTime: int64(startTime + step*len(m.Values)), - } + fr3 := carbonapi_v3_pb.FetchResponse{ + Name: m.MetricName, + PathExpression: target, + ConsolidationFunc: "avg", + StartTime: int64(startTime), + StopTime: int64(startTime + step*len(m.Values)), + StepTime: int64(step), + XFilesFactor: 0, + HighPrecisionTimestamps: false, + Values: m.Values, + RequestStartTime: 1, + RequestStopTime: int64(startTime + step*len(m.Values)), + } - multiv2.Metrics = append(multiv2.Metrics, fr2) - multiv3.Metrics = append(multiv3.Metrics, fr3) + multiv2.Metrics = append(multiv2.Metrics, fr2) + multiv3.Metrics = append(multiv3.Metrics, fr3) + } + } } } - if cfg.Listener.ShuffleResults { - rand.Shuffle(len(multiv2.Metrics), func(i, j int) { - multiv2.Metrics[i], multiv2.Metrics[j] = multiv2.Metrics[j], multiv2.Metrics[i] - }) - rand.Shuffle(len(multiv3.Metrics), func(i, j int) { - multiv3.Metrics[i], multiv3.Metrics[j] = multiv3.Metrics[j], multiv3.Metrics[i] - }) - } + if httpCode == http.StatusOK { + if len(multiv2.Metrics) == 0 { + wr.WriteHeader(http.StatusNotFound) + _, _ = wr.Write([]byte("Not found")) + return + } - contentType, d := cfg.marshalResponse(wr, logger, format, multiv3, multiv2) - if d == nil { - return + if cfg.Listener.ShuffleResults { + rand.Shuffle(len(multiv2.Metrics), func(i, j int) { + multiv2.Metrics[i], multiv2.Metrics[j] = multiv2.Metrics[j], multiv2.Metrics[i] + }) + rand.Shuffle(len(multiv3.Metrics), func(i, j int) { + multiv3.Metrics[i], multiv3.Metrics[j] = multiv3.Metrics[j], multiv3.Metrics[i] + }) + } + + contentType, d := cfg.marshalResponse(wr, logger, format, multiv3, multiv2) + if d == nil { + return + } + wr.Header().Set("Content-Type", contentType) + _, _ = wr.Write(d) + } else { + wr.WriteHeader(httpCode) + _, _ = wr.Write([]byte(http.StatusText(httpCode))) } - wr.Header().Set("Content-Type", contentType) - _, _ = wr.Write(d) } func (cfg *listener) marshalResponse(wr http.ResponseWriter, logger *zap.Logger, format responseFormat, multiv3 carbonapi_v3_pb.MultiFetchResponse, multiv2 carbonapi_v2_pb.MultiFetchResponse) (string, []byte) { diff --git a/cmd/mockbackend/tags.go b/cmd/mockbackend/tags.go new file mode 100644 index 000000000..33b2ae5b8 --- /dev/null +++ b/cmd/mockbackend/tags.go @@ -0,0 +1,104 @@ +package main + +import ( + "bytes" + "net/http" + "time" + + "go.uber.org/zap" +) + +func (cfg *listener) tagsAutocompleteHandler(wr http.ResponseWriter, req *http.Request, isValues bool) { + _ = req.ParseMultipartForm(16 * 1024 * 1024) + hdrs := make(map[string][]string) + + for n, v := range req.Header { + hdrs[n] = v + } + + logger := cfg.logger.With( + zap.String("function", "findHandler"), + zap.String("method", req.Method), + zap.String("path", req.URL.Path), + zap.Any("form", req.Form), + zap.Any("headers", hdrs), + ) + logger.Info("got request") + + if cfg.Code != http.StatusOK { + wr.WriteHeader(cfg.Code) + return + } + + format, err := getFormat(req) + if err != nil { + wr.WriteHeader(http.StatusBadRequest) + _, _ = wr.Write([]byte(err.Error())) + return + } + + url := req.URL.String() + + logger.Info("request details", + zap.Any("query", req.Form), + ) + + returnCode := http.StatusOK + var tags []string + if response, ok := cfg.Expressions[url]; ok { + if response.ReplyDelayMS > 0 { + delay := time.Duration(response.ReplyDelayMS) * time.Millisecond + time.Sleep(delay) + } + if response.Code == http.StatusNotFound { + returnCode = http.StatusNotFound + } else if response.Code != 0 && response.Code != http.StatusOK { + returnCode = response.Code + http.Error(wr, http.StatusText(returnCode), returnCode) + return + } else { + tags = response.Tags + } + } + + if returnCode == http.StatusNotFound { + // return 404 when no data + http.Error(wr, http.StatusText(returnCode), returnCode) + return + } + + logger.Info("will return", zap.Strings("response", tags)) + + var b []byte + switch format { + case jsonFormat: + var buf bytes.Buffer + buf.WriteByte('[') + for i, t := range tags { + if i == 0 { + buf.WriteByte('"') + } else { + buf.WriteString(`, "`) + } + buf.WriteString(t) + buf.WriteByte('"') + } + buf.WriteByte(']') + b = buf.Bytes() + wr.Header().Set("Content-Type", contentTypeJSON) + default: + returCode := http.StatusBadRequest + http.Error(wr, http.StatusText(returnCode), returCode) + return + } + + _, _ = wr.Write(b) +} + +func (cfg *listener) tagsValuesHandler(wr http.ResponseWriter, req *http.Request) { + cfg.tagsAutocompleteHandler(wr, req, true) +} + +func (cfg *listener) tagsNamesHandler(wr http.ResponseWriter, req *http.Request) { + cfg.tagsAutocompleteHandler(wr, req, false) +} diff --git a/cmd/mockbackend/testcases/bad_requests/bad_requests.yaml b/cmd/mockbackend/testcases/bad_requests/bad_requests.yaml index 4c3251854..b0e9459de 100644 --- a/cmd/mockbackend/testcases/bad_requests/bad_requests.yaml +++ b/cmd/mockbackend/testcases/bad_requests/bad_requests.yaml @@ -6,6 +6,7 @@ test: args: - "-config" - "./cmd/mockbackend/carbonapi_singlebackend.yaml" + - "-exact-config" queries: - endpoint: "http://127.0.0.1:8081" delay: 1 @@ -14,14 +15,14 @@ test: expectedResponse: httpCode: 400 contentType: "text/plain; charset=utf-8" - emptyBody: true + - endpoint: "http://127.0.0.1:8081" type: "GET" URL: "/render?format=json&target=applyByNode(metric[123], 2, 'transform')" expectedResponse: httpCode: 400 contentType: "text/plain; charset=utf-8" - emptyBody: true + listeners: - address: ":9070" expressions: diff --git a/cmd/mockbackend/testcases/find_error/find_error.yaml b/cmd/mockbackend/testcases/find_error/find_error.yaml new file mode 100644 index 000000000..53b5003f7 --- /dev/null +++ b/cmd/mockbackend/testcases/find_error/find_error.yaml @@ -0,0 +1,93 @@ +version: "v1" +test: + apps: + - name: "carbonapi" + binary: "./carbonapi" + args: + - "-config" + - "./cmd/mockbackend/testcases/render_error/carbonapi.yaml" + - "-exact-config" + queries: + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/metrics/find?query=a&format=json" + expectedResponse: + httpCode: 200 + contentType: "application/json" + expectedResults: + - metricsFind: + - allowChildren: 0 + expandable: 0 + leaf: 1 + id: "a" + text: "a" + context: {} + + # empty + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=b&format=json" + expectedResponse: + httpCode: 200 + contentType: "application/json" + + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/metrics/find?query=a&query=b&format=json" + expectedResponse: + httpCode: 200 + contentType: "application/json" + expectedResults: + - metricsFind: + - allowChildren: 0 + expandable: 0 + leaf: 1 + id: "a" + text: "a" + context: {} + + # timeout + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/metrics/find?query=c&format=json" + expectedResponse: + httpCode: 503 + contentType: "text/plain; charset=utf-8" + + # 503 + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/metrics/find?query=d&format=json" + expectedResponse: + httpCode: 503 + contentType: "text/plain; charset=utf-8" + + # 503, partial success + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/metrics/find?query=a&query=d&format=json" + expectedResponse: + httpCode: 503 + contentType: "text/plain; charset=utf-8" + +listeners: + - address: ":9070" + expressions: + "a": + pathExpression: "a" + data: + - metricName: "a" + values: [0,1,2,2,3] + + # timeout + "c": + pathExpression: "b" + code: 404 + replyDelayMS: 7000 + data: + - metricName: "c" + values: [0,1,2,2,3] + + "d": + pathExpression: "d" + code: 503 diff --git a/cmd/mockbackend/testcases/i484/carbonapi.yaml b/cmd/mockbackend/testcases/i484/carbonapi.yaml index e1f8903e7..b7dbb3314 100644 --- a/cmd/mockbackend/testcases/i484/carbonapi.yaml +++ b/cmd/mockbackend/testcases/i484/carbonapi.yaml @@ -65,7 +65,7 @@ upstreams: servers: - "http://127.0.0.1:9072" - "http://127.0.0.1:9073" - graphite09compat: false +graphite09compat: false expireDelaySec: 10 logger: - logger: "" diff --git a/cmd/mockbackend/testcases/i484/i484.yaml b/cmd/mockbackend/testcases/i484/i484.yaml index 2f0010999..628e7f558 100644 --- a/cmd/mockbackend/testcases/i484/i484.yaml +++ b/cmd/mockbackend/testcases/i484/i484.yaml @@ -6,6 +6,7 @@ test: args: - "-config" - "./cmd/mockbackend/testcases/i484/carbonapi.yaml" + - "-exact-config" queries: - endpoint: "http://127.0.0.1:8081" delay: 1 diff --git a/cmd/mockbackend/testcases/i503/i503.yaml b/cmd/mockbackend/testcases/i503/i503.yaml index 4c13d0cab..4a794df2a 100644 --- a/cmd/mockbackend/testcases/i503/i503.yaml +++ b/cmd/mockbackend/testcases/i503/i503.yaml @@ -6,6 +6,7 @@ test: args: - "-config" - "./cmd/mockbackend/carbonapi_singlebackend.yaml" + - "-exact-config" queries: - endpoint: "http://127.0.0.1:8081" delay: 1 @@ -25,5 +26,4 @@ listeners: expressions: "a": pathExpression: "a" - emptyBody: true - httpCode: 404 + code: 404 diff --git a/cmd/mockbackend/testcases/i506/carbonapi.yaml b/cmd/mockbackend/testcases/i506/carbonapi.yaml index 8caaba51a..aa38ac56f 100644 --- a/cmd/mockbackend/testcases/i506/carbonapi.yaml +++ b/cmd/mockbackend/testcases/i506/carbonapi.yaml @@ -19,15 +19,14 @@ notFoundStatusCode: 200 upstreams: buckets: 10 timeouts: - global: "300s" - afterStarted: "120s" - connect: "500ms" - concurrencyLimit: 0 + find: "15s" + render: "50s" + connect: "200ms" keepAliveInterval: "30s" maxIdleConnsPerHost: 100 backends: - "http://127.0.0.1:9070" - graphite09compat: false +graphite09compat: false expireDelaySec: 10 logger: - logger: "" diff --git a/cmd/mockbackend/testcases/i506/i506.yaml b/cmd/mockbackend/testcases/i506/i506.yaml index a87759747..98737d16b 100644 --- a/cmd/mockbackend/testcases/i506/i506.yaml +++ b/cmd/mockbackend/testcases/i506/i506.yaml @@ -6,6 +6,7 @@ test: args: - "-config" - "./cmd/mockbackend/testcases/i506/carbonapi.yaml" + - "-exact-config" queries: - endpoint: "http://127.0.0.1:8081" delay: 1 @@ -21,10 +22,8 @@ test: - "cbb6fb095dfcfefd44d42e86c0bf3c677693cf4fa5e45897777405ed272c8915" # sha256(nodata svg) on Arch Linux, Ubuntu Bionic (Github Actions) - "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" # sha256(nodata svg) on macos listeners: - - address: ":9070" - expressions: - "a": - pathExpression: "a" - emptyBody: true - httpCode: 200 - + - address: ":9070" + expressions: + "a": + pathExpression: "a" + code: 200 diff --git a/cmd/mockbackend/testcases/i516/i516.yaml b/cmd/mockbackend/testcases/i516/i516.yaml index c717441e9..d2f0ff424 100644 --- a/cmd/mockbackend/testcases/i516/i516.yaml +++ b/cmd/mockbackend/testcases/i516/i516.yaml @@ -6,6 +6,7 @@ test: args: - "-config" - "./cmd/mockbackend/carbonapi_singlebackend.yaml" + - "-exact-config" queries: - endpoint: "http://127.0.0.1:8081" delay: 1 diff --git a/cmd/mockbackend/testcases/i517/i517.yaml b/cmd/mockbackend/testcases/i517/i517.yaml index 342dcc8b8..e9158ef3d 100644 --- a/cmd/mockbackend/testcases/i517/i517.yaml +++ b/cmd/mockbackend/testcases/i517/i517.yaml @@ -6,6 +6,7 @@ test: args: - "-config" - "./cmd/mockbackend/carbonapi_singlebackend.yaml" + - "-exact-config" queries: - endpoint: "http://127.0.0.1:8081" delay: 1 diff --git a/cmd/mockbackend/testcases/i545/carbonapi.yaml b/cmd/mockbackend/testcases/i545/carbonapi.yaml index 2536747ad..4cf44ad92 100644 --- a/cmd/mockbackend/testcases/i545/carbonapi.yaml +++ b/cmd/mockbackend/testcases/i545/carbonapi.yaml @@ -11,8 +11,8 @@ backendCache: size_mb: 0 defaultTimeoutSec: 1 +graphite09compat: false upstreams: - graphite09compat: false buckets: 10 concurrencyLimitPerServer: 0 diff --git a/cmd/mockbackend/testcases/i545/i545.yaml b/cmd/mockbackend/testcases/i545/i545.yaml index 24446a03e..bb9329ed7 100644 --- a/cmd/mockbackend/testcases/i545/i545.yaml +++ b/cmd/mockbackend/testcases/i545/i545.yaml @@ -6,6 +6,7 @@ test: args: - "-config" - "./cmd/mockbackend/testcases/i545/carbonapi.yaml" + - "-exact-config" queries: - endpoint: "http://127.0.0.1:8081" delay: 1 diff --git a/cmd/mockbackend/testcases/i565/i565.yaml b/cmd/mockbackend/testcases/i565/i565.yaml index b30f09b18..1fc4762ff 100644 --- a/cmd/mockbackend/testcases/i565/i565.yaml +++ b/cmd/mockbackend/testcases/i565/i565.yaml @@ -6,6 +6,7 @@ test: args: - "-config" - "./cmd/mockbackend/carbonapi_singlebackend.yaml" + - "-exact-config" queries: - endpoint: "http://127.0.0.1:8081" delay: 1 diff --git a/cmd/mockbackend/testcases/i580/i580.yaml b/cmd/mockbackend/testcases/i580/i580.yaml index 3cb725bb2..29baa4d35 100644 --- a/cmd/mockbackend/testcases/i580/i580.yaml +++ b/cmd/mockbackend/testcases/i580/i580.yaml @@ -6,6 +6,7 @@ test: args: - "-config" - "./cmd/mockbackend/carbonapi_singlebackend.yaml" + - "-exact-config" queries: - endpoint: "http://127.0.0.1:8081" delay: 1 diff --git a/cmd/mockbackend/testcases/i584/i584.yaml b/cmd/mockbackend/testcases/i584/i584.yaml index 4a8fecffd..909c0b3dc 100644 --- a/cmd/mockbackend/testcases/i584/i584.yaml +++ b/cmd/mockbackend/testcases/i584/i584.yaml @@ -6,6 +6,7 @@ test: args: - "-config" - "./cmd/mockbackend/carbonapi_singlebackend.yaml" + - "-exact-config" queries: - endpoint: "http://127.0.0.1:8081" delay: 1 diff --git a/cmd/mockbackend/testcases/i589/i589.yaml b/cmd/mockbackend/testcases/i589/i589.yaml index 3df0b8f2a..49565dec3 100644 --- a/cmd/mockbackend/testcases/i589/i589.yaml +++ b/cmd/mockbackend/testcases/i589/i589.yaml @@ -6,6 +6,7 @@ test: args: - "-config" - "./cmd/mockbackend/carbonapi_singlebackend.yaml" + - "-exact-config" queries: - endpoint: "http://127.0.0.1:8081" delay: 1 diff --git a/cmd/mockbackend/testcases/i598/carbonapi.yaml b/cmd/mockbackend/testcases/i598/carbonapi.yaml index a814df3e9..36b5d7bce 100644 --- a/cmd/mockbackend/testcases/i598/carbonapi.yaml +++ b/cmd/mockbackend/testcases/i598/carbonapi.yaml @@ -65,7 +65,7 @@ upstreams: servers: - "http://127.0.0.1:9072" - "http://127.0.0.1:9073" - graphite09compat: false +graphite09compat: false expireDelaySec: 10 unicodeRangeTables: - "Latin" diff --git a/cmd/mockbackend/testcases/i598/i598.yaml b/cmd/mockbackend/testcases/i598/i598.yaml index 865254df6..3ca392499 100644 --- a/cmd/mockbackend/testcases/i598/i598.yaml +++ b/cmd/mockbackend/testcases/i598/i598.yaml @@ -6,6 +6,7 @@ test: args: - "-config" - "./cmd/mockbackend/testcases/i598/carbonapi.yaml" + - "-exact-config" queries: - endpoint: "http://127.0.0.1:8081" delay: 1 diff --git a/cmd/mockbackend/testcases/i661/carbonapi.yaml b/cmd/mockbackend/testcases/i661/carbonapi.yaml index a814df3e9..36b5d7bce 100644 --- a/cmd/mockbackend/testcases/i661/carbonapi.yaml +++ b/cmd/mockbackend/testcases/i661/carbonapi.yaml @@ -65,7 +65,7 @@ upstreams: servers: - "http://127.0.0.1:9072" - "http://127.0.0.1:9073" - graphite09compat: false +graphite09compat: false expireDelaySec: 10 unicodeRangeTables: - "Latin" diff --git a/cmd/mockbackend/testcases/i661/i661.yaml b/cmd/mockbackend/testcases/i661/i661.yaml index bf5b6de12..6b2cda9c3 100644 --- a/cmd/mockbackend/testcases/i661/i661.yaml +++ b/cmd/mockbackend/testcases/i661/i661.yaml @@ -6,6 +6,7 @@ test: args: - "-config" - "./cmd/mockbackend/testcases/i598/carbonapi.yaml" + - "-exact-config" queries: - endpoint: "http://127.0.0.1:8081" delay: 1 diff --git a/cmd/mockbackend/testcases/pr500/pr500.yaml b/cmd/mockbackend/testcases/pr500/pr500.yaml index d694de7c5..f994ad017 100644 --- a/cmd/mockbackend/testcases/pr500/pr500.yaml +++ b/cmd/mockbackend/testcases/pr500/pr500.yaml @@ -6,6 +6,7 @@ test: args: - "-config" - "./cmd/mockbackend/carbonapi_singlebackend.yaml" + - "-exact-config" queries: - endpoint: "http://127.0.0.1:8081" delay: 1 diff --git a/cmd/mockbackend/testcases/pr529/pr529.yaml b/cmd/mockbackend/testcases/pr529/pr529.yaml index 1dd4df40b..0a75eefcf 100644 --- a/cmd/mockbackend/testcases/pr529/pr529.yaml +++ b/cmd/mockbackend/testcases/pr529/pr529.yaml @@ -6,6 +6,7 @@ test: args: - "-config" - "./cmd/mockbackend/carbonapi_singlebackend.yaml" + - "-exact-config" queries: - endpoint: "http://127.0.0.1:8081" delay: 1 diff --git a/cmd/mockbackend/testcases/pr560/pr560.yaml b/cmd/mockbackend/testcases/pr560/pr560.yaml index 7444d8a9b..57202e0d2 100644 --- a/cmd/mockbackend/testcases/pr560/pr560.yaml +++ b/cmd/mockbackend/testcases/pr560/pr560.yaml @@ -6,6 +6,7 @@ test: args: - "-config" - "./cmd/mockbackend/carbonapi_singlebackend.yaml" + - "-exact-config" queries: - endpoint: "http://127.0.0.1:8081" delay: 1 diff --git a/cmd/mockbackend/testcases/pr594-403-504/carbonapi.yaml b/cmd/mockbackend/testcases/pr594-403-504/carbonapi.yaml index 4a0677ba6..2e26db250 100644 --- a/cmd/mockbackend/testcases/pr594-403-504/carbonapi.yaml +++ b/cmd/mockbackend/testcases/pr594-403-504/carbonapi.yaml @@ -47,7 +47,7 @@ upstreams: servers: - "http://127.0.0.1:9070" - "http://127.0.0.1:9071" - graphite09compat: false +graphite09compat: false expireDelaySec: 10 logger: - logger: "" diff --git a/cmd/mockbackend/testcases/pr594-403-504/pr594-403-504.yaml b/cmd/mockbackend/testcases/pr594-403-504/pr594-403-504.yaml index 526f1ba0a..0cf2e6c68 100644 --- a/cmd/mockbackend/testcases/pr594-403-504/pr594-403-504.yaml +++ b/cmd/mockbackend/testcases/pr594-403-504/pr594-403-504.yaml @@ -6,6 +6,7 @@ test: args: - "-config" - "./cmd/mockbackend/testcases/pr594-403-504/carbonapi.yaml" + - "-exact-config" queries: - endpoint: "http://127.0.0.1:8081" delay: 1 diff --git a/cmd/mockbackend/testcases/pr594-maskerror/carbonapi.yaml b/cmd/mockbackend/testcases/pr594-maskerror/carbonapi.yaml index 4a0677ba6..2e26db250 100644 --- a/cmd/mockbackend/testcases/pr594-maskerror/carbonapi.yaml +++ b/cmd/mockbackend/testcases/pr594-maskerror/carbonapi.yaml @@ -47,7 +47,7 @@ upstreams: servers: - "http://127.0.0.1:9070" - "http://127.0.0.1:9071" - graphite09compat: false +graphite09compat: false expireDelaySec: 10 logger: - logger: "" diff --git a/cmd/mockbackend/testcases/pr594-maskerror/pr594-maskerror.yaml b/cmd/mockbackend/testcases/pr594-maskerror/pr594-maskerror.yaml index b62518c77..415ede7a4 100644 --- a/cmd/mockbackend/testcases/pr594-maskerror/pr594-maskerror.yaml +++ b/cmd/mockbackend/testcases/pr594-maskerror/pr594-maskerror.yaml @@ -6,6 +6,7 @@ test: args: - "-config" - "./cmd/mockbackend/testcases/pr594-maskerror/carbonapi.yaml" + - "-exact-config" queries: - endpoint: "http://127.0.0.1:8081" delay: 1 diff --git a/cmd/mockbackend/testcases/pr743/carbonapi.yaml b/cmd/mockbackend/testcases/pr743/carbonapi.yaml index fd0289d55..2c818ccd7 100644 --- a/cmd/mockbackend/testcases/pr743/carbonapi.yaml +++ b/cmd/mockbackend/testcases/pr743/carbonapi.yaml @@ -49,7 +49,7 @@ upstreams: connect: "200ms" servers: - "http://127.0.0.1:9070" - graphite09compat: false +graphite09compat: false expireDelaySec: 10 logger: - logger: "" diff --git a/cmd/mockbackend/testcases/pr743/pr743.yaml b/cmd/mockbackend/testcases/pr743/pr743.yaml index 5d01d3e6f..7425befb4 100644 --- a/cmd/mockbackend/testcases/pr743/pr743.yaml +++ b/cmd/mockbackend/testcases/pr743/pr743.yaml @@ -6,6 +6,7 @@ test: args: - "-config" - "./cmd/mockbackend/testcases/pr743/carbonapi.yaml" + - "-exact-config" queries: - endpoint: "http://127.0.0.1:8081" delay: 1 diff --git a/cmd/mockbackend/testcases/pr817/carbonapi.yaml b/cmd/mockbackend/testcases/pr817/carbonapi.yaml index 368baaa66..6eba3ad82 100644 --- a/cmd/mockbackend/testcases/pr817/carbonapi.yaml +++ b/cmd/mockbackend/testcases/pr817/carbonapi.yaml @@ -50,7 +50,7 @@ upstreams: connect: "200ms" servers: - "http://127.0.0.1:9070" - graphite09compat: false +graphite09compat: false expireDelaySec: 10 logger: - logger: "" diff --git a/cmd/mockbackend/testcases/pr817/pr817.yaml b/cmd/mockbackend/testcases/pr817/pr817.yaml index 0330930dc..6e5ca6c36 100644 --- a/cmd/mockbackend/testcases/pr817/pr817.yaml +++ b/cmd/mockbackend/testcases/pr817/pr817.yaml @@ -6,6 +6,7 @@ test: args: - "-config" - "./cmd/mockbackend/testcases/pr817/carbonapi.yaml" + - "-exact-config" queries: - endpoint: "http://127.0.0.1:8081" type: "GET" @@ -13,35 +14,30 @@ test: expectedResponse: httpCode: 400 contentType: "text/plain; charset=utf-8" - emptyBody: true - endpoint: "http://127.0.0.1:8081" type: "GET" URL: "/metrics/find?query=a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.*&format=json" expectedResponse: httpCode: 400 contentType: "text/plain; charset=utf-8" - emptyBody: true - endpoint: "http://127.0.0.1:8081" type: "GET" URL: "/metrics/expand?query=a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.b&format=json" expectedResponse: httpCode: 400 contentType: "text/plain; charset=utf-8" - emptyBody: true - endpoint: "http://127.0.0.1:8081" type: "GET" URL: "/tags/autoComplete/tags?query=a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.b" expectedResponse: httpCode: 400 contentType: "text/plain; charset=utf-8" - emptyBody: true - endpoint: "http://127.0.0.1:8081" type: "GET" URL: "/tags/autoComplete/values?query=a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.b" expectedResponse: httpCode: 400 contentType: "text/plain; charset=utf-8" - emptyBody: true - endpoint: "http://127.0.0.1:8081" type: "GET" URL: "/render/?target=a.b.c&target=a.b.d&format=json" @@ -49,11 +45,11 @@ test: httpCode: 200 contentType: "application/json" expectedResults: - - metrics: - - target: "a.b.c" - datapoints: [[0,1],[1,2],[2,3],[2,4],[3,5]] - - target: "a.b.d" - datapoints: [[31,1],[10,2],[4,3],[7,4],[3,5]] + - metrics: + - target: "a.b.c" + datapoints: [[0,1],[1,2],[2,3],[2,4],[3,5]] + - target: "a.b.d" + datapoints: [[31,1],[10,2],[4,3],[7,4],[3,5]] - endpoint: "http://127.0.0.1:8081" type: "GET" URL: "/metrics/find?query=a.b.*&format=json" diff --git a/cmd/mockbackend/testcases/render_error/carbonapi.yaml b/cmd/mockbackend/testcases/render_error/carbonapi.yaml new file mode 100644 index 000000000..23622488f --- /dev/null +++ b/cmd/mockbackend/testcases/render_error/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:9070" +graphite09compat: false +expireDelaySec: 10 +logger: + - logger: "" + file: "stderr" + level: "debug" + encoding: "console" + encodingTime: "iso8601" + encodingDuration: "seconds" diff --git a/cmd/mockbackend/testcases/render_error/render_error.yaml b/cmd/mockbackend/testcases/render_error/render_error.yaml new file mode 100644 index 000000000..e57dd6a3a --- /dev/null +++ b/cmd/mockbackend/testcases/render_error/render_error.yaml @@ -0,0 +1,98 @@ +version: "v1" +test: + apps: + - name: "carbonapi" + binary: "./carbonapi" + args: + - "-config" + - "./cmd/mockbackend/testcases/render_error/carbonapi.yaml" + - "-exact-config" + queries: + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=a&format=json" + expectedResponse: + httpCode: 200 + contentType: "application/json" + expectedResults: + - metrics: + - target: "a" + datapoints: [[0,1],[1,2],[2,3],[2,4],[3,5]] + + # empty + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=b&format=json" + expectedResponse: + httpCode: 200 + contentType: "application/json" + + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=a&target=b&format=json" + expectedResponse: + httpCode: 200 + contentType: "application/json" + expectedResults: + - metrics: + - target: "a" + datapoints: [[0,1],[1,2],[2,3],[2,4],[3,5]] + + # timeout + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=c&format=json" + expectedResponse: + httpCode: 503 + contentType: "text/plain; charset=utf-8" + + # 503 + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=d&format=json" + expectedResponse: + httpCode: 503 + contentType: "text/plain; charset=utf-8" + + # partial success + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=a&target=d&format=json" + expectedResponse: + httpCode: 200 + contentType: "application/json" + expectedResults: + - metrics: + - target: "a" + datapoints: [[0,1],[1,2],[2,3],[2,4],[3,5]] + + # partial success + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=divideSeries(a,d)&format=json" + expectedResponse: + httpCode: 200 + contentType: "application/json" + expectedResults: + - metrics: + - target: "divideSeries(a,MISSING)" + datapoints: [[nan,1],[nan,2],[nan,3],[nan,4],[nan,5]] + +listeners: + - address: ":9070" + expressions: + "a": + pathExpression: "a" + data: + - metricName: "a" + values: [0,1,2,2,3] + + # timeout + "c": + pathExpression: "c" + code: 404 + replyDelayMS: 7000 + + "d": + pathExpression: "d" + code: 503 diff --git a/cmd/mockbackend/testcases/render_error_all/carbonapi.yaml b/cmd/mockbackend/testcases/render_error_all/carbonapi.yaml new file mode 100644 index 000000000..c8e968bb1 --- /dev/null +++ b/cmd/mockbackend/testcases/render_error_all/carbonapi.yaml @@ -0,0 +1,58 @@ +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: + requireSuccessAll: true + buckets: 10 + timeouts: + find: "2s" + render: "60s" + 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: "600s" + render: "5s" + connect: "200ms" + servers: + - "http://127.0.0.1:9070" +graphite09compat: false +expireDelaySec: 10 +logger: + - logger: "" + file: "stderr" + level: "debug" + encoding: "console" + encodingTime: "iso8601" + encodingDuration: "seconds" diff --git a/cmd/mockbackend/testcases/render_error_all/render_error_all.yaml b/cmd/mockbackend/testcases/render_error_all/render_error_all.yaml new file mode 100644 index 000000000..d312d89d0 --- /dev/null +++ b/cmd/mockbackend/testcases/render_error_all/render_error_all.yaml @@ -0,0 +1,90 @@ +version: "v1" +test: + apps: + - name: "carbonapi" + binary: "./carbonapi" + args: + - "-config" + - "./cmd/mockbackend/testcases/render_error_all/carbonapi.yaml" + - "-exact-config" + queries: + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=a&format=json" + expectedResponse: + httpCode: 200 + contentType: "application/json" + expectedResults: + - metrics: + - target: "a" + datapoints: [[0,1],[1,2],[2,3],[2,4],[3,5]] + + # empty + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=b&format=json" + expectedResponse: + httpCode: 200 + contentType: "application/json" + + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=a&target=b&format=json" + expectedResponse: + httpCode: 200 + contentType: "application/json" + expectedResults: + - metrics: + - target: "a" + datapoints: [[0,1],[1,2],[2,3],[2,4],[3,5]] + + # timeout + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=c&format=json" + expectedResponse: + httpCode: 503 + contentType: "text/plain; charset=utf-8" + + # 503 + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=d&format=json" + expectedResponse: + httpCode: 503 + contentType: "text/plain; charset=utf-8" + + # partial success + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=a&target=d&format=json" + expectedResponse: + httpCode: 503 + contentType: "text/plain; charset=utf-8" + + # partial success, must fail, target d failed + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=divideSeries(a,d)&format=json" + expectedResponse: + httpCode: 503 + contentType: "text/plain; charset=utf-8" + +listeners: + - address: ":9070" + expressions: + "a": + pathExpression: "a" + data: + - metricName: "a" + values: [0,1,2,2,3] + + # timeout + "c": + pathExpression: "c" + code: 404 + replyDelayMS: 7000 + + "d": + pathExpression: "d" + code: 503 diff --git a/cmd/mockbackend/testcases/render_error_all_rr/carbonapi.yaml b/cmd/mockbackend/testcases/render_error_all_rr/carbonapi.yaml new file mode 100644 index 000000000..ed6101937 --- /dev/null +++ b/cmd/mockbackend/testcases/render_error_all_rr/carbonapi.yaml @@ -0,0 +1,59 @@ +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: + requireSuccessAll: true + buckets: 10 + timeouts: + find: "2s" + render: "10s" + connect: "200ms" + concurrencyLimitPerServer: 0 + keepAliveInterval: "30s" + maxIdleConnsPerHost: 100 + backendsv2: + backends: + - + groupName: "mock-001" + protocol: "auto" + lbMethod: "rr" + maxTries: 2 + maxBatchSize: 0 + keepAliveInterval: "10s" + concurrencyLimit: 0 + forceAttemptHTTP2: true + maxIdleConnsPerHost: 1000 + timeouts: + find: "3s" + render: "5s" + connect: "200ms" + servers: + - "http://127.0.0.1:9070" + - "http://127.0.0.1:9071" +graphite09compat: false +expireDelaySec: 10 +logger: + - logger: "" + file: "stderr" + level: "debug" + encoding: "console" + encodingTime: "iso8601" + encodingDuration: "seconds" diff --git a/cmd/mockbackend/testcases/render_error_all_rr/render_error_all_rr.yaml b/cmd/mockbackend/testcases/render_error_all_rr/render_error_all_rr.yaml new file mode 100644 index 000000000..a401b99ae --- /dev/null +++ b/cmd/mockbackend/testcases/render_error_all_rr/render_error_all_rr.yaml @@ -0,0 +1,134 @@ +version: "v1" +test: + apps: + - name: "carbonapi" + binary: "./carbonapi" + args: + - "-config" + - "./cmd/mockbackend/testcases/render_error_all_rr/carbonapi.yaml" + - "-exact-config" + queries: + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=a&format=json" + expectedResponse: + httpCode: 200 + contentType: "application/json" + expectedResults: + - metrics: + - target: "a" + datapoints: [[0,1],[1,2],[2,3],[2,4],[3,5]] + + # empty + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=b&format=json" + expectedResponse: + httpCode: 200 + contentType: "application/json" + + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=a&target=b&format=json" + expectedResponse: + httpCode: 200 + contentType: "application/json" + expectedResults: + - metrics: + - target: "a" + datapoints: [[0,1],[1,2],[2,3],[2,4],[3,5]] + + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=c&format=json" + expectedResponse: + httpCode: 200 + contentType: "application/json" + expectedResults: + - metrics: + - target: "c" + datapoints: [[0,1],[1,2],[2,3],[2,4],[4,5]] + + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=a&target=b&target=c&format=json" + expectedResponse: + httpCode: 200 + contentType: "application/json" + expectedResults: + - metrics: + - target: "a" + datapoints: [[0,1],[1,2],[2,3],[2,4],[3,5]] + - target: "c" + datapoints: [[0,1],[1,2],[2,3],[2,4],[4,5]] + + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=divideSeries(a,c)&format=json" + expectedResponse: + httpCode: 200 + contentType: "application/json" + expectedResults: + - metrics: + - target: "divideSeries(a,c)" + tags: {"name": "a"} + datapoints: [[NaN,1],[1,2],[1,3],[1,4],[0.75,5]] + + # 503 + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=d&format=json" + expectedResponse: + httpCode: 503 + contentType: "text/plain; charset=utf-8" + + # partial success + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=a&target=d&format=json" + expectedResponse: + httpCode: 503 + contentType: "text/plain; charset=utf-8" + + # partial success + # TODO: must fail, target d failed + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=divideSeries(a,d)&format=json" + expectedResponse: + httpCode: 503 + contentType: "text/plain; charset=utf-8" + +listeners: + - address: ":9070" + expressions: + "a": + pathExpression: "a" + code: 503 + + # 503 + "c": + pathExpression: "c" + code: 503 + + "d": + pathExpression: "d" + code: 503 + + - address: ":9071" + expressions: + "a": + pathExpression: "a" + data: + - metricName: "a" + values: [0,1,2,2,3] + + "c": + pathExpression: "c" + data: + - metricName: "c" + values: [0,1,2,2,4] + + "d": + pathExpression: "d" + code: 503 diff --git a/cmd/mockbackend/testcases/tags_error/tags_error.yaml b/cmd/mockbackend/testcases/tags_error/tags_error.yaml new file mode 100644 index 000000000..e21dd36a5 --- /dev/null +++ b/cmd/mockbackend/testcases/tags_error/tags_error.yaml @@ -0,0 +1,86 @@ +version: "v1" +test: + apps: + - name: "carbonapi" + binary: "./carbonapi" + args: + - "-config" + - "./cmd/mockbackend/carbonapi_singlebackend.yaml" + - "-exact-config" + queries: + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/tags/autoComplete/values?expr=tag1%3Dv1&tag=tag2" + expectedResponse: + httpCode: 200 + contentType: "application/json" + expectedResults: + - tagsAutocompelete: + - "value1" + - "value2" + + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/tags/autoComplete/tags?expr=tag1%3Dv1&tagPrefix=tag" + expectedResponse: + httpCode: 200 + contentType: "application/json" + expectedResults: + - tagsAutocompelete: + - "tag2" + + # empty + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/tags/autoComplete/values?expr=tag1%3Dv1&tag=tag3" + expectedResponse: + httpCode: 200 + contentType: "application/json" + expectedResults: + - tagsAutocompelete: [] + + # timeout + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/tags/autoComplete/values?expr=tag2%3Dv1&tag=tag3" + expectedResponse: + httpCode: 200 + contentType: "application/json" + expectedResults: + - tagsAutocompelete: [] + # TODO: error check + # httpCode: 503 + # contentType: "text/plain; charset=utf-8" + + # 503 + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/tags/autoComplete/values?expr=tag2%3Dv1&tag=tag4" + expectedResponse: + httpCode: 200 + contentType: "application/json" + expectedResults: + - tagsAutocompelete: [] + # httpCode: 503 + # contentType: "text/plain; charset=utf-8" + +listeners: + - address: ":9070" + expressions: + "/tags/autoComplete/values?expr=tag1%3Dv1&tag=tag2": + tags: + - "value1" + - "value2" + + "/tags/autoComplete/tags?expr=tag1%3Dv1&tagPrefix=tag": + tags: + - "tag2" + + "/tags/autoComplete/values?expr=tag2%3Dv1&tag=tag3": + replyDelayMS: 7000 + tags: + - "value3" + - "value4" + + "/tags/autoComplete/values?expr=tag2%3Dv1&tag=tag4": + code: 503 diff --git a/zipper/broadcast/broadcast_group.go b/zipper/broadcast/broadcast_group.go index 023b5949e..b475a8e24 100644 --- a/zipper/broadcast/broadcast_group.go +++ b/zipper/broadcast/broadcast_group.go @@ -30,6 +30,7 @@ type BroadcastGroup struct { doMultipleRequestsIfSplit bool tldCacheDisabled bool concurrencyLimit int + requireSuccessAll bool fetcher types.Fetcher pathCache pathcache.PathCache @@ -112,6 +113,12 @@ func WithDialer(dialer *net.Dialer) Option { } } +func WithSuccess(requireSuccessAll bool) Option { + return func(bg *BroadcastGroup) { + bg.requireSuccessAll = requireSuccessAll + } +} + func New(opts ...Option) (*BroadcastGroup, merry.Error) { bg := &BroadcastGroup{ limiter: limiter.NoopLimiter{}, @@ -152,7 +159,7 @@ func (bg *BroadcastGroup) SetDoMultipleRequestIfSplit(v bool) { } } -func NewBroadcastGroup(logger *zap.Logger, groupName string, doMultipleRequestsIfSplit bool, servers []types.BackendServer, expireDelaySec int32, concurrencyLimit, maxBatchSize int, timeouts types.Timeouts, tldCacheDisabled bool) (*BroadcastGroup, merry.Error) { +func NewBroadcastGroup(logger *zap.Logger, groupName string, doMultipleRequestsIfSplit bool, servers []types.BackendServer, expireDelaySec int32, concurrencyLimit, maxBatchSize int, timeouts types.Timeouts, tldCacheDisabled bool, requireSuccessAll bool) (*BroadcastGroup, merry.Error) { return New( WithLogger(logger), WithGroupName(groupName), @@ -163,6 +170,7 @@ func NewBroadcastGroup(logger *zap.Logger, groupName string, doMultipleRequestsI WithMaxMetricsPerRequest(maxBatchSize), WithTimeouts(timeouts), WithTLDCache(!tldCacheDisabled), + WithSuccess(requireSuccessAll), ) } @@ -299,12 +307,14 @@ func (bg *BroadcastGroup) doSingleFetch(ctx context.Context, logger *zap.Logger, // TODO(Civil): migrate limiter to merry requests, splitErr := bg.splitRequest(ctx, request, backend) - if len(requests) == 0 && splitErr != nil { - response := types.NewServerFetchResponse() - response.Server = backend.Name() - response.AddError(splitErr) - resCh <- response - return + if len(requests) == 0 { + if splitErr != nil { + response := types.NewServerFetchResponse() + response.Server = backend.Name() + response.AddError(splitErr) + resCh <- response + return + } } logger = logger.With(zap.String("backend_name", backend.Name())) @@ -502,7 +512,7 @@ func (bg *BroadcastGroup) Fetch(ctx context.Context, request *protov3.MultiFetch ) } - if len(result.Response.Metrics) == 0 { + if len(result.Response.Metrics) == 0 || (bg.requireSuccessAll && len(result.Err) > 0) { code, errors := helper.MergeHttpErrors(result.Err) if len(errors) > 0 { err := types.ErrFailedToFetch.WithHTTPCode(code).WithMessage(strings.Join(errors, "\n")) @@ -529,10 +539,22 @@ func (bg *BroadcastGroup) Fetch(ctx context.Context, request *protov3.MultiFetch ) var err merry.Error - if result.Err != nil && len(result.Err) > 0 { - err = types.ErrNonFatalErrors - for _, e := range result.Err { - err = err.WithCause(e) + if len(result.Err) > 0 { + if bg.requireSuccessAll { + code, errors := helper.MergeHttpErrors(result.Err) + if len(errors) > 0 { + err := types.ErrFailedToFetch.WithHTTPCode(code).WithMessage(strings.Join(errors, "\n")) + logger.Debug("errors while fetching data from backends", + zap.Int("httpCode", code), + zap.Strings("errors", errors), + ) + return nil, result.Stats, err + } + } else { + err = types.ErrNonFatalErrors + for _, e := range result.Err { + err = err.WithCause(e) + } } } @@ -605,25 +627,19 @@ func (bg *BroadcastGroup) Find(ctx context.Context, request *protov3.MultiGlobRe ) } - if len(result.Response.Metrics) == 0 { - nonNotFoundErrors := types.ReturnNonNotFoundError(result.Err) - if nonNotFoundErrors != nil { - err := types.ErrFailedToFetch.WithHTTPCode(500) - for _, e := range nonNotFoundErrors { - err = err.WithCause(e) - } - logger.Debug("non-404 errors while fetching data from backends", - zap.Any("errors", result.Err), + var err merry.Error + if len(result.Response.Metrics) == 0 || (bg.requireSuccessAll && len(result.Err) > 0) { + code, errors := helper.MergeHttpErrors(result.Err) + if len(errors) > 0 { + err = types.ErrFailedToFetch.WithHTTPCode(code).WithMessage(strings.Join(errors, "\n")) + logger.Debug("errors while fetching data from backends", + zap.Int("httpCode", code), + zap.Strings("errors", errors), ) - return &protov3.MultiGlobResponse{}, result.Stats, err + return nil, result.Stats, err } - - return &protov3.MultiGlobResponse{}, result.Stats, types.ErrNotFound.WithHTTPCode(404) - } - result.Stats.TotalMetricsCount = 0 - for _, x := range result.Response.Metrics { - result.Stats.TotalMetricsCount += uint64(len(x.Matches)) } + logger.Debug("got some find responses", zap.Int("backends_count", len(backends)), zap.Int("response_count", responseCount), @@ -632,7 +648,14 @@ func (bg *BroadcastGroup) Find(ctx context.Context, request *protov3.MultiGlobRe zap.Any("response", result.Response), ) - var err merry.Error + if len(result.Response.Metrics) == 0 { + return &protov3.MultiGlobResponse{}, result.Stats, types.ErrNotFound.WithHTTPCode(404) + } + result.Stats.TotalMetricsCount = 0 + for _, x := range result.Response.Metrics { + result.Stats.TotalMetricsCount += uint64(len(x.Matches)) + } + if result.Err != nil { err = types.ErrNonFatalErrors for _, e := range result.Err { @@ -705,7 +728,11 @@ func (bg *BroadcastGroup) Info(ctx context.Context, request *protov3.MultiMetric var err merry.Error if result.Err != nil { - err = types.ErrNonFatalErrors + if bg.requireSuccessAll { + err = types.ErrFailedToFetch + } else { + err = types.ErrNonFatalErrors + } for _, e := range result.Err { err = err.WithCause(e) } diff --git a/zipper/broadcast/broadcast_group_test.go b/zipper/broadcast/broadcast_group_test.go index 6125f5d88..0df33f9c4 100644 --- a/zipper/broadcast/broadcast_group_test.go +++ b/zipper/broadcast/broadcast_group_test.go @@ -68,7 +68,7 @@ func TestNewBroadcastGroup(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - b, err := NewBroadcastGroup(logger, tt.name, true, tt.servers, 60, 500, 100, timeouts, false) + b, err := NewBroadcastGroup(logger, tt.name, true, tt.servers, 60, 500, 100, timeouts, false, false) if !errorsAreEqual(err, tt.expectedErr) { t.Fatalf("unexpected error %v, expected %v", err, tt.expectedErr) } @@ -107,18 +107,7 @@ func TestProbeTLDs(t *testing.T) { } for _, tt := range tests { - b, err := New( - WithLogger(logger), - WithGroupName(tt.name), - WithSplitMultipleRequests(true), - WithBackends(tt.servers), - WithPathCache(60), - WithLimiter(500), - WithMaxMetricsPerRequest(100), - WithTimeouts(timeouts), - WithTLDCache(true), - ) - + b, err := NewBroadcastGroup(logger, tt.name, true, tt.servers, 60, 500, 100, timeouts, false, false) if err != nil { t.Fatalf("unexpected error %v", err) } diff --git a/zipper/config/config.go b/zipper/config/config.go index f096d22a2..3abf5ad42 100644 --- a/zipper/config/config.go +++ b/zipper/config/config.go @@ -25,6 +25,7 @@ type Config struct { FallbackMaxBatchSize int `mapstructure:"-"` MaxTries int `mapstructure:"maxTries"` DoMultipleRequestsIfSplit bool `mapstructure:"doMultipleRequestsIfSplit"` + RequireSuccessAll bool `mapstructure:"requireSuccessAll"` // require full success for upstreams queries (for multi-target query) ExpireDelaySec int32 TLDCacheDisabled bool `mapstructure:"tldCacheDisabled"` diff --git a/zipper/helper/requests.go b/zipper/helper/requests.go index 6b047a266..b31e99a13 100644 --- a/zipper/helper/requests.go +++ b/zipper/helper/requests.go @@ -98,6 +98,32 @@ func requestError(err error, server string) merry.Error { return types.ErrResponceError.WithValue("server", server) } +func HttpErrorCode(err merry.Error) (code int) { + if err == nil { + code = http.StatusOK + } else { + c := merry.RootCause(err) + if c == nil { + c = err + } + + code = merry.HTTPCode(err) + if code == http.StatusNotFound { + return + } 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 || merry.Is(c, types.ErrFailedToFetch) { + // simplify code, one error type for communications errors, all we can retry + code = http.StatusServiceUnavailable + } + } + + return +} + func MergeHttpErrors(errors []merry.Error) (int, []string) { returnCode := http.StatusNotFound errMsgs := make([]string, 0) diff --git a/zipper/metadata/metadata.go b/zipper/metadata/metadata.go index 8057da53e..ac1f846cf 100644 --- a/zipper/metadata/metadata.go +++ b/zipper/metadata/metadata.go @@ -13,12 +13,12 @@ import ( type md struct { sync.RWMutex SupportedProtocols map[string]struct{} - ProtocolInits map[string]func(*zap.Logger, types.BackendV2, bool) (types.BackendServer, merry.Error) - ProtocolInitsWithLimiter map[string]func(*zap.Logger, types.BackendV2, bool, limiter.ServerLimiter) (types.BackendServer, merry.Error) + ProtocolInits map[string]func(*zap.Logger, types.BackendV2, bool, bool) (types.BackendServer, merry.Error) + ProtocolInitsWithLimiter map[string]func(*zap.Logger, types.BackendV2, bool, bool, limiter.ServerLimiter) (types.BackendServer, merry.Error) } var Metadata = md{ SupportedProtocols: make(map[string]struct{}), - ProtocolInits: make(map[string]func(*zap.Logger, types.BackendV2, bool) (types.BackendServer, merry.Error)), - ProtocolInitsWithLimiter: make(map[string]func(*zap.Logger, types.BackendV2, bool, limiter.ServerLimiter) (types.BackendServer, merry.Error)), + ProtocolInits: make(map[string]func(*zap.Logger, types.BackendV2, bool, bool) (types.BackendServer, merry.Error)), + ProtocolInitsWithLimiter: make(map[string]func(*zap.Logger, types.BackendV2, bool, bool, limiter.ServerLimiter) (types.BackendServer, merry.Error)), } diff --git a/zipper/protocols/auto/auto_group.go b/zipper/protocols/auto/auto_group.go index 686a0b53e..e10e62e11 100644 --- a/zipper/protocols/auto/auto_group.go +++ b/zipper/protocols/auto/auto_group.go @@ -35,7 +35,7 @@ type capabilityResponse struct { protocol string } -//_internal/capabilities/ +// _internal/capabilities/ func doQuery(ctx context.Context, logger *zap.Logger, groupName string, httpClient *http.Client, limiter limiter.ServerLimiter, server string, request types.Request, resChan chan<- capabilityResponse) { httpQuery := helper.NewHttpQuery(groupName, []string{server}, 1, limiter, httpClient, httpHeaders.ContentTypeCarbonAPIv3PB) rewrite, _ := url.Parse("http://127.0.0.1/_internal/capabilities/") @@ -136,11 +136,11 @@ type AutoGroup struct { groupName string } -func NewWithLimiter(logger *zap.Logger, config types.BackendV2, tldCacheDisabled bool, limiter limiter.ServerLimiter) (types.BackendServer, merry.Error) { +func NewWithLimiter(logger *zap.Logger, config types.BackendV2, tldCacheDisabled, requireSuccessAll bool, limiter limiter.ServerLimiter) (types.BackendServer, merry.Error) { return nil, merry.New("auto group doesn't support anything useful except for New") } -func New(logger *zap.Logger, config types.BackendV2, tldCacheDisabled bool) (types.BackendServer, merry.Error) { +func New(logger *zap.Logger, config types.BackendV2, tldCacheDisabled, requireSuccessAll bool) (types.BackendServer, merry.Error) { logger = logger.With(zap.String("type", "autoGroup"), zap.String("name", config.GroupName)) if config.ConcurrencyLimit == nil { @@ -176,7 +176,7 @@ func New(logger *zap.Logger, config types.BackendV2, tldCacheDisabled bool) (typ cfg := config cfg.GroupName = config.GroupName + "_" + proto cfg.Servers = servers - c, ePtr := backendInit(logger, cfg, tldCacheDisabled) + c, ePtr := backendInit(logger, cfg, tldCacheDisabled, requireSuccessAll) if ePtr != nil { return nil, ePtr } @@ -184,17 +184,8 @@ func New(logger *zap.Logger, config types.BackendV2, tldCacheDisabled bool) (typ backends = append(backends, c) } - return broadcast.New( - broadcast.WithLogger(logger), - broadcast.WithGroupName(config.GroupName+"_broadcast"), - broadcast.WithSplitMultipleRequests(config.DoMultipleRequestsIfSplit), - broadcast.WithBackends(backends), - broadcast.WithPathCache(600), - broadcast.WithLimiter(*config.ConcurrencyLimit), - broadcast.WithMaxMetricsPerRequest(*config.MaxBatchSize), - broadcast.WithTimeouts(*config.Timeouts), - broadcast.WithTLDCache(!tldCacheDisabled), - ) + return broadcast.NewBroadcastGroup(logger, config.GroupName+"_broadcast", config.DoMultipleRequestsIfSplit, backends, + 600, *config.ConcurrencyLimit, *config.MaxBatchSize, *config.Timeouts, tldCacheDisabled, requireSuccessAll) } func (c AutoGroup) MaxMetricsPerRequest() int { diff --git a/zipper/protocols/graphite/graphite_group.go b/zipper/protocols/graphite/graphite_group.go index 26854dc57..fc6d1a46a 100644 --- a/zipper/protocols/graphite/graphite_group.go +++ b/zipper/protocols/graphite/graphite_group.go @@ -55,7 +55,7 @@ func (g *GraphiteGroup) Children() []types.BackendServer { return []types.BackendServer{g} } -func NewWithLimiter(logger *zap.Logger, config types.BackendV2, tldCacheDisabled bool, limiter limiter.ServerLimiter) (types.BackendServer, merry.Error) { +func NewWithLimiter(logger *zap.Logger, config types.BackendV2, tldCacheDisabled, requireSuccessAll bool, limiter limiter.ServerLimiter) (types.BackendServer, merry.Error) { logger = logger.With(zap.String("type", "graphite"), zap.String("protocol", config.Protocol), zap.String("name", config.GroupName)) httpClient := helper.GetHTTPClient(logger, config) @@ -79,7 +79,7 @@ func NewWithLimiter(logger *zap.Logger, config types.BackendV2, tldCacheDisabled return c, nil } -func New(logger *zap.Logger, config types.BackendV2, tldCacheDisabled bool) (types.BackendServer, merry.Error) { +func New(logger *zap.Logger, config types.BackendV2, tldCacheDisabled, requireSuccessAll bool) (types.BackendServer, merry.Error) { if config.ConcurrencyLimit == nil { return nil, types.ErrConcurrencyLimitNotSet } @@ -88,7 +88,7 @@ func New(logger *zap.Logger, config types.BackendV2, tldCacheDisabled bool) (typ } limiter := limiter.NewServerLimiter([]string{config.GroupName}, *config.ConcurrencyLimit) - return NewWithLimiter(logger, config, tldCacheDisabled, limiter) + return NewWithLimiter(logger, config, tldCacheDisabled, requireSuccessAll, limiter) } func (c GraphiteGroup) MaxMetricsPerRequest() int { diff --git a/zipper/protocols/irondb/irondb_group.go b/zipper/protocols/irondb/irondb_group.go index 8dc2d4031..416809783 100644 --- a/zipper/protocols/irondb/irondb_group.go +++ b/zipper/protocols/irondb/irondb_group.go @@ -50,7 +50,7 @@ type IronDBGroup struct { graphitePrefix string } -func NewWithLimiter(logger *zap.Logger, config types.BackendV2, tldCacheDisabled bool, limiter limiter.ServerLimiter) (types.BackendServer, merry.Error) { +func NewWithLimiter(logger *zap.Logger, config types.BackendV2, tldCacheDisabled, requireSuccessAll bool, limiter limiter.ServerLimiter) (types.BackendServer, merry.Error) { logger = logger.With(zap.String("type", "irondb"), zap.String("protocol", config.Protocol), zap.String("name", config.GroupName)) logger.Warn("support for this backend protocol is experimental, use with caution") @@ -215,7 +215,7 @@ func NewWithLimiter(logger *zap.Logger, config types.BackendV2, tldCacheDisabled return c, nil } -func New(logger *zap.Logger, config types.BackendV2, tldCacheDisabled bool) (types.BackendServer, merry.Error) { +func New(logger *zap.Logger, config types.BackendV2, tldCacheDisabled, requireSuccessAll bool) (types.BackendServer, merry.Error) { if config.ConcurrencyLimit == nil { return nil, types.ErrConcurrencyLimitNotSet } @@ -224,7 +224,7 @@ func New(logger *zap.Logger, config types.BackendV2, tldCacheDisabled bool) (typ } l := limiter.NewServerLimiter([]string{config.GroupName}, *config.ConcurrencyLimit) - return NewWithLimiter(logger, config, tldCacheDisabled, l) + return NewWithLimiter(logger, config, tldCacheDisabled, requireSuccessAll, l) } func (c *IronDBGroup) Children() []types.BackendServer { diff --git a/zipper/protocols/prometheus/prometheus_group.go b/zipper/protocols/prometheus/prometheus_group.go index 130776b48..261d537da 100644 --- a/zipper/protocols/prometheus/prometheus_group.go +++ b/zipper/protocols/prometheus/prometheus_group.go @@ -77,7 +77,7 @@ type PrometheusGroup struct { httpQuery *helper.HttpQuery } -func NewWithLimiter(logger *zap.Logger, config types.BackendV2, tldCacheDisabled bool, limiter limiter.ServerLimiter) (types.BackendServer, merry.Error) { +func NewWithLimiter(logger *zap.Logger, config types.BackendV2, tldCacheDisabled, requireSuccessAll bool, limiter limiter.ServerLimiter) (types.BackendServer, merry.Error) { logger = logger.With(zap.String("type", "prometheus"), zap.String("protocol", config.Protocol), zap.String("name", config.GroupName)) logger.Warn("support for this backend protocol is experimental, use with caution") @@ -172,10 +172,10 @@ func NewWithLimiter(logger *zap.Logger, config types.BackendV2, tldCacheDisabled httpQuery := helper.NewHttpQuery(config.GroupName, config.Servers, *config.MaxTries, limiter, httpClient, httpHeaders.ContentTypeCarbonAPIv2PB) - return NewWithEverythingInitialized(logger, config, tldCacheDisabled, limiter, step, maxPointsPerQuery, forceMinStepInterval, delay, httpQuery, httpClient) + return NewWithEverythingInitialized(logger, config, tldCacheDisabled, requireSuccessAll, limiter, step, maxPointsPerQuery, forceMinStepInterval, delay, httpQuery, httpClient) } -func NewWithEverythingInitialized(logger *zap.Logger, config types.BackendV2, tldCacheDisabled bool, limiter limiter.ServerLimiter, step, maxPointsPerQuery int64, forceMinStepInterval time.Duration, delay StartDelay, httpQuery *helper.HttpQuery, httpClient *http.Client) (types.BackendServer, merry.Error) { +func NewWithEverythingInitialized(logger *zap.Logger, config types.BackendV2, tldCacheDisabled, requireSuccessAll bool, limiter limiter.ServerLimiter, step, maxPointsPerQuery int64, forceMinStepInterval time.Duration, delay StartDelay, httpQuery *helper.HttpQuery, httpClient *http.Client) (types.BackendServer, merry.Error) { c := &PrometheusGroup{ groupName: config.GroupName, servers: config.Servers, @@ -197,7 +197,7 @@ func NewWithEverythingInitialized(logger *zap.Logger, config types.BackendV2, tl return c, nil } -func New(logger *zap.Logger, config types.BackendV2, tldCacheDisabled bool) (types.BackendServer, merry.Error) { +func New(logger *zap.Logger, config types.BackendV2, tldCacheDisabled, requireSuccessAll bool) (types.BackendServer, merry.Error) { if config.ConcurrencyLimit == nil { return nil, types.ErrConcurrencyLimitNotSet } @@ -206,7 +206,7 @@ func New(logger *zap.Logger, config types.BackendV2, tldCacheDisabled bool) (typ } l := limiter.NewServerLimiter([]string{config.GroupName}, *config.ConcurrencyLimit) - return NewWithLimiter(logger, config, tldCacheDisabled, l) + return NewWithLimiter(logger, config, tldCacheDisabled, requireSuccessAll, l) } func (c *PrometheusGroup) Children() []types.BackendServer { diff --git a/zipper/protocols/v2/protobuf_group.go b/zipper/protocols/v2/protobuf_group.go index 083307f8b..065fd1a40 100644 --- a/zipper/protocols/v2/protobuf_group.go +++ b/zipper/protocols/v2/protobuf_group.go @@ -58,7 +58,7 @@ func (c *ClientProtoV2Group) Children() []types.BackendServer { return []types.BackendServer{c} } -func NewWithLimiter(logger *zap.Logger, config types.BackendV2, tldCacheDisabled bool, l limiter.ServerLimiter) (types.BackendServer, merry.Error) { +func NewWithLimiter(logger *zap.Logger, config types.BackendV2, tldCacheDisabled, requireSuccessAll bool, l limiter.ServerLimiter) (types.BackendServer, merry.Error) { logger = logger.With(zap.String("type", "protoV2Group"), zap.String("name", config.GroupName)) httpClient := helper.GetHTTPClient(logger, config) @@ -82,7 +82,7 @@ func NewWithLimiter(logger *zap.Logger, config types.BackendV2, tldCacheDisabled return c, nil } -func New(logger *zap.Logger, config types.BackendV2, tldCacheDisabled bool) (types.BackendServer, merry.Error) { +func New(logger *zap.Logger, config types.BackendV2, tldCacheDisabled, requireSuccessAll bool) (types.BackendServer, merry.Error) { if config.ConcurrencyLimit == nil { return nil, types.ErrConcurrencyLimitNotSet } @@ -91,7 +91,7 @@ func New(logger *zap.Logger, config types.BackendV2, tldCacheDisabled bool) (typ } limiter := limiter.NewServerLimiter(config.Servers, *config.ConcurrencyLimit) - return NewWithLimiter(logger, config, tldCacheDisabled, limiter) + return NewWithLimiter(logger, config, tldCacheDisabled, requireSuccessAll, limiter) } func (c ClientProtoV2Group) MaxMetricsPerRequest() int { diff --git a/zipper/protocols/v3/protobuf_group.go b/zipper/protocols/v3/protobuf_group.go index 033e35c4a..35a08677f 100644 --- a/zipper/protocols/v3/protobuf_group.go +++ b/zipper/protocols/v3/protobuf_group.go @@ -52,7 +52,7 @@ func (c *ClientProtoV3Group) Children() []types.BackendServer { return []types.BackendServer{c} } -func New(logger *zap.Logger, config types.BackendV2, tldCacheDisabled bool) (types.BackendServer, merry.Error) { +func New(logger *zap.Logger, config types.BackendV2, tldCacheDisabled, requireSuccessAll bool) (types.BackendServer, merry.Error) { if config.ConcurrencyLimit == nil { return nil, types.ErrConcurrencyLimitNotSet } @@ -61,10 +61,10 @@ func New(logger *zap.Logger, config types.BackendV2, tldCacheDisabled bool) (typ } l := limiter.NewServerLimiter(config.Servers, *config.ConcurrencyLimit) - return NewWithLimiter(logger, config, tldCacheDisabled, l) + return NewWithLimiter(logger, config, tldCacheDisabled, requireSuccessAll, l) } -func NewWithLimiter(logger *zap.Logger, config types.BackendV2, tldCacheDisabled bool, l limiter.ServerLimiter) (types.BackendServer, merry.Error) { +func NewWithLimiter(logger *zap.Logger, config types.BackendV2, tldCacheDisabled, requireSuccessAll bool, l limiter.ServerLimiter) (types.BackendServer, merry.Error) { logger = logger.With(zap.String("type", "protoV3Group"), zap.String("name", config.GroupName)) httpClient := helper.GetHTTPClient(logger, config) diff --git a/zipper/protocols/victoriametrics/victoriametrics_group.go b/zipper/protocols/victoriametrics/victoriametrics_group.go index 00031ebda..855a5a809 100644 --- a/zipper/protocols/victoriametrics/victoriametrics_group.go +++ b/zipper/protocols/victoriametrics/victoriametrics_group.go @@ -64,7 +64,7 @@ type VictoriaMetricsGroup struct { featureSet atomic.Value // *vmSupportedFeatures } -func NewWithLimiter(logger *zap.Logger, config types.BackendV2, tldCacheDisabled bool, limiter limiter.ServerLimiter) (types.BackendServer, merry.Error) { +func NewWithLimiter(logger *zap.Logger, config types.BackendV2, tldCacheDisabled, requireSuccessAll bool, limiter limiter.ServerLimiter) (types.BackendServer, merry.Error) { logger = logger.With(zap.String("type", "victoriametrics"), zap.String("protocol", config.Protocol), zap.String("name", config.GroupName)) httpClient := helper.GetHTTPClient(logger, config) @@ -225,7 +225,7 @@ func NewWithLimiter(logger *zap.Logger, config types.BackendV2, tldCacheDisabled } promLogger := logger.With(zap.String("subclass", "prometheus")) - c.BackendServer, _ = prometheus.NewWithEverythingInitialized(promLogger, config, tldCacheDisabled, limiter, step, maxPointsPerQuery, forceMinStepInterval, delay, httpQuery, httpClient) + c.BackendServer, _ = prometheus.NewWithEverythingInitialized(promLogger, config, tldCacheDisabled, requireSuccessAll, limiter, step, maxPointsPerQuery, forceMinStepInterval, delay, httpQuery, httpClient) c.updateFeatureSet(context.Background()) @@ -240,7 +240,7 @@ func NewWithLimiter(logger *zap.Logger, config types.BackendV2, tldCacheDisabled return c, nil } -func New(logger *zap.Logger, config types.BackendV2, tldCacheDisabled bool) (types.BackendServer, merry.Error) { +func New(logger *zap.Logger, config types.BackendV2, tldCacheDisabled, requireSuccessAll bool) (types.BackendServer, merry.Error) { if config.ConcurrencyLimit == nil { return nil, types.ErrConcurrencyLimitNotSet } @@ -249,5 +249,5 @@ func New(logger *zap.Logger, config types.BackendV2, tldCacheDisabled bool) (typ } l := limiter.NewServerLimiter([]string{config.GroupName}, *config.ConcurrencyLimit) - return NewWithLimiter(logger, config, tldCacheDisabled, l) + return NewWithLimiter(logger, config, tldCacheDisabled, requireSuccessAll, l) } diff --git a/zipper/types/response.go b/zipper/types/response.go index 3a3d6e3d3..9227fdce4 100644 --- a/zipper/types/response.go +++ b/zipper/types/response.go @@ -12,7 +12,7 @@ import ( ) // type Fetcher func(ctx context.Context, logger *zap.Logger, client types.BackendServer, reqs interface{}, resCh chan<- types.ServerFetchResponse) { -//type Fetcher func(ctx context.Context, logger *zap.Logger, client BackendServer, reqs interface{}, resCh chan ServerFetchResponse) { +// type Fetcher func(ctx context.Context, logger *zap.Logger, client BackendServer, reqs interface{}, resCh chan ServerFetchResponse) { type Fetcher func(ctx context.Context, logger *zap.Logger, client BackendServer, reqs interface{}, resCh chan ServerFetcherResponse) type ServerFetcherResponse interface { @@ -52,9 +52,11 @@ GATHER: select { case res := <-resCh: answeredServers[res.GetServer()] = struct{}{} - _ = result.MergeI(res) - responseCount++ - + if err := result.MergeI(res); err == nil { + responseCount++ + } else { + result.AddError(err) + } case <-ctx.Done(): err := ErrTimeoutExceeded.WithValue("timedout_backends", NoAnswerBackends(clients, answeredServers)) result.AddError(err) diff --git a/zipper/zipper.go b/zipper/zipper.go index 70d769b7a..b1c3a4187 100644 --- a/zipper/zipper.go +++ b/zipper/zipper.go @@ -46,7 +46,7 @@ type Zipper struct { logger *zap.Logger } -func createBackendsV2(logger *zap.Logger, backends types.BackendsV2, expireDelaySec int32, tldCacheDisabled bool) ([]types.BackendServer, merry.Error) { +func createBackendsV2(logger *zap.Logger, backends types.BackendsV2, expireDelaySec int32, tldCacheDisabled, requireSuccessAll bool) ([]types.BackendServer, merry.Error) { backendServers := make([]types.BackendServer, 0) var e merry.Error timeouts := backends.Timeouts @@ -110,7 +110,7 @@ func createBackendsV2(logger *zap.Logger, backends types.BackendsV2, expireDelay ) } if lbMethod == types.RoundRobinLB { - backendServer, e = backendInit(logger, backend, tldCacheDisabled) + backendServer, e = backendInit(logger, backend, tldCacheDisabled, requireSuccessAll) if e != nil { return nil, e } @@ -121,23 +121,15 @@ func createBackendsV2(logger *zap.Logger, backends types.BackendsV2, expireDelay for _, server := range backend.Servers { config.Servers = []string{server} config.GroupName = server - backendServer, e = backendInit(logger, config, tldCacheDisabled) + backendServer, e = backendInit(logger, config, tldCacheDisabled, requireSuccessAll) if e != nil { return nil, e } backendServers = append(backendServers, backendServer) } - backendServer, err = broadcast.New( - broadcast.WithLogger(logger), - broadcast.WithGroupName(backend.GroupName), - broadcast.WithSplitMultipleRequests(backend.DoMultipleRequestsIfSplit), - broadcast.WithBackends(backendServers), - broadcast.WithPathCache(expireDelaySec), - broadcast.WithLimiter(*backend.ConcurrencyLimit), - broadcast.WithMaxMetricsPerRequest(*backend.MaxBatchSize), - broadcast.WithTimeouts(timeouts), - broadcast.WithTLDCache(!tldCacheDisabled), + backendServer, err = broadcast.NewBroadcastGroup(logger, backend.GroupName, backend.DoMultipleRequestsIfSplit, backendServers, + expireDelaySec, *backend.ConcurrencyLimit, *backend.MaxBatchSize, timeouts, tldCacheDisabled, requireSuccessAll, ) if err != nil { return nil, merry.Wrap(err) @@ -154,25 +146,16 @@ func NewZipper(sender func(*types.Stats), cfg *config.Config, logger *zap.Logger cfg = config.SanitizeConfig(logger, *cfg) } - backends, err := createBackendsV2(logger, cfg.BackendsV2, int32(cfg.InternalRoutingCache.Seconds()), cfg.TLDCacheDisabled) + backends, err := createBackendsV2(logger, cfg.BackendsV2, int32(cfg.InternalRoutingCache.Seconds()), cfg.TLDCacheDisabled, cfg.RequireSuccessAll) if err != nil { logger.Fatal("errors while initialing zipper store backend", zap.Any("error", err), ) } - logger.Error("DEBUG ERROR LOGGGGG", zap.Any("cfg", cfg)) - broadcastGroup, err := broadcast.New( - broadcast.WithLogger(logger), - broadcast.WithGroupName("root"), - broadcast.WithSplitMultipleRequests(cfg.DoMultipleRequestsIfSplit), - broadcast.WithBackends(backends), - broadcast.WithPathCache(int32(cfg.InternalRoutingCache.Seconds())), - broadcast.WithLimiter(cfg.ConcurrencyLimitPerServer), - broadcast.WithMaxMetricsPerRequest(*cfg.MaxBatchSize), - broadcast.WithTimeouts(cfg.Timeouts), - broadcast.WithTLDCache(cfg.TLDCacheDisabled), + broadcastGroup, err := broadcast.NewBroadcastGroup(logger, "root", cfg.DoMultipleRequestsIfSplit, backends, + int32(cfg.InternalRoutingCache.Seconds()), cfg.ConcurrencyLimitPerServer, *cfg.MaxBatchSize, cfg.Timeouts, cfg.TLDCacheDisabled, cfg.RequireSuccessAll, ) if err != nil { logger.Fatal("error while initialing zipper store backend", @@ -284,9 +267,9 @@ func (z Zipper) FindProtoV3(ctx context.Context, request *protov3.MultiGlobReque if len(findResponse.Err) > 0 { var e merry.Error if len(findResponse.Err) == 1 { - e = findResponse.Err[0] + e = helper.HttpErrorByCode(findResponse.Err[0]) } else { - e = findResponse.Err[1].WithCause(findResponse.Err[0]) + e = helper.HttpErrorByCode(findResponse.Err[1].WithCause(findResponse.Err[0])) } logger.Debug("had errors while fetching result", zap.Any("errors", e), From 2fe7b3bb9351105a3a328239f7e346d52e7711a5 Mon Sep 17 00:00:00 2001 From: Michail Safronov Date: Wed, 27 Mar 2024 02:02:07 +0500 Subject: [PATCH 2/2] feat(render,find): strip private info from returned errors info --- cmd/carbonapi/http/find_handlers.go | 3 +- cmd/carbonapi/http/helper.go | 71 ++++- cmd/carbonapi/http/render_handler.go | 6 +- cmd/carbonapi/http/tags_handler.go | 9 +- cmd/mockbackend/e2etesting.go | 28 +- cmd/mockbackend/e2etesting_test.go | 37 +++ .../connection_refused/carbonapi.yaml | 57 ++++ .../connection_refused.yaml | 72 +++++ .../testcases/find_error/find_error.yaml | 24 ++ .../render_error_all/render_error_all.yaml | 4 + .../render_error_all_rr.yaml | 3 + zipper/helper/errors.go | 168 +++++++++++ zipper/helper/errors_test.go | 279 ++++++++++++++++++ zipper/helper/requests.go | 129 +------- zipper/helper/requests_test.go | 165 ++--------- 15 files changed, 782 insertions(+), 273 deletions(-) create mode 100644 cmd/mockbackend/e2etesting_test.go create mode 100644 cmd/mockbackend/testcases/connection_refused/carbonapi.yaml create mode 100644 cmd/mockbackend/testcases/connection_refused/connection_refused.yaml create mode 100644 zipper/helper/errors.go create mode 100644 zipper/helper/errors_test.go diff --git a/cmd/carbonapi/http/find_handlers.go b/cmd/carbonapi/http/find_handlers.go index 298cae2c4..b84d318e0 100644 --- a/cmd/carbonapi/http/find_handlers.go +++ b/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 diff --git a/cmd/carbonapi/http/helper.go b/cmd/carbonapi/http/helper.go index d35f5a56b..7757e94cd 100644 --- a/cmd/carbonapi/http/helper.go +++ b/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 { diff --git a/cmd/carbonapi/http/render_handler.go b/cmd/carbonapi/http/render_handler.go index f2938527c..af3b6f65e 100644 --- a/cmd/carbonapi/http/render_handler.go +++ b/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 } diff --git a/cmd/carbonapi/http/tags_handler.go b/cmd/carbonapi/http/tags_handler.go index 2663f37d3..4f3216e2c 100644 --- a/cmd/carbonapi/http/tags_handler.go +++ b/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 diff --git a/cmd/mockbackend/e2etesting.go b/cmd/mockbackend/e2etesting.go index b8ab00ef3..2c7283706 100644 --- a/cmd/mockbackend/e2etesting.go +++ b/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"` } @@ -177,6 +180,20 @@ func max(a, b int) int { return b } +func resortErr(errStr string) string { + first := strings.Index(errStr, "\n") + if first >= 0 && first != len(errStr)-1 { + // resort error string + errs := strings.Split(errStr, "\n") + if errs[len(errs)-1] == "" { + errs = errs[:len(errs)-1] + } + sort.Strings(errs) + errStr = strings.Join(errs, "\n") + "\n" + } + return errStr +} + func doTest(logger *zap.Logger, t *Query, verbose bool) []error { client := http.Client{} failures := make([]error, 0) @@ -245,8 +262,17 @@ 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 { + errStr = resortErr(errStr) + } + if t.ExpectedResponse.ErrBody != errStr { + failures = append(failures, merry2.Errorf("mismatch error body, got '%s', expected '%s'", string(b), t.ExpectedResponse.ErrBody)) + } + } return failures } diff --git a/cmd/mockbackend/e2etesting_test.go b/cmd/mockbackend/e2etesting_test.go new file mode 100644 index 000000000..325dcf47f --- /dev/null +++ b/cmd/mockbackend/e2etesting_test.go @@ -0,0 +1,37 @@ +package main + +import ( + "strconv" + "testing" +) + +func Test_resortErr(t *testing.T) { + tests := []struct { + errStr string + want string + }{ + { + errStr: "b: connection refused\na: connection refused\n", + want: "a: connection refused\nb: connection refused\n", + }, + { + errStr: "a: connection refused\nb: connection refused\n", + want: "a: connection refused\nb: connection refused\n", + }, + { + errStr: "", + want: "", + }, + { + errStr: "\n", + want: "\n", + }, + } + for i, tt := range tests { + t.Run(strconv.Itoa(i), func(t *testing.T) { + if got := resortErr(tt.errStr); got != tt.want { + t.Errorf("resortErr(%q) = %q, want %q", tt.errStr, got, tt.want) + } + }) + } +} diff --git a/cmd/mockbackend/testcases/connection_refused/carbonapi.yaml b/cmd/mockbackend/testcases/connection_refused/carbonapi.yaml new file mode 100644 index 000000000..e9109f72d --- /dev/null +++ b/cmd/mockbackend/testcases/connection_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" diff --git a/cmd/mockbackend/testcases/connection_refused/connection_refused.yaml b/cmd/mockbackend/testcases/connection_refused/connection_refused.yaml new file mode 100644 index 000000000..0ca748654 --- /dev/null +++ b/cmd/mockbackend/testcases/connection_refused/connection_refused.yaml @@ -0,0 +1,72 @@ +version: "v1" +test: + apps: + - name: "carbonapi" + binary: "./carbonapi" + args: + - "-config" + - "./cmd/mockbackend/testcases/connection_refused/carbonapi.yaml" + - "-exact-config" + queries: + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=a&format=json" + expectedResponse: + httpCode: 503 + contentType: "text/plain; charset=utf-8" + errBody: "a: connection refused\n" + errSort: true + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=a&target=b&format=json" + expectedResponse: + httpCode: 503 + contentType: "text/plain; charset=utf-8" + errBody: "a: connection refused\nb: connection refused\n" + errSort: true + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/metrics/find/?query=a&format=json" + expectedResponse: + httpCode: 503 + contentType: "text/plain; charset=utf-8" + errBody: "connection refused\n" + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/metrics/find/?query=a&query=b&format=json" + expectedResponse: + httpCode: 503 + contentType: "text/plain; charset=utf-8" + errBody: "connection refused\n" + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/tags/autoComplete/values?expr=tag2%3Dv1&tag=tag4" + expectedResponse: + httpCode: 200 + contentType: "application/json" + expectedResults: + - tagsAutocompelete: [] + # TODO: query must fail + # httpCode: 503 + # contentType: "text/plain; charset=utf-8" + # errBody: "connection refused\n" + # errSort: true + +listeners: + - address: ":9070" + expressions: + "a": + pathExpression: "a" + data: + - metricName: "a" + values: [0,1,2,2,3] + + # timeout + "c": + pathExpression: "c" + code: 404 + replyDelayMS: 7000 + + "d": + pathExpression: "d" + code: 503 diff --git a/cmd/mockbackend/testcases/find_error/find_error.yaml b/cmd/mockbackend/testcases/find_error/find_error.yaml index 53b5003f7..ad1fdf526 100644 --- a/cmd/mockbackend/testcases/find_error/find_error.yaml +++ b/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 diff --git a/cmd/mockbackend/testcases/render_error_all/render_error_all.yaml b/cmd/mockbackend/testcases/render_error_all/render_error_all.yaml index d312d89d0..b8ee69c27 100644 --- a/cmd/mockbackend/testcases/render_error_all/render_error_all.yaml +++ b/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" diff --git a/cmd/mockbackend/testcases/render_error_all_rr/render_error_all_rr.yaml b/cmd/mockbackend/testcases/render_error_all_rr/render_error_all_rr.yaml index a401b99ae..2c0e64748 100644 --- a/cmd/mockbackend/testcases/render_error_all_rr/render_error_all_rr.yaml +++ b/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" diff --git a/zipper/helper/errors.go b/zipper/helper/errors.go new file mode 100644 index 000000000..1a59f612e --- /dev/null +++ b/zipper/helper/errors.go @@ -0,0 +1,168 @@ +package helper + +import ( + "context" + "net" + "net/http" + "net/url" + "strings" + + "github.com/ansel1/merry" + "github.com/go-graphite/carbonapi/pkg/parser" + "github.com/go-graphite/carbonapi/zipper/types" +) + +func requestError(err error, server string) merry.Error { + // with code InternalServerError by default, overwritten by custom error + if merry.Is(err, context.DeadlineExceeded) { + return types.ErrTimeoutExceeded.WithValue("server", server).WithCause(err) + } + if urlErr, ok := err.(*url.Error); ok { + if netErr, ok := urlErr.Err.(*net.OpError); ok { + return types.ErrBackendError.WithValue("server", server).WithCause(netErr) + } + } + if netErr, ok := err.(*net.OpError); ok { + return types.ErrBackendError.WithValue("server", server).WithCause(netErr) + } + return types.ErrResponceError.WithValue("server", server) +} + +func HttpErrorCode(err merry.Error) (code int) { + if err == nil { + code = http.StatusOK + } else { + c := merry.RootCause(err) + if c == nil { + c = err + } + + code = merry.HTTPCode(err) + if code == http.StatusNotFound { + return + } 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 || merry.Is(c, types.ErrFailedToFetch) { + // simplify code, one error type for communications errors, all we can retry + code = http.StatusServiceUnavailable + } + } + + return +} + +// for stable return code on multiply errors +func recalcCode(code, newCode int) int { + if newCode == http.StatusGatewayTimeout || newCode == http.StatusBadGateway { + // simplify code, one error type for communications errors, all we can retry + newCode = http.StatusServiceUnavailable + } + if code == 0 || code == http.StatusNotFound { + return newCode + } + + if newCode >= 400 && newCode < 500 && code >= 400 && code < 500 { + if newCode == http.StatusBadRequest { + return newCode + } else if newCode == http.StatusForbidden && code != http.StatusBadRequest { + return newCode + } + } + if newCode < code { + code = newCode + } + return code +} + +// MerryRootError strip merry error chain +func MerryRootError(err error) string { + c := merry.RootCause(err) + if c == nil { + c = err + } + return merryError(c) +} + +func merryError(err error) string { + if msg := merry.Message(err); len(msg) > 0 { + return strings.TrimRight(msg, "\n") + } else { + return err.Error() + } +} + +func MergeHttpErrors(errors []merry.Error) (int, []string) { + returnCode := http.StatusNotFound + errMsgs := make([]string, 0) + 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 + } + + errMsgs = append(errMsgs, merryError(c)) + + returnCode = recalcCode(returnCode, code) + } + + return returnCode, errMsgs +} + +func MergeHttpErrorMap(errorsMap map[string]merry.Error) (returnCode int, errMap map[string]string) { + returnCode = http.StatusNotFound + errMap = make(map[string]string) + for key, err := range errorsMap { + 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 + } + + msg := merryError(c) + errMap[key] = msg + returnCode = recalcCode(returnCode, code) + } + + return +} + +func HttpErrorByCode(err merry.Error) merry.Error { + var returnErr merry.Error + if err == nil { + returnErr = types.ErrNoMetricsFetched + } else { + code := merry.HTTPCode(err) + msg := stripHtmlTags(merry.Message(err), 0) + if code == http.StatusForbidden { + returnErr = types.ErrForbidden + if len(msg) > 0 { + // pass message to caller + returnErr = returnErr.WithMessage(msg) + } + } else if code == http.StatusServiceUnavailable || code == http.StatusBadGateway || code == http.StatusGatewayTimeout { + returnErr = types.ErrFailedToFetch.WithHTTPCode(code).WithMessage(msg) + } else { + returnErr = types.ErrFailed.WithHTTPCode(code).WithMessage(msg) + } + } + + return returnErr +} diff --git a/zipper/helper/errors_test.go b/zipper/helper/errors_test.go new file mode 100644 index 000000000..87bcd983e --- /dev/null +++ b/zipper/helper/errors_test.go @@ -0,0 +1,279 @@ +package helper + +import ( + "fmt" + "net" + "net/http" + "reflect" + "testing" + + "github.com/ansel1/merry" + "github.com/go-graphite/carbonapi/zipper/types" +) + +func TestMergeHttpErrors(t *testing.T) { + tests := []struct { + name string + errors []merry.Error + wantCode int + want []string + }{ + { + name: "NotFound", + errors: []merry.Error{}, + wantCode: http.StatusNotFound, + want: []string{}, + }, + { + name: "NetErr", + errors: []merry.Error{ + types.ErrBackendError.WithValue("server", "test").WithCause(&net.OpError{Op: "connect", Err: fmt.Errorf("refused")}).WithHTTPCode(http.StatusServiceUnavailable), + }, + wantCode: http.StatusServiceUnavailable, + want: []string{"connect: refused"}, + }, + { + name: "NetErr (incapsulated)", + errors: []merry.Error{ + types.ErrMaxTriesExceeded.WithCause(types.ErrBackendError.WithValue("server", "test").WithCause(&net.OpError{Op: "connect", Err: fmt.Errorf("refused")})).WithHTTPCode(http.StatusServiceUnavailable), + }, + wantCode: http.StatusServiceUnavailable, + want: []string{"connect: refused"}, + }, + { + name: "ServiceUnavailable", + errors: []merry.Error{ + merry.New("unavaliable").WithHTTPCode(http.StatusServiceUnavailable), + }, + wantCode: http.StatusServiceUnavailable, + want: []string{"unavaliable"}, + }, + { + name: "GatewayTimeout and ServiceUnavailable", + errors: []merry.Error{ + merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), + merry.New("unavaliable").WithHTTPCode(http.StatusServiceUnavailable), + }, + wantCode: http.StatusServiceUnavailable, + want: []string{"timeout", "unavaliable"}, + }, + { + name: "ServiceUnavailable and GatewayTimeout", + errors: []merry.Error{ + merry.New("unavaliable").WithHTTPCode(http.StatusServiceUnavailable), + merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), + }, + wantCode: http.StatusServiceUnavailable, + want: []string{"unavaliable", "timeout"}, + }, + { + name: "Forbidden and GatewayTimeout", + errors: []merry.Error{ + merry.New("limit").WithHTTPCode(http.StatusForbidden), + merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), + }, + wantCode: http.StatusForbidden, + want: []string{"limit", "timeout"}, + }, + { + name: "GatewayTimeout and Forbidden", + errors: []merry.Error{ + merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), + merry.New("limit").WithHTTPCode(http.StatusForbidden), + }, + wantCode: http.StatusForbidden, + want: []string{"timeout", "limit"}, + }, + { + name: "InternalServerError and Forbidden", + errors: []merry.Error{ + merry.New("error").WithHTTPCode(http.StatusInternalServerError), + merry.New("limit").WithHTTPCode(http.StatusForbidden), + }, + wantCode: http.StatusForbidden, + want: []string{"error", "limit"}, + }, + { + name: "InternalServerError and GatewayTimeout", + errors: []merry.Error{ + merry.New("error").WithHTTPCode(http.StatusInternalServerError), + merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), + }, + wantCode: http.StatusInternalServerError, + want: []string{"error", "timeout"}, + }, + { + name: "GatewayTimeout and InternalServerError", + errors: []merry.Error{ + merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), + merry.New("error").WithHTTPCode(http.StatusInternalServerError), + }, + wantCode: http.StatusInternalServerError, + want: []string{"timeout", "error"}, + }, + { + name: "BadRequest and Forbidden", + errors: []merry.Error{ + merry.New("error").WithHTTPCode(http.StatusBadRequest), + merry.New("limit").WithHTTPCode(http.StatusForbidden), + }, + wantCode: http.StatusBadRequest, + want: []string{"error", "limit"}, + }, + { + name: "Forbidden and BadRequest", + errors: []merry.Error{ + merry.New("limit").WithHTTPCode(http.StatusForbidden), + merry.New("error").WithHTTPCode(http.StatusBadRequest), + }, + wantCode: http.StatusBadRequest, + want: []string{"limit", "error"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotCode, got := MergeHttpErrors(tt.errors) + if gotCode != tt.wantCode { + t.Errorf("MergeHttpErrors() gotCode = %v, want %v", gotCode, tt.wantCode) + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("MergeHttpErrors() got = %v, want %v", got, tt.want) + } + }) + } +} + +func TestMergeHttpErrorMap(t *testing.T) { + tests := []struct { + name string + errors map[string]merry.Error + wantCode int + want map[string]string + }{ + { + name: "NotFound", + errors: map[string]merry.Error{}, + wantCode: http.StatusNotFound, + want: map[string]string{}, + }, + { + name: "NetErr", + errors: map[string]merry.Error{ + "a": types.ErrBackendError.WithValue("server", "test").WithCause(&net.OpError{Op: "connect", Err: fmt.Errorf("refused")}).WithHTTPCode(http.StatusServiceUnavailable), + }, + wantCode: http.StatusServiceUnavailable, + want: map[string]string{"a": "connect: refused"}, + }, + { + name: "NetErr (incapsulated)", + errors: map[string]merry.Error{ + "b": types.ErrMaxTriesExceeded.WithCause(types.ErrBackendError.WithValue("server", "test").WithCause(&net.OpError{Op: "connect", Err: fmt.Errorf("refused")})).WithHTTPCode(http.StatusServiceUnavailable), + }, + wantCode: http.StatusServiceUnavailable, + want: map[string]string{"b": "connect: refused"}, + }, + { + name: "ServiceUnavailable", + errors: map[string]merry.Error{ + "d": merry.New("unavaliable").WithHTTPCode(http.StatusServiceUnavailable), + }, + wantCode: http.StatusServiceUnavailable, + want: map[string]string{"d": "unavaliable"}, + }, + { + name: "GatewayTimeout and ServiceUnavailable", + errors: map[string]merry.Error{ + "a": merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), + "de": merry.New("unavaliable").WithHTTPCode(http.StatusServiceUnavailable), + }, + wantCode: http.StatusServiceUnavailable, + want: map[string]string{"a": "timeout", "de": "unavaliable"}, + }, + { + name: "ServiceUnavailable and GatewayTimeout", + errors: map[string]merry.Error{ + "de": merry.New("unavaliable").WithHTTPCode(http.StatusServiceUnavailable), + "a": merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), + }, + wantCode: http.StatusServiceUnavailable, + want: map[string]string{"a": "timeout", "de": "unavaliable"}, + }, + { + name: "Forbidden and GatewayTimeout", + errors: map[string]merry.Error{ + "de": merry.New("limit").WithHTTPCode(http.StatusForbidden), + "c": merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), + }, + wantCode: http.StatusForbidden, + want: map[string]string{"c": "timeout", "de": "limit"}, + }, + { + name: "GatewayTimeout and Forbidden", + errors: map[string]merry.Error{ + "a": merry.New("limit").WithHTTPCode(http.StatusForbidden), + "c": merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), + }, + wantCode: http.StatusForbidden, + want: map[string]string{"a": "limit", "c": "timeout"}, + }, + { + name: "InternalServerError and Forbidden", + errors: map[string]merry.Error{ + "a": merry.New("error").WithHTTPCode(http.StatusInternalServerError), + "cd": merry.New("limit").WithHTTPCode(http.StatusForbidden), + }, + wantCode: http.StatusForbidden, + want: map[string]string{"a": "error", "cd": "limit"}, + }, + { + name: "InternalServerError and GatewayTimeout", + errors: map[string]merry.Error{ + "a": merry.New("error").WithHTTPCode(http.StatusInternalServerError), + "b": merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), + }, + wantCode: http.StatusInternalServerError, + want: map[string]string{"a": "error", "b": "timeout"}, + }, + { + name: "GatewayTimeout and InternalServerError", + errors: map[string]merry.Error{ + "a": merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), + "cd": merry.New("error").WithHTTPCode(http.StatusInternalServerError), + }, + wantCode: http.StatusInternalServerError, + want: map[string]string{"a": "timeout", "cd": "error"}, + }, + { + name: "BadRequest and Forbidden", + errors: map[string]merry.Error{ + "de": merry.New("error").WithHTTPCode(http.StatusBadRequest), + "a": merry.New("limit").WithHTTPCode(http.StatusForbidden), + }, + wantCode: http.StatusBadRequest, + want: map[string]string{"a": "limit", "de": "error"}, + }, + { + name: "Forbidden and BadRequest", + errors: map[string]merry.Error{ + "a": merry.New("limit").WithHTTPCode(http.StatusForbidden), + "b{c,de,klmn}.cde.d{c,de,klmn}.e{c,de,klmn}.k{c,de,klmn}.b{c,de,klmn}.cde.d{c,de,klmn}.e{c,de,klmn}.k{c,de,klmn}.e{c,de,klmn}.k{c,de,klmn}": merry.New("error").WithHTTPCode(http.StatusBadRequest), + }, + wantCode: http.StatusBadRequest, + want: map[string]string{ + "a": "limit", + "b{c,de,klmn}.cde.d{c,de,klmn}.e{c,de,klmn}.k{c,de,klmn}.b{c,de,klmn}.cde.d{c,de,klmn}.e{c,de,klmn}.k{c,de,klmn}.e{c,de,klmn}.k{c,de,klmn}": "error", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotCode, got := MergeHttpErrorMap(tt.errors) + if gotCode != tt.wantCode { + t.Errorf("MergeHttpErrors() gotCode = %v, want %v", gotCode, tt.wantCode) + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("MergeHttpErrors() got = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/zipper/helper/requests.go b/zipper/helper/requests.go index b31e99a13..2b8d0ed2d 100644 --- a/zipper/helper/requests.go +++ b/zipper/helper/requests.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "io" - "net" "net/http" "net/url" "strings" @@ -15,7 +14,6 @@ import ( "go.uber.org/zap" "github.com/go-graphite/carbonapi/limiter" - "github.com/go-graphite/carbonapi/pkg/parser" util "github.com/go-graphite/carbonapi/util/ctx" "github.com/go-graphite/carbonapi/zipper/types" ) @@ -82,124 +80,6 @@ func stripHtmlTags(s string, maxLen int) string { return s } -func requestError(err error, server string) merry.Error { - // with code InternalServerError by default, overwritten by custom error - if merry.Is(err, context.DeadlineExceeded) { - return types.ErrTimeoutExceeded.WithValue("server", server).WithCause(err) - } - if urlErr, ok := err.(*url.Error); ok { - if netErr, ok := urlErr.Err.(*net.OpError); ok { - return types.ErrBackendError.WithValue("server", server).WithCause(netErr) - } - } - if netErr, ok := err.(*net.OpError); ok { - return types.ErrBackendError.WithValue("server", server).WithCause(netErr) - } - return types.ErrResponceError.WithValue("server", server) -} - -func HttpErrorCode(err merry.Error) (code int) { - if err == nil { - code = http.StatusOK - } else { - c := merry.RootCause(err) - if c == nil { - c = err - } - - code = merry.HTTPCode(err) - if code == http.StatusNotFound { - return - } 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 || merry.Is(c, types.ErrFailedToFetch) { - // simplify code, one error type for communications errors, all we can retry - code = http.StatusServiceUnavailable - } - } - - return -} - -func MergeHttpErrors(errors []merry.Error) (int, []string) { - returnCode := http.StatusNotFound - errMsgs := make([]string, 0) - 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 msg := merry.Message(c); len(msg) > 0 { - errMsgs = append(errMsgs, strings.TrimRight(msg, "\n")) - } else { - errMsgs = append(errMsgs, c.Error()) - } - - 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, errMsgs -} - -func MergeHttpErrorMap(errorsMap map[string]merry.Error) (int, []string) { - errors := make([]merry.Error, len(errorsMap)) - i := 0 - for _, err := range errorsMap { - errors[i] = err - i++ - } - - return MergeHttpErrors(errors) -} - -func HttpErrorByCode(err merry.Error) merry.Error { - var returnErr merry.Error - if err == nil { - returnErr = types.ErrNoMetricsFetched - } else { - code := merry.HTTPCode(err) - msg := stripHtmlTags(merry.Message(err), 0) - if code == http.StatusForbidden { - returnErr = types.ErrForbidden - if len(msg) > 0 { - // pass message to caller - returnErr = returnErr.WithMessage(msg) - } - } else if code == http.StatusServiceUnavailable || code == http.StatusBadGateway || code == http.StatusGatewayTimeout { - returnErr = types.ErrFailedToFetch.WithHTTPCode(code).WithMessage(msg) - } else { - returnErr = types.ErrFailed.WithHTTPCode(code).WithMessage(msg) - } - } - - return returnErr -} - type ServerResponse struct { Server string Response []byte @@ -332,7 +212,7 @@ func (c *HttpQuery) doRequest(ctx context.Context, logger *zap.Logger, server, u return &ServerResponse{Server: server, Response: body}, nil } -func (c *HttpQuery) DoQuery(ctx context.Context, logger *zap.Logger, uri string, r types.Request) (*ServerResponse, merry.Error) { +func (c *HttpQuery) DoQuery(ctx context.Context, logger *zap.Logger, uri string, r types.Request) (resp *ServerResponse, err merry.Error) { maxTries := c.maxTries if len(c.servers) > maxTries { maxTries = len(c.servers) @@ -345,11 +225,14 @@ func (c *HttpQuery) DoQuery(ctx context.Context, logger *zap.Logger, uri string, res, err := c.doRequest(ctx, logger, server, uri, r) if err != nil { logger.Debug("have errors", - zap.Error(err), + zap.String("error", err.Error()), + zap.String("server", server), ) e = e.WithCause(err).WithHTTPCode(merry.HTTPCode(err)) code = merry.HTTPCode(err) + // TODO (msaf1980): may be metric for server failures ? + // TODO (msaf1980): may be retry policy for avoid retry bad queries ? continue } @@ -359,7 +242,7 @@ func (c *HttpQuery) DoQuery(ctx context.Context, logger *zap.Logger, uri string, return nil, types.ErrMaxTriesExceeded.WithCause(e).WithHTTPCode(code) } -func (c *HttpQuery) DoQueryToAll(ctx context.Context, logger *zap.Logger, uri string, r types.Request) ([]*ServerResponse, merry.Error) { +func (c *HttpQuery) DoQueryToAll(ctx context.Context, logger *zap.Logger, uri string, r types.Request) (resp []*ServerResponse, err merry.Error) { maxTries := c.maxTries if len(c.servers) > maxTries { maxTries = len(c.servers) diff --git a/zipper/helper/requests_test.go b/zipper/helper/requests_test.go index 0b60d7187..afd2ecb36 100644 --- a/zipper/helper/requests_test.go +++ b/zipper/helper/requests_test.go @@ -1,151 +1,12 @@ package helper import ( - "fmt" - "net" - "net/http" - "reflect" + "strconv" "testing" - "github.com/ansel1/merry" - "github.com/go-graphite/carbonapi/zipper/types" "github.com/stretchr/testify/assert" ) -func TestMergeHttpErrors(t *testing.T) { - type args struct { - } - tests := []struct { - name string - errors []merry.Error - wantCode int - want []string - }{ - { - name: "NotFound", - errors: []merry.Error{}, - wantCode: http.StatusNotFound, - want: []string{}, - }, - { - name: "NetErr", - errors: []merry.Error{ - types.ErrBackendError.WithValue("server", "test").WithCause(&net.OpError{Op: "connect", Err: fmt.Errorf("refused")}).WithHTTPCode(http.StatusServiceUnavailable), - }, - wantCode: http.StatusServiceUnavailable, - want: []string{"connect: refused"}, - }, - { - name: "NetErr (incapsulated)", - errors: []merry.Error{ - types.ErrMaxTriesExceeded.WithCause(types.ErrBackendError.WithValue("server", "test").WithCause(&net.OpError{Op: "connect", Err: fmt.Errorf("refused")})).WithHTTPCode(http.StatusServiceUnavailable), - }, - wantCode: http.StatusServiceUnavailable, - want: []string{"connect: refused"}, - }, - { - name: "ServiceUnavailable", - errors: []merry.Error{ - merry.New("unavaliable").WithHTTPCode(http.StatusServiceUnavailable), - }, - wantCode: http.StatusServiceUnavailable, - want: []string{"unavaliable"}, - }, - { - name: "GatewayTimeout and ServiceUnavailable", - errors: []merry.Error{ - merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), - merry.New("unavaliable").WithHTTPCode(http.StatusServiceUnavailable), - }, - wantCode: http.StatusServiceUnavailable, - want: []string{"timeout", "unavaliable"}, - }, - { - name: "ServiceUnavailable and GatewayTimeout", - errors: []merry.Error{ - merry.New("unavaliable").WithHTTPCode(http.StatusServiceUnavailable), - merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), - }, - wantCode: http.StatusServiceUnavailable, - want: []string{"unavaliable", "timeout"}, - }, - { - name: "Forbidden and GatewayTimeout", - errors: []merry.Error{ - merry.New("limit").WithHTTPCode(http.StatusForbidden), - merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), - }, - wantCode: http.StatusForbidden, - want: []string{"limit", "timeout"}, - }, - { - name: "GatewayTimeout and Forbidden", - errors: []merry.Error{ - merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), - merry.New("limit").WithHTTPCode(http.StatusForbidden), - }, - wantCode: http.StatusForbidden, - want: []string{"timeout", "limit"}, - }, - { - name: "InternalServerError and Forbidden", - errors: []merry.Error{ - merry.New("error").WithHTTPCode(http.StatusInternalServerError), - merry.New("limit").WithHTTPCode(http.StatusForbidden), - }, - wantCode: http.StatusForbidden, - want: []string{"error", "limit"}, - }, - { - name: "InternalServerError and GatewayTimeout", - errors: []merry.Error{ - merry.New("error").WithHTTPCode(http.StatusInternalServerError), - merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), - }, - wantCode: http.StatusInternalServerError, - want: []string{"error", "timeout"}, - }, - { - name: "GatewayTimeout and InternalServerError", - errors: []merry.Error{ - merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), - merry.New("error").WithHTTPCode(http.StatusInternalServerError), - }, - wantCode: http.StatusInternalServerError, - want: []string{"timeout", "error"}, - }, - { - name: "BadRequest and Forbidden", - errors: []merry.Error{ - merry.New("error").WithHTTPCode(http.StatusBadRequest), - merry.New("limit").WithHTTPCode(http.StatusForbidden), - }, - wantCode: http.StatusForbidden, // Last win - want: []string{"error", "limit"}, - }, - { - name: "Forbidden and BadRequest", - errors: []merry.Error{ - merry.New("limit").WithHTTPCode(http.StatusForbidden), - merry.New("error").WithHTTPCode(http.StatusBadRequest), - }, - wantCode: http.StatusBadRequest, // Last win - want: []string{"limit", "error"}, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - gotCode, got := MergeHttpErrors(tt.errors) - if gotCode != tt.wantCode { - t.Errorf("MergeHttpErrors() gotCode = %v, want %v", gotCode, tt.wantCode) - } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("MergeHttpErrors() got = %v, want %v", got, tt.want) - } - }) - } -} - func Test_stripHtmlTags(t *testing.T) { tests := []struct { name string @@ -184,3 +45,27 @@ func Test_stripHtmlTags(t *testing.T) { }) } } + +func Test_recalcCode(t *testing.T) { + tests := []struct { + code int + newCode int + want int + }{ + {code: 500, newCode: 403, want: 403}, + {code: 403, newCode: 500, want: 403}, + {code: 403, newCode: 400, want: 400}, + {code: 400, newCode: 403, want: 400}, + {code: 500, newCode: 503, want: 500}, + {code: 503, newCode: 500, want: 500}, + {code: 503, newCode: 502, want: 503}, + {code: 0, newCode: 502, want: 503}, + } + for i, tt := range tests { + t.Run(strconv.Itoa(i), func(t *testing.T) { + if got := recalcCode(tt.code, tt.newCode); got != tt.want { + t.Errorf("recalcCode(%d, %d) = %d, want %d", tt.code, tt.newCode, got, tt.want) + } + }) + } +}