Move metric name check to the registry

js/modules/k6/metrics/metrics.go

@@ -23,8 +23,6 @@ package metrics
 import (
 	"context"
 	"errors"
-	"fmt"
-	"regexp"
 	"time"
 
 	"github.com/dop251/goja"
@@ -34,14 +32,6 @@ import (
 	"go.k6.io/k6/stats"
 )
 
-var nameRegexString = "^[\\p{L}\\p{N}\\._ !\\?/&#\\(\\)<>%-]{1,128}$"
-
-var compileNameRegex = regexp.MustCompile(nameRegexString)
-
-func checkName(name string) bool {
-	return compileNameRegex.Match([]byte(name))
-}
-
 type Metric struct {
 	metric *stats.Metric
 }
@@ -55,11 +45,6 @@ func newMetric(ctxPtr *context.Context, name string, t stats.MetricType, isTime
 		return nil, errors.New("metrics must be declared in the init context")
 	}
 
-	// TODO: move verification outside the JS
-	if !checkName(name) {
-		return nil, common.NewInitContextError(fmt.Sprintf("Invalid metric name: '%s'", name))
-	}
-
 	valueType := stats.Default
 	if len(isTime) > 0 && isTime[0] {
 		valueType = stats.Time

js/modules/k6/metrics/metrics_test.go

@@ -152,28 +152,6 @@ func TestMetrics(t *testing.T) {
 	}
 }
 
-func TestMetricNames(t *testing.T) {
-	t.Parallel()
-	testMap := map[string]bool{
-		"simple":       true,
-		"still_simple": true,
-		"":             false,
-		"@":            false,
-		"a":            true,
-		"special\n\t":  false,
-		// this has both hangul and japanese numerals .
-		"hello.World_in_한글一안녕一세상": true,
-		// too long
-		"tooolooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooog": false,
-	}
-
-	for key, value := range testMap {
-		t.Run(key, func(t *testing.T) {
-			assert.Equal(t, value, checkName(key), key)
-		})
-	}
-}
-
 func TestMetricGetName(t *testing.T) {
 	t.Parallel()
 	rt := goja.New()

lib/metrics/registry.go

@@ -22,6 +22,7 @@ package metrics
 
 import (
 	"fmt"
+	"regexp"
 	"sync"
 
 	"go.k6.io/k6/stats"
@@ -40,19 +41,30 @@ func NewRegistry() *Registry {
 	}
 }
 
+const nameRegexString = "^[\\p{L}\\p{N}\\._ !\\?/&#\\(\\)<>%-]{1,128}$"
+
+var compileNameRegex = regexp.MustCompile(nameRegexString)
+
+func checkName(name string) bool {
+	return compileNameRegex.Match([]byte(name))
+}
+
 // NewMetric returns new metric registered to this registry
 // TODO have multiple versions returning specific metric types when we have such things
 func (r *Registry) NewMetric(name string, typ stats.MetricType, t ...stats.ValueType) (*stats.Metric, error) {
 	r.l.Lock()
 	defer r.l.Unlock()
+
+	if !checkName(name) {
+		return nil, fmt.Errorf("Invalid metric name: '%s'", name) //nolint:golint,stylecheck
+	}
 	oldMetric, ok := r.metrics[name]
 
 	if !ok {
 		m := stats.New(name, typ, t...)
 		r.metrics[name] = m
 		return m, nil
 	}
-	// Name is checked. TODO check Contains as well
 	if oldMetric.Type != typ {
 		return nil, fmt.Errorf("metric `%s` already exist but with type %s, instead of %s", name, oldMetric.Type, typ)
 	}

lib/metrics/registry_test.go

@@ -3,6 +3,7 @@ package metrics
 import (
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.k6.io/k6/stats"
 )
@@ -26,3 +27,27 @@ func TestRegistryNewMetric(t *testing.T) {
 	_, err = r.NewMetric("something", stats.Counter, stats.Time)
 	require.Error(t, err)
 }
+
+func TestMetricNames(t *testing.T) {
+	t.Parallel()
+	testMap := map[string]bool{
+		"simple":       true,
+		"still_simple": true,
+		"":             false,
+		"@":            false,
+		"a":            true,
+		"special\n\t":  false,
+		// this has both hangul and japanese numerals .
+		"hello.World_in_한글一안녕一세상": true,
+		// too long
+		"tooolooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooog": false,
+	}
+
+	for key, value := range testMap {
+		key, value := key, value
+		t.Run(key, func(t *testing.T) {
+			t.Parallel()
+			assert.Equal(t, value, checkName(key), key)
+		})
+	}
+}