fix: preserve channel configurator for grpc-gcp and add opt-out for gcp OTel metrics

Author: rahul2393

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

@@ -227,6 +227,7 @@ public static GcpChannelPoolOptions createDefaultDynamicChannelPoolOptions() {
   private final boolean autoThrottleAdministrativeRequests;
   private final RetrySettings retryAdministrativeRequestsSettings;
   private final boolean trackTransactionStarter;
+  private final boolean enableGrpcGcpOtelMetrics;
   private final BuiltInMetricsProvider builtInMetricsProvider = BuiltInMetricsProvider.INSTANCE;
 
   /**
@@ -895,6 +896,7 @@ protected SpannerOptions(Builder builder) {
     autoThrottleAdministrativeRequests = builder.autoThrottleAdministrativeRequests;
     retryAdministrativeRequestsSettings = builder.retryAdministrativeRequestsSettings;
     trackTransactionStarter = builder.trackTransactionStarter;
+    enableGrpcGcpOtelMetrics = builder.enableGrpcGcpOtelMetrics;
     defaultQueryOptions = builder.defaultQueryOptions;
     envQueryOptions = builder.getEnvironmentQueryOptions();
     if (envQueryOptions.equals(QueryOptions.getDefaultInstance())) {
@@ -975,6 +977,10 @@ default boolean isEnableGRPCBuiltInMetrics() {
       return false;
     }
 
+    default boolean isEnableGrpcGcpOtelMetrics() {
+      return true;
+    }
+
     default boolean isEnableEndToEndTracing() {
       return false;
     }
@@ -1013,6 +1019,8 @@ private static class SpannerEnvironmentImpl implements SpannerEnvironment {
     private static final String SPANNER_DISABLE_BUILTIN_METRICS = "SPANNER_DISABLE_BUILTIN_METRICS";
     private static final String SPANNER_DISABLE_DIRECT_ACCESS_GRPC_BUILTIN_METRICS =
         "SPANNER_DISABLE_DIRECT_ACCESS_GRPC_BUILTIN_METRICS";
+    private static final String SPANNER_DISABLE_GRPC_GCP_OTEL_METRICS =
+        "SPANNER_DISABLE_GRPC_GCP_OTEL_METRICS";
     private static final String SPANNER_MONITORING_HOST = "SPANNER_MONITORING_HOST";
 
     private SpannerEnvironmentImpl() {}
@@ -1058,6 +1066,11 @@ public boolean isEnableGRPCBuiltInMetrics() {
           System.getenv(SPANNER_DISABLE_DIRECT_ACCESS_GRPC_BUILTIN_METRICS));
     }
 
+    @Override
+    public boolean isEnableGrpcGcpOtelMetrics() {
+      return !Boolean.parseBoolean(System.getenv(SPANNER_DISABLE_GRPC_GCP_OTEL_METRICS));
+    }
+
     @Override
     public boolean isEnableEndToEndTracing() {
       return Boolean.parseBoolean(System.getenv(SPANNER_ENABLE_END_TO_END_TRACING));
@@ -1128,6 +1141,8 @@ public static class Builder
     private boolean autoThrottleAdministrativeRequests = false;
     private boolean trackTransactionStarter = false;
     private Map<DatabaseId, QueryOptions> defaultQueryOptions = new HashMap<>();
+    private boolean enableGrpcGcpOtelMetrics =
+        SpannerOptions.environment.isEnableGrpcGcpOtelMetrics();
     private CallCredentialsProvider callCredentialsProvider;
     private CloseableExecutorProvider asyncExecutorProvider;
     private String compressorName;
@@ -1231,6 +1246,7 @@ protected Builder() {
       this.autoThrottleAdministrativeRequests = options.autoThrottleAdministrativeRequests;
       this.retryAdministrativeRequestsSettings = options.retryAdministrativeRequestsSettings;
       this.trackTransactionStarter = options.trackTransactionStarter;
+      this.enableGrpcGcpOtelMetrics = options.enableGrpcGcpOtelMetrics;
       this.defaultQueryOptions = options.defaultQueryOptions;
       this.callCredentialsProvider = options.callCredentialsProvider;
       this.asyncExecutorProvider = options.asyncExecutorProvider;
@@ -1750,6 +1766,17 @@ public Builder disableDynamicChannelPool() {
       return this;
     }
 
+    /**
+     * Sets whether to enable or disable grpc-gcp OpenTelemetry metrics injection. When disabled,
+     * Spanner will not automatically inject an OpenTelemetry {@link
+     * io.opentelemetry.api.metrics.Meter} into grpc-gcp. If a Meter or MetricRegistry is explicitly
+     * provided via {@link GcpManagedChannelOptions}, those settings will still be honored.
+     */
+    public Builder setGrpcGcpOtelMetricsEnabled(boolean enableGrpcGcpOtelMetrics) {
+      this.enableGrpcGcpOtelMetrics = enableGrpcGcpOtelMetrics;
+      return this;
+    }
+
     /**
      * Sets the channel pool options for dynamic channel pooling. Use this to configure the dynamic
      * channel pool behavior when {@link #enableDynamicChannelPool()} is enabled.
@@ -2211,6 +2238,10 @@ public boolean isGrpcGcpExtensionEnabled() {
     return grpcGcpExtensionEnabled;
   }
 
+  public boolean isGrpcGcpOtelMetricsEnabled() {
+    return enableGrpcGcpOtelMetrics;
+  }
+
   public GcpManagedChannelOptions getGrpcGcpOptions() {
     return grpcGcpOptions;
   }

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

@@ -622,7 +622,7 @@ private static GcpManagedChannelOptions grpcGcpOptionsWithMetricsAndDcp(SpannerO
       metricsOptionsBuilder.withNamePrefix("cloud.google.com/java/spanner/gcp-channel-pool/");
     }
     // Pass OpenTelemetry meter to grpc-gcp for channel pool metrics
-    if (metricsOptions.getOpenTelemetryMeter() == null) {
+    if (metricsOptions.getOpenTelemetryMeter() == null && options.isGrpcGcpOtelMetricsEnabled()) {
       metricsOptionsBuilder.withOpenTelemetryMeter(
           options.getOpenTelemetry().getMeter("com.google.cloud.spanner"));
     }
@@ -655,8 +655,10 @@ private static void maybeEnableGrpcGcpExtension(
 
     ApiFunction<ManagedChannelBuilder, ManagedChannelBuilder> apiFunction =
         channelBuilder -> {
-          if (options.getChannelConfigurator() != null) {
-            channelBuilder = options.getChannelConfigurator().apply(channelBuilder);
+          ApiFunction<ManagedChannelBuilder, ManagedChannelBuilder> baseConfigurator =
+              defaultChannelProviderBuilder.getChannelConfigurator();
+          if (baseConfigurator != null) {
+            channelBuilder = baseConfigurator.apply(channelBuilder);
           }
           return GcpManagedChannelBuilder.forDelegateBuilder(channelBuilder)
               .withApiConfigJsonString(jsonApiConfig)

google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java

@@ -27,9 +27,11 @@
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assume.assumeTrue;
 
+import com.google.api.core.ApiFunction;
 import com.google.api.gax.core.GaxProperties;
 import com.google.api.gax.grpc.GrpcCallContext;
 import com.google.api.gax.grpc.GrpcTransportChannel;
+import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider;
 import com.google.api.gax.rpc.ApiCallContext;
 import com.google.api.gax.rpc.ApiClientHeaderProvider;
 import com.google.api.gax.rpc.HeaderProvider;
@@ -38,6 +40,8 @@
 import com.google.auth.oauth2.AccessToken;
 import com.google.auth.oauth2.OAuth2Credentials;
 import com.google.cloud.ServiceOptions;
+import com.google.cloud.grpc.GcpManagedChannelOptions;
+import com.google.cloud.grpc.GcpManagedChannelOptions.GcpMetricsOptions;
 import com.google.cloud.spanner.DatabaseClient;
 import com.google.cloud.spanner.DatabaseId;
 import com.google.cloud.spanner.Dialect;
@@ -80,6 +84,7 @@
 import io.grpc.ServerInterceptor;
 import io.grpc.Status;
 import io.grpc.auth.MoreCallCredentials;
+import io.grpc.netty.shaded.io.grpc.netty.NettyChannelBuilder;
 import io.grpc.netty.shaded.io.grpc.netty.NettyServerBuilder;
 import io.grpc.protobuf.lite.ProtoLiteUtils;
 import io.opentelemetry.api.OpenTelemetry;
@@ -963,6 +968,55 @@ public void testLocationApiDoesNotOverrideExplicitChannelProvider() throws Excep
     }
   }
 
+  @Test
+  public void testGrpcGcpExtensionPreservesChannelConfigurator() throws Exception {
+    InstantiatingGrpcChannelProvider.Builder channelProviderBuilder =
+        InstantiatingGrpcChannelProvider.newBuilder();
+    AtomicBoolean baseConfiguratorCalled = new AtomicBoolean(false);
+    channelProviderBuilder.setChannelConfigurator(
+        builder -> {
+          baseConfiguratorCalled.set(true);
+          return builder;
+        });
+
+    SpannerOptions options =
+        SpannerOptions.newBuilder().setProjectId("[PROJECT]").enableGrpcGcpExtension().build();
+
+    java.lang.reflect.Method method =
+        GapicSpannerRpc.class.getDeclaredMethod(
+            "maybeEnableGrpcGcpExtension",
+            InstantiatingGrpcChannelProvider.Builder.class,
+            SpannerOptions.class);
+    method.setAccessible(true);
+    method.invoke(null, channelProviderBuilder, options);
+
+    ApiFunction<ManagedChannelBuilder, ManagedChannelBuilder> chainedConfigurator =
+        channelProviderBuilder.getChannelConfigurator();
+    chainedConfigurator.apply(NettyChannelBuilder.forAddress("localhost", 1));
+
+    assertTrue(baseConfiguratorCalled.get());
+  }
+
+  @Test
+  public void testGrpcGcpOtelMetricsDisabledSkipsMeterInjection() throws Exception {
+    SpannerOptions options =
+        SpannerOptions.newBuilder()
+            .setProjectId("[PROJECT]")
+            .setGrpcGcpOtelMetricsEnabled(false)
+            .build();
+
+    java.lang.reflect.Method method =
+        GapicSpannerRpc.class.getDeclaredMethod(
+            "grpcGcpOptionsWithMetricsAndDcp", SpannerOptions.class);
+    method.setAccessible(true);
+    GcpManagedChannelOptions grpcGcpOptions =
+        (GcpManagedChannelOptions) method.invoke(null, options);
+    GcpMetricsOptions metricsOptions = grpcGcpOptions.getMetricsOptions();
+
+    assertNotNull(metricsOptions);
+    assertNull(metricsOptions.getOpenTelemetryMeter());
+  }
+
   private static final class RecordingTransportChannelProvider implements TransportChannelProvider {
     private final String host;
     private final int port;