k6/metrics: Warn on metric names not compatible with Prometheus

This adds a warning for custom metrics that are not Prometheus
compatible.

The warning is in the k6/metrics as the alternative was threading a
logger in the registry specifically for this and then removing it.

The major downside of this is that extension can still register
incompatible metric names without any warnings.

Updates #3065

cmd/tests/cmd_run_test.go

@@ -1543,6 +1543,42 @@ func TestMinIterationDuration(t *testing.T) {
 	assert.Contains(t, stdout, "✓ test_counter.........: 3")
 }
 
+func TestMetricNameWarning(t *testing.T) {
+	t.Parallel()
+	script := `
+		import { Counter } from 'k6/metrics';
+
+		export let options = {
+			vus: 2,
+			iterations: 2,
+			thresholds: {
+				'test counter': ['count == 4'],
+			},
+		};
+
+		var c = new Counter('test counter');
+		new Counter('test_counter_#');
+
+		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)
+
+	cmd.ExecuteWithGlobalState(ts.GlobalState)
+
+	stdout := ts.Stdout.String()
+	t.Log(stdout)
+
+	logEntries := ts.LoggerHook.Drain()
+	expectedMsg := `Metric name should only include ASCII letters, numbers and underscores. This name will stop working in k6 v0.49.0.`
+	filteredEntries := testutils.FilterEntries(logEntries, logrus.WarnLevel, expectedMsg)
+	require.Len(t, filteredEntries, 2)
+	require.Equal(t, "test counter", filteredEntries[0].Data["name"])
+	require.Equal(t, "test_counter_#", filteredEntries[1].Data["name"])
+}
+
 func TestRunTags(t *testing.T) {
 	t.Parallel()
 

examples/custom_metrics.js

@@ -15,28 +15,28 @@ import { check } from "k6";
  *          on it
  */
 
-let myCounter = new Counter("my_counter");
-let myGauge = new Gauge("my_gauge");
+let myCounter = new Counter("my_counter asdad");
+let myGauge = new Gauge("my_gauge/");
 let myRate = new Rate("my_rate");
 let myTrend = new Trend("my_trend");
 
 let maxResponseTime = 0.0;
 
-export default function () {
-    let res = http.get("http://httpbin.org/");
-    let passed = check(res, { "status is 200": (r) => r.status === 200 });
+export default function() {
+	let res = http.get("http://httpbin.org/");
+	let passed = check(res, { "status is 200": (r) => r.status === 200 });
 
-    // Add one for number of requests
-    myCounter.add(1);
-    console.log(myCounter.name, " is config ready")
+	// Add one for number of requests
+	myCounter.add(1);
+	console.log(myCounter.name, " is config ready");
 
-    // Set max response time seen
-    maxResponseTime = Math.max(maxResponseTime, res.timings.duration);
-    myGauge.add(maxResponseTime);
+	// Set max response time seen
+	maxResponseTime = Math.max(maxResponseTime, res.timings.duration);
+	myGauge.add(maxResponseTime);
 
-    // Add check success or failure to keep track of rate
-    myRate.add(passed);
+	// Add check success or failure to keep track of rate
+	myRate.add(passed);
 
-    // Keep track of TCP-connecting and TLS handshaking part of the response time
-    myTrend.add(res.timings.connecting + res.timings.tls_handshaking);
+	// Keep track of TCP-connecting and TLS handshaking part of the response time
+	myTrend.add(res.timings.connecting + res.timings.tls_handshaking);
 }

js/modules/k6/metrics/metrics.go

@@ -4,8 +4,10 @@ import (
 	"errors"
 	"fmt"
 	"math"
+	"regexp"
 	"strconv"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/dop251/goja"
@@ -23,6 +25,19 @@ type Metric struct {
 // ErrMetricsAddInInitContext is error returned when adding to metric is done in the init context
 var ErrMetricsAddInInitContext = common.NewInitContextError("Adding to metrics in the init context is not supported")
 
+// TODO: this is temporary while this is just a warning. To be moved to the metrics' registry when it becomes an error.
+const (
+	nameRegexString = "^[a-zA-Z_][a-zA-Z0-9_]{1,128}$"
+	badNameWarning  = "Metric name should only include ASCII letters, numbers and underscores. " +
+		"This name will stop working in k6 v0.49.0."
+)
+
+var compileNameRegex = regexp.MustCompile(nameRegexString)
+
+func checkName(name string) bool {
+	return compileNameRegex.Match([]byte(name))
+}
+
 func (mi *ModuleInstance) newMetric(call goja.ConstructorCall, t metrics.MetricType) (*goja.Object, error) {
 	initEnv := mi.vu.InitEnv()
 	if initEnv == nil {
@@ -38,6 +53,9 @@ func (mi *ModuleInstance) newMetric(call goja.ConstructorCall, t metrics.MetricT
 		if err != nil {
 			return nil, err
 		}
+		if !checkName(name) && mi.rm.observeBadName(name) { // TODO drop this when it goes in Registry.NewMetric
+			initEnv.Logger.WithField("name", name).Warn(badNameWarning)
+		}
 		metric := &Metric{metric: m, vu: mi.vu}
 		o := rt.NewObject()
 		err = o.DefineDataProperty("name", rt.ToValue(name), goja.FLAG_FALSE, goja.FLAG_FALSE, goja.FLAG_TRUE)
@@ -128,10 +146,15 @@ func (m Metric) add(v goja.Value, addTags goja.Value) (bool, error) {
 
 type (
 	// RootModule is the root metrics module
-	RootModule struct{}
+	RootModule struct {
+		// TODO all of this is temporary until the new restrictions become an error instead of a warning
+		observedBadNames     map[string]struct{}
+		observedBadNamesLock sync.Mutex
+	}
 	// ModuleInstance represents an instance of the metrics module
 	ModuleInstance struct {
 		vu modules.VU
+		rm *RootModule
 	}
 )
 
@@ -141,13 +164,27 @@ var (
 )
 
 // NewModuleInstance implements modules.Module interface
-func (*RootModule) NewModuleInstance(m modules.VU) modules.Instance {
-	return &ModuleInstance{vu: m}
+func (rm *RootModule) NewModuleInstance(m modules.VU) modules.Instance {
+	return &ModuleInstance{vu: m, rm: rm}
+}
+
+// obseves a name and returns true if it is a new one
+func (rm *RootModule) observeBadName(name string) bool {
+	rm.observedBadNamesLock.Lock()
+	defer rm.observedBadNamesLock.Unlock()
+	_, ok := rm.observedBadNames[name]
+	if ok {
+		return false
+	}
+	rm.observedBadNames[name] = struct{}{}
+	return true
 }
 
 // New returns a new RootModule.
 func New() *RootModule {
-	return &RootModule{}
+	return &RootModule{
+		observedBadNames: make(map[string]struct{}),
+	}
 }
 
 // Exports returns the exports of the metrics module

lib/testutils/logrus_hook.go

@@ -87,3 +87,15 @@ func LogContains(logEntries []logrus.Entry, expLevel logrus.Level, expContents s
 	}
 	return false
 }
+
+// FilterEntries is a helper function that checks the provided list of log entries
+// for a messages matching the provided level and contents and returns an array with only them.
+func FilterEntries(logEntries []logrus.Entry, expLevel logrus.Level, expContents string) []logrus.Entry {
+	filtered := make([]logrus.Entry, 0)
+	for _, entry := range logEntries {
+		if entry.Level == expLevel && strings.Contains(entry.Message, expContents) {
+			filtered = append(filtered, entry)
+		}
+	}
+	return filtered
+}