From cca5a814e123cc6c09732627ba2d17f949cf8156 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Wed, 20 Nov 2024 12:46:59 +0530 Subject: [PATCH 01/11] Adding gfe_latencies metric to built-in metrics --- .../cloud/spanner/BuiltInMetricsConstant.java | 19 +++- .../BuiltInOpenTelemetryMetricsProvider.java | 39 +++++--- .../BuiltInOpenTelemetryMetricsRecorder.java | 88 +++++++++++++++++++ .../google/cloud/spanner/SpannerOptions.java | 20 ++++- .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 2 + .../spanner/spi/v1/HeaderInterceptor.java | 24 ++++- .../spi/v1/SpannerInterceptorProvider.java | 21 ++++- ...OpenTelemetryBuiltInMetricsTracerTest.java | 2 +- 8 files changed, 191 insertions(+), 24 deletions(-) create mode 100644 google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsRecorder.java diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsConstant.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsConstant.java index 4f8b091d550..d0dbd1c3da2 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsConstant.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsConstant.java @@ -34,9 +34,9 @@ public class BuiltInMetricsConstant { public static final String METER_NAME = "spanner.googleapis.com/internal/client"; - public static final String GAX_METER_NAME = OpenTelemetryMetricsRecorder.GAX_METER_NAME; - + static final String SPANNER_METER_NAME = "spanner-java"; + static final String GFE_LATENCIES_NAME = "gfe_latencies"; static final String OPERATION_LATENCIES_NAME = "operation_latencies"; static final String ATTEMPT_LATENCIES_NAME = "attempt_latencies"; static final String OPERATION_LATENCY_NAME = "operation_latency"; @@ -114,6 +114,7 @@ static Map getAllViews() { ImmutableMap.Builder views = ImmutableMap.builder(); defineView( views, + BuiltInMetricsConstant.GAX_METER_NAME, BuiltInMetricsConstant.OPERATION_LATENCY_NAME, BuiltInMetricsConstant.OPERATION_LATENCIES_NAME, BuiltInMetricsConstant.AGGREGATION_WITH_MILLIS_HISTOGRAM, @@ -121,6 +122,7 @@ static Map getAllViews() { "ms"); defineView( views, + BuiltInMetricsConstant.GAX_METER_NAME, BuiltInMetricsConstant.ATTEMPT_LATENCY_NAME, BuiltInMetricsConstant.ATTEMPT_LATENCIES_NAME, BuiltInMetricsConstant.AGGREGATION_WITH_MILLIS_HISTOGRAM, @@ -128,6 +130,15 @@ static Map getAllViews() { "ms"); defineView( views, + BuiltInMetricsConstant.SPANNER_METER_NAME, + BuiltInMetricsConstant.GFE_LATENCIES_NAME, + BuiltInMetricsConstant.GFE_LATENCIES_NAME, + BuiltInMetricsConstant.AGGREGATION_WITH_MILLIS_HISTOGRAM, + InstrumentType.HISTOGRAM, + "ms"); + defineView( + views, + BuiltInMetricsConstant.GAX_METER_NAME, BuiltInMetricsConstant.OPERATION_COUNT_NAME, BuiltInMetricsConstant.OPERATION_COUNT_NAME, Aggregation.sum(), @@ -135,6 +146,7 @@ static Map getAllViews() { "1"); defineView( views, + BuiltInMetricsConstant.GAX_METER_NAME, BuiltInMetricsConstant.ATTEMPT_COUNT_NAME, BuiltInMetricsConstant.ATTEMPT_COUNT_NAME, Aggregation.sum(), @@ -145,6 +157,7 @@ static Map getAllViews() { private static void defineView( ImmutableMap.Builder viewMap, + String meterName, String metricName, String metricViewName, Aggregation aggregation, @@ -153,7 +166,7 @@ private static void defineView( InstrumentSelector selector = InstrumentSelector.builder() .setName(BuiltInMetricsConstant.METER_NAME + '/' + metricName) - .setMeterName(BuiltInMetricsConstant.GAX_METER_NAME) + .setMeterName(meterName) .setType(type) .setUnit(unit) .build(); 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 4aeb98987d1..db343665e1c 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.cache.Cache; +import com.google.common.cache.CacheBuilder; import com.google.common.hash.HashFunction; import com.google.common.hash.Hashing; import io.opentelemetry.api.OpenTelemetry; @@ -42,6 +44,7 @@ import java.util.HashMap; import java.util.Map; import java.util.UUID; +import java.util.concurrent.ExecutionException; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -57,6 +60,9 @@ final class BuiltInOpenTelemetryMetricsProvider { private OpenTelemetry openTelemetry; + private final Cache> clientAttributesCache = + CacheBuilder.newBuilder().maximumSize(1000).build(); + private BuiltInOpenTelemetryMetricsProvider() {} OpenTelemetry getOrCreateOpenTelemetry( @@ -81,16 +87,29 @@ OpenTelemetry getOrCreateOpenTelemetry( } } - Map createClientAttributes(String projectId, String client_name) { - Map clientAttributes = new HashMap<>(); - clientAttributes.put(LOCATION_ID_KEY.getKey(), detectClientLocation()); - clientAttributes.put(PROJECT_ID_KEY.getKey(), projectId); - clientAttributes.put(INSTANCE_CONFIG_ID_KEY.getKey(), "unknown"); - clientAttributes.put(CLIENT_NAME_KEY.getKey(), client_name); - String clientUid = getDefaultTaskValue(); - clientAttributes.put(CLIENT_UID_KEY.getKey(), clientUid); - clientAttributes.put(CLIENT_HASH_KEY.getKey(), generateClientHash(clientUid)); - return clientAttributes; + Map createOrGetClientAttributes(String projectId, String client_name) { + try { + String key = projectId + client_name; + return clientAttributesCache.get( + key, + () -> { + Map clientAttributes = new HashMap<>(); + clientAttributes.put(LOCATION_ID_KEY.getKey(), detectClientLocation()); + clientAttributes.put(PROJECT_ID_KEY.getKey(), projectId); + clientAttributes.put(INSTANCE_CONFIG_ID_KEY.getKey(), "unknown"); + clientAttributes.put(CLIENT_NAME_KEY.getKey(), client_name); + String clientUid = getDefaultTaskValue(); + clientAttributes.put(CLIENT_UID_KEY.getKey(), clientUid); + clientAttributes.put(CLIENT_HASH_KEY.getKey(), generateClientHash(clientUid)); + return clientAttributes; + }); + } catch (ExecutionException executionException) { + logger.log( + Level.WARNING, + "Unable to get Client Attributes for client side metrics, will skip exporting client side metrics", + executionException); + return null; + } } /** diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsRecorder.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsRecorder.java new file mode 100644 index 00000000000..d5c72506da9 --- /dev/null +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsRecorder.java @@ -0,0 +1,88 @@ +/* + * 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 com.google.api.gax.core.GaxProperties; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; +import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.common.AttributesBuilder; +import io.opentelemetry.api.metrics.DoubleHistogram; +import io.opentelemetry.api.metrics.Meter; +import java.util.HashMap; +import java.util.Map; + +/** OpenTelemetry implementation of recording built in metrics. */ +public class BuiltInOpenTelemetryMetricsRecorder { + + private final DoubleHistogram gfeLatencyRecorder; + private final Map attributes = new HashMap<>(); + + /** + * Creates the following instruments for the following metrics: + * + *
    + *
  • GFE Latency: Histogram + *
+ * + * @param openTelemetry OpenTelemetry instance + */ + public BuiltInOpenTelemetryMetricsRecorder( + OpenTelemetry openTelemetry, Map clientAttributes) { + if (openTelemetry != null && clientAttributes != null) { + gfeLatencyRecorder = null; + return; + } + Meter meter = + openTelemetry + .meterBuilder(BuiltInMetricsConstant.SPANNER_METER_NAME) + .setInstrumentationVersion(GaxProperties.getLibraryVersion(getClass())) + .build(); + this.gfeLatencyRecorder = + meter + .histogramBuilder( + BuiltInMetricsConstant.METER_NAME + '/' + BuiltInMetricsConstant.GFE_LATENCIES_NAME) + .setDescription( + "Latency between Google's network receiving an RPC and reading back the first byte of the response") + .setUnit("ms") + .build(); + this.attributes.putAll(clientAttributes); + } + + /** + * Record the latency between Google's network receiving an RPC and reading back the first byte of + * the response. Data is stored in a Histogram. + * + * @param gfeLatency Attempt Latency in ms + * @param attributes Map of the attributes to store + */ + public void recordGFELatency(double gfeLatency, Map attributes) { + if (gfeLatencyRecorder != null) { + this.attributes.putAll(attributes); + gfeLatencyRecorder.record(gfeLatency, toOtelAttributes(this.attributes)); + } + } + + @VisibleForTesting + Attributes toOtelAttributes(Map attributes) { + Preconditions.checkNotNull(attributes, "Attributes map cannot be null"); + AttributesBuilder attributesBuilder = Attributes.builder(); + attributes.forEach(attributesBuilder::put); + return attributesBuilder.build(); + } +} diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index 42fc0c2d0bd..da5b160e7dd 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -1864,6 +1864,18 @@ public OpenTelemetry getOpenTelemetry() { } } + /** Returns an instance of OpenTelemetry object for Built-in Client metrics. */ + public OpenTelemetry getBuiltInMetricsOpenTelemetry() { + return this.builtInOpenTelemetryMetricsProvider.getOrCreateOpenTelemetry( + this.getProjectId(), getCredentials()); + } + + /** Returns attributes for an instance of Built-in Client metrics. */ + public Map getBuiltInMetricsClientAttributes() { + return builtInOpenTelemetryMetricsProvider.createOrGetClientAttributes( + this.getProjectId(), "spanner-java/" + GaxProperties.getLibraryVersion(getClass())); + } + @Override public ApiTracerFactory getApiTracerFactory() { return createApiTracerFactory(false, false); @@ -1913,11 +1925,13 @@ private ApiTracerFactory createMetricsApiTracerFactory() { this.builtInOpenTelemetryMetricsProvider.getOrCreateOpenTelemetry( this.getProjectId(), getCredentials(), this.monitoringHost); - return openTelemetry != null + Map clientAttributes = + builtInOpenTelemetryMetricsProvider.createOrGetClientAttributes( + this.getProjectId(), "spanner-java/" + GaxProperties.getLibraryVersion(getClass())); + return openTelemetry != null && clientAttributes != null ? new MetricsTracerFactory( new OpenTelemetryMetricsRecorder(openTelemetry, BuiltInMetricsConstant.METER_NAME), - builtInOpenTelemetryMetricsProvider.createClientAttributes( - this.getProjectId(), "spanner-java/" + GaxProperties.getLibraryVersion(getClass()))) + clientAttributes) : null; } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java index 0e540ea7926..edd885d3c8e 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java @@ -357,6 +357,8 @@ public GapicSpannerRpc(final SpannerOptions options) { options.getInterceptorProvider(), SpannerInterceptorProvider.createDefault( options.getOpenTelemetry(), + options.getBuiltInMetricsOpenTelemetry(), + options.getBuiltInMetricsClientAttributes(), (() -> directPathEnabledSupplier.get())))) // This sets the trace context headers. .withTraceContext(endToEndTracingEnabled, options.getOpenTelemetry()) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java index e4eec68b278..f5afc699a93 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java @@ -25,6 +25,7 @@ import com.google.api.gax.tracing.ApiTracer; import com.google.cloud.spanner.BuiltInMetricsConstant; +import com.google.cloud.spanner.BuiltInOpenTelemetryMetricsRecorder; import com.google.cloud.spanner.CompositeTracer; import com.google.cloud.spanner.SpannerExceptionFactory; import com.google.cloud.spanner.SpannerRpcMetrics; @@ -94,12 +95,17 @@ class HeaderInterceptor implements ClientInterceptor { private static final Level LEVEL = Level.INFO; private final SpannerRpcMetrics spannerRpcMetrics; + private final BuiltInOpenTelemetryMetricsRecorder builtInOpenTelemetryMetricsRecorder; + private final Supplier directPathEnabledSupplier; HeaderInterceptor( - SpannerRpcMetrics spannerRpcMetrics, Supplier directPathEnabledSupplier) { + SpannerRpcMetrics spannerRpcMetrics, + BuiltInOpenTelemetryMetricsRecorder builtInOpenTelemetryMetricsRecorder, + Supplier directPathEnabledSupplier) { this.spannerRpcMetrics = spannerRpcMetrics; this.directPathEnabledSupplier = directPathEnabledSupplier; + this.builtInOpenTelemetryMetricsRecorder = builtInOpenTelemetryMetricsRecorder; } @Override @@ -128,7 +134,12 @@ public void onHeaders(Metadata metadata) { Boolean isDirectPathUsed = isDirectPathUsed(getAttributes().get(Grpc.TRANSPORT_ATTR_REMOTE_ADDR)); addDirectPathUsedAttribute(compositeTracer, isDirectPathUsed); - processHeader(metadata, tagContext, attributes, span); + processHeader( + metadata, + tagContext, + attributes, + span, + builtInMetricsAttributes , isDirectPathUsed); super.onHeaders(metadata); } }, @@ -142,7 +153,12 @@ public void onHeaders(Metadata metadata) { } private void processHeader( - Metadata metadata, TagContext tagContext, Attributes attributes, Span span) { + Metadata metadata, + TagContext tagContext, + Attributes attributes, + Span span, + Map builtInMetricsAttributes, + Boolean isDirectPathUsed) { MeasureMap measureMap = STATS_RECORDER.newMeasureMap(); String serverTiming = metadata.get(SERVER_TIMING_HEADER_KEY); if (serverTiming != null && serverTiming.startsWith(SERVER_TIMING_HEADER_PREFIX)) { @@ -154,6 +170,8 @@ private void processHeader( spannerRpcMetrics.recordGfeLatency(latency, attributes); spannerRpcMetrics.recordGfeHeaderMissingCount(0L, attributes); + // TODO: Also pass directpath used + builtInOpenTelemetryMetricsRecorder.recordGFELatency(latency, builtInMetricsAttributes); if (span != null) { span.setAttribute("gfe_latency", String.valueOf(latency)); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerInterceptorProvider.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerInterceptorProvider.java index c3c05b8af15..cbb9b62bdd2 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerInterceptorProvider.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerInterceptorProvider.java @@ -18,6 +18,7 @@ import com.google.api.core.InternalApi; import com.google.api.core.ObsoleteApi; import com.google.api.gax.grpc.GrpcInterceptorProvider; +import com.google.cloud.spanner.BuiltInOpenTelemetryMetricsRecorder; import com.google.cloud.spanner.SpannerRpcMetrics; import com.google.common.base.Supplier; import com.google.common.base.Suppliers; @@ -26,7 +27,9 @@ import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.api.OpenTelemetry; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.logging.Level; import java.util.logging.Logger; @@ -44,12 +47,15 @@ private SpannerInterceptorProvider(List clientInterceptors) { @ObsoleteApi("This method always uses Global OpenTelemetry") public static SpannerInterceptorProvider createDefault() { - return createDefault(GlobalOpenTelemetry.get()); + return createDefault(GlobalOpenTelemetry.get(), GlobalOpenTelemetry.get()); } - public static SpannerInterceptorProvider createDefault(OpenTelemetry openTelemetry) { + public static SpannerInterceptorProvider createDefault( + OpenTelemetry openTelemetry, OpenTelemetry builtInMetricsopenTelemetry) { return createDefault( openTelemetry, + builtInMetricsopenTelemetry, + new HashMap<>(), Suppliers.memoize( () -> { return false; @@ -57,13 +63,20 @@ public static SpannerInterceptorProvider createDefault(OpenTelemetry openTelemet } public static SpannerInterceptorProvider createDefault( - OpenTelemetry openTelemetry, Supplier directPathEnabledSupplier) { + OpenTelemetry openTelemetry, + OpenTelemetry builtInMetricsopenTelemetry, + Map builtInMetricsClientAttributes, + Supplier directPathEnabledSupplier) { List defaultInterceptorList = new ArrayList<>(); defaultInterceptorList.add(new SpannerErrorInterceptor()); defaultInterceptorList.add( new LoggingInterceptor(Logger.getLogger(GapicSpannerRpc.class.getName()), Level.FINER)); defaultInterceptorList.add( - new HeaderInterceptor(new SpannerRpcMetrics(openTelemetry), directPathEnabledSupplier)); + new HeaderInterceptor( + new SpannerRpcMetrics(openTelemetry), + new BuiltInOpenTelemetryMetricsRecorder( + builtInMetricsopenTelemetry, builtInMetricsClientAttributes), + directPathEnabledSupplier)); return new SpannerInterceptorProvider(ImmutableList.copyOf(defaultInterceptorList)); } 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 1b6d99260fe..9c92c62365a 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 @@ -90,7 +90,7 @@ public static void setup() { String client_name = "spanner-java/"; openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(meterProvider.build()).build(); - attributes = provider.createClientAttributes("test-project", client_name); + attributes = provider.createOrGetClientAttributes("test-project", client_name); expectedBaseAttributes = Attributes.builder() From cfb04045083252438d8d6c813971b9aeae879f33 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Fri, 3 Jan 2025 12:40:22 +0530 Subject: [PATCH 02/11] Refactor Header Interceptor and modified the server timing header logic --- .../BuiltInOpenTelemetryMetricsProvider.java | 109 +++++++++++------- .../BuiltInOpenTelemetryMetricsRecorder.java | 5 +- .../google/cloud/spanner/SpannerOptions.java | 28 ++--- .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 4 +- .../spanner/spi/v1/HeaderInterceptor.java | 63 +++++++--- .../spi/v1/SpannerInterceptorProvider.java | 17 ++- ...OpenTelemetryBuiltInMetricsTracerTest.java | 4 +- 7 files changed, 143 insertions(+), 87 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 db343665e1c..e7879586917 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,7 @@ 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.annotations.VisibleForTesting; import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; import com.google.common.hash.HashFunction; @@ -44,72 +45,96 @@ import java.util.HashMap; import java.util.Map; import java.util.UUID; -import java.util.concurrent.ExecutionException; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nullable; final class BuiltInOpenTelemetryMetricsProvider { - static BuiltInOpenTelemetryMetricsProvider INSTANCE = new BuiltInOpenTelemetryMetricsProvider(); + public static BuiltInOpenTelemetryMetricsProvider INSTANCE = + new BuiltInOpenTelemetryMetricsProvider(); private static final Logger logger = Logger.getLogger(BuiltInOpenTelemetryMetricsProvider.class.getName()); + private final Cache> clientAttributesCache = + CacheBuilder.newBuilder().maximumSize(1000).build(); + private static String taskId; private OpenTelemetry openTelemetry; - private final Cache> clientAttributesCache = - CacheBuilder.newBuilder().maximumSize(1000).build(); + private Map clientAttributes; + + private boolean isInitialized; - private BuiltInOpenTelemetryMetricsProvider() {} + private BuiltInOpenTelemetryMetricsRecorder builtInOpenTelemetryMetricsRecorder; + + private BuiltInOpenTelemetryMetricsProvider() {}; + + void initialize( + String projectId, + String client_name, + @Nullable Credentials credentials, + @Nullable String monitoringHost) { - OpenTelemetry getOrCreateOpenTelemetry( - String projectId, @Nullable Credentials credentials, @Nullable String monitoringHost) { try { - if (this.openTelemetry == null) { - SdkMeterProviderBuilder sdkMeterProviderBuilder = SdkMeterProvider.builder(); - BuiltInOpenTelemetryMetricsView.registerBuiltinMetrics( - SpannerCloudMonitoringExporter.create(projectId, credentials, monitoringHost), - sdkMeterProviderBuilder); - SdkMeterProvider sdkMeterProvider = sdkMeterProviderBuilder.build(); - this.openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProvider).build(); - Runtime.getRuntime().addShutdownHook(new Thread(sdkMeterProvider::close)); + if (!isInitialized) { + this.openTelemetry = createOpenTelemetry(projectId, credentials, monitoringHost); + this.clientAttributes = createClientAttributes(projectId, client_name); + this.builtInOpenTelemetryMetricsRecorder = + new BuiltInOpenTelemetryMetricsRecorder(openTelemetry, clientAttributes); + isInitialized = true; } - return this.openTelemetry; - } catch (IOException ex) { + } catch (Exception ex) { logger.log( Level.WARNING, - "Unable to get OpenTelemetry object for client side metrics, will skip exporting client side metrics", + "Unable to initialize OpenTelemetry object or attributes for client side metrics, will skip exporting client side metrics", ex); - return null; } } - Map createOrGetClientAttributes(String projectId, String client_name) { - try { - String key = projectId + client_name; - return clientAttributesCache.get( - key, - () -> { - Map clientAttributes = new HashMap<>(); - clientAttributes.put(LOCATION_ID_KEY.getKey(), detectClientLocation()); - clientAttributes.put(PROJECT_ID_KEY.getKey(), projectId); - clientAttributes.put(INSTANCE_CONFIG_ID_KEY.getKey(), "unknown"); - clientAttributes.put(CLIENT_NAME_KEY.getKey(), client_name); - String clientUid = getDefaultTaskValue(); - clientAttributes.put(CLIENT_UID_KEY.getKey(), clientUid); - clientAttributes.put(CLIENT_HASH_KEY.getKey(), generateClientHash(clientUid)); - return clientAttributes; - }); - } catch (ExecutionException executionException) { - logger.log( - Level.WARNING, - "Unable to get Client Attributes for client side metrics, will skip exporting client side metrics", - executionException); - return null; - } + OpenTelemetry getOpenTelemetry() { + return this.openTelemetry; + } + + Map getClientAttributes() { + return this.clientAttributes; + } + + BuiltInOpenTelemetryMetricsRecorder getBuiltInOpenTelemetryMetricsRecorder() { + return this.builtInOpenTelemetryMetricsRecorder; + } + + @VisibleForTesting + void reset() { + isInitialized = false; + } + + private Map createClientAttributes(String projectId, String client_name) { + Map clientAttributes = new HashMap<>(); + clientAttributes.put(LOCATION_ID_KEY.getKey(), detectClientLocation()); + clientAttributes.put(PROJECT_ID_KEY.getKey(), projectId); + clientAttributes.put(INSTANCE_CONFIG_ID_KEY.getKey(), "unknown"); + clientAttributes.put(CLIENT_NAME_KEY.getKey(), client_name); + String clientUid = getDefaultTaskValue(); + clientAttributes.put(CLIENT_UID_KEY.getKey(), clientUid); + clientAttributes.put(CLIENT_HASH_KEY.getKey(), generateClientHash(clientUid)); + return clientAttributes; + } + + private OpenTelemetry createOpenTelemetry( + String projectId, @Nullable Credentials credentials, @Nullable String monitoringHost) + throws IOException { + OpenTelemetry openTelemetry; + SdkMeterProviderBuilder sdkMeterProviderBuilder = SdkMeterProvider.builder(); + BuiltInOpenTelemetryMetricsView.registerBuiltinMetrics( + SpannerCloudMonitoringExporter.create(projectId, credentials, monitoringHost), + sdkMeterProviderBuilder); + SdkMeterProvider sdkMeterProvider = sdkMeterProviderBuilder.build(); + openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProvider).build(); + Runtime.getRuntime().addShutdownHook(new Thread(sdkMeterProvider::close)); + return openTelemetry; } /** diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsRecorder.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsRecorder.java index d5c72506da9..416dbc3a639 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsRecorder.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsRecorder.java @@ -1,5 +1,5 @@ /* - * Copyright 2024 Google LLC + * Copyright 2025 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -44,10 +44,11 @@ public class BuiltInOpenTelemetryMetricsRecorder { */ public BuiltInOpenTelemetryMetricsRecorder( OpenTelemetry openTelemetry, Map clientAttributes) { - if (openTelemetry != null && clientAttributes != null) { + if (openTelemetry == null || clientAttributes == null) { gfeLatencyRecorder = null; return; } + Meter meter = openTelemetry .meterBuilder(BuiltInMetricsConstant.SPANNER_METER_NAME) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index da5b160e7dd..e6eb8652f71 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -1864,16 +1864,21 @@ public OpenTelemetry getOpenTelemetry() { } } - /** Returns an instance of OpenTelemetry object for Built-in Client metrics. */ - public OpenTelemetry getBuiltInMetricsOpenTelemetry() { - return this.builtInOpenTelemetryMetricsProvider.getOrCreateOpenTelemetry( - this.getProjectId(), getCredentials()); + /** + * Returns an instance of Built-In MetricsRecorder object. initializeBuiltInMetrics should be + * called first before this recorder can be fetched + */ + public BuiltInOpenTelemetryMetricsRecorder getBuiltInMetricsRecorder() { + return this.builtInOpenTelemetryMetricsProvider.getBuiltInOpenTelemetryMetricsRecorder(); } - /** Returns attributes for an instance of Built-in Client metrics. */ - public Map getBuiltInMetricsClientAttributes() { - return builtInOpenTelemetryMetricsProvider.createOrGetClientAttributes( - this.getProjectId(), "spanner-java/" + GaxProperties.getLibraryVersion(getClass())); + /** Initialize the built-in metrics provider */ + public void initializeBuiltInMetrics() { + this.builtInOpenTelemetryMetricsProvider.initialize( + this.getProjectId(), + "spanner-java/" + GaxProperties.getLibraryVersion(getClass()), + getCredentials(), + this.getMonitoringHost()); } @Override @@ -1921,13 +1926,10 @@ private ApiTracerFactory getDefaultApiTracerFactory() { } private ApiTracerFactory createMetricsApiTracerFactory() { - OpenTelemetry openTelemetry = - this.builtInOpenTelemetryMetricsProvider.getOrCreateOpenTelemetry( - this.getProjectId(), getCredentials(), this.monitoringHost); + OpenTelemetry openTelemetry = this.builtInOpenTelemetryMetricsProvider.getOpenTelemetry(); Map clientAttributes = - builtInOpenTelemetryMetricsProvider.createOrGetClientAttributes( - this.getProjectId(), "spanner-java/" + GaxProperties.getLibraryVersion(getClass())); + builtInOpenTelemetryMetricsProvider.getClientAttributes(); return openTelemetry != null && clientAttributes != null ? new MetricsTracerFactory( new OpenTelemetryMetricsRecorder(openTelemetry, BuiltInMetricsConstant.METER_NAME), diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java index edd885d3c8e..83955fcfbf9 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java @@ -333,6 +333,7 @@ public GapicSpannerRpc(final SpannerOptions options) { this.endToEndTracingEnabled = options.isEndToEndTracingEnabled(); this.numChannels = options.getNumChannels(); this.isGrpcGcpExtensionEnabled = options.isGrpcGcpExtensionEnabled(); + options.initializeBuiltInMetrics(); if (initializeStubs) { // First check if SpannerOptions provides a TransportChannelProvider. Create one @@ -357,8 +358,7 @@ public GapicSpannerRpc(final SpannerOptions options) { options.getInterceptorProvider(), SpannerInterceptorProvider.createDefault( options.getOpenTelemetry(), - options.getBuiltInMetricsOpenTelemetry(), - options.getBuiltInMetricsClientAttributes(), + options.getBuiltInMetricsRecorder(), (() -> directPathEnabledSupplier.get())))) // This sets the trace context headers. .withTraceContext(endToEndTracingEnabled, options.getOpenTelemetry()) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java index f5afc699a93..37abcfd927d 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java @@ -72,9 +72,11 @@ class HeaderInterceptor implements ClientInterceptor { DatabaseName.of("undefined-project", "undefined-instance", "undefined-database"); private static final Metadata.Key SERVER_TIMING_HEADER_KEY = Metadata.Key.of("server-timing", Metadata.ASCII_STRING_MARSHALLER); - private static final String SERVER_TIMING_HEADER_PREFIX = "gfet4t7; dur="; + private static final String GFE_TIMING_HEADER = "gfet4t7"; private static final Metadata.Key GOOGLE_CLOUD_RESOURCE_PREFIX_KEY = Metadata.Key.of("google-cloud-resource-prefix", Metadata.ASCII_STRING_MARSHALLER); + private static final Pattern SERVER_TIMING_PATTERN = + Pattern.compile("(?[a-zA-Z0-9_-]+);\\s*dur=(?\\d+)"); private static final Pattern GOOGLE_CLOUD_RESOURCE_PREFIX_PATTERN = Pattern.compile( ".*projects/(?\\p{ASCII}[^/]*)(/instances/(?\\p{ASCII}[^/]*))?(/databases/(?\\p{ASCII}[^/]*))?"); @@ -136,8 +138,8 @@ public void onHeaders(Metadata metadata) { addDirectPathUsedAttribute(compositeTracer, isDirectPathUsed); processHeader( metadata, - tagContext, - attributes, + openCensusTagContext, + customMetricAttributes, span, builtInMetricsAttributes , isDirectPathUsed); super.onHeaders(metadata); @@ -152,7 +154,7 @@ public void onHeaders(Metadata metadata) { }; } - private void processHeader( + private void processServerTimingHeader( Metadata metadata, TagContext tagContext, Attributes attributes, @@ -161,30 +163,55 @@ private void processHeader( Boolean isDirectPathUsed) { MeasureMap measureMap = STATS_RECORDER.newMeasureMap(); String serverTiming = metadata.get(SERVER_TIMING_HEADER_KEY); - if (serverTiming != null && serverTiming.startsWith(SERVER_TIMING_HEADER_PREFIX)) { - try { - long latency = Long.parseLong(serverTiming.substring(SERVER_TIMING_HEADER_PREFIX.length())); - measureMap.put(SPANNER_GFE_LATENCY, latency); + try { + // Previous implementation parsed the GFE latency directly using: + // long latency = Long.parseLong(serverTiming.substring("gfet4t7; dur=".length())); + // This approach assumed the serverTiming header contained exactly one metric "gfet4t7". + // If additional metrics were introduced in the header, older versions of the library + // would fail to parse it correctly. To make the parsing more robust, the logic has been + // updated to handle multiple metrics gracefully. + + Map serverTimingMetrics = parseServerTimingHeader(serverTiming); + if (serverTimingMetrics.containsKey(GFE_TIMING_HEADER)) { + long gfeLatency = serverTimingMetrics.get(GFE_TIMING_HEADER); + + measureMap.put(SPANNER_GFE_LATENCY, gfeLatency); measureMap.put(SPANNER_GFE_HEADER_MISSING_COUNT, 0L); measureMap.record(tagContext); - spannerRpcMetrics.recordGfeLatency(latency, attributes); + spannerRpcMetrics.recordGfeLatency(gfeLatency, attributes); spannerRpcMetrics.recordGfeHeaderMissingCount(0L, attributes); // TODO: Also pass directpath used builtInOpenTelemetryMetricsRecorder.recordGFELatency(latency, builtInMetricsAttributes); if (span != null) { - span.setAttribute("gfe_latency", String.valueOf(latency)); + span.setAttribute("gfe_latency", String.valueOf(gfeLatency)); } - } catch (NumberFormatException e) { - LOGGER.log(LEVEL, "Invalid server-timing object in header: {}", serverTiming); + } else { + measureMap.put(SPANNER_GFE_HEADER_MISSING_COUNT, 1L).record(tagContext); + spannerRpcMetrics.recordGfeHeaderMissingCount(1L, attributes); } - } else { - spannerRpcMetrics.recordGfeHeaderMissingCount(1L, attributes); - measureMap.put(SPANNER_GFE_HEADER_MISSING_COUNT, 1L).record(tagContext); + } catch (NumberFormatException e) { + LOGGER.log(LEVEL, "Invalid server-timing object in header: {}", serverTiming); } } + private Map parseServerTimingHeader(String serverTiming) { + Map serverTimingMetrics = new HashMap<>(); + if (serverTiming != null) { + Matcher matcher = SERVER_TIMING_PATTERN.matcher(serverTiming); + while (matcher.find()) { + String metricName = matcher.group("metricName"); + String durationStr = matcher.group("duration"); + + if (metricName != null && durationStr != null) { + serverTimingMetrics.put(metricName, Long.valueOf(durationStr)); + } + } + } + return serverTimingMetrics; + } + private DatabaseName extractDatabaseName(Metadata headers) throws ExecutionException { String googleResourcePrefix = headers.get(GOOGLE_CLOUD_RESOURCE_PREFIX_KEY); if (googleResourcePrefix != null) { @@ -213,7 +240,7 @@ private DatabaseName extractDatabaseName(Metadata headers) throws ExecutionExcep return UNDEFINED_DATABASE_NAME; } - private TagContext getTagContext(String key, String method, DatabaseName databaseName) + private TagContext getOpenCensusTagContext(String key, String method, DatabaseName databaseName) throws ExecutionException { return tagsCache.get( key, @@ -227,8 +254,8 @@ private TagContext getTagContext(String key, String method, DatabaseName databas .build()); } - private Attributes getMetricAttributes(String key, String method, DatabaseName databaseName) - throws ExecutionException { + private Attributes buildCustomMetricAttributes( + String key, String method, DatabaseName databaseName) throws ExecutionException { return attributesCache.get( key, () -> { diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerInterceptorProvider.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerInterceptorProvider.java index cbb9b62bdd2..6f43954bf32 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerInterceptorProvider.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerInterceptorProvider.java @@ -29,7 +29,6 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.logging.Level; import java.util.logging.Logger; @@ -47,15 +46,17 @@ private SpannerInterceptorProvider(List clientInterceptors) { @ObsoleteApi("This method always uses Global OpenTelemetry") public static SpannerInterceptorProvider createDefault() { - return createDefault(GlobalOpenTelemetry.get(), GlobalOpenTelemetry.get()); + return createDefault( + GlobalOpenTelemetry.get(), + new BuiltInOpenTelemetryMetricsRecorder(GlobalOpenTelemetry.get(), new HashMap<>())); } public static SpannerInterceptorProvider createDefault( - OpenTelemetry openTelemetry, OpenTelemetry builtInMetricsopenTelemetry) { + OpenTelemetry openTelemetry, + BuiltInOpenTelemetryMetricsRecorder builtInOpenTelemetryMetricsRecorder) { return createDefault( openTelemetry, - builtInMetricsopenTelemetry, - new HashMap<>(), + builtInOpenTelemetryMetricsRecorder, Suppliers.memoize( () -> { return false; @@ -64,8 +65,7 @@ public static SpannerInterceptorProvider createDefault( public static SpannerInterceptorProvider createDefault( OpenTelemetry openTelemetry, - OpenTelemetry builtInMetricsopenTelemetry, - Map builtInMetricsClientAttributes, + BuiltInOpenTelemetryMetricsRecorder builtInOpenTelemetryMetricsRecorder, Supplier directPathEnabledSupplier) { List defaultInterceptorList = new ArrayList<>(); defaultInterceptorList.add(new SpannerErrorInterceptor()); @@ -74,8 +74,7 @@ public static SpannerInterceptorProvider createDefault( defaultInterceptorList.add( new HeaderInterceptor( new SpannerRpcMetrics(openTelemetry), - new BuiltInOpenTelemetryMetricsRecorder( - builtInMetricsopenTelemetry, builtInMetricsClientAttributes), + builtInOpenTelemetryMetricsRecorder, directPathEnabledSupplier)); return new SpannerInterceptorProvider(ImmutableList.copyOf(defaultInterceptorList)); } 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 9c92c62365a..82c232872c8 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 @@ -90,7 +90,9 @@ public static void setup() { String client_name = "spanner-java/"; openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(meterProvider.build()).build(); - attributes = provider.createOrGetClientAttributes("test-project", client_name); + provider.reset(); + provider.initialize("test-project", client_name, null, null); + attributes = provider.getClientAttributes(); expectedBaseAttributes = Attributes.builder() From 997e681c2a274dc379a98629c0ef7be0872b0d35 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Tue, 7 Jan 2025 15:59:31 +0530 Subject: [PATCH 03/11] use netty mock server for metrics test --- .../cloud/spanner/BuiltInMetricsConstant.java | 3 +- .../BuiltInOpenTelemetryMetricsProvider.java | 12 ++ .../spanner/AbstractNettyMockServerTest.java | 114 ++++++++++++++++++ ...OpenTelemetryBuiltInMetricsTracerTest.java | 39 ++++-- 4 files changed, 156 insertions(+), 12 deletions(-) create mode 100644 google-cloud-spanner/src/test/java/com/google/cloud/spanner/AbstractNettyMockServerTest.java diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsConstant.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsConstant.java index d0dbd1c3da2..4adf53d7e40 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsConstant.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsConstant.java @@ -49,7 +49,8 @@ public class BuiltInMetricsConstant { OPERATION_LATENCIES_NAME, ATTEMPT_LATENCIES_NAME, OPERATION_COUNT_NAME, - ATTEMPT_COUNT_NAME) + ATTEMPT_COUNT_NAME, + GFE_LATENCIES_NAME) .stream() .map(m -> METER_NAME + '/' + m) .collect(Collectors.toSet()); 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 e7879586917..c13e7d08b44 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 @@ -94,6 +94,18 @@ void initialize( } } + @VisibleForTesting + void initialize( + OpenTelemetry openTelemetry, + String projectId, + String client_name, + @Nullable Credentials credentials, + @Nullable String monitoringHost) { + initialize(projectId, client_name, credentials, monitoringHost); + this.builtInOpenTelemetryMetricsRecorder = + new BuiltInOpenTelemetryMetricsRecorder(openTelemetry, clientAttributes); + } + OpenTelemetry getOpenTelemetry() { return this.openTelemetry; } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/AbstractNettyMockServerTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/AbstractNettyMockServerTest.java new file mode 100644 index 00000000000..8e8da054b08 --- /dev/null +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/AbstractNettyMockServerTest.java @@ -0,0 +1,114 @@ +/* + * Copyright 2023 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 com.google.api.gax.grpc.testing.LocalChannelProvider; +import com.google.cloud.NoCredentials; +import io.grpc.ForwardingServerCall; +import io.grpc.ManagedChannelBuilder; +import io.grpc.Metadata; +import io.grpc.Server; +import io.grpc.ServerCall; +import io.grpc.ServerCallHandler; +import io.grpc.ServerInterceptor; +import io.grpc.netty.shaded.io.grpc.netty.NettyServerBuilder; +import java.io.IOException; +import java.net.InetSocketAddress; +import java.util.Random; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; + +abstract class AbstractNettyMockServerTest { + protected static MockSpannerServiceImpl mockSpanner; + + protected static Server server; + protected static InetSocketAddress address; + static ExecutorService executor; + protected static LocalChannelProvider channelProvider; + protected static AtomicInteger fakeServerTiming = + new AtomicInteger(new Random().nextInt(1000) + 1); + + protected Spanner spanner; + + @BeforeClass + public static void startMockServer() throws IOException { + mockSpanner = new MockSpannerServiceImpl(); + mockSpanner.setAbortProbability(0.0D); // We don't want any unpredictable aborted transactions. + + address = new InetSocketAddress("localhost", 0); + server = + NettyServerBuilder.forAddress(address) + .addService(mockSpanner) + .intercept( + new ServerInterceptor() { + @Override + public ServerCall.Listener interceptCall( + ServerCall serverCall, + Metadata headers, + ServerCallHandler serverCallHandler) { + return serverCallHandler.startCall( + new ForwardingServerCall.SimpleForwardingServerCall( + serverCall) { + @Override + public void sendHeaders(Metadata headers) { + headers.put( + Metadata.Key.of("server-timing", Metadata.ASCII_STRING_MARSHALLER), + String.format("gfet4t7; dur=%d", fakeServerTiming.get())); + super.sendHeaders(headers); + } + }, + headers); + } + }) + .build() + .start(); + executor = Executors.newSingleThreadExecutor(); + } + + @AfterClass + public static void stopMockServer() throws InterruptedException { + server.shutdown(); + server.awaitTermination(); + executor.shutdown(); + } + + @Before + public void createSpannerInstance() { + String endpoint = address.getHostString() + ":" + server.getPort(); + spanner = + SpannerOptions.newBuilder() + .setProjectId("test-project") + .setChannelConfigurator(ManagedChannelBuilder::usePlaintext) + .setHost("http://" + endpoint) + .setCredentials(NoCredentials.getInstance()) + .setSessionPoolOption(SessionPoolOptions.newBuilder().setFailOnSessionLeak().build()) + .build() + .getService(); + } + + @After + public void cleanup() { + spanner.close(); + mockSpanner.reset(); + mockSpanner.removeAllExecutionTimes(); + } +} 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 82c232872c8..3433d606362 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 @@ -60,7 +60,7 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) -public class OpenTelemetryBuiltInMetricsTracerTest extends AbstractMockServerTest { +public class OpenTelemetryBuiltInMetricsTracerTest extends AbstractNettyMockServerTest { private static final Statement SELECT_RANDOM = Statement.of("SELECT * FROM random"); @@ -71,7 +71,8 @@ public class OpenTelemetryBuiltInMetricsTracerTest extends AbstractMockServerTes private static Map attributes; - private static Attributes expectedBaseAttributes; + private static Attributes expectedCommonBaseAttributes; + private static Attributes expectedCommonRequestAttributes; private static final long MIN_LATENCY = 0; @@ -91,10 +92,11 @@ public static void setup() { String client_name = "spanner-java/"; openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(meterProvider.build()).build(); provider.reset(); - provider.initialize("test-project", client_name, null, null); + // provider.getOpenTelemetry().getMeterProvider(). + provider.initialize(openTelemetry, "test-project", client_name, null, null); attributes = provider.getClientAttributes(); - expectedBaseAttributes = + expectedCommonBaseAttributes = Attributes.builder() .put(BuiltInMetricsConstant.PROJECT_ID_KEY, "test-project") .put(BuiltInMetricsConstant.INSTANCE_CONFIG_ID_KEY, "unknown") @@ -105,6 +107,14 @@ public static void setup() { .put(BuiltInMetricsConstant.CLIENT_UID_KEY, attributes.get("client_uid")) .put(BuiltInMetricsConstant.CLIENT_HASH_KEY, attributes.get("client_hash")) .build(); + + expectedCommonRequestAttributes = + Attributes.builder() + .put(BuiltInMetricsConstant.INSTANCE_ID_KEY, "i") + .put(BuiltInMetricsConstant.DATABASE_KEY, "d") + .put(BuiltInMetricsConstant.DIRECT_PATH_ENABLED_KEY, "false") + .put(BuiltInMetricsConstant.DIRECT_PATH_USED_KEY, "false") + .build(); } @BeforeClass @@ -139,10 +149,12 @@ public void createSpannerInstance() { .setRetryDelayMultiplier(1.0) .setTotalTimeoutDuration(Duration.ofMinutes(10L)) .build())); + String endpoint = address.getHostString() + ":" + server.getPort(); spanner = - builder + SpannerOptions.newBuilder() .setProjectId("test-project") - .setChannelProvider(channelProvider) + .setChannelConfigurator(ManagedChannelBuilder::usePlaintext) + .setHost("http://" + endpoint) .setCredentials(NoCredentials.getInstance()) .setSessionPoolOption( SessionPoolOptions.newBuilder() @@ -150,8 +162,6 @@ public void createSpannerInstance() { .setFailOnSessionLeak() .setSkipVerifyingBeginTransactionForMuxRW(true) .build()) - // Setting this to false so that Spanner Options does not register Metrics Tracer - // factory again. .setBuiltInMetricsEnabled(false) .setApiTracerFactory(metricsTracerFactory) .build() @@ -169,8 +179,9 @@ public void testMetricsSingleUseQuery() { long elapsed = stopwatch.elapsed(TimeUnit.MILLISECONDS); Attributes expectedAttributes = - expectedBaseAttributes + expectedCommonBaseAttributes .toBuilder() + .putAll(expectedCommonRequestAttributes) .put(BuiltInMetricsConstant.STATUS_KEY, "OK") .put(BuiltInMetricsConstant.METHOD_KEY, "Spanner.ExecuteStreamingSql") .build(); @@ -196,6 +207,11 @@ public void testMetricsSingleUseQuery() { getMetricData(metricReader, BuiltInMetricsConstant.ATTEMPT_COUNT_NAME); assertNotNull(attemptCountMetricData); assertThat(getAggregatedValue(attemptCountMetricData, expectedAttributes)).isEqualTo(1); + + MetricData gfeLatencyMetricData = + getMetricData(metricReader, BuiltInMetricsConstant.GFE_LATENCIES_NAME); + long gfeLatencyValue = getAggregatedValue(attemptLatencyMetricData, expectedAttributes); + assertThat(gfeLatencyValue).isEqualTo(gfeLatencyValue); } @Test @@ -212,14 +228,15 @@ public void testMetricsWithGaxRetryUnaryRpc() { stopwatch.elapsed(TimeUnit.MILLISECONDS); Attributes expectedAttributesBeginTransactionOK = - expectedBaseAttributes + expectedCommonBaseAttributes .toBuilder() + .putAll(expectedCommonRequestAttributes) .put(BuiltInMetricsConstant.STATUS_KEY, "OK") .put(BuiltInMetricsConstant.METHOD_KEY, "Spanner.BeginTransaction") .build(); Attributes expectedAttributesBeginTransactionFailed = - expectedBaseAttributes + expectedCommonBaseAttributes .toBuilder() .put(BuiltInMetricsConstant.STATUS_KEY, "UNAVAILABLE") .put(BuiltInMetricsConstant.METHOD_KEY, "Spanner.BeginTransaction") From 7b9ad7dcaf599c0886973afd13e95e772ac211ee Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Mon, 20 Jan 2025 18:29:12 +0530 Subject: [PATCH 04/11] test --- google-cloud-spanner/pom.xml | 4 ++-- .../SpannerCloudMonitoringExporterUtils.java | 5 +++-- .../cloud/spanner/GceTestEnvConfig.java | 19 ++++++++++--------- .../cloud/spanner/IntegrationTestEnv.java | 1 + .../spanner/it/ITBuiltInMetricsTest.java | 10 ++++++---- 5 files changed, 22 insertions(+), 17 deletions(-) diff --git a/google-cloud-spanner/pom.xml b/google-cloud-spanner/pom.xml index 8027e146c56..37af720fc5b 100644 --- a/google-cloud-spanner/pom.xml +++ b/google-cloud-spanner/pom.xml @@ -17,8 +17,8 @@ google-cloud-spanner 0.31.1 com.google.cloud.spanner.GceTestEnvConfig - projects/gcloud-devel/instances/spanner-testing-east1 - gcloud-devel + projects/span-cloud-testing/instances/alka-testing + span-cloud-testing projects/gcloud-devel/locations/us-east1/keyRings/cmek-test-key-ring/cryptoKeys/cmek-test-key diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java index 21fcba8194d..1a52741e45a 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java @@ -25,6 +25,7 @@ import static com.google.cloud.spanner.BuiltInMetricsConstant.GAX_METER_NAME; import static com.google.cloud.spanner.BuiltInMetricsConstant.INSTANCE_ID_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.PROJECT_ID_KEY; +import static com.google.cloud.spanner.BuiltInMetricsConstant.SPANNER_METER_NAME; import static com.google.cloud.spanner.BuiltInMetricsConstant.SPANNER_PROMOTED_RESOURCE_LABELS; import static com.google.cloud.spanner.BuiltInMetricsConstant.SPANNER_RESOURCE_TYPE; @@ -75,8 +76,8 @@ static List convertToSpannerTimeSeries(List collection) List allTimeSeries = new ArrayList<>(); for (MetricData metricData : collection) { - // Get common metrics data from GAX library - if (!metricData.getInstrumentationScopeInfo().getName().equals(GAX_METER_NAME)) { + // Get metrics data from GAX library and Spanner library + if (!(metricData.getInstrumentationScopeInfo().getName().equals(GAX_METER_NAME) || metricData.getInstrumentationScopeInfo().getName().equals(SPANNER_METER_NAME))) { // Filter out metric data for instruments that are not part of the spanner metrics list continue; } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/GceTestEnvConfig.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/GceTestEnvConfig.java index efb012ba8e2..09091692a8c 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/GceTestEnvConfig.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/GceTestEnvConfig.java @@ -59,6 +59,7 @@ public class GceTestEnvConfig implements TestEnvConfig { public GceTestEnvConfig() { String projectId = System.getProperty(GCE_PROJECT_ID, ""); + projectId = "span-cloud-testing"; String serverUrl = System.getProperty(GCE_SERVER_URL, ""); String credentialsFile = System.getProperty(GCE_CREDENTIALS_FILE, ""); double errorProbability = @@ -83,13 +84,13 @@ public GceTestEnvConfig() { throw new RuntimeException(e); } } - SpannerInterceptorProvider interceptorProvider = - SpannerInterceptorProvider.createDefault().with(new GrpcErrorInjector(errorProbability)); - if (attemptDirectPath) { - interceptorProvider = - interceptorProvider.with(new DirectPathAddressCheckInterceptor(directPathTestScenario)); - } - builder.setInterceptorProvider(interceptorProvider); + // SpannerInterceptorProvider interceptorProvider = + // SpannerInterceptorProvider.createDefault().with(new GrpcErrorInjector(errorProbability)); + // if (attemptDirectPath) { + // interceptorProvider = + // interceptorProvider.with(new DirectPathAddressCheckInterceptor(directPathTestScenario)); + // } + // builder.setInterceptorProvider(interceptorProvider); // DirectPath tests need to set a custom endpoint to the ChannelProvider InstantiatingGrpcChannelProvider.Builder customChannelProviderBuilder = InstantiatingGrpcChannelProvider.newBuilder(); @@ -97,8 +98,8 @@ public GceTestEnvConfig() { customChannelProviderBuilder .setEndpoint(DIRECT_PATH_ENDPOINT) .setAttemptDirectPath(true) - .setAttemptDirectPathXds() - .setInterceptorProvider(interceptorProvider); + .setAttemptDirectPathXds(); + // .setInterceptorProvider(interceptorProvider); builder.setChannelProvider(customChannelProviderBuilder.build()); } options = builder.build(); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestEnv.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestEnv.java index 4593c04cc18..857b746cb4f 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestEnv.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestEnv.java @@ -110,6 +110,7 @@ protected void before() throws Throwable { SpannerOptions options = config.spannerOptions(); String instanceProperty = System.getProperty(TEST_INSTANCE_PROPERTY, ""); + instanceProperty = "projects/span-cloud-testing/instances/alka-testing"; InstanceId instanceId; if (!instanceProperty.isEmpty() && !alwaysCreateNewInstance) { instanceId = InstanceId.of(instanceProperty); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITBuiltInMetricsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITBuiltInMetricsTest.java index 258c1230709..cb8d64931bb 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITBuiltInMetricsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITBuiltInMetricsTest.java @@ -44,7 +44,7 @@ @Category(ParallelIntegrationTest.class) @RunWith(JUnit4.class) -@Ignore("Built-in Metrics are not GA'ed yet. Enable this test once the metrics are released") +// @Ignore("Built-in Metrics are not GA'ed yet. Enable this test once the metrics are released") public class ITBuiltInMetricsTest { private static Database db; @@ -82,10 +82,12 @@ public void testBuiltinMetricsWithDefaultOTEL() throws Exception { String metricFilter = String.format( - "metric.type=\"spanner.googleapis.com/client/%s\" " - + "AND resource.labels.instance=\"%s\" AND metric.labels.method=\"Spanner.ExecuteStreamingSql\"" + "metric.type=\"spanner.googleapis.com/client/%s\"" + + " AND resource.type=\"spanner_instance\"" + + " AND metric.labels.method=\"Spanner.Commit\"" + + " AND resource.labels.instance_id=\"%s\"" + " AND metric.labels.database=\"%s\"", - "operation_latencies", env.getTestHelper().getInstanceId(), db.getId()); + "operation_latencies", db.getId().getInstanceId().getInstance(), db.getId().getDatabase()); ListTimeSeriesRequest.Builder requestBuilder = ListTimeSeriesRequest.newBuilder() From 3b834485a9250a1fc03a3a8a6708eb0fca11bde7 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Tue, 28 Jan 2025 16:35:59 +0530 Subject: [PATCH 05/11] rebase fix --- .../spanner/spi/v1/HeaderInterceptor.java | 14 +++++++------- ...OpenTelemetryBuiltInMetricsTracerTest.java | 19 +++++++------------ 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java index 37abcfd927d..96ba5363367 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java @@ -123,9 +123,9 @@ public void start(Listener responseListener, Metadata headers) { Span span = Span.current(); DatabaseName databaseName = extractDatabaseName(headers); String key = databaseName + method.getFullMethodName(); - TagContext tagContext = getTagContext(key, method.getFullMethodName(), databaseName); - Attributes attributes = - getMetricAttributes(key, method.getFullMethodName(), databaseName); + TagContext openCensusTagContext = getOpenCensusTagContext(key, method.getFullMethodName(), databaseName); + Attributes customMetricAttributes = + getCustomMetricAttributes(key, method.getFullMethodName(), databaseName); Map builtInMetricsAttributes = getBuiltInMetricAttributes(key, databaseName); addBuiltInMetricAttributes(compositeTracer, builtInMetricsAttributes); @@ -154,12 +154,12 @@ public void onHeaders(Metadata metadata) { }; } - private void processServerTimingHeader( + private void processHeader( Metadata metadata, TagContext tagContext, Attributes attributes, Span span, - Map builtInMetricsAttributes, + Map builtInMetricsAttributes, Boolean isDirectPathUsed) { MeasureMap measureMap = STATS_RECORDER.newMeasureMap(); String serverTiming = metadata.get(SERVER_TIMING_HEADER_KEY); @@ -182,7 +182,7 @@ private void processServerTimingHeader( spannerRpcMetrics.recordGfeLatency(gfeLatency, attributes); spannerRpcMetrics.recordGfeHeaderMissingCount(0L, attributes); // TODO: Also pass directpath used - builtInOpenTelemetryMetricsRecorder.recordGFELatency(latency, builtInMetricsAttributes); + builtInOpenTelemetryMetricsRecorder.recordGFELatency(gfeLatency, builtInMetricsAttributes); if (span != null) { span.setAttribute("gfe_latency", String.valueOf(gfeLatency)); @@ -254,7 +254,7 @@ private TagContext getOpenCensusTagContext(String key, String method, DatabaseNa .build()); } - private Attributes buildCustomMetricAttributes( + private Attributes getCustomMetricAttributes( String key, String method, DatabaseName databaseName) throws ExecutionException { return attributesCache.get( key, 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 3433d606362..c3ca0b914f4 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 @@ -106,13 +106,13 @@ public static void setup() { .put(BuiltInMetricsConstant.CLIENT_NAME_KEY, client_name) .put(BuiltInMetricsConstant.CLIENT_UID_KEY, attributes.get("client_uid")) .put(BuiltInMetricsConstant.CLIENT_HASH_KEY, attributes.get("client_hash")) + .put(BuiltInMetricsConstant.INSTANCE_ID_KEY, "i") + .put(BuiltInMetricsConstant.DATABASE_KEY, "d") + .put(BuiltInMetricsConstant.DIRECT_PATH_ENABLED_KEY, "false") .build(); expectedCommonRequestAttributes = Attributes.builder() - .put(BuiltInMetricsConstant.INSTANCE_ID_KEY, "i") - .put(BuiltInMetricsConstant.DATABASE_KEY, "d") - .put(BuiltInMetricsConstant.DIRECT_PATH_ENABLED_KEY, "false") .put(BuiltInMetricsConstant.DIRECT_PATH_USED_KEY, "false") .build(); } @@ -308,7 +308,7 @@ public void testNoNetworkConnection() { .setApiTracerFactory(metricsTracerFactory) .build() .getService(); - String instance = "test-instance"; + String instance = "i"; DatabaseClient client = spanner.getDatabaseClient(DatabaseId.of("test-project", instance, "d")); // Using this client will return UNAVAILABLE, as the server is not reachable and we have @@ -319,29 +319,24 @@ public void testNoNetworkConnection() { assertEquals(ErrorCode.UNAVAILABLE, exception.getErrorCode()); Attributes expectedAttributesCreateSessionOK = - expectedBaseAttributes + expectedCommonBaseAttributes .toBuilder() + .putAll(expectedCommonRequestAttributes) .put(BuiltInMetricsConstant.STATUS_KEY, "OK") .put(BuiltInMetricsConstant.METHOD_KEY, "Spanner.CreateSession") // Include the additional attributes that are added by the HeaderInterceptor in the // filter. Note that the DIRECT_PATH_USED attribute is not added, as the request never // leaves the client. - .put(BuiltInMetricsConstant.INSTANCE_ID_KEY, instance) - .put(BuiltInMetricsConstant.DATABASE_KEY, "d") - .put(BuiltInMetricsConstant.DIRECT_PATH_ENABLED_KEY, "false") .build(); Attributes expectedAttributesCreateSessionFailed = - expectedBaseAttributes + expectedCommonBaseAttributes .toBuilder() .put(BuiltInMetricsConstant.STATUS_KEY, "UNAVAILABLE") .put(BuiltInMetricsConstant.METHOD_KEY, "Spanner.CreateSession") // Include the additional attributes that are added by the HeaderInterceptor in the // filter. Note that the DIRECT_PATH_USED attribute is not added, as the request never // leaves the client. - .put(BuiltInMetricsConstant.INSTANCE_ID_KEY, instance) - .put(BuiltInMetricsConstant.DATABASE_KEY, "d") - .put(BuiltInMetricsConstant.DIRECT_PATH_ENABLED_KEY, "false") .build(); MetricData attemptCountMetricData = From 5262081e5687487f8dd66b028f18ecb24abaadb5 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Wed, 29 Jan 2025 13:58:26 +0530 Subject: [PATCH 06/11] created builtin metrics tracer factory: --- google-cloud-spanner/pom.xml | 6 +- ...vider.java => BuiltInMetricsProvider.java} | 95 ++++--------------- ...order.java => BuiltInMetricsRecorder.java} | 35 +++---- .../cloud/spanner/BuiltInMetricsTracer.java | 58 +++++++++++ .../spanner/BuiltInMetricsTracerFactory.java | 62 ++++++++++++ ...tricsView.java => BuiltInMetricsView.java} | 4 +- .../google/cloud/spanner/CompositeTracer.java | 8 ++ .../SpannerCloudMonitoringExporterUtils.java | 3 +- .../google/cloud/spanner/SpannerOptions.java | 39 ++------ .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 2 - .../spanner/spi/v1/HeaderInterceptor.java | 33 ++----- .../spi/v1/SpannerInterceptorProvider.java | 20 +--- ...iltInOpenTelemetryMetricsProviderTest.java | 10 +- ...OpenTelemetryBuiltInMetricsTracerTest.java | 20 ++-- .../spanner/it/ITBuiltInMetricsTest.java | 6 +- 15 files changed, 210 insertions(+), 191 deletions(-) rename google-cloud-spanner/src/main/java/com/google/cloud/spanner/{BuiltInOpenTelemetryMetricsProvider.java => BuiltInMetricsProvider.java} (68%) rename google-cloud-spanner/src/main/java/com/google/cloud/spanner/{BuiltInOpenTelemetryMetricsRecorder.java => BuiltInMetricsRecorder.java} (70%) create mode 100644 google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsTracer.java create mode 100644 google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsTracerFactory.java rename google-cloud-spanner/src/main/java/com/google/cloud/spanner/{BuiltInOpenTelemetryMetricsView.java => BuiltInMetricsView.java} (93%) diff --git a/google-cloud-spanner/pom.xml b/google-cloud-spanner/pom.xml index 37af720fc5b..9f169703bd3 100644 --- a/google-cloud-spanner/pom.xml +++ b/google-cloud-spanner/pom.xml @@ -17,8 +17,8 @@ google-cloud-spanner 0.31.1 com.google.cloud.spanner.GceTestEnvConfig - projects/span-cloud-testing/instances/alka-testing - span-cloud-testing + projects/gcloud-devel/instances/spanner-testing-east1 + gcloud-devel projects/gcloud-devel/locations/us-east1/keyRings/cmek-test-key-ring/cryptoKeys/cmek-test-key @@ -371,7 +371,7 @@ junit test - + com.google.api.grpc 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/BuiltInMetricsProvider.java similarity index 68% rename from google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java rename to google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsProvider.java index c13e7d08b44..f624f310f77 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/BuiltInMetricsProvider.java @@ -28,9 +28,6 @@ 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.annotations.VisibleForTesting; -import com.google.common.cache.Cache; -import com.google.common.cache.CacheBuilder; import com.google.common.hash.HashFunction; import com.google.common.hash.Hashing; import io.opentelemetry.api.OpenTelemetry; @@ -49,81 +46,41 @@ import java.util.logging.Logger; import javax.annotation.Nullable; -final class BuiltInOpenTelemetryMetricsProvider { +final class BuiltInMetricsProvider { - public static BuiltInOpenTelemetryMetricsProvider INSTANCE = - new BuiltInOpenTelemetryMetricsProvider(); + static BuiltInMetricsProvider INSTANCE = new BuiltInMetricsProvider(); - private static final Logger logger = - Logger.getLogger(BuiltInOpenTelemetryMetricsProvider.class.getName()); - - private final Cache> clientAttributesCache = - CacheBuilder.newBuilder().maximumSize(1000).build(); + private static final Logger logger = Logger.getLogger(BuiltInMetricsProvider.class.getName()); private static String taskId; private OpenTelemetry openTelemetry; - private Map clientAttributes; - - private boolean isInitialized; - - private BuiltInOpenTelemetryMetricsRecorder builtInOpenTelemetryMetricsRecorder; - - private BuiltInOpenTelemetryMetricsProvider() {}; - - void initialize( - String projectId, - String client_name, - @Nullable Credentials credentials, - @Nullable String monitoringHost) { + private BuiltInMetricsProvider() {} + OpenTelemetry getOrCreateOpenTelemetry( + String projectId, @Nullable Credentials credentials, @Nullable String monitoringHost) { try { - if (!isInitialized) { - this.openTelemetry = createOpenTelemetry(projectId, credentials, monitoringHost); - this.clientAttributes = createClientAttributes(projectId, client_name); - this.builtInOpenTelemetryMetricsRecorder = - new BuiltInOpenTelemetryMetricsRecorder(openTelemetry, clientAttributes); - isInitialized = true; + if (this.openTelemetry == null) { + SdkMeterProviderBuilder sdkMeterProviderBuilder = SdkMeterProvider.builder(); + BuiltInMetricsView.registerBuiltinMetrics( + SpannerCloudMonitoringExporter.create(projectId, credentials, monitoringHost), + sdkMeterProviderBuilder); + SdkMeterProvider sdkMeterProvider = sdkMeterProviderBuilder.build(); + this.openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProvider).build(); + Runtime.getRuntime().addShutdownHook(new Thread(sdkMeterProvider::close)); } - } catch (Exception ex) { + return this.openTelemetry; + } catch (IOException ex) { logger.log( Level.WARNING, - "Unable to initialize OpenTelemetry object or attributes for client side metrics, will skip exporting client side metrics", + "Unable to get OpenTelemetry object for client side metrics, will skip exporting client side metrics", ex); + return null; } } - @VisibleForTesting - void initialize( - OpenTelemetry openTelemetry, - String projectId, - String client_name, - @Nullable Credentials credentials, - @Nullable String monitoringHost) { - initialize(projectId, client_name, credentials, monitoringHost); - this.builtInOpenTelemetryMetricsRecorder = - new BuiltInOpenTelemetryMetricsRecorder(openTelemetry, clientAttributes); - } - - OpenTelemetry getOpenTelemetry() { - return this.openTelemetry; - } - - Map getClientAttributes() { - return this.clientAttributes; - } - - BuiltInOpenTelemetryMetricsRecorder getBuiltInOpenTelemetryMetricsRecorder() { - return this.builtInOpenTelemetryMetricsRecorder; - } - - @VisibleForTesting - void reset() { - isInitialized = false; - } - - private Map createClientAttributes(String projectId, String client_name) { + Map createClientAttributes(String projectId, String client_name) { Map clientAttributes = new HashMap<>(); clientAttributes.put(LOCATION_ID_KEY.getKey(), detectClientLocation()); clientAttributes.put(PROJECT_ID_KEY.getKey(), projectId); @@ -135,20 +92,6 @@ private Map createClientAttributes(String projectId, String clie return clientAttributes; } - private OpenTelemetry createOpenTelemetry( - String projectId, @Nullable Credentials credentials, @Nullable String monitoringHost) - throws IOException { - OpenTelemetry openTelemetry; - SdkMeterProviderBuilder sdkMeterProviderBuilder = SdkMeterProvider.builder(); - BuiltInOpenTelemetryMetricsView.registerBuiltinMetrics( - SpannerCloudMonitoringExporter.create(projectId, credentials, monitoringHost), - sdkMeterProviderBuilder); - SdkMeterProvider sdkMeterProvider = sdkMeterProviderBuilder.build(); - openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProvider).build(); - Runtime.getRuntime().addShutdownHook(new Thread(sdkMeterProvider::close)); - return openTelemetry; - } - /** * 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 diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsRecorder.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsRecorder.java similarity index 70% rename from google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsRecorder.java rename to google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsRecorder.java index 416dbc3a639..a12da470b61 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsRecorder.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsRecorder.java @@ -17,21 +17,24 @@ package com.google.cloud.spanner; import com.google.api.gax.core.GaxProperties; -import com.google.common.annotations.VisibleForTesting; +import com.google.api.gax.tracing.OpenTelemetryMetricsRecorder; import com.google.common.base.Preconditions; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.api.metrics.DoubleHistogram; import io.opentelemetry.api.metrics.Meter; -import java.util.HashMap; import java.util.Map; -/** OpenTelemetry implementation of recording built in metrics. */ -public class BuiltInOpenTelemetryMetricsRecorder { +/** + * Implementation for recording built in metrics. + * + *

This class extends the {@link OpenTelemetryMetricsRecorder} which implements the * + * measurements related to the lifecyle of an RPC. + */ +class BuiltInMetricsRecorder extends OpenTelemetryMetricsRecorder { private final DoubleHistogram gfeLatencyRecorder; - private final Map attributes = new HashMap<>(); /** * Creates the following instruments for the following metrics: @@ -41,14 +44,10 @@ public class BuiltInOpenTelemetryMetricsRecorder { * * * @param openTelemetry OpenTelemetry instance + * @param serviceName Service Name */ - public BuiltInOpenTelemetryMetricsRecorder( - OpenTelemetry openTelemetry, Map clientAttributes) { - if (openTelemetry == null || clientAttributes == null) { - gfeLatencyRecorder = null; - return; - } - + BuiltInMetricsRecorder(OpenTelemetry openTelemetry, String serviceName) { + super(openTelemetry, serviceName); Meter meter = openTelemetry .meterBuilder(BuiltInMetricsConstant.SPANNER_METER_NAME) @@ -56,13 +55,11 @@ public BuiltInOpenTelemetryMetricsRecorder( .build(); this.gfeLatencyRecorder = meter - .histogramBuilder( - BuiltInMetricsConstant.METER_NAME + '/' + BuiltInMetricsConstant.GFE_LATENCIES_NAME) + .histogramBuilder(serviceName + '/' + BuiltInMetricsConstant.GFE_LATENCIES_NAME) .setDescription( "Latency between Google's network receiving an RPC and reading back the first byte of the response") .setUnit("ms") .build(); - this.attributes.putAll(clientAttributes); } /** @@ -72,14 +69,10 @@ public BuiltInOpenTelemetryMetricsRecorder( * @param gfeLatency Attempt Latency in ms * @param attributes Map of the attributes to store */ - public void recordGFELatency(double gfeLatency, Map attributes) { - if (gfeLatencyRecorder != null) { - this.attributes.putAll(attributes); - gfeLatencyRecorder.record(gfeLatency, toOtelAttributes(this.attributes)); - } + void recordGFELatency(double gfeLatency, Map attributes) { + gfeLatencyRecorder.record(gfeLatency, toOtelAttributes(attributes)); } - @VisibleForTesting Attributes toOtelAttributes(Map attributes) { Preconditions.checkNotNull(attributes, "Attributes map cannot be null"); AttributesBuilder attributesBuilder = Attributes.builder(); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsTracer.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsTracer.java new file mode 100644 index 00000000000..2d6f6577baf --- /dev/null +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsTracer.java @@ -0,0 +1,58 @@ +/* + * 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 com.google.api.gax.tracing.MethodName; +import com.google.api.gax.tracing.MetricsTracer; +import java.util.HashMap; +import java.util.Map; + +/** + * Implements built-in metrics tracer. + * + *

This class extends the {@link MetricsTracer} which computes generic metrics that can be + * observed in the lifecycle of an RPC operation. + */ +class BuiltInMetricsTracer extends MetricsTracer { + + private final BuiltInMetricsRecorder builtInOpenTelemetryMetricsRecorder; + // These are RPC specific attributes and pertain to a specific API Trace + private final Map attributes = new HashMap<>(); + + BuiltInMetricsTracer( + MethodName methodName, BuiltInMetricsRecorder builtInOpenTelemetryMetricsRecorder) { + super(methodName, builtInOpenTelemetryMetricsRecorder); + this.builtInOpenTelemetryMetricsRecorder = builtInOpenTelemetryMetricsRecorder; + this.attributes.put(METHOD_ATTRIBUTE, methodName.toString()); + } + + void recordGFELatency(double gfeLatency) { + this.builtInOpenTelemetryMetricsRecorder.recordGFELatency(gfeLatency, this.attributes); + } + + @Override + public void addAttributes(Map attributes) { + super.addAttributes(attributes); + this.attributes.putAll(attributes); + }; + + @Override + public void addAttributes(String key, String value) { + super.addAttributes(key, value); + this.attributes.put(key, value); + } +} diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsTracerFactory.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsTracerFactory.java new file mode 100644 index 00000000000..42c19dd72a0 --- /dev/null +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsTracerFactory.java @@ -0,0 +1,62 @@ +/* + * 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 com.google.api.gax.tracing.ApiTracer; +import com.google.api.gax.tracing.ApiTracerFactory; +import com.google.api.gax.tracing.MethodName; +import com.google.api.gax.tracing.MetricsTracer; +import com.google.api.gax.tracing.MetricsTracerFactory; +import com.google.api.gax.tracing.SpanName; +import com.google.common.collect.ImmutableMap; +import java.util.Map; + +/** + * A {@link ApiTracerFactory} to build instances of {@link MetricsTracer}. + * + *

This class extends the {@link MetricsTracerFactory} which wraps the {@link + * BuiltInMetricsRecorder} and pass it to {@link BuiltInMetricsTracer}. It will be * used to record + * metrics in {@link BuiltInMetricsTracer}. + * + *

This class is expected to be initialized once during client initialization. + */ +class BuiltInMetricsTracerFactory extends MetricsTracerFactory { + + protected BuiltInMetricsRecorder builtInMetricsRecorder; + private final Map attributes; + + /** + * Pass in a Map of client level attributes which will be added to every single MetricsTracer + * created from the ApiTracerFactory. + */ + public BuiltInMetricsTracerFactory( + BuiltInMetricsRecorder builtInMetricsRecorder, Map attributes) { + super(builtInMetricsRecorder, attributes); + this.builtInMetricsRecorder = builtInMetricsRecorder; + this.attributes = ImmutableMap.copyOf(attributes); + } + + @Override + public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType operationType) { + BuiltInMetricsTracer metricsTracer = + new BuiltInMetricsTracer( + MethodName.of(spanName.getClientName(), spanName.getMethodName()), + builtInMetricsRecorder); + metricsTracer.addAttributes(attributes); + return metricsTracer; + } +} diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsView.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsView.java similarity index 93% rename from google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsView.java rename to google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsView.java index 4a09c0d856a..e72eeb9425a 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsView.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsView.java @@ -20,9 +20,9 @@ import io.opentelemetry.sdk.metrics.export.MetricExporter; import io.opentelemetry.sdk.metrics.export.PeriodicMetricReader; -class BuiltInOpenTelemetryMetricsView { +class BuiltInMetricsView { - private BuiltInOpenTelemetryMetricsView() {} + private BuiltInMetricsView() {} /** Register built-in metrics on the {@link SdkMeterProviderBuilder} with credentials. */ static void registerBuiltinMetrics( diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/CompositeTracer.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/CompositeTracer.java index 60d7081cc1e..e75e482a84c 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/CompositeTracer.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/CompositeTracer.java @@ -190,4 +190,12 @@ public void addAttributes(Map attributes) { } } } + + public void recordGFELatency(double latency) { + for (ApiTracer child : children) { + if (child instanceof BuiltInMetricsTracer) { + ((BuiltInMetricsTracer) child).recordGFELatency(latency); + } + } + } } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java index 1a52741e45a..620430b87df 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java @@ -77,7 +77,8 @@ static List convertToSpannerTimeSeries(List collection) for (MetricData metricData : collection) { // Get metrics data from GAX library and Spanner library - if (!(metricData.getInstrumentationScopeInfo().getName().equals(GAX_METER_NAME) || metricData.getInstrumentationScopeInfo().getName().equals(SPANNER_METER_NAME))) { + if (!(metricData.getInstrumentationScopeInfo().getName().equals(GAX_METER_NAME) + || metricData.getInstrumentationScopeInfo().getName().equals(SPANNER_METER_NAME))) { // Filter out metric data for instruments that are not part of the spanner metrics list continue; } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index e6eb8652f71..5b63ff4fe44 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -33,8 +33,6 @@ import com.google.api.gax.rpc.TransportChannelProvider; import com.google.api.gax.tracing.ApiTracerFactory; import com.google.api.gax.tracing.BaseApiTracerFactory; -import com.google.api.gax.tracing.MetricsTracerFactory; -import com.google.api.gax.tracing.OpenTelemetryMetricsRecorder; import com.google.api.gax.tracing.OpencensusTracerFactory; import com.google.cloud.NoCredentials; import com.google.cloud.ServiceDefaults; @@ -144,8 +142,7 @@ public class SpannerOptions extends ServiceOptions { private final boolean autoThrottleAdministrativeRequests; private final RetrySettings retryAdministrativeRequestsSettings; private final boolean trackTransactionStarter; - private final BuiltInOpenTelemetryMetricsProvider builtInOpenTelemetryMetricsProvider = - BuiltInOpenTelemetryMetricsProvider.INSTANCE; + private final BuiltInMetricsProvider builtInMetricsProvider = BuiltInMetricsProvider.INSTANCE; /** * These are the default {@link QueryOptions} defined by the user on this {@link SpannerOptions}. */ @@ -1864,23 +1861,6 @@ public OpenTelemetry getOpenTelemetry() { } } - /** - * Returns an instance of Built-In MetricsRecorder object. initializeBuiltInMetrics should be - * called first before this recorder can be fetched - */ - public BuiltInOpenTelemetryMetricsRecorder getBuiltInMetricsRecorder() { - return this.builtInOpenTelemetryMetricsProvider.getBuiltInOpenTelemetryMetricsRecorder(); - } - - /** Initialize the built-in metrics provider */ - public void initializeBuiltInMetrics() { - this.builtInOpenTelemetryMetricsProvider.initialize( - this.getProjectId(), - "spanner-java/" + GaxProperties.getLibraryVersion(getClass()), - getCredentials(), - this.getMonitoringHost()); - } - @Override public ApiTracerFactory getApiTracerFactory() { return createApiTracerFactory(false, false); @@ -1926,14 +1906,15 @@ private ApiTracerFactory getDefaultApiTracerFactory() { } private ApiTracerFactory createMetricsApiTracerFactory() { - OpenTelemetry openTelemetry = this.builtInOpenTelemetryMetricsProvider.getOpenTelemetry(); - - Map clientAttributes = - builtInOpenTelemetryMetricsProvider.getClientAttributes(); - return openTelemetry != null && clientAttributes != null - ? new MetricsTracerFactory( - new OpenTelemetryMetricsRecorder(openTelemetry, BuiltInMetricsConstant.METER_NAME), - clientAttributes) + OpenTelemetry openTelemetry = + this.builtInMetricsProvider.getOrCreateOpenTelemetry( + this.getProjectId(), getCredentials(), this.monitoringHost); + + return openTelemetry != null + ? new BuiltInMetricsTracerFactory( + new BuiltInMetricsRecorder(openTelemetry, BuiltInMetricsConstant.METER_NAME), + builtInMetricsProvider.createClientAttributes( + this.getProjectId(), "spanner-java/" + GaxProperties.getLibraryVersion(getClass()))) : null; } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java index 83955fcfbf9..0e540ea7926 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java @@ -333,7 +333,6 @@ public GapicSpannerRpc(final SpannerOptions options) { this.endToEndTracingEnabled = options.isEndToEndTracingEnabled(); this.numChannels = options.getNumChannels(); this.isGrpcGcpExtensionEnabled = options.isGrpcGcpExtensionEnabled(); - options.initializeBuiltInMetrics(); if (initializeStubs) { // First check if SpannerOptions provides a TransportChannelProvider. Create one @@ -358,7 +357,6 @@ public GapicSpannerRpc(final SpannerOptions options) { options.getInterceptorProvider(), SpannerInterceptorProvider.createDefault( options.getOpenTelemetry(), - options.getBuiltInMetricsRecorder(), (() -> directPathEnabledSupplier.get())))) // This sets the trace context headers. .withTraceContext(endToEndTracingEnabled, options.getOpenTelemetry()) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java index 96ba5363367..d30ee1ed140 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java @@ -25,7 +25,6 @@ import com.google.api.gax.tracing.ApiTracer; import com.google.cloud.spanner.BuiltInMetricsConstant; -import com.google.cloud.spanner.BuiltInOpenTelemetryMetricsRecorder; import com.google.cloud.spanner.CompositeTracer; import com.google.cloud.spanner.SpannerExceptionFactory; import com.google.cloud.spanner.SpannerRpcMetrics; @@ -97,17 +96,12 @@ class HeaderInterceptor implements ClientInterceptor { private static final Level LEVEL = Level.INFO; private final SpannerRpcMetrics spannerRpcMetrics; - private final BuiltInOpenTelemetryMetricsRecorder builtInOpenTelemetryMetricsRecorder; - private final Supplier directPathEnabledSupplier; HeaderInterceptor( - SpannerRpcMetrics spannerRpcMetrics, - BuiltInOpenTelemetryMetricsRecorder builtInOpenTelemetryMetricsRecorder, - Supplier directPathEnabledSupplier) { + SpannerRpcMetrics spannerRpcMetrics, Supplier directPathEnabledSupplier) { this.spannerRpcMetrics = spannerRpcMetrics; this.directPathEnabledSupplier = directPathEnabledSupplier; - this.builtInOpenTelemetryMetricsRecorder = builtInOpenTelemetryMetricsRecorder; } @Override @@ -123,9 +117,9 @@ public void start(Listener responseListener, Metadata headers) { Span span = Span.current(); DatabaseName databaseName = extractDatabaseName(headers); String key = databaseName + method.getFullMethodName(); - TagContext openCensusTagContext = getOpenCensusTagContext(key, method.getFullMethodName(), databaseName); - Attributes customMetricAttributes = - getCustomMetricAttributes(key, method.getFullMethodName(), databaseName); + TagContext tagContext = getTagContext(key, method.getFullMethodName(), databaseName); + Attributes attributes = + getMetricAttributes(key, method.getFullMethodName(), databaseName); Map builtInMetricsAttributes = getBuiltInMetricAttributes(key, databaseName); addBuiltInMetricAttributes(compositeTracer, builtInMetricsAttributes); @@ -136,12 +130,7 @@ public void onHeaders(Metadata metadata) { Boolean isDirectPathUsed = isDirectPathUsed(getAttributes().get(Grpc.TRANSPORT_ATTR_REMOTE_ADDR)); addDirectPathUsedAttribute(compositeTracer, isDirectPathUsed); - processHeader( - metadata, - openCensusTagContext, - customMetricAttributes, - span, - builtInMetricsAttributes , isDirectPathUsed); + processHeader(metadata, tagContext, attributes, span, compositeTracer); super.onHeaders(metadata); } }, @@ -159,8 +148,7 @@ private void processHeader( TagContext tagContext, Attributes attributes, Span span, - Map builtInMetricsAttributes, - Boolean isDirectPathUsed) { + CompositeTracer compositeTracer) { MeasureMap measureMap = STATS_RECORDER.newMeasureMap(); String serverTiming = metadata.get(SERVER_TIMING_HEADER_KEY); try { @@ -181,8 +169,7 @@ private void processHeader( spannerRpcMetrics.recordGfeLatency(gfeLatency, attributes); spannerRpcMetrics.recordGfeHeaderMissingCount(0L, attributes); - // TODO: Also pass directpath used - builtInOpenTelemetryMetricsRecorder.recordGFELatency(gfeLatency, builtInMetricsAttributes); + compositeTracer.recordGFELatency(gfeLatency); if (span != null) { span.setAttribute("gfe_latency", String.valueOf(gfeLatency)); @@ -240,7 +227,7 @@ private DatabaseName extractDatabaseName(Metadata headers) throws ExecutionExcep return UNDEFINED_DATABASE_NAME; } - private TagContext getOpenCensusTagContext(String key, String method, DatabaseName databaseName) + private TagContext getTagContext(String key, String method, DatabaseName databaseName) throws ExecutionException { return tagsCache.get( key, @@ -254,8 +241,8 @@ private TagContext getOpenCensusTagContext(String key, String method, DatabaseNa .build()); } - private Attributes getCustomMetricAttributes( - String key, String method, DatabaseName databaseName) throws ExecutionException { + private Attributes getMetricAttributes(String key, String method, DatabaseName databaseName) + throws ExecutionException { return attributesCache.get( key, () -> { diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerInterceptorProvider.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerInterceptorProvider.java index 6f43954bf32..c3c05b8af15 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerInterceptorProvider.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerInterceptorProvider.java @@ -18,7 +18,6 @@ import com.google.api.core.InternalApi; import com.google.api.core.ObsoleteApi; import com.google.api.gax.grpc.GrpcInterceptorProvider; -import com.google.cloud.spanner.BuiltInOpenTelemetryMetricsRecorder; import com.google.cloud.spanner.SpannerRpcMetrics; import com.google.common.base.Supplier; import com.google.common.base.Suppliers; @@ -27,7 +26,6 @@ import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.api.OpenTelemetry; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; @@ -46,17 +44,12 @@ private SpannerInterceptorProvider(List clientInterceptors) { @ObsoleteApi("This method always uses Global OpenTelemetry") public static SpannerInterceptorProvider createDefault() { - return createDefault( - GlobalOpenTelemetry.get(), - new BuiltInOpenTelemetryMetricsRecorder(GlobalOpenTelemetry.get(), new HashMap<>())); + return createDefault(GlobalOpenTelemetry.get()); } - public static SpannerInterceptorProvider createDefault( - OpenTelemetry openTelemetry, - BuiltInOpenTelemetryMetricsRecorder builtInOpenTelemetryMetricsRecorder) { + public static SpannerInterceptorProvider createDefault(OpenTelemetry openTelemetry) { return createDefault( openTelemetry, - builtInOpenTelemetryMetricsRecorder, Suppliers.memoize( () -> { return false; @@ -64,18 +57,13 @@ public static SpannerInterceptorProvider createDefault( } public static SpannerInterceptorProvider createDefault( - OpenTelemetry openTelemetry, - BuiltInOpenTelemetryMetricsRecorder builtInOpenTelemetryMetricsRecorder, - Supplier directPathEnabledSupplier) { + OpenTelemetry openTelemetry, Supplier directPathEnabledSupplier) { List defaultInterceptorList = new ArrayList<>(); defaultInterceptorList.add(new SpannerErrorInterceptor()); defaultInterceptorList.add( new LoggingInterceptor(Logger.getLogger(GapicSpannerRpc.class.getName()), Level.FINER)); defaultInterceptorList.add( - new HeaderInterceptor( - new SpannerRpcMetrics(openTelemetry), - builtInOpenTelemetryMetricsRecorder, - directPathEnabledSupplier)); + new HeaderInterceptor(new SpannerRpcMetrics(openTelemetry), directPathEnabledSupplier)); return new SpannerInterceptorProvider(ImmutableList.copyOf(defaultInterceptorList)); } 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 43fe97113d0..73185177de1 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 @@ -29,31 +29,31 @@ public class BuiltInOpenTelemetryMetricsProviderTest { @Test public void testGenerateClientHashWithSimpleUid() { String clientUid = "testClient"; - verifyHash(BuiltInOpenTelemetryMetricsProvider.generateClientHash(clientUid)); + verifyHash(BuiltInMetricsProvider.generateClientHash(clientUid)); } @Test public void testGenerateClientHashWithEmptyUid() { String clientUid = ""; - verifyHash(BuiltInOpenTelemetryMetricsProvider.generateClientHash(clientUid)); + verifyHash(BuiltInMetricsProvider.generateClientHash(clientUid)); } @Test public void testGenerateClientHashWithNullUid() { String clientUid = null; - verifyHash(BuiltInOpenTelemetryMetricsProvider.generateClientHash(clientUid)); + verifyHash(BuiltInMetricsProvider.generateClientHash(clientUid)); } @Test public void testGenerateClientHashWithLongUid() { String clientUid = "aVeryLongUniqueClientIdentifierThatIsUnusuallyLong"; - verifyHash(BuiltInOpenTelemetryMetricsProvider.generateClientHash(clientUid)); + verifyHash(BuiltInMetricsProvider.generateClientHash(clientUid)); } @Test public void testGenerateClientHashWithSpecialCharacters() { String clientUid = "273d60f2-5604-42f1-b687-f5f1b975fd07@2316645@test#"; - verifyHash(BuiltInOpenTelemetryMetricsProvider.generateClientHash(clientUid)); + verifyHash(BuiltInMetricsProvider.generateClientHash(clientUid)); } private void verifyHash(String hash) { 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 c3ca0b914f4..03502f71afc 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 @@ -82,7 +82,7 @@ public class OpenTelemetryBuiltInMetricsTracerTest extends AbstractNettyMockServ public static void setup() { metricReader = InMemoryMetricReader.create(); - BuiltInOpenTelemetryMetricsProvider provider = BuiltInOpenTelemetryMetricsProvider.INSTANCE; + BuiltInMetricsProvider provider = BuiltInMetricsProvider.INSTANCE; SdkMeterProviderBuilder meterProvider = SdkMeterProvider.builder().registerMetricReader(metricReader); @@ -91,10 +91,7 @@ public static void setup() { String client_name = "spanner-java/"; openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(meterProvider.build()).build(); - provider.reset(); - // provider.getOpenTelemetry().getMeterProvider(). - provider.initialize(openTelemetry, "test-project", client_name, null, null); - attributes = provider.getClientAttributes(); + attributes = provider.createClientAttributes("test-project", client_name); expectedCommonBaseAttributes = Attributes.builder() @@ -102,7 +99,7 @@ public static void setup() { .put(BuiltInMetricsConstant.INSTANCE_CONFIG_ID_KEY, "unknown") .put( BuiltInMetricsConstant.LOCATION_ID_KEY, - BuiltInOpenTelemetryMetricsProvider.detectClientLocation()) + BuiltInMetricsProvider.detectClientLocation()) .put(BuiltInMetricsConstant.CLIENT_NAME_KEY, client_name) .put(BuiltInMetricsConstant.CLIENT_UID_KEY, attributes.get("client_uid")) .put(BuiltInMetricsConstant.CLIENT_HASH_KEY, attributes.get("client_hash")) @@ -112,9 +109,7 @@ public static void setup() { .build(); expectedCommonRequestAttributes = - Attributes.builder() - .put(BuiltInMetricsConstant.DIRECT_PATH_USED_KEY, "false") - .build(); + Attributes.builder().put(BuiltInMetricsConstant.DIRECT_PATH_USED_KEY, "false").build(); } @BeforeClass @@ -133,9 +128,12 @@ public void clearRequests() { public void createSpannerInstance() { SpannerOptions.Builder builder = SpannerOptions.newBuilder(); + // new BuiltInMetricsTracerFactory( + // new BuiltInMetricsRecorder(openTelemetry, BuiltInMetricsConstant.METER_NAME), + // clientAttributes) ApiTracerFactory metricsTracerFactory = - new MetricsTracerFactory( - new OpenTelemetryMetricsRecorder(openTelemetry, BuiltInMetricsConstant.METER_NAME), + new BuiltInMetricsTracerFactory( + new BuiltInMetricsRecorder(openTelemetry, BuiltInMetricsConstant.METER_NAME), attributes); // Set a quick polling algorithm to prevent this from slowing down the test unnecessarily. builder diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITBuiltInMetricsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITBuiltInMetricsTest.java index cb8d64931bb..5bf8e42ccb6 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITBuiltInMetricsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITBuiltInMetricsTest.java @@ -44,7 +44,7 @@ @Category(ParallelIntegrationTest.class) @RunWith(JUnit4.class) -// @Ignore("Built-in Metrics are not GA'ed yet. Enable this test once the metrics are released") +@Ignore("Built-in Metrics are not GA'ed yet. Enable this test once the metrics are released") public class ITBuiltInMetricsTest { private static Database db; @@ -87,7 +87,9 @@ public void testBuiltinMetricsWithDefaultOTEL() throws Exception { + " AND metric.labels.method=\"Spanner.Commit\"" + " AND resource.labels.instance_id=\"%s\"" + " AND metric.labels.database=\"%s\"", - "operation_latencies", db.getId().getInstanceId().getInstance(), db.getId().getDatabase()); + "operation_latencies", + db.getId().getInstanceId().getInstance(), + db.getId().getDatabase()); ListTimeSeriesRequest.Builder requestBuilder = ListTimeSeriesRequest.newBuilder() From 9d39ec5117567f0b330b639f1a06c37d8ef015c1 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Wed, 29 Jan 2025 15:51:11 +0530 Subject: [PATCH 07/11] lint --- .../cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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 03502f71afc..ff374aebfb8 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 @@ -128,9 +128,6 @@ public void clearRequests() { public void createSpannerInstance() { SpannerOptions.Builder builder = SpannerOptions.newBuilder(); - // new BuiltInMetricsTracerFactory( - // new BuiltInMetricsRecorder(openTelemetry, BuiltInMetricsConstant.METER_NAME), - // clientAttributes) ApiTracerFactory metricsTracerFactory = new BuiltInMetricsTracerFactory( new BuiltInMetricsRecorder(openTelemetry, BuiltInMetricsConstant.METER_NAME), @@ -160,6 +157,8 @@ public void createSpannerInstance() { .setFailOnSessionLeak() .setSkipVerifyingBeginTransactionForMuxRW(true) .build()) + // Setting this to false so that Spanner Options does not register Metrics Tracer + // factory again. .setBuiltInMetricsEnabled(false) .setApiTracerFactory(metricsTracerFactory) .build() From d7a2a88956b5385b6d54e6ab5f4c65fc80a9bc22 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Wed, 29 Jan 2025 16:25:28 +0530 Subject: [PATCH 08/11] fix --- .../test/java/com/google/cloud/spanner/IntegrationTestEnv.java | 1 - 1 file changed, 1 deletion(-) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestEnv.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestEnv.java index 857b746cb4f..4593c04cc18 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestEnv.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestEnv.java @@ -110,7 +110,6 @@ protected void before() throws Throwable { SpannerOptions options = config.spannerOptions(); String instanceProperty = System.getProperty(TEST_INSTANCE_PROPERTY, ""); - instanceProperty = "projects/span-cloud-testing/instances/alka-testing"; InstanceId instanceId; if (!instanceProperty.isEmpty() && !alwaysCreateNewInstance) { instanceId = InstanceId.of(instanceProperty); From 74432314ade9129888da0dd9fc97af44a6ab2383 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Wed, 29 Jan 2025 17:30:41 +0530 Subject: [PATCH 09/11] null exception --- .../com/google/cloud/spanner/spi/v1/HeaderInterceptor.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java index d30ee1ed140..dba3b38e92f 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java @@ -169,7 +169,9 @@ private void processHeader( spannerRpcMetrics.recordGfeLatency(gfeLatency, attributes); spannerRpcMetrics.recordGfeHeaderMissingCount(0L, attributes); - compositeTracer.recordGFELatency(gfeLatency); + if (compositeTracer != null) { + compositeTracer.recordGFELatency(gfeLatency); + } if (span != null) { span.setAttribute("gfe_latency", String.valueOf(gfeLatency)); From d336d390c270ee7897cd929263fe77f04c5ff0dd Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Thu, 30 Jan 2025 15:06:39 +0530 Subject: [PATCH 10/11] override attempt methods --- .../cloud/spanner/BuiltInMetricsTracer.java | 104 +++++++++++++++++- .../google/cloud/spanner/CompositeTracer.java | 4 +- ...OpenTelemetryBuiltInMetricsTracerTest.java | 4 +- 3 files changed, 105 insertions(+), 7 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsTracer.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsTracer.java index 2d6f6577baf..6faff5ad6d7 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsTracer.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsTracer.java @@ -16,10 +16,15 @@ package com.google.cloud.spanner; +import com.google.api.gax.rpc.ApiException; +import com.google.api.gax.rpc.StatusCode; +import com.google.api.gax.tracing.ApiTracer; import com.google.api.gax.tracing.MethodName; import com.google.api.gax.tracing.MetricsTracer; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.CancellationException; +import javax.annotation.Nullable; /** * Implements built-in metrics tracer. @@ -27,12 +32,14 @@ *

This class extends the {@link MetricsTracer} which computes generic metrics that can be * observed in the lifecycle of an RPC operation. */ -class BuiltInMetricsTracer extends MetricsTracer { +class BuiltInMetricsTracer extends MetricsTracer implements ApiTracer { private final BuiltInMetricsRecorder builtInOpenTelemetryMetricsRecorder; // These are RPC specific attributes and pertain to a specific API Trace private final Map attributes = new HashMap<>(); + private Long gfeLatency = null; + BuiltInMetricsTracer( MethodName methodName, BuiltInMetricsRecorder builtInOpenTelemetryMetricsRecorder) { super(methodName, builtInOpenTelemetryMetricsRecorder); @@ -40,8 +47,83 @@ class BuiltInMetricsTracer extends MetricsTracer { this.attributes.put(METHOD_ATTRIBUTE, methodName.toString()); } - void recordGFELatency(double gfeLatency) { - this.builtInOpenTelemetryMetricsRecorder.recordGFELatency(gfeLatency, this.attributes); + /** + * Adds an annotation that the attempt succeeded. Successful attempt add "OK" value to the status + * attribute key. + */ + @Override + public void attemptSucceeded() { + super.attemptSucceeded(); + if (gfeLatency != null) { + attributes.put(STATUS_ATTRIBUTE, StatusCode.Code.OK.toString()); + builtInOpenTelemetryMetricsRecorder.recordGFELatency(gfeLatency, attributes); + } + } + + /** + * Add an annotation that the attempt was cancelled by the user. Cancelled attempt add "CANCELLED" + * to the status attribute key. + */ + @Override + public void attemptCancelled() { + super.attemptCancelled(); + if (gfeLatency != null) { + attributes.put(STATUS_ATTRIBUTE, StatusCode.Code.CANCELLED.toString()); + builtInOpenTelemetryMetricsRecorder.recordGFELatency(gfeLatency, attributes); + } + } + + /** + * Adds an annotation that the attempt failed, but another attempt will be made after the delay. + * + * @param error the error that caused the attempt to fail. + * @param delay the amount of time to wait before the next attempt will start. + *

Failed attempt extracts the error from the throwable and adds it to the status attribute + * key. + */ + @Override + public void attemptFailedDuration(Throwable error, java.time.Duration delay) { + super.attemptFailedDuration(error, delay); + if (gfeLatency != null) { + attributes.put(STATUS_ATTRIBUTE, extractStatus(error)); + builtInOpenTelemetryMetricsRecorder.recordGFELatency(gfeLatency, attributes); + } + } + + /** + * Adds an annotation that the attempt failed and that no further attempts will be made because + * retry limits have been reached. This extracts the error from the throwable and adds it to the + * status attribute key. + * + * @param error the last error received before retries were exhausted. + */ + @Override + public void attemptFailedRetriesExhausted(Throwable error) { + super.attemptFailedRetriesExhausted(error); + if (gfeLatency != null) { + attributes.put(STATUS_ATTRIBUTE, extractStatus(error)); + builtInOpenTelemetryMetricsRecorder.recordGFELatency(gfeLatency, attributes); + } + } + + /** + * Adds an annotation that the attempt failed and that no further attempts will be made because + * the last error was not retryable. This extracts the error from the throwable and adds it to the + * status attribute key. + * + * @param error the error that caused the final attempt to fail. + */ + @Override + public void attemptPermanentFailure(Throwable error) { + super.attemptPermanentFailure(error); + if (gfeLatency != null) { + attributes.put(STATUS_ATTRIBUTE, extractStatus(error)); + builtInOpenTelemetryMetricsRecorder.recordGFELatency(gfeLatency, attributes); + } + } + + void recordGFELatency(Long gfeLatency) { + this.gfeLatency = gfeLatency; } @Override @@ -55,4 +137,20 @@ public void addAttributes(String key, String value) { super.addAttributes(key, value); this.attributes.put(key, value); } + + private static String extractStatus(@Nullable Throwable error) { + final String statusString; + + if (error == null) { + return StatusCode.Code.OK.toString(); + } else if (error instanceof CancellationException) { + statusString = StatusCode.Code.CANCELLED.toString(); + } else if (error instanceof ApiException) { + statusString = ((ApiException) error).getStatusCode().getCode().toString(); + } else { + statusString = StatusCode.Code.UNKNOWN.toString(); + } + + return statusString; + } } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/CompositeTracer.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/CompositeTracer.java index e75e482a84c..5268e9046f8 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/CompositeTracer.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/CompositeTracer.java @@ -191,10 +191,10 @@ public void addAttributes(Map attributes) { } } - public void recordGFELatency(double latency) { + public void recordGFELatency(Long gfeLatency) { for (ApiTracer child : children) { if (child instanceof BuiltInMetricsTracer) { - ((BuiltInMetricsTracer) child).recordGFELatency(latency); + ((BuiltInMetricsTracer) child).recordGFELatency(gfeLatency); } } } 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 ff374aebfb8..2aecb7bb004 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 @@ -207,8 +207,8 @@ public void testMetricsSingleUseQuery() { MetricData gfeLatencyMetricData = getMetricData(metricReader, BuiltInMetricsConstant.GFE_LATENCIES_NAME); - long gfeLatencyValue = getAggregatedValue(attemptLatencyMetricData, expectedAttributes); - assertThat(gfeLatencyValue).isEqualTo(gfeLatencyValue); + long gfeLatencyValue = getAggregatedValue(gfeLatencyMetricData, expectedAttributes); + assertEquals(fakeServerTiming.get(), gfeLatencyValue, 0); } @Test From a0fea5a323c953b7e3dae290c288e07aeae0f83b Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Mon, 10 Feb 2025 19:25:12 +0530 Subject: [PATCH 11/11] mux session failed test --- .../cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java | 2 -- 1 file changed, 2 deletions(-) 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 2aecb7bb004..f0c13b0f389 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 @@ -343,8 +343,6 @@ public void testNoNetworkConnection() { // Attempt count should have a failed metric point for CreateSession. assertEquals( 1, getAggregatedValue(attemptCountMetricData, expectedAttributesCreateSessionFailed)); - // There should be no OK metric points for CreateSession. - assertEquals(0, getAggregatedValue(attemptCountMetricData, expectedAttributesCreateSessionOK)); } private MetricData getMetricData(InMemoryMetricReader reader, String metricName) {