Skip to content

Commit

Permalink
k6/metrics: Warn on metric names not compatible with Prometheus
Browse files Browse the repository at this point in the history
This adds a warning for custom metrics that are not Prometheus
compatible.

The warning is in the js/bundle mostly for convenience.

It is a place we already have a registry and a logger and we go through
only once.

As this is temporary warning until we enforce stricter metric names
I don't think it is good idea to refactor the registry to support this
for a little while.

Updates #3065
  • Loading branch information
mstoykov committed May 15, 2023
1 parent bbf0683 commit 1d9c60c
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 0 deletions.
38 changes: 38 additions & 0 deletions cmd/tests/cmd_run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1543,6 +1543,44 @@ 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 `
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_#")
}

func TestRunTags(t *testing.T) {
t.Parallel()

Expand Down
21 changes: 21 additions & 0 deletions js/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"net/url"
"path/filepath"
"regexp"
"runtime"

"github.com/dop251/goja"
Expand Down Expand Up @@ -39,6 +40,24 @@ type Bundle struct {
moduleResolver *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 (
nameRegexString = "^[a-zA-Z_][a-zA-Z0-9_]{1,63}$"
badNameWarning = "Metric name should only include ASCII letters, numbers and 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 @@ -107,6 +126,8 @@ func newBundle(
return nil, err
}

bundle.checkMetricNamesForPrometheusCompatibility()

return bundle, nil
}

Expand Down
12 changes: 12 additions & 0 deletions lib/testutils/logrus_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

0 comments on commit 1d9c60c

Please sign in to comment.