From b8b4685a783ae20873ddbcfd471fbd654cb2e69b Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Tue, 1 Oct 2024 10:16:34 +0530 Subject: [PATCH 1/2] chore: Add client_hash metric attribute --- .../BuiltInOpenTelemetryMetricsProvider.java | 36 +++++++++- ...iltInOpenTelemetryMetricsProviderTest.java | 66 +++++++++++++++++++ ...OpenTelemetryBuiltInMetricsTracerTest.java | 2 +- 3 files changed, 101 insertions(+), 3 deletions(-) create mode 100644 google-cloud-spanner/src/test/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProviderTest.java diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java index ad177c1f36d..8db81758ff5 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java @@ -28,6 +28,8 @@ import com.google.cloud.opentelemetry.detection.AttributeKeys; import com.google.cloud.opentelemetry.detection.DetectedPlatform; import com.google.cloud.opentelemetry.detection.GCPPlatformDetector; +import com.google.common.hash.HashFunction; +import com.google.common.hash.Hashing; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.metrics.SdkMeterProvider; @@ -83,11 +85,41 @@ Map createClientAttributes(String projectId, String client_name) // TODO: Replace this with real value. clientAttributes.put(INSTANCE_CONFIG_ID_KEY.getKey(), "unknown"); clientAttributes.put(CLIENT_NAME_KEY.getKey(), client_name); - clientAttributes.put(CLIENT_UID_KEY.getKey(), getDefaultTaskValue()); - clientAttributes.put(CLIENT_HASH_KEY.getKey(), "cloud_spanner_client_raw_metrics"); + String clientUid = getDefaultTaskValue(); + clientAttributes.put(CLIENT_UID_KEY.getKey(), clientUid); + clientAttributes.put(CLIENT_HASH_KEY.getKey(), generateClientHash(clientUid)); return clientAttributes; } + // Returns a 6-digit zero-padded all lower case hexadecimal representation + // of hash of the accounting group. + // + // The hash utilizes the 10 most significant bits of the value returned by + // `Hashing.goodFastHash(64).hashBytes()`, so effectively the returned values are + // uniformly distributed in the range [000000, 0003ff]. + // + // 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. + static String generateClientHash(String clientUid) { + if (clientUid == null) { + return "000000"; + } + + HashFunction hashFunction = Hashing.goodFastHash(64); + Long hash = hashFunction.hashBytes(clientUid.getBytes()).asLong(); + // Don't change this value without reading above comment + int kPrefixLength = 10; + long shiftedValue = hash >>> (64 - kPrefixLength); + return String.format("%06x", shiftedValue); + } + static String detectClientLocation() { GCPPlatformDetector detector = GCPPlatformDetector.DEFAULT_INSTANCE; DetectedPlatform detectedPlatform = detector.detectPlatform(); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProviderTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProviderTest.java new file mode 100644 index 00000000000..613f1840cef --- /dev/null +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProviderTest.java @@ -0,0 +1,66 @@ +/* + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.spanner; + +import static com.google.common.truth.Truth.assertThat; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class BuiltInOpenTelemetryMetricsProviderTest { + + @Test + public void testGenerateClientHashWithSimpleUid() { + String clientUid = "testClient"; + verifyHash(BuiltInOpenTelemetryMetricsProvider.generateClientHash(clientUid)); + } + + @Test + public void testGenerateClientHashWithEmptyUid() { + String clientUid = ""; + verifyHash(BuiltInOpenTelemetryMetricsProvider.generateClientHash(clientUid)); + } + + @Test + public void testGenerateClientHashWithNullUid() { + String clientUid = null; + verifyHash(BuiltInOpenTelemetryMetricsProvider.generateClientHash(clientUid)); + } + + @Test + public void testGenerateClientHashWithLongUid() { + String clientUid = "aVeryLongUniqueClientIdentifierThatIsUnusuallyLong"; + verifyHash(BuiltInOpenTelemetryMetricsProvider.generateClientHash(clientUid)); + } + + @Test + public void testGenerateClientHashWithSpecialCharacters() { + String clientUid = "273d60f2-5604-42f1-b687-f5f1b975fd07@2316645@test#"; + verifyHash(BuiltInOpenTelemetryMetricsProvider.generateClientHash(clientUid)); + } + + private void verifyHash(String hash) { + // Check if the hash length is 6 + assertThat(hash.length()).isEqualTo(6); + // Check if the hash is in the range [000000, 0003ff] + long hashValue = Long.parseLong(hash, 16); // Convert hash from hex to decimal + assertThat(hashValue).isAtLeast(0); + assertThat(hashValue).isAtMost(0x3FF); + } +} diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java index 9e3b1bfcc83..d9586acc956 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java @@ -99,7 +99,7 @@ public static void setup() { BuiltInOpenTelemetryMetricsProvider.detectClientLocation()) .put(BuiltInMetricsConstant.CLIENT_NAME_KEY, client_name) .put(BuiltInMetricsConstant.CLIENT_UID_KEY, attributes.get("client_uid")) - .put(BuiltInMetricsConstant.CLIENT_HASH_KEY, "cloud_spanner_client_raw_metrics") + .put(BuiltInMetricsConstant.CLIENT_HASH_KEY, attributes.get("client_hash")) .build(); } From 4d7a45d5ffec163aabf48fec28a33cd038ee23e1 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Wed, 9 Oct 2024 16:42:52 +0530 Subject: [PATCH 2/2] review comments --- .../BuiltInOpenTelemetryMetricsProvider.java | 31 +++++++++---------- ...iltInOpenTelemetryMetricsProviderTest.java | 8 ++--- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java index 8db81758ff5..25624c7eabf 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java @@ -91,22 +91,21 @@ Map createClientAttributes(String projectId, String client_name) return clientAttributes; } - // Returns a 6-digit zero-padded all lower case hexadecimal representation - // of hash of the accounting group. - // - // The hash utilizes the 10 most significant bits of the value returned by - // `Hashing.goodFastHash(64).hashBytes()`, so effectively the returned values are - // uniformly distributed in the range [000000, 0003ff]. - // - // 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. + /** + * Generates a 6-digit zero-padded all lower case hexadecimal representation of hash of the + * accounting group. The hash utilizes the 10 most significant bits of the value returned by + * `Hashing.goodFastHash(64).hashBytes()`, so effectively the returned values are uniformly + * distributed in the range [000000, 0003ff]. + * + *

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. + * + * @return Returns a 6-digit zero-padded all lower case hexadecimal representation of hash of the + * accounting group. + */ static String generateClientHash(String clientUid) { if (clientUid == null) { return "000000"; diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProviderTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProviderTest.java index 613f1840cef..43fe97113d0 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProviderTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProviderTest.java @@ -16,7 +16,8 @@ package com.google.cloud.spanner; -import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import org.junit.Test; import org.junit.runner.RunWith; @@ -57,10 +58,9 @@ public void testGenerateClientHashWithSpecialCharacters() { private void verifyHash(String hash) { // Check if the hash length is 6 - assertThat(hash.length()).isEqualTo(6); + assertEquals(hash.length(), 6); // Check if the hash is in the range [000000, 0003ff] long hashValue = Long.parseLong(hash, 16); // Convert hash from hex to decimal - assertThat(hashValue).isAtLeast(0); - assertThat(hashValue).isAtMost(0x3FF); + assertTrue(hashValue >= 0 && hashValue <= 0x3FF); } }