Skip to content

Commit 6b48606

Browse files
mutianfmutianf
authored
chore: refactor settings (#2040)
* chore: refactor settings * fix format * create a shared context * remove patchSettings * reformat * update
1 parent dad7517 commit 6b48606

File tree

9 files changed

+203
-129
lines changed

9 files changed

+203
-129
lines changed

‎google-cloud-bigtable/clirr-ignored-differences.xml

+6
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@
3434
<className>com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub</className>
3535
<method>*</method>
3636
</difference>
37+
<difference>
38+
<!-- method name change is ok because EnhancedBigtableStub is InternalApi -->
39+
<differenceType>7002</differenceType>
40+
<className>com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub</className>
41+
<method>*</method>
42+
</difference>
3743
<!-- InternalApi that was renamed -->
3844
<difference>
3945
<differenceType>8001</differenceType>

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataClient.java

+13
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.google.api.gax.batching.Batcher;
2626
import com.google.api.gax.grpc.GrpcCallContext;
2727
import com.google.api.gax.rpc.ApiExceptions;
28+
import com.google.api.gax.rpc.ClientContext;
2829
import com.google.api.gax.rpc.ResponseObserver;
2930
import com.google.api.gax.rpc.ServerStream;
3031
import com.google.api.gax.rpc.ServerStreamingCallable;
@@ -166,6 +167,18 @@ public static BigtableDataClient create(BigtableDataSettings settings) throws IO
166167
return new BigtableDataClient(stub);
167168
}
168169

