From dd2edd7814db88be82e17bd991d721810f1293f1 Mon Sep 17 00:00:00 2001 From: oleiade Date: Tue, 16 Nov 2021 19:59:57 +0100 Subject: [PATCH 1/2] Add threshold parsing functions to the stats package In this commit we use the parser combinator library introduced in the previous commit to build a threshold expression parser. We match the existing supported expression format, without any modifications or additions. --- stats/thresholds_parser.go | 208 ++++++++++++++++++ stats/thresholds_parser_test.go | 361 ++++++++++++++++++++++++++++++++ 2 files changed, 569 insertions(+) create mode 100644 stats/thresholds_parser.go create mode 100644 stats/thresholds_parser_test.go diff --git a/stats/thresholds_parser.go b/stats/thresholds_parser.go new file mode 100644 index 00000000000..72b0f82e2a7 --- /dev/null +++ b/stats/thresholds_parser.go @@ -0,0 +1,208 @@ +/* + * + * k6 - a next-generation load testing tool + * Copyright (C) 2021 Load Impact + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +package stats + +import ( + "fmt" + "strconv" + "strings" + + "gopkg.in/guregu/null.v3" +) + +// thresholdExpression holds the parsed result of a threshold expression, +// as described in: https://k6.io/docs/using-k6/thresholds/#threshold-syntax +type thresholdExpression struct { + // AggregationMethod holds the aggregation method parsed + // from the threshold expression. Possible values are described + // by `aggregationMethodTokens`. + AggregationMethod string + + // AggregationValue will hold the aggregation method's pivot value + // in the event it is a percentile. For instance: an expression of the form p(99.9) < 200, + // would result in AggregationValue to be set to 99.9. + AggregationValue null.Float + + // Operator holds the operator parsed from the threshold expression. + // Possible values are described by `operatorTokens`. + Operator string + + // Value holds the value parsed from the threshold expression. + Value float64 +} + +// parseThresholdAssertion parses a threshold condition expression, +// as defined in a JS script (for instance p(95)<1000), into a thresholdExpression +// instance. +// +// It is expected to be of the form: `aggregation_method operator value`. +// As defined by the following BNF: +// ``` +// assertion -> aggregation_method whitespace* operator whitespace* float +// aggregation_method -> trend | rate | gauge | counter +// counter -> "count" | "rate" +// gauge -> "value" +// rate -> "rate" +// trend -> "avg" | "min" | "max" | "med" | percentile +// percentile -> "p(" float ")" +// operator -> ">" | ">=" | "<=" | "<" | "==" | "===" | "!=" +// float -> digit+ ("." digit+)? +// digit -> "0" | "1" | "2" | "3" | "4" | "5" | "6" | "7" | "8" | "9" +// whitespace -> " " +// ``` +func parseThresholdExpression(input string) (*thresholdExpression, error) { + // Scanning makes no assumption on the underlying values, and only + // checks that the expression has the right format. + method, operator, value, err := scanThresholdExpression(input) + if err != nil { + return nil, fmt.Errorf("failed parsing threshold expression; reason: %w", err) + } + + parsedMethod, parsedMethodValue, err := parseThresholdAggregationMethod(method) + if err != nil { + return nil, fmt.Errorf("failed parsing threshold expression's left hand side; reason: %w", err) + } + + parsedValue, err := strconv.ParseFloat(value, 64) + if err != nil { + return nil, fmt.Errorf("failed parsing threshold expresion's right hand side; reason: %w", err) + } + + condition := &thresholdExpression{ + AggregationMethod: parsedMethod, + AggregationValue: parsedMethodValue, + Operator: operator, + Value: parsedValue, + } + + return condition, nil +} + +// Define accepted threshold expression operators tokens +const ( + tokenLessEqual = "<=" + tokenLess = "<" + tokenGreaterEqual = ">=" + tokenGreater = ">" + tokenStrictlyEqual = "===" + tokenLooselyEqual = "==" + tokenBangEqual = "!=" +) + +// operatorTokens defines the list of operator-related tokens +// used in threshold expressions parsing. +// +// It is meant to be used during the scan of threshold expressions. +// Although declared as a `var`, being an array, it is effectively +// immutable and can be considered constant. +// +// Note that because scanning uses a substring parser, and will match +// the smallest common substring, the actual slice order matters. +// Longer tokens with symbols in common with shorter ones must appear +// first in the list in order to be effectively matched. +var operatorTokens = [7]string{ // nolint:gochecknoglobals + tokenLessEqual, + tokenLess, + tokenGreaterEqual, + tokenGreater, + tokenStrictlyEqual, + tokenLooselyEqual, + tokenBangEqual, +} + +// scanThresholdExpression scans a threshold condition expression of the +// form: `aggregation_method operator value`. An invalid or unknown operator +// will produce an error. However, no assertions regarding +// either the left-hand side aggregation method nor the right-hand +// side value will be made: they will be returned as is, only trimmed from +// their spaces. +func scanThresholdExpression(input string) (string, string, string, error) { + for _, op := range operatorTokens { + substrings := strings.SplitN(input, op, 2) + if len(substrings) == 2 { + return strings.TrimSpace(substrings[0]), op, strings.TrimSpace(substrings[1]), nil + } + } + + return "", "", "", fmt.Errorf("malformed threshold expression") +} + +// Define accepted threshold expression aggregation tokens +// Percentile token `p(..)` is accepted too but handled separately. +const ( + tokenValue = "value" + tokenCount = "count" + tokenRate = "rate" + tokenAvg = "avg" + tokenMin = "min" + tokenMed = "med" + tokenMax = "max" + tokenPercentile = "p" +) + +// aggregationMethodTokens defines the list of aggregation method +// used in the parsing of threshold expressions. +// +// It is meant to be used during the parsing of threshold expressions. +// Although declared as a `var`, being an array, it is effectively +// immutable and can be considered constant. +var aggregationMethodTokens = [8]string{ // nolint:gochecknoglobals + tokenValue, + tokenCount, + tokenRate, + tokenAvg, + tokenMin, + tokenMed, + tokenMax, + tokenPercentile, +} + +// parseThresholdMethod will parse a threshold condition expression's method. +// It assumes the provided input argument is already trimmed and cleaned up. +// If it encounters a percentile method, it will parse it and verify it +// boils down to an expression of the form: `p(float64)`, but will return +// it verbatim, as a string. +func parseThresholdAggregationMethod(input string) (string, null.Float, error) { + // Is the input one of the methods keywords? + for _, m := range aggregationMethodTokens { + // Percentile expressions being of the form p(value), + // they won't be matched here. + if m == input { + return input, null.Float{}, nil + } + } + + // Otherwise, attempt to parse a percentile expression + if strings.HasPrefix(input, tokenPercentile+"(") && strings.HasSuffix(input, ")") { + aggregationValue, err := strconv.ParseFloat(trimDelimited("p(", input, ")"), 64) + if err != nil { + return "", null.Float{}, fmt.Errorf("malformed percentile value; reason: %w", err) + } + + return input, null.FloatFrom(aggregationValue), nil + } + + return "", null.Float{}, fmt.Errorf("failed parsing method from expression") +} + +func trimDelimited(prefix, input, suffix string) string { + return strings.TrimSuffix(strings.TrimPrefix(input, prefix), suffix) +} diff --git a/stats/thresholds_parser_test.go b/stats/thresholds_parser_test.go new file mode 100644 index 00000000000..25120684d6e --- /dev/null +++ b/stats/thresholds_parser_test.go @@ -0,0 +1,361 @@ +/* + * + * k6 - a next-generation load testing tool + * Copyright (C) 2021 Load Impact + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +package stats + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "gopkg.in/guregu/null.v3" +) + +func TestParseThresholdExpression(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input string + wantExpression *thresholdExpression + wantErr bool + }{ + { + name: "unknown expression's operator fails", + input: "count!20", + wantExpression: nil, + wantErr: true, + }, + { + name: "unknown expression's method fails", + input: "foo>20", + wantExpression: nil, + wantErr: true, + }, + { + name: "non numerical expression's value fails", + input: "count>abc", + wantExpression: nil, + wantErr: true, + }, + { + name: "valid threshold expression syntax", + input: "count>20", + wantExpression: &thresholdExpression{AggregationMethod: "count", Operator: ">", Value: 20}, + wantErr: false, + }, + } + for _, testCase := range tests { + testCase := testCase + + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + gotExpression, gotErr := parseThresholdExpression(testCase.input) + + assert.Equal(t, + testCase.wantErr, + gotErr != nil, + "parseThresholdExpression() error = %v, wantErr %v", gotErr, testCase.wantErr, + ) + + assert.Equal(t, + testCase.wantExpression, + gotExpression, + "parseThresholdExpression() gotExpression = %v, want %v", gotExpression, testCase.wantExpression, + ) + }) + } +} + +func BenchmarkParseThresholdExpression(b *testing.B) { + for i := 0; i < b.N; i++ { + parseThresholdExpression("count>20") // nolint + } +} + +func TestParseThresholdAggregationMethod(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input string + wantMethod string + wantMethodValue null.Float + wantErr bool + }{ + { + name: "count method is parsed", + input: "count", + wantMethod: "count", + wantMethodValue: null.Float{}, + wantErr: false, + }, + { + name: "rate method is parsed", + input: "rate", + wantMethod: "rate", + wantMethodValue: null.Float{}, + wantErr: false, + }, + { + name: "value method is parsed", + input: "value", + wantMethod: "value", + wantMethodValue: null.Float{}, + wantErr: false, + }, + { + name: "avg method is parsed", + input: "avg", + wantMethod: "avg", + wantMethodValue: null.Float{}, + wantErr: false, + }, + { + name: "min method is parsed", + input: "min", + wantMethod: "min", + wantMethodValue: null.Float{}, + wantErr: false, + }, + { + name: "max method is parsed", + input: "max", + wantMethod: "max", + wantMethodValue: null.Float{}, + wantErr: false, + }, + { + name: "med method is parsed", + input: "med", + wantMethod: "med", + wantMethodValue: null.Float{}, + wantErr: false, + }, + { + name: "percentile method with integer value is parsed", + input: "p(99)", + wantMethod: "p(99)", + wantMethodValue: null.FloatFrom(99), + wantErr: false, + }, + { + name: "percentile method with floating point value is parsed", + input: "p(99.9)", + wantMethod: "p(99.9)", + wantMethodValue: null.FloatFrom(99.9), + wantErr: false, + }, + { + name: "parsing invalid method fails", + input: "foo", + wantMethod: "", + wantMethodValue: null.Float{}, + wantErr: true, + }, + { + name: "parsing empty percentile expression fails", + input: "p()", + wantMethod: "", + wantMethodValue: null.Float{}, + wantErr: true, + }, + { + name: "parsing incomplete percentile expression fails", + input: "p(99", + wantMethod: "", + wantMethodValue: null.Float{}, + wantErr: true, + }, + { + name: "parsing non-numerical percentile value fails", + input: "p(foo)", + wantMethod: "", + wantMethodValue: null.Float{}, + wantErr: true, + }, + } + for _, testCase := range tests { + testCase := testCase + + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + gotMethod, gotMethodValue, gotErr := parseThresholdAggregationMethod(testCase.input) + + assert.Equal(t, + testCase.wantErr, + gotErr != nil, + "parseThresholdAggregationMethod() error = %v, wantErr %v", gotErr, testCase.wantErr, + ) + + assert.Equal(t, + testCase.wantMethod, + gotMethod, + "parseThresholdAggregationMethod() gotMethod = %v, want %v", gotMethod, testCase.wantMethod, + ) + + assert.Equal(t, + testCase.wantMethodValue, + gotMethodValue, + "parseThresholdAggregationMethod() gotMethodValue = %v, want %v", gotMethodValue, testCase.wantMethodValue, + ) + }) + } +} + +func BenchmarkParseThresholdAggregationMethod(b *testing.B) { + for i := 0; i < b.N; i++ { + parseThresholdAggregationMethod("p(99.9)") // nolint + } +} + +func TestScanThresholdExpression(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input string + wantMethod string + wantOperator string + wantValue string + wantErr bool + }{ + { + name: "expression with <= operator is scanned", + input: "foo<=bar", + wantMethod: "foo", + wantOperator: "<=", + wantValue: "bar", + wantErr: false, + }, + { + name: "expression with < operator is scanned", + input: "foo= operator is scanned", + input: "foo>=bar", + wantMethod: "foo", + wantOperator: ">=", + wantValue: "bar", + wantErr: false, + }, + { + name: "expression with > operator is scanned", + input: "foo>bar", + wantMethod: "foo", + wantOperator: ">", + wantValue: "bar", + wantErr: false, + }, + { + name: "expression with === operator is scanned", + input: "foo===bar", + wantMethod: "foo", + wantOperator: "===", + wantValue: "bar", + wantErr: false, + }, + { + name: "expression with == operator is scanned", + input: "foo==bar", + wantMethod: "foo", + wantOperator: "==", + wantValue: "bar", + wantErr: false, + }, + { + name: "expression with != operator is scanned", + input: "foo!=bar", + wantMethod: "foo", + wantOperator: "!=", + wantValue: "bar", + wantErr: false, + }, + { + name: "expression's method is trimmed", + input: " foo <=bar", + wantMethod: "foo", + wantOperator: "<=", + wantValue: "bar", + wantErr: false, + }, + { + name: "expression's value is trimmed", + input: "foo<= bar ", + wantMethod: "foo", + wantOperator: "<=", + wantValue: "bar", + wantErr: false, + }, + { + name: "expression with unknown operator fails", + input: "foo!bar", + wantMethod: "", + wantOperator: "", + wantValue: "", + wantErr: true, + }, + } + for _, testCase := range tests { + testCase := testCase + + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + gotMethod, gotOperator, gotValue, gotErr := scanThresholdExpression(testCase.input) + + assert.Equal(t, + testCase.wantErr, + gotErr != nil, + "scanThresholdExpression() error = %v, wantErr %v", gotErr, testCase.wantErr, + ) + + assert.Equal(t, + testCase.wantMethod, + gotMethod, + "scanThresholdExpression() gotMethod = %v, want %v", gotMethod, testCase.wantMethod, + ) + + assert.Equal(t, + testCase.wantOperator, + gotOperator, + "scanThresholdExpression() gotOperator = %v, want %v", gotOperator, testCase.wantOperator, + ) + + assert.Equal(t, + testCase.wantValue, + gotValue, + "scanThresholdExpression() gotValue = %v, want %v", gotValue, testCase.wantValue, + ) + }) + } +} + +func BenchmarkScanThresholdExpression(b *testing.B) { + for i := 0; i < b.N; i++ { + scanThresholdExpression("foo<=bar") // nolint + } +} From 81cccd996555d146a9323a4adfb6928ddfcabf9d Mon Sep 17 00:00:00 2001 From: oleiade Date: Wed, 17 Nov 2021 09:07:33 +0100 Subject: [PATCH 2/2] Remove the JS runtime from threshold calculations In this commit we replace the previously existing thresholds condition evaluation, which was depending on Goja's Runtime, with a new pure-Go one. Thresholds are now parsed, and evaluated in o, and no JS rutimes are involved in the process anymore. It is built upong the thresholds parser, and parser combinators library introduced in previous commits. --- core/engine_test.go | 29 +-- stats/thresholds.go | 181 ++++++++++------- stats/thresholds_test.go | 411 +++++++++++++++++++++++++++++---------- 3 files changed, 441 insertions(+), 180 deletions(-) diff --git a/core/engine_test.go b/core/engine_test.go index c9c60166029..7a4dc20d095 100644 --- a/core/engine_test.go +++ b/core/engine_test.go @@ -258,7 +258,7 @@ func TestEngine_processSamples(t *testing.T) { }) t.Run("submetric", func(t *testing.T) { t.Parallel() - ths, err := stats.NewThresholds([]string{`1+1==2`}) + ths, err := stats.NewThresholds([]string{`value<2`}) assert.NoError(t, err) e, _, wait := newTestEngine(t, nil, nil, nil, lib.Options{ @@ -286,7 +286,10 @@ func TestEngineThresholdsWillAbort(t *testing.T) { t.Parallel() metric := stats.New("my_metric", stats.Gauge) - ths, err := stats.NewThresholds([]string{"1+1==3"}) + // The incoming samples for the metric set it to 1.25. Considering + // the metric is of type Gauge, value > 1.25 should always fail, and + // trigger an abort. + ths, err := stats.NewThresholds([]string{"value>1.25"}) assert.NoError(t, err) ths.Thresholds[0].AbortOnFail = true @@ -305,7 +308,11 @@ func TestEngineAbortedByThresholds(t *testing.T) { t.Parallel() metric := stats.New("my_metric", stats.Gauge) - ths, err := stats.NewThresholds([]string{"1+1==3"}) + // The MiniRunner sets the value of the metric to 1.25. Considering + // the metric is of type Gauge, value > 1.25 should always fail, and + // trigger an abort. + // **N.B**: a threshold returning an error, won't trigger an abort. + ths, err := stats.NewThresholds([]string{"value>1.25"}) assert.NoError(t, err) ths.Thresholds[0].AbortOnFail = true @@ -343,14 +350,14 @@ func TestEngine_processThresholds(t *testing.T) { ths map[string][]string abort bool }{ - "passing": {true, map[string][]string{"my_metric": {"1+1==2"}}, false}, - "failing": {false, map[string][]string{"my_metric": {"1+1==3"}}, false}, - "aborting": {false, map[string][]string{"my_metric": {"1+1==3"}}, true}, - - "submetric,match,passing": {true, map[string][]string{"my_metric{a:1}": {"1+1==2"}}, false}, - "submetric,match,failing": {false, map[string][]string{"my_metric{a:1}": {"1+1==3"}}, false}, - "submetric,nomatch,passing": {true, map[string][]string{"my_metric{a:2}": {"1+1==2"}}, false}, - "submetric,nomatch,failing": {true, map[string][]string{"my_metric{a:2}": {"1+1==3"}}, false}, + "passing": {true, map[string][]string{"my_metric": {"value<2"}}, false}, + "failing": {false, map[string][]string{"my_metric": {"value>1.25"}}, false}, + "aborting": {false, map[string][]string{"my_metric": {"value>1.25"}}, true}, + + "submetric,match,passing": {true, map[string][]string{"my_metric{a:1}": {"value<2"}}, false}, + "submetric,match,failing": {false, map[string][]string{"my_metric{a:1}": {"value>1.25"}}, false}, + "submetric,nomatch,passing": {true, map[string][]string{"my_metric{a:2}": {"value<2"}}, false}, + "submetric,nomatch,failing": {true, map[string][]string{"my_metric{a:2}": {"value>1.25"}}, false}, } for name, data := range testdata { diff --git a/stats/thresholds.go b/stats/thresholds.go index bb3d58c4ebc..1b7cbaea53f 100644 --- a/stats/thresholds.go +++ b/stats/thresholds.go @@ -17,54 +17,35 @@ * along with this program. If not, see . * */ - package stats import ( "bytes" "encoding/json" "fmt" + "strings" "time" - "github.com/dop251/goja" - "go.k6.io/k6/lib/types" ) -const jsEnvSrc = ` -function p(pct) { - return __sink__.P(pct/100.0); -}; -` - -var jsEnv *goja.Program - -func init() { - pgm, err := goja.Compile("__env__", jsEnvSrc, true) - if err != nil { - panic(err) - } - jsEnv = pgm -} - // Threshold is a representation of a single threshold for a single metric type Threshold struct { // Source is the text based source of the threshold Source string - // LastFailed is a makrer if the last testing of this threshold failed + // LastFailed is a marker if the last testing of this threshold failed LastFailed bool // AbortOnFail marks if a given threshold fails that the whole test should be aborted AbortOnFail bool // AbortGracePeriod is a the minimum amount of time a test should be running before a failing // this threshold will abort the test AbortGracePeriod types.NullDuration - - pgm *goja.Program - rt *goja.Runtime + // parsed is the threshold expression parsed from the Source + parsed *thresholdExpression } -func newThreshold(src string, newThreshold *goja.Runtime, abortOnFail bool, gracePeriod types.NullDuration) (*Threshold, error) { - pgm, err := goja.Compile("__threshold__", src, true) +func newThreshold(src string, abortOnFail bool, gracePeriod types.NullDuration) (*Threshold, error) { + parsedExpression, err := parseThresholdExpression(src) if err != nil { return nil, err } @@ -73,23 +54,57 @@ func newThreshold(src string, newThreshold *goja.Runtime, abortOnFail bool, grac Source: src, AbortOnFail: abortOnFail, AbortGracePeriod: gracePeriod, - pgm: pgm, - rt: newThreshold, + parsed: parsedExpression, }, nil } -func (t Threshold) runNoTaint() (bool, error) { - v, err := t.rt.RunProgram(t.pgm) - if err != nil { - return false, err +func (t *Threshold) runNoTaint(sinks map[string]float64) (bool, error) { + // Extract the sink value for the aggregation method used in the threshold + // expression + lhs, ok := sinks[t.parsed.AggregationMethod] + if !ok { + return false, fmt.Errorf("unable to apply threshold %s over metrics; reason: "+ + "no metric supporting the %s aggregation method found", + t.Source, + t.parsed.AggregationMethod) } - return v.ToBoolean(), nil + + // Apply the threshold expression operator to the left and + // right hand side values + var passes bool + switch t.parsed.Operator { + case ">": + passes = lhs > t.parsed.Value + case ">=": + passes = lhs >= t.parsed.Value + case "<=": + passes = lhs <= t.parsed.Value + case "<": + passes = lhs < t.parsed.Value + case "==", "===": + // Considering a sink always maps to float64 values, + // strictly equal is equivalent to loosely equal + passes = lhs == t.parsed.Value + case "!=": + passes = lhs != t.parsed.Value + default: + // The parseThresholdExpression function should ensure that no invalid + // operator gets through, but let's protect our future selves anyhow. + return false, fmt.Errorf("unable to apply threshold %s over metrics; "+ + "reason: %s is an invalid operator", + t.Source, + t.parsed.Operator, + ) + } + + // Perform the actual threshold verification + return passes, nil } -func (t *Threshold) run() (bool, error) { - b, err := t.runNoTaint() - t.LastFailed = !b - return b, err +func (t *Threshold) run(sinks map[string]float64) (bool, error) { + passes, err := t.runNoTaint(sinks) + t.LastFailed = !passes + return passes, err } type thresholdConfig struct { @@ -98,11 +113,11 @@ type thresholdConfig struct { AbortGracePeriod types.NullDuration `json:"delayAbortEval"` } -//used internally for JSON marshalling +// used internally for JSON marshalling type rawThresholdConfig thresholdConfig func (tc *thresholdConfig) UnmarshalJSON(data []byte) error { - //shortcircuit unmarshalling for simple string format + // shortcircuit unmarshalling for simple string format if err := json.Unmarshal(data, &tc.Threshold); err == nil { return nil } @@ -122,9 +137,9 @@ func (tc thresholdConfig) MarshalJSON() ([]byte, error) { // Thresholds is the combination of all Thresholds for a given metric type Thresholds struct { - Runtime *goja.Runtime Thresholds []*Threshold Abort bool + sinked map[string]float64 } // NewThresholds returns Thresholds objects representing the provided source strings @@ -138,60 +153,88 @@ func NewThresholds(sources []string) (Thresholds, error) { } func newThresholdsWithConfig(configs []thresholdConfig) (Thresholds, error) { - rt := goja.New() - if _, err := rt.RunProgram(jsEnv); err != nil { - return Thresholds{}, fmt.Errorf("threshold builtin error: %w", err) - } + thresholds := make([]*Threshold, len(configs)) + sinked := make(map[string]float64) - ts := make([]*Threshold, len(configs)) for i, config := range configs { - t, err := newThreshold(config.Threshold, rt, config.AbortOnFail, config.AbortGracePeriod) + t, err := newThreshold(config.Threshold, config.AbortOnFail, config.AbortGracePeriod) if err != nil { return Thresholds{}, fmt.Errorf("threshold %d error: %w", i, err) } - ts[i] = t + thresholds[i] = t } - return Thresholds{rt, ts, false}, nil + return Thresholds{thresholds, false, sinked}, nil } -func (ts *Thresholds) updateVM(sink Sink, t time.Duration) error { - ts.Runtime.Set("__sink__", sink) - f := sink.Format(t) - for k, v := range f { - ts.Runtime.Set(k, v) - } - return nil -} - -func (ts *Thresholds) runAll(t time.Duration) (bool, error) { - succ := true - for i, th := range ts.Thresholds { - b, err := th.run() +func (ts *Thresholds) runAll(timeSpentInTest time.Duration) (bool, error) { + succeeded := true + for i, threshold := range ts.Thresholds { + b, err := threshold.run(ts.sinked) if err != nil { return false, fmt.Errorf("threshold %d run error: %w", i, err) } + if !b { - succ = false + succeeded = false - if ts.Abort || !th.AbortOnFail { + if ts.Abort || !threshold.AbortOnFail { continue } - ts.Abort = !th.AbortGracePeriod.Valid || - th.AbortGracePeriod.Duration < types.Duration(t) + ts.Abort = !threshold.AbortGracePeriod.Valid || + threshold.AbortGracePeriod.Duration < types.Duration(timeSpentInTest) } } - return succ, nil + + return succeeded, nil } // Run processes all the thresholds with the provided Sink at the provided time and returns if any // of them fails -func (ts *Thresholds) Run(sink Sink, t time.Duration) (bool, error) { - if err := ts.updateVM(sink, t); err != nil { - return false, err +func (ts *Thresholds) Run(sink Sink, duration time.Duration) (bool, error) { + // Initialize the sinks store + ts.sinked = make(map[string]float64) + + // FIXME: Remove this comment as soon as the stats.Sink does not expose Format anymore. + // + // As of December 2021, this block reproduces the behavior of the + // stats.Sink.Format behavior. As we intend to try to get away from it, + // we instead implement the behavior directly here. + // + // For more details, see https://github.com/grafana/k6/issues/2320 + switch sinkImpl := sink.(type) { + case *CounterSink: + ts.sinked["count"] = sinkImpl.Value + ts.sinked["rate"] = sinkImpl.Value / (float64(duration) / float64(time.Second)) + case *GaugeSink: + ts.sinked["value"] = sinkImpl.Value + case *TrendSink: + ts.sinked["min"] = sinkImpl.Min + ts.sinked["max"] = sinkImpl.Max + ts.sinked["avg"] = sinkImpl.Avg + ts.sinked["med"] = sinkImpl.Med + + // Parse the percentile thresholds and insert them in + // the sinks mapping. + for _, threshold := range ts.Thresholds { + if !strings.HasPrefix(threshold.parsed.AggregationMethod, "p(") { + continue + } + + ts.sinked[threshold.parsed.AggregationMethod] = sinkImpl.P(threshold.parsed.AggregationValue.Float64 / 100) + } + case *RateSink: + ts.sinked["rate"] = float64(sinkImpl.Trues) / float64(sinkImpl.Total) + case DummySink: + for k, v := range sinkImpl { + ts.sinked[k] = v + } + default: + return false, fmt.Errorf("unable to run Thresholds; reason: unknown sink type") } - return ts.runAll(t) + + return ts.runAll(duration) } // UnmarshalJSON is implementation of json.Unmarshaler diff --git a/stats/thresholds_test.go b/stats/thresholds_test.go index 4d06dd0f05f..9e381bf75fb 100644 --- a/stats/thresholds_test.go +++ b/stats/thresholds_test.go @@ -25,76 +25,257 @@ import ( "testing" "time" - "github.com/dop251/goja" "github.com/stretchr/testify/assert" - + "github.com/stretchr/testify/require" "go.k6.io/k6/lib/types" + "gopkg.in/guregu/null.v3" ) func TestNewThreshold(t *testing.T) { - src := `1+1==2` - rt := goja.New() + t.Parallel() + + src := `rate<0.01` abortOnFail := false gracePeriod := types.NullDurationFrom(2 * time.Second) - th, err := newThreshold(src, rt, abortOnFail, gracePeriod) + wantParsed := &thresholdExpression{tokenRate, null.Float{}, tokenLess, 0.01} + + gotThreshold, err := newThreshold(src, abortOnFail, gracePeriod) + assert.NoError(t, err) + assert.Equal(t, src, gotThreshold.Source) + assert.False(t, gotThreshold.LastFailed) + assert.Equal(t, abortOnFail, gotThreshold.AbortOnFail) + assert.Equal(t, gracePeriod, gotThreshold.AbortGracePeriod) + assert.Equal(t, wantParsed, gotThreshold.parsed) +} + +func TestNewThreshold_InvalidThresholdConditionExpression(t *testing.T) { + t.Parallel() + + src := "1+1==2" + abortOnFail := false + gracePeriod := types.NullDurationFrom(2 * time.Second) - assert.Equal(t, src, th.Source) - assert.False(t, th.LastFailed) - assert.NotNil(t, th.pgm) - assert.Equal(t, rt, th.rt) - assert.Equal(t, abortOnFail, th.AbortOnFail) - assert.Equal(t, gracePeriod, th.AbortGracePeriod) + gotThreshold, err := newThreshold(src, abortOnFail, gracePeriod) + + assert.Error(t, err, "instantiating a threshold with an invalid expression should fail") + assert.Nil(t, gotThreshold, "instantiating a threshold with an invalid expression should return a nil Threshold") +} + +func TestThreshold_runNoTaint(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + parsed *thresholdExpression + abortGracePeriod types.NullDuration + sinks map[string]float64 + wantOk bool + wantErr bool + }{ + { + name: "valid expression using the > operator over passing threshold", + parsed: &thresholdExpression{tokenRate, null.Float{}, tokenGreater, 0.01}, + abortGracePeriod: types.NullDurationFrom(0 * time.Second), + sinks: map[string]float64{"rate": 1}, + wantOk: true, + wantErr: false, + }, + { + name: "valid expression using the > operator over passing threshold and defined abort grace period", + parsed: &thresholdExpression{tokenRate, null.Float{}, tokenGreater, 0.01}, + abortGracePeriod: types.NullDurationFrom(2 * time.Second), + sinks: map[string]float64{"rate": 1}, + wantOk: true, + wantErr: false, + }, + { + name: "valid expression using the >= operator over passing threshold", + parsed: &thresholdExpression{tokenRate, null.Float{}, tokenGreaterEqual, 0.01}, + abortGracePeriod: types.NullDurationFrom(0 * time.Second), + sinks: map[string]float64{"rate": 0.01}, + wantOk: true, + wantErr: false, + }, + { + name: "valid expression using the <= operator over passing threshold", + parsed: &thresholdExpression{tokenRate, null.Float{}, tokenLessEqual, 0.01}, + abortGracePeriod: types.NullDurationFrom(0 * time.Second), + sinks: map[string]float64{"rate": 0.01}, + wantOk: true, + wantErr: false, + }, + { + name: "valid expression using the < operator over passing threshold", + parsed: &thresholdExpression{tokenRate, null.Float{}, tokenLess, 0.01}, + abortGracePeriod: types.NullDurationFrom(0 * time.Second), + sinks: map[string]float64{"rate": 0.00001}, + wantOk: true, + wantErr: false, + }, + { + name: "valid expression using the == operator over passing threshold", + parsed: &thresholdExpression{tokenRate, null.Float{}, tokenLooselyEqual, 0.01}, + abortGracePeriod: types.NullDurationFrom(0 * time.Second), + sinks: map[string]float64{"rate": 0.01}, + wantOk: true, + wantErr: false, + }, + { + name: "valid expression using the === operator over passing threshold", + parsed: &thresholdExpression{tokenRate, null.Float{}, tokenStrictlyEqual, 0.01}, + abortGracePeriod: types.NullDurationFrom(0 * time.Second), + sinks: map[string]float64{"rate": 0.01}, + wantOk: true, + wantErr: false, + }, + { + name: "valid expression using != operator over passing threshold", + parsed: &thresholdExpression{tokenRate, null.Float{}, tokenBangEqual, 0.01}, + abortGracePeriod: types.NullDurationFrom(0 * time.Second), + sinks: map[string]float64{"rate": 0.02}, + wantOk: true, + wantErr: false, + }, + { + name: "valid expression over failing threshold", + parsed: &thresholdExpression{tokenRate, null.Float{}, tokenGreater, 0.01}, + abortGracePeriod: types.NullDurationFrom(0 * time.Second), + sinks: map[string]float64{"rate": 0.00001}, + wantOk: false, + wantErr: false, + }, + { + name: "valid expression over non-existing sink", + parsed: &thresholdExpression{tokenRate, null.Float{}, tokenGreater, 0.01}, + abortGracePeriod: types.NullDurationFrom(0 * time.Second), + sinks: map[string]float64{"med": 27.2}, + wantOk: false, + wantErr: true, + }, + { + // The ParseThresholdCondition constructor should ensure that no invalid + // operator gets through, but let's protect our future selves anyhow. + name: "invalid expression operator", + parsed: &thresholdExpression{tokenRate, null.Float{}, "&", 0.01}, + abortGracePeriod: types.NullDurationFrom(0 * time.Second), + sinks: map[string]float64{"rate": 0.00001}, + wantOk: false, + wantErr: true, + }, + } + for _, testCase := range tests { + testCase := testCase + + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + threshold := &Threshold{ + LastFailed: false, + AbortOnFail: false, + AbortGracePeriod: testCase.abortGracePeriod, + parsed: testCase.parsed, + } + + gotOk, gotErr := threshold.runNoTaint(testCase.sinks) + + assert.Equal(t, + testCase.wantErr, + gotErr != nil, + "Threshold.runNoTaint() error = %v, wantErr %v", gotErr, testCase.wantErr, + ) + + assert.Equal(t, + testCase.wantOk, + gotOk, + "Threshold.runNoTaint() gotOk = %v, want %v", gotOk, testCase.wantOk, + ) + }) + } +} + +func BenchmarkRunNoTaint(b *testing.B) { + threshold := &Threshold{ + Source: "rate>0.01", + LastFailed: false, + AbortOnFail: false, + AbortGracePeriod: types.NullDurationFrom(2 * time.Second), + parsed: &thresholdExpression{tokenRate, null.Float{}, tokenGreater, 0.01}, + } + + sinks := map[string]float64{"rate": 1} + + b.ResetTimer() + + for i := 0; i < b.N; i++ { + threshold.runNoTaint(sinks) // nolint + } } func TestThresholdRun(t *testing.T) { + t.Parallel() + t.Run("true", func(t *testing.T) { - th, err := newThreshold(`1+1==2`, goja.New(), false, types.NullDuration{}) + t.Parallel() + + sinks := map[string]float64{"rate": 0.0001} + threshold, err := newThreshold(`rate<0.01`, false, types.NullDuration{}) assert.NoError(t, err) t.Run("no taint", func(t *testing.T) { - b, err := th.runNoTaint() + b, err := threshold.runNoTaint(sinks) assert.NoError(t, err) assert.True(t, b) - assert.False(t, th.LastFailed) + assert.False(t, threshold.LastFailed) }) t.Run("taint", func(t *testing.T) { - b, err := th.run() + t.Parallel() + + b, err := threshold.run(sinks) assert.NoError(t, err) assert.True(t, b) - assert.False(t, th.LastFailed) + assert.False(t, threshold.LastFailed) }) }) t.Run("false", func(t *testing.T) { - th, err := newThreshold(`1+1==4`, goja.New(), false, types.NullDuration{}) + t.Parallel() + + sinks := map[string]float64{"rate": 1} + threshold, err := newThreshold(`rate<0.01`, false, types.NullDuration{}) assert.NoError(t, err) t.Run("no taint", func(t *testing.T) { - b, err := th.runNoTaint() + b, err := threshold.runNoTaint(sinks) assert.NoError(t, err) assert.False(t, b) - assert.False(t, th.LastFailed) + assert.False(t, threshold.LastFailed) }) t.Run("taint", func(t *testing.T) { - b, err := th.run() + b, err := threshold.run(sinks) assert.NoError(t, err) assert.False(t, b) - assert.True(t, th.LastFailed) + assert.True(t, threshold.LastFailed) }) }) } func TestNewThresholds(t *testing.T) { + t.Parallel() + t.Run("empty", func(t *testing.T) { + t.Parallel() + ts, err := NewThresholds([]string{}) assert.NoError(t, err) assert.Len(t, ts.Thresholds, 0) }) t.Run("two", func(t *testing.T) { - sources := []string{`1+1==2`, `1+1==4`} + t.Parallel() + + sources := []string{`rate<0.01`, `p(95)<200`} ts, err := NewThresholds(sources) assert.NoError(t, err) assert.Len(t, ts.Thresholds, 2) @@ -102,22 +283,26 @@ func TestNewThresholds(t *testing.T) { assert.Equal(t, sources[i], th.Source) assert.False(t, th.LastFailed) assert.False(t, th.AbortOnFail) - assert.NotNil(t, th.pgm) - assert.Equal(t, ts.Runtime, th.rt) } }) } func TestNewThresholdsWithConfig(t *testing.T) { + t.Parallel() + t.Run("empty", func(t *testing.T) { + t.Parallel() + ts, err := NewThresholds([]string{}) assert.NoError(t, err) assert.Len(t, ts.Thresholds, 0) }) t.Run("two", func(t *testing.T) { + t.Parallel() + configs := []thresholdConfig{ - {`1+1==2`, false, types.NullDuration{}}, - {`1+1==4`, true, types.NullDuration{}}, + {`rate<0.01`, false, types.NullDuration{}}, + {`p(95)<200`, true, types.NullDuration{}}, } ts, err := newThresholdsWithConfig(configs) assert.NoError(t, err) @@ -126,53 +311,47 @@ func TestNewThresholdsWithConfig(t *testing.T) { assert.Equal(t, configs[i].Threshold, th.Source) assert.False(t, th.LastFailed) assert.Equal(t, configs[i].AbortOnFail, th.AbortOnFail) - assert.NotNil(t, th.pgm) - assert.Equal(t, ts.Runtime, th.rt) } }) } -func TestThresholdsUpdateVM(t *testing.T) { - ts, err := NewThresholds(nil) - assert.NoError(t, err) - assert.NoError(t, ts.updateVM(DummySink{"a": 1234.5}, 0)) - assert.Equal(t, 1234.5, ts.Runtime.Get("a").ToFloat()) -} - func TestThresholdsRunAll(t *testing.T) { + t.Parallel() + zero := types.NullDuration{} oneSec := types.NullDurationFrom(time.Second) twoSec := types.NullDurationFrom(2 * time.Second) testdata := map[string]struct { - succ bool - err bool - abort bool - grace types.NullDuration - srcs []string + succeeded bool + err bool + abort bool + grace types.NullDuration + sources []string }{ - "one passing": {true, false, false, zero, []string{`1+1==2`}}, - "one failing": {false, false, false, zero, []string{`1+1==4`}}, - "two passing": {true, false, false, zero, []string{`1+1==2`, `2+2==4`}}, - "two failing": {false, false, false, zero, []string{`1+1==4`, `2+2==2`}}, - "two mixed": {false, false, false, zero, []string{`1+1==2`, `1+1==4`}}, - "one erroring": {false, true, false, zero, []string{`throw new Error('?!');`}}, - "one aborting": {false, false, true, zero, []string{`1+1==4`}}, - "abort with grace period": {false, false, true, oneSec, []string{`1+1==4`}}, - "no abort with grace period": {false, false, true, twoSec, []string{`1+1==4`}}, + "one passing": {true, false, false, zero, []string{`rate<0.01`}}, + "one failing": {false, false, false, zero, []string{`p(95)<200`}}, + "two passing": {true, false, false, zero, []string{`rate<0.1`, `rate<0.01`}}, + "two failing": {false, false, false, zero, []string{`p(95)<200`, `rate<0.1`}}, + "two mixed": {false, false, false, zero, []string{`rate<0.01`, `p(95)<200`}}, + "one aborting": {false, false, true, zero, []string{`p(95)<200`}}, + "abort with grace period": {false, false, true, oneSec, []string{`p(95)<200`}}, + "no abort with grace period": {false, false, true, twoSec, []string{`p(95)<200`}}, } for name, data := range testdata { t.Run(name, func(t *testing.T) { - ts, err := NewThresholds(data.srcs) - assert.Nil(t, err) - ts.Thresholds[0].AbortOnFail = data.abort - ts.Thresholds[0].AbortGracePeriod = data.grace + t.Parallel() + + thresholds, err := NewThresholds(data.sources) + thresholds.sinked = map[string]float64{"rate": 0.0001, "p(95)": 500} + thresholds.Thresholds[0].AbortOnFail = data.abort + thresholds.Thresholds[0].AbortGracePeriod = data.grace runDuration := 1500 * time.Millisecond assert.NoError(t, err) - b, err := ts.runAll(runDuration) + succeeded, err := thresholds.runAll(runDuration) if data.err { assert.Error(t, err) @@ -180,48 +359,74 @@ func TestThresholdsRunAll(t *testing.T) { assert.NoError(t, err) } - if data.succ { - assert.True(t, b) + if data.succeeded { + assert.True(t, succeeded) } else { - assert.False(t, b) + assert.False(t, succeeded) } if data.abort && data.grace.Duration < types.Duration(runDuration) { - assert.True(t, ts.Abort) + assert.True(t, thresholds.Abort) } else { - assert.False(t, ts.Abort) + assert.False(t, thresholds.Abort) } }) } } -func TestThresholdsRun(t *testing.T) { - ts, err := NewThresholds([]string{"a>0"}) - assert.NoError(t, err) +func TestThresholds_Run(t *testing.T) { + t.Parallel() - t.Run("error", func(t *testing.T) { - b, err := ts.Run(DummySink{}, 0) - assert.Error(t, err) - assert.False(t, b) - }) + type args struct { + sink Sink + duration time.Duration + } + tests := []struct { + name string + args args + want bool + wantErr bool + }{ + { + "Running thresholds of existing sink", + args{DummySink{"p(95)": 1234.5}, 0}, + true, + false, + }, + { + "Running thresholds of existing sink but failing threshold", + args{DummySink{"p(95)": 3000}, 0}, + false, + false, + }, + { + "Running threshold on non existing sink fails", + args{DummySink{"dummy": 0}, 0}, + false, + true, + }, + } + for _, testCase := range tests { + testCase := testCase + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() - t.Run("pass", func(t *testing.T) { - b, err := ts.Run(DummySink{"a": 1234.5}, 0) - assert.NoError(t, err) - assert.True(t, b) - }) + thresholds, err := NewThresholds([]string{"p(95)<2000"}) + require.NoError(t, err, "Initializing new thresholds should not fail") - t.Run("fail", func(t *testing.T) { - b, err := ts.Run(DummySink{"a": 0}, 0) - assert.NoError(t, err) - assert.False(t, b) - }) + gotOk, gotErr := thresholds.Run(testCase.args.sink, testCase.args.duration) + assert.Equal(t, gotErr != nil, testCase.wantErr, "Thresholds.Run() error = %v, wantErr %v", gotErr, testCase.wantErr) + assert.Equal(t, gotOk, testCase.want, "Thresholds.Run() = %v, want %v", gotOk, testCase.want) + }) + } } func TestThresholdsJSON(t *testing.T) { - var testdata = []struct { + t.Parallel() + + testdata := []struct { JSON string - srcs []string + sources []string abortOnFail bool gracePeriod types.NullDuration outputJSON string @@ -234,8 +439,8 @@ func TestThresholdsJSON(t *testing.T) { "", }, { - `["1+1==2"]`, - []string{"1+1==2"}, + `["rate<0.01"]`, + []string{"rate<0.01"}, false, types.NullDuration{}, "", @@ -248,55 +453,59 @@ func TestThresholdsJSON(t *testing.T) { `["rate<0.01"]`, }, { - `["1+1==2","1+1==3"]`, - []string{"1+1==2", "1+1==3"}, + `["rate<0.01","p(95)<200"]`, + []string{"rate<0.01", "p(95)<200"}, false, types.NullDuration{}, "", }, { - `[{"threshold":"1+1==2"}]`, - []string{"1+1==2"}, + `[{"threshold":"rate<0.01"}]`, + []string{"rate<0.01"}, false, types.NullDuration{}, - `["1+1==2"]`, + `["rate<0.01"]`, }, { - `[{"threshold":"1+1==2","abortOnFail":true,"delayAbortEval":null}]`, - []string{"1+1==2"}, + `[{"threshold":"rate<0.01","abortOnFail":true,"delayAbortEval":null}]`, + []string{"rate<0.01"}, true, types.NullDuration{}, "", }, { - `[{"threshold":"1+1==2","abortOnFail":true,"delayAbortEval":"2s"}]`, - []string{"1+1==2"}, + `[{"threshold":"rate<0.01","abortOnFail":true,"delayAbortEval":"2s"}]`, + []string{"rate<0.01"}, true, types.NullDurationFrom(2 * time.Second), "", }, { - `[{"threshold":"1+1==2","abortOnFail":false}]`, - []string{"1+1==2"}, + `[{"threshold":"rate<0.01","abortOnFail":false}]`, + []string{"rate<0.01"}, false, types.NullDuration{}, - `["1+1==2"]`, + `["rate<0.01"]`, }, { - `[{"threshold":"1+1==2"}, "1+1==3"]`, - []string{"1+1==2", "1+1==3"}, + `[{"threshold":"rate<0.01"}, "p(95)<200"]`, + []string{"rate<0.01", "p(95)<200"}, false, types.NullDuration{}, - `["1+1==2","1+1==3"]`, + `["rate<0.01","p(95)<200"]`, }, } for _, data := range testdata { + data := data + t.Run(data.JSON, func(t *testing.T) { + t.Parallel() + var ts Thresholds assert.NoError(t, json.Unmarshal([]byte(data.JSON), &ts)) - assert.Equal(t, len(data.srcs), len(ts.Thresholds)) - for i, src := range data.srcs { + assert.Equal(t, len(data.sources), len(ts.Thresholds)) + for i, src := range data.sources { assert.Equal(t, src, ts.Thresholds[i].Source) assert.Equal(t, data.abortOnFail, ts.Thresholds[i].AbortOnFail) assert.Equal(t, data.gracePeriod, ts.Thresholds[i].AbortGracePeriod) @@ -315,18 +524,20 @@ func TestThresholdsJSON(t *testing.T) { } t.Run("bad JSON", func(t *testing.T) { + t.Parallel() + var ts Thresholds assert.Error(t, json.Unmarshal([]byte("42"), &ts)) assert.Nil(t, ts.Thresholds) - assert.Nil(t, ts.Runtime) assert.False(t, ts.Abort) }) t.Run("bad source", func(t *testing.T) { + t.Parallel() + var ts Thresholds assert.Error(t, json.Unmarshal([]byte(`["="]`), &ts)) assert.Nil(t, ts.Thresholds) - assert.Nil(t, ts.Runtime) assert.False(t, ts.Abort) }) }