From 3ef313c7a023cd80fa0f150bcf59c7d1025e1de7 Mon Sep 17 00:00:00 2001 From: Rahul Yadav Date: Mon, 25 May 2026 14:49:02 +0530 Subject: [PATCH 1/5] fix(spanner): derive built-in metrics project from database client --- .../cloud/spanner/BuiltInMetricsProvider.java | 40 ++++++++++++- .../SpannerCloudMonitoringExporter.java | 58 ++++++++----------- .../SpannerCloudMonitoringExporterUtils.java | 6 +- .../com/google/cloud/spanner/SpannerImpl.java | 2 + .../google/cloud/spanner/SpannerOptions.java | 11 ++++ ...iltInOpenTelemetryMetricsProviderTest.java | 51 ++++++++++++++++ .../SpannerCloudMonitoringExporterTest.java | 41 +++++++++++++ .../google/cloud/spanner/SpannerImplTest.java | 2 + 8 files changed, 170 insertions(+), 41 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsProvider.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsProvider.java index 0a51ebfae26f..f6c78b0c83d2 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsProvider.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsProvider.java @@ -32,6 +32,7 @@ import com.google.cloud.opentelemetry.detection.AttributeKeys; import com.google.cloud.opentelemetry.detection.DetectedPlatform; import com.google.cloud.opentelemetry.detection.GCPPlatformDetector; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; import com.google.common.hash.HashFunction; import com.google.common.hash.Hashing; @@ -75,10 +76,12 @@ final class BuiltInMetricsProvider { private static final String default_location = "global"; private OpenTelemetry openTelemetry; + private String projectId; + private boolean mismatchedProjectIdLogged; private BuiltInMetricsProvider() {} - OpenTelemetry getOrCreateOpenTelemetry( + synchronized OpenTelemetry getOrCreateOpenTelemetry( String projectId, @Nullable Credentials credentials, @Nullable String monitoringHost, @@ -88,7 +91,7 @@ OpenTelemetry getOrCreateOpenTelemetry( SdkMeterProviderBuilder sdkMeterProviderBuilder = SdkMeterProvider.builder(); BuiltInMetricsView.registerBuiltinMetrics( SpannerCloudMonitoringExporter.create( - projectId, credentials, monitoringHost, universeDomain), + this::getProjectId, credentials, monitoringHost, universeDomain), sdkMeterProviderBuilder); sdkMeterProviderBuilder.setResource(Resource.create(createResourceAttributes(projectId))); SdkMeterProvider sdkMeterProvider = sdkMeterProviderBuilder.build(); @@ -106,6 +109,39 @@ OpenTelemetry getOrCreateOpenTelemetry( } } + synchronized void setProjectIdIfAbsent(String projectId) { + if (this.projectId == null) { + this.projectId = projectId; + } else if (!this.projectId.equals(projectId) && !mismatchedProjectIdLogged) { + mismatchedProjectIdLogged = true; + logger.log( + Level.WARNING, + "Built-in metrics are already initialized for project {0}. Metrics for project {1} will" + + " be exported using the existing project.", + new Object[] {this.projectId, projectId}); + } + } + + @Nullable + synchronized OpenTelemetry getOpenTelemetry() { + return this.openTelemetry; + } + + @VisibleForTesting + synchronized String getProjectId() { + return this.projectId; + } + + @VisibleForTesting + synchronized void reset() { + if (this.openTelemetry instanceof OpenTelemetrySdk) { + ((OpenTelemetrySdk) this.openTelemetry).getSdkMeterProvider().close(); + } + this.openTelemetry = null; + this.projectId = null; + this.mismatchedProjectIdLogged = false; + } + // TODO: Remove when // https://github.com/GoogleCloudPlatform/opentelemetry-operations-java/issues/421 // has been fixed. diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java index bedf6600075a..e611882aeda1 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java @@ -42,16 +42,15 @@ import io.opentelemetry.sdk.metrics.data.AggregationTemporality; import io.opentelemetry.sdk.metrics.data.MetricData; 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; import java.util.Collection; import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Supplier; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.stream.Collectors; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -72,7 +71,7 @@ class SpannerCloudMonitoringExporter implements MetricExporter { private final AtomicBoolean spannerExportFailureLogged = new AtomicBoolean(false); private final AtomicBoolean lastExportSkippedData = new AtomicBoolean(false); private final MetricServiceClient client; - private final String spannerProjectId; + private final Supplier spannerProjectIdSupplier; static SpannerCloudMonitoringExporter create( String projectId, @@ -80,6 +79,15 @@ static SpannerCloudMonitoringExporter create( @Nullable String monitoringHost, String universeDomain) throws IOException { + return create(() -> projectId, credentials, monitoringHost, universeDomain); + } + + static SpannerCloudMonitoringExporter create( + Supplier projectIdSupplier, + @Nullable Credentials credentials, + @Nullable String monitoringHost, + String universeDomain) + throws IOException { MetricServiceSettings.Builder settingsBuilder = MetricServiceSettings.newBuilder(); CredentialsProvider credentialsProvider; if (credentials == null || credentials instanceof NoCredentials) { @@ -114,13 +122,18 @@ static SpannerCloudMonitoringExporter create( settingsBuilder.createServiceTimeSeriesSettings().setSimpleTimeoutNoRetriesDuration(timeout); return new SpannerCloudMonitoringExporter( - projectId, MetricServiceClient.create(settingsBuilder.build())); + projectIdSupplier, MetricServiceClient.create(settingsBuilder.build())); } @VisibleForTesting SpannerCloudMonitoringExporter(String projectId, MetricServiceClient client) { + this(() -> projectId, client); + } + + @VisibleForTesting + SpannerCloudMonitoringExporter(Supplier projectIdSupplier, MetricServiceClient client) { this.client = client; - this.spannerProjectId = projectId; + this.spannerProjectIdSupplier = projectIdSupplier; } @Override @@ -140,29 +153,14 @@ MetricServiceClient getMetricServiceClient() { /** Export client built in metrics */ private CompletableResultCode exportSpannerClientMetrics(Collection collection) { - // 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. - boolean mustFilter = false; - if (spannerMetricData.stream() - .map(metricData -> metricData.getResource()) - .anyMatch(this::shouldSkipPointDataDueToProjectId)) { - logger.log( - Level.WARNING, "Some metric data contain a different projectId. These will be skipped."); - mustFilter = true; - } - - if (mustFilter) { - spannerMetricData = - spannerMetricData.stream() - .filter(this::shouldSkipMetricData) - .collect(Collectors.toList()); + String spannerProjectId = spannerProjectIdSupplier.get(); + if (Strings.isNullOrEmpty(spannerProjectId)) { + return CompletableResultCode.ofSuccess(); } - lastExportSkippedData.set(mustFilter); + lastExportSkippedData.set(false); // Skips exporting if there's none - if (spannerMetricData.isEmpty()) { + if (collection.isEmpty()) { return CompletableResultCode.ofSuccess(); } @@ -170,7 +168,7 @@ private CompletableResultCode exportSpannerClientMetrics(Collection try { spannerTimeSeries = SpannerCloudMonitoringExporterUtils.convertToSpannerTimeSeries( - spannerMetricData, this.spannerProjectId); + new ArrayList<>(collection), spannerProjectId); } catch (Throwable e) { logger.log( Level.WARNING, @@ -218,14 +216,6 @@ public void onSuccess(List empty) { return spannerExportCode; } - private boolean shouldSkipMetricData(MetricData metricData) { - return shouldSkipPointDataDueToProjectId(metricData.getResource()); - } - - private boolean shouldSkipPointDataDueToProjectId(Resource resource) { - return !spannerProjectId.equals(SpannerCloudMonitoringExporterUtils.getProjectId(resource)); - } - boolean lastExportSkippedData() { return this.lastExportSkippedData.get(); } diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java index 0f6d8006866e..57f8827d393d 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java @@ -61,7 +61,6 @@ 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; @@ -75,10 +74,6 @@ class SpannerCloudMonitoringExporterUtils { private SpannerCloudMonitoringExporterUtils() {} - static String getProjectId(Resource resource) { - return resource.getAttributes().get(PROJECT_ID_KEY); - } - static List convertToSpannerTimeSeries( List collection, String projectId) { List allTimeSeries = new ArrayList<>(); @@ -102,6 +97,7 @@ static List convertToSpannerTimeSeries( monitoredResourceBuilder.putLabels( key.getKey(), String.valueOf(resourceAttributes.get(key))); } + monitoredResourceBuilder.putLabels(PROJECT_ID_KEY.getKey(), projectId); metricData.getData().getPoints().stream() .map( diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java index c201924dfbe7..206f58726257 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java @@ -309,6 +309,7 @@ public DatabaseClient getDatabaseClient(DatabaseId db) { if (clientId == null) { clientId = nextDatabaseClientId(db); } + getOptions().initializeBuiltInMetrics(db); MultiplexedSessionDatabaseClient multiplexedSessionDatabaseClient = new MultiplexedSessionDatabaseClient(SpannerImpl.this.getSessionClient(db)); DatabaseClientImpl dbClient = @@ -337,6 +338,7 @@ public BatchClient getBatchClient(DatabaseId db) { if (this.dbBatchClients.containsKey(db)) { return this.dbBatchClients.get(db); } + getOptions().initializeBuiltInMetrics(db); BatchClientImpl batchClient = new BatchClientImpl(getSessionClient(db)); this.dbBatchClients.put(db, batchClient); return batchClient; diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index 3b4190a237c7..a2fcfd9a81b1 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -2520,6 +2520,17 @@ public void enablegRPCMetrics(InstantiatingGrpcChannelProvider.Builder channelPr } } + void initializeBuiltInMetrics(DatabaseId databaseId) { + if (isEnableBuiltInMetrics() && !usesNoCredentials()) { + this.builtInMetricsProvider.setProjectIdIfAbsent(databaseId.getInstanceId().getProject()); + this.builtInMetricsProvider.getOrCreateOpenTelemetry( + databaseId.getInstanceId().getProject(), + getCredentials(), + this.monitoringHost, + getUniverseDomain()); + } + } + public ApiTracerFactory getApiTracerFactory(boolean isAdminClient, boolean isEmulatorEnabled) { return createApiTracerFactory(isAdminClient, isEmulatorEnabled); } diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProviderTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProviderTest.java index 73185177de19..5c7e8977227d 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProviderTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProviderTest.java @@ -17,8 +17,14 @@ package com.google.cloud.spanner; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import com.google.auth.oauth2.AccessToken; +import com.google.auth.oauth2.OAuth2Credentials; +import java.util.Date; +import org.junit.After; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -26,6 +32,16 @@ @RunWith(JUnit4.class) public class BuiltInOpenTelemetryMetricsProviderTest { + @Before + public void setUp() { + BuiltInMetricsProvider.INSTANCE.reset(); + } + + @After + public void tearDown() { + BuiltInMetricsProvider.INSTANCE.reset(); + } + @Test public void testGenerateClientHashWithSimpleUid() { String clientUid = "testClient"; @@ -56,6 +72,33 @@ public void testGenerateClientHashWithSpecialCharacters() { verifyHash(BuiltInMetricsProvider.generateClientHash(clientUid)); } + @Test + public void testApiTracerFactoryDoesNotSetBuiltInMetricsProject() { + SpannerOptions options = newTestOptions(); + + options.getApiTracerFactory(/* isAdminClient= */ false, /* isEmulatorEnabled= */ false); + + assertNull(BuiltInMetricsProvider.INSTANCE.getProjectId()); + } + + @Test + public void testBuiltInOpenTelemetryDoesNotSetMetricsProject() { + SpannerOptions options = newTestOptions(); + + options.getBuiltInOpenTelemetry(); + + assertNull(BuiltInMetricsProvider.INSTANCE.getProjectId()); + } + + @Test + public void testInitializeBuiltInMetricsUsesDatabaseProject() { + SpannerOptions options = newTestOptions(); + + options.initializeBuiltInMetrics(DatabaseId.of("database-project", "i", "d")); + + assertEquals("database-project", BuiltInMetricsProvider.INSTANCE.getProjectId()); + } + private void verifyHash(String hash) { // Check if the hash length is 6 assertEquals(hash.length(), 6); @@ -63,4 +106,12 @@ private void verifyHash(String hash) { long hashValue = Long.parseLong(hash, 16); // Convert hash from hex to decimal assertTrue(hashValue >= 0 && hashValue <= 0x3FF); } + + private SpannerOptions newTestOptions() { + return SpannerOptions.newBuilder() + .setProjectId("host-project") + .setCredentials( + OAuth2Credentials.create(new AccessToken("test-token", new Date(Long.MAX_VALUE)))) + .build(); + } } diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java index 590d62db7b5e..b336263f55a0 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java @@ -202,6 +202,47 @@ public void testExportingSumData() { assertThat(timeSeries.getPoints(0).getInterval().getEndTime().getNanos()).isEqualTo(endEpoch); } + @Test + public void testExportingOverridesResourceProject() { + 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); + + Resource resourceWithDifferentProject = + Resource.create( + resourceAttributes.toBuilder().put(PROJECT_ID_KEY, "resource-project").build()); + LongPointData longPointData = ImmutableLongPointData.create(10, 15, attributes, 11L); + MetricData longData = + ImmutableMetricData.createLongSum( + resourceWithDifferentProject, + scope, + "spanner.googleapis.com/internal/client/" + OPERATION_COUNT_NAME, + "description", + "1", + ImmutableSumData.create( + true, AggregationTemporality.CUMULATIVE, ImmutableList.of(longPointData))); + + exporter.export(Collections.singletonList(longData)); + + CreateTimeSeriesRequest request = argumentCaptor.getValue(); + assertThat(request.getName()).isEqualTo("projects/" + projectId); + assertThat(request.getTimeSeries(0).getResource().getLabelsMap()) + .containsEntry(PROJECT_ID_KEY.getKey(), projectId); + } + + @Test + public void testExportingSkipsUntilProjectIsSet() { + exporter = new SpannerCloudMonitoringExporter(() -> null, fakeMetricServiceClient); + + exporter.export(Collections.emptyList()); + + Mockito.verify(mockMetricServiceStub, Mockito.never()).createServiceTimeSeriesCallable(); + } + @Test public void testExportingHistogramData() { ArgumentCaptor argumentCaptor = diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java index a675605e7685..f1db693e8808 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java @@ -21,6 +21,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThrows; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.google.api.core.NanoClock; @@ -113,6 +114,7 @@ public void getDbclientAgainGivesSame() { DatabaseClient databaseClient1 = impl.getDatabaseClient(db); assertThat(databaseClient1).isSameInstanceAs(databaseClient); + verify(spannerOptions).initializeBuiltInMetrics(db); } @Test From 52234f48afcc4e79cd2c82b6d66c99d5f1820ebf Mon Sep 17 00:00:00 2001 From: Rahul Yadav Date: Mon, 25 May 2026 15:02:16 +0530 Subject: [PATCH 2/5] address review comments --- .../SpannerCloudMonitoringExporter.java | 9 +------- .../SpannerCloudMonitoringExporterUtils.java | 3 ++- .../SpannerCloudMonitoringExporterTest.java | 21 ++++++++++--------- 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java index e611882aeda1..da565fd13089 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java @@ -69,7 +69,6 @@ class SpannerCloudMonitoringExporter implements MetricExporter { // https://cloud.google.com/monitoring/quotas#custom_metrics_quotas. private static final int EXPORT_BATCH_SIZE_LIMIT = 200; private final AtomicBoolean spannerExportFailureLogged = new AtomicBoolean(false); - private final AtomicBoolean lastExportSkippedData = new AtomicBoolean(false); private final MetricServiceClient client; private final Supplier spannerProjectIdSupplier; @@ -157,8 +156,6 @@ private CompletableResultCode exportSpannerClientMetrics(Collection if (Strings.isNullOrEmpty(spannerProjectId)) { return CompletableResultCode.ofSuccess(); } - lastExportSkippedData.set(false); - // Skips exporting if there's none if (collection.isEmpty()) { return CompletableResultCode.ofSuccess(); @@ -168,7 +165,7 @@ private CompletableResultCode exportSpannerClientMetrics(Collection try { spannerTimeSeries = SpannerCloudMonitoringExporterUtils.convertToSpannerTimeSeries( - new ArrayList<>(collection), spannerProjectId); + collection, spannerProjectId); } catch (Throwable e) { logger.log( Level.WARNING, @@ -216,10 +213,6 @@ public void onSuccess(List empty) { return spannerExportCode; } - boolean lastExportSkippedData() { - return this.lastExportSkippedData.get(); - } - private ApiFuture> exportTimeSeriesInBatch( ProjectName projectName, List timeSeries) { List> batchResults = new ArrayList<>(); diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java index 57f8827d393d..92e8c70e5573 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java @@ -62,6 +62,7 @@ import io.opentelemetry.sdk.metrics.data.PointData; import io.opentelemetry.sdk.metrics.data.SumData; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; @@ -75,7 +76,7 @@ class SpannerCloudMonitoringExporterUtils { private SpannerCloudMonitoringExporterUtils() {} static List convertToSpannerTimeSeries( - List collection, String projectId) { + Collection collection, String projectId) { List allTimeSeries = new ArrayList<>(); for (MetricData metricData : collection) { diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java index b336263f55a0..2d2598a3bff6 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java @@ -31,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.assertEquals; -import static org.junit.Assert.assertFalse; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -164,8 +163,6 @@ public void testExportingSumData() { true, AggregationTemporality.CUMULATIVE, ImmutableList.of(longPointData))); exporter.export(Collections.singletonList(longData)); - assertFalse(exporter.lastExportSkippedData()); - CreateTimeSeriesRequest request = argumentCaptor.getValue(); assertThat(request.getTimeSeriesList()).hasSize(1); @@ -237,8 +234,18 @@ public void testExportingOverridesResourceProject() { @Test public void testExportingSkipsUntilProjectIsSet() { exporter = new SpannerCloudMonitoringExporter(() -> null, fakeMetricServiceClient); + LongPointData longPointData = ImmutableLongPointData.create(10, 15, attributes, 11L); + MetricData longData = + ImmutableMetricData.createLongSum( + resource, + scope, + "spanner.googleapis.com/internal/client/" + OPERATION_COUNT_NAME, + "description", + "1", + ImmutableSumData.create( + true, AggregationTemporality.CUMULATIVE, ImmutableList.of(longPointData))); - exporter.export(Collections.emptyList()); + exporter.export(Collections.singletonList(longData)); Mockito.verify(mockMetricServiceStub, Mockito.never()).createServiceTimeSeriesCallable(); } @@ -279,8 +286,6 @@ public void testExportingHistogramData() { AggregationTemporality.CUMULATIVE, ImmutableList.of(histogramPointData))); exporter.export(Collections.singletonList(histogramData)); - assertFalse(exporter.lastExportSkippedData()); - CreateTimeSeriesRequest request = argumentCaptor.getValue(); assertThat(request.getTimeSeriesList()).hasSize(1); @@ -355,8 +360,6 @@ public void testExportingSumDataInBatches() { assertThat(firstRequest.getTimeSeriesList()).hasSize(200); assertThat(secondRequest.getTimeSeriesList()).hasSize(50); - assertFalse(exporter.lastExportSkippedData()); - for (int i = 0; i < 250; i++) { TimeSeries timeSeries; if (i < 200) { @@ -449,8 +452,6 @@ public void testExportingHistogramDataWithExemplars() { AggregationTemporality.CUMULATIVE, ImmutableList.of(histogramPointData))); exporter.export(Collections.singletonList(histogramData)); - assertFalse(exporter.lastExportSkippedData()); - CreateTimeSeriesRequest request = argumentCaptor.getValue(); TimeSeries timeSeries = request.getTimeSeriesList().get(0); Distribution distribution = timeSeries.getPoints(0).getValue().getDistributionValue(); From 592d47d4df1d9a477695f641b583b04d95dae842 Mon Sep 17 00:00:00 2001 From: Rahul Yadav Date: Fri, 29 May 2026 18:57:34 +0530 Subject: [PATCH 3/5] incorporate suggestions --- .../cloud/spanner/BuiltInMetricsConstant.java | 1 + .../cloud/spanner/BuiltInMetricsProvider.java | 18 +- .../SpannerCloudMonitoringExporter.java | 115 +++++++---- .../SpannerCloudMonitoringExporterUtils.java | 43 ++++- .../google/cloud/spanner/SpannerOptions.java | 5 - .../spanner/spi/v1/HeaderInterceptor.java | 7 +- ...iltInOpenTelemetryMetricsProviderTest.java | 1 + .../SpannerCloudMonitoringExporterTest.java | 179 ++++++++++++++++-- 8 files changed, 297 insertions(+), 72 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsConstant.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsConstant.java index dff490832c86..76291a53458c 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsConstant.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsConstant.java @@ -144,6 +144,7 @@ public class BuiltInMetricsConstant { ImmutableList.of(EEF_FALLBACK_COUNT_NAME, EEF_CALL_STATUS_NAME); public static final String SPANNER_RESOURCE_TYPE = "spanner_instance_client"; + public static final String UNDEFINED_PROJECT_ID = "undefined-project"; public static final AttributeKey PROJECT_ID_KEY = AttributeKey.stringKey("project_id"); public static final AttributeKey INSTANCE_ID_KEY = AttributeKey.stringKey("instance_id"); diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsProvider.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsProvider.java index f6c78b0c83d2..5ba34207ea08 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsProvider.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsProvider.java @@ -78,6 +78,7 @@ final class BuiltInMetricsProvider { private OpenTelemetry openTelemetry; private String projectId; private boolean mismatchedProjectIdLogged; + private Thread shutdownHook; private BuiltInMetricsProvider() {} @@ -96,7 +97,8 @@ synchronized OpenTelemetry getOrCreateOpenTelemetry( sdkMeterProviderBuilder.setResource(Resource.create(createResourceAttributes(projectId))); SdkMeterProvider sdkMeterProvider = sdkMeterProviderBuilder.build(); this.openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProvider).build(); - Runtime.getRuntime().addShutdownHook(new Thread(sdkMeterProvider::close)); + this.shutdownHook = new Thread(sdkMeterProvider::close); + Runtime.getRuntime().addShutdownHook(this.shutdownHook); } return this.openTelemetry; } catch (IOException ex) { @@ -116,8 +118,9 @@ synchronized void setProjectIdIfAbsent(String projectId) { mismatchedProjectIdLogged = true; logger.log( Level.WARNING, - "Built-in metrics are already initialized for project {0}. Metrics for project {1} will" - + " be exported using the existing project.", + "Built-in metrics fallback project is already initialized to project {0}. Non-Spanner" + + " metrics without project information will be exported using that project instead" + + " of project {1}.", new Object[] {this.projectId, projectId}); } } @@ -127,7 +130,6 @@ synchronized OpenTelemetry getOpenTelemetry() { return this.openTelemetry; } - @VisibleForTesting synchronized String getProjectId() { return this.projectId; } @@ -137,9 +139,17 @@ synchronized void reset() { if (this.openTelemetry instanceof OpenTelemetrySdk) { ((OpenTelemetrySdk) this.openTelemetry).getSdkMeterProvider().close(); } + if (this.shutdownHook != null) { + try { + Runtime.getRuntime().removeShutdownHook(this.shutdownHook); + } catch (IllegalStateException ignored) { + // The JVM is already shutting down. + } + } this.openTelemetry = null; this.projectId = null; this.mismatchedProjectIdLogged = false; + this.shutdownHook = null; } // TODO: Remove when diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java index da565fd13089..db995387cc48 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java @@ -47,10 +47,13 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; -import java.util.concurrent.atomic.AtomicBoolean; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.Supplier; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.stream.Collectors; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -68,21 +71,12 @@ class SpannerCloudMonitoringExporter implements MetricExporter { // This the quota limit from Cloud Monitoring. More details in // https://cloud.google.com/monitoring/quotas#custom_metrics_quotas. private static final int EXPORT_BATCH_SIZE_LIMIT = 200; - private final AtomicBoolean spannerExportFailureLogged = new AtomicBoolean(false); + private final Set spannerExportFailureLoggedProjects = ConcurrentHashMap.newKeySet(); private final MetricServiceClient client; - private final Supplier spannerProjectIdSupplier; + private final Supplier fallbackProjectIdSupplier; static SpannerCloudMonitoringExporter create( - String projectId, - @Nullable Credentials credentials, - @Nullable String monitoringHost, - String universeDomain) - throws IOException { - return create(() -> projectId, credentials, monitoringHost, universeDomain); - } - - static SpannerCloudMonitoringExporter create( - Supplier projectIdSupplier, + Supplier fallbackProjectIdSupplier, @Nullable Credentials credentials, @Nullable String monitoringHost, String universeDomain) @@ -121,18 +115,19 @@ static SpannerCloudMonitoringExporter create( settingsBuilder.createServiceTimeSeriesSettings().setSimpleTimeoutNoRetriesDuration(timeout); return new SpannerCloudMonitoringExporter( - projectIdSupplier, MetricServiceClient.create(settingsBuilder.build())); + fallbackProjectIdSupplier, MetricServiceClient.create(settingsBuilder.build())); } @VisibleForTesting - SpannerCloudMonitoringExporter(String projectId, MetricServiceClient client) { - this(() -> projectId, client); + SpannerCloudMonitoringExporter(MetricServiceClient client) { + this(() -> null, client); } @VisibleForTesting - SpannerCloudMonitoringExporter(Supplier projectIdSupplier, MetricServiceClient client) { + SpannerCloudMonitoringExporter( + Supplier fallbackProjectIdSupplier, MetricServiceClient client) { this.client = client; - this.spannerProjectIdSupplier = projectIdSupplier; + this.fallbackProjectIdSupplier = fallbackProjectIdSupplier; } @Override @@ -152,10 +147,6 @@ MetricServiceClient getMetricServiceClient() { /** Export client built in metrics */ private CompletableResultCode exportSpannerClientMetrics(Collection collection) { - String spannerProjectId = spannerProjectIdSupplier.get(); - if (Strings.isNullOrEmpty(spannerProjectId)) { - return CompletableResultCode.ofSuccess(); - } // Skips exporting if there's none if (collection.isEmpty()) { return CompletableResultCode.ofSuccess(); @@ -165,7 +156,7 @@ private CompletableResultCode exportSpannerClientMetrics(Collection try { spannerTimeSeries = SpannerCloudMonitoringExporterUtils.convertToSpannerTimeSeries( - collection, spannerProjectId); + collection, fallbackProjectIdSupplier.get()); } catch (Throwable e) { logger.log( Level.WARNING, @@ -174,9 +165,48 @@ private CompletableResultCode exportSpannerClientMetrics(Collection return CompletableResultCode.ofFailure(); } - ProjectName projectName = ProjectName.of(spannerProjectId); + if (spannerTimeSeries.isEmpty()) { + return CompletableResultCode.ofSuccess(); + } - ApiFuture> futureList = exportTimeSeriesInBatch(projectName, spannerTimeSeries); + Map> timeSeriesByProject = + spannerTimeSeries.stream() + .collect( + Collectors.groupingBy( + timeSeries -> + timeSeries + .getResource() + .getLabelsMap() + .get(BuiltInMetricsConstant.PROJECT_ID_KEY.getKey()))); + + List>> futures = new ArrayList<>(); + for (Map.Entry> entry : timeSeriesByProject.entrySet()) { + ProjectName projectName = ProjectName.of(entry.getKey()); + ApiFuture> future = exportTimeSeriesInBatch(projectName, entry.getValue()); + ApiFutures.addCallback( + future, + new ApiFutureCallback>() { + @Override + public void onFailure(Throwable throwable) { + logExportFailure(throwable, projectName); + } + + @Override + public void onSuccess(List ignored) { + spannerExportFailureLoggedProjects.remove(projectName.getProject()); + } + }, + MoreExecutors.directExecutor()); + futures.add(future); + } + + ApiFuture>> groupedFuture = ApiFutures.allAsList(futures); + ApiFuture> futureList = + ApiFutures.transform( + groupedFuture, + groupedResults -> + groupedResults.stream().flatMap(List::stream).collect(Collectors.toList()), + MoreExecutors.directExecutor()); CompletableResultCode spannerExportCode = new CompletableResultCode(); ApiFutures.addCallback( @@ -184,27 +214,11 @@ private CompletableResultCode exportSpannerClientMetrics(Collection new ApiFutureCallback>() { @Override public void onFailure(Throwable throwable) { - if (spannerExportFailureLogged.compareAndSet(false, true)) { - String msg = "createServiceTimeSeries request failed for spanner metrics."; - if (throwable instanceof PermissionDeniedException) { - // TODO: Add the link of public documentation when available in the log message. - msg += - String.format( - " Need monitoring metric writer permission on project=%s. Follow" - + " https://cloud.google.com/spanner/docs/view-manage-client-side-metrics#access-client-side-metrics" - + " to set up permissions", - projectName.getProject()); - } - logger.log(Level.WARNING, msg, throwable); - } spannerExportCode.fail(); } @Override public void onSuccess(List empty) { - // When an export succeeded reset the export failure flag to false so if there's a - // transient failure it'll be logged. - spannerExportFailureLogged.set(false); spannerExportCode.succeed(); } }, @@ -213,6 +227,25 @@ public void onSuccess(List empty) { return spannerExportCode; } + private void logExportFailure(Throwable throwable, ProjectName projectName) { + if (spannerExportFailureLoggedProjects.add(projectName.getProject())) { + String msg = "createServiceTimeSeries request failed for spanner metrics."; + if (throwable instanceof PermissionDeniedException) { + // TODO: Add the link of public documentation when available in the log message. + msg += + String.format( + " Need monitoring metric writer permission on project=%s. Follow" + + " https://cloud.google.com/spanner/docs/view-manage-client-side-metrics" + + "#access-client-side-metrics" + + " to set up permissions", + projectName.getProject()); + } else { + msg += String.format(" project=%s.", projectName.getProject()); + } + logger.log(Level.WARNING, msg, throwable); + } + } + private ApiFuture> exportTimeSeriesInBatch( ProjectName projectName, List timeSeries) { List> batchResults = new ArrayList<>(); diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java index 92e8c70e5573..35ec4bf15ccf 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java @@ -30,6 +30,7 @@ import static com.google.cloud.spanner.BuiltInMetricsConstant.SPANNER_METER_NAME; import static com.google.cloud.spanner.BuiltInMetricsConstant.SPANNER_PROMOTED_RESOURCE_LABELS; import static com.google.cloud.spanner.BuiltInMetricsConstant.SPANNER_RESOURCE_TYPE; +import static com.google.cloud.spanner.BuiltInMetricsConstant.UNDEFINED_PROJECT_ID; import com.google.api.Distribution; import com.google.api.Distribution.BucketOptions; @@ -64,9 +65,11 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.Objects; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; +import javax.annotation.Nullable; class SpannerCloudMonitoringExporterUtils { @@ -76,7 +79,7 @@ class SpannerCloudMonitoringExporterUtils { private SpannerCloudMonitoringExporterUtils() {} static List convertToSpannerTimeSeries( - Collection collection, String projectId) { + Collection collection, String fallbackProjectId) { List allTimeSeries = new ArrayList<>(); for (MetricData metricData : collection) { @@ -95,26 +98,30 @@ static List convertToSpannerTimeSeries( Attributes resourceAttributes = metricData.getResource().getAttributes(); for (AttributeKey key : resourceAttributes.asMap().keySet()) { - monitoredResourceBuilder.putLabels( - key.getKey(), String.valueOf(resourceAttributes.get(key))); + if (!PROJECT_ID_KEY.equals(key)) { + monitoredResourceBuilder.putLabels( + key.getKey(), String.valueOf(resourceAttributes.get(key))); + } } - monitoredResourceBuilder.putLabels(PROJECT_ID_KEY.getKey(), projectId); + MonitoredResource baseMonitoredResource = monitoredResourceBuilder.build(); metricData.getData().getPoints().stream() .map( pointData -> convertPointToSpannerTimeSeries( - metricData, pointData, monitoredResourceBuilder, projectId)) + metricData, pointData, baseMonitoredResource, fallbackProjectId)) + .filter(Objects::nonNull) .forEach(allTimeSeries::add); } return allTimeSeries; } + @Nullable private static TimeSeries convertPointToSpannerTimeSeries( MetricData metricData, PointData pointData, - MonitoredResource.Builder monitoredResourceBuilder, - String projectId) { + MonitoredResource baseMonitoredResource, + String fallbackProjectId) { MetricKind metricKind = convertMetricKind(metricData); TimeSeries.Builder builder = TimeSeries.newBuilder() @@ -123,9 +130,20 @@ private static TimeSeries convertPointToSpannerTimeSeries( Metric.Builder metricBuilder = Metric.newBuilder().setType(metricData.getName()); Attributes attributes = pointData.getAttributes(); + String projectId = attributes.get(PROJECT_ID_KEY); + if (!isUsableProjectId(projectId)) { + projectId = shouldUseFallbackProject(metricData) ? fallbackProjectId : null; + } + if (!isUsableProjectId(projectId)) { + return null; + } + MonitoredResource.Builder monitoredResourceBuilder = baseMonitoredResource.toBuilder(); + monitoredResourceBuilder.putLabels(PROJECT_ID_KEY.getKey(), projectId); for (AttributeKey key : attributes.asMap().keySet()) { - if (SPANNER_PROMOTED_RESOURCE_LABELS.contains(key)) { + if (PROJECT_ID_KEY.equals(key)) { + continue; + } else 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 @@ -156,6 +174,15 @@ private static TimeSeries convertPointToSpannerTimeSeries( return builder.build(); } + private static boolean isUsableProjectId(String projectId) { + return projectId != null && !projectId.isEmpty() && !UNDEFINED_PROJECT_ID.equals(projectId); + } + + private static boolean shouldUseFallbackProject(MetricData metricData) { + String meterName = metricData.getInstrumentationScopeInfo().getName(); + return GRPC_METER_NAME.equals(meterName) || GRPC_GCP_METER_NAME.equals(meterName); + } + private static MetricKind convertMetricKind(MetricData metricData) { switch (metricData.getType()) { case HISTOGRAM: diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index a2fcfd9a81b1..dab7892f4a80 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -2523,11 +2523,6 @@ public void enablegRPCMetrics(InstantiatingGrpcChannelProvider.Builder channelPr void initializeBuiltInMetrics(DatabaseId databaseId) { if (isEnableBuiltInMetrics() && !usesNoCredentials()) { this.builtInMetricsProvider.setProjectIdIfAbsent(databaseId.getInstanceId().getProject()); - this.builtInMetricsProvider.getOrCreateOpenTelemetry( - databaseId.getInstanceId().getProject(), - getCredentials(), - this.monitoringHost, - getUniverseDomain()); } } diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java index 638804b1633a..9374334a86ad 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java @@ -16,6 +16,7 @@ package com.google.cloud.spanner.spi.v1; import static com.google.api.gax.grpc.GrpcCallContext.TRACER_KEY; +import static com.google.cloud.spanner.BuiltInMetricsConstant.UNDEFINED_PROJECT_ID; 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; @@ -268,7 +269,7 @@ private DatabaseName extractDatabaseName(Metadata headers) throws ExecutionExcep return databaseNameCache.get( googleResourcePrefix, () -> { - String projectId = "undefined-project"; + String projectId = UNDEFINED_PROJECT_ID; String instanceId = "undefined-database"; String databaseId = "undefined-database"; Matcher matcher = GOOGLE_CLOUD_RESOURCE_PREFIX_PATTERN.matcher(googleResourcePrefix); @@ -329,6 +330,10 @@ private Map getBuiltInMetricAttributes(String key, DatabaseName key, () -> { Map attributes = new HashMap<>(); + if (!UNDEFINED_PROJECT_ID.equals(databaseName.getProject())) { + attributes.put( + BuiltInMetricsConstant.PROJECT_ID_KEY.getKey(), databaseName.getProject()); + } attributes.put(BuiltInMetricsConstant.DATABASE_KEY.getKey(), databaseName.getDatabase()); attributes.put( BuiltInMetricsConstant.INSTANCE_ID_KEY.getKey(), databaseName.getInstance()); diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProviderTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProviderTest.java index 5c7e8977227d..1f2adb41a218 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProviderTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProviderTest.java @@ -95,6 +95,7 @@ public void testInitializeBuiltInMetricsUsesDatabaseProject() { SpannerOptions options = newTestOptions(); options.initializeBuiltInMetrics(DatabaseId.of("database-project", "i", "d")); + options.initializeBuiltInMetrics(DatabaseId.of("other-project", "i", "d")); assertEquals("database-project", BuiltInMetricsProvider.INSTANCE.getProjectId()); } diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java index 2d2598a3bff6..8f642eefe120 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java @@ -23,12 +23,14 @@ import static com.google.cloud.spanner.BuiltInMetricsConstant.DIRECT_PATH_ENABLED_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.DIRECT_PATH_USED_KEY; 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_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.OPERATION_COUNT_NAME; import static com.google.cloud.spanner.BuiltInMetricsConstant.OPERATION_LATENCIES_NAME; import static com.google.cloud.spanner.BuiltInMetricsConstant.PROJECT_ID_KEY; +import static com.google.cloud.spanner.BuiltInMetricsConstant.UNDEFINED_PROJECT_ID; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.mock; @@ -106,12 +108,13 @@ public class SpannerCloudMonitoringExporterTest { @Before public void setUp() { fakeMetricServiceClient = new FakeMetricServiceClient(mockMetricServiceStub); - exporter = new SpannerCloudMonitoringExporter(projectId, fakeMetricServiceClient); + exporter = new SpannerCloudMonitoringExporter(fakeMetricServiceClient); this.client_uid = BuiltInMetricsProvider.INSTANCE.createClientAttributes().get("client_uid"); attributes = Attributes.builder() + .put(PROJECT_ID_KEY, projectId) .put(INSTANCE_ID_KEY, instanceId) .put(DATABASE_KEY, databaseId) .put(CLIENT_NAME_KEY, clientName) @@ -155,7 +158,7 @@ public void testExportingSumData() { MetricData longData = ImmutableMetricData.createLongSum( resource, - scope, + InstrumentationScopeInfo.create(GRPC_METER_NAME), "spanner.googleapis.com/internal/client/" + OPERATION_COUNT_NAME, "description", "1", @@ -200,7 +203,7 @@ public void testExportingSumData() { } @Test - public void testExportingOverridesResourceProject() { + public void testExportingUsesPointProjectOverResourceProject() { ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(CreateTimeSeriesRequest.class); @@ -212,7 +215,10 @@ public void testExportingOverridesResourceProject() { Resource resourceWithDifferentProject = Resource.create( resourceAttributes.toBuilder().put(PROJECT_ID_KEY, "resource-project").build()); - LongPointData longPointData = ImmutableLongPointData.create(10, 15, attributes, 11L); + Attributes attributesWithDifferentProject = + attributes.toBuilder().put(PROJECT_ID_KEY, "target-project").build(); + LongPointData longPointData = + ImmutableLongPointData.create(10, 15, attributesWithDifferentProject, 11L); MetricData longData = ImmutableMetricData.createLongSum( resourceWithDifferentProject, @@ -226,18 +232,30 @@ public void testExportingOverridesResourceProject() { exporter.export(Collections.singletonList(longData)); CreateTimeSeriesRequest request = argumentCaptor.getValue(); - assertThat(request.getName()).isEqualTo("projects/" + projectId); + assertThat(request.getName()).isEqualTo("projects/target-project"); assertThat(request.getTimeSeries(0).getResource().getLabelsMap()) - .containsEntry(PROJECT_ID_KEY.getKey(), projectId); + .containsEntry(PROJECT_ID_KEY.getKey(), "target-project"); } @Test - public void testExportingSkipsUntilProjectIsSet() { - exporter = new SpannerCloudMonitoringExporter(() -> null, fakeMetricServiceClient); - LongPointData longPointData = ImmutableLongPointData.create(10, 15, attributes, 11L); + public void testExportingSkipsMetricsWithoutPointProjectOrFallbackProject() { + Resource resourceWithProject = + Resource.create( + resourceAttributes.toBuilder().put(PROJECT_ID_KEY, "resource-project").build()); + Attributes attributesWithoutProject = + Attributes.builder() + .put(INSTANCE_ID_KEY, instanceId) + .put(DATABASE_KEY, databaseId) + .put(CLIENT_NAME_KEY, clientName) + .put(CLIENT_UID_KEY, this.client_uid) + .put(String.valueOf(DIRECT_PATH_ENABLED_KEY), true) + .put(String.valueOf(DIRECT_PATH_USED_KEY), true) + .build(); + LongPointData longPointData = + ImmutableLongPointData.create(10, 15, attributesWithoutProject, 11L); MetricData longData = ImmutableMetricData.createLongSum( - resource, + resourceWithProject, scope, "spanner.googleapis.com/internal/client/" + OPERATION_COUNT_NAME, "description", @@ -250,6 +268,141 @@ public void testExportingSkipsUntilProjectIsSet() { Mockito.verify(mockMetricServiceStub, Mockito.never()).createServiceTimeSeriesCallable(); } + @Test + public void testExportingUsesFallbackProjectForMetricsWithUndefinedPointProject() { + exporter = + new SpannerCloudMonitoringExporter(() -> "fallback-project", fakeMetricServiceClient); + ArgumentCaptor argumentCaptor = + ArgumentCaptor.forClass(CreateTimeSeriesRequest.class); + + UnaryCallable mockCallable = mock(UnaryCallable.class); + when(mockMetricServiceStub.createServiceTimeSeriesCallable()).thenReturn(mockCallable); + ApiFuture future = ApiFutures.immediateFuture(Empty.getDefaultInstance()); + when(mockCallable.futureCall(argumentCaptor.capture())).thenReturn(future); + + Attributes attributesWithUndefinedProject = + Attributes.builder() + .put(PROJECT_ID_KEY, UNDEFINED_PROJECT_ID) + .put(INSTANCE_ID_KEY, instanceId) + .put(DATABASE_KEY, databaseId) + .put(CLIENT_NAME_KEY, clientName) + .put(CLIENT_UID_KEY, this.client_uid) + .build(); + LongPointData longPointData = + ImmutableLongPointData.create(10, 15, attributesWithUndefinedProject, 11L); + MetricData longData = + ImmutableMetricData.createLongSum( + resource, + InstrumentationScopeInfo.create(GRPC_METER_NAME), + "spanner.googleapis.com/internal/client/" + OPERATION_COUNT_NAME, + "description", + "1", + ImmutableSumData.create( + true, AggregationTemporality.CUMULATIVE, ImmutableList.of(longPointData))); + + exporter.export(Collections.singletonList(longData)); + + CreateTimeSeriesRequest request = argumentCaptor.getValue(); + assertThat(request.getName()).isEqualTo("projects/fallback-project"); + assertThat(request.getTimeSeries(0).getResource().getLabelsMap()) + .containsEntry(PROJECT_ID_KEY.getKey(), "fallback-project"); + } + + @Test + public void testExportingRoutesPointProjectAndFallbackProject() { + exporter = + new SpannerCloudMonitoringExporter(() -> "fallback-project", fakeMetricServiceClient); + ArgumentCaptor argumentCaptor = + ArgumentCaptor.forClass(CreateTimeSeriesRequest.class); + + UnaryCallable mockCallable = mock(UnaryCallable.class); + when(mockMetricServiceStub.createServiceTimeSeriesCallable()).thenReturn(mockCallable); + ApiFuture future = ApiFutures.immediateFuture(Empty.getDefaultInstance()); + when(mockCallable.futureCall(argumentCaptor.capture())).thenReturn(future); + + LongPointData pointWithProject = + ImmutableLongPointData.create( + 10, 15, attributes.toBuilder().put(PROJECT_ID_KEY, "point-project").build(), 100L); + LongPointData pointWithoutProject = + ImmutableLongPointData.create( + 10, 15, Attributes.builder().put(INSTANCE_ID_KEY, instanceId).build(), 200L); + MetricData metricWithProject = + ImmutableMetricData.createLongSum( + resource, + scope, + "spanner.googleapis.com/internal/client/" + OPERATION_COUNT_NAME, + "description", + "1", + ImmutableSumData.create( + true, AggregationTemporality.CUMULATIVE, ImmutableList.of(pointWithProject))); + MetricData metricWithoutProject = + ImmutableMetricData.createLongSum( + resource, + InstrumentationScopeInfo.create(GRPC_METER_NAME), + "spanner.googleapis.com/internal/client/" + OPERATION_COUNT_NAME, + "description", + "1", + ImmutableSumData.create( + true, AggregationTemporality.CUMULATIVE, ImmutableList.of(pointWithoutProject))); + + exporter.export(Arrays.asList(metricWithProject, metricWithoutProject)); + + Map requestsByName = + argumentCaptor.getAllValues().stream() + .collect(Collectors.toMap(CreateTimeSeriesRequest::getName, request -> request)); + assertThat(requestsByName.keySet()) + .containsExactly("projects/point-project", "projects/fallback-project"); + } + + @Test + public void testExportingRoutesMultipleProjects() { + ArgumentCaptor argumentCaptor = + ArgumentCaptor.forClass(CreateTimeSeriesRequest.class); + + UnaryCallable mockCallable = mock(UnaryCallable.class); + when(mockMetricServiceStub.createServiceTimeSeriesCallable()).thenReturn(mockCallable); + ApiFuture future = ApiFutures.immediateFuture(Empty.getDefaultInstance()); + when(mockCallable.futureCall(argumentCaptor.capture())).thenReturn(future); + + Attributes attributesProjectX = attributes.toBuilder().put(PROJECT_ID_KEY, "project-x").build(); + Attributes attributesProjectY = attributes.toBuilder().put(PROJECT_ID_KEY, "project-y").build(); + LongPointData pointX = ImmutableLongPointData.create(10, 15, attributesProjectX, 100L); + LongPointData pointY = ImmutableLongPointData.create(10, 15, attributesProjectY, 200L); + + MetricData metricX = + ImmutableMetricData.createLongSum( + resource, + scope, + "spanner.googleapis.com/internal/client/" + OPERATION_COUNT_NAME, + "description", + "1", + ImmutableSumData.create( + true, AggregationTemporality.CUMULATIVE, ImmutableList.of(pointX))); + MetricData metricY = + ImmutableMetricData.createLongSum( + resource, + scope, + "spanner.googleapis.com/internal/client/" + OPERATION_COUNT_NAME, + "description", + "1", + ImmutableSumData.create( + true, AggregationTemporality.CUMULATIVE, ImmutableList.of(pointY))); + + exporter.export(Arrays.asList(metricX, metricY)); + + assertThat(argumentCaptor.getAllValues()).hasSize(2); + Map requestsByName = + argumentCaptor.getAllValues().stream() + .collect(Collectors.toMap(CreateTimeSeriesRequest::getName, request -> request)); + assertThat(requestsByName.keySet()).containsExactly("projects/project-x", "projects/project-y"); + assertThat( + requestsByName.get("projects/project-x").getTimeSeries(0).getResource().getLabelsMap()) + .containsEntry(PROJECT_ID_KEY.getKey(), "project-x"); + assertThat( + requestsByName.get("projects/project-y").getTimeSeries(0).getResource().getLabelsMap()) + .containsEntry(PROJECT_ID_KEY.getKey(), "project-y"); + } + @Test public void testExportingHistogramData() { ArgumentCaptor argumentCaptor = @@ -500,7 +653,7 @@ public void testExportingHistogramDataWithExemplars() { @Test public void getAggregationTemporality() throws IOException { SpannerCloudMonitoringExporter actualExporter = - SpannerCloudMonitoringExporter.create(projectId, null, null, null); + SpannerCloudMonitoringExporter.create(() -> null, null, null, null); assertThat(actualExporter.getAggregationTemporality(InstrumentType.COUNTER)) .isEqualTo(AggregationTemporality.CUMULATIVE); } @@ -508,7 +661,7 @@ public void getAggregationTemporality() throws IOException { @Test public void testUniverseDomain() throws IOException { SpannerCloudMonitoringExporter actualExporter = - SpannerCloudMonitoringExporter.create(projectId, null, null, "abc.goog"); + SpannerCloudMonitoringExporter.create(() -> null, null, null, "abc.goog"); MetricServiceSettings metricServiceSettings = actualExporter.getMetricServiceClient().getSettings(); @@ -517,7 +670,7 @@ public void testUniverseDomain() throws IOException { actualExporter = SpannerCloudMonitoringExporter.create( - projectId, null, "monitoringa.abc.goog:443", "abc.goog"); + () -> null, null, "monitoringa.abc.goog:443", "abc.goog"); metricServiceSettings = actualExporter.getMetricServiceClient().getSettings(); assertEquals("abc.goog", metricServiceSettings.getUniverseDomain()); From f8cfb70a9370c41f777de9077ddc73b61ce7150f Mon Sep 17 00:00:00 2001 From: Rahul Yadav Date: Fri, 29 May 2026 19:13:04 +0530 Subject: [PATCH 4/5] fix test --- .../cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java index a086ad46db7a..8a0c75c86179 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java @@ -74,6 +74,7 @@ public class OpenTelemetryBuiltInMetricsTracerTest extends AbstractNettyMockServ BuiltInMetricsProvider.INSTANCE.createClientAttributes(); private static final Attributes expectedCommonBaseAttributes = Attributes.builder() + .put(BuiltInMetricsConstant.PROJECT_ID_KEY, "test-project") .put(BuiltInMetricsConstant.CLIENT_NAME_KEY, "spanner-java/") .put(BuiltInMetricsConstant.CLIENT_UID_KEY, attributes.get("client_uid")) .put(BuiltInMetricsConstant.INSTANCE_ID_KEY, "i") From 07599d7f0243164838a20df22b10290a8616a5b3 Mon Sep 17 00:00:00 2001 From: Rahul Yadav Date: Fri, 29 May 2026 20:07:00 +0530 Subject: [PATCH 5/5] remove TODO --- .../com/google/cloud/spanner/SpannerCloudMonitoringExporter.java | 1 - 1 file changed, 1 deletion(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java index db995387cc48..be1b04f1df07 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java @@ -231,7 +231,6 @@ private void logExportFailure(Throwable throwable, ProjectName projectName) { if (spannerExportFailureLoggedProjects.add(projectName.getProject())) { String msg = "createServiceTimeSeries request failed for spanner metrics."; if (throwable instanceof PermissionDeniedException) { - // TODO: Add the link of public documentation when available in the log message. msg += String.format( " Need monitoring metric writer permission on project=%s. Follow"