improvement: Drop keyValue field in metrics.Tag; normalizeTag short-c…

…ircuits simple strings (#324)

metrics/benchmark_test.go

@@ -33,6 +33,19 @@ func BenchmarkNewTag(b *testing.B) {
 	}
 }
 
+func BenchmarkNormalizeTag(b *testing.B) {
+	for _, val := range []string{
+		"a❌Long❌Tag❌With❌Emoji❌Chars",
+		"a Long Tag With Space Chars",
+		"aLongTagValueWithUpperChars",
+		"alongtagvaluewithnospecials",
+		"UPPER",
+		"lower",
+	} {
+		b.Run(val, newTagBenchFunc("key", val))
+	}
+}
+
 func newTagBenchFunc(tagKey, tagValue string) func(*testing.B) {
 	return func(b *testing.B) {
 		b.ReportAllocs()

metrics/registry.go

@@ -251,7 +251,7 @@ func (r *rootRegistry) Subregistry(prefix string, tags ...Tag) Registry {
 	}
 	return &childRegistry{
 		prefix: prefix,
-		tags:   Tags(tags),
+		tags:   tags,
 		root:   r,
 	}
 }
@@ -385,14 +385,16 @@ func toMetricTagsID(name string, tags Tags) metricTagsID {
 	// calculate how large to make our byte buffer below
 	bufSize := len(name)
 	for _, t := range tags {
-		bufSize += len(t.keyValue) + 1 // 1 for separator
+		bufSize += len(t.key) + len(t.value) + 2 // 2 for separators
 	}
 	buf := strings.Builder{}
 	buf.Grow(bufSize)
 	_, _ = buf.WriteString(name)
 	for _, tag := range tags {
 		_, _ = buf.WriteRune('|')
-		_, _ = buf.WriteString(tag.keyValue)
+		_, _ = buf.WriteString(tag.key)
+		_, _ = buf.WriteRune(':')
+		_, _ = buf.WriteString(tag.value)
 	}
 	return metricTagsID(buf.String())
 }

metrics/sort_go121.go

@@ -20,11 +20,15 @@ func sortTags(tags Tags) {
 
 func compareTags(a, b Tag) int {
 	switch {
-	case a.keyValue > b.keyValue:
+	case a.key < b.key:
+		return -1
+	case a.key > b.key:
 		return 1
-	case a.keyValue == b.keyValue:
-		return 0
-	default:
+	case a.value < b.value:
 		return -1
+	case a.value > b.value:
+		return 1
+	default:
+		return 0
 	}
 }

metrics/tag.go

@@ -18,8 +18,6 @@ import (
 type Tag struct {
 	key   string
 	value string
-	// Store the concatenated key and value so we don't need to reconstruct it in String() (used in toMetricTagID)
-	keyValue string
 }
 
 func (t Tag) Key() string {
@@ -32,7 +30,7 @@ func (t Tag) Value() string {
 
 // The full representation of the tag, which is "key:value".
 func (t Tag) String() string {
-	return t.keyValue
+	return t.key + ":" + t.value
 }
 
 type Tags []Tag
@@ -61,7 +59,10 @@ func (t Tags) Len() int {
 }
 
 func (t Tags) Less(i, j int) bool {
-	return t[i].keyValue < t[j].keyValue
+	if t[i].key == t[j].key {
+		return t[i].value < t[j].value
+	}
+	return t[i].key < t[j].key
 }
 
 func (t Tags) Swap(i, j int) {
@@ -118,9 +119,8 @@ func newTag(k, v string) Tag {
 	normalizedKey := normalizeTag(k, validKeyChars)
 	normalizedValue := normalizeTag(v, validValueChars)
 	return Tag{
-		key:      normalizedKey,
-		value:    normalizedValue,
-		keyValue: normalizedKey + ":" + normalizedValue,
+		key:   normalizedKey,
+		value: normalizedValue,
 	}
 }
 
@@ -180,6 +180,17 @@ func init() {
 //
 // Note that this function does not impose the length restriction described above.
 func normalizeTag(in string, validChars [utf8.RuneSelf]bool) string {
+	foundSpecial := false
+	for _, r := range in {
+		if r >= utf8.RuneSelf || !validChars[r] {
+			foundSpecial = true
+			break
+		}
+	}
+	if !foundSpecial {
+		return in
+	}
+
 	var builder strings.Builder
 	builder.Grow(len(in))
 	for _, r := range in {

metrics/tag_test.go

@@ -16,9 +16,8 @@ func TestMustTag(t *testing.T) {
 		actual, err := NewTag("keyWithUpper", "valueWithUpper")
 		assert.NoError(t, err)
 		assert.Equal(t, Tag{
-			key:      "keywithupper",
-			value:    "valuewithupper",
-			keyValue: "keywithupper:valuewithupper",
+			key:   "keywithupper",
+			value: "valuewithupper",
 		},
 			actual)
 	})
@@ -27,9 +26,8 @@ func TestMustTag(t *testing.T) {
 		actual, err := NewTag("a_-./0", "a_-:./0")
 		assert.NoError(t, err)
 		assert.Equal(t, Tag{
-			key:      "a_-./0",
-			value:    "a_-:./0",
-			keyValue: "a_-./0:a_-:./0",
+			key:   "a_-./0",
+			value: "a_-:./0",
 		},
 			actual)
 	})
@@ -38,9 +36,8 @@ func TestMustTag(t *testing.T) {
 		actual, err := NewTag("a(❌)", "a(❌)")
 		assert.NoError(t, err)
 		assert.Equal(t, Tag{
-			key:      "a___",
-			value:    "a___",
-			keyValue: "a___:a___",
+			key:   "a___",
+			value: "a___",
 		},
 			actual)
 	})