review comments

google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java

@@ -46,8 +46,7 @@
 
 final class BuiltInOpenTelemetryMetricsProvider {
 
-  public static BuiltInOpenTelemetryMetricsProvider INSTANCE =
-      new BuiltInOpenTelemetryMetricsProvider();
+  static BuiltInOpenTelemetryMetricsProvider INSTANCE = new BuiltInOpenTelemetryMetricsProvider();
 
   private static final Logger logger =
       Logger.getLogger(BuiltInOpenTelemetryMetricsProvider.class.getName());
@@ -77,7 +76,7 @@ OpenTelemetry getOrCreateOpenTelemetry(String projectId, @Nullable Credentials c
     }
   }
 
-  Map<String, String> getClientAttributes(
+  Map<String, String> createClientAttributes(
       String projectId, boolean isDirectPathChannelCreated, String client_name) {
     Map<String, String> clientAttributes = new HashMap<>();
     clientAttributes.put(LOCATION_ID_KEY.getKey(), detectClientLocation());

google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java

@@ -1674,7 +1674,7 @@ private ApiTracerFactory createApiTracerFactory(
     // and if emulator is not enabled.
     if (isEnableBuiltInMetrics() && !isAdminClient && !isEmulatorEnabled) {
       ApiTracerFactory metricsTracerFactory =
-          getMetricsApiTracerFactory(isDirectPathChannelCreated);
+          createMetricsApiTracerFactory(isDirectPathChannelCreated);
       if (metricsTracerFactory != null) {
         apiTracerFactories.add(metricsTracerFactory);
       }
@@ -1699,15 +1699,15 @@ private ApiTracerFactory getDefaultApiTracerFactory() {
     return BaseApiTracerFactory.getInstance();
   }
 
-  private ApiTracerFactory getMetricsApiTracerFactory(boolean isDirectPathChannelCreated) {
+  private ApiTracerFactory createMetricsApiTracerFactory(boolean isDirectPathChannelCreated) {
     OpenTelemetry openTelemetry =
         this.builtInOpenTelemetryMetricsProvider.getOrCreateOpenTelemetry(
             getDefaultProjectId(), getCredentials());
 
     return openTelemetry != null
         ? new MetricsTracerFactory(
             new OpenTelemetryMetricsRecorder(openTelemetry, BuiltInMetricsConstant.METER_NAME),
-            builtInOpenTelemetryMetricsProvider.getClientAttributes(
+            builtInOpenTelemetryMetricsProvider.createClientAttributes(
                 getDefaultProjectId(),
                 isDirectPathChannelCreated,
                 "spanner-java/" + GaxProperties.getLibraryVersion(getClass())))

google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java

@@ -366,16 +366,19 @@ public GapicSpannerRpc(final SpannerOptions options) {
       // If it is enabled in options uses the channel pool provided by the gRPC-GCP extension.
       maybeEnableGrpcGcpExtension(defaultChannelProviderBuilder, options);
 
-      InstantiatingGrpcChannelProvider defaultChannelProvider =
-          defaultChannelProviderBuilder.build();
+      InstantiatingGrpcChannelProvider defaultChannelProvider = null;
+      boolean isDirectPathChannelCreated = false;
+
+      if (options.getChannelProvider() == null) {
+        defaultChannelProvider = defaultChannelProviderBuilder.build();
+        isDirectPathChannelCreated =
+            defaultChannelProvider.canUseDirectPath()
+                && defaultChannelProvider.isDirectPathXdsEnabled();
+      }
 
       TransportChannelProvider channelProvider =
           MoreObjects.firstNonNull(options.getChannelProvider(), defaultChannelProvider);
 
-      boolean isDirectPathChannelCreated =
-          defaultChannelProvider.canUseDirectPath()
-              && defaultChannelProvider.isDirectPathXdsEnabled();
-
       CredentialsProvider credentialsProvider =
           GrpcTransportOptions.setUpCredentialsProvider(options);
 
@@ -405,7 +408,7 @@ public GapicSpannerRpc(final SpannerOptions options) {
                     .setTracerFactory(
                         options.getApiTracerFactory(
                             isDirectPathChannelCreated,
-                            false,
+                            /* isAdminClient = */ false,
                             isEmulatorEnabled(options, emulatorHost)))
                     .build());
         this.readRetrySettings =
@@ -436,7 +439,9 @@ public GapicSpannerRpc(final SpannerOptions options) {
             .setStreamWatchdogProvider(watchdogProvider)
             .setTracerFactory(
                 options.getApiTracerFactory(
-                    isDirectPathChannelCreated, false, isEmulatorEnabled(options, emulatorHost)))
+                    isDirectPathChannelCreated,
+                    /* isAdminClient = */ false,
+                    isEmulatorEnabled(options, emulatorHost)))
             .executeSqlSettings()
             .setRetrySettings(partitionedDmlRetrySettings);
         pdmlSettings.executeStreamingSqlSettings().setRetrySettings(partitionedDmlRetrySettings);
@@ -465,7 +470,9 @@ isDirectPathChannelCreated, false, isEmulatorEnabled(options, emulatorHost)))
                 .setStreamWatchdogProvider(watchdogProvider)
                 .setTracerFactory(
                     options.getApiTracerFactory(
-                        isDirectPathChannelCreated, true, isEmulatorEnabled(options, emulatorHost)))
+                        isDirectPathChannelCreated,
+                        /* isAdminClient = */ true,
+                        isEmulatorEnabled(options, emulatorHost)))
                 .build();
         this.instanceAdminStub = GrpcInstanceAdminStub.create(instanceAdminStubSettings);
 
@@ -478,7 +485,9 @@ isDirectPathChannelCreated, true, isEmulatorEnabled(options, emulatorHost)))
                 .setStreamWatchdogProvider(watchdogProvider)
                 .setTracerFactory(
                     options.getApiTracerFactory(
-                        isDirectPathChannelCreated, true, isEmulatorEnabled(options, emulatorHost)))
+                        isDirectPathChannelCreated,
+                        /* isAdminClient = */ true,
+                        isEmulatorEnabled(options, emulatorHost)))
                 .build();
 
         // Automatically retry RESOURCE_EXHAUSTED for GetOperation if auto-throttling of

