Skip to content

Commit

Permalink
Merge 498222e into 24ae4d9
Browse files Browse the repository at this point in the history
  • Loading branch information
mstoykov committed Nov 15, 2023
2 parents 24ae4d9 + 498222e commit 3019269
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 37 deletions.
17 changes: 7 additions & 10 deletions cmd/tests/cmd_run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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) {
Expand Down
23 changes: 0 additions & 23 deletions js/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"fmt"
"net/url"
"path/filepath"
"regexp"
"runtime"

"github.com/dop251/goja"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -134,8 +113,6 @@ func newBundle(
return nil, err
}

bundle.checkMetricNamesForPrometheusCompatibility()

return bundle, nil
}

Expand Down
8 changes: 6 additions & 2 deletions metrics/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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]

Expand Down
4 changes: 2 additions & 2 deletions metrics/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down

0 comments on commit 3019269

Please sign in to comment.