Skip to content

Commit

Permalink
Validate threshold configuration before starting the execution
Browse files Browse the repository at this point in the history
This commit makes sure that the threshold configuration (as passed in
the script exported options for instance) is valid, before starting the
execution.

A valid threshold must pass the following assertion:
- Its expression is syntaxically correct and is parsable
- It applies to a metrics that's known to k6, either builtin or custom
- Its expression's aggregation method is valid for the metric it applies
to

Threshold validation will be made in the context of the `run`, `cloud`,
`archive`, `inspect`, and `archive` commands. If a threshold definition
is invalid, the k6 program will exit with
a status code of 104.

ref #2330
  • Loading branch information
oleiade committed Jan 27, 2022
1 parent ee8277b commit db73489
Show file tree
Hide file tree
Showing 10 changed files with 102 additions and 6 deletions.
2 changes: 1 addition & 1 deletion cmd/archive.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ An archive is a fully self-contained test run, and can be executed identically e
return err
}

_, err = deriveAndValidateConfig(conf, r.IsExecutable)
_, err = deriveAndValidateConfig(conf, registry, r.IsExecutable)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth
return err
}

derivedConf, err := deriveAndValidateConfig(conf, r.IsExecutable)
derivedConf, err := deriveAndValidateConfig(conf, registry, r.IsExecutable)
if err != nil {
return err
}
Expand Down
6 changes: 6 additions & 0 deletions cmd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,12 @@ func deriveAndValidateConfig(
if err != nil {
return result, err
}

err = validateThresholdsConfig(conf, registry)
if err != nil {
return result, err
}

return result, errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig)
}

Expand Down
5 changes: 3 additions & 2 deletions cmd/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import (
"go.k6.io/k6/lib"
"go.k6.io/k6/lib/executor"
"go.k6.io/k6/lib/metrics"
"go.k6.io/k6/lib/testutils"
"go.k6.io/k6/lib/types"
"go.k6.io/k6/stats"
"gopkg.in/guregu/null.v3"
Expand Down Expand Up @@ -205,7 +204,7 @@ func TestDeriveAndValidateConfig(t *testing.T) {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
_, err := deriveAndValidateConfig(tc.conf,
_, err := deriveAndValidateConfig(tc.conf, nil,
func(_ string) bool { return tc.isExec })
if tc.err != "" {
var ecerr errext.HasExitCode
Expand All @@ -220,6 +219,7 @@ func TestDeriveAndValidateConfig(t *testing.T) {
}

func TestValidateThresholdsConfigWithNilRegistry(t *testing.T) {
t.Parallel()
var registry *metrics.Registry
config := Config{}
var wantErrType errext.HasExitCode
Expand All @@ -231,6 +231,7 @@ func TestValidateThresholdsConfigWithNilRegistry(t *testing.T) {
}

func TestValidateThresholdsConfigAppliesToBuiltinMetrics(t *testing.T) {
t.Parallel()
// Prepare a registry loaded with builtin metrics
registry := metrics.NewRegistry()
metrics.RegisterBuiltinMetrics(registry)
Expand Down
2 changes: 1 addition & 1 deletion cmd/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func addExecRequirements(b *js.Bundle,
return nil, err
}

conf, err = deriveAndValidateConfig(conf, runner.IsExecutable)
conf, err = deriveAndValidateConfig(conf, registry, runner.IsExecutable)
if err != nil {
return nil, err
}
Expand Down
7 changes: 6 additions & 1 deletion cmd/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import (
"go.k6.io/k6/lib/consts"
"go.k6.io/k6/lib/metrics"
"go.k6.io/k6/loader"
"go.k6.io/k6/stats"
"go.k6.io/k6/ui/pb"
)

Expand Down Expand Up @@ -114,6 +115,10 @@ a commandline interface for interacting with it.`,
builtinMetrics := metrics.RegisterBuiltinMetrics(registry)
initRunner, err := newRunner(logger, src, runType, filesystems, runtimeOptions, builtinMetrics, registry)
if err != nil {
if errors.Is(err, stats.ErrThresholdParsing) {
return errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig)
}

return common.UnwrapGojaInterruptedError(err)
}

Expand All @@ -128,7 +133,7 @@ a commandline interface for interacting with it.`,
return err
}

conf, err = deriveAndValidateConfig(conf, initRunner.IsExecutable)
conf, err = deriveAndValidateConfig(conf, registry, initRunner.IsExecutable)
if err != nil {
return err
}
Expand Down
45 changes: 45 additions & 0 deletions cmd/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,3 +216,48 @@ func TestInitErrExitCode(t *testing.T) {
"Status code must be %d", exitcodes.ScriptException)
assert.Contains(t, err.Error(), "ReferenceError: someUndefinedVar is not defined")
}

func TestInvalidOptionsThresholdErrExitCode(t *testing.T) {
t.Parallel()

testCases := []struct {
name string
testFilename string
}{
{
"run should fail with exit status 104 on a malformed threshold expression",
"testdata/thresholds/malformed_expression.js",
},
{
"run should fail with exit status 104 on a threshold applied to a non existing metric",
"testdata/thresholds/non_existing_metric.js",
},
{
"run should fail with exit status 104 on a threshold method being unsupported by the metric",
"testdata/thresholds/unsupported_aggregation_method.js",
},
}

for _, testCase := range testCases {
testCase := testCase
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()

ctx, cancel := context.WithCancel(context.Background())
defer cancel()
cmd := getRunCmd(ctx, testutils.NewLogger(t))
a, err := filepath.Abs(testCase.testFilename)
require.NoError(t, err)
cmd.SetArgs([]string{a})
wantExitCode := exitcodes.InvalidConfig

var gotErrExt errext.HasExitCode
gotErr := cmd.Execute()

require.ErrorAs(t, gotErr, &gotErrExt)
assert.Equalf(t, wantExitCode, gotErrExt.ExitCode(),
"status code must be %d", wantExitCode,
)
})
}
}
11 changes: 11 additions & 0 deletions cmd/testdata/thresholds/malformed_expression.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
export const options = {
thresholds: {
http_reqs: ["foo>0"], // Counter
},
};

export default function () {
console.log(
"asserting that a malformed threshold fails with exit code 104 (Invalid config)"
);
}
13 changes: 13 additions & 0 deletions cmd/testdata/thresholds/non_existing_metric.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
export const options = {
thresholds: {
// non existing is neither registered, nor a builtin metric.
// k6 should catch that.
"non existing": ["rate>0"],
},
};

export default function () {
console.log(
"asserting that a threshold over a non-existing metric fails with exit code 104 (Invalid config)"
);
}
15 changes: 15 additions & 0 deletions cmd/testdata/thresholds/unsupported_aggregation_method.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
export const options = {
thresholds: {
// http_reqs is a Counter metric. As such, it supports
// only the 'count' and 'rate' operations. Thus, 'value'
// being a Gauge's metric aggregation method, the threshold
// configuration evaluation should fail.
http_reqs: ["value>0"],
},
};

export default function () {
console.log(
"asserting that a threshold applying a method over a metric not supporting it fails with exit code 104 (Invalid config)"
);
}

0 comments on commit db73489

Please sign in to comment.