Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from May 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 38 additions & 0 deletions cmd/tests/cmd_run_test.go
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
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
codebien marked this conversation as resolved.
Show resolved Hide resolved
// 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)
imiric marked this conversation as resolved.
Show resolved Hide resolved

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