chore(bigtable): skip server polling on client-config-override and add override-use-default property

java-bigtable/google-cloud-bigtable/pom.xml

@@ -606,6 +606,7 @@
                     <bigtable.data-jwt-audience>${bigtable.cfe-jwt-audience}</bigtable.data-jwt-audience>
                     <bigtable.enable-grpc-logs>${bigtable.enable-grpc-logs}</bigtable.enable-grpc-logs>
                     <bigtable.grpc-log-dir>${project.build.directory}/test-grpc-logs/session-it</bigtable.grpc-log-dir>
+                    <bigtable.internal.client-config-override-use-default>true</bigtable.internal.client-config-override-use-default>
                     <bigtable.internal.client-config-override>session_configuration { session_diversion_configuration { global_diversion { session_load: 1.0 } } }</bigtable.internal.client-config-override>
                   </systemPropertyVariables>
                   <includes>

java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/util/ClientConfigurationManager.java

@@ -67,14 +67,45 @@
 import javax.annotation.concurrent.GuardedBy;
 
 /**
- * ClientConfiguration controls how the data client should behave. For example, it controls how much
- * of the traffic should go through jetstream vs unary. It also controls how frequent the client
- * should poll for a new configuration and how long the configuration is valid for.
+ * Manages the {@link ClientConfiguration} that controls how the data client behaves (for example,
+ * how much traffic is routed through sessions vs unary RPCs, how frequently to poll for a new
+ * configuration, and how long a configuration remains valid).
+ *
+ * <p><b>Configuration Resolution:</b>
+ *
+ * <ul>
+ *   <li><b>Default behavior:</b> The client starts up with the built-in default configuration
+ *       ({@link #loadDefault()}) and then overwrites it with the configuration fetched from the
+ *       server.
+ *   <li><b>System property override ({@link #OVERRIDE_SYS_PROP_KEY}):</b> If set, the server
+ *       configuration is completely ignored (server polling is skipped) and the parsed override
+ *       textproto is used instead.
+ *   <li><b>Default-backed override ({@link #OVERRIDE_USE_DEFAULT_SYS_PROP_KEY}):</b> Provided for
+ *       ergonomics (primarily for testing and probing) so callers do not have to specify the full
+ *       {@link ClientConfiguration} in {@link #OVERRIDE_SYS_PROP_KEY}. When {@code true}, the
+ *       override textproto is merged on top of {@link #loadDefault()} so probers and tests only
+ *       need to specify the fields they care about.
+ * </ul>
  */
 public class ClientConfigurationManager implements AutoCloseable {
   private static final Logger logger = Logger.getLogger(ClientConfigurationManager.class.getName());
 
+  /**
+   * System property providing a textproto {@link ClientConfiguration} to use locally. When set, the
+   * server configuration is ignored. Used as-is by default, or merged on top of {@link
+   * #loadDefault()} when {@link #OVERRIDE_USE_DEFAULT_SYS_PROP_KEY} is {@code true}.
+   */
   public static final String OVERRIDE_SYS_PROP_KEY = "bigtable.internal.client-config-override";
+
+  /**
+   * Boolean system property for ergonomics in testing and probing that, when {@code true}, merges
+   * {@link #OVERRIDE_SYS_PROP_KEY} on top of {@link #loadDefault()} so callers only need to specify
+   * the fields they care about rather than the full configuration. Ignored when {@link
+   * #OVERRIDE_SYS_PROP_KEY} is not set.
+   */
+  public static final String OVERRIDE_USE_DEFAULT_SYS_PROP_KEY =
+      "bigtable.internal.client-config-override-use-default";
+
   public static final String DISABLE_DIRECT_ACCESS_FALLBACK_ENV_VAR =
       "CBT_DISABLE_DIRECT_ACCESS_FALLBACK";
   public static final String DISABLE_DIRECT_ACCESS_FALLBACK_SYS_PROP_KEY =
@@ -125,6 +156,7 @@ public void close() {
   private final ChannelProvider channelProvider;
 
   @GuardedBy("this")
+  @Nullable
   private ManagedChannel channel;
 
   private final DebugTagTracer debugTagTracer;
@@ -173,12 +205,18 @@ public ClientConfigurationManager(
       ScheduledExecutorService executor)
       throws IOException {
     this.defaultConfig = loadDefault();
+    boolean overrideUseDefault =
+        Boolean.parseBoolean(sysProps.getProperty(OVERRIDE_USE_DEFAULT_SYS_PROP_KEY));
     this.overrideConfig =
         Optional.ofNullable(sysProps.getProperty(OVERRIDE_SYS_PROP_KEY))
             .map(
                 str -> {
                   try {
-                    return TextFormat.parse(str, ClientConfiguration.class);
+                    ClientConfiguration parsed = TextFormat.parse(str, ClientConfiguration.class);
+                    if (overrideUseDefault) {
+                      return defaultConfig.toBuilder().mergeFrom(parsed).build();
+                    }
+                    return parsed;
                   } catch (Exception e) {
                     throw new RuntimeException(
                         "Failed to parse bigtable.internal.client-config-override", e);
@@ -200,7 +238,7 @@ public ClientConfigurationManager(
                       .printToString(overrideConfig.get()));
     }
     featureFlags = channelProvider.updateFeatureFlags(featureFlags);
-    this.clientConfiguration = normalizeConfig(defaultConfig);
+    this.clientConfiguration = normalizeConfig(overrideConfig.orElse(defaultConfig));
 
     this.metadata =
         Util.composeMetadata(
@@ -215,7 +253,8 @@ public ClientConfigurationManager(
             .build();
 
     this.channelProvider = channelProvider;
-    this.channel = channelProvider.newChannelBuilder().build();
+    this.channel =
+        overrideConfig.isPresent() ? null : channelProvider.newChannelBuilder().build();
     this.debugTagTracer = tracer;
     this.executor = executor;
   }
@@ -239,6 +278,11 @@ public static ClientConfiguration loadDefault() throws IOException {
    * back to unary if it fails.
    */
   public CompletableFuture<ClientConfiguration> start() {
+    if (overrideConfig.isPresent()) {
+      ClientConfiguration cfg = getClientConfiguration();
+      logger.log(Level.CONFIG, "Using local override config: {0}", cfg);
+      return CompletableFuture.completedFuture(cfg);
+    }
     CompletableFuture<ClientConfiguration> f = startPolling();
     @SuppressWarnings("UnusedVariable")
     CompletableFuture<ClientConfiguration> ignored =
@@ -261,7 +305,9 @@ public synchronized void close() {
     if (nextPoll != null) {
       nextPoll.cancel(true);
     }
-    channel.shutdown();
+    if (channel != null) {
+      channel.shutdown();
+    }
   }
 
   /** Gets the client configuration. */
@@ -398,9 +444,6 @@ private ClientConfiguration normalizeConfig(ClientConfiguration cfg) {
             .setDebugTagLevel(defaultConfig.getTelemetryConfiguration().getDebugTagLevel());
     }
 
-    // Inject overrides
-    overrideConfig.ifPresent(builder::mergeFrom);
-
     if (disableDirectPathFallback
         && builder
             .getSessionConfiguration()
@@ -413,9 +456,7 @@ private ClientConfiguration normalizeConfig(ClientConfiguration cfg) {
           .setDirectAccessOnly(DirectAccessOnly.getDefaultInstance());
     }
 
-    // When sessions are disabled make sure to clear out the config. Read from the builder, not
-    // cfg, so that a diversion config supplied via the override sys-prop is honoured even when
-    // the server-returned config disables sessions.
+    // When sessions are disabled make sure to clear out the config.
     if (!divertsAnyTrafficToSessions(builder.getSessionConfiguration())) {
       builder.clearSessionConfiguration();
       return builder.build();