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 0a51ebfae26f..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 @@ -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,13 @@ final class BuiltInMetricsProvider { private static final String default_location = "global"; private OpenTelemetry openTelemetry; + private String projectId; + private boolean mismatchedProjectIdLogged; + private Thread shutdownHook; private BuiltInMetricsProvider() {} - OpenTelemetry getOrCreateOpenTelemetry( + synchronized OpenTelemetry getOrCreateOpenTelemetry( String projectId, @Nullable Credentials credentials, @Nullable String monitoringHost, @@ -88,12 +92,13 @@ 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(); 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) { @@ -106,6 +111,47 @@ 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 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}); + } + } + + @Nullable + synchronized OpenTelemetry getOpenTelemetry() { + return this.openTelemetry; + } + + synchronized String getProjectId() { + return this.projectId; + } + + @VisibleForTesting + 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 // 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..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 @@ -42,13 +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.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; @@ -69,13 +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 AtomicBoolean lastExportSkippedData = new AtomicBoolean(false); + private final Set spannerExportFailureLoggedProjects = ConcurrentHashMap.newKeySet(); private final MetricServiceClient client; - private final String spannerProjectId; + private final Supplier fallbackProjectIdSupplier; static SpannerCloudMonitoringExporter create( - String projectId, + Supplier fallbackProjectIdSupplier, @Nullable Credentials credentials, @Nullable String monitoringHost, String universeDomain) @@ -114,13 +115,19 @@ static SpannerCloudMonitoringExporter create( settingsBuilder.createServiceTimeSeriesSettings().setSimpleTimeoutNoRetriesDuration(timeout); return new SpannerCloudMonitoringExporter( - projectId, MetricServiceClient.create(settingsBuilder.build())); + fallbackProjectIdSupplier, MetricServiceClient.create(settingsBuilder.build())); } @VisibleForTesting - SpannerCloudMonitoringExporter(String projectId, MetricServiceClient client) { + SpannerCloudMonitoringExporter(MetricServiceClient client) { + this(() -> null, client); + } + + @VisibleForTesting + SpannerCloudMonitoringExporter( + Supplier fallbackProjectIdSupplier, MetricServiceClient client) { this.client = client; - this.spannerProjectId = projectId; + this.fallbackProjectIdSupplier = fallbackProjectIdSupplier; } @Override @@ -140,29 +147,8 @@ 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()); - } - lastExportSkippedData.set(mustFilter); - // Skips exporting if there's none - if (spannerMetricData.isEmpty()) { + if (collection.isEmpty()) { return CompletableResultCode.ofSuccess(); } @@ -170,7 +156,7 @@ private CompletableResultCode exportSpannerClientMetrics(Collection try { spannerTimeSeries = SpannerCloudMonitoringExporterUtils.convertToSpannerTimeSeries( - spannerMetricData, this.spannerProjectId); + collection, fallbackProjectIdSupplier.get()); } catch (Throwable e) { logger.log( Level.WARNING, @@ -179,9 +165,48 @@ private CompletableResultCode exportSpannerClientMetrics(Collection return CompletableResultCode.ofFailure(); } - ProjectName projectName = ProjectName.of(spannerProjectId); + if (spannerTimeSeries.isEmpty()) { + return CompletableResultCode.ofSuccess(); + } + + 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> futureList = exportTimeSeriesInBatch(projectName, spannerTimeSeries); + 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( @@ -189,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(); } }, @@ -218,16 +227,22 @@ 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(); + private void logExportFailure(Throwable throwable, ProjectName projectName) { + if (spannerExportFailureLoggedProjects.add(projectName.getProject())) { + String msg = "createServiceTimeSeries request failed for spanner metrics."; + if (throwable instanceof PermissionDeniedException) { + 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( 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..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; @@ -61,12 +62,14 @@ 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.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 { @@ -75,12 +78,8 @@ class SpannerCloudMonitoringExporterUtils { private SpannerCloudMonitoringExporterUtils() {} - static String getProjectId(Resource resource) { - return resource.getAttributes().get(PROJECT_ID_KEY); - } - static List convertToSpannerTimeSeries( - List collection, String projectId) { + Collection collection, String fallbackProjectId) { List allTimeSeries = new ArrayList<>(); for (MetricData metricData : collection) { @@ -99,25 +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))); + } } + 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() @@ -126,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 @@ -159,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/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..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 @@ -2520,6 +2520,12 @@ public void enablegRPCMetrics(InstantiatingGrpcChannelProvider.Builder channelPr } } + void initializeBuiltInMetrics(DatabaseId databaseId) { + if (isEnableBuiltInMetrics() && !usesNoCredentials()) { + this.builtInMetricsProvider.setProjectIdIfAbsent(databaseId.getInstanceId().getProject()); + } + } + public ApiTracerFactory getApiTracerFactory(boolean isAdminClient, boolean isEmulatorEnabled) { return createApiTracerFactory(isAdminClient, isEmulatorEnabled); } 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 73185177de19..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 @@ -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,34 @@ 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")); + options.initializeBuiltInMetrics(DatabaseId.of("other-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 +107,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/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") 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..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,15 +23,16 @@ 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.junit.Assert.assertFalse; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -107,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) @@ -156,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", @@ -164,8 +166,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); @@ -202,6 +202,207 @@ public void testExportingSumData() { assertThat(timeSeries.getPoints(0).getInterval().getEndTime().getNanos()).isEqualTo(endEpoch); } + @Test + public void testExportingUsesPointProjectOverResourceProject() { + 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()); + Attributes attributesWithDifferentProject = + attributes.toBuilder().put(PROJECT_ID_KEY, "target-project").build(); + LongPointData longPointData = + ImmutableLongPointData.create(10, 15, attributesWithDifferentProject, 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/target-project"); + assertThat(request.getTimeSeries(0).getResource().getLabelsMap()) + .containsEntry(PROJECT_ID_KEY.getKey(), "target-project"); + } + + @Test + 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( + resourceWithProject, + scope, + "spanner.googleapis.com/internal/client/" + OPERATION_COUNT_NAME, + "description", + "1", + ImmutableSumData.create( + true, AggregationTemporality.CUMULATIVE, ImmutableList.of(longPointData))); + + exporter.export(Collections.singletonList(longData)); + + 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 = @@ -238,8 +439,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); @@ -314,8 +513,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) { @@ -408,8 +605,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(); @@ -458,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); } @@ -466,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(); @@ -475,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()); 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