From fdd9287847e69ecddc058cdce934dd4a60cabed2 Mon Sep 17 00:00:00 2001 From: Vishal Sharma Date: Wed, 2 Feb 2022 11:40:30 +0530 Subject: [PATCH] Fix 414 errors trace filter API (#660) * fix: change trace filter APIs method from GET to POST * fix: error filter param * fix: json of aggregate params --- .../app/clickhouseReader/reader.go | 39 +- pkg/query-service/app/http_handler.go | 10 +- pkg/query-service/app/parser.go | 343 +++++------------- pkg/query-service/model/queryParams.go | 131 +++---- 4 files changed, 185 insertions(+), 338 deletions(-) diff --git a/pkg/query-service/app/clickhouseReader/reader.go b/pkg/query-service/app/clickhouseReader/reader.go index 2158408631..e99e1ada0c 100644 --- a/pkg/query-service/app/clickhouseReader/reader.go +++ b/pkg/query-service/app/clickhouseReader/reader.go @@ -1622,13 +1622,12 @@ func (r *ClickHouseReader) GetFilteredSpans(ctx context.Context, queryParams *mo query = query + " AND durationNano <= ?" args = append(args, queryParams.MaxDuration) } - if len(queryParams.Status) != 0 { - for _, e := range queryParams.Status { - if e == "error" { - query += " AND ( ( has(tags, 'error:true') OR statusCode>=500 OR statusCode=2))" - } else if e == "ok" { - query += " AND (NOT ( has(tags, 'error:true') AND statusCode<500 AND statusCode!=2))" - } + // status can only be two and if both are selected than they are equivalent to none selected + if len(queryParams.Status) == 1 { + if queryParams.Status[0] == "error" { + query += " AND ( ( has(tags, 'error:true') OR statusCode>=500 OR statusCode=2))" + } else if queryParams.Status[0] == "ok" { + query += " AND ((NOT ( has(tags, 'error:true')) AND statusCode<500 AND statusCode!=2))" } } if len(queryParams.Kind) != 0 { @@ -1776,13 +1775,12 @@ func (r *ClickHouseReader) GetTagFilters(ctx context.Context, queryParams *model query = query + " AND durationNano <= ?" args = append(args, queryParams.MaxDuration) } - if len(queryParams.Status) != 0 { - for _, e := range queryParams.Status { - if e == "error" { - query += " AND ( ( has(tags, 'error:true') OR statusCode>=500 OR statusCode=2))" - } else if e == "ok" { - query += " AND (NOT ( has(tags, 'error:true') AND statusCode<500 AND statusCode!=2))" - } + // status can only be two and if both are selected than they are equivalent to none selected + if len(queryParams.Status) == 1 { + if queryParams.Status[0] == "error" { + query += " AND ( ( has(tags, 'error:true') OR statusCode>=500 OR statusCode=2))" + } else if queryParams.Status[0] == "ok" { + query += " AND ((NOT ( has(tags, 'error:true')) AND statusCode<500 AND statusCode!=2))" } } tagFilters := []model.TagFilters{} @@ -2361,13 +2359,12 @@ func (r *ClickHouseReader) GetFilteredSpansAggregates(ctx context.Context, query query = query + " AND durationNano <= ?" args = append(args, queryParams.MaxDuration) } - if len(queryParams.Status) != 0 { - for _, e := range queryParams.Status { - if e == "error" { - query += " AND ( ( has(tags, 'error:true') OR statusCode>=500 OR statusCode=2))" - } else if e == "ok" { - query += " AND (NOT ( has(tags, 'error:true') AND statusCode<500 AND statusCode!=2))" - } + // status can only be two and if both are selected than they are equivalent to none selected + if len(queryParams.Status) == 1 { + if queryParams.Status[0] == "error" { + query += " AND ( ( has(tags, 'error:true') OR statusCode>=500 OR statusCode=2))" + } else if queryParams.Status[0] == "ok" { + query += " AND ((NOT ( has(tags, 'error:true')) AND statusCode<500 AND statusCode!=2))" } } if len(queryParams.Kind) != 0 { diff --git a/pkg/query-service/app/http_handler.go b/pkg/query-service/app/http_handler.go index e8a7ebca7f..d76e8c6ca1 100644 --- a/pkg/query-service/app/http_handler.go +++ b/pkg/query-service/app/http_handler.go @@ -209,10 +209,10 @@ func (aH *APIHandler) RegisterRoutes(router *mux.Router) { router.HandleFunc("/api/v1/userPreferences", aH.getUserPreferences).Methods(http.MethodGet) router.HandleFunc("/api/v1/version", aH.getVersion).Methods(http.MethodGet) - router.HandleFunc("/api/v1/getSpanFilters", aH.getSpanFilters).Methods(http.MethodGet) - router.HandleFunc("/api/v1/getTagFilters", aH.getTagFilters).Methods(http.MethodGet) - router.HandleFunc("/api/v1/getFilteredSpans", aH.getFilteredSpans).Methods(http.MethodGet) - router.HandleFunc("/api/v1/getFilteredSpans/aggregates", aH.getFilteredSpanAggregates).Methods(http.MethodGet) + router.HandleFunc("/api/v1/getSpanFilters", aH.getSpanFilters).Methods(http.MethodPost) + router.HandleFunc("/api/v1/getTagFilters", aH.getTagFilters).Methods(http.MethodPost) + router.HandleFunc("/api/v1/getFilteredSpans", aH.getFilteredSpans).Methods(http.MethodPost) + router.HandleFunc("/api/v1/getFilteredSpans/aggregates", aH.getFilteredSpanAggregates).Methods(http.MethodPost) router.HandleFunc("/api/v1/errors", aH.getErrors).Methods(http.MethodGet) router.HandleFunc("/api/v1/errorWithId", aH.getErrorForId).Methods(http.MethodGet) @@ -978,7 +978,7 @@ func (aH *APIHandler) searchSpans(w http.ResponseWriter, r *http.Request) { func (aH *APIHandler) getSpanFilters(w http.ResponseWriter, r *http.Request) { - query, err := parseSpanFilterRequest(r) + query, err := parseSpanFilterRequestBody(r) if aH.handleError(w, err, http.StatusBadRequest) { return } diff --git a/pkg/query-service/app/parser.go b/pkg/query-service/app/parser.go index 87c1c00303..89d1a6c07f 100644 --- a/pkg/query-service/app/parser.go +++ b/pkg/query-service/app/parser.go @@ -482,183 +482,76 @@ func parseSpanSearchRequest(r *http.Request) (*model.SpanSearchParams, error) { return params, nil } -func parseSpanFilterRequest(r *http.Request) (*model.SpanFilterParams, error) { +func parseSpanFilterRequestBody(r *http.Request) (*model.SpanFilterParams, error) { + + var postData *model.SpanFilterParams + err := json.NewDecoder(r.Body).Decode(&postData) - startTime, err := parseTime("start", r) - if err != nil { - return nil, err - } - endTime, err := parseTimeMinusBuffer("end", r) if err != nil { return nil, err } - params := &model.SpanFilterParams{ - Start: startTime, - End: endTime, - ServiceName: []string{}, - HttpRoute: []string{}, - HttpCode: []string{}, - HttpUrl: []string{}, - HttpHost: []string{}, - HttpMethod: []string{}, - Component: []string{}, - Status: []string{}, - Operation: []string{}, - GetFilters: []string{}, - Exclude: []string{}, + postData.Start, err = parseTimeStr(postData.StartStr, "start") + if err != nil { + return nil, err + } + postData.End, err = parseTimeMinusBufferStr(postData.EndStr, "end") + if err != nil { + return nil, err } - params.ServiceName = fetchArrayValues("serviceName", r) - - params.Status = fetchArrayValues("status", r) - - params.Operation = fetchArrayValues("operation", r) - - params.HttpCode = fetchArrayValues("httpCode", r) - - params.HttpUrl = fetchArrayValues("httpUrl", r) - - params.HttpHost = fetchArrayValues("httpHost", r) - - params.HttpRoute = fetchArrayValues("httpRoute", r) - - params.HttpMethod = fetchArrayValues("httpMethod", r) - - params.Component = fetchArrayValues("component", r) - - params.GetFilters = fetchArrayValues("getFilters", r) - - params.Exclude = fetchArrayValues("exclude", r) - - minDuration, err := parseTimestamp("minDuration", r) - if err == nil { - params.MinDuration = *minDuration - } - maxDuration, err := parseTimestamp("maxDuration", r) - if err == nil { - params.MaxDuration = *maxDuration - } - - return params, nil + return postData, nil } func parseFilteredSpansRequest(r *http.Request) (*model.GetFilteredSpansParams, error) { - startTime, err := parseTime("start", r) - if err != nil { - return nil, err - } - endTime, err := parseTimeMinusBuffer("end", r) + var postData *model.GetFilteredSpansParams + err := json.NewDecoder(r.Body).Decode(&postData) + if err != nil { return nil, err } - params := &model.GetFilteredSpansParams{ - Start: startTime, - End: endTime, - ServiceName: []string{}, - HttpRoute: []string{}, - HttpCode: []string{}, - HttpUrl: []string{}, - HttpHost: []string{}, - HttpMethod: []string{}, - Component: []string{}, - Status: []string{}, - Operation: []string{}, - Limit: 100, - Order: "descending", - Exclude: []string{}, - } - - params.ServiceName = fetchArrayValues("serviceName", r) - - params.Status = fetchArrayValues("status", r) - - params.Operation = fetchArrayValues("operation", r) - - params.HttpCode = fetchArrayValues("httpCode", r) - - params.HttpUrl = fetchArrayValues("httpUrl", r) - - params.HttpHost = fetchArrayValues("httpHost", r) - - params.HttpRoute = fetchArrayValues("httpRoute", r) - - params.HttpMethod = fetchArrayValues("httpMethod", r) - - params.Component = fetchArrayValues("component", r) - - params.Exclude = fetchArrayValues("exclude", r) - - limitStr := r.URL.Query().Get("limit") - if len(limitStr) != 0 { - limit, err := strconv.ParseInt(limitStr, 10, 64) - if err != nil { - return nil, errors.New("Limit param is not in correct format") - } - params.Limit = limit - } else { - params.Limit = 100 - } - - offsetStr := r.URL.Query().Get("offset") - if len(offsetStr) != 0 { - offset, err := strconv.ParseInt(offsetStr, 10, 64) - if err != nil { - return nil, errors.New("Offset param is not in correct format") - } - params.Offset = offset - } - - tags, err := parseTagsV2("tags", r) + postData.Start, err = parseTimeStr(postData.StartStr, "start") if err != nil { return nil, err } - if len(*tags) != 0 { - params.Tags = *tags + postData.End, err = parseTimeMinusBufferStr(postData.EndStr, "end") + if err != nil { + return nil, err } - minDuration, err := parseTimestamp("minDuration", r) - if err == nil { - params.MinDuration = *minDuration - } - maxDuration, err := parseTimestamp("maxDuration", r) - if err == nil { - params.MaxDuration = *maxDuration + if postData.Limit == 0 { + postData.Limit = 100 } - kind := r.URL.Query().Get("kind") - if len(kind) != 0 { - params.Kind = kind - } - - return params, nil + return postData, nil } func parseFilteredSpanAggregatesRequest(r *http.Request) (*model.GetFilteredSpanAggregatesParams, error) { - startTime, err := parseTime("start", r) + var postData *model.GetFilteredSpanAggregatesParams + err := json.NewDecoder(r.Body).Decode(&postData) + if err != nil { return nil, err } - endTime, err := parseTimeMinusBuffer("end", r) + postData.Start, err = parseTimeStr(postData.StartStr, "start") + if err != nil { + return nil, err + } + postData.End, err = parseTimeMinusBufferStr(postData.EndStr, "end") if err != nil { return nil, err } - stepStr := r.URL.Query().Get("step") - if len(stepStr) == 0 { + step := postData.StepSeconds + if step == 0 { return nil, errors.New("step param missing in query") } - stepInt, err := strconv.Atoi(stepStr) - if err != nil { - return nil, errors.New("step param is not in correct format") - } - - function := r.URL.Query().Get("function") + function := postData.Function if len(function) == 0 { return nil, errors.New("function param missing in query") } else { @@ -702,71 +595,17 @@ func parseFilteredSpanAggregatesRequest(r *http.Request) (*model.GetFilteredSpan aggregationOption = "max" } - params := &model.GetFilteredSpanAggregatesParams{ - Start: startTime, - End: endTime, - ServiceName: []string{}, - HttpRoute: []string{}, - HttpCode: []string{}, - HttpUrl: []string{}, - HttpHost: []string{}, - HttpMethod: []string{}, - Component: []string{}, - Status: []string{}, - Operation: []string{}, - StepSeconds: stepInt, - Dimension: dimension, - AggregationOption: aggregationOption, - Exclude: []string{}, - } + postData.AggregationOption = aggregationOption + postData.Dimension = dimension + // tags, err := parseTagsV2("tags", r) + // if err != nil { + // return nil, err + // } + // if len(*tags) != 0 { + // params.Tags = *tags + // } - params.ServiceName = fetchArrayValues("serviceName", r) - - params.Status = fetchArrayValues("status", r) - - params.Operation = fetchArrayValues("operation", r) - - params.HttpCode = fetchArrayValues("httpCode", r) - - params.HttpUrl = fetchArrayValues("httpUrl", r) - - params.HttpHost = fetchArrayValues("httpHost", r) - - params.HttpRoute = fetchArrayValues("httpRoute", r) - - params.HttpMethod = fetchArrayValues("httpMethod", r) - - params.Component = fetchArrayValues("component", r) - - params.Exclude = fetchArrayValues("exclude", r) - - tags, err := parseTagsV2("tags", r) - if err != nil { - return nil, err - } - if len(*tags) != 0 { - params.Tags = *tags - } - - minDuration, err := parseTimestamp("minDuration", r) - if err == nil { - params.MinDuration = *minDuration - } - maxDuration, err := parseTimestamp("maxDuration", r) - if err == nil { - params.MaxDuration = *maxDuration - } - - kind := r.URL.Query().Get("kind") - if len(kind) != 0 { - params.Kind = kind - } - groupBy := r.URL.Query().Get("groupBy") - if len(groupBy) != 0 { - params.GroupBy = groupBy - } - - return params, nil + return postData, nil } func parseErrorRequest(r *http.Request) (*model.GetErrorParams, error) { @@ -792,61 +631,24 @@ func parseErrorRequest(r *http.Request) (*model.GetErrorParams, error) { } func parseTagFilterRequest(r *http.Request) (*model.TagFilterParams, error) { + var postData *model.TagFilterParams + err := json.NewDecoder(r.Body).Decode(&postData) - startTime, err := parseTime("start", r) - if err != nil { - return nil, err - } - endTime, err := parseTimeMinusBuffer("end", r) if err != nil { return nil, err } - params := &model.TagFilterParams{ - Start: startTime, - End: endTime, - ServiceName: []string{}, - HttpRoute: []string{}, - HttpCode: []string{}, - HttpUrl: []string{}, - HttpHost: []string{}, - HttpMethod: []string{}, - Component: []string{}, - Status: []string{}, - Operation: []string{}, - Exclude: []string{}, + postData.Start, err = parseTimeStr(postData.StartStr, "start") + if err != nil { + return nil, err + } + postData.End, err = parseTimeMinusBufferStr(postData.EndStr, "end") + if err != nil { + return nil, err } - params.ServiceName = fetchArrayValues("serviceName", r) + return postData, nil - params.Status = fetchArrayValues("status", r) - - params.Operation = fetchArrayValues("operation", r) - - params.HttpCode = fetchArrayValues("httpCode", r) - - params.HttpUrl = fetchArrayValues("httpUrl", r) - - params.HttpHost = fetchArrayValues("httpHost", r) - - params.HttpRoute = fetchArrayValues("httpRoute", r) - - params.HttpMethod = fetchArrayValues("httpMethod", r) - - params.Component = fetchArrayValues("component", r) - - params.Exclude = fetchArrayValues("exclude", r) - - minDuration, err := parseTimestamp("minDuration", r) - if err == nil { - params.MinDuration = *minDuration - } - maxDuration, err := parseTimestamp("maxDuration", r) - if err == nil { - params.MaxDuration = *maxDuration - } - - return params, nil } func parseErrorsRequest(r *http.Request) (*model.GetErrorsParams, error) { @@ -957,6 +759,45 @@ func parseApplicationPercentileRequest(r *http.Request) (*model.ApplicationPerce } +func parseTimeStr(timeStr string, param string) (*time.Time, error) { + + if len(timeStr) == 0 { + return nil, fmt.Errorf("%s param missing in query", param) + } + + timeUnix, err := strconv.ParseInt(timeStr, 10, 64) + if err != nil || len(timeStr) == 0 { + return nil, fmt.Errorf("%s param is not in correct timestamp format", param) + } + + timeFmt := time.Unix(0, timeUnix) + + return &timeFmt, nil + +} + +func parseTimeMinusBufferStr(timeStr string, param string) (*time.Time, error) { + + if len(timeStr) == 0 { + return nil, fmt.Errorf("%s param missing in query", param) + } + + timeUnix, err := strconv.ParseInt(timeStr, 10, 64) + if err != nil || len(timeStr) == 0 { + return nil, fmt.Errorf("%s param is not in correct timestamp format", param) + } + + timeUnixNow := time.Now().UnixNano() + if timeUnix > timeUnixNow-30000000000 { + timeUnix = timeUnix - 30000000000 + } + + timeFmt := time.Unix(0, timeUnix) + + return &timeFmt, nil + +} + func parseTime(param string, r *http.Request) (*time.Time, error) { timeStr := r.URL.Query().Get(param) diff --git a/pkg/query-service/model/queryParams.go b/pkg/query-service/model/queryParams.go index 2ae9411b4e..8ceaddd9a6 100644 --- a/pkg/query-service/model/queryParams.go +++ b/pkg/query-service/model/queryParams.go @@ -117,85 +117,94 @@ type SpanSearchParams struct { } type GetFilteredSpansParams struct { - ServiceName []string - Operation []string - Kind string - Status []string - HttpRoute []string - HttpCode []string - HttpUrl []string - HttpHost []string - HttpMethod []string - Component []string + ServiceName []string `json:"serviceName"` + Operation []string `json:"operation"` + Kind string `json:"kind"` + Status []string `json:"status"` + HttpRoute []string `json:"httpRoute"` + HttpCode []string `json:"httpCode"` + HttpUrl []string `json:"httpUrl"` + HttpHost []string `json:"httpHost"` + HttpMethod []string `json:"httpMethod"` + Component []string `json:"component"` + StartStr string `json:"start"` + EndStr string `json:"end"` + MinDuration string `json:"minDuration"` + MaxDuration string `json:"maxDuration"` + Limit int64 `json:"limit"` + Order string `json:"order"` + Offset int64 `json:"offset"` + Tags []TagQueryV2 `json:"tags"` + Exclude []string `json:"exclude"` Start *time.Time End *time.Time - MinDuration string - MaxDuration string - Limit int64 - Order string - Offset int64 - Tags []TagQueryV2 - Exclude []string } type GetFilteredSpanAggregatesParams struct { - ServiceName []string - Operation []string - Kind string - Status []string - HttpRoute []string - HttpCode []string - HttpUrl []string - HttpHost []string - HttpMethod []string - Component []string - MinDuration string - MaxDuration string - Tags []TagQueryV2 + ServiceName []string `json:"serviceName"` + Operation []string `json:"operation"` + Kind string `json:"kind"` + Status []string `json:"status"` + HttpRoute []string `json:"httpRoute"` + HttpCode []string `json:"httpCode"` + HttpUrl []string `json:"httpUrl"` + HttpHost []string `json:"httpHost"` + HttpMethod []string `json:"httpMethod"` + Component []string `json:"component"` + MinDuration string `json:"minDuration"` + MaxDuration string `json:"maxDuration"` + Tags []TagQueryV2 `json:"tags"` + StartStr string `json:"start"` + EndStr string `json:"end"` + StepSeconds int `json:"step"` + Dimension string `json:"dimension"` + AggregationOption string `json:"aggregationOption"` + GroupBy string `json:"groupBy"` + Function string `json:"function"` + Exclude []string `json:"exclude"` Start *time.Time End *time.Time - StepSeconds int - Dimension string - AggregationOption string - GroupBy string - Function string - Exclude []string } type SpanFilterParams struct { - Status []string - ServiceName []string - HttpRoute []string - HttpCode []string - HttpUrl []string - HttpHost []string - HttpMethod []string - Component []string - Operation []string - GetFilters []string - Exclude []string - MinDuration string - MaxDuration string + Status []string `json:"status"` + ServiceName []string `json:"serviceName"` + HttpRoute []string `json:"httpRoute"` + HttpCode []string `json:"httpCode"` + HttpUrl []string `json:"httpUrl"` + HttpHost []string `json:"httpHost"` + HttpMethod []string `json:"httpMethod"` + Component []string `json:"component"` + Operation []string `json:"operation"` + GetFilters []string `json:"getFilters"` + Exclude []string `json:"exclude"` + MinDuration string `json:"minDuration"` + MaxDuration string `json:"maxDuration"` + StartStr string `json:"start"` + EndStr string `json:"end"` Start *time.Time End *time.Time } type TagFilterParams struct { - Status []string - ServiceName []string - HttpRoute []string - HttpCode []string - HttpUrl []string - HttpHost []string - HttpMethod []string - Component []string - Operation []string - Exclude []string - MinDuration string - MaxDuration string + Status []string `json:"status"` + ServiceName []string `json:"serviceName"` + HttpRoute []string `json:"httpRoute"` + HttpCode []string `json:"httpCode"` + HttpUrl []string `json:"httpUrl"` + HttpHost []string `json:"httpHost"` + HttpMethod []string `json:"httpMethod"` + Component []string `json:"component"` + Operation []string `json:"operation"` + Exclude []string `json:"exclude"` + MinDuration string `json:"minDuration"` + MaxDuration string `json:"maxDuration"` + StartStr string `json:"start"` + EndStr string `json:"end"` Start *time.Time End *time.Time } + type TTLParams struct { Type string Duration string