Merge #143728

143728: aggmetric: update hash key for cache child metric r=aa-joshi a=aa-joshi

This patch updates hash key generation for child metric with cache storage. It relies on `fnv` package to generate hash for given label values.

Epic: CRDB-43153
Part of: CRDB-48253
Release note: None

Co-authored-by: Akshay Joshi <[email protected]>

Author: craig[bot]

pkg/util/metric/aggmetric/agg_metric.go

@@ -9,6 +9,7 @@
 package aggmetric
 
 import (
+	"hash/fnv"
 	"strings"
 
 	"github.com/cockroachdb/cockroach/pkg/util/cache"
@@ -19,6 +20,8 @@ import (
 	io_prometheus_client "github.com/prometheus/client_model/go"
 )
 
+var delimiter = []byte{'_'}
+
 // Builder is used to ease constructing metrics with the same labels.
 type Builder struct {
 	labels []string
@@ -185,8 +188,13 @@ type labelValuesSlice []string
 
 func (lv *labelValuesSlice) labelValues() []string { return []string(*lv) }
 
-func metricKey(labels ...string) string {
-	return strings.Join(labels, ",")
+func metricKey(labels ...string) uint64 {
+	hash := fnv.New64a()
+	for _, label := range labels {
+		_, _ = hash.Write([]byte(label))
+		_, _ = hash.Write(delimiter)
+	}
+	return hash.Sum64()
 }
 
 type ChildrenStorage interface {
@@ -210,27 +218,27 @@ func (ucw *UnorderedCacheWrapper) GetChildMetric(e interface{}) ChildMetric {
 }
 
 func (ucw *UnorderedCacheWrapper) Get(labelVals ...string) (ChildMetric, bool) {
-	cacheKey := metricKey(labelVals...)
-	value, ok := ucw.cache.Get(cacheKey)
+	hashKey := metricKey(labelVals...)
+	value, ok := ucw.cache.Get(hashKey)
 	if !ok {
 		return nil, false
 	}
 	return value.(ChildMetric), ok
 }
 
 func (ucw *UnorderedCacheWrapper) Add(metric ChildMetric) {
-	lvs := metric.labelValues()
-	key := metricKey(lvs...)
-	if _, ok := ucw.cache.Get(key); ok {
+	labelValues := metric.labelValues()
+	hashKey := metricKey(labelValues...)
+	if _, ok := ucw.cache.Get(hashKey); ok {
 		panic(errors.AssertionFailedf("child %v already exists", metric.labelValues()))
 	}
-	ucw.cache.Add(key, metric)
+	ucw.cache.Add(hashKey, metric)
 }
 
 func (ucw *UnorderedCacheWrapper) Del(metric ChildMetric) {
-	cacheKey := metricKey(metric.labelValues()...)
-	if _, ok := ucw.Get(cacheKey); ok {
-		ucw.cache.Del(cacheKey)
+	hashKey := metricKey(metric.labelValues()...)
+	if _, ok := ucw.cache.Get(hashKey); ok {
+		ucw.cache.Del(hashKey)
 	}
 }
 

pkg/util/metric/aggmetric/agg_metric_test.go

@@ -318,6 +318,38 @@ func TestAggMetricClear(t *testing.T) {
 	})
 }
 
+func TestMetricKey(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+
+	defer leaktest.AfterTest(t)()
+
+	for _, tc := range []struct {
+		name              string
+		labelValues       []string
+		expectedHashValue uint64
+	}{
+		{
+			name:              "empty label values",
+			labelValues:       []string{},
+			expectedHashValue: 0xcbf29ce484222325,
+		},
+		{
+			name:              "single label value",
+			labelValues:       []string{"test_db"},
+			expectedHashValue: 0x7b629443ea81c091,
+		},
+		{
+			name:              "multiple label values",
+			labelValues:       []string{"test_db", "test_app", "test_tenant"},
+			expectedHashValue: 0xa1aaab8437836050,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			require.Equal(t, tc.expectedHashValue, metricKey(tc.labelValues...))
+		})
+	}
+}
+
 func WritePrometheusMetricsFunc(r *metric.Registry) func(t *testing.T) string {
 	writePrometheusMetrics := func(t *testing.T) string {
 		var in bytes.Buffer