google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java

@@ -88,7 +88,7 @@ public static void setup() {
 
     String client_name = "spanner-java/";
     openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(meterProvider.build()).build();
-    attributes = provider.getClientAttributes("test-project", true, client_name);
+    attributes = provider.createClientAttributes("test-project", true, client_name);
 
     expectedBaseAttributes =
         Attributes.builder()
@@ -240,7 +240,7 @@ private MetricData getMetricData(InMemoryMetricReader reader, String metricName)
     Collection<MetricData> allMetricData = Collections.emptyList();
 
     // Fetch the MetricData with retries
-    for (int attemptsLeft = 10; attemptsLeft > 0; attemptsLeft--) {
+    for (int attemptsLeft = 1000; attemptsLeft > 0; attemptsLeft--) {
       allMetricData = reader.collectAllMetrics();
       List<MetricData> matchingMetadata =
           allMetricData.stream()
@@ -257,14 +257,14 @@ private MetricData getMetricData(InMemoryMetricReader reader, String metricName)
       }
 
       try {
-        Thread.sleep(100);
+        Thread.sleep(1);
       } catch (InterruptedException interruptedException) {
         Thread.currentThread().interrupt();
         throw new RuntimeException(interruptedException);
       }
     }
 
-    assertFalse(String.format("MetricData is missing for metric {0}", fullMetricName), false);
+    assertTrue(String.format("MetricData is missing for metric {0}", fullMetricName), false);
     return null;
   }
 

google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITBuiltInMetricsTest.java

@@ -18,7 +18,6 @@
 
 import static com.google.common.truth.Truth.assertWithMessage;
 
-import com.google.api.gax.longrunning.OperationFuture;
 import com.google.cloud.monitoring.v3.MetricServiceClient;
 import com.google.cloud.spanner.Database;
 import com.google.cloud.spanner.DatabaseClient;
@@ -31,9 +30,7 @@
 import com.google.monitoring.v3.ProjectName;
 import com.google.monitoring.v3.TimeInterval;
 import com.google.protobuf.util.Timestamps;
-import com.google.spanner.admin.database.v1.UpdateDatabaseDdlMetadata;
 import java.io.IOException;
-import java.util.Collections;
 import java.util.concurrent.TimeUnit;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
@@ -78,14 +75,6 @@ public void testBuiltinMetricsWithDefaultOTEL() throws Exception {
             .setStartTime(Timestamps.fromMillis(start.toEpochMilli()))
             .setEndTime(Timestamps.fromMillis(end.toEpochMilli()))
             .build();
-    String ddl =
-        "CREATE TABLE FOO ("
-            + "  K    STRING(MAX) NOT NULL,"
-            + "  V    INT64,"
-            + ") PRIMARY KEY (K)";
-    OperationFuture<Void, UpdateDatabaseDdlMetadata> op =
-        db.updateDdl(Collections.singletonList(ddl), null);
-    op.get();
 
     client
         .readWriteTransaction()