Skip to content

Commit

Permalink
feat(render,find): return error on partial targets fetch and return d…
Browse files Browse the repository at this point in the history 
…etailed error instead of 500
  • Loading branch information
@msaf1980
msaf1980 committed Mar 14, 2024
1 parent 7f00f02 commit 9d39747
Show file tree
Hide file tree
Showing 28 changed files with 968 additions and 271 deletions.
26 changes: 8 additions & 18 deletions cmd/carbonapi/http/find_handlers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
17 changes: 8 additions & 9 deletions cmd/carbonapi/http/render_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,19 +201,15 @@ func renderHandler(w http.ResponseWriter, r *http.Request) {
if format == protoV3Format {
body, err := io.ReadAll(r.Body)
if err != nil {
accessLogDetails.HTTPCode = http.StatusBadRequest
accessLogDetails.Reason = "failed to parse message body: " + err.Error()
http.Error(w, "bad request (failed to parse format): "+err.Error(), http.StatusBadRequest)
setError(w, accessLogDetails, "failed to parse message body: "+err.Error(), http.StatusBadRequest, uid.String())
return
}

var pv3Request pb.MultiFetchRequest
err = pv3Request.Unmarshal(body)

if err != nil {
accessLogDetails.HTTPCode = http.StatusBadRequest
accessLogDetails.Reason = "failed to parse message body: " + err.Error()
http.Error(w, "bad request (failed to parse format): "+err.Error(), http.StatusBadRequest)
setError(w, accessLogDetails, "failed to parse message body: "+err.Error(), http.StatusBadRequest, uid.String())
return
}

Expand Down Expand Up @@ -327,6 +323,9 @@ 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 {
// break
// }
}

results = append(results, result...)
Expand All @@ -347,19 +346,19 @@ func renderHandler(w http.ResponseWriter, r *http.Request) {
var body []byte

returnCode := http.StatusOK
if len(results) == 0 {
if len(results) == 0 || (len(errors) > 0 && config.Config.Upstreams.RequireSuccessAll) {
// Obtain error code from the errors
// In case we have only "Not Found" errors, result should be 404
// Otherwise it should be 500
var errMsgs []string
returnCode, errMsgs = helper.MergeHttpErrorMap(errors)
logger.Debug("error response or no response", zap.Strings("error", errMsgs))
// Allow override status code for 404-not-found replies.
if returnCode == 404 {
if returnCode == http.StatusNotFound {
returnCode = config.Config.NotFoundStatusCode
}

if returnCode == 400 || returnCode == http.StatusForbidden || returnCode >= 500 {
if returnCode == http.StatusBadRequest || returnCode == http.StatusNotFound || returnCode == http.StatusForbidden || returnCode >= 500 {
setError(w, accessLogDetails, strings.Join(errMsgs, ","), returnCode, uid.String())
logAsError = true
return
Expand Down
138 changes: 101 additions & 37 deletions cmd/mockbackend/e2etesting.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"net/http"
"net/url"
"os"
"reflect"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -48,11 +49,21 @@ type ExpectedResponse struct {
}

type ExpectedResult struct {
SHA256 []string `yaml:"sha256"`
Metrics []CarbonAPIResponse
SHA256 []string `yaml:"sha256"`
Metrics []RenderResponse
MetricsFind []MetricsFindResponse `json:"metricsFind" yaml:"metricsFind"`
}

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"`
Expand Down Expand Up @@ -121,7 +132,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)
}
Expand Down Expand Up @@ -158,7 +169,7 @@ func isMetricsEqual(m1, m2 CarbonAPIResponse) error {
return nil
}

func doTest(logger *zap.Logger, t *Query) []error {
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")
Expand Down Expand Up @@ -201,14 +212,6 @@ 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,
Expand All @@ -226,6 +229,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
Expand All @@ -245,45 +256,98 @@ 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 {
for i := range t.ExpectedResponse.ExpectedResults[0].MetricsFind {
if len(res) > i || !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 {
err = fmt.Errorf("metrics find[%d] got unexpected=`%+v`", 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 {
// render
res := make([]RenderResponse, 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].Metrics) {
failures = append(failures, merry2.Errorf("unexpected amount of results, got %v, expected %v",
len(res),
len(t.ExpectedResponse.ExpectedResults[0].Metrics)))
if verbose {
for i := range t.ExpectedResponse.ExpectedResults[0].Metrics {
if len(res) > i || !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)
} else {
err = fmt.Errorf("metrics[%d] got unexpected=`%+v`", 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 {
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),
Expand All @@ -307,12 +371,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() {
Expand Down
Loading

0 comments on commit 9d39747

Please sign in to comment.