metrics: enforce more otel/prometheus aligned names (#3448)

Closes #3065


Co-authored-by: Oleg Bespalov <oleg.bespalov@grafana.com>

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,28 +1566,25 @@ 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)
 
 	stdout := ts.Stdout.String()
 	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) {

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
 }
 

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]
 

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,
 	}