From cf06aa72bacc46b2b46c86a819531ea341f8fe3b Mon Sep 17 00:00:00 2001 From: Vladimir Smirnov Date: Wed, 19 Aug 2020 13:48:01 +0200 Subject: [PATCH] percentileOfSeries: return first of filtered datapoints in case only one series have valid data percentileOfSeries should return first of filtered datapoints and not first of all datapoints in case all other series have NaNs for the values. Fixes #516 --- CHANGES.md | 1 + cmd/mockbackend/e2etesting.go | 9 +++++ cmd/mockbackend/testcases/i516/i516.yaml | 36 +++++++++++++++++++ e2e_test.sh | 4 +++ expr/consolidations/consolidations.go | 2 +- .../percentileOfSeries/function_test.go | 13 +++++++ 6 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 cmd/mockbackend/testcases/i516/i516.yaml diff --git a/CHANGES.md b/CHANGES.md index cde0f84b7..f605356de 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -29,6 +29,7 @@ CHANGELOG - [Fix] prometheus backend: carbonapi should send 'end' instead of 'stop' in queries (thx to Alexandre Vincent) - [Fix] change metrics resulting tags to match graphite-web in some cases (thx to @Felixoid) - [Fix] Prometheus backend: trust timestamps from the backend (Fixes #504, Fixes #514) + - [Fix] percentileOfSeries: return first of filtered datapoints in case only one series have valid data (Fixes #516) - [Code] merge aliasByNode and aliasByTags (thx to @Felixoid) **0.14.0** diff --git a/cmd/mockbackend/e2etesting.go b/cmd/mockbackend/e2etesting.go index 18ebaa584..1eea5e157 100644 --- a/cmd/mockbackend/e2etesting.go +++ b/cmd/mockbackend/e2etesting.go @@ -8,6 +8,7 @@ import ( "fmt" "io" "io/ioutil" + "math" "net/http" "net/url" "strconv" @@ -74,6 +75,10 @@ func (d *Datapoint) UnmarshalJSON(data []byte) error { return fmt.Errorf("failed to parse Timestamp: %v", err) } + if valueStr == "null" || valueStr == "\"null\"" { + d.Value = math.NaN() + return nil + } d.Value, err = strconv.ParseFloat(valueStr, 64) if err != nil { return fmt.Errorf("failed to parse Value: %v", err) @@ -101,6 +106,10 @@ func (d *Datapoint) UnmarshalYAML(unmarshal func(interface{}) error) error { return fmt.Errorf("failed to parse Timestamp: %v", err) } + if valueStr == "null" || valueStr == "\"null\"" { + d.Value = math.NaN() + return nil + } d.Value, err = strconv.ParseFloat(valueStr, 64) if err != nil { return fmt.Errorf("failed to parse Value: %v", err) diff --git a/cmd/mockbackend/testcases/i516/i516.yaml b/cmd/mockbackend/testcases/i516/i516.yaml new file mode 100644 index 000000000..c717441e9 --- /dev/null +++ b/cmd/mockbackend/testcases/i516/i516.yaml @@ -0,0 +1,36 @@ +version: "v1" +test: + apps: + - name: "carbonapi" + binary: "./carbonapi" + args: + - "-config" + - "./cmd/mockbackend/carbonapi_singlebackend.yaml" + queries: + - endpoint: "http://127.0.0.1:8081" + delay: 1 + type: "GET" + URL: "/render?format=json&target=percentileOfSeries(carbon.api.*.cache_size, 95, false)" + expectedResponse: + httpCode: 200 + contentType: "application/json" + expectedResults: + - metrics: + - target: "percentileOfSeries(carbon.api.*.cache_size, 95, false)" + datapoints: [[0.0, 1],[0.0, 2],[0.0, 3],[100500.0, 4],[100500.0, 5],[100500.0, 6]] +listeners: + - address: ":9070" + expressions: + "carbon.api.*.cache_size": + pathExpression: "carbon.api.*.cache_size" + data: + - metricName: "carbon.api.host001.cache_size" + values: [.NaN, .NaN, .NaN, .NaN, .NaN, .NaN] + - metricName: "carbon.api.host002.cache_size" + values: [.NaN, .NaN, .NaN, .NaN, .NaN, .NaN] + - metricName: "carbon.api.host003.cache_size" + values: [0.0, 0.0, 0.0, 100500.0, 100500.0, 100500.0] + - metricName: "carbon.api.host004.cache_size" + values: [.NaN, .NaN, .NaN, .NaN, .NaN, .NaN] + - metricName: "carbon.api.host005.cache_size" + values: [.NaN, .NaN, .NaN, .NaN, .NaN, .NaN] diff --git a/e2e_test.sh b/e2e_test.sh index 49d07d7db..c2cd010de 100755 --- a/e2e_test.sh +++ b/e2e_test.sh @@ -2,6 +2,10 @@ TESTS=$(ls cmd/mockbackend/testcases/) +if [[ ! -z "${1}" ]]; then + TESTS="${1}" +fi + FAILED_TESTS="" for t in ${TESTS}; do if [[ "${TRAVIS}" == "true" ]]; then diff --git a/expr/consolidations/consolidations.go b/expr/consolidations/consolidations.go index 65dca02e7..d5f1f5869 100644 --- a/expr/consolidations/consolidations.go +++ b/expr/consolidations/consolidations.go @@ -79,7 +79,7 @@ func Percentile(data []float64, percent float64, interpolate bool) float64 { return math.NaN() } if len(dataFiltered) == 1 { - return data[0] + return dataFiltered[0] } k := (float64(len(dataFiltered)-1) * percent) / 100 diff --git a/expr/functions/percentileOfSeries/function_test.go b/expr/functions/percentileOfSeries/function_test.go index 87044d97e..77d7765cc 100644 --- a/expr/functions/percentileOfSeries/function_test.go +++ b/expr/functions/percentileOfSeries/function_test.go @@ -57,6 +57,19 @@ func TestPercentileOfSeriesSeries(t *testing.T) { }, []*types.MetricData{types.MakeMetricData("percentileOfSeries(metric1.foo.*.*,50,interpolate=true)", []float64{6.5, 7.5, 8.5, 9.5, 11, math.NaN()}, 1, now32)}, }, + { + `percentileOfSeries(metric1.foo.*.*,95,false)`, + map[parser.MetricRequest][]*types.MetricData{ + {"metric1.foo.*.*", 0, 1}: { + types.MakeMetricData("metric1.foo.bar1.qux", []float64{math.NaN(), math.NaN(), math.NaN(), math.NaN(), math.NaN(), math.NaN()}, 1, now32), + types.MakeMetricData("metric1.foo.bar2.qux", []float64{math.NaN(), math.NaN(), math.NaN(), math.NaN(), math.NaN(), 0}, 1, now32), + types.MakeMetricData("metric1.foo.bar3.qux", []float64{0, 0, 0, 100500, 100501, 1005002}, 1, now32), + types.MakeMetricData("metric1.foo.bar4.qux", []float64{math.NaN(), math.NaN(), math.NaN(), math.NaN(), math.NaN(), 0}, 1, now32), + types.MakeMetricData("metric1.foo.bar5.qux", []float64{math.NaN(), math.NaN(), math.NaN(), math.NaN(), math.NaN(), 0}, 1, now32), + }, + }, + []*types.MetricData{types.MakeMetricData("percentileOfSeries(metric1.foo.*.*,95,false)", []float64{0, 0, 0, 100500, 100501, 1005002}, 1, now32)}, + }, } for _, tt := range tests {