Speed up build metric name. (#1671)

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

Author: jeschkies

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) {
@@ -210,36 +232,3 @@ func sanitize(text string) string {
 	}
 	return string(b)
 }
-
-// splitString replaces consecutive occurrences of a lowercase and uppercase letter,
-// or a number and an upper case letter, by putting a dot between the two chars.
-//
-// This is an optimised version of the original implementation:
-//
-//	  var splitRegexp = regexp.MustCompile(`([a-z0-9])([A-Z])`)
-//
-//		func splitString(text string) string {
-//		  return splitRegexp.ReplaceAllString(text, `$1.$2`)
-//		}
-func splitString(text string) string {
-	sb := strings.Builder{}
-	sb.Grow(len(text) + 4) // make some room for replacements
-
-	i := 0
-	for i < len(text) {
-		c := text[i]
-		sb.WriteByte(c)
-		if (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') {
-			if i < (len(text) - 1) {
-				c = text[i+1]
-				if c >= 'A' && c <= 'Z' {
-					sb.WriteByte('.')
-					sb.WriteByte(c)
-					i++
-				}
-			}
-		}
-		i++
-	}
-	return sb.String()
-}

Diff for: pkg/promutil/prometheus_test.go

@@ -23,30 +23,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestSplitString(t *testing.T) {
-	testCases := []struct {
-		input  string
-		output string
-	}{
-		{
-			input:  "GlobalTopicCount",
-			output: "Global.Topic.Count",
-		},
-		{
-			input:  "CPUUtilization",
-			output: "CPUUtilization",
-		},
-		{
-			input:  "StatusCheckFailed_Instance",
-			output: "Status.Check.Failed_Instance",
-		},
-	}
-
-	for _, tc := range testCases {
-		assert.Equal(t, tc.output, splitString(tc.input))
-	}
-}
-
 func TestSanitize(t *testing.T) {
 	testCases := []struct {
 		input  string