From 3b64f56e80a9026e8a000d82b37a7d102349a0bd Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Wed, 19 Jun 2024 15:23:04 +0530 Subject: [PATCH 1/9] feat: client metrics --- google-cloud-spanner/pom.xml | 15 +- .../cloud/spanner/BuiltInMetricsConstant.java | 106 ++++++- .../BuiltInOpenTelemetryMetricsProvider.java | 117 +++++++ .../BuiltInOpenTelemetryMetricsView.java | 33 ++ .../google/cloud/spanner/SpannerOptions.java | 65 +++- .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 8 +- .../spanner/spi/v1/HeaderInterceptor.java | 14 + ...OpenTelemetryBuiltInMetricsTracerTest.java | 293 ++++++++++++++++++ .../spanner/it/ITBuiltInMetricsTest.java | 117 +++++++ 9 files changed, 744 insertions(+), 24 deletions(-) create mode 100644 google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java create mode 100644 google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsView.java create mode 100644 google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java create mode 100644 google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITBuiltInMetricsTest.java diff --git a/google-cloud-spanner/pom.xml b/google-cloud-spanner/pom.xml index f4398b42efc..0ced9401ed0 100644 --- a/google-cloud-spanner/pom.xml +++ b/google-cloud-spanner/pom.xml @@ -246,6 +246,10 @@ io.opentelemetry opentelemetry-context + + io.opentelemetry + opentelemetry-sdk + io.opentelemetry opentelemetry-sdk-common @@ -254,6 +258,10 @@ io.opentelemetry opentelemetry-sdk-metrics + + com.google.cloud.opentelemetry + detector-resources-support + com.google.cloud google-cloud-monitoring @@ -437,11 +445,6 @@ test - - io.opentelemetry - opentelemetry-sdk - test - io.opentelemetry opentelemetry-sdk-trace @@ -610,4 +613,4 @@ - + \ No newline at end of file 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 179eafcf53c..a5912e1123a 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 @@ -16,16 +16,24 @@ package com.google.cloud.spanner; +import com.google.api.gax.tracing.OpenTelemetryMetricsRecorder; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.sdk.metrics.Aggregation; +import io.opentelemetry.sdk.metrics.InstrumentSelector; +import io.opentelemetry.sdk.metrics.InstrumentType; +import io.opentelemetry.sdk.metrics.View; +import java.util.Map; import java.util.Set; import java.util.stream.Collectors; public class BuiltInMetricsConstant { - public static final String METER_NAME = "spanner.googleapis.com/internal/client"; + static final String METER_NAME = "spanner.googleapis.com/internal/client"; - public static final String GAX_METER_NAME = "gax-java"; + static final String GAX_METER_NAME = OpenTelemetryMetricsRecorder.GAX_METER_NAME; static final String OPERATION_LATENCIES_NAME = "operation_latencies"; static final String ATTEMPT_LATENCIES_NAME = "attempt_latencies"; @@ -34,7 +42,7 @@ public class BuiltInMetricsConstant { static final String OPERATION_COUNT_NAME = "operation_count"; static final String ATTEMPT_COUNT_NAME = "attempt_count"; - public static final Set SPANNER_METRICS = + static final Set SPANNER_METRICS = ImmutableSet.of( OPERATION_LATENCIES_NAME, ATTEMPT_LATENCIES_NAME, @@ -44,29 +52,29 @@ public class BuiltInMetricsConstant { .map(m -> METER_NAME + '/' + m) .collect(Collectors.toSet()); - public static final String SPANNER_RESOURCE_TYPE = "spanner_instance_client"; + static final String SPANNER_RESOURCE_TYPE = "spanner_instance_client"; - public static final AttributeKey PROJECT_ID_KEY = AttributeKey.stringKey("project_id"); + static final AttributeKey PROJECT_ID_KEY = AttributeKey.stringKey("project_id"); public static final AttributeKey INSTANCE_ID_KEY = AttributeKey.stringKey("instance_id"); - public static final AttributeKey LOCATION_ID_KEY = AttributeKey.stringKey("location"); - public static final AttributeKey INSTANCE_CONFIG_ID_KEY = + static final AttributeKey LOCATION_ID_KEY = AttributeKey.stringKey("location"); + static final AttributeKey INSTANCE_CONFIG_ID_KEY = AttributeKey.stringKey("instance_config"); // These metric labels will be promoted to the spanner monitored resource fields - public static final Set> SPANNER_PROMOTED_RESOURCE_LABELS = + static final Set> SPANNER_PROMOTED_RESOURCE_LABELS = ImmutableSet.of(PROJECT_ID_KEY, INSTANCE_ID_KEY, INSTANCE_CONFIG_ID_KEY, LOCATION_ID_KEY); public static final AttributeKey DATABASE_KEY = AttributeKey.stringKey("database"); - public static final AttributeKey CLIENT_UID_KEY = AttributeKey.stringKey("client_uid"); - public static final AttributeKey CLIENT_NAME_KEY = AttributeKey.stringKey("client_name"); - public static final AttributeKey METHOD_KEY = AttributeKey.stringKey("method"); - public static final AttributeKey STATUS_KEY = AttributeKey.stringKey("status"); - public static final AttributeKey DIRECT_PATH_ENABLED_KEY = + static final AttributeKey CLIENT_UID_KEY = AttributeKey.stringKey("client_uid"); + static final AttributeKey CLIENT_NAME_KEY = AttributeKey.stringKey("client_name"); + static final AttributeKey METHOD_KEY = AttributeKey.stringKey("method"); + static final AttributeKey STATUS_KEY = AttributeKey.stringKey("status"); + static final AttributeKey DIRECT_PATH_ENABLED_KEY = AttributeKey.stringKey("directpath_enabled"); - public static final AttributeKey DIRECT_PATH_USED_KEY = + static final AttributeKey DIRECT_PATH_USED_KEY = AttributeKey.stringKey("directpath_used"); - public static final Set COMMON_ATTRIBUTES = + static final Set COMMON_ATTRIBUTES = ImmutableSet.of( PROJECT_ID_KEY, INSTANCE_ID_KEY, @@ -79,4 +87,72 @@ public class BuiltInMetricsConstant { CLIENT_NAME_KEY, DIRECT_PATH_ENABLED_KEY, DIRECT_PATH_USED_KEY); + + static Aggregation AGGREGATION_WITH_MILLIS_HISTOGRAM = + Aggregation.explicitBucketHistogram( + ImmutableList.of( + 0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 8.0, 10.0, 13.0, 16.0, 20.0, 25.0, 30.0, 40.0, + 50.0, 65.0, 80.0, 100.0, 130.0, 160.0, 200.0, 250.0, 300.0, 400.0, 500.0, 650.0, + 800.0, 1000.0, 2000.0, 5000.0, 10000.0, 20000.0, 50000.0, 100000.0, 200000.0, + 400000.0, 800000.0, 1600000.0, 3200000.0)); + + static Map getAllViews() { + ImmutableMap.Builder views = ImmutableMap.builder(); + defineView( + views, + BuiltInMetricsConstant.OPERATION_LATENCY_NAME, + BuiltInMetricsConstant.OPERATION_LATENCIES_NAME, + BuiltInMetricsConstant.AGGREGATION_WITH_MILLIS_HISTOGRAM, + InstrumentType.HISTOGRAM, + "ms"); + defineView( + views, + BuiltInMetricsConstant.ATTEMPT_LATENCY_NAME, + BuiltInMetricsConstant.ATTEMPT_LATENCIES_NAME, + BuiltInMetricsConstant.AGGREGATION_WITH_MILLIS_HISTOGRAM, + InstrumentType.HISTOGRAM, + "ms"); + defineView( + views, + BuiltInMetricsConstant.OPERATION_COUNT_NAME, + BuiltInMetricsConstant.OPERATION_COUNT_NAME, + Aggregation.sum(), + InstrumentType.COUNTER, + "1"); + defineView( + views, + BuiltInMetricsConstant.ATTEMPT_COUNT_NAME, + BuiltInMetricsConstant.ATTEMPT_COUNT_NAME, + Aggregation.sum(), + InstrumentType.COUNTER, + "1"); + return views.build(); + } + + private static void defineView( + ImmutableMap.Builder viewMap, + String metricName, + String metricViewName, + Aggregation aggregation, + InstrumentType type, + String unit) { + InstrumentSelector selector = + InstrumentSelector.builder() + .setName(BuiltInMetricsConstant.METER_NAME + '/' + metricName) + .setMeterName(BuiltInMetricsConstant.GAX_METER_NAME) + .setType(type) + .setUnit(unit) + .build(); + Set attributesFilter = + BuiltInMetricsConstant.COMMON_ATTRIBUTES.stream() + .map(AttributeKey::getKey) + .collect(Collectors.toSet()); + View view = + View.builder() + .setName(BuiltInMetricsConstant.METER_NAME + '/' + metricViewName) + .setAggregation(aggregation) + .setAttributeFilter(attributesFilter) + .build(); + viewMap.put(selector, view); + } } 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 new file mode 100644 index 00000000000..0ac1a43f48d --- /dev/null +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java @@ -0,0 +1,117 @@ +/* + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.spanner; + +import static com.google.cloud.spanner.BuiltInMetricsConstant.CLIENT_NAME_KEY; +import static com.google.cloud.spanner.BuiltInMetricsConstant.CLIENT_UID_KEY; +import static com.google.cloud.spanner.BuiltInMetricsConstant.DIRECT_PATH_ENABLED_KEY; +import static com.google.cloud.spanner.BuiltInMetricsConstant.LOCATION_ID_KEY; +import static com.google.cloud.spanner.BuiltInMetricsConstant.PROJECT_ID_KEY; + +import com.google.api.gax.core.GaxProperties; +import com.google.auth.Credentials; +import com.google.cloud.opentelemetry.detection.DetectedPlatform; +import com.google.cloud.opentelemetry.detection.GCPPlatformDetector; +import com.google.common.annotations.VisibleForTesting; +import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.sdk.OpenTelemetrySdk; +import io.opentelemetry.sdk.metrics.SdkMeterProvider; +import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder; +import io.opentelemetry.sdk.metrics.export.MetricExporter; +import java.io.IOException; +import java.lang.management.ManagementFactory; +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.util.HashMap; +import java.util.Map; +import java.util.UUID; +import java.util.logging.Level; +import java.util.logging.Logger; +import javax.annotation.Nullable; + +class BuiltInOpenTelemetryMetricsProvider { + + private static final Logger logger = + Logger.getLogger(BuiltInOpenTelemetryMetricsProvider.class.getName()); + + private OpenTelemetry openTelemetry; + + OpenTelemetry getOpenTelemetry(String projectId, @Nullable Credentials credentials) { + try { + return getOpenTelemetry(SpannerCloudMonitoringExporter.create(projectId, credentials)); + } catch (IOException ex) { + logger.log( + Level.WARNING, + "Unable to get OpenTelemetry object for client side metrics, will skip exporting client side metrics", + ex); + return null; + } + } + + @VisibleForTesting + OpenTelemetry getOpenTelemetry(MetricExporter metricExporter) { + if (this.openTelemetry == null) { + SdkMeterProviderBuilder sdkMeterProviderBuilder = SdkMeterProvider.builder(); + BuiltInOpenTelemetryMetricsView.registerBuiltinMetrics( + metricExporter, sdkMeterProviderBuilder); + this.openTelemetry = + OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProviderBuilder.build()).build(); + } + return this.openTelemetry; + } + + Map getClientAttributes(String projectId, boolean canUseDirectPath) { + Map clientAttributes = new HashMap<>(); + clientAttributes.put(LOCATION_ID_KEY.getKey(), detectClientLocation()); + clientAttributes.put(PROJECT_ID_KEY.getKey(), projectId); + clientAttributes.put(DIRECT_PATH_ENABLED_KEY.getKey(), String.valueOf(canUseDirectPath)); + clientAttributes.put( + CLIENT_NAME_KEY.getKey(), + "spanner-java/" + + GaxProperties.getLibraryVersion(SpannerCloudMonitoringExporterUtils.class)); + clientAttributes.put(CLIENT_UID_KEY.getKey(), getDefaultTaskValue()); + return clientAttributes; + } + + private String detectClientLocation() { + GCPPlatformDetector detector = GCPPlatformDetector.DEFAULT_INSTANCE; + DetectedPlatform detectedPlatform = detector.detectPlatform(); + String region = detectedPlatform.getAttributes().get("cloud.region"); + return region == null ? "global" : region; + } + + /** + * In most cases this should look like ${UUID}@${hostname}. The hostname will be retrieved from + * the jvm name and fallback to the local hostname. + */ + private String getDefaultTaskValue() { + // Something like '@' + final String jvmName = ManagementFactory.getRuntimeMXBean().getName(); + // If jvm doesn't have the expected format, fallback to the local hostname + if (jvmName.indexOf('@') < 1) { + String hostname = "localhost"; + try { + hostname = InetAddress.getLocalHost().getHostName(); + } catch (UnknownHostException e) { + logger.log(Level.INFO, "Unable to get the hostname.", e); + } + // Generate a random number and use the same format "random_number@hostname". + return UUID.randomUUID() + "@" + hostname; + } + return UUID.randomUUID() + jvmName; + } +} 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/BuiltInOpenTelemetryMetricsView.java new file mode 100644 index 00000000000..6a2d7357ca2 --- /dev/null +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsView.java @@ -0,0 +1,33 @@ +/* + * 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 io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder; +import io.opentelemetry.sdk.metrics.export.MetricExporter; +import io.opentelemetry.sdk.metrics.export.PeriodicMetricReader; + +public class BuiltInOpenTelemetryMetricsView { + + private BuiltInOpenTelemetryMetricsView() {} + + /** Register built-in metrics on the {@link SdkMeterProviderBuilder} with credentials. */ + public static void registerBuiltinMetrics( + MetricExporter metricExporter, SdkMeterProviderBuilder builder) { + BuiltInMetricsConstant.getAllViews().forEach(builder::registerView); + builder.registerMetricReader(PeriodicMetricReader.create(metricExporter)); + } +} 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 3a8632e2ebe..5dc8cb84d62 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 @@ -30,6 +30,8 @@ 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; @@ -157,7 +159,9 @@ public class SpannerOptions extends ServiceOptions { private final boolean useVirtualThreads; private final OpenTelemetry openTelemetry; private final boolean enableApiTracing; + private final boolean enableBuiltInMetrics; private final boolean enableExtendedTracing; + private final boolean canUseDirectPath; enum TracingFramework { OPEN_CENSUS, @@ -664,6 +668,8 @@ protected SpannerOptions(Builder builder) { openTelemetry = builder.openTelemetry; enableApiTracing = builder.enableApiTracing; enableExtendedTracing = builder.enableExtendedTracing; + enableBuiltInMetrics = builder.enableBuiltInMetrics; + canUseDirectPath = builder.canUseDirectPath; } /** @@ -696,6 +702,10 @@ default boolean isEnableExtendedTracing() { default boolean isEnableApiTracing() { return false; } + + default boolean isEnableBuiltInMetrics() { + return false; + } } /** @@ -709,6 +719,7 @@ private static class SpannerEnvironmentImpl implements SpannerEnvironment { "SPANNER_OPTIMIZER_STATISTICS_PACKAGE"; private static final String SPANNER_ENABLE_EXTENDED_TRACING = "SPANNER_ENABLE_EXTENDED_TRACING"; private static final String SPANNER_ENABLE_API_TRACING = "SPANNER_ENABLE_API_TRACING"; + private static final String SPANNER_ENABLE_BUILTIN_METRICS = "SPANNER_ENABLE_BUILTIN_METRICS"; private SpannerEnvironmentImpl() {} @@ -734,6 +745,11 @@ public boolean isEnableExtendedTracing() { public boolean isEnableApiTracing() { return Boolean.parseBoolean(System.getenv(SPANNER_ENABLE_API_TRACING)); } + + @Override + public boolean isEnableBuiltInMetrics() { + return Boolean.parseBoolean(System.getenv(SPANNER_ENABLE_BUILTIN_METRICS)); + } } /** Builder for {@link SpannerOptions} instances. */ @@ -797,6 +813,8 @@ public static class Builder private OpenTelemetry openTelemetry; private boolean enableApiTracing = SpannerOptions.environment.isEnableApiTracing(); private boolean enableExtendedTracing = SpannerOptions.environment.isEnableExtendedTracing(); + private boolean enableBuiltInMetrics = SpannerOptions.environment.isEnableBuiltInMetrics(); + private boolean canUseDirectPath = false; private static String createCustomClientLibToken(String token) { return token + " " + ServiceOptions.getGoogApiClientLibName(); @@ -862,6 +880,7 @@ protected Builder() { this.useVirtualThreads = options.useVirtualThreads; this.enableApiTracing = options.enableApiTracing; this.enableExtendedTracing = options.enableExtendedTracing; + this.enableBuiltInMetrics = options.enableBuiltInMetrics; } @Override @@ -1375,6 +1394,19 @@ public Builder setEnableApiTracing(boolean enableApiTracing) { return this; } + /** Enabling this will enable built in metrics for each individual RPC execution. */ + @VisibleForTesting + public Builder setEnableBuiltInMetrics(boolean enableBuiltInMetrics) { + this.enableBuiltInMetrics = enableBuiltInMetrics; + return this; + } + + @InternalApi + public Builder canUseDirectPath(boolean canUseDirectPath) { + this.canUseDirectPath = canUseDirectPath; + return this; + } + /** * Sets whether to enable extended OpenTelemetry tracing. Enabling this option will add the * following additional attributes to the traces that are generated by the client: @@ -1621,6 +1653,7 @@ public boolean isAttemptDirectPath() { public OpenTelemetry getOpenTelemetry() { if (this.openTelemetry != null) { return this.openTelemetry; + } else { return GlobalOpenTelemetry.get(); } @@ -1632,7 +1665,13 @@ public ApiTracerFactory getApiTracerFactory() { // Prefer any direct ApiTracerFactory that might have been set on the builder. apiTracerFactories.add( MoreObjects.firstNonNull(super.getApiTracerFactory(), getDefaultApiTracerFactory())); - + // Add Metrics Tracer factory + if (isEnableBuiltInMetrics()) { + ApiTracerFactory metricsTracerFactory = getMetricsApiTracerFactory(canUseDirectPath); + if (metricsTracerFactory != null) { + apiTracerFactories.add(metricsTracerFactory); + } + } return new CompositeTracerFactory(apiTracerFactories); } @@ -1652,6 +1691,21 @@ private ApiTracerFactory getDefaultApiTracerFactory() { return BaseApiTracerFactory.getInstance(); } + private ApiTracerFactory getMetricsApiTracerFactory(boolean canUseDirectPath) { + BuiltInOpenTelemetryMetricsProvider builtInOpenTelemetryMetricsProvider = + new BuiltInOpenTelemetryMetricsProvider(); + OpenTelemetry openTelemetry = + builtInOpenTelemetryMetricsProvider.getOpenTelemetry( + getDefaultProjectId(), getCredentials()); + + return openTelemetry != null + ? new MetricsTracerFactory( + new OpenTelemetryMetricsRecorder(openTelemetry, BuiltInMetricsConstant.METER_NAME), + builtInOpenTelemetryMetricsProvider.getClientAttributes( + getDefaultProjectId(), canUseDirectPath)) + : null; + } + /** * Returns true if an {@link com.google.api.gax.tracing.ApiTracer} should be created and set on * the Spanner client. Enabling this only has effect if an OpenTelemetry or OpenCensus trace @@ -1661,6 +1715,15 @@ public boolean isEnableApiTracing() { return enableApiTracing; } + /** + * Returns true if an {@link com.google.api.gax.tracing.ApiTracer} should be created and set on + * the Spanner client. Enabling this only has effect if an OpenTelemetry or OpenCensus trace + * exporter has been configured. + */ + boolean isEnableBuiltInMetrics() { + return enableBuiltInMetrics; + } + @BetaApi public boolean isUseVirtualThreads() { return useVirtualThreads; 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 e1e15b851b4..4c442f89566 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 @@ -370,9 +370,13 @@ public GapicSpannerRpc(final SpannerOptions options) { // If it is enabled in options uses the channel pool provided by the gRPC-GCP extension. maybeEnableGrpcGcpExtension(defaultChannelProviderBuilder, options); + InstantiatingGrpcChannelProvider defaultChannelProvider = + defaultChannelProviderBuilder.build(); + TransportChannelProvider channelProvider = - MoreObjects.firstNonNull( - options.getChannelProvider(), defaultChannelProviderBuilder.build()); + MoreObjects.firstNonNull(options.getChannelProvider(), defaultChannelProvider); + + options.toBuilder().canUseDirectPath(defaultChannelProvider.canUseDirectPath()).build(); CredentialsProvider credentialsProvider = GrpcTransportOptions.setUpCredentialsProvider(options); 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 76b6c65a9b8..99245b00958 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 @@ -15,6 +15,7 @@ */ package com.google.cloud.spanner.spi.v1; +import static com.google.api.gax.grpc.GrpcCallContext.TRACER_KEY; import static com.google.cloud.spanner.spi.v1.SpannerRpcViews.DATABASE_ID; import static com.google.cloud.spanner.spi.v1.SpannerRpcViews.INSTANCE_ID; import static com.google.cloud.spanner.spi.v1.SpannerRpcViews.METHOD; @@ -22,6 +23,8 @@ import static com.google.cloud.spanner.spi.v1.SpannerRpcViews.SPANNER_GFE_HEADER_MISSING_COUNT; import static com.google.cloud.spanner.spi.v1.SpannerRpcViews.SPANNER_GFE_LATENCY; +import com.google.cloud.spanner.BuiltInMetricsConstant; +import com.google.cloud.spanner.CompositeTracer; import com.google.cloud.spanner.SpannerExceptionFactory; import com.google.cloud.spanner.SpannerRpcMetrics; import com.google.common.cache.Cache; @@ -96,8 +99,10 @@ public void start(Listener responseListener, Metadata headers) { DatabaseName databaseName = extractDatabaseName(headers); String key = databaseName + method.getFullMethodName(); TagContext tagContext = getTagContext(key, method.getFullMethodName(), databaseName); + CompositeTracer compositeTracer = (CompositeTracer) callOptions.getOption(TRACER_KEY); Attributes attributes = getMetricAttributes(key, method.getFullMethodName(), databaseName); + addBuiltInMetricAttributes(compositeTracer, databaseName); super.start( new SimpleForwardingClientCallListener(responseListener) { @Override @@ -197,4 +202,13 @@ private Attributes getMetricAttributes(String key, String method, DatabaseName d return attributesBuilder.build(); }); } + + private void addBuiltInMetricAttributes( + CompositeTracer compositeTracer, DatabaseName databaseName) { + // Built in metrics Attributes. + compositeTracer.addAttributes( + BuiltInMetricsConstant.DATABASE_KEY.getKey(), databaseName.getDatabase()); + compositeTracer.addAttributes( + BuiltInMetricsConstant.INSTANCE_ID_KEY.getKey(), databaseName.getInstance()); + } } 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 new file mode 100644 index 00000000000..8e397f0172b --- /dev/null +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java @@ -0,0 +1,293 @@ +/* + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.spanner; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth.assertWithMessage; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import com.google.api.gax.core.GaxProperties; +import com.google.api.gax.longrunning.OperationTimedPollAlgorithm; +import com.google.api.gax.retrying.RetrySettings; +import com.google.api.gax.tracing.ApiTracerFactory; +import com.google.api.gax.tracing.MetricsTracerFactory; +import com.google.api.gax.tracing.OpenTelemetryMetricsRecorder; +import com.google.cloud.NoCredentials; +import com.google.cloud.spanner.MockSpannerServiceImpl.SimulatedExecutionTime; +import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult; +import com.google.cloud.spanner.connection.RandomResultSetGenerator; +import com.google.common.base.Stopwatch; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Range; +import io.grpc.Status; +import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.sdk.OpenTelemetrySdk; +import io.opentelemetry.sdk.metrics.SdkMeterProvider; +import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder; +import io.opentelemetry.sdk.metrics.data.HistogramPointData; +import io.opentelemetry.sdk.metrics.data.LongPointData; +import io.opentelemetry.sdk.metrics.data.MetricData; +import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; +import org.junit.After; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.threeten.bp.Duration; + +@RunWith(JUnit4.class) +public class OpenTelemetryBuiltInMetricsTracerTest extends AbstractMockServerTest { + + private static final Statement SELECT_RANDOM = Statement.of("SELECT * FROM random"); + + private static final Statement UPDATE_RANDOM = Statement.of("UPDATE random SET foo=1 WHERE id=1"); + private static InMemoryMetricReader metricReader; + + private static OpenTelemetry openTelemetry; + + private static Map attributes; + + private static Attributes expectedBaseAttributes; + + private static final long MIN_LATENCY = 0; + + private DatabaseClient client; + + @BeforeClass + public static void setup() { + metricReader = InMemoryMetricReader.create(); + BuiltInOpenTelemetryMetricsProvider provider = new BuiltInOpenTelemetryMetricsProvider(); + + SdkMeterProviderBuilder meterProvider = + SdkMeterProvider.builder().registerMetricReader(metricReader); + + BuiltInMetricsConstant.getAllViews().forEach(meterProvider::registerView); + + openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(meterProvider.build()).build(); + boolean canUseDirectPath = true; + attributes = provider.getClientAttributes("test-project", canUseDirectPath); + + expectedBaseAttributes = + Attributes.builder() + .put(BuiltInMetricsConstant.PROJECT_ID_KEY, "test-project") + .put(BuiltInMetricsConstant.DIRECT_PATH_ENABLED_KEY, "true") + .put(BuiltInMetricsConstant.LOCATION_ID_KEY, "global") + .put( + BuiltInMetricsConstant.CLIENT_NAME_KEY, + "spanner-java/" + + GaxProperties.getLibraryVersion(SpannerCloudMonitoringExporterUtils.class)) + .put(BuiltInMetricsConstant.CLIENT_UID_KEY, attributes.get("client_uid")) + .build(); + } + + @BeforeClass + public static void setupResults() { + RandomResultSetGenerator generator = new RandomResultSetGenerator(1); + mockSpanner.putStatementResult(StatementResult.query(SELECT_RANDOM, generator.generate())); + mockSpanner.putStatementResults(StatementResult.update(UPDATE_RANDOM, 1L)); + } + + @After + public void clearRequests() { + mockSpanner.clearRequests(); + } + + @Override + public void createSpannerInstance() { + SpannerOptions.Builder builder = SpannerOptions.newBuilder(); + + ApiTracerFactory metricsTracerFactory = + new MetricsTracerFactory( + new OpenTelemetryMetricsRecorder(openTelemetry, BuiltInMetricsConstant.METER_NAME), + attributes); + // Set a quick polling algorithm to prevent this from slowing down the test unnecessarily. + builder + .getDatabaseAdminStubSettingsBuilder() + .updateDatabaseDdlOperationSettings() + .setPollingAlgorithm( + OperationTimedPollAlgorithm.create( + RetrySettings.newBuilder() + .setInitialRetryDelay(Duration.ofNanos(1L)) + .setMaxRetryDelay(Duration.ofNanos(1L)) + .setRetryDelayMultiplier(1.0) + .setTotalTimeout(Duration.ofMinutes(10L)) + .build())); + spanner = + builder + .setProjectId("test-project") + .setChannelProvider(channelProvider) + .setCredentials(NoCredentials.getInstance()) + .setSessionPoolOption( + SessionPoolOptions.newBuilder() + .setWaitForMinSessions(Duration.ofSeconds(5L)) + .setFailOnSessionLeak() + .build()) + // Setting this to false so that Spanner Options does not register Metrics Tracer + // factory again. + .setEnableBuiltInMetrics(false) + .setApiTracerFactory(metricsTracerFactory) + .build() + .getService(); + client = spanner.getDatabaseClient(DatabaseId.of("test-project", "i", "d")); + } + + @Test + public void testMetricsSingleUseQuery() { + Stopwatch stopwatch = Stopwatch.createStarted(); + try (ResultSet resultSet = client.singleUse().executeQuery(SELECT_RANDOM)) { + assertTrue(resultSet.next()); + assertFalse(resultSet.next()); + } + + long elapsed = stopwatch.elapsed(TimeUnit.MILLISECONDS); + Attributes expectedAttributes = + expectedBaseAttributes + .toBuilder() + .put(BuiltInMetricsConstant.STATUS_KEY, "OK") + .put(BuiltInMetricsConstant.METHOD_KEY, "Spanner.ExecuteStreamingSql") + .build(); + + MetricData operationLatencyMetricData = + getMetricData(metricReader, BuiltInMetricsConstant.OPERATION_LATENCIES_NAME); + long operationLatencyValue = getAggregatedValue(operationLatencyMetricData, expectedAttributes); + assertThat(operationLatencyValue).isIn(Range.closed(MIN_LATENCY, elapsed)); + + MetricData attemptLatencyMetricData = + getMetricData(metricReader, BuiltInMetricsConstant.ATTEMPT_LATENCIES_NAME); + long attemptLatencyValue = getAggregatedValue(attemptLatencyMetricData, expectedAttributes); + assertThat(attemptLatencyValue).isIn(Range.closed(MIN_LATENCY, elapsed)); + + MetricData operationCountMetricData = + getMetricData(metricReader, BuiltInMetricsConstant.OPERATION_COUNT_NAME); + assertThat(getAggregatedValue(operationCountMetricData, expectedAttributes)).isEqualTo(1); + + MetricData attemptCountMetricData = + getMetricData(metricReader, BuiltInMetricsConstant.ATTEMPT_COUNT_NAME); + assertThat(getAggregatedValue(attemptCountMetricData, expectedAttributes)).isEqualTo(1); + } + + @Test + public void testMetricsWithGaxRetryUnaryRpc() { + Stopwatch stopwatch = Stopwatch.createStarted(); + mockSpanner.setBeginTransactionExecutionTime( + SimulatedExecutionTime.ofException(Status.UNAVAILABLE.asRuntimeException())); + + // Execute a simple read/write transaction using only mutations. This will use the + // BeginTransaction RPC to start the transaction. That RPC will first return UNAVAILABLE, then + // be retried by Gax, and succeed. The retry should show up in the tracing. + client.write(ImmutableList.of(Mutation.newInsertBuilder("foo").set("bar").to(1L).build())); + + stopwatch.elapsed(TimeUnit.MILLISECONDS); + + Attributes expectedAttributesBeginTransactionOK = + expectedBaseAttributes + .toBuilder() + .put(BuiltInMetricsConstant.STATUS_KEY, "OK") + .put(BuiltInMetricsConstant.METHOD_KEY, "Spanner.BeginTransaction") + .build(); + + Attributes expectedAttributesBeginTransactionFailed = + expectedBaseAttributes + .toBuilder() + .put(BuiltInMetricsConstant.STATUS_KEY, "UNAVAILABLE") + .put(BuiltInMetricsConstant.METHOD_KEY, "Spanner.BeginTransaction") + .build(); + + MetricData attemptCountMetricData = + getMetricData(metricReader, BuiltInMetricsConstant.ATTEMPT_COUNT_NAME); + assertThat(getAggregatedValue(attemptCountMetricData, expectedAttributesBeginTransactionOK)) + .isEqualTo(1); + // Attempt count should have a failed metric point for Begin Transaction. + assertThat(getAggregatedValue(attemptCountMetricData, expectedAttributesBeginTransactionFailed)) + .isEqualTo(1); + + MetricData operationCountMetricData = + getMetricData(metricReader, BuiltInMetricsConstant.OPERATION_COUNT_NAME); + assertThat(getAggregatedValue(operationCountMetricData, expectedAttributesBeginTransactionOK)) + .isEqualTo(1); + // Operation count should not have a failed metric point for Begin Transaction as overall + // operation is success.. + assertThat( + getAggregatedValue(operationCountMetricData, expectedAttributesBeginTransactionFailed)) + .isEqualTo(0); + } + + private MetricData getMetricData(InMemoryMetricReader reader, String metricName) { + String fullMetricName = BuiltInMetricsConstant.METER_NAME + "/" + metricName; + Collection allMetricData = Collections.emptyList(); + + // Fetch the MetricData with retries + for (int attemptsLeft = 10; attemptsLeft > 0; attemptsLeft--) { + allMetricData = reader.collectAllMetrics(); + List matchingMetadata = + allMetricData.stream() + .filter(md -> md.getName().equals(fullMetricName)) + .collect(Collectors.toList()); + assertWithMessage( + "Found multiple MetricData with the same name: %s, in: %s", + fullMetricName, matchingMetadata) + .that(matchingMetadata.size()) + .isAtMost(1); + + if (!matchingMetadata.isEmpty()) { + return matchingMetadata.get(0); + } + + try { + Thread.sleep(100); + } catch (InterruptedException interruptedException) { + Thread.currentThread().interrupt(); + throw new RuntimeException(interruptedException); + } + } + + assertFalse(String.format("MetricData is missing for metric {0}", fullMetricName), false); + return null; + } + + private long getAggregatedValue(MetricData metricData, Attributes attributes) { + switch (metricData.getType()) { + case HISTOGRAM: + Optional hd = + metricData.getHistogramData().getPoints().stream() + .filter(pd -> pd.getAttributes().equals(attributes)) + .collect(Collectors.toList()) + .stream() + .findFirst(); + return hd.isPresent() ? (long) hd.get().getSum() / hd.get().getCount() : 0; + case LONG_SUM: + Optional ld = + metricData.getLongSumData().getPoints().stream() + .filter(pd -> pd.getAttributes().equals(attributes)) + .collect(Collectors.toList()) + .stream() + .findFirst(); + return ld.isPresent() ? ld.get().getValue() : 0; + default: + return 0; + } + } +} 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 new file mode 100644 index 00000000000..869e8f4d2a8 --- /dev/null +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITBuiltInMetricsTest.java @@ -0,0 +1,117 @@ +/* + * 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.it; + +import static com.google.common.truth.Truth.assertWithMessage; + +import com.google.api.gax.longrunning.OperationFuture; +import com.google.cloud.monitoring.v3.MetricServiceClient; +import com.google.cloud.spanner.Database; +import com.google.cloud.spanner.DatabaseClient; +import com.google.cloud.spanner.IntegrationTestEnv; +import com.google.cloud.spanner.Statement; +import com.google.common.base.Stopwatch; +import com.google.monitoring.v3.ListTimeSeriesRequest; +import com.google.monitoring.v3.ListTimeSeriesResponse; +import com.google.monitoring.v3.ProjectName; +import com.google.monitoring.v3.TimeInterval; +import com.google.protobuf.util.Timestamps; +import com.google.spanner.admin.database.v1.UpdateDatabaseDdlMetadata; +import java.io.IOException; +import java.util.Collections; +import java.util.concurrent.TimeUnit; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.threeten.bp.Duration; +import org.threeten.bp.Instant; + +@RunWith(JUnit4.class) +public class ITBuiltInMetricsTest { + + private static Database db; + @ClassRule public static IntegrationTestEnv env = new IntegrationTestEnv(); + + private static DatabaseClient client; + + private static MetricServiceClient metricClient; + + @BeforeClass + public static void setUp() throws IOException { + metricClient = MetricServiceClient.create(); + env.getTestHelper().getOptions().toBuilder().setEnableBuiltInMetrics(true); + db = env.getTestHelper().createTestDatabase(); + client = env.getTestHelper().getDatabaseClient(db); + } + + @Test + public void testBuiltinMetricsWithDefaultOTEL() throws Exception { + // This stopwatch is used for to limit fetching of metric data in verifyMetrics + Stopwatch metricsPollingStopwatch = Stopwatch.createStarted(); + Instant start = Instant.now().minus(Duration.ofMinutes(10)); + Instant end = Instant.now().plus(Duration.ofMinutes(3)); + ProjectName name = ProjectName.of(env.getTestHelper().getOptions().getProjectId()); + + TimeInterval interval = + TimeInterval.newBuilder() + .setStartTime(Timestamps.fromMillis(start.toEpochMilli())) + .setEndTime(Timestamps.fromMillis(end.toEpochMilli())) + .build(); + String ddl = + "CREATE TABLE FOO (" + + " K STRING(MAX) NOT NULL," + + " V INT64," + + ") PRIMARY KEY (K)"; + OperationFuture op = + db.updateDdl(Collections.singletonList(ddl), null); + op.get(); + + client + .readWriteTransaction() + .run(transaction -> transaction.executeQuery(Statement.of("Select 1"))); + + String metricFilter = + String.format( + "metric.type=\"spanner.googleapis.com/client/%s\" " + + "AND resource.labels.instance=\"%s\" AND metric.labels.method=\"Spanner.ExecuteStreamingSql\"" + + " AND metric.labels.database=\"%s\"", + "operation_latencies", env.getTestHelper().getInstanceId(), db.getId()); + + ListTimeSeriesRequest.Builder requestBuilder = + ListTimeSeriesRequest.newBuilder() + .setName(name.toString()) + .setFilter(metricFilter) + .setInterval(interval) + .setView(ListTimeSeriesRequest.TimeSeriesView.FULL); + + ListTimeSeriesRequest request = requestBuilder.build(); + + ListTimeSeriesResponse response = metricClient.listTimeSeriesCallable().call(request); + while (response.getTimeSeriesCount() == 0 + && metricsPollingStopwatch.elapsed(TimeUnit.MINUTES) < 10) { + // Call listTimeSeries every minute + Thread.sleep(Duration.ofMinutes(1).toMillis()); + response = metricClient.listTimeSeriesCallable().call(request); + } + + assertWithMessage("View operation_latencies didn't return any data.") + .that(response.getTimeSeriesCount()) + .isGreaterThan(0); + } +} From cedbb610029d9d127d502b9386aebcb2649da439 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Tue, 23 Jul 2024 13:09:45 +0530 Subject: [PATCH 2/9] Review comments --- .../clirr-ignored-differences.xml | 9 +- .../cloud/spanner/BuiltInMetricsConstant.java | 32 ++++--- .../BuiltInOpenTelemetryMetricsProvider.java | 91 ++++++++++++------- .../BuiltInOpenTelemetryMetricsView.java | 4 +- .../google/cloud/spanner/SpannerOptions.java | 46 +++++----- .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 13 ++- .../spanner/spi/v1/HeaderInterceptor.java | 6 +- ...OpenTelemetryBuiltInMetricsTracerTest.java | 7 +- .../spanner/it/ITBuiltInMetricsTest.java | 9 +- 9 files changed, 134 insertions(+), 83 deletions(-) diff --git a/google-cloud-spanner/clirr-ignored-differences.xml b/google-cloud-spanner/clirr-ignored-differences.xml index 9433fcba5ad..db1526835e2 100644 --- a/google-cloud-spanner/clirr-ignored-differences.xml +++ b/google-cloud-spanner/clirr-ignored-differences.xml @@ -695,6 +695,13 @@ boolean isEnableApiTracing() + + + 7012 + com/google/cloud/spanner/SpannerOptions$SpannerEnvironment + boolean isEnableBuiltInMetrics() + + 7012 @@ -725,7 +732,7 @@ com/google/cloud/spanner/SessionPoolOptions$Builder com.google.cloud.spanner.SessionPoolOptions$Builder setUseMultiplexedSession(boolean) - + 7012 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 a5912e1123a..4bb71b680ff 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 @@ -16,6 +16,7 @@ package com.google.cloud.spanner; +import com.google.api.core.InternalApi; import com.google.api.gax.tracing.OpenTelemetryMetricsRecorder; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -29,11 +30,12 @@ import java.util.Set; import java.util.stream.Collectors; +@InternalApi public class BuiltInMetricsConstant { - static final String METER_NAME = "spanner.googleapis.com/internal/client"; + public static final String METER_NAME = "spanner.googleapis.com/internal/client"; - static final String GAX_METER_NAME = OpenTelemetryMetricsRecorder.GAX_METER_NAME; + public static final String GAX_METER_NAME = OpenTelemetryMetricsRecorder.GAX_METER_NAME; static final String OPERATION_LATENCIES_NAME = "operation_latencies"; static final String ATTEMPT_LATENCIES_NAME = "attempt_latencies"; @@ -42,7 +44,7 @@ public class BuiltInMetricsConstant { static final String OPERATION_COUNT_NAME = "operation_count"; static final String ATTEMPT_COUNT_NAME = "attempt_count"; - static final Set SPANNER_METRICS = + public static final Set SPANNER_METRICS = ImmutableSet.of( OPERATION_LATENCIES_NAME, ATTEMPT_LATENCIES_NAME, @@ -52,29 +54,29 @@ public class BuiltInMetricsConstant { .map(m -> METER_NAME + '/' + m) .collect(Collectors.toSet()); - static final String SPANNER_RESOURCE_TYPE = "spanner_instance_client"; + public static final String SPANNER_RESOURCE_TYPE = "spanner_instance_client"; - static final AttributeKey PROJECT_ID_KEY = AttributeKey.stringKey("project_id"); + public static final AttributeKey PROJECT_ID_KEY = AttributeKey.stringKey("project_id"); public static final AttributeKey INSTANCE_ID_KEY = AttributeKey.stringKey("instance_id"); - static final AttributeKey LOCATION_ID_KEY = AttributeKey.stringKey("location"); - static final AttributeKey INSTANCE_CONFIG_ID_KEY = + public static final AttributeKey LOCATION_ID_KEY = AttributeKey.stringKey("location"); + public static final AttributeKey INSTANCE_CONFIG_ID_KEY = AttributeKey.stringKey("instance_config"); // These metric labels will be promoted to the spanner monitored resource fields - static final Set> SPANNER_PROMOTED_RESOURCE_LABELS = + public static final Set> SPANNER_PROMOTED_RESOURCE_LABELS = ImmutableSet.of(PROJECT_ID_KEY, INSTANCE_ID_KEY, INSTANCE_CONFIG_ID_KEY, LOCATION_ID_KEY); public static final AttributeKey DATABASE_KEY = AttributeKey.stringKey("database"); - static final AttributeKey CLIENT_UID_KEY = AttributeKey.stringKey("client_uid"); - static final AttributeKey CLIENT_NAME_KEY = AttributeKey.stringKey("client_name"); - static final AttributeKey METHOD_KEY = AttributeKey.stringKey("method"); - static final AttributeKey STATUS_KEY = AttributeKey.stringKey("status"); - static final AttributeKey DIRECT_PATH_ENABLED_KEY = + public static final AttributeKey CLIENT_UID_KEY = AttributeKey.stringKey("client_uid"); + public static final AttributeKey CLIENT_NAME_KEY = AttributeKey.stringKey("client_name"); + public static final AttributeKey METHOD_KEY = AttributeKey.stringKey("method"); + public static final AttributeKey STATUS_KEY = AttributeKey.stringKey("status"); + public static final AttributeKey DIRECT_PATH_ENABLED_KEY = AttributeKey.stringKey("directpath_enabled"); - static final AttributeKey DIRECT_PATH_USED_KEY = + public static final AttributeKey DIRECT_PATH_USED_KEY = AttributeKey.stringKey("directpath_used"); - static final Set COMMON_ATTRIBUTES = + public static final Set COMMON_ATTRIBUTES = ImmutableSet.of( PROJECT_ID_KEY, INSTANCE_ID_KEY, 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 0ac1a43f48d..4aff7bccd0c 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 @@ -19,6 +19,7 @@ import static com.google.cloud.spanner.BuiltInMetricsConstant.CLIENT_NAME_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.CLIENT_UID_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.DIRECT_PATH_ENABLED_KEY; +import static com.google.cloud.spanner.BuiltInMetricsConstant.INSTANCE_CONFIG_ID_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.LOCATION_ID_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.PROJECT_ID_KEY; @@ -26,14 +27,13 @@ import com.google.auth.Credentials; import com.google.cloud.opentelemetry.detection.DetectedPlatform; import com.google.cloud.opentelemetry.detection.GCPPlatformDetector; -import com.google.common.annotations.VisibleForTesting; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder; -import io.opentelemetry.sdk.metrics.export.MetricExporter; import java.io.IOException; import java.lang.management.ManagementFactory; +import java.lang.reflect.Method; import java.net.InetAddress; import java.net.UnknownHostException; import java.util.HashMap; @@ -43,16 +43,30 @@ import java.util.logging.Logger; import javax.annotation.Nullable; -class BuiltInOpenTelemetryMetricsProvider { +final class BuiltInOpenTelemetryMetricsProvider { + + public static BuiltInOpenTelemetryMetricsProvider INSTANCE = + new BuiltInOpenTelemetryMetricsProvider(); private static final Logger logger = Logger.getLogger(BuiltInOpenTelemetryMetricsProvider.class.getName()); + private static String taskId; + private OpenTelemetry openTelemetry; - OpenTelemetry getOpenTelemetry(String projectId, @Nullable Credentials credentials) { + private BuiltInOpenTelemetryMetricsProvider() {} + + OpenTelemetry getOrCreateOpenTelemetry(String projectId, @Nullable Credentials credentials) { try { - return getOpenTelemetry(SpannerCloudMonitoringExporter.create(projectId, credentials)); + if (this.openTelemetry == null) { + SdkMeterProviderBuilder sdkMeterProviderBuilder = SdkMeterProvider.builder(); + BuiltInOpenTelemetryMetricsView.registerBuiltinMetrics( + SpannerCloudMonitoringExporter.create(projectId, credentials), sdkMeterProviderBuilder); + this.openTelemetry = + OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProviderBuilder.build()).build(); + } + return this.openTelemetry; } catch (IOException ex) { logger.log( Level.WARNING, @@ -62,23 +76,14 @@ OpenTelemetry getOpenTelemetry(String projectId, @Nullable Credentials credentia } } - @VisibleForTesting - OpenTelemetry getOpenTelemetry(MetricExporter metricExporter) { - if (this.openTelemetry == null) { - SdkMeterProviderBuilder sdkMeterProviderBuilder = SdkMeterProvider.builder(); - BuiltInOpenTelemetryMetricsView.registerBuiltinMetrics( - metricExporter, sdkMeterProviderBuilder); - this.openTelemetry = - OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProviderBuilder.build()).build(); - } - return this.openTelemetry; - } - - Map getClientAttributes(String projectId, boolean canUseDirectPath) { + Map getClientAttributes(String projectId, boolean isDirectPathChannelCreated) { Map clientAttributes = new HashMap<>(); clientAttributes.put(LOCATION_ID_KEY.getKey(), detectClientLocation()); clientAttributes.put(PROJECT_ID_KEY.getKey(), projectId); - clientAttributes.put(DIRECT_PATH_ENABLED_KEY.getKey(), String.valueOf(canUseDirectPath)); + // TODO: Replace this with real value. + clientAttributes.put(INSTANCE_CONFIG_ID_KEY.getKey(), "unknown"); + clientAttributes.put( + DIRECT_PATH_ENABLED_KEY.getKey(), String.valueOf(isDirectPathChannelCreated)); clientAttributes.put( CLIENT_NAME_KEY.getKey(), "spanner-java/" @@ -95,23 +100,47 @@ private String detectClientLocation() { } /** - * In most cases this should look like ${UUID}@${hostname}. The hostname will be retrieved from - * the jvm name and fallback to the local hostname. + * Generates a unique identifier for the Client_uid metric field. The identifier is composed of a + * UUID, the process ID (PID), and the hostname. + * + *

For Java 9 and later, the PID is obtained using the ProcessHandle API. For Java 8, the PID + * is extracted from ManagementFactory.getRuntimeMXBean().getName(). + * + * @return A unique identifier string in the format UUID@PID@hostname */ - private String getDefaultTaskValue() { - // Something like '@' - final String jvmName = ManagementFactory.getRuntimeMXBean().getName(); - // If jvm doesn't have the expected format, fallback to the local hostname - if (jvmName.indexOf('@') < 1) { - String hostname = "localhost"; + private static String getDefaultTaskValue() { + if (taskId == null) { + String identifier = UUID.randomUUID().toString(); + String pid = getProcessId(); + try { - hostname = InetAddress.getLocalHost().getHostName(); + String hostname = InetAddress.getLocalHost().getHostName(); + taskId = identifier + "@" + pid + "@" + hostname; } catch (UnknownHostException e) { logger.log(Level.INFO, "Unable to get the hostname.", e); + taskId = identifier + "@" + pid + "@localhost"; + } + } + return taskId; + } + + private static String getProcessId() { + try { + // Check if Java 9+ and ProcessHandle class is available + Class processHandleClass = Class.forName("java.lang.ProcessHandle"); + Method currentMethod = processHandleClass.getMethod("current"); + Object processHandleInstance = currentMethod.invoke(null); + Method pidMethod = processHandleClass.getMethod("pid"); + long pid = (long) pidMethod.invoke(processHandleInstance); + return Long.toString(pid); + } catch (Exception e) { + // Fallback to Java 8 method + final String jvmName = ManagementFactory.getRuntimeMXBean().getName(); + if (jvmName != null && jvmName.contains("@")) { + return jvmName.split("@")[0]; + } else { + return "unknown"; } - // Generate a random number and use the same format "random_number@hostname". - return UUID.randomUUID() + "@" + hostname; } - return UUID.randomUUID() + jvmName; } } 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/BuiltInOpenTelemetryMetricsView.java index 6a2d7357ca2..4a09c0d856a 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/BuiltInOpenTelemetryMetricsView.java @@ -20,12 +20,12 @@ import io.opentelemetry.sdk.metrics.export.MetricExporter; import io.opentelemetry.sdk.metrics.export.PeriodicMetricReader; -public class BuiltInOpenTelemetryMetricsView { +class BuiltInOpenTelemetryMetricsView { private BuiltInOpenTelemetryMetricsView() {} /** Register built-in metrics on the {@link SdkMeterProviderBuilder} with credentials. */ - public static void registerBuiltinMetrics( + static void registerBuiltinMetrics( MetricExporter metricExporter, SdkMeterProviderBuilder builder) { BuiltInMetricsConstant.getAllViews().forEach(builder::registerView); builder.registerMetricReader(PeriodicMetricReader.create(metricExporter)); 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 5dc8cb84d62..29f17688fb8 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 @@ -136,6 +136,8 @@ public class SpannerOptions extends ServiceOptions { private final boolean autoThrottleAdministrativeRequests; private final RetrySettings retryAdministrativeRequestsSettings; private final boolean trackTransactionStarter; + private final BuiltInOpenTelemetryMetricsProvider builtInOpenTelemetryMetricsProvider = + BuiltInOpenTelemetryMetricsProvider.INSTANCE; /** * These are the default {@link QueryOptions} defined by the user on this {@link SpannerOptions}. */ @@ -161,7 +163,6 @@ public class SpannerOptions extends ServiceOptions { private final boolean enableApiTracing; private final boolean enableBuiltInMetrics; private final boolean enableExtendedTracing; - private final boolean canUseDirectPath; enum TracingFramework { OPEN_CENSUS, @@ -669,7 +670,6 @@ protected SpannerOptions(Builder builder) { enableApiTracing = builder.enableApiTracing; enableExtendedTracing = builder.enableExtendedTracing; enableBuiltInMetrics = builder.enableBuiltInMetrics; - canUseDirectPath = builder.canUseDirectPath; } /** @@ -814,7 +814,6 @@ public static class Builder private boolean enableApiTracing = SpannerOptions.environment.isEnableApiTracing(); private boolean enableExtendedTracing = SpannerOptions.environment.isEnableExtendedTracing(); private boolean enableBuiltInMetrics = SpannerOptions.environment.isEnableBuiltInMetrics(); - private boolean canUseDirectPath = false; private static String createCustomClientLibToken(String token) { return token + " " + ServiceOptions.getGoogApiClientLibName(); @@ -1401,12 +1400,6 @@ public Builder setEnableBuiltInMetrics(boolean enableBuiltInMetrics) { return this; } - @InternalApi - public Builder canUseDirectPath(boolean canUseDirectPath) { - this.canUseDirectPath = canUseDirectPath; - return this; - } - /** * Sets whether to enable extended OpenTelemetry tracing. Enabling this option will add the * following additional attributes to the traces that are generated by the client: @@ -1653,7 +1646,6 @@ public boolean isAttemptDirectPath() { public OpenTelemetry getOpenTelemetry() { if (this.openTelemetry != null) { return this.openTelemetry; - } else { return GlobalOpenTelemetry.get(); } @@ -1661,17 +1653,30 @@ public OpenTelemetry getOpenTelemetry() { @Override public ApiTracerFactory getApiTracerFactory() { - List apiTracerFactories = new ArrayList(); + return createApiTracerFactory(false, false); + } + + public ApiTracerFactory getApiTracerFactory( + boolean isDirectPathChannelCreated, boolean isAdminClient) { + return createApiTracerFactory(isDirectPathChannelCreated, isAdminClient); + } + + private ApiTracerFactory createApiTracerFactory( + boolean isDirectPathChannelCreated, boolean isAdminClient) { + List apiTracerFactories = new ArrayList<>(); // Prefer any direct ApiTracerFactory that might have been set on the builder. apiTracerFactories.add( MoreObjects.firstNonNull(super.getApiTracerFactory(), getDefaultApiTracerFactory())); - // Add Metrics Tracer factory - if (isEnableBuiltInMetrics()) { - ApiTracerFactory metricsTracerFactory = getMetricsApiTracerFactory(canUseDirectPath); + + // Add Metrics Tracer factory if enabled and if data client + if (isEnableBuiltInMetrics() && !isAdminClient) { + ApiTracerFactory metricsTracerFactory = + getMetricsApiTracerFactory(isDirectPathChannelCreated); if (metricsTracerFactory != null) { apiTracerFactories.add(metricsTracerFactory); } } + return new CompositeTracerFactory(apiTracerFactories); } @@ -1691,18 +1696,16 @@ private ApiTracerFactory getDefaultApiTracerFactory() { return BaseApiTracerFactory.getInstance(); } - private ApiTracerFactory getMetricsApiTracerFactory(boolean canUseDirectPath) { - BuiltInOpenTelemetryMetricsProvider builtInOpenTelemetryMetricsProvider = - new BuiltInOpenTelemetryMetricsProvider(); + private ApiTracerFactory getMetricsApiTracerFactory(boolean isDirectPathChannelCreated) { OpenTelemetry openTelemetry = - builtInOpenTelemetryMetricsProvider.getOpenTelemetry( + this.builtInOpenTelemetryMetricsProvider.getOrCreateOpenTelemetry( getDefaultProjectId(), getCredentials()); return openTelemetry != null ? new MetricsTracerFactory( new OpenTelemetryMetricsRecorder(openTelemetry, BuiltInMetricsConstant.METER_NAME), builtInOpenTelemetryMetricsProvider.getClientAttributes( - getDefaultProjectId(), canUseDirectPath)) + getDefaultProjectId(), isDirectPathChannelCreated)) : null; } @@ -1716,9 +1719,8 @@ public boolean isEnableApiTracing() { } /** - * Returns true if an {@link com.google.api.gax.tracing.ApiTracer} should be created and set on - * the Spanner client. Enabling this only has effect if an OpenTelemetry or OpenCensus trace - * exporter has been configured. + * Returns true if an {@link com.google.api.gax.tracing.MetricsTracer} should be created and set + * on the Spanner client. */ boolean isEnableBuiltInMetrics() { return enableBuiltInMetrics; 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 4c442f89566..6a276db1cf4 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 @@ -376,7 +376,9 @@ public GapicSpannerRpc(final SpannerOptions options) { TransportChannelProvider channelProvider = MoreObjects.firstNonNull(options.getChannelProvider(), defaultChannelProvider); - options.toBuilder().canUseDirectPath(defaultChannelProvider.canUseDirectPath()).build(); + boolean isDirectPathChannelCreated = + defaultChannelProvider.canUseDirectPath() + && defaultChannelProvider.isDirectPathXdsEnabled(); CredentialsProvider credentialsProvider = GrpcTransportOptions.setUpCredentialsProvider(options); @@ -402,7 +404,8 @@ public GapicSpannerRpc(final SpannerOptions options) { .setTransportChannelProvider(channelProvider) .setCredentialsProvider(credentialsProvider) .setStreamWatchdogProvider(watchdogProvider) - .setTracerFactory(options.getApiTracerFactory()) + .setTracerFactory( + options.getApiTracerFactory(isDirectPathChannelCreated, false)) .build()); this.readRetrySettings = options.getSpannerStubSettings().streamingReadSettings().getRetrySettings(); @@ -430,7 +433,7 @@ public GapicSpannerRpc(final SpannerOptions options) { .setTransportChannelProvider(channelProvider) .setCredentialsProvider(credentialsProvider) .setStreamWatchdogProvider(watchdogProvider) - .setTracerFactory(options.getApiTracerFactory()) + .setTracerFactory(options.getApiTracerFactory(isDirectPathChannelCreated, false)) .executeSqlSettings() .setRetrySettings(partitionedDmlRetrySettings); pdmlSettings.executeStreamingSqlSettings().setRetrySettings(partitionedDmlRetrySettings); @@ -457,7 +460,7 @@ public GapicSpannerRpc(final SpannerOptions options) { .setTransportChannelProvider(channelProvider) .setCredentialsProvider(credentialsProvider) .setStreamWatchdogProvider(watchdogProvider) - .setTracerFactory(options.getApiTracerFactory()) + .setTracerFactory(options.getApiTracerFactory(isDirectPathChannelCreated, true)) .build(); this.instanceAdminStub = GrpcInstanceAdminStub.create(instanceAdminStubSettings); @@ -468,7 +471,7 @@ public GapicSpannerRpc(final SpannerOptions options) { .setTransportChannelProvider(channelProvider) .setCredentialsProvider(credentialsProvider) .setStreamWatchdogProvider(watchdogProvider) - .setTracerFactory(options.getApiTracerFactory()) + .setTracerFactory(options.getApiTracerFactory(isDirectPathChannelCreated, true)) .build(); // Automatically retry RESOURCE_EXHAUSTED for GetOperation if auto-throttling of 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 99245b00958..202bb675a66 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 @@ -99,10 +99,12 @@ public void start(Listener responseListener, Metadata headers) { DatabaseName databaseName = extractDatabaseName(headers); String key = databaseName + method.getFullMethodName(); TagContext tagContext = getTagContext(key, method.getFullMethodName(), databaseName); - CompositeTracer compositeTracer = (CompositeTracer) callOptions.getOption(TRACER_KEY); + if (callOptions.getOption(TRACER_KEY) instanceof CompositeTracer) { + CompositeTracer compositeTracer = (CompositeTracer) callOptions.getOption(TRACER_KEY); + addBuiltInMetricAttributes(compositeTracer, databaseName); + } Attributes attributes = getMetricAttributes(key, method.getFullMethodName(), databaseName); - addBuiltInMetricAttributes(compositeTracer, databaseName); super.start( new SimpleForwardingClientCallListener(responseListener) { @Override 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 8e397f0172b..a227be223ff 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 @@ -79,7 +79,8 @@ public class OpenTelemetryBuiltInMetricsTracerTest extends AbstractMockServerTes @BeforeClass public static void setup() { metricReader = InMemoryMetricReader.create(); - BuiltInOpenTelemetryMetricsProvider provider = new BuiltInOpenTelemetryMetricsProvider(); + + BuiltInOpenTelemetryMetricsProvider provider = BuiltInOpenTelemetryMetricsProvider.INSTANCE; SdkMeterProviderBuilder meterProvider = SdkMeterProvider.builder().registerMetricReader(metricReader); @@ -87,12 +88,12 @@ public static void setup() { BuiltInMetricsConstant.getAllViews().forEach(meterProvider::registerView); openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(meterProvider.build()).build(); - boolean canUseDirectPath = true; - attributes = provider.getClientAttributes("test-project", canUseDirectPath); + attributes = provider.getClientAttributes("test-project", true); expectedBaseAttributes = Attributes.builder() .put(BuiltInMetricsConstant.PROJECT_ID_KEY, "test-project") + .put(BuiltInMetricsConstant.INSTANCE_CONFIG_ID_KEY, "unknown") .put(BuiltInMetricsConstant.DIRECT_PATH_ENABLED_KEY, "true") .put(BuiltInMetricsConstant.LOCATION_ID_KEY, "global") .put( 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 869e8f4d2a8..d2624e97ab1 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 @@ -23,6 +23,7 @@ import com.google.cloud.spanner.Database; import com.google.cloud.spanner.DatabaseClient; import com.google.cloud.spanner.IntegrationTestEnv; +import com.google.cloud.spanner.ParallelIntegrationTest; import com.google.cloud.spanner.Statement; import com.google.common.base.Stopwatch; import com.google.monitoring.v3.ListTimeSeriesRequest; @@ -36,13 +37,17 @@ import java.util.concurrent.TimeUnit; import org.junit.BeforeClass; import org.junit.ClassRule; +import org.junit.Ignore; import org.junit.Test; +import org.junit.experimental.categories.Category; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.threeten.bp.Duration; import org.threeten.bp.Instant; +@Category(ParallelIntegrationTest.class) @RunWith(JUnit4.class) +@Ignore("Built-in Metrics are not GA'ed yet. Enable this test once the metrics are released") public class ITBuiltInMetricsTest { private static Database db; @@ -64,7 +69,7 @@ public static void setUp() throws IOException { public void testBuiltinMetricsWithDefaultOTEL() throws Exception { // This stopwatch is used for to limit fetching of metric data in verifyMetrics Stopwatch metricsPollingStopwatch = Stopwatch.createStarted(); - Instant start = Instant.now().minus(Duration.ofMinutes(10)); + Instant start = Instant.now().minus(Duration.ofMinutes(2)); Instant end = Instant.now().plus(Duration.ofMinutes(3)); ProjectName name = ProjectName.of(env.getTestHelper().getOptions().getProjectId()); @@ -104,7 +109,7 @@ public void testBuiltinMetricsWithDefaultOTEL() throws Exception { ListTimeSeriesResponse response = metricClient.listTimeSeriesCallable().call(request); while (response.getTimeSeriesCount() == 0 - && metricsPollingStopwatch.elapsed(TimeUnit.MINUTES) < 10) { + && metricsPollingStopwatch.elapsed(TimeUnit.MINUTES) < 3) { // Call listTimeSeries every minute Thread.sleep(Duration.ofMinutes(1).toMillis()); response = metricClient.listTimeSeriesCallable().call(request); From a7aeb5766a64f875e82b2b093e56954ad8500e6e Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Wed, 14 Aug 2024 15:22:27 +0530 Subject: [PATCH 3/9] Few issues and code optimisations --- .../BuiltInOpenTelemetryMetricsProvider.java | 19 +++++---- .../google/cloud/spanner/SpannerOptions.java | 17 ++++---- .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 40 +++++++++++++------ .../spanner/spi/v1/HeaderInterceptor.java | 5 ++- ...OpenTelemetryBuiltInMetricsTracerTest.java | 11 +++-- 5 files changed, 57 insertions(+), 35 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 4aff7bccd0c..b006938bede 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 @@ -16,6 +16,7 @@ package com.google.cloud.spanner; +import static com.google.cloud.opentelemetry.detection.GCPPlatformDetector.SupportedPlatform.GOOGLE_KUBERNETES_ENGINE; import static com.google.cloud.spanner.BuiltInMetricsConstant.CLIENT_NAME_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.CLIENT_UID_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.DIRECT_PATH_ENABLED_KEY; @@ -23,8 +24,8 @@ import static com.google.cloud.spanner.BuiltInMetricsConstant.LOCATION_ID_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.PROJECT_ID_KEY; -import com.google.api.gax.core.GaxProperties; import com.google.auth.Credentials; +import com.google.cloud.opentelemetry.detection.AttributeKeys; import com.google.cloud.opentelemetry.detection.DetectedPlatform; import com.google.cloud.opentelemetry.detection.GCPPlatformDetector; import io.opentelemetry.api.OpenTelemetry; @@ -76,7 +77,8 @@ OpenTelemetry getOrCreateOpenTelemetry(String projectId, @Nullable Credentials c } } - Map getClientAttributes(String projectId, boolean isDirectPathChannelCreated) { + Map getClientAttributes( + String projectId, boolean isDirectPathChannelCreated, String client_name) { Map clientAttributes = new HashMap<>(); clientAttributes.put(LOCATION_ID_KEY.getKey(), detectClientLocation()); clientAttributes.put(PROJECT_ID_KEY.getKey(), projectId); @@ -84,18 +86,19 @@ Map getClientAttributes(String projectId, boolean isDirectPathCh clientAttributes.put(INSTANCE_CONFIG_ID_KEY.getKey(), "unknown"); clientAttributes.put( DIRECT_PATH_ENABLED_KEY.getKey(), String.valueOf(isDirectPathChannelCreated)); - clientAttributes.put( - CLIENT_NAME_KEY.getKey(), - "spanner-java/" - + GaxProperties.getLibraryVersion(SpannerCloudMonitoringExporterUtils.class)); + clientAttributes.put(CLIENT_NAME_KEY.getKey(), client_name); clientAttributes.put(CLIENT_UID_KEY.getKey(), getDefaultTaskValue()); return clientAttributes; } - private String detectClientLocation() { + static String detectClientLocation() { GCPPlatformDetector detector = GCPPlatformDetector.DEFAULT_INSTANCE; DetectedPlatform detectedPlatform = detector.detectPlatform(); - String region = detectedPlatform.getAttributes().get("cloud.region"); + // All platform except GKE uses "cloud_region" for region attribute. + String region = detectedPlatform.getAttributes().get("cloud_region"); + if (detectedPlatform.getSupportedPlatform() == GOOGLE_KUBERNETES_ENGINE) { + region = detectedPlatform.getAttributes().get(AttributeKeys.GKE_LOCATION_TYPE_REGION); + } return region == null ? "global" : region; } 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 29f17688fb8..c16653546c0 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 @@ -1653,23 +1653,24 @@ public OpenTelemetry getOpenTelemetry() { @Override public ApiTracerFactory getApiTracerFactory() { - return createApiTracerFactory(false, false); + return createApiTracerFactory(false, false, false); } public ApiTracerFactory getApiTracerFactory( - boolean isDirectPathChannelCreated, boolean isAdminClient) { - return createApiTracerFactory(isDirectPathChannelCreated, isAdminClient); + boolean isDirectPathChannelCreated, boolean isAdminClient, boolean isEmulatorEnabled) { + return createApiTracerFactory(isDirectPathChannelCreated, isAdminClient, isEmulatorEnabled); } private ApiTracerFactory createApiTracerFactory( - boolean isDirectPathChannelCreated, boolean isAdminClient) { + boolean isDirectPathChannelCreated, boolean isAdminClient, boolean isEmulatorEnabled) { List apiTracerFactories = new ArrayList<>(); // Prefer any direct ApiTracerFactory that might have been set on the builder. apiTracerFactories.add( MoreObjects.firstNonNull(super.getApiTracerFactory(), getDefaultApiTracerFactory())); - // Add Metrics Tracer factory if enabled and if data client - if (isEnableBuiltInMetrics() && !isAdminClient) { + // Add Metrics Tracer factory if built in metrics are enabled and if the client is data client + // and if emulator is not enabled. + if (isEnableBuiltInMetrics() && !isAdminClient && !isEmulatorEnabled) { ApiTracerFactory metricsTracerFactory = getMetricsApiTracerFactory(isDirectPathChannelCreated); if (metricsTracerFactory != null) { @@ -1705,7 +1706,9 @@ private ApiTracerFactory getMetricsApiTracerFactory(boolean isDirectPathChannelC ? new MetricsTracerFactory( new OpenTelemetryMetricsRecorder(openTelemetry, BuiltInMetricsConstant.METER_NAME), builtInOpenTelemetryMetricsProvider.getClientAttributes( - getDefaultProjectId(), isDirectPathChannelCreated)) + getDefaultProjectId(), + isDirectPathChannelCreated, + "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 6a276db1cf4..a1ca92e5111 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 @@ -395,6 +395,8 @@ public GapicSpannerRpc(final SpannerOptions options) { .withCheckInterval(checkInterval) .withClock(NanoClock.getDefaultClock()); + final String emulatorHost = System.getenv("SPANNER_EMULATOR_HOST"); + try { this.spannerStub = GrpcSpannerStub.create( @@ -405,7 +407,10 @@ public GapicSpannerRpc(final SpannerOptions options) { .setCredentialsProvider(credentialsProvider) .setStreamWatchdogProvider(watchdogProvider) .setTracerFactory( - options.getApiTracerFactory(isDirectPathChannelCreated, false)) + options.getApiTracerFactory( + isDirectPathChannelCreated, + false, + isEmulatorEnabled(options, emulatorHost))) .build()); this.readRetrySettings = options.getSpannerStubSettings().streamingReadSettings().getRetrySettings(); @@ -433,7 +438,9 @@ public GapicSpannerRpc(final SpannerOptions options) { .setTransportChannelProvider(channelProvider) .setCredentialsProvider(credentialsProvider) .setStreamWatchdogProvider(watchdogProvider) - .setTracerFactory(options.getApiTracerFactory(isDirectPathChannelCreated, false)) + .setTracerFactory( + options.getApiTracerFactory( + isDirectPathChannelCreated, false, isEmulatorEnabled(options, emulatorHost))) .executeSqlSettings() .setRetrySettings(partitionedDmlRetrySettings); pdmlSettings.executeStreamingSqlSettings().setRetrySettings(partitionedDmlRetrySettings); @@ -460,7 +467,9 @@ public GapicSpannerRpc(final SpannerOptions options) { .setTransportChannelProvider(channelProvider) .setCredentialsProvider(credentialsProvider) .setStreamWatchdogProvider(watchdogProvider) - .setTracerFactory(options.getApiTracerFactory(isDirectPathChannelCreated, true)) + .setTracerFactory( + options.getApiTracerFactory( + isDirectPathChannelCreated, true, isEmulatorEnabled(options, emulatorHost))) .build(); this.instanceAdminStub = GrpcInstanceAdminStub.create(instanceAdminStubSettings); @@ -471,7 +480,9 @@ public GapicSpannerRpc(final SpannerOptions options) { .setTransportChannelProvider(channelProvider) .setCredentialsProvider(credentialsProvider) .setStreamWatchdogProvider(watchdogProvider) - .setTracerFactory(options.getApiTracerFactory(isDirectPathChannelCreated, true)) + .setTracerFactory( + options.getApiTracerFactory( + isDirectPathChannelCreated, true, isEmulatorEnabled(options, emulatorHost))) .build(); // Automatically retry RESOURCE_EXHAUSTED for GetOperation if auto-throttling of @@ -515,7 +526,7 @@ public UnaryCallable createUnaryCalla // Check whether the SPANNER_EMULATOR_HOST env var has been set, and if so, if the emulator // is actually running. - checkEmulatorConnection(options, channelProvider, credentialsProvider); + checkEmulatorConnection(options, channelProvider, credentialsProvider, emulatorHost); } catch (Exception e) { throw newSpannerException(e); } @@ -614,15 +625,11 @@ private static HeaderProvider headerProviderWithUserAgentFrom(HeaderProvider hea private static void checkEmulatorConnection( SpannerOptions options, TransportChannelProvider channelProvider, - CredentialsProvider credentialsProvider) + CredentialsProvider credentialsProvider, + String emulatorHost) throws IOException { - final String emulatorHost = System.getenv("SPANNER_EMULATOR_HOST"); // Only do the check if the emulator environment variable has been set to localhost. - if (options.getChannelProvider() == null - && emulatorHost != null - && options.getHost() != null - && options.getHost().startsWith("http://localhost") - && options.getHost().endsWith(emulatorHost)) { + if (isEmulatorEnabled(options, emulatorHost)) { // Do a quick check to see if the emulator is actually running. try { InstanceAdminStubSettings.Builder testEmulatorSettings = @@ -655,6 +662,15 @@ private static void checkEmulatorConnection( } } + private static boolean isEmulatorEnabled(SpannerOptions options, String emulatorHost) { + // Only do the check if the emulator environment variable has been set to localhost. + return options.getChannelProvider() == null + && emulatorHost != null + && options.getHost() != null + && options.getHost().startsWith("http://localhost") + && options.getHost().endsWith(emulatorHost); + } + private static final RetrySettings ADMIN_REQUESTS_LIMIT_EXCEEDED_RETRY_SETTINGS = RetrySettings.newBuilder() .setInitialRetryDelay(Duration.ofSeconds(5L)) 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 202bb675a66..20e9f7b3e2f 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 @@ -91,6 +91,7 @@ class HeaderInterceptor implements ClientInterceptor { @Override public ClientCall interceptCall( MethodDescriptor method, CallOptions callOptions, Channel next) { + Object tracer = callOptions.getOption(TRACER_KEY); return new SimpleForwardingClientCall(next.newCall(method, callOptions)) { @Override public void start(Listener responseListener, Metadata headers) { @@ -99,8 +100,8 @@ public void start(Listener responseListener, Metadata headers) { DatabaseName databaseName = extractDatabaseName(headers); String key = databaseName + method.getFullMethodName(); TagContext tagContext = getTagContext(key, method.getFullMethodName(), databaseName); - if (callOptions.getOption(TRACER_KEY) instanceof CompositeTracer) { - CompositeTracer compositeTracer = (CompositeTracer) callOptions.getOption(TRACER_KEY); + if (tracer instanceof CompositeTracer) { + CompositeTracer compositeTracer = (CompositeTracer) tracer; addBuiltInMetricAttributes(compositeTracer, databaseName); } Attributes attributes = 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 a227be223ff..791e87ec2b9 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 @@ -21,7 +21,6 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import com.google.api.gax.core.GaxProperties; import com.google.api.gax.longrunning.OperationTimedPollAlgorithm; import com.google.api.gax.retrying.RetrySettings; import com.google.api.gax.tracing.ApiTracerFactory; @@ -87,19 +86,19 @@ public static void setup() { BuiltInMetricsConstant.getAllViews().forEach(meterProvider::registerView); + String client_name = "spanner-java/"; openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(meterProvider.build()).build(); - attributes = provider.getClientAttributes("test-project", true); + attributes = provider.getClientAttributes("test-project", true, client_name); expectedBaseAttributes = Attributes.builder() .put(BuiltInMetricsConstant.PROJECT_ID_KEY, "test-project") .put(BuiltInMetricsConstant.INSTANCE_CONFIG_ID_KEY, "unknown") .put(BuiltInMetricsConstant.DIRECT_PATH_ENABLED_KEY, "true") - .put(BuiltInMetricsConstant.LOCATION_ID_KEY, "global") .put( - BuiltInMetricsConstant.CLIENT_NAME_KEY, - "spanner-java/" - + GaxProperties.getLibraryVersion(SpannerCloudMonitoringExporterUtils.class)) + BuiltInMetricsConstant.LOCATION_ID_KEY, + BuiltInOpenTelemetryMetricsProvider.detectClientLocation()) + .put(BuiltInMetricsConstant.CLIENT_NAME_KEY, client_name) .put(BuiltInMetricsConstant.CLIENT_UID_KEY, attributes.get("client_uid")) .build(); } From a6532a54369807d338dd8a758983df7ac4ffe321 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Fri, 16 Aug 2024 15:16:32 +0530 Subject: [PATCH 4/9] review comments --- .../BuiltInOpenTelemetryMetricsProvider.java | 5 ++-- .../google/cloud/spanner/SpannerOptions.java | 9 +++--- .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 29 ++++++++++++------- ...OpenTelemetryBuiltInMetricsTracerTest.java | 8 ++--- .../spanner/it/ITBuiltInMetricsTest.java | 13 +-------- 5 files changed, 30 insertions(+), 34 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 b006938bede..58f98db22c7 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 @@ -46,8 +46,7 @@ final class BuiltInOpenTelemetryMetricsProvider { - public static BuiltInOpenTelemetryMetricsProvider INSTANCE = - new BuiltInOpenTelemetryMetricsProvider(); + static BuiltInOpenTelemetryMetricsProvider INSTANCE = new BuiltInOpenTelemetryMetricsProvider(); private static final Logger logger = Logger.getLogger(BuiltInOpenTelemetryMetricsProvider.class.getName()); @@ -77,7 +76,7 @@ OpenTelemetry getOrCreateOpenTelemetry(String projectId, @Nullable Credentials c } } - Map getClientAttributes( + Map createClientAttributes( String projectId, boolean isDirectPathChannelCreated, String client_name) { Map clientAttributes = new HashMap<>(); clientAttributes.put(LOCATION_ID_KEY.getKey(), detectClientLocation()); 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 c16653546c0..8968b7ecde4 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 @@ -1394,8 +1394,7 @@ public Builder setEnableApiTracing(boolean enableApiTracing) { } /** Enabling this will enable built in metrics for each individual RPC execution. */ - @VisibleForTesting - public Builder setEnableBuiltInMetrics(boolean enableBuiltInMetrics) { + Builder setEnableBuiltInMetrics(boolean enableBuiltInMetrics) { this.enableBuiltInMetrics = enableBuiltInMetrics; return this; } @@ -1672,7 +1671,7 @@ private ApiTracerFactory createApiTracerFactory( // and if emulator is not enabled. if (isEnableBuiltInMetrics() && !isAdminClient && !isEmulatorEnabled) { ApiTracerFactory metricsTracerFactory = - getMetricsApiTracerFactory(isDirectPathChannelCreated); + createMetricsApiTracerFactory(isDirectPathChannelCreated); if (metricsTracerFactory != null) { apiTracerFactories.add(metricsTracerFactory); } @@ -1697,7 +1696,7 @@ private ApiTracerFactory getDefaultApiTracerFactory() { return BaseApiTracerFactory.getInstance(); } - private ApiTracerFactory getMetricsApiTracerFactory(boolean isDirectPathChannelCreated) { + private ApiTracerFactory createMetricsApiTracerFactory(boolean isDirectPathChannelCreated) { OpenTelemetry openTelemetry = this.builtInOpenTelemetryMetricsProvider.getOrCreateOpenTelemetry( getDefaultProjectId(), getCredentials()); @@ -1705,7 +1704,7 @@ private ApiTracerFactory getMetricsApiTracerFactory(boolean isDirectPathChannelC return openTelemetry != null ? new MetricsTracerFactory( new OpenTelemetryMetricsRecorder(openTelemetry, BuiltInMetricsConstant.METER_NAME), - builtInOpenTelemetryMetricsProvider.getClientAttributes( + builtInOpenTelemetryMetricsProvider.createClientAttributes( getDefaultProjectId(), isDirectPathChannelCreated, "spanner-java/" + GaxProperties.getLibraryVersion(getClass()))) 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 a1ca92e5111..d4779ba956f 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 @@ -370,16 +370,19 @@ public GapicSpannerRpc(final SpannerOptions options) { // If it is enabled in options uses the channel pool provided by the gRPC-GCP extension. maybeEnableGrpcGcpExtension(defaultChannelProviderBuilder, options); - InstantiatingGrpcChannelProvider defaultChannelProvider = - defaultChannelProviderBuilder.build(); + InstantiatingGrpcChannelProvider defaultChannelProvider = null; + boolean isDirectPathChannelCreated = false; + + if (options.getChannelProvider() == null) { + defaultChannelProvider = defaultChannelProviderBuilder.build(); + isDirectPathChannelCreated = + defaultChannelProvider.canUseDirectPath() + && defaultChannelProvider.isDirectPathXdsEnabled(); + } TransportChannelProvider channelProvider = MoreObjects.firstNonNull(options.getChannelProvider(), defaultChannelProvider); - boolean isDirectPathChannelCreated = - defaultChannelProvider.canUseDirectPath() - && defaultChannelProvider.isDirectPathXdsEnabled(); - CredentialsProvider credentialsProvider = GrpcTransportOptions.setUpCredentialsProvider(options); @@ -409,7 +412,7 @@ public GapicSpannerRpc(final SpannerOptions options) { .setTracerFactory( options.getApiTracerFactory( isDirectPathChannelCreated, - false, + /* isAdminClient = */ false, isEmulatorEnabled(options, emulatorHost))) .build()); this.readRetrySettings = @@ -440,7 +443,9 @@ public GapicSpannerRpc(final SpannerOptions options) { .setStreamWatchdogProvider(watchdogProvider) .setTracerFactory( options.getApiTracerFactory( - isDirectPathChannelCreated, false, isEmulatorEnabled(options, emulatorHost))) + isDirectPathChannelCreated, + /* isAdminClient = */ false, + isEmulatorEnabled(options, emulatorHost))) .executeSqlSettings() .setRetrySettings(partitionedDmlRetrySettings); pdmlSettings.executeStreamingSqlSettings().setRetrySettings(partitionedDmlRetrySettings); @@ -469,7 +474,9 @@ isDirectPathChannelCreated, false, isEmulatorEnabled(options, emulatorHost))) .setStreamWatchdogProvider(watchdogProvider) .setTracerFactory( options.getApiTracerFactory( - isDirectPathChannelCreated, true, isEmulatorEnabled(options, emulatorHost))) + isDirectPathChannelCreated, + /* isAdminClient = */ true, + isEmulatorEnabled(options, emulatorHost))) .build(); this.instanceAdminStub = GrpcInstanceAdminStub.create(instanceAdminStubSettings); @@ -482,7 +489,9 @@ isDirectPathChannelCreated, true, isEmulatorEnabled(options, emulatorHost))) .setStreamWatchdogProvider(watchdogProvider) .setTracerFactory( options.getApiTracerFactory( - isDirectPathChannelCreated, true, isEmulatorEnabled(options, emulatorHost))) + isDirectPathChannelCreated, + /* isAdminClient = */ true, + isEmulatorEnabled(options, emulatorHost))) .build(); // Automatically retry RESOURCE_EXHAUSTED for GetOperation if auto-throttling of 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 791e87ec2b9..7bfdb99ca90 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 @@ -88,7 +88,7 @@ public static void setup() { String client_name = "spanner-java/"; openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(meterProvider.build()).build(); - attributes = provider.getClientAttributes("test-project", true, client_name); + attributes = provider.createClientAttributes("test-project", true, client_name); expectedBaseAttributes = Attributes.builder() @@ -240,7 +240,7 @@ private MetricData getMetricData(InMemoryMetricReader reader, String metricName) Collection allMetricData = Collections.emptyList(); // Fetch the MetricData with retries - for (int attemptsLeft = 10; attemptsLeft > 0; attemptsLeft--) { + for (int attemptsLeft = 1000; attemptsLeft > 0; attemptsLeft--) { allMetricData = reader.collectAllMetrics(); List matchingMetadata = allMetricData.stream() @@ -257,14 +257,14 @@ private MetricData getMetricData(InMemoryMetricReader reader, String metricName) } try { - Thread.sleep(100); + Thread.sleep(1); } catch (InterruptedException interruptedException) { Thread.currentThread().interrupt(); throw new RuntimeException(interruptedException); } } - assertFalse(String.format("MetricData is missing for metric {0}", fullMetricName), false); + assertTrue(String.format("MetricData is missing for metric {0}", fullMetricName), false); return null; } 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 d2624e97ab1..9ff7e06e813 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 @@ -18,7 +18,6 @@ import static com.google.common.truth.Truth.assertWithMessage; -import com.google.api.gax.longrunning.OperationFuture; import com.google.cloud.monitoring.v3.MetricServiceClient; import com.google.cloud.spanner.Database; import com.google.cloud.spanner.DatabaseClient; @@ -31,9 +30,7 @@ import com.google.monitoring.v3.ProjectName; import com.google.monitoring.v3.TimeInterval; import com.google.protobuf.util.Timestamps; -import com.google.spanner.admin.database.v1.UpdateDatabaseDdlMetadata; import java.io.IOException; -import java.util.Collections; import java.util.concurrent.TimeUnit; import org.junit.BeforeClass; import org.junit.ClassRule; @@ -60,7 +57,7 @@ public class ITBuiltInMetricsTest { @BeforeClass public static void setUp() throws IOException { metricClient = MetricServiceClient.create(); - env.getTestHelper().getOptions().toBuilder().setEnableBuiltInMetrics(true); + // Enable BuiltinMetrics when the metrics are GA'ed db = env.getTestHelper().createTestDatabase(); client = env.getTestHelper().getDatabaseClient(db); } @@ -78,14 +75,6 @@ public void testBuiltinMetricsWithDefaultOTEL() throws Exception { .setStartTime(Timestamps.fromMillis(start.toEpochMilli())) .setEndTime(Timestamps.fromMillis(end.toEpochMilli())) .build(); - String ddl = - "CREATE TABLE FOO (" - + " K STRING(MAX) NOT NULL," - + " V INT64," - + ") PRIMARY KEY (K)"; - OperationFuture op = - db.updateDdl(Collections.singletonList(ddl), null); - op.get(); client .readWriteTransaction() From eeed7d39c7cc5163ebd3cbc110dcb0872743a50b Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Wed, 21 Aug 2024 11:42:52 +0530 Subject: [PATCH 5/9] directpath_used attribute --- .../cloud/spanner/BuiltInMetricsConstant.java | 4 ++ .../google/cloud/spanner/CompositeTracer.java | 12 +++- .../spanner/spi/v1/HeaderInterceptor.java | 61 +++++++++++++++---- 3 files changed, 65 insertions(+), 12 deletions(-) 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 4bb71b680ff..d4e183a84c9 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 @@ -76,6 +76,10 @@ public class BuiltInMetricsConstant { public static final AttributeKey DIRECT_PATH_USED_KEY = AttributeKey.stringKey("directpath_used"); + // IP address prefixes allocated for DirectPath backends. + public static final String DP_IPV6_PREFIX = "2001:4860:8040"; + public static final String DP_IPV4_PREFIX = "34.126"; + public static final Set COMMON_ATTRIBUTES = ImmutableSet.of( PROJECT_ID_KEY, 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 07d1310e91b..085a91fb88e 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 @@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableList; import java.util.ArrayList; import java.util.List; +import java.util.Map; import org.threeten.bp.Duration; @InternalApi @@ -177,5 +178,14 @@ public void addAttributes(String key, String value) { metricsTracer.addAttributes(key, value); } } - }; + } + + public void addAttributes(Map attributes) { + for (ApiTracer child : children) { + if (child instanceof MetricsTracer) { + MetricsTracer metricsTracer = (MetricsTracer) child; + attributes.forEach((key, value) -> metricsTracer.addAttributes(key, value)); + } + } + } } 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 20e9f7b3e2f..8b345cd6600 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 @@ -23,6 +23,7 @@ import static com.google.cloud.spanner.spi.v1.SpannerRpcViews.SPANNER_GFE_HEADER_MISSING_COUNT; import static com.google.cloud.spanner.spi.v1.SpannerRpcViews.SPANNER_GFE_LATENCY; +import com.google.api.gax.tracing.ApiTracer; import com.google.cloud.spanner.BuiltInMetricsConstant; import com.google.cloud.spanner.CompositeTracer; import com.google.cloud.spanner.SpannerExceptionFactory; @@ -36,6 +37,7 @@ import io.grpc.ClientInterceptor; import io.grpc.ForwardingClientCall.SimpleForwardingClientCall; import io.grpc.ForwardingClientCallListener.SimpleForwardingClientCallListener; +import io.grpc.Grpc; import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.opencensus.stats.MeasureMap; @@ -48,6 +50,11 @@ import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.api.trace.Span; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.SocketAddress; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.ExecutionException; import java.util.logging.Level; import java.util.logging.Logger; @@ -75,6 +82,8 @@ class HeaderInterceptor implements ClientInterceptor { CacheBuilder.newBuilder().maximumSize(1000).build(); private final Cache attributesCache = CacheBuilder.newBuilder().maximumSize(1000).build(); + private final Cache> builtInAttributesCache = + CacheBuilder.newBuilder().maximumSize(1000).build(); // Get the global singleton Tagger object. private static final Tagger TAGGER = Tags.getTagger(); @@ -91,7 +100,9 @@ class HeaderInterceptor implements ClientInterceptor { @Override public ClientCall interceptCall( MethodDescriptor method, CallOptions callOptions, Channel next) { - Object tracer = callOptions.getOption(TRACER_KEY); + ApiTracer tracer = callOptions.getOption(TRACER_KEY); + CompositeTracer compositeTracer = + tracer instanceof CompositeTracer ? (CompositeTracer) tracer : null; return new SimpleForwardingClientCall(next.newCall(method, callOptions)) { @Override public void start(Listener responseListener, Metadata headers) { @@ -100,16 +111,18 @@ public void start(Listener responseListener, Metadata headers) { DatabaseName databaseName = extractDatabaseName(headers); String key = databaseName + method.getFullMethodName(); TagContext tagContext = getTagContext(key, method.getFullMethodName(), databaseName); - if (tracer instanceof CompositeTracer) { - CompositeTracer compositeTracer = (CompositeTracer) tracer; - addBuiltInMetricAttributes(compositeTracer, databaseName); - } Attributes attributes = getMetricAttributes(key, method.getFullMethodName(), databaseName); + Map builtInMetricsAttributes = + getBuiltInMetricAttributes(key, databaseName); super.start( new SimpleForwardingClientCallListener(responseListener) { @Override public void onHeaders(Metadata metadata) { + addBuiltInMetricAttributes( + compositeTracer, + builtInMetricsAttributes, + getAttributes().get(Grpc.TRANSPORT_ATTR_REMOTE_ADDR)); processHeader(metadata, tagContext, attributes, span); super.onHeaders(metadata); } @@ -206,12 +219,38 @@ private Attributes getMetricAttributes(String key, String method, DatabaseName d }); } + private Map getBuiltInMetricAttributes(String key, DatabaseName databaseName) + throws ExecutionException { + return builtInAttributesCache.get( + key, + () -> { + Map attributes = new HashMap<>(); + attributes.put(BuiltInMetricsConstant.DATABASE_KEY.getKey(), databaseName.getDatabase()); + attributes.put( + BuiltInMetricsConstant.INSTANCE_ID_KEY.getKey(), databaseName.getInstance()); + return attributes; + }); + } + private void addBuiltInMetricAttributes( - CompositeTracer compositeTracer, DatabaseName databaseName) { - // Built in metrics Attributes. - compositeTracer.addAttributes( - BuiltInMetricsConstant.DATABASE_KEY.getKey(), databaseName.getDatabase()); - compositeTracer.addAttributes( - BuiltInMetricsConstant.INSTANCE_ID_KEY.getKey(), databaseName.getInstance()); + CompositeTracer compositeTracer, Map attributes, SocketAddress remoteAddr) { + if (compositeTracer != null) { + // Direct Path used attribute + attributes.put( + BuiltInMetricsConstant.DIRECT_PATH_USED_KEY.getKey(), + Boolean.toString(isDirectPathUsed(remoteAddr))); + + compositeTracer.addAttributes(attributes); + } + } + + private Boolean isDirectPathUsed(SocketAddress remoteAddr) { + if (remoteAddr instanceof InetSocketAddress) { + InetAddress inetAddress = ((InetSocketAddress) remoteAddr).getAddress(); + String addr = inetAddress.getHostAddress(); + return addr.startsWith(BuiltInMetricsConstant.DP_IPV4_PREFIX) + || addr.startsWith(BuiltInMetricsConstant.DP_IPV6_PREFIX); + } + return false; } } From e8a587f1b206334fc4b5a54cbb7dafc9f683e81b Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Wed, 4 Sep 2024 11:14:10 +0530 Subject: [PATCH 6/9] removed directpath enabled attribute --- .../BuiltInOpenTelemetryMetricsProvider.java | 6 +--- .../google/cloud/spanner/SpannerOptions.java | 15 ++++------ .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 29 ++++--------------- .../spanner/spi/v1/HeaderInterceptor.java | 11 ++++--- ...OpenTelemetryBuiltInMetricsTracerTest.java | 3 +- 5 files changed, 19 insertions(+), 45 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 58f98db22c7..a980204950a 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 @@ -19,7 +19,6 @@ import static com.google.cloud.opentelemetry.detection.GCPPlatformDetector.SupportedPlatform.GOOGLE_KUBERNETES_ENGINE; import static com.google.cloud.spanner.BuiltInMetricsConstant.CLIENT_NAME_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.CLIENT_UID_KEY; -import static com.google.cloud.spanner.BuiltInMetricsConstant.DIRECT_PATH_ENABLED_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.INSTANCE_CONFIG_ID_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.LOCATION_ID_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.PROJECT_ID_KEY; @@ -76,15 +75,12 @@ OpenTelemetry getOrCreateOpenTelemetry(String projectId, @Nullable Credentials c } } - Map createClientAttributes( - String projectId, boolean isDirectPathChannelCreated, 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); // TODO: Replace this with real value. clientAttributes.put(INSTANCE_CONFIG_ID_KEY.getKey(), "unknown"); - clientAttributes.put( - DIRECT_PATH_ENABLED_KEY.getKey(), String.valueOf(isDirectPathChannelCreated)); clientAttributes.put(CLIENT_NAME_KEY.getKey(), client_name); clientAttributes.put(CLIENT_UID_KEY.getKey(), getDefaultTaskValue()); return clientAttributes; 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 8968b7ecde4..1a9f2c49a42 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 @@ -1652,16 +1652,15 @@ public OpenTelemetry getOpenTelemetry() { @Override public ApiTracerFactory getApiTracerFactory() { - return createApiTracerFactory(false, false, false); + return createApiTracerFactory(false, false); } - public ApiTracerFactory getApiTracerFactory( - boolean isDirectPathChannelCreated, boolean isAdminClient, boolean isEmulatorEnabled) { - return createApiTracerFactory(isDirectPathChannelCreated, isAdminClient, isEmulatorEnabled); + public ApiTracerFactory getApiTracerFactory(boolean isAdminClient, boolean isEmulatorEnabled) { + return createApiTracerFactory(isAdminClient, isEmulatorEnabled); } private ApiTracerFactory createApiTracerFactory( - boolean isDirectPathChannelCreated, boolean isAdminClient, boolean isEmulatorEnabled) { + boolean isAdminClient, boolean isEmulatorEnabled) { List apiTracerFactories = new ArrayList<>(); // Prefer any direct ApiTracerFactory that might have been set on the builder. apiTracerFactories.add( @@ -1670,8 +1669,7 @@ private ApiTracerFactory createApiTracerFactory( // Add Metrics Tracer factory if built in metrics are enabled and if the client is data client // and if emulator is not enabled. if (isEnableBuiltInMetrics() && !isAdminClient && !isEmulatorEnabled) { - ApiTracerFactory metricsTracerFactory = - createMetricsApiTracerFactory(isDirectPathChannelCreated); + ApiTracerFactory metricsTracerFactory = createMetricsApiTracerFactory(); if (metricsTracerFactory != null) { apiTracerFactories.add(metricsTracerFactory); } @@ -1696,7 +1694,7 @@ private ApiTracerFactory getDefaultApiTracerFactory() { return BaseApiTracerFactory.getInstance(); } - private ApiTracerFactory createMetricsApiTracerFactory(boolean isDirectPathChannelCreated) { + private ApiTracerFactory createMetricsApiTracerFactory() { OpenTelemetry openTelemetry = this.builtInOpenTelemetryMetricsProvider.getOrCreateOpenTelemetry( getDefaultProjectId(), getCredentials()); @@ -1706,7 +1704,6 @@ private ApiTracerFactory createMetricsApiTracerFactory(boolean isDirectPathChann new OpenTelemetryMetricsRecorder(openTelemetry, BuiltInMetricsConstant.METER_NAME), builtInOpenTelemetryMetricsProvider.createClientAttributes( getDefaultProjectId(), - isDirectPathChannelCreated, "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 d4779ba956f..b389ea6a31a 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 @@ -370,18 +370,9 @@ public GapicSpannerRpc(final SpannerOptions options) { // If it is enabled in options uses the channel pool provided by the gRPC-GCP extension. maybeEnableGrpcGcpExtension(defaultChannelProviderBuilder, options); - InstantiatingGrpcChannelProvider defaultChannelProvider = null; - boolean isDirectPathChannelCreated = false; - - if (options.getChannelProvider() == null) { - defaultChannelProvider = defaultChannelProviderBuilder.build(); - isDirectPathChannelCreated = - defaultChannelProvider.canUseDirectPath() - && defaultChannelProvider.isDirectPathXdsEnabled(); - } - TransportChannelProvider channelProvider = - MoreObjects.firstNonNull(options.getChannelProvider(), defaultChannelProvider); + MoreObjects.firstNonNull( + options.getChannelProvider(), defaultChannelProviderBuilder.build()); CredentialsProvider credentialsProvider = GrpcTransportOptions.setUpCredentialsProvider(options); @@ -411,9 +402,7 @@ public GapicSpannerRpc(final SpannerOptions options) { .setStreamWatchdogProvider(watchdogProvider) .setTracerFactory( options.getApiTracerFactory( - isDirectPathChannelCreated, - /* isAdminClient = */ false, - isEmulatorEnabled(options, emulatorHost))) + /* isAdminClient = */ false, isEmulatorEnabled(options, emulatorHost))) .build()); this.readRetrySettings = options.getSpannerStubSettings().streamingReadSettings().getRetrySettings(); @@ -443,9 +432,7 @@ public GapicSpannerRpc(final SpannerOptions options) { .setStreamWatchdogProvider(watchdogProvider) .setTracerFactory( options.getApiTracerFactory( - isDirectPathChannelCreated, - /* isAdminClient = */ false, - isEmulatorEnabled(options, emulatorHost))) + /* isAdminClient = */ false, isEmulatorEnabled(options, emulatorHost))) .executeSqlSettings() .setRetrySettings(partitionedDmlRetrySettings); pdmlSettings.executeStreamingSqlSettings().setRetrySettings(partitionedDmlRetrySettings); @@ -474,9 +461,7 @@ public GapicSpannerRpc(final SpannerOptions options) { .setStreamWatchdogProvider(watchdogProvider) .setTracerFactory( options.getApiTracerFactory( - isDirectPathChannelCreated, - /* isAdminClient = */ true, - isEmulatorEnabled(options, emulatorHost))) + /* isAdminClient = */ true, isEmulatorEnabled(options, emulatorHost))) .build(); this.instanceAdminStub = GrpcInstanceAdminStub.create(instanceAdminStubSettings); @@ -489,9 +474,7 @@ public GapicSpannerRpc(final SpannerOptions options) { .setStreamWatchdogProvider(watchdogProvider) .setTracerFactory( options.getApiTracerFactory( - isDirectPathChannelCreated, - /* isAdminClient = */ true, - isEmulatorEnabled(options, emulatorHost))) + /* isAdminClient = */ true, isEmulatorEnabled(options, emulatorHost))) .build(); // Automatically retry RESOURCE_EXHAUSTED for GetOperation if auto-throttling of 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 8b345cd6600..96ce68b1ee1 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 @@ -119,10 +119,10 @@ public void start(Listener responseListener, Metadata headers) { new SimpleForwardingClientCallListener(responseListener) { @Override public void onHeaders(Metadata metadata) { + Boolean isDirectPathUsed = + isDirectPathUsed(getAttributes().get(Grpc.TRANSPORT_ATTR_REMOTE_ADDR)); addBuiltInMetricAttributes( - compositeTracer, - builtInMetricsAttributes, - getAttributes().get(Grpc.TRANSPORT_ATTR_REMOTE_ADDR)); + compositeTracer, builtInMetricsAttributes, isDirectPathUsed); processHeader(metadata, tagContext, attributes, span); super.onHeaders(metadata); } @@ -233,12 +233,11 @@ private Map getBuiltInMetricAttributes(String key, DatabaseName } private void addBuiltInMetricAttributes( - CompositeTracer compositeTracer, Map attributes, SocketAddress remoteAddr) { + CompositeTracer compositeTracer, Map attributes, Boolean isDirectPathUsed) { if (compositeTracer != null) { // Direct Path used attribute attributes.put( - BuiltInMetricsConstant.DIRECT_PATH_USED_KEY.getKey(), - Boolean.toString(isDirectPathUsed(remoteAddr))); + BuiltInMetricsConstant.DIRECT_PATH_USED_KEY.getKey(), Boolean.toString(isDirectPathUsed)); compositeTracer.addAttributes(attributes); } 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 7bfdb99ca90..51d334c1726 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 @@ -88,13 +88,12 @@ public static void setup() { String client_name = "spanner-java/"; openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(meterProvider.build()).build(); - attributes = provider.createClientAttributes("test-project", true, client_name); + attributes = provider.createClientAttributes("test-project", client_name); expectedBaseAttributes = Attributes.builder() .put(BuiltInMetricsConstant.PROJECT_ID_KEY, "test-project") .put(BuiltInMetricsConstant.INSTANCE_CONFIG_ID_KEY, "unknown") - .put(BuiltInMetricsConstant.DIRECT_PATH_ENABLED_KEY, "true") .put( BuiltInMetricsConstant.LOCATION_ID_KEY, BuiltInOpenTelemetryMetricsProvider.detectClientLocation()) From b2f93619adf6469caf74f24fc7a85953c3ac8bd0 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Wed, 4 Sep 2024 11:14:10 +0530 Subject: [PATCH 7/9] removed directpath enabled attribute --- .../com/google/cloud/spanner/spi/v1/HeaderInterceptor.java | 3 ++- 1 file changed, 2 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 96ce68b1ee1..1805df5cdc8 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 @@ -233,9 +233,10 @@ private Map getBuiltInMetricAttributes(String key, DatabaseName } private void addBuiltInMetricAttributes( - CompositeTracer compositeTracer, Map attributes, Boolean isDirectPathUsed) { + CompositeTracer compositeTracer, Map builtInMetricsAttributes, Boolean isDirectPathUsed) { if (compositeTracer != null) { // Direct Path used attribute + Map attributes = new HashMap<>(builtInMetricsAttributes); attributes.put( BuiltInMetricsConstant.DIRECT_PATH_USED_KEY.getKey(), Boolean.toString(isDirectPathUsed)); From cf8e92d8efd11572723b1ae90b8936d3720cabd6 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Thu, 19 Sep 2024 10:53:22 +0530 Subject: [PATCH 8/9] lint --- .../main/java/com/google/cloud/spanner/SpannerOptions.java | 2 ++ .../com/google/cloud/spanner/spi/v1/HeaderInterceptor.java | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) 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 1a9f2c49a42..3bfa3ee4069 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 @@ -748,6 +748,8 @@ public boolean isEnableApiTracing() { @Override public boolean isEnableBuiltInMetrics() { + // The environment variable SPANNER_ENABLE_BUILTIN_METRICS is used for testing and will be + // removed in the future. return Boolean.parseBoolean(System.getenv(SPANNER_ENABLE_BUILTIN_METRICS)); } } 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 1805df5cdc8..dd414bed397 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 @@ -233,7 +233,9 @@ private Map getBuiltInMetricAttributes(String key, DatabaseName } private void addBuiltInMetricAttributes( - CompositeTracer compositeTracer, Map builtInMetricsAttributes, Boolean isDirectPathUsed) { + CompositeTracer compositeTracer, + Map builtInMetricsAttributes, + Boolean isDirectPathUsed) { if (compositeTracer != null) { // Direct Path used attribute Map attributes = new HashMap<>(builtInMetricsAttributes); From 8259d03d398469f086f7d32af4cda4ce69801e4a Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Thu, 19 Sep 2024 11:18:05 +0530 Subject: [PATCH 9/9] bucket boundaries --- .../com/google/cloud/spanner/BuiltInMetricsConstant.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) 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 d4e183a84c9..90dfaef0e63 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 @@ -97,10 +97,11 @@ public class BuiltInMetricsConstant { static Aggregation AGGREGATION_WITH_MILLIS_HISTOGRAM = Aggregation.explicitBucketHistogram( ImmutableList.of( - 0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 8.0, 10.0, 13.0, 16.0, 20.0, 25.0, 30.0, 40.0, - 50.0, 65.0, 80.0, 100.0, 130.0, 160.0, 200.0, 250.0, 300.0, 400.0, 500.0, 650.0, - 800.0, 1000.0, 2000.0, 5000.0, 10000.0, 20000.0, 50000.0, 100000.0, 200000.0, - 400000.0, 800000.0, 1600000.0, 3200000.0)); + 0.0, 0.5, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 11.0, 12.0, 13.0, 14.0, + 15.0, 16.0, 17.0, 18.0, 19.0, 20.0, 25.0, 30.0, 40.0, 50.0, 65.0, 80.0, 100.0, 130.0, + 160.0, 200.0, 250.0, 300.0, 400.0, 500.0, 650.0, 800.0, 1000.0, 2000.0, 5000.0, + 10000.0, 20000.0, 50000.0, 100000.0, 200000.0, 400000.0, 800000.0, 1600000.0, + 3200000.0)); static Map getAllViews() { ImmutableMap.Builder views = ImmutableMap.builder();