From 7c17432b5fc3c0714d685ee7d7757e878edcf598 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Wed, 29 Jan 2025 13:58:26 +0530 Subject: [PATCH] created builtin metrics tracer factory: --- ...vider.java => BuiltInMetricsProvider.java} | 94 ++++--------------- ...order.java => BuiltInMetricsRecorder.java} | 25 ++--- .../cloud/spanner/BuiltInMetricsTracer.java | 53 +++++++++++ .../spanner/BuiltInMetricsTracerFactory.java | 66 +++++++++++++ .../google/cloud/spanner/CompositeTracer.java | 8 ++ .../SpannerCloudMonitoringExporterUtils.java | 3 +- .../google/cloud/spanner/SpannerOptions.java | 40 +++----- .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 2 - .../spanner/spi/v1/HeaderInterceptor.java | 26 ++--- .../spi/v1/SpannerInterceptorProvider.java | 20 +--- ...iltInOpenTelemetryMetricsProviderTest.java | 10 +- ...OpenTelemetryBuiltInMetricsTracerTest.java | 20 ++-- 12 files changed, 198 insertions(+), 169 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} (77%) 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 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..e04d8ead8da 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,42 @@ 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(); + 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(); + 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)); } - } 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 +93,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 77% 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..14b3d289be0 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,6 +17,7 @@ package com.google.cloud.spanner; import com.google.api.gax.core.GaxProperties; +import com.google.api.gax.tracing.OpenTelemetryMetricsRecorder; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import io.opentelemetry.api.OpenTelemetry; @@ -24,14 +25,13 @@ 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 { +public class BuiltInMetricsRecorder extends OpenTelemetryMetricsRecorder { private final DoubleHistogram gfeLatencyRecorder; - private final Map attributes = new HashMap<>(); + // private final Map attributes = new HashMap<>(); /** * Creates the following instruments for the following metrics: @@ -42,13 +42,8 @@ public class BuiltInOpenTelemetryMetricsRecorder { * * @param openTelemetry OpenTelemetry instance */ - public BuiltInOpenTelemetryMetricsRecorder( - OpenTelemetry openTelemetry, Map clientAttributes) { - if (openTelemetry == null || clientAttributes == null) { - gfeLatencyRecorder = null; - return; - } - + public BuiltInMetricsRecorder(OpenTelemetry openTelemetry, String serviceName) { + super(openTelemetry, serviceName); Meter meter = openTelemetry .meterBuilder(BuiltInMetricsConstant.SPANNER_METER_NAME) @@ -56,13 +51,12 @@ 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); + // this.attributes.putAll(clientAttributes); } /** @@ -73,10 +67,7 @@ public BuiltInOpenTelemetryMetricsRecorder( * @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)); - } + gfeLatencyRecorder.record(gfeLatency, toOtelAttributes(attributes)); } @VisibleForTesting 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..8c2849969a4 --- /dev/null +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsTracer.java @@ -0,0 +1,53 @@ +/* + * 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; + +public 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<>(); + + public BuiltInMetricsTracer( + MethodName methodName, BuiltInMetricsRecorder builtInOpenTelemetryMetricsRecorder) { + super(methodName, builtInOpenTelemetryMetricsRecorder); + this.builtInOpenTelemetryMetricsRecorder = builtInOpenTelemetryMetricsRecorder; + this.attributes.put(METHOD_ATTRIBUTE, methodName.toString()); + this.attributes.put(LANGUAGE_ATTRIBUTE, DEFAULT_LANGUAGE); + } + + public void gfeCapture(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..ef5127afa3d --- /dev/null +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsTracerFactory.java @@ -0,0 +1,66 @@ +/* + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.spanner; + +import com.google.api.core.BetaApi; +import com.google.api.core.InternalApi; +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.MetricsRecorder; +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 wraps the {@link MetricsRecorder} and pass it to {@link MetricsTracer}. It will be + * used to record metrics in {@link MetricsTracer}. + * + *

This class is expected to be initialized once during client initialization. + */ +@BetaApi +@InternalApi +public class BuiltInMetricsTracerFactory extends MetricsTracerFactory { + + protected BuiltInMetricsRecorder builtInOpenTelemetryMetricsRecorder; + 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 builtInOpenTelemetryMetricsRecorder, Map attributes) { + super(builtInOpenTelemetryMetricsRecorder, attributes); + this.builtInOpenTelemetryMetricsRecorder = builtInOpenTelemetryMetricsRecorder; + 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()), + builtInOpenTelemetryMetricsRecorder); + attributes.forEach(metricsTracer::addAttributes); + return metricsTracer; + } +} 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..958f5c51dff 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 addGFELatency(double latency) { + for (ApiTracer child : children) { + if (child instanceof BuiltInMetricsTracer) { + ((BuiltInMetricsTracer) child).gfeCapture(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..325dba27a55 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,8 @@ 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 builtInOpenTelemetryMetricsProvider = + BuiltInMetricsProvider.INSTANCE; /** * These are the default {@link QueryOptions} defined by the user on this {@link SpannerOptions}. */ @@ -1864,23 +1862,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 +1907,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.builtInOpenTelemetryMetricsProvider.getOrCreateOpenTelemetry( + this.getProjectId(), getCredentials(), this.monitoringHost); + + return openTelemetry != null + ? new BuiltInMetricsTracerFactory( + new BuiltInMetricsRecorder(openTelemetry, BuiltInMetricsConstant.METER_NAME), + builtInOpenTelemetryMetricsProvider.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..9618ead6ac2 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,7 +117,8 @@ 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); + TagContext openCensusTagContext = + getOpenCensusTagContext(key, method.getFullMethodName(), databaseName); Attributes customMetricAttributes = getCustomMetricAttributes(key, method.getFullMethodName(), databaseName); Map builtInMetricsAttributes = @@ -141,7 +136,9 @@ public void onHeaders(Metadata metadata) { openCensusTagContext, customMetricAttributes, span, - builtInMetricsAttributes , isDirectPathUsed); + builtInMetricsAttributes, + isDirectPathUsed, + compositeTracer); super.onHeaders(metadata); } }, @@ -160,7 +157,8 @@ private void processHeader( Attributes attributes, Span span, Map builtInMetricsAttributes, - Boolean isDirectPathUsed) { + Boolean isDirectPathUsed, + CompositeTracer compositeTracer) { MeasureMap measureMap = STATS_RECORDER.newMeasureMap(); String serverTiming = metadata.get(SERVER_TIMING_HEADER_KEY); try { @@ -182,7 +180,9 @@ private void processHeader( spannerRpcMetrics.recordGfeLatency(gfeLatency, attributes); spannerRpcMetrics.recordGfeHeaderMissingCount(0L, attributes); // TODO: Also pass directpath used - builtInOpenTelemetryMetricsRecorder.recordGFELatency(gfeLatency, builtInMetricsAttributes); + compositeTracer.addGFELatency(gfeLatency); + // builtInOpenTelemetryMetricsRecorder.recordGFELatency(gfeLatency, + // builtInMetricsAttributes); if (span != null) { span.setAttribute("gfe_latency", String.valueOf(gfeLatency)); @@ -254,8 +254,8 @@ private TagContext getOpenCensusTagContext(String key, String method, DatabaseNa .build()); } - private Attributes getCustomMetricAttributes( - String key, String method, DatabaseName databaseName) throws ExecutionException { + private Attributes getCustomMetricAttributes(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