Skip to content
This repository was archived by the owner on May 8, 2026. It is now read-only.

Commit de4ff3b

Browse files
committed
address comments
1 parent d057536 commit de4ff3b

4 files changed

Lines changed: 80 additions & 32 deletions

File tree

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableClientContext.java

Lines changed: 24 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,8 @@
1919
import com.google.api.core.InternalApi;
2020
import com.google.api.gax.core.BackgroundResource;
2121
import com.google.api.gax.core.CredentialsProvider;
22-
import com.google.api.gax.core.ExecutorAsBackgroundResource;
22+
import com.google.api.gax.core.ExecutorProvider;
2323
import com.google.api.gax.core.FixedCredentialsProvider;
24-
import com.google.api.gax.core.FixedExecutorProvider;
2524
import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider;
2625
import com.google.api.gax.rpc.ClientContext;
2726
import com.google.auth.Credentials;
@@ -36,7 +35,6 @@
3635
import com.google.cloud.bigtable.data.v2.stub.metrics.NoopMetricsProvider;
3736
import com.google.cloud.bigtable.gaxx.grpc.BigtableTransportChannelProvider;
3837
import com.google.cloud.bigtable.gaxx.grpc.ChannelPrimer;
39-
import com.google.common.collect.ImmutableList;
4038
import io.grpc.ManagedChannelBuilder;
4139
import io.grpc.opentelemetry.GrpcOpenTelemetry;
4240
import io.opentelemetry.api.OpenTelemetry;
@@ -62,6 +60,9 @@ public class BigtableClientContext {
6260
@Nullable private final OpenTelemetrySdk internalOpenTelemetry;
6361
private final MetricsProvider metricsProvider;
6462
private final ClientContext clientContext;
63+
// the background executor shared for OTEL instances and monitoring client and all other
64+
// background tasks
65+
private final ExecutorProvider backgroundExecutorProvider;
6566

6667
public static BigtableClientContext create(EnhancedBigtableStubSettings settings)
6768
throws IOException {
@@ -79,16 +80,12 @@ public static BigtableClientContext create(EnhancedBigtableStubSettings settings
7980

8081
String universeDomain = settings.getUniverseDomain();
8182

82-
boolean canAutoCloseExecutor = true;
83+
boolean shouldAutoClose = settings.getBackgroundExecutorProvider().shouldAutoClose();
8384
ScheduledExecutorService backgroundExecutor =
8485
settings.getBackgroundExecutorProvider().getExecutor();
85-
if (settings.getBackgroundExecutorProvider() instanceof FixedExecutorProvider) {
86-
// if the background executor in the settings is already a FixedExecutorProvider,
87-
// we can't assume that we can autoclose it and the life cycle should be managed
88-
// by the application
89-
canAutoCloseExecutor = false;
90-
}
91-
FixedExecutorProvider executorProvider = FixedExecutorProvider.create(backgroundExecutor);
86+
// TODO: after gax change is merged, migrate to use gax's FixedExecutorProvider
87+
BigtableExecutorProvider executorProvider =
88+
BigtableExecutorProvider.create(backgroundExecutor, shouldAutoClose);
9289
builder.setBackgroundExecutorProvider(executorProvider);
9390

9491
// Set up OpenTelemetry
@@ -162,23 +159,16 @@ public static BigtableClientContext create(EnhancedBigtableStubSettings settings
162159
}
163160

164161
ClientContext clientContext = ClientContext.create(builder.build());
165-
if (canAutoCloseExecutor) {
166-
// Since we converted background executor to a FixedExecutorProvider, we need
167-
// to add it back to the background resources, so it will be closed when we close the
168-
// client context.
169-
ImmutableList<BackgroundResource> backgroundResources =
170-
ImmutableList.<BackgroundResource>builder()
171-
.addAll(clientContext.getBackgroundResources())
172-
.add(new ExecutorAsBackgroundResource(backgroundExecutor))
173-
.build();
174-
clientContext = clientContext.toBuilder().setBackgroundResources(backgroundResources).build();
175-
}
176162
if (channelPoolMetricsTracer != null) {
177163
channelPoolMetricsTracer.start(clientContext.getExecutor());
178164
}
179165

180166
return new BigtableClientContext(
181-
clientContext, openTelemetry, internalOtel, settings.getMetricsProvider());
167+
clientContext,
168+
openTelemetry,
169+
internalOtel,
170+
settings.getMetricsProvider(),
171+
executorProvider);
182172
}
183173

184174
private static void configureGrpcOtel(
@@ -212,11 +202,13 @@ private BigtableClientContext(
212202
ClientContext clientContext,
213203
@Nullable OpenTelemetry openTelemetry,
214204
@Nullable OpenTelemetrySdk internalOtel,
215-
MetricsProvider metricsProvider) {
205+
MetricsProvider metricsProvider,
206+
ExecutorProvider backgroundExecutorProvider) {
216207
this.clientContext = clientContext;
217208
this.openTelemetry = openTelemetry;
218209
this.internalOpenTelemetry = internalOtel;
219210
this.metricsProvider = metricsProvider;
211+
this.backgroundExecutorProvider = backgroundExecutorProvider;
220212
}
221213

222214
public OpenTelemetry getOpenTelemetry() {
@@ -229,7 +221,11 @@ public ClientContext getClientContext() {
229221

230222
public BigtableClientContext withClientContext(ClientContext clientContext) {
231223
return new BigtableClientContext(
232-
clientContext, openTelemetry, internalOpenTelemetry, metricsProvider);
224+
clientContext,
225+
openTelemetry,
226+
internalOpenTelemetry,
227+
metricsProvider,
228+
backgroundExecutorProvider);
233229
}
234230

235231
public void close() throws Exception {
@@ -242,6 +238,9 @@ public void close() throws Exception {
242238
if (metricsProvider instanceof DefaultMetricsProvider && openTelemetry != null) {
243239
((OpenTelemetrySdk) openTelemetry).close();
244240
}
241+
if (backgroundExecutorProvider.shouldAutoClose()) {
242+
backgroundExecutorProvider.getExecutor().shutdownNow();
243+
}
245244
}
246245

247246
private static OpenTelemetry getOpenTelemetryFromMetricsProvider(
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/*
2+
* Copyright 2025 Google LLC
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.google.cloud.bigtable.data.v2.stub;
17+
18+
import com.google.api.gax.core.ExecutorProvider;
19+
import java.util.concurrent.ScheduledExecutorService;
20+
21+
// TODO: migrate to gax's FixedExecutorProvider once the change is merged
22+
class BigtableExecutorProvider implements ExecutorProvider {
23+
24+
private final ScheduledExecutorService executorService;
25+
private final boolean shouldAutoClose;
26+
27+
@Override
28+
public boolean shouldAutoClose() {
29+
return shouldAutoClose;
30+
}
31+
32+
@Override
33+
public ScheduledExecutorService getExecutor() {
34+
return executorService;
35+
}
36+
37+
static BigtableExecutorProvider create(
38+
ScheduledExecutorService executor, boolean shouldAutoClose) {
39+
return new BigtableExecutorProvider(executor, shouldAutoClose);
40+
}
41+
42+
private BigtableExecutorProvider(
43+
ScheduledExecutorService executorService, boolean shouldAutoClose) {
44+
this.shouldAutoClose = shouldAutoClose;
45+
this.executorService = executorService;
46+
}
47+
}

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,9 @@ static BigtableCloudMonitoringExporter create(
132132

133133
// If background executor is not null, use it for the monitoring client. This allows us to
134134
// share the same background executor with the data client. When it's null, the monitoring
135-
// client will create a new executor service from InstantiatingExecutorProvider.
135+
// client will create a new executor service from InstantiatingExecutorProvider. It could be
136+
// null if someone uses a CustomOpenTelemetryMetricsProvider#setupSdkMeterProvider without
137+
// the executor.
136138
if (executorService != null) {
137139
settingsBuilder.setBackgroundExecutorProvider(FixedExecutorProvider.create(executorService));
138140
}

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CustomOpenTelemetryMetricsProvider.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,31 +71,31 @@ public OpenTelemetry getOpenTelemetry() {
7171
* Convenient method to set up SdkMeterProviderBuilder with the default credential and endpoint.
7272
*/
7373
public static void setupSdkMeterProvider(SdkMeterProviderBuilder builder) throws IOException {
74-
setupSdkMeterProvider(builder, null, null);
74+
setupSdkMeterProvider(builder, null, null, null);
7575
}
7676

7777
/** Convenient method to set up SdkMeterProviderBuilder with a custom credential. */
7878
public static void setupSdkMeterProvider(SdkMeterProviderBuilder builder, Credentials credentials)
7979
throws IOException {
80-
setupSdkMeterProvider(builder, credentials, null);
80+
setupSdkMeterProvider(builder, credentials, null, null);
8181
}
8282

8383
/** Convenient method to set up SdkMeterProviderBuilder with a custom endpoint. */
8484
public static void setupSdkMeterProvider(SdkMeterProviderBuilder builder, String endpoint)
8585
throws IOException {
86-
setupSdkMeterProvider(builder, null, endpoint);
86+
setupSdkMeterProvider(builder, null, endpoint, null);
8787
}
8888

8989
/** Convenient method to set up SdkMeterProviderBuilder with custom credentials and endpoint. */
9090
public static void setupSdkMeterProvider(
9191
SdkMeterProviderBuilder builder, Credentials credentials, String endpoint)
9292
throws IOException {
93-
BuiltinMetricsView.registerBuiltinMetrics(credentials, builder, endpoint);
93+
setupSdkMeterProvider(builder, credentials, endpoint, null);
9494
}
9595

9696
/**
97-
* Convenient method to set up SdkMeterProviderBuilder with custom credentials, endpoint and
98-
* executor service.
97+
* Convenient method to set up SdkMeterProviderBuilder with custom credentials, endpoint and a
98+
* shared executor service.
9999
*/
100100
public static void setupSdkMeterProvider(
101101
SdkMeterProviderBuilder builder,

0 commit comments

Comments
 (0)