From 4bfec5dfea2b279ed7d18faf1ffc0b0532a547dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20Crevon?= Date: Wed, 12 Jan 2022 14:14:29 +0100 Subject: [PATCH] Merge pull request #2251 from grafana/fix/1443_remove_the_js_runtime_from_threshold_calcultations Fix/1443 remove the JS runtime from threshold calculations --- core/engine_test.go | 29 ++- stats/thresholds.go | 181 ++++++++------ stats/thresholds_parser.go | 208 ++++++++++++++++ stats/thresholds_parser_test.go | 361 ++++++++++++++++++++++++++++ stats/thresholds_test.go | 411 ++++++++++++++++++++++++-------- 5 files changed, 1010 insertions(+), 180 deletions(-) create mode 100644 stats/thresholds_parser.go create mode 100644 stats/thresholds_parser_test.go diff --git a/core/engine_test.go b/core/engine_test.go index 5427a4579e3a..f1b343a2cd38 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 bb3d58c4ebc8..1b7cbaea53fa 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_parser.go b/stats/thresholds_parser.go new file mode 100644 index 000000000000..72b0f82e2a73 --- /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 000000000000..25120684d6e1 --- /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 + } +} diff --git a/stats/thresholds_test.go b/stats/thresholds_test.go index 4d06dd0f05f8..9e381bf75fb8 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) }) }