From 4c98f7a56031c6dc42c16e62f38f959fe880903e Mon Sep 17 00:00:00 2001 From: rahul2393 Date: Fri, 11 Oct 2024 14:33:50 +0530 Subject: [PATCH] chore(spanner): generate client_hash metric attribute using FNV (#10983) --- spanner/metrics.go | 58 +++++++++++++++++++++++++++---------- spanner/metrics_test.go | 63 ++++++++++++++++++++++++++++++++++++----- 2 files changed, 99 insertions(+), 22 deletions(-) diff --git a/spanner/metrics.go b/spanner/metrics.go index b6a3d3d01063..083f2999ca59 100644 --- a/spanner/metrics.go +++ b/spanner/metrics.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "hash/fnv" "strings" "log" @@ -125,6 +126,47 @@ var ( return uuid.NewString() + "@" + strconv.FormatInt(int64(os.Getpid()), 10) + "@" + hostname, nil } + // generateClientHash generates a 6-digit zero-padded lowercase hexadecimal hash + // using the 10 most significant bits of a 64-bit hash value. + // + // The primary purpose of this function is to generate a hash value for the `client_hash` + // resource label using `client_uid` metric field. The range of values is chosen to be small + // enough to keep the cardinality of the Resource targets under control. Note: If at later time + // the range needs to be increased, it can be done by increasing the value of `kPrefixLength` to + // up to 24 bits without changing the format of the returned value. + generateClientHash = func(clientUID string) string { + if clientUID == "" { + return "000000" + } + + // Use FNV hash function to generate a 64-bit hash + hasher := fnv.New64() + hasher.Write([]byte(clientUID)) + hashValue := hasher.Sum64() + + // Extract the 10 most significant bits + // Shift right by 54 bits to get the 10 most significant bits + kPrefixLength := 10 + tenMostSignificantBits := hashValue >> (64 - kPrefixLength) + + // Format the result as a 6-digit zero-padded hexadecimal string + return fmt.Sprintf("%06x", tenMostSignificantBits) + } + + detectClientLocation = func(ctx context.Context) string { + resource, err := gcp.NewDetector().Detect(ctx) + if err != nil { + return "global" + } + for _, attr := range resource.Attributes() { + if attr.Key == semconv.CloudRegionKey { + return attr.Value.AsString() + } + } + // If region is not found, return global + return "global" + } + exporterOpts = []option.ClientOption{} ) @@ -151,20 +193,6 @@ type builtinMetricsTracerFactory struct { attemptCount metric.Int64Counter // Counter for the number of attempts. } -func detectClientLocation(ctx context.Context) string { - resource, err := gcp.NewDetector().Detect(ctx) - if err != nil { - return "global" - } - for _, attr := range resource.Attributes() { - if attr.Key == semconv.CloudRegionKey { - return attr.Value.AsString() - } - } - // If region is not found, return global - return "global" -} - func newBuiltinMetricsTracerFactory(ctx context.Context, dbpath string, metricsProvider metric.MeterProvider) (*builtinMetricsTracerFactory, error) { clientUID, err := generateClientUID() if err != nil { @@ -183,7 +211,7 @@ func newBuiltinMetricsTracerFactory(ctx context.Context, dbpath string, metricsP attribute.String(metricLabelKeyDatabase, database), attribute.String(metricLabelKeyClientUID, clientUID), attribute.String(metricLabelKeyClientName, clientName), - attribute.String(monitoredResLabelKeyClientHash, "cloud_spanner_client_raw_metrics"), + attribute.String(monitoredResLabelKeyClientHash, generateClientHash(clientUID)), // Skipping instance config until we have a way to get it attribute.String(monitoredResLabelKeyInstanceConfig, "unknown"), attribute.String(monitoredResLabelKeyLocation, detectClientLocation(ctx)), diff --git a/spanner/metrics_test.go b/spanner/metrics_test.go index fac8c8ed9601..e05cf8cba5be 100644 --- a/spanner/metrics_test.go +++ b/spanner/metrics_test.go @@ -18,6 +18,7 @@ package spanner import ( "context" + "fmt" "os" "sort" "testing" @@ -41,8 +42,6 @@ func TestNewBuiltinMetricsTracerFactory(t *testing.T) { os.Setenv("SPANNER_ENABLE_BUILTIN_METRICS", "true") defer os.Unsetenv("SPANNER_ENABLE_BUILTIN_METRICS") ctx := context.Background() - project := "test-project" - instance := "test-instance" clientUID := "test-uid" createSessionRPC := "Spanner.BatchCreateSessions" if isMultiplexEnabled { @@ -50,12 +49,12 @@ func TestNewBuiltinMetricsTracerFactory(t *testing.T) { } wantClientAttributes := []attribute.KeyValue{ - attribute.String(monitoredResLabelKeyProject, project), - attribute.String(monitoredResLabelKeyInstance, instance), + attribute.String(monitoredResLabelKeyProject, "[PROJECT]"), + attribute.String(monitoredResLabelKeyInstance, "[INSTANCE]"), attribute.String(metricLabelKeyDatabase, "[DATABASE]"), attribute.String(metricLabelKeyClientUID, clientUID), attribute.String(metricLabelKeyClientName, clientName), - attribute.String(monitoredResLabelKeyClientHash, "cloud_spanner_client_raw_metrics"), + attribute.String(monitoredResLabelKeyClientHash, "0000ed"), attribute.String(monitoredResLabelKeyInstanceConfig, "unknown"), attribute.String(monitoredResLabelKeyLocation, "global"), } @@ -74,11 +73,16 @@ func TestNewBuiltinMetricsTracerFactory(t *testing.T) { // return constant client UID instead of random, so that attributes can be compared origGenerateClientUID := generateClientUID + origDetectClientLocation := detectClientLocation generateClientUID = func() (string, error) { return clientUID, nil } + detectClientLocation = func(ctx context.Context) string { + return "global" + } defer func() { generateClientUID = origGenerateClientUID + detectClientLocation = origDetectClientLocation }() // Setup mock monitoring server @@ -153,8 +157,7 @@ func TestNewBuiltinMetricsTracerFactory(t *testing.T) { t.Errorf("builtinEnabled: got: %v, want: %v", client.metricsTracerFactory.enabled, test.wantBuiltinEnabled) } - if diff := testutil.Diff(client.metricsTracerFactory.clientAttributes, wantClientAttributes, - cmpopts.IgnoreUnexported(attribute.KeyValue{}, attribute.Value{})); diff != "" { + if diff := testutil.Diff(client.metricsTracerFactory.clientAttributes, wantClientAttributes, cmpopts.EquateComparable(attribute.KeyValue{}, attribute.Value{})); diff != "" { t.Errorf("clientAttributes: got=-, want=+ \n%v", diff) } @@ -235,3 +238,49 @@ func TestNewBuiltinMetricsTracerFactory(t *testing.T) { }) } } + +// TestGenerateClientHash tests the generateClientHash function. +func TestGenerateClientHash(t *testing.T) { + tests := []struct { + name string + clientUID string + expectedValue string + expectedLength int + expectedMaxValue int64 + }{ + {"Simple UID", "exampleUID", "00006b", 6, 0x3FF}, + {"Empty UID", "", "000000", 6, 0x3FF}, + {"Special Characters", "!@#$%^&*()", "000389", 6, 0x3FF}, + {"Very Long UID", "aVeryLongUniqueIdentifierThatExceedsNormalLength", "000125", 6, 0x3FF}, + {"Numeric UID", "1234567890", "00003e", 6, 0x3FF}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + hash := generateClientHash(tt.clientUID) + if hash != tt.expectedValue { + t.Errorf("expected hash value %s, got %s", tt.expectedValue, hash) + } + // Check if the hash length is 6 + if len(hash) != tt.expectedLength { + t.Errorf("expected hash length %d, got %d", tt.expectedLength, len(hash)) + } + + // Check if the hash is in the range [000000, 0003ff] + hashValue, err := parseHex(hash) + if err != nil { + t.Errorf("failed to parse hash: %v", err) + } + if hashValue < 0 || hashValue > tt.expectedMaxValue { + t.Errorf("expected hash value in range [0, %d], got %d", tt.expectedMaxValue, hashValue) + } + }) + } +} + +// parseHex converts a hexadecimal string to an int64. +func parseHex(hexStr string) (int64, error) { + var value int64 + _, err := fmt.Sscanf(hexStr, "%x", &value) + return value, err +}