170+
/**
171+
* Constructs an instance of BigtableDataClient with the provided client context. This is used by
172+
* {@link BigtableDataClientFactory} and the client context will not be closed unless {@link
173+
* BigtableDataClientFactory#close()} is called.
174+
*/
175+
static BigtableDataClient createWithClientContext(
176+
BigtableDataSettings settings, ClientContext context) throws IOException {
177+
EnhancedBigtableStub stub =
178+
EnhancedBigtableStub.createWithClientContext(settings.getStubSettings(), context);
179+
return new BigtableDataClient(stub);
180+
}
181+
169182
@InternalApi("Visible for testing")
170183
BigtableDataClient(EnhancedBigtableStub stub) {
171184
this.stub = stub;

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactory.java

+41-44
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,8 @@
1717

1818
import com.google.api.core.BetaApi;
1919
import com.google.api.gax.core.BackgroundResource;
20-
import com.google.api.gax.core.FixedCredentialsProvider;
21-
import com.google.api.gax.core.FixedExecutorProvider;
2220
import com.google.api.gax.rpc.ClientContext;
23-
import com.google.api.gax.rpc.FixedHeaderProvider;
24-
import com.google.api.gax.rpc.FixedTransportChannelProvider;
25-
import com.google.api.gax.rpc.FixedWatchdogProvider;
26-
import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStubSettings;
21+
import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStub;
2722
import java.io.IOException;
2823
import javax.annotation.Nonnull;
2924

@@ -78,7 +73,8 @@ public final class BigtableDataClientFactory implements AutoCloseable {
7873
*/
7974
public static BigtableDataClientFactory create(BigtableDataSettings defaultSettings)
8075
throws IOException {
81-
ClientContext sharedClientContext = ClientContext.create(defaultSettings.getStubSettings());
76+
ClientContext sharedClientContext =
77+
EnhancedBigtableStub.createClientContext(defaultSettings.getStubSettings());
8278
return new BigtableDataClientFactory(sharedClientContext, defaultSettings);
8379
}
8480

@@ -110,12 +106,16 @@ public void close() throws Exception {
110106
* release all resources, first close all of the created clients and then this factory instance.
111107
*/
112108
public BigtableDataClient createDefault() {
113-
BigtableDataSettings.Builder settingsBuilder = defaultSettings.toBuilder();
114-
patchStubSettings(settingsBuilder.stubSettings());
115-
BigtableDataSettings settings = settingsBuilder.build();
116-
117109
try {
118-
return BigtableDataClient.create(settings);
110+
ClientContext clientContext =
111+
sharedClientContext
112+
.toBuilder()
113+
.setTracerFactory(
114+
EnhancedBigtableStub.createBigtableTracerFactory(
115+
defaultSettings.getStubSettings()))
116+
.build();
117+
118+
return BigtableDataClient.createWithClientContext(defaultSettings, clientContext);
119119
} catch (IOException e) {
120120
// Should never happen because the connection has been established already
121121
throw new RuntimeException(
@@ -133,12 +133,16 @@ public BigtableDataClient createDefault() {
133133
* release all resources, first close all of the created clients and then this factory instance.
134134
*/
135135
public BigtableDataClient createForAppProfile(@Nonnull String appProfileId) throws IOException {
136-
BigtableDataSettings.Builder settingsBuilder =
137-
defaultSettings.toBuilder().setAppProfileId(appProfileId);
138-
139-
patchStubSettings(settingsBuilder.stubSettings());
136+
BigtableDataSettings settings =
137+
defaultSettings.toBuilder().setAppProfileId(appProfileId).build();
140138

141-
return BigtableDataClient.create(settingsBuilder.build());
139+
ClientContext clientContext =
140+
sharedClientContext
141+
.toBuilder()
142+
.setTracerFactory(
143+
EnhancedBigtableStub.createBigtableTracerFactory(settings.getStubSettings()))
144+
.build();
145+
return BigtableDataClient.createWithClientContext(settings, clientContext);
142146
}
143147

144148
/**
@@ -152,16 +156,22 @@ public BigtableDataClient createForAppProfile(@Nonnull String appProfileId) thro
152156
*/
153157
public BigtableDataClient createForInstance(@Nonnull String projectId, @Nonnull String instanceId)
154158
throws IOException {
155-
BigtableDataSettings.Builder settingsBuilder =
159+
BigtableDataSettings settings =
156160
defaultSettings
157161
.toBuilder()
158162
.setProjectId(projectId)
159163
.setInstanceId(instanceId)
160-
.setDefaultAppProfileId();
164+
.setDefaultAppProfileId()
165+
.build();
161166

162-
patchStubSettings(settingsBuilder.stubSettings());
167+
ClientContext clientContext =
168+
sharedClientContext
169+
.toBuilder()
170+
.setTracerFactory(
171+
EnhancedBigtableStub.createBigtableTracerFactory(settings.getStubSettings()))
172+
.build();
163173

164-
return BigtableDataClient.create(settingsBuilder.build());
174+
return BigtableDataClient.createWithClientContext(settings, clientContext);
165175
}
166176

167177
/**
@@ -176,32 +186,19 @@ public BigtableDataClient createForInstance(@Nonnull String projectId, @Nonnull
176186
public BigtableDataClient createForInstance(
177187
@Nonnull String projectId, @Nonnull String instanceId, @Nonnull String appProfileId)
178188
throws IOException {
179-
BigtableDataSettings.Builder settingsBuilder =
189+
BigtableDataSettings settings =
180190
defaultSettings
181191
.toBuilder()
182192
.setProjectId(projectId)
183193
.setInstanceId(instanceId)
184-
.setAppProfileId(appProfileId);
185-
186-
patchStubSettings(settingsBuilder.stubSettings());
187-
188-
return BigtableDataClient.create(settingsBuilder.build());
189-
}
190-
191-
// Update stub settings to use shared resources in this factory
192-
private void patchStubSettings(EnhancedBigtableStubSettings.Builder stubSettings) {
193-
stubSettings
194-
// Channel refreshing will be configured in the shared ClientContext. Derivative clients
195-
// won't be able to reconfigure the refreshing logic
196-
.setRefreshingChannel(false)
197-
.setTransportChannelProvider(
198-
FixedTransportChannelProvider.create(sharedClientContext.getTransportChannel()))
199-
.setCredentialsProvider(
200-
FixedCredentialsProvider.create(sharedClientContext.getCredentials()))
201-
.setExecutorProvider(FixedExecutorProvider.create(sharedClientContext.getExecutor()))
202-
.setStreamWatchdogProvider(
203-
FixedWatchdogProvider.create(sharedClientContext.getStreamWatchdog()))
204-
.setHeaderProvider(FixedHeaderProvider.create(sharedClientContext.getHeaders()))
205-
.setClock(sharedClientContext.getClock());
194+
.setAppProfileId(appProfileId)
195+
.build();
196+
ClientContext clientContext =
197+
sharedClientContext
198+
.toBuilder()
199+
.setTracerFactory(
200+
EnhancedBigtableStub.createBigtableTracerFactory(settings.getStubSettings()))
201+
.build();
202+
return BigtableDataClient.createWithClientContext(settings, clientContext);
206203
}
207204
}

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

+71-46
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import com.google.api.gax.rpc.ServerStreamingCallable;
4141
import com.google.api.gax.rpc.UnaryCallSettings;
4242
import com.google.api.gax.rpc.UnaryCallable;
43+
import com.google.api.gax.tracing.ApiTracerFactory;
4344
import com.google.api.gax.tracing.OpencensusTracerFactory;
4445
import com.google.api.gax.tracing.SpanName;
4546
import com.google.api.gax.tracing.TracedServerStreamingCallable;
@@ -110,6 +111,7 @@
110111
import com.google.cloud.bigtable.data.v2.stub.readrows.RowMergingCallable;
111112
import com.google.cloud.bigtable.gaxx.retrying.ApiResultRetryAlgorithm;
112113
import com.google.cloud.bigtable.gaxx.retrying.RetryInfoRetryAlgorithm;
114+
import com.google.common.annotations.VisibleForTesting;
113115
import com.google.common.base.MoreObjects;
114116
import com.google.common.base.Preconditions;
115117
import com.google.common.collect.ImmutableList;
@@ -150,6 +152,8 @@ public class EnhancedBigtableStub implements AutoCloseable {
150152

151153
private final EnhancedBigtableStubSettings settings;
152154
private final ClientContext clientContext;
155+
156+
private final boolean closeClientContext;
153157
private final RequestContext requestContext;
154158
private final FlowController bulkMutationFlowController;
155159
private final DynamicFlowControlStats bulkMutationDynamicFlowControlStats;
@@ -172,13 +176,20 @@ public class EnhancedBigtableStub implements AutoCloseable {
172176

173177
public static EnhancedBigtableStub create(EnhancedBigtableStubSettings settings)
174178
throws IOException {
175-
settings = finalizeSettings(settings, Tags.getTagger(), Stats.getStatsRecorder());
176179

177-
return new EnhancedBigtableStub(settings, ClientContext.create(settings));
180+
settings = settings.toBuilder().setTracerFactory(createBigtableTracerFactory(settings)).build();
181+
ClientContext clientContext = createClientContext(settings);
182+
183+
return new EnhancedBigtableStub(settings, clientContext);
178184
}
179185

180-
public static EnhancedBigtableStubSettings finalizeSettings(
181-
EnhancedBigtableStubSettings settings, Tagger tagger, StatsRecorder stats)
186+
public static EnhancedBigtableStub createWithClientContext(
187+
EnhancedBigtableStubSettings settings, ClientContext clientContext) throws IOException {
188+
189+
return new EnhancedBigtableStub(settings, clientContext, false);
190+
}
191+
192+
public static ClientContext createClientContext(EnhancedBigtableStubSettings settings)
182193
throws IOException {
183194
EnhancedBigtableStubSettings.Builder builder = settings.toBuilder();
184195

@@ -222,49 +233,53 @@ public static EnhancedBigtableStubSettings finalizeSettings(
222233
builder.setTransportChannelProvider(transportProvider.build());
223234
}
224235

236+
return ClientContext.create(builder.build());
237+
}
238+
239+
public static ApiTracerFactory createBigtableTracerFactory(
240+
EnhancedBigtableStubSettings settings) {
241+
return createBigtableTracerFactory(settings, Tags.getTagger(), Stats.getStatsRecorder());
242+
}
243+
244+
@VisibleForTesting
245+
public static ApiTracerFactory createBigtableTracerFactory(
246+
EnhancedBigtableStubSettings settings, Tagger tagger, StatsRecorder stats) {
247+
String projectId = settings.getProjectId();
248+
String instanceId = settings.getInstanceId();
249+
String appProfileId = settings.getAppProfileId();
250+
225251
ImmutableMap<TagKey, TagValue> attributes =
226252
ImmutableMap.<TagKey, TagValue>builder()
227-
.put(RpcMeasureConstants.BIGTABLE_PROJECT_ID, TagValue.create(settings.getProjectId()))
228-
.put(
229-
RpcMeasureConstants.BIGTABLE_INSTANCE_ID, TagValue.create(settings.getInstanceId()))
230-
.put(
231-
RpcMeasureConstants.BIGTABLE_APP_PROFILE_ID,
232-
TagValue.create(settings.getAppProfileId()))
253+
.put(RpcMeasureConstants.BIGTABLE_PROJECT_ID, TagValue.create(projectId))
254+
.put(RpcMeasureConstants.BIGTABLE_INSTANCE_ID, TagValue.create(instanceId))
255+
.put(RpcMeasureConstants.BIGTABLE_APP_PROFILE_ID, TagValue.create(appProfileId))
233256
.build();
234257
ImmutableMap<String, String> builtinAttributes =
235258
ImmutableMap.<String, String>builder()
236-
.put("project_id", settings.getProjectId())
237-
.put("instance", settings.getInstanceId())
238-
.put("app_profile", settings.getAppProfileId())
259+
.put("project_id", projectId)
260+
.put("instance", instanceId)
261+
.put("app_profile", appProfileId)
239262
.build();
240-
// Inject Opencensus instrumentation
241-
builder.setTracerFactory(
242-
new CompositeTracerFactory(
243-
ImmutableList.of(
244-
// Add OpenCensus Tracing
245-
new OpencensusTracerFactory(
246-
ImmutableMap.<String, String>builder()
247-
// Annotate traces with the same tags as metrics
248-
.put(
249-
RpcMeasureConstants.BIGTABLE_PROJECT_ID.getName(),
250-
settings.getProjectId())
251-
.put(
252-
RpcMeasureConstants.BIGTABLE_INSTANCE_ID.getName(),
253-
settings.getInstanceId())
254-
.put(
255-
RpcMeasureConstants.BIGTABLE_APP_PROFILE_ID.getName(),
256-
settings.getAppProfileId())
257-
// Also annotate traces with library versions
258-
.put("gax", GaxGrpcProperties.getGaxGrpcVersion())
259-
.put("grpc", GaxGrpcProperties.getGrpcVersion())
260-
.put("gapic", Version.VERSION)
261-
.build()),
262-
// Add OpenCensus Metrics
263-
MetricsTracerFactory.create(tagger, stats, attributes),
264-
BuiltinMetricsTracerFactory.create(builtinAttributes),
265-
// Add user configured tracer
266-
settings.getTracerFactory())));
267-
return builder.build();
263+
264+
return new CompositeTracerFactory(
265+
ImmutableList.of(
266+
// Add OpenCensus Tracing
267+
new OpencensusTracerFactory(
268+
ImmutableMap.<String, String>builder()
269+
// Annotate traces with the same tags as metrics
270+
.put(RpcMeasureConstants.BIGTABLE_PROJECT_ID.getName(), projectId)
271+
.put(RpcMeasureConstants.BIGTABLE_INSTANCE_ID.getName(), instanceId)
272+
.put(RpcMeasureConstants.BIGTABLE_APP_PROFILE_ID.getName(), appProfileId)
273+
// Also annotate traces with library versions
274+
.put("gax", GaxGrpcProperties.getGaxGrpcVersion())
275+
.put("grpc", GaxGrpcProperties.getGrpcVersion())
276+
.put("gapic", Version.VERSION)
277+
.build()),
278+
// Add OpenCensus Metrics
279+
MetricsTracerFactory.create(tagger, stats, attributes),
280+
BuiltinMetricsTracerFactory.create(builtinAttributes),
281+
// Add user configured tracer
282+
settings.getTracerFactory()));
268283
}
269284

270285
private static void patchCredentials(EnhancedBigtableStubSettings.Builder settings)
@@ -303,8 +318,16 @@ private static void patchCredentials(EnhancedBigtableStubSettings.Builder settin
303318
}
304319

305320
public EnhancedBigtableStub(EnhancedBigtableStubSettings settings, ClientContext clientContext) {
321+
this(settings, clientContext, true);
322+
}
323+
324+
public EnhancedBigtableStub(
325+
EnhancedBigtableStubSettings settings,
326+
ClientContext clientContext,
327+
boolean closeClientContext) {
306328
this.settings = settings;
307329
this.clientContext = clientContext;
330+
this.closeClientContext = closeClientContext;
308331
this.requestContext =
309332
RequestContext.create(
310333
settings.getProjectId(), settings.getInstanceId(), settings.getAppProfileId());
@@ -1166,11 +1189,13 @@ private SpanName getSpanName(String methodName) {
11661189

11671190
@Override
11681191
public void close() {
1169-
for (BackgroundResource backgroundResource : clientContext.getBackgroundResources()) {
1170-
try {
1171-
backgroundResource.close();
1172-
} catch (Exception e) {
1173-
throw new IllegalStateException("Failed to close resource", e);
1192+
if (closeClientContext) {
1193+
for (BackgroundResource backgroundResource : clientContext.getBackgroundResources()) {
1194+
try {
1195+
backgroundResource.close();
1196+
} catch (Exception e) {
1197+
throw new IllegalStateException("Failed to close resource", e);
1198+
}
11741199
}
11751200
}
11761201
}

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

-22
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import com.google.api.gax.batching.FlowControlSettings;
2323
import com.google.api.gax.batching.FlowController;
2424
import com.google.api.gax.batching.FlowController.LimitExceededBehavior;
25-
import com.google.api.gax.core.FixedCredentialsProvider;
2625
import com.google.api.gax.core.GoogleCredentialsProvider;
2726
import com.google.api.gax.grpc.ChannelPoolSettings;
2827
import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider;
@@ -33,7 +32,6 @@
3332
import com.google.api.gax.rpc.StubSettings;
3433
import com.google.api.gax.rpc.TransportChannelProvider;
3534
import com.google.api.gax.rpc.UnaryCallSettings;
36-
import com.google.auth.Credentials;
3735
import com.google.bigtable.v2.FeatureFlags;
3836
import com.google.bigtable.v2.PingAndWarmRequest;
3937
import com.google.cloud.bigtable.Version;
@@ -1023,26 +1021,6 @@ public UnaryCallSettings.Builder<PingAndWarmRequest, Void> pingAndWarmSettings()
10231021
public EnhancedBigtableStubSettings build() {
10241022
Preconditions.checkState(projectId != null, "Project id must be set");
10251023
Preconditions.checkState(instanceId != null, "Instance id must be set");
1026-
if (isRefreshingChannel) {
1027-
Preconditions.checkArgument(
1028-
getTransportChannelProvider() instanceof InstantiatingGrpcChannelProvider,
1029-
"refreshingChannel only works with InstantiatingGrpcChannelProviders");
1030-
InstantiatingGrpcChannelProvider.Builder channelProviderBuilder =
1031-
((InstantiatingGrpcChannelProvider) getTransportChannelProvider()).toBuilder();
1032-
Credentials credentials = null;
1033-
if (getCredentialsProvider() != null) {
1034-
try {
1035-
credentials = getCredentialsProvider().getCredentials();
1036-
} catch (IOException e) {
1037-
throw new RuntimeException(e);
1038-
}
1039-
}
1040-
// Use shared credentials
1041-
this.setCredentialsProvider(FixedCredentialsProvider.create(credentials));
1042-
channelProviderBuilder.setChannelPrimer(
1043-
BigtableChannelPrimer.create(credentials, projectId, instanceId, appProfileId));
1044-
this.setTransportChannelProvider(channelProviderBuilder.build());
1045-
}
10461024

10471025
if (this.bulkMutateRowsSettings().isServerInitiatedFlowControlEnabled()) {
10481026
// only set mutate rows feature flag when this feature is enabled

0 commit comments

Comments
 (0)