From 5806943f504454aed03302f1a9df8896c5016053 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Tue, 25 Mar 2025 10:50:07 +0530 Subject: [PATCH 01/13] feat: grpc metrics --- google-cloud-spanner/pom.xml | 4 ++ .../cloud/spanner/BuiltInMetricsConstant.java | 54 ++++++++++++++-- .../cloud/spanner/BuiltInMetricsProvider.java | 61 ++++++++++++++++--- .../SpannerCloudMonitoringExporter.java | 27 +++++--- .../SpannerCloudMonitoringExporterUtils.java | 32 +++++++--- .../google/cloud/spanner/SpannerOptions.java | 9 ++- .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 5 ++ ...OpenTelemetryBuiltInMetricsTracerTest.java | 8 +-- .../SpannerCloudMonitoringExporterTest.java | 42 ++++++++----- .../spanner/it/ITBuiltInMetricsTest.java | 2 +- 10 files changed, 189 insertions(+), 55 deletions(-) diff --git a/google-cloud-spanner/pom.xml b/google-cloud-spanner/pom.xml index eed3735b857..909b96698c5 100644 --- a/google-cloud-spanner/pom.xml +++ b/google-cloud-spanner/pom.xml @@ -191,6 +191,10 @@ io.grpc grpc-stub + + io.grpc + grpc-opentelemetry + com.google.api api-common 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 4adf53d7e40..7d34f11198e 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 @@ -21,11 +21,13 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; 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.Collection; import java.util.Map; import java.util.Set; import java.util.stream.Collectors; @@ -36,6 +38,7 @@ public class BuiltInMetricsConstant { public static final String METER_NAME = "spanner.googleapis.com/internal/client"; public static final String GAX_METER_NAME = OpenTelemetryMetricsRecorder.GAX_METER_NAME; static final String SPANNER_METER_NAME = "spanner-java"; + static final String GRPC_METER_NAME = "grpc-java"; static final String GFE_LATENCIES_NAME = "gfe_latencies"; static final String OPERATION_LATENCIES_NAME = "operation_latencies"; static final String ATTEMPT_LATENCIES_NAME = "attempt_latencies"; @@ -66,12 +69,7 @@ public class BuiltInMetricsConstant { // These metric labels will be promoted to the spanner monitored resource fields public static final Set> SPANNER_PROMOTED_RESOURCE_LABELS = - ImmutableSet.of( - PROJECT_ID_KEY, - INSTANCE_ID_KEY, - INSTANCE_CONFIG_ID_KEY, - LOCATION_ID_KEY, - CLIENT_HASH_KEY); + ImmutableSet.of(INSTANCE_ID_KEY); public static final AttributeKey DATABASE_KEY = AttributeKey.stringKey("database"); public static final AttributeKey CLIENT_UID_KEY = AttributeKey.stringKey("client_uid"); @@ -102,6 +100,11 @@ public class BuiltInMetricsConstant { DIRECT_PATH_ENABLED_KEY, DIRECT_PATH_USED_KEY); + public static final Set GRPC_ATTRIBUTES = + ImmutableSet.of( + "grpc_lb_rls_data_plane_target", + "grpc_lb_pick_result"); + static Aggregation AGGREGATION_WITH_MILLIS_HISTOGRAM = Aggregation.explicitBucketHistogram( ImmutableList.of( @@ -111,6 +114,21 @@ public class BuiltInMetricsConstant { 10000.0, 20000.0, 50000.0, 100000.0, 200000.0, 400000.0, 800000.0, 1600000.0, 3200000.0)); + static final Collection GRPC_METRICS_TO_ENABLE = + ImmutableList.of( + "grpc.lb.rls.default_target_picks", + "grpc.lb.rls.target_picks", + "grpc.xds_client.server_failure", + "grpc.xds_client.resource_updates_invalid"); + + static final Collection GRPC_METRICS_ENABLED_BY_DEFAULT = + ImmutableList.of( + "grpc.client.attempt.sent_total_compressed_message_size", + "grpc.client.attempt.rcvd_total_compressed_message_size", + "grpc.client.attempt.started", + "grpc.client.attempt.duration", + "grpc.client.call.duration"); + static Map getAllViews() { ImmutableMap.Builder views = ImmutableMap.builder(); defineView( @@ -153,6 +171,7 @@ static Map getAllViews() { Aggregation.sum(), InstrumentType.COUNTER, "1"); + defineGRPCView(views); return views.build(); } @@ -183,4 +202,27 @@ private static void defineView( .build(); viewMap.put(selector, view); } + + private static void defineGRPCView(ImmutableMap.Builder viewMap) { + for (String metric : + ImmutableList.copyOf(Iterables.concat(BuiltInMetricsConstant.GRPC_METRICS_TO_ENABLE, BuiltInMetricsConstant.GRPC_METRICS_ENABLED_BY_DEFAULT))) { + InstrumentSelector selector = + InstrumentSelector.builder() + .setName(metric) + .setMeterName(BuiltInMetricsConstant.GRPC_METER_NAME) + .build(); + Set attributesFilter = + BuiltInMetricsConstant.COMMON_ATTRIBUTES.stream() + .map(AttributeKey::getKey) + .collect(Collectors.toSet()); + + attributesFilter.addAll(BuiltInMetricsConstant.GRPC_ATTRIBUTES); + View view = + View.builder() + .setName(BuiltInMetricsConstant.METER_NAME + '/' + metric.replace(".", "/")) + .setAttributeFilter(attributesFilter) + .build(); + viewMap.put(selector, view); + } + } } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsProvider.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsProvider.java index f624f310f77..1be27b29bc6 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsProvider.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsProvider.java @@ -24,16 +24,28 @@ import static com.google.cloud.spanner.BuiltInMetricsConstant.LOCATION_ID_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.PROJECT_ID_KEY; +import com.google.api.core.ApiFunction; +import com.google.api.gax.core.GaxProperties; +import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; 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 com.google.common.collect.ImmutableList; +import com.google.common.collect.Iterables; import com.google.common.hash.HashFunction; import com.google.common.hash.Hashing; +import io.grpc.ManagedChannelBuilder; +import io.grpc.opentelemetry.GrpcOpenTelemetry; import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.sdk.OpenTelemetrySdk; +import io.opentelemetry.sdk.metrics.InstrumentSelector; import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder; +import io.opentelemetry.sdk.metrics.View; +import io.opentelemetry.sdk.resources.Resource; import java.io.IOException; import java.lang.management.ManagementFactory; import java.lang.reflect.Method; @@ -66,6 +78,9 @@ OpenTelemetry getOrCreateOpenTelemetry( BuiltInMetricsView.registerBuiltinMetrics( SpannerCloudMonitoringExporter.create(projectId, credentials, monitoringHost), sdkMeterProviderBuilder); + + + sdkMeterProviderBuilder.setResource(Resource.create(createResourceAttributes(projectId))); SdkMeterProvider sdkMeterProvider = sdkMeterProviderBuilder.build(); this.openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProvider).build(); Runtime.getRuntime().addShutdownHook(new Thread(sdkMeterProvider::close)); @@ -80,15 +95,45 @@ OpenTelemetry getOrCreateOpenTelemetry( } } - Map createClientAttributes(String projectId, String client_name) { + public void enableGrpcMetrics( + InstantiatingGrpcChannelProvider.Builder channelProviderBuilder, + String projectId, + @Nullable Credentials credentials, + @Nullable String monitoringHost) { + GrpcOpenTelemetry grpcOpenTelemetry = + GrpcOpenTelemetry.newBuilder() + .sdk(this.getOrCreateOpenTelemetry(projectId, credentials, monitoringHost)) + .enableMetrics(BuiltInMetricsConstant.GRPC_METRICS_TO_ENABLE) + // .disableMetrics(BuiltInMetricsConstant.GRPC_METRICS_ENABLED_BY_DEFAULT) + .build(); + ApiFunction channelConfigurator = + channelProviderBuilder.getChannelConfigurator(); + channelProviderBuilder.setChannelConfigurator( + b -> { + grpcOpenTelemetry.configureChannelBuilder(b); + if (channelConfigurator != null) { + return channelConfigurator.apply(b); + } + return b; + }); + } + + Attributes createResourceAttributes(String projectId) { + AttributesBuilder attributesBuilder = + Attributes.builder() + .put(PROJECT_ID_KEY.getKey(), projectId) + .put(INSTANCE_CONFIG_ID_KEY.getKey(), "unknown") + .put(CLIENT_HASH_KEY.getKey(), generateClientHash(getDefaultTaskValue())) + .put(LOCATION_ID_KEY.getKey(), detectClientLocation()); + + return attributesBuilder.build(); + } + + Map createClientAttributes() { Map clientAttributes = new HashMap<>(); - clientAttributes.put(LOCATION_ID_KEY.getKey(), detectClientLocation()); - clientAttributes.put(PROJECT_ID_KEY.getKey(), projectId); - clientAttributes.put(INSTANCE_CONFIG_ID_KEY.getKey(), "unknown"); - clientAttributes.put(CLIENT_NAME_KEY.getKey(), client_name); - String clientUid = getDefaultTaskValue(); - clientAttributes.put(CLIENT_UID_KEY.getKey(), clientUid); - clientAttributes.put(CLIENT_HASH_KEY.getKey(), generateClientHash(clientUid)); + clientAttributes.put( + CLIENT_NAME_KEY.getKey(), "spanner-java/" + GaxProperties.getLibraryVersion(getClass())); + clientAttributes.put(CLIENT_UID_KEY.getKey(), getDefaultTaskValue()); return clientAttributes; } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java index f9c91b91833..5ada1711f50 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java @@ -41,6 +41,7 @@ import io.opentelemetry.sdk.metrics.data.MetricData; import io.opentelemetry.sdk.metrics.data.PointData; import io.opentelemetry.sdk.metrics.export.MetricExporter; +import io.opentelemetry.sdk.resources.Resource; import java.io.IOException; import java.time.Duration; import java.util.ArrayList; @@ -104,6 +105,18 @@ static SpannerCloudMonitoringExporter create( @Override public CompletableResultCode export(@Nonnull Collection collection) { + // Print + collection.stream() + .forEach(md -> { + System.out.println("Name: " + md.getName()); // Print the name + + md.getData().getPoints().forEach(point -> { + System.out.println("Attributes: " + point.getAttributes()); // Print attributes for each point + }); + + System.out.println("----------------------"); // Separator for readability + }); + if (client.isShutdown()) { logger.log(Level.WARNING, "Exporter is shut down"); return CompletableResultCode.ofFailure(); @@ -123,7 +136,7 @@ private CompletableResultCode exportSpannerClientMetrics(Collection // Log warnings for metrics that will be skipped. boolean mustFilter = false; if (spannerMetricData.stream() - .flatMap(metricData -> metricData.getData().getPoints().stream()) + .map(metricData -> metricData.getResource()) .anyMatch(this::shouldSkipPointDataDueToProjectId)) { logger.log( Level.WARNING, "Some metric data contain a different projectId. These will be skipped."); @@ -198,15 +211,13 @@ public void onSuccess(List empty) { } private boolean shouldSkipMetricData(MetricData metricData) { - return metricData.getData().getPoints().stream() - .anyMatch( - pd -> - shouldSkipPointDataDueToProjectId(pd) - || shouldSkipPointDataDueToMissingInstanceId(pd)); + return shouldSkipPointDataDueToProjectId(metricData.getResource()) + || metricData.getData().getPoints().stream() + .anyMatch(pd -> shouldSkipPointDataDueToMissingInstanceId(pd)); } - private boolean shouldSkipPointDataDueToProjectId(PointData pointData) { - return !spannerProjectId.equals(SpannerCloudMonitoringExporterUtils.getProjectId(pointData)); + private boolean shouldSkipPointDataDueToProjectId(Resource resource) { + return !spannerProjectId.equals(SpannerCloudMonitoringExporterUtils.getProjectId(resource)); } private boolean shouldSkipPointDataDueToMissingInstanceId(PointData pointData) { diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java index 620430b87df..0784df4e291 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java @@ -23,6 +23,7 @@ import static com.google.api.MetricDescriptor.ValueType.DOUBLE; import static com.google.api.MetricDescriptor.ValueType.INT64; import static com.google.cloud.spanner.BuiltInMetricsConstant.GAX_METER_NAME; +import static com.google.cloud.spanner.BuiltInMetricsConstant.GRPC_METER_NAME; import static com.google.cloud.spanner.BuiltInMetricsConstant.INSTANCE_ID_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.PROJECT_ID_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.SPANNER_METER_NAME; @@ -52,6 +53,7 @@ import io.opentelemetry.sdk.metrics.data.MetricDataType; import io.opentelemetry.sdk.metrics.data.PointData; import io.opentelemetry.sdk.metrics.data.SumData; +import io.opentelemetry.sdk.resources.Resource; import java.util.ArrayList; import java.util.List; import java.util.logging.Level; @@ -64,8 +66,8 @@ class SpannerCloudMonitoringExporterUtils { private SpannerCloudMonitoringExporterUtils() {} - static String getProjectId(PointData pointData) { - return pointData.getAttributes().get(PROJECT_ID_KEY); + static String getProjectId(Resource resource) { + return resource.getAttributes().get(PROJECT_ID_KEY); } static String getInstanceId(PointData pointData) { @@ -78,12 +80,26 @@ static List convertToSpannerTimeSeries(List collection) for (MetricData metricData : collection) { // Get metrics data from GAX library and Spanner library if (!(metricData.getInstrumentationScopeInfo().getName().equals(GAX_METER_NAME) - || metricData.getInstrumentationScopeInfo().getName().equals(SPANNER_METER_NAME))) { + || metricData.getInstrumentationScopeInfo().getName().equals(SPANNER_METER_NAME) + || metricData.getInstrumentationScopeInfo().getName().equals(GRPC_METER_NAME))) { // Filter out metric data for instruments that are not part of the spanner metrics list continue; } + + // Create MonitoredResource Builder + MonitoredResource.Builder monitoredResourceBuilder = + MonitoredResource.newBuilder().setType(SPANNER_RESOURCE_TYPE); + + Attributes resourceAttributes = metricData.getResource().getAttributes(); + for (AttributeKey key : resourceAttributes.asMap().keySet()) { + monitoredResourceBuilder.putLabels( + key.getKey(), String.valueOf(resourceAttributes.get(key))); + } + metricData.getData().getPoints().stream() - .map(pointData -> convertPointToSpannerTimeSeries(metricData, pointData)) + .map( + pointData -> + convertPointToSpannerTimeSeries(metricData, pointData, monitoredResourceBuilder)) .forEach(allTimeSeries::add); } @@ -91,7 +107,9 @@ static List convertToSpannerTimeSeries(List collection) } private static TimeSeries convertPointToSpannerTimeSeries( - MetricData metricData, PointData pointData) { + MetricData metricData, + PointData pointData, + MonitoredResource.Builder monitoredResourceBuilder) { TimeSeries.Builder builder = TimeSeries.newBuilder() .setMetricKind(convertMetricKind(metricData)) @@ -99,8 +117,6 @@ private static TimeSeries convertPointToSpannerTimeSeries( Metric.Builder metricBuilder = Metric.newBuilder().setType(metricData.getName()); Attributes attributes = pointData.getAttributes(); - MonitoredResource.Builder monitoredResourceBuilder = - MonitoredResource.newBuilder().setType(SPANNER_RESOURCE_TYPE); for (AttributeKey key : attributes.asMap().keySet()) { if (SPANNER_PROMOTED_RESOURCE_LABELS.contains(key)) { @@ -110,6 +126,8 @@ private static TimeSeries convertPointToSpannerTimeSeries( } } + metricBuilder.putAllLabels(BuiltInMetricsProvider.INSTANCE.createClientAttributes()); + builder.setResource(monitoredResourceBuilder.build()); builder.setMetric(metricBuilder.build()); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index 695e156dfc3..a69fb3d2985 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 @@ -27,6 +27,7 @@ import com.google.api.gax.core.GaxProperties; import com.google.api.gax.grpc.GrpcCallContext; import com.google.api.gax.grpc.GrpcInterceptorProvider; +import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; import com.google.api.gax.longrunning.OperationTimedPollAlgorithm; import com.google.api.gax.retrying.RetrySettings; import com.google.api.gax.rpc.ApiCallContext; @@ -1971,6 +1972,11 @@ public ApiTracerFactory getApiTracerFactory() { return createApiTracerFactory(false, false); } + public void enablegRPCMetrics(InstantiatingGrpcChannelProvider.Builder channelProviderBuilder) { + this.builtInMetricsProvider.enableGrpcMetrics( + channelProviderBuilder, this.getProjectId(), getCredentials(), this.monitoringHost); + } + public ApiTracerFactory getApiTracerFactory(boolean isAdminClient, boolean isEmulatorEnabled) { return createApiTracerFactory(isAdminClient, isEmulatorEnabled); } @@ -2018,8 +2024,7 @@ private ApiTracerFactory createMetricsApiTracerFactory() { return openTelemetry != null ? new BuiltInMetricsTracerFactory( new BuiltInMetricsRecorder(openTelemetry, BuiltInMetricsConstant.METER_NAME), - builtInMetricsProvider.createClientAttributes( - this.getProjectId(), "spanner-java/" + GaxProperties.getLibraryVersion(getClass()))) + new HashMap<>()) : 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 0f51c9544f7..1088bf5cbf3 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 @@ -373,6 +373,11 @@ public GapicSpannerRpc(final SpannerOptions options) { defaultChannelProviderBuilder.setAttemptDirectPath(true); defaultChannelProviderBuilder.setAttemptDirectPathXds(); } + + // Use condition to enable gRPC metrics + if (true) { + options.enablegRPCMetrics(defaultChannelProviderBuilder); + } if (options.isUseVirtualThreads()) { ExecutorService executor = tryCreateVirtualThreadPerTaskExecutor("spanner-virtual-grpc-executor"); 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 f0c13b0f389..d2db2e4bca0 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 @@ -91,18 +91,12 @@ public static void setup() { String client_name = "spanner-java/"; openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(meterProvider.build()).build(); - attributes = provider.createClientAttributes("test-project", client_name); + attributes = provider.createClientAttributes(); expectedCommonBaseAttributes = Attributes.builder() - .put(BuiltInMetricsConstant.PROJECT_ID_KEY, "test-project") - .put(BuiltInMetricsConstant.INSTANCE_CONFIG_ID_KEY, "unknown") - .put( - BuiltInMetricsConstant.LOCATION_ID_KEY, - BuiltInMetricsProvider.detectClientLocation()) .put(BuiltInMetricsConstant.CLIENT_NAME_KEY, client_name) .put(BuiltInMetricsConstant.CLIENT_UID_KEY, attributes.get("client_uid")) - .put(BuiltInMetricsConstant.CLIENT_HASH_KEY, attributes.get("client_hash")) .put(BuiltInMetricsConstant.INSTANCE_ID_KEY, "i") .put(BuiltInMetricsConstant.DATABASE_KEY, "d") .put(BuiltInMetricsConstant.DIRECT_PATH_ENABLED_KEY, "false") diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java index f9a6e9df9a6..c7796cb0f4b 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java @@ -81,7 +81,7 @@ public class SpannerCloudMonitoringExporterTest { private static final String instanceId = "fake-instance"; private static final String locationId = "global"; private static final String databaseId = "fake-database"; - private static final String clientName = "spanner-java"; + private static final String clientName = "spanner-java/"; private static final String clientHash = "spanner-test"; private static final String instanceConfigId = "fake-instance-config-id"; @@ -93,28 +93,38 @@ public class SpannerCloudMonitoringExporterTest { private SpannerCloudMonitoringExporter exporter; private Attributes attributes; + + private Attributes resourceAttributes; private Resource resource; private InstrumentationScopeInfo scope; + private String client_uid; + @Before public void setUp() { fakeMetricServiceClient = new FakeMetricServiceClient(mockMetricServiceStub); exporter = new SpannerCloudMonitoringExporter(projectId, fakeMetricServiceClient); + this.client_uid = BuiltInMetricsProvider.INSTANCE.createClientAttributes().get("client_uid"); + attributes = Attributes.builder() - .put(PROJECT_ID_KEY, projectId) .put(INSTANCE_ID_KEY, instanceId) - .put(LOCATION_ID_KEY, locationId) - .put(INSTANCE_CONFIG_ID_KEY, instanceConfigId) .put(DATABASE_KEY, databaseId) .put(CLIENT_NAME_KEY, clientName) - .put(CLIENT_HASH_KEY, clientHash) + .put(CLIENT_UID_KEY, this.client_uid) .put(String.valueOf(DIRECT_PATH_ENABLED_KEY), true) .put(String.valueOf(DIRECT_PATH_USED_KEY), true) .build(); - resource = Resource.create(Attributes.empty()); + resourceAttributes = + Attributes.builder() + .put(PROJECT_ID_KEY, projectId) + .put(LOCATION_ID_KEY, locationId) + .put(CLIENT_HASH_KEY, clientHash) + .put(INSTANCE_CONFIG_ID_KEY, instanceConfigId) + .build(); + resource = Resource.create(resourceAttributes); scope = InstrumentationScopeInfo.create(GAX_METER_NAME); } @@ -177,8 +187,10 @@ public void testExportingSumData() { DIRECT_PATH_ENABLED_KEY.getKey(), "true", DIRECT_PATH_USED_KEY.getKey(), - "true"); - assertThat(timeSeries.getMetric().getLabelsMap()).hasSize(4); + "true", + CLIENT_UID_KEY.getKey(), + this.client_uid); + assertThat(timeSeries.getMetric().getLabelsMap()).hasSize(5); assertThat(timeSeries.getPoints(0).getValue().getInt64Value()).isEqualTo(fakeValue); assertThat(timeSeries.getPoints(0).getInterval().getStartTime().getNanos()) @@ -239,7 +251,7 @@ public void testExportingHistogramData() { INSTANCE_CONFIG_ID_KEY.getKey(), instanceConfigId, CLIENT_HASH_KEY.getKey(), clientHash); - assertThat(timeSeries.getMetric().getLabelsMap()).hasSize(4); + assertThat(timeSeries.getMetric().getLabelsMap()).hasSize(5); assertThat(timeSeries.getMetric().getLabelsMap()) .containsExactly( DATABASE_KEY.getKey(), @@ -249,7 +261,9 @@ public void testExportingHistogramData() { DIRECT_PATH_ENABLED_KEY.getKey(), "true", DIRECT_PATH_USED_KEY.getKey(), - "true"); + "true", + CLIENT_UID_KEY.getKey(), + this.client_uid); Distribution distribution = timeSeries.getPoints(0).getValue().getDistributionValue(); assertThat(distribution.getCount()).isEqualTo(3); @@ -274,11 +288,7 @@ public void testExportingSumDataInBatches() { Collection toExport = new ArrayList<>(); for (int i = 0; i < 250; i++) { LongPointData longPointData = - ImmutableLongPointData.create( - startEpoch, - endEpoch, - attributes.toBuilder().put(CLIENT_UID_KEY, "client_uid" + i).build(), - i); + ImmutableLongPointData.create(startEpoch, endEpoch, attributes, i); MetricData longData = ImmutableMetricData.createLongSum( @@ -331,7 +341,7 @@ public void testExportingSumDataInBatches() { DIRECT_PATH_USED_KEY.getKey(), "true", CLIENT_UID_KEY.getKey(), - "client_uid" + i); + this.client_uid); assertThat(timeSeries.getPoints(0).getValue().getInt64Value()).isEqualTo(i); assertThat(timeSeries.getPoints(0).getInterval().getStartTime().getNanos()) 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 5bf8e42ccb6..51650006d50 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITBuiltInMetricsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITBuiltInMetricsTest.java @@ -44,7 +44,7 @@ @Category(ParallelIntegrationTest.class) @RunWith(JUnit4.class) -@Ignore("Built-in Metrics are not GA'ed yet. Enable this test once the metrics are released") +// @Ignore("Built-in Metrics are not GA'ed yet. Enable this test once the metrics are released") public class ITBuiltInMetricsTest { private static Database db; From 4329770b408bc905a387216d7bccaa44716a7bc1 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Tue, 25 Mar 2025 16:49:51 +0530 Subject: [PATCH 02/13] test --- .../cloud/spanner/BuiltInMetricsConstant.java | 10 +++---- .../cloud/spanner/BuiltInMetricsProvider.java | 7 +---- .../SpannerCloudMonitoringExporter.java | 27 ++++++++++++------- .../SpannerCloudMonitoringExporterUtils.java | 3 ++- .../spanner/it/ITBuiltInMetricsTest.java | 1 - 5 files changed, 23 insertions(+), 25 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 7d34f11198e..09844a3a1ba 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 @@ -21,7 +21,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Iterables; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.sdk.metrics.Aggregation; import io.opentelemetry.sdk.metrics.InstrumentSelector; @@ -101,9 +100,7 @@ public class BuiltInMetricsConstant { DIRECT_PATH_USED_KEY); public static final Set GRPC_ATTRIBUTES = - ImmutableSet.of( - "grpc_lb_rls_data_plane_target", - "grpc_lb_pick_result"); + ImmutableSet.of("grpc.lb.rls.data_plane_target", "grpc.lb.pick_result"); static Aggregation AGGREGATION_WITH_MILLIS_HISTOGRAM = Aggregation.explicitBucketHistogram( @@ -204,8 +201,7 @@ private static void defineView( } private static void defineGRPCView(ImmutableMap.Builder viewMap) { - for (String metric : - ImmutableList.copyOf(Iterables.concat(BuiltInMetricsConstant.GRPC_METRICS_TO_ENABLE, BuiltInMetricsConstant.GRPC_METRICS_ENABLED_BY_DEFAULT))) { + for (String metric : BuiltInMetricsConstant.GRPC_METRICS_TO_ENABLE) { InstrumentSelector selector = InstrumentSelector.builder() .setName(metric) @@ -215,8 +211,8 @@ private static void defineGRPCView(ImmutableMap.Builder channelConfigurator = channelProviderBuilder.getChannelConfigurator(); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java index 5ada1711f50..147d838f224 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java @@ -105,17 +105,24 @@ static SpannerCloudMonitoringExporter create( @Override public CompletableResultCode export(@Nonnull Collection collection) { - // Print + // TODO: Remove collection.stream() - .forEach(md -> { - System.out.println("Name: " + md.getName()); // Print the name - - md.getData().getPoints().forEach(point -> { - System.out.println("Attributes: " + point.getAttributes()); // Print attributes for each point - }); - - System.out.println("----------------------"); // Separator for readability - }); + .filter(md -> "grpc-java".equals(md.getInstrumentationScopeInfo().getName())) + .forEach( + md -> { + System.out.println("Name: " + md.getName()); // Print the name + + md.getData() + .getPoints() + .forEach( + point -> { + System.out.println( + "Attributes: " + + point.getAttributes()); // Print attributes for each point + }); + + System.out.println("----------------------"); // Separator for readability + }); if (client.isShutdown()) { logger.log(Level.WARNING, "Exporter is shut down"); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java index 0784df4e291..e73e4060f03 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java @@ -122,7 +122,8 @@ private static TimeSeries convertPointToSpannerTimeSeries( if (SPANNER_PROMOTED_RESOURCE_LABELS.contains(key)) { monitoredResourceBuilder.putLabels(key.getKey(), String.valueOf(attributes.get(key))); } else { - metricBuilder.putLabels(key.getKey(), String.valueOf(attributes.get(key))); + metricBuilder.putLabels( + key.getKey().replace(".", "/"), String.valueOf(attributes.get(key))); } } 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 51650006d50..9b2f51de9c0 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 @@ -36,7 +36,6 @@ 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; From 1f87e9169e1e272c74703c06c86983b6d902baf5 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Wed, 26 Mar 2025 16:04:32 +0530 Subject: [PATCH 03/13] grpc metrics env check --- .../google/cloud/spanner/SpannerOptions.java | 18 ++++- .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 6 +- .../spanner/it/ITBuiltInMetricsTest.java | 76 +++++++++++-------- 3 files changed, 63 insertions(+), 37 deletions(-) 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 a69fb3d2985..d8707dece5e 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 @@ -849,6 +849,10 @@ default boolean isEnableBuiltInMetrics() { return true; } + default boolean isEnableGRPCBuiltInMetrics() { + return false; + } + default boolean isEnableEndToEndTracing() { return false; } @@ -879,6 +883,8 @@ private static class SpannerEnvironmentImpl implements SpannerEnvironment { private static final String SPANNER_ENABLE_END_TO_END_TRACING = "SPANNER_ENABLE_END_TO_END_TRACING"; private static final String SPANNER_DISABLE_BUILTIN_METRICS = "SPANNER_DISABLE_BUILTIN_METRICS"; + private static final String SPANNER_DISABLE_DIRECT_ACCESS_GRPC_BUILTIN_METRICS = + "SPANNER_DISABLE_DIRECT_ACCESS_GRPC_BUILTIN_METRICS"; private static final String SPANNER_MONITORING_HOST = "SPANNER_MONITORING_HOST"; private SpannerEnvironmentImpl() {} @@ -911,6 +917,12 @@ public boolean isEnableBuiltInMetrics() { return !Boolean.parseBoolean(System.getenv(SPANNER_DISABLE_BUILTIN_METRICS)); } + @Override + public boolean isEnableGRPCBuiltInMetrics() { + return !Boolean.parseBoolean( + System.getenv(SPANNER_DISABLE_DIRECT_ACCESS_GRPC_BUILTIN_METRICS)); + } + @Override public boolean isEnableEndToEndTracing() { return Boolean.parseBoolean(System.getenv(SPANNER_ENABLE_END_TO_END_TRACING)); @@ -1973,8 +1985,10 @@ public ApiTracerFactory getApiTracerFactory() { } public void enablegRPCMetrics(InstantiatingGrpcChannelProvider.Builder channelProviderBuilder) { - this.builtInMetricsProvider.enableGrpcMetrics( - channelProviderBuilder, this.getProjectId(), getCredentials(), this.monitoringHost); + if (SpannerOptions.environment.isEnableGRPCBuiltInMetrics()) { + this.builtInMetricsProvider.enableGrpcMetrics( + channelProviderBuilder, this.getProjectId(), getCredentials(), this.monitoringHost); + } } public ApiTracerFactory getApiTracerFactory(boolean isAdminClient, boolean isEmulatorEnabled) { 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 1088bf5cbf3..2a0ac1839dd 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 @@ -374,10 +374,8 @@ public GapicSpannerRpc(final SpannerOptions options) { defaultChannelProviderBuilder.setAttemptDirectPathXds(); } - // Use condition to enable gRPC metrics - if (true) { - options.enablegRPCMetrics(defaultChannelProviderBuilder); - } + options.enablegRPCMetrics(defaultChannelProviderBuilder); + if (options.isUseVirtualThreads()) { ExecutorService executor = tryCreateVirtualThreadPerTaskExecutor("spanner-virtual-grpc-executor"); 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 9b2f51de9c0..9fa19e0f6d0 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 @@ -17,6 +17,7 @@ package com.google.cloud.spanner.it; import static com.google.common.truth.Truth.assertWithMessage; +import static org.junit.Assume.assumeFalse; import com.google.cloud.monitoring.v3.MetricServiceClient; import com.google.cloud.spanner.Database; @@ -24,6 +25,7 @@ import com.google.cloud.spanner.IntegrationTestEnv; import com.google.cloud.spanner.ParallelIntegrationTest; import com.google.cloud.spanner.Statement; +import com.google.cloud.spanner.testing.EmulatorSpannerHelper; import com.google.common.base.Stopwatch; import com.google.monitoring.v3.ListTimeSeriesRequest; import com.google.monitoring.v3.ListTimeSeriesResponse; @@ -34,6 +36,7 @@ import java.time.Duration; import java.time.Instant; import java.util.concurrent.TimeUnit; +import org.junit.After; import org.junit.BeforeClass; import org.junit.ClassRule; import org.junit.Test; @@ -43,7 +46,6 @@ @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; @@ -53,6 +55,10 @@ public class ITBuiltInMetricsTest { private static MetricServiceClient metricClient; + private static String[] METRICS = { + "operation_latencies", "attempt_latencies", "operation_count", "attempt_count", + }; + @BeforeClass public static void setUp() throws IOException { metricClient = MetricServiceClient.create(); @@ -61,8 +67,16 @@ public static void setUp() throws IOException { client = env.getTestHelper().getDatabaseClient(db); } + @After + public void tearDown() { + if (metricClient != null) { + metricClient.close(); + } + } + @Test public void testBuiltinMetricsWithDefaultOTEL() throws Exception { + assumeFalse("This test requires credentials", EmulatorSpannerHelper.isUsingEmulator()); // This stopwatch is used for to limit fetching of metric data in verifyMetrics Stopwatch metricsPollingStopwatch = Stopwatch.createStarted(); Instant start = Instant.now().minus(Duration.ofMinutes(2)); @@ -79,36 +93,36 @@ public void testBuiltinMetricsWithDefaultOTEL() throws Exception { .readWriteTransaction() .run(transaction -> transaction.executeQuery(Statement.of("Select 1"))); - String metricFilter = - String.format( - "metric.type=\"spanner.googleapis.com/client/%s\"" - + " AND resource.type=\"spanner_instance\"" - + " AND metric.labels.method=\"Spanner.Commit\"" - + " AND resource.labels.instance_id=\"%s\"" - + " AND metric.labels.database=\"%s\"", - "operation_latencies", - db.getId().getInstanceId().getInstance(), - db.getId().getDatabase()); - - 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) < 3) { - // Call listTimeSeries every minute - Thread.sleep(Duration.ofMinutes(1).toMillis()); - response = metricClient.listTimeSeriesCallable().call(request); + for (String metric : METRICS) { + String metricFilter = + String.format( + "metric.type=\"spanner.googleapis.com/client/%s\"" + + " AND resource.type=\"spanner_instance\"" + + " AND metric.labels.method=\"Spanner.Commit\"" + + " AND resource.labels.instance_id=\"%s\"" + + " AND metric.labels.database=\"%s\"", + metric, db.getId().getInstanceId().getInstance(), db.getId().getDatabase()); + + 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) < 3) { + // Call listTimeSeries every minute + Thread.sleep(Duration.ofMinutes(1).toMillis()); + response = metricClient.listTimeSeriesCallable().call(request); + } + + assertWithMessage("Metric" + metric + "didn't return any data.") + .that(response.getTimeSeriesCount()) + .isGreaterThan(0); } - - assertWithMessage("View operation_latencies didn't return any data.") - .that(response.getTimeSeriesCount()) - .isGreaterThan(0); } } From c18fec22b527f905d2d4db29a56d3dd3c0d1c95b Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Wed, 26 Mar 2025 16:15:00 +0530 Subject: [PATCH 04/13] skip test and clirr --- google-cloud-spanner/clirr-ignored-differences.xml | 13 ++++++++++--- .../cloud/spanner/it/ITBuiltInMetricsTest.java | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/google-cloud-spanner/clirr-ignored-differences.xml b/google-cloud-spanner/clirr-ignored-differences.xml index b6b19d1d577..d6d36b0e147 100644 --- a/google-cloud-spanner/clirr-ignored-differences.xml +++ b/google-cloud-spanner/clirr-ignored-differences.xml @@ -751,6 +751,13 @@ boolean isEnableBuiltInMetrics() + + + 7012 + com/google/cloud/spanner/SpannerOptions$SpannerEnvironment + boolean isEnableGRPCBuiltInMetrics() + + 7012 @@ -807,7 +814,7 @@ com/google/cloud/spanner/connection/Connection boolean isKeepTransactionAlive() - + 7012 @@ -839,7 +846,7 @@ com/google/cloud/spanner/connection/Connection boolean isAutoBatchDmlUpdateCountVerification() - + 7012 @@ -863,7 +870,7 @@ com/google/cloud/spanner/connection/Connection java.lang.Object runTransaction(com.google.cloud.spanner.connection.Connection$TransactionCallable) - + 7012 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 9fa19e0f6d0..7eda6677764 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 @@ -61,6 +61,7 @@ public class ITBuiltInMetricsTest { @BeforeClass public static void setUp() throws IOException { + assumeFalse("This test requires credentials", EmulatorSpannerHelper.isUsingEmulator()); metricClient = MetricServiceClient.create(); // Enable BuiltinMetrics when the metrics are GA'ed db = env.getTestHelper().createTestDatabase(); @@ -76,7 +77,6 @@ public void tearDown() { @Test public void testBuiltinMetricsWithDefaultOTEL() throws Exception { - assumeFalse("This test requires credentials", EmulatorSpannerHelper.isUsingEmulator()); // This stopwatch is used for to limit fetching of metric data in verifyMetrics Stopwatch metricsPollingStopwatch = Stopwatch.createStarted(); Instant start = Instant.now().minus(Duration.ofMinutes(2)); From 7b6c0a717150e5a9e26a44dc0554b6caf0761a44 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Wed, 26 Mar 2025 17:11:21 +0530 Subject: [PATCH 05/13] add grpc metrics in allowed metrics --- .../cloud/spanner/BuiltInMetricsConstant.java | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 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 09844a3a1ba..d2d263f2dd7 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 @@ -30,6 +30,7 @@ import java.util.Map; import java.util.Set; import java.util.stream.Collectors; +import java.util.stream.Stream; @InternalApi public class BuiltInMetricsConstant { @@ -46,14 +47,22 @@ public class BuiltInMetricsConstant { static final String OPERATION_COUNT_NAME = "operation_count"; static final String ATTEMPT_COUNT_NAME = "attempt_count"; + static final Collection GRPC_METRICS_TO_ENABLE = + ImmutableList.of( + "grpc.lb.rls.default_target_picks", + "grpc.lb.rls.target_picks", + "grpc.xds_client.server_failure", + "grpc.xds_client.resource_updates_invalid"); + public static final Set SPANNER_METRICS = - ImmutableSet.of( - OPERATION_LATENCIES_NAME, - ATTEMPT_LATENCIES_NAME, - OPERATION_COUNT_NAME, - ATTEMPT_COUNT_NAME, - GFE_LATENCIES_NAME) - .stream() + Stream.concat( + Stream.of( + OPERATION_LATENCIES_NAME, + ATTEMPT_LATENCIES_NAME, + OPERATION_COUNT_NAME, + ATTEMPT_COUNT_NAME, + GFE_LATENCIES_NAME), + GRPC_METRICS_TO_ENABLE.stream()) .map(m -> METER_NAME + '/' + m) .collect(Collectors.toSet()); @@ -111,13 +120,6 @@ public class BuiltInMetricsConstant { 10000.0, 20000.0, 50000.0, 100000.0, 200000.0, 400000.0, 800000.0, 1600000.0, 3200000.0)); - static final Collection GRPC_METRICS_TO_ENABLE = - ImmutableList.of( - "grpc.lb.rls.default_target_picks", - "grpc.lb.rls.target_picks", - "grpc.xds_client.server_failure", - "grpc.xds_client.resource_updates_invalid"); - static final Collection GRPC_METRICS_ENABLED_BY_DEFAULT = ImmutableList.of( "grpc.client.attempt.sent_total_compressed_message_size", From 01adb671e6642f7dc3bcaf5ca4729854b5ac2526 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Wed, 2 Apr 2025 22:17:18 +0530 Subject: [PATCH 06/13] test: logs --- .../com/google/cloud/spanner/BuiltInMetricsProvider.java | 2 ++ .../google/cloud/spanner/SpannerCloudMonitoringExporter.java | 2 +- .../cloud/spanner/SpannerCloudMonitoringExporterUtils.java | 5 +++-- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsProvider.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsProvider.java index de151e34440..5a977bb60e3 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsProvider.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsProvider.java @@ -21,6 +21,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.INSTANCE_CONFIG_ID_KEY; +import static com.google.cloud.spanner.BuiltInMetricsConstant.INSTANCE_ID_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.LOCATION_ID_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.PROJECT_ID_KEY; @@ -119,6 +120,7 @@ Attributes createResourceAttributes(String projectId) { .put(PROJECT_ID_KEY.getKey(), projectId) .put(INSTANCE_CONFIG_ID_KEY.getKey(), "unknown") .put(CLIENT_HASH_KEY.getKey(), generateClientHash(getDefaultTaskValue())) + .put(INSTANCE_ID_KEY.getKey(), "unknown") .put(LOCATION_ID_KEY.getKey(), detectClientLocation()); return attributesBuilder.build(); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java index 147d838f224..705bb2b6bcb 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java @@ -137,7 +137,7 @@ private CompletableResultCode exportSpannerClientMetrics(Collection // Filter spanner metrics. Only include metrics that contain a project and instance ID. List spannerMetricData = collection.stream() - .filter(md -> SPANNER_METRICS.contains(md.getName())) + // .filter(md -> SPANNER_METRICS.contains(md.getName())) .collect(Collectors.toList()); // Log warnings for metrics that will be skipped. diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java index e73e4060f03..9ce4a3fd9d9 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java @@ -83,6 +83,7 @@ static List convertToSpannerTimeSeries(List collection) || metricData.getInstrumentationScopeInfo().getName().equals(SPANNER_METER_NAME) || metricData.getInstrumentationScopeInfo().getName().equals(GRPC_METER_NAME))) { // Filter out metric data for instruments that are not part of the spanner metrics list + System.out.println("Skipped some data" + metricData.getInstrumentationScopeInfo().getName().toString()); continue; } @@ -102,7 +103,6 @@ static List convertToSpannerTimeSeries(List collection) convertPointToSpannerTimeSeries(metricData, pointData, monitoredResourceBuilder)) .forEach(allTimeSeries::add); } - return allTimeSeries; } @@ -114,6 +114,7 @@ private static TimeSeries convertPointToSpannerTimeSeries( TimeSeries.newBuilder() .setMetricKind(convertMetricKind(metricData)) .setValueType(convertValueType(metricData.getType())); + System.out.println("convertPointToSpannerTimeSeries Metric name " +metricData.getName()); Metric.Builder metricBuilder = Metric.newBuilder().setType(metricData.getName()); Attributes attributes = pointData.getAttributes(); @@ -123,7 +124,7 @@ private static TimeSeries convertPointToSpannerTimeSeries( monitoredResourceBuilder.putLabels(key.getKey(), String.valueOf(attributes.get(key))); } else { metricBuilder.putLabels( - key.getKey().replace(".", "/"), String.valueOf(attributes.get(key))); + key.getKey().replace(".", "_"), String.valueOf(attributes.get(key))); } } From 70ff8bf0d6d0cc3f455e7213976c83d54a22d6b7 Mon Sep 17 00:00:00 2001 From: cloud-java-bot Date: Wed, 2 Apr 2025 17:17:32 +0000 Subject: [PATCH 07/13] chore: generate libraries at Wed Apr 2 17:15:18 UTC 2025 --- .../google/cloud/spanner/SpannerCloudMonitoringExporter.java | 1 - .../cloud/spanner/SpannerCloudMonitoringExporterUtils.java | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java index 705bb2b6bcb..8d6f037bba1 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java @@ -16,7 +16,6 @@ package com.google.cloud.spanner; -import static com.google.cloud.spanner.BuiltInMetricsConstant.SPANNER_METRICS; import com.google.api.core.ApiFuture; import com.google.api.core.ApiFutureCallback; diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java index 9ce4a3fd9d9..39d69400b15 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java @@ -83,7 +83,8 @@ static List convertToSpannerTimeSeries(List collection) || metricData.getInstrumentationScopeInfo().getName().equals(SPANNER_METER_NAME) || metricData.getInstrumentationScopeInfo().getName().equals(GRPC_METER_NAME))) { // Filter out metric data for instruments that are not part of the spanner metrics list - System.out.println("Skipped some data" + metricData.getInstrumentationScopeInfo().getName().toString()); + System.out.println( + "Skipped some data" + metricData.getInstrumentationScopeInfo().getName().toString()); continue; } @@ -114,7 +115,7 @@ private static TimeSeries convertPointToSpannerTimeSeries( TimeSeries.newBuilder() .setMetricKind(convertMetricKind(metricData)) .setValueType(convertValueType(metricData.getType())); - System.out.println("convertPointToSpannerTimeSeries Metric name " +metricData.getName()); + System.out.println("convertPointToSpannerTimeSeries Metric name " + metricData.getName()); Metric.Builder metricBuilder = Metric.newBuilder().setType(metricData.getName()); Attributes attributes = pointData.getAttributes(); From 80d3017fde14aaf3c962dce32f4f670138becb1a Mon Sep 17 00:00:00 2001 From: cloud-java-bot Date: Wed, 2 Apr 2025 17:20:09 +0000 Subject: [PATCH 08/13] chore: generate libraries at Wed Apr 2 17:18:00 UTC 2025 --- .../com/google/cloud/spanner/SpannerCloudMonitoringExporter.java | 1 - 1 file changed, 1 deletion(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java index 8d6f037bba1..eb026c72d02 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java @@ -16,7 +16,6 @@ package com.google.cloud.spanner; - import com.google.api.core.ApiFuture; import com.google.api.core.ApiFutureCallback; import com.google.api.core.ApiFutures; From b2f42932f06df6381d543703d9c0fa2e38622819 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Thu, 3 Apr 2025 12:41:35 +0530 Subject: [PATCH 09/13] remove logs --- .../cloud/spanner/BuiltInMetricsConstant.java | 13 ---------- .../SpannerCloudMonitoringExporter.java | 24 +------------------ .../SpannerCloudMonitoringExporterUtils.java | 8 +++---- 3 files changed, 5 insertions(+), 40 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 d2d263f2dd7..abf635f9dc5 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 @@ -30,7 +30,6 @@ import java.util.Map; import java.util.Set; import java.util.stream.Collectors; -import java.util.stream.Stream; @InternalApi public class BuiltInMetricsConstant { @@ -54,18 +53,6 @@ public class BuiltInMetricsConstant { "grpc.xds_client.server_failure", "grpc.xds_client.resource_updates_invalid"); - public static final Set SPANNER_METRICS = - Stream.concat( - Stream.of( - OPERATION_LATENCIES_NAME, - ATTEMPT_LATENCIES_NAME, - OPERATION_COUNT_NAME, - ATTEMPT_COUNT_NAME, - GFE_LATENCIES_NAME), - GRPC_METRICS_TO_ENABLE.stream()) - .map(m -> METER_NAME + '/' + m) - .collect(Collectors.toSet()); - public static final String SPANNER_RESOURCE_TYPE = "spanner_instance_client"; public static final AttributeKey PROJECT_ID_KEY = AttributeKey.stringKey("project_id"); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java index eb026c72d02..de9377fdf74 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java @@ -103,25 +103,6 @@ static SpannerCloudMonitoringExporter create( @Override public CompletableResultCode export(@Nonnull Collection collection) { - // TODO: Remove - collection.stream() - .filter(md -> "grpc-java".equals(md.getInstrumentationScopeInfo().getName())) - .forEach( - md -> { - System.out.println("Name: " + md.getName()); // Print the name - - md.getData() - .getPoints() - .forEach( - point -> { - System.out.println( - "Attributes: " - + point.getAttributes()); // Print attributes for each point - }); - - System.out.println("----------------------"); // Separator for readability - }); - if (client.isShutdown()) { logger.log(Level.WARNING, "Exporter is shut down"); return CompletableResultCode.ofFailure(); @@ -133,10 +114,7 @@ public CompletableResultCode export(@Nonnull Collection collection) /** Export client built in metrics */ private CompletableResultCode exportSpannerClientMetrics(Collection collection) { // Filter spanner metrics. Only include metrics that contain a project and instance ID. - List spannerMetricData = - collection.stream() - // .filter(md -> SPANNER_METRICS.contains(md.getName())) - .collect(Collectors.toList()); + List spannerMetricData = collection.stream().collect(Collectors.toList()); // Log warnings for metrics that will be skipped. boolean mustFilter = false; diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java index 39d69400b15..967e3e62f7a 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java @@ -78,13 +78,11 @@ static List convertToSpannerTimeSeries(List collection) List allTimeSeries = new ArrayList<>(); for (MetricData metricData : collection) { - // Get metrics data from GAX library and Spanner library + // Get metrics data from GAX library, GRPC library and Spanner library if (!(metricData.getInstrumentationScopeInfo().getName().equals(GAX_METER_NAME) || metricData.getInstrumentationScopeInfo().getName().equals(SPANNER_METER_NAME) || metricData.getInstrumentationScopeInfo().getName().equals(GRPC_METER_NAME))) { // Filter out metric data for instruments that are not part of the spanner metrics list - System.out.println( - "Skipped some data" + metricData.getInstrumentationScopeInfo().getName().toString()); continue; } @@ -115,7 +113,6 @@ private static TimeSeries convertPointToSpannerTimeSeries( TimeSeries.newBuilder() .setMetricKind(convertMetricKind(metricData)) .setValueType(convertValueType(metricData.getType())); - System.out.println("convertPointToSpannerTimeSeries Metric name " + metricData.getName()); Metric.Builder metricBuilder = Metric.newBuilder().setType(metricData.getName()); Attributes attributes = pointData.getAttributes(); @@ -124,11 +121,14 @@ private static TimeSeries convertPointToSpannerTimeSeries( if (SPANNER_PROMOTED_RESOURCE_LABELS.contains(key)) { monitoredResourceBuilder.putLabels(key.getKey(), String.valueOf(attributes.get(key))); } else { + // Replace metric label names by converting "." to "_" since Cloud Monitoring does not + // support labels containing "." metricBuilder.putLabels( key.getKey().replace(".", "_"), String.valueOf(attributes.get(key))); } } + // Add common labels like "client_name" and "client_uid" for all the exported metrics. metricBuilder.putAllLabels(BuiltInMetricsProvider.INSTANCE.createClientAttributes()); builder.setResource(monitoredResourceBuilder.build()); From 5b29243cdc41583822f03ff54121508bcbb2df9d Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Thu, 3 Apr 2025 12:57:44 +0530 Subject: [PATCH 10/13] review --- .../cloud/spanner/BuiltInMetricsConstant.java | 3 +- .../cloud/spanner/BuiltInMetricsProvider.java | 3 +- .../SpannerCloudMonitoringExporter.java | 18 ++----- .../SpannerCloudMonitoringExporterUtils.java | 5 -- .../SpannerCloudMonitoringExporterTest.java | 53 ------------------- 5 files changed, 6 insertions(+), 76 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 abf635f9dc5..de1cb83c35f 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 @@ -51,7 +51,8 @@ public class BuiltInMetricsConstant { "grpc.lb.rls.default_target_picks", "grpc.lb.rls.target_picks", "grpc.xds_client.server_failure", - "grpc.xds_client.resource_updates_invalid"); + "grpc.xds_client.resource_updates_invalid", + "grpc.xds_client.resource_updates_valid"); public static final String SPANNER_RESOURCE_TYPE = "spanner_instance_client"; diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsProvider.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsProvider.java index 5a977bb60e3..2b4bde55e88 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsProvider.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsProvider.java @@ -75,7 +75,6 @@ OpenTelemetry getOrCreateOpenTelemetry( BuiltInMetricsView.registerBuiltinMetrics( SpannerCloudMonitoringExporter.create(projectId, credentials, monitoringHost), sdkMeterProviderBuilder); - sdkMeterProviderBuilder.setResource(Resource.create(createResourceAttributes(projectId))); SdkMeterProvider sdkMeterProvider = sdkMeterProviderBuilder.build(); this.openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProvider).build(); @@ -91,7 +90,7 @@ OpenTelemetry getOrCreateOpenTelemetry( } } - public void enableGrpcMetrics( + void enableGrpcMetrics( InstantiatingGrpcChannelProvider.Builder channelProviderBuilder, String projectId, @Nullable Credentials credentials, diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java index de9377fdf74..35503fff337 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java @@ -37,7 +37,6 @@ import io.opentelemetry.sdk.metrics.InstrumentType; import io.opentelemetry.sdk.metrics.data.AggregationTemporality; import io.opentelemetry.sdk.metrics.data.MetricData; -import io.opentelemetry.sdk.metrics.data.PointData; import io.opentelemetry.sdk.metrics.export.MetricExporter; import io.opentelemetry.sdk.resources.Resource; import java.io.IOException; @@ -113,7 +112,7 @@ public CompletableResultCode export(@Nonnull Collection collection) /** Export client built in metrics */ private CompletableResultCode exportSpannerClientMetrics(Collection collection) { - // Filter spanner metrics. Only include metrics that contain a project and instance ID. + // Filter spanner metrics. Only include metrics that contain a valid project. List spannerMetricData = collection.stream().collect(Collectors.toList()); // Log warnings for metrics that will be skipped. @@ -125,12 +124,7 @@ private CompletableResultCode exportSpannerClientMetrics(Collection Level.WARNING, "Some metric data contain a different projectId. These will be skipped."); mustFilter = true; } - if (spannerMetricData.stream() - .flatMap(metricData -> metricData.getData().getPoints().stream()) - .anyMatch(this::shouldSkipPointDataDueToMissingInstanceId)) { - logger.log(Level.WARNING, "Some metric data miss instanceId. These will be skipped."); - mustFilter = true; - } + if (mustFilter) { spannerMetricData = spannerMetricData.stream() @@ -194,19 +188,13 @@ public void onSuccess(List empty) { } private boolean shouldSkipMetricData(MetricData metricData) { - return shouldSkipPointDataDueToProjectId(metricData.getResource()) - || metricData.getData().getPoints().stream() - .anyMatch(pd -> shouldSkipPointDataDueToMissingInstanceId(pd)); + return shouldSkipPointDataDueToProjectId(metricData.getResource()); } private boolean shouldSkipPointDataDueToProjectId(Resource resource) { return !spannerProjectId.equals(SpannerCloudMonitoringExporterUtils.getProjectId(resource)); } - private boolean shouldSkipPointDataDueToMissingInstanceId(PointData pointData) { - return SpannerCloudMonitoringExporterUtils.getInstanceId(pointData) == null; - } - boolean lastExportSkippedData() { return this.lastExportSkippedData.get(); } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java index 967e3e62f7a..f67621db963 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java @@ -24,7 +24,6 @@ import static com.google.api.MetricDescriptor.ValueType.INT64; import static com.google.cloud.spanner.BuiltInMetricsConstant.GAX_METER_NAME; import static com.google.cloud.spanner.BuiltInMetricsConstant.GRPC_METER_NAME; -import static com.google.cloud.spanner.BuiltInMetricsConstant.INSTANCE_ID_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.PROJECT_ID_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.SPANNER_METER_NAME; import static com.google.cloud.spanner.BuiltInMetricsConstant.SPANNER_PROMOTED_RESOURCE_LABELS; @@ -70,10 +69,6 @@ static String getProjectId(Resource resource) { return resource.getAttributes().get(PROJECT_ID_KEY); } - static String getInstanceId(PointData pointData) { - return pointData.getAttributes().get(INSTANCE_ID_KEY); - } - static List convertToSpannerTimeSeries(List collection) { List allTimeSeries = new ArrayList<>(); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java index c7796cb0f4b..84a8cf4460c 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java @@ -16,7 +16,6 @@ package com.google.cloud.spanner; -import static com.google.cloud.spanner.BuiltInMetricsConstant.ATTEMPT_COUNT_NAME; import static com.google.cloud.spanner.BuiltInMetricsConstant.CLIENT_HASH_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.CLIENT_NAME_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.CLIENT_UID_KEY; @@ -32,7 +31,6 @@ import static com.google.cloud.spanner.BuiltInMetricsConstant.PROJECT_ID_KEY; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -47,7 +45,6 @@ import com.google.monitoring.v3.TimeSeries; import com.google.protobuf.Empty; import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.sdk.common.CompletableResultCode; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; import io.opentelemetry.sdk.metrics.InstrumentType; import io.opentelemetry.sdk.metrics.data.AggregationTemporality; @@ -358,56 +355,6 @@ public void getAggregationTemporality() throws IOException { .isEqualTo(AggregationTemporality.CUMULATIVE); } - @Test - public void testSkipExportingDataIfMissingInstanceId() throws IOException { - Attributes attributesWithoutInstanceId = - Attributes.builder().putAll(attributes).remove(INSTANCE_ID_KEY).build(); - - SpannerCloudMonitoringExporter actualExporter = - SpannerCloudMonitoringExporter.create(projectId, null, null); - assertThat(actualExporter.getAggregationTemporality(InstrumentType.COUNTER)) - .isEqualTo(AggregationTemporality.CUMULATIVE); - ArgumentCaptor argumentCaptor = - ArgumentCaptor.forClass(CreateTimeSeriesRequest.class); - - UnaryCallable mockCallable = Mockito.mock(UnaryCallable.class); - Mockito.when(mockMetricServiceStub.createServiceTimeSeriesCallable()).thenReturn(mockCallable); - ApiFuture future = ApiFutures.immediateFuture(Empty.getDefaultInstance()); - Mockito.when(mockCallable.futureCall(argumentCaptor.capture())).thenReturn(future); - - long fakeValue = 11L; - - long startEpoch = 10; - long endEpoch = 15; - LongPointData longPointData = - ImmutableLongPointData.create(startEpoch, endEpoch, attributesWithoutInstanceId, fakeValue); - - MetricData operationLongData = - ImmutableMetricData.createLongSum( - resource, - scope, - "spanner.googleapis.com/internal/client/" + OPERATION_COUNT_NAME, - "description", - "1", - ImmutableSumData.create( - true, AggregationTemporality.CUMULATIVE, ImmutableList.of(longPointData))); - - MetricData attemptLongData = - ImmutableMetricData.createLongSum( - resource, - scope, - "spanner.googleapis.com/internal/client/" + ATTEMPT_COUNT_NAME, - "description", - "1", - ImmutableSumData.create( - true, AggregationTemporality.CUMULATIVE, ImmutableList.of(longPointData))); - - CompletableResultCode resultCode = - exporter.export(Arrays.asList(operationLongData, attemptLongData)); - assertTrue(resultCode.isSuccess()); - assertTrue(exporter.lastExportSkippedData()); - } - private static class FakeMetricServiceClient extends MetricServiceClient { protected FakeMetricServiceClient(MetricServiceStub stub) { From 64e02f7dac8314dc6fc0396db4ab16a91bdcb290 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Thu, 3 Apr 2025 14:24:57 +0530 Subject: [PATCH 11/13] clirr --- .../google/cloud/spanner/BuiltInMetricsConstant.java | 11 +++++++++++ 1 file changed, 11 insertions(+) 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 de1cb83c35f..3911e85a724 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 @@ -46,6 +46,17 @@ 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 = + ImmutableSet.of( + OPERATION_LATENCIES_NAME, + ATTEMPT_LATENCIES_NAME, + OPERATION_COUNT_NAME, + ATTEMPT_COUNT_NAME, + GFE_LATENCIES_NAME) + .stream() + .map(m -> METER_NAME + '/' + m) + .collect(Collectors.toSet()); + static final Collection GRPC_METRICS_TO_ENABLE = ImmutableList.of( "grpc.lb.rls.default_target_picks", From 180143332044f7feddc7d07682e14496af378d36 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Wed, 16 Apr 2025 10:52:24 +0530 Subject: [PATCH 12/13] review comments --- .../java/com/google/cloud/spanner/BuiltInMetricsConstant.java | 4 ++-- .../java/com/google/cloud/spanner/BuiltInMetricsProvider.java | 1 + 2 files changed, 3 insertions(+), 2 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 3911e85a724..050484ae66e 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 @@ -107,7 +107,7 @@ public class BuiltInMetricsConstant { DIRECT_PATH_ENABLED_KEY, DIRECT_PATH_USED_KEY); - public static final Set GRPC_ATTRIBUTES = + static final Set GRPC_LB_RLS_ATTRIBUTES = ImmutableSet.of("grpc.lb.rls.data_plane_target", "grpc.lb.pick_result"); static Aggregation AGGREGATION_WITH_MILLIS_HISTOGRAM = @@ -212,7 +212,7 @@ private static void defineGRPCView(ImmutableMap.Builder channelConfigurator = From 94e81616ccd1b5adbd0b03f3b0ed69f296cc4fd3 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Wed, 16 Apr 2025 22:39:21 +0530 Subject: [PATCH 13/13] enable grpc metrics only env is set to false --- .../main/java/com/google/cloud/spanner/SpannerOptions.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 d8707dece5e..8a9c5050d13 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 @@ -919,8 +919,8 @@ public boolean isEnableBuiltInMetrics() { @Override public boolean isEnableGRPCBuiltInMetrics() { - return !Boolean.parseBoolean( - System.getenv(SPANNER_DISABLE_DIRECT_ACCESS_GRPC_BUILTIN_METRICS)); + return "false" + .equalsIgnoreCase(System.getenv(SPANNER_DISABLE_DIRECT_ACCESS_GRPC_BUILTIN_METRICS)); } @Override