Improve BuildMetricName

Signed-off-by: Karsten Jeschkies <[email protected]>

Author: jeschkies

Diff for: cmd/yace/scraper.go

@@ -38,7 +38,7 @@ type cachingFactory interface {
 	Clear()
 }
 
-func NewScraper(featureFlags []string) *scraper { //nolint:revive
+func NewScraper(featureFlags []string) *scraper {
 	s := &scraper{
 		registry:     atomic.Pointer[prometheus.Registry]{},
 		featureFlags: featureFlags,

Diff for: pkg/promutil/migrate.go

@@ -31,27 +31,38 @@ var Percentile = regexp.MustCompile(`^p(\d{1,2}(\.\d{0,2})?|100)$`)
 
 func BuildMetricName(namespace, metricName, statistic string) string {
 	sb := strings.Builder{}
-	promNs := PromString(strings.ToLower(namespace))
+
 	// Some namespaces have a leading forward slash like
-	// /aws/sagemaker/TrainingJobs, which gets converted to
-	// a leading _ by PromString().
-	promNs = strings.TrimPrefix(promNs, "_")
+	// /aws/sagemaker/TrainingJobs, which should be removed.
+	var promNs string
+	if strings.HasPrefix(namespace, "/") {
+		promNs = PromString(strings.ToLower(namespace[1:]))
+	} else {
+		promNs = PromString(strings.ToLower(namespace))
+	}
+
 	if !strings.HasPrefix(promNs, "aws") {
 		sb.WriteString("aws_")
 	}
 	sb.WriteString(promNs)
+
 	sb.WriteString("_")
+
 	promMetricName := PromString(metricName)
 	// Some metric names duplicate parts of the namespace as a prefix,
 	// For example, the `Glue` namespace metrics have names prefixed also by `glue``
+	skip := 0
 	for _, part := range strings.Split(promNs, "_") {
-		promMetricName = strings.TrimPrefix(promMetricName, part)
+		if strings.HasPrefix(promMetricName[skip:], part) {
+			skip = len(part)
+		}
 	}
-	promMetricName = strings.TrimPrefix(promMetricName, "_")
+	promMetricName = strings.TrimPrefix(promMetricName[skip:], "_")
+
 	sb.WriteString(promMetricName)
 	if statistic != "" {
 		sb.WriteString("_")
-		sb.WriteString(PromString(statistic))
+		PromStringToBuilder(statistic, &sb)
 	}
 	return sb.String()
 }

Diff for: pkg/promutil/migrate_test.go

@@ -1124,6 +1124,94 @@ func Benchmark_BuildMetrics(b *testing.B) {
 	require.Equal(b, expectedLabels, labels)
 }
 
+func TestBuildMetricName(t *testing.T) {
+	type testCase struct {
+		name      string
+		namespace string
+		metric    string
+		statistic string
+		expected  string
+	}
+
+	testCases := []testCase{
+		{
+			name:      "standard AWS namespace",
+			namespace: "AWS/ElastiCache",
+			metric:    "CPUUtilization",
+			statistic: "Average",
+			expected:  "aws_elasticache_cpuutilization_average",
+		},
+		{
+			name:      "nonstandard namespace with slashes",
+			namespace: "/aws/sagemaker/TrainingJobs",
+			metric:    "CPUUtilization",
+			statistic: "Average",
+			expected:  "aws_sagemaker_trainingjobs_cpuutilization_average",
+		},
+		{
+			name:      "metric name duplicating namespace",
+			namespace: "Glue",
+			metric:    "glue.driver.aggregate.bytesRead",
+			statistic: "Average",
+			expected:  "aws_glue_driver_aggregate_bytes_read_average",
+		},
+		{
+			name:      "metric name not duplicating namespace",
+			namespace: "Glue",
+			metric:    "aggregate.glue.jobs.bytesRead",
+			statistic: "Average",
+			expected:  "aws_glue_aggregate_glue_jobs_bytes_read_average",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			result := BuildMetricName(tc.namespace, tc.metric, tc.statistic)
+			require.Equal(t, tc.expected, result)
+		})
+	}
+}
+
+func Benchmark_BuildMetricName(b *testing.B) {
+	testCases := []struct {
+		namespace string
+		metric    string
+		statistic string
+	}{
+		{
+			namespace: "AWS/ElastiCache",
+			metric:    "CPUUtilization",
+			statistic: "Average",
+		},
+		{
+			namespace: "/aws/sagemaker/TrainingJobs",
+			metric:    "CPUUtilization",
+			statistic: "Average",
+		},
+		{
+			namespace: "Glue",
+			metric:    "glue.driver.aggregate.bytesRead",
+			statistic: "Average",
+		},
+		{
+			namespace: "Glue",
+			metric:    "aggregate.glue.jobs.bytesRead",
+			statistic: "Average",
+		},
+	}
+
+	for _, tc := range testCases {
+		testName := BuildMetricName(tc.namespace, tc.metric, tc.statistic)
+		b.ResetTimer()
+		b.ReportAllocs()
+		b.Run(testName, func(b *testing.B) {
+			for i := 0; i < b.N; i++ {
+				BuildMetricName(tc.namespace, tc.metric, tc.statistic)
+			}
+		})
+	}
+}
+
 // replaceNaNValues replaces any NaN floating-point values with a marker value (54321.0)
 // so that require.Equal() can compare them. By default, require.Equal() will fail if any
 // struct values are NaN because NaN != NaN

Diff for: pkg/promutil/prometheus.go

@@ -15,6 +15,7 @@ package promutil
 import (
 	"strings"
 	"time"
+	"unicode"
 
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/common/model"
@@ -178,8 +179,29 @@ func toConstMetrics(metrics []*PrometheusMetric) []prometheus.Metric {
 }
 
 func PromString(text string) string {
-	text = splitString(text)
-	return strings.ToLower(sanitize(text))
+	var buf strings.Builder
+	PromStringToBuilder(text, &buf)
+	return buf.String()
+}
+
+func PromStringToBuilder(text string, buf *strings.Builder) {
+	buf.Grow(len(text))
+
+	var prev rune
+	for _, c := range text {
+		switch c {
+		case ' ', ',', '\t', '/', '\\', '.', '-', ':', '=', '@', '<', '>', '(', ')', '“':
+			buf.WriteRune('_')
+		case '%':
+			buf.WriteString("_percent")
+		default:
+			if unicode.IsUpper(c) && (unicode.IsLower(prev) || unicode.IsDigit(prev)) {
+				buf.WriteRune('_')
+			}
+			buf.WriteRune(unicode.ToLower(c))
+		}
+		prev = c
+	}
 }
 
 func PromStringTag(text string, labelsSnakeCase bool) (bool, string) {