diff --git a/cmd/tests/cmd_run_test.go b/cmd/tests/cmd_run_test.go index 1c874963bf5..65bc2e54eef 100644 --- a/cmd/tests/cmd_run_test.go +++ b/cmd/tests/cmd_run_test.go @@ -1552,7 +1552,7 @@ func TestMinIterationDuration(t *testing.T) { assert.Contains(t, stdout, "✓ test_counter.........: 3") } -func TestMetricNameWarning(t *testing.T) { +func TestMetricNameError(t *testing.T) { t.Parallel() script := ` import { Counter } from 'k6/metrics'; @@ -1566,14 +1566,14 @@ func TestMetricNameWarning(t *testing.T) { }; var c = new Counter('test counter'); - new Counter('test_counter_#'); + new Counter('test_counter_#'); // this is also bad but we error on the one above export function setup() { c.add(1); }; export default function () { c.add(1); }; export function teardown() { c.add(1); }; ` - ts := getSimpleCloudOutputTestState(t, script, nil, cloudapi.RunStatusFinished, cloudapi.ResultStatusPassed, 0) + ts := getSingleFileTestState(t, script, nil, exitcodes.ScriptException) cmd.ExecuteWithGlobalState(ts.GlobalState) @@ -1581,13 +1581,10 @@ func TestMetricNameWarning(t *testing.T) { t.Log(stdout) logEntries := ts.LoggerHook.Drain() - expectedMsg := `Metric name should only include up to 128 ASCII letters, numbers and/or underscores.` - filteredEntries := testutils.FilterEntries(logEntries, logrus.WarnLevel, expectedMsg) - require.Len(t, filteredEntries, 2) - // we do it this way as ordering is not guaranteed - names := []interface{}{filteredEntries[0].Data["name"], filteredEntries[1].Data["name"]} - require.Contains(t, names, "test counter") - require.Contains(t, names, "test_counter_#") + expectedMsg := `Metric names must only include up to 128 ASCII letters, numbers, or underscores` + filteredEntries := testutils.FilterEntries(logEntries, logrus.ErrorLevel, expectedMsg) + require.Len(t, filteredEntries, 1) + require.Contains(t, filteredEntries[0].Message, "'test counter'") } func TestRunTags(t *testing.T) { diff --git a/js/bundle.go b/js/bundle.go index 0f694339722..6525c50b9f0 100644 --- a/js/bundle.go +++ b/js/bundle.go @@ -8,7 +8,6 @@ import ( "fmt" "net/url" "path/filepath" - "regexp" "runtime" "github.com/dop251/goja" @@ -42,26 +41,6 @@ type Bundle struct { ModuleResolver *modules.ModuleResolver } -// TODO: this is to be removed once this is not a warning and it can be moved to the registry -// https://github.com/grafana/k6/issues/3065 -func (b *Bundle) checkMetricNamesForPrometheusCompatibility() { - const ( - // The name restrictions are the union of Otel and Prometheus naming restrictions, with the length restrictions of 128 - // coming from old k6 restrictions where character set was way bigger though. - nameRegexString = "^[a-zA-Z_][a-zA-Z0-9_]{1,128}$" - badNameWarning = "Metric name should only include up to 128 ASCII letters, numbers and/or underscores. " + - "This name will stop working in k6 v0.48.0 (around December 2023)." - ) - - compileNameRegex := regexp.MustCompile(nameRegexString) - - for _, metric := range b.preInitState.Registry.All() { - if !compileNameRegex.MatchString(metric.Name) { - b.preInitState.Logger.WithField("name", metric.Name).Warn(badNameWarning) - } - } -} - // A BundleInstance is a self-contained instance of a Bundle. type BundleInstance struct { Runtime *goja.Runtime @@ -134,8 +113,6 @@ func newBundle( return nil, err } - bundle.checkMetricNamesForPrometheusCompatibility() - return bundle, nil } diff --git a/metrics/registry.go b/metrics/registry.go index 3a60466bba3..6f4e9b16fce 100644 --- a/metrics/registry.go +++ b/metrics/registry.go @@ -26,7 +26,11 @@ func NewRegistry() *Registry { } } -const nameRegexString = "^[\\p{L}\\p{N}\\._ !\\?/&#\\(\\)<>%-]{1,128}$" +const ( + nameRegexString = "^[a-zA-Z_][a-zA-Z0-9_]{1,128}$" + badNameWarning = "Metric names must only include up to 128 ASCII letters, numbers, or underscores " + + "and start with a letter or an underscore." +) var compileNameRegex = regexp.MustCompile(nameRegexString) @@ -41,7 +45,7 @@ func (r *Registry) NewMetric(name string, typ MetricType, t ...ValueType) (*Metr defer r.l.Unlock() if !checkName(name) { - return nil, fmt.Errorf("Invalid metric name: '%s'", name) //nolint:golint,stylecheck + return nil, fmt.Errorf("Invalid metric name: '%s'. %s", name, badNameWarning) //nolint:golint,stylecheck } oldMetric, ok := r.metrics[name] diff --git a/metrics/registry_test.go b/metrics/registry_test.go index b20d3d60324..6784ecf9ff7 100644 --- a/metrics/registry_test.go +++ b/metrics/registry_test.go @@ -35,10 +35,10 @@ func TestMetricNames(t *testing.T) { "still_simple": true, "": false, "@": false, - "a": true, + "a": false, // too short "special\n\t": false, // this has both hangul and japanese numerals . - "hello.World_in_한글一안녕一세상": true, + "hello.World_in_한글一안녕一세상": false, // too long "tooolooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooog": false, }