Skip to content

Commit cec010a

Browse files
mutianfmutianf
authored
fix: move resource detection to the first export to avoid slow start (#2450)
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: - [ ] Make sure to open an issue as a [bug/issue](https://togithub.com/googleapis/java-bigtable/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [ ] Ensure the tests and linter pass - [ ] Code coverage does not decrease (if any source code was changed) - [ ] Appropriate docs were updated (if necessary) - [ ] Rollback plan is reviewed and LGTMed - [ ] All new data plane features have a completed end to end testing plan Fixes #<issue_number_goes_here> ☕️ If you write sample code, please follow the [samples format]( https://togithub.com/GoogleCloudPlatform/java-docs-samples/blob/main/SAMPLE_FORMAT.md).
1 parent bb96c3e commit cec010a

File tree

3 files changed

+33
-27
lines changed

3 files changed

+33
-27
lines changed

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

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@
4040
import com.google.cloud.monitoring.v3.MetricServiceClient;
4141
import com.google.cloud.monitoring.v3.MetricServiceSettings;
4242
import com.google.common.annotations.VisibleForTesting;
43+
import com.google.common.base.Supplier;
44+
import com.google.common.base.Suppliers;
4345
import com.google.common.collect.ImmutableList;
4446
import com.google.common.collect.ImmutableSet;
4547
import com.google.common.collect.Iterables;
@@ -96,8 +98,9 @@ public final class BigtableCloudMonitoringExporter implements MetricExporter {
9698

9799
private final String taskId;
98100

99-
// The resource the client application is running on
100-
private final MonitoredResource applicationResource;
101+
// Application resource is initialized on the first export, which runs on a background thread
102+
// to avoid slowness when starting the client.
103+
private final Supplier<MonitoredResource> applicationResource;
101104

102105
private final AtomicBoolean isShutdown = new AtomicBoolean(false);
103106

@@ -148,28 +151,15 @@ public static BigtableCloudMonitoringExporter create(
148151
// it as not retried for now.
149152
settingsBuilder.createServiceTimeSeriesSettings().setSimpleTimeoutNoRetriesDuration(timeout);
150153

151-
// Detect the resource that the client application is running on. For example,
152-
// this could be a GCE instance or a GKE pod. Currently, we only support GCE instance and
153-
// GKE pod. This method will return null for everything else.
154-
MonitoredResource applicationResource = null;
155-
try {
156-
applicationResource = BigtableExporterUtils.detectResource();
157-
} catch (Exception e) {
158-
logger.log(
159-
Level.WARNING,
160-
"Failed to detect resource, will skip exporting application level metrics ",
161-
e);
162-
}
163-
164154
return new BigtableCloudMonitoringExporter(
165155
MetricServiceClient.create(settingsBuilder.build()),
166-
applicationResource,
156+
Suppliers.memoize(BigtableExporterUtils::detectResourceSafe),
167157
BigtableExporterUtils.getDefaultTaskValue());
168158
}
169159

170160
@VisibleForTesting
171161
BigtableCloudMonitoringExporter(
172-
MetricServiceClient client, @Nullable MonitoredResource applicationResource, String taskId) {
162+
MetricServiceClient client, Supplier<MonitoredResource> applicationResource, String taskId) {
173163
this.client = client;
174164
this.taskId = taskId;
175165
this.applicationResource = applicationResource;
@@ -257,7 +247,7 @@ public void onSuccess(List<Empty> emptyList) {
257247
/** Export metrics associated with the resource the Application is running on. */
258248
private CompletableResultCode exportApplicationResourceMetrics(
259249
Collection<MetricData> collection) {
260-
if (applicationResource == null) {
250+
if (applicationResource.get() == null) {
261251
return CompletableResultCode.ofSuccess();
262252
}
263253

@@ -276,7 +266,7 @@ private CompletableResultCode exportApplicationResourceMetrics(
276266
try {
277267
timeSeries =
278268
BigtableExporterUtils.convertToApplicationResourceTimeSeries(
279-
metricData, taskId, applicationResource);
269+
metricData, taskId, applicationResource.get());
280270
} catch (Throwable e) {
281271
logger.log(
282272
Level.WARNING,
@@ -291,7 +281,8 @@ private CompletableResultCode exportApplicationResourceMetrics(
291281
CompletableResultCode exportCode = new CompletableResultCode();
292282
try {
293283
ProjectName projectName =
294-
ProjectName.of(applicationResource.getLabelsOrThrow(APPLICATION_RESOURCE_PROJECT_ID));
284+
ProjectName.of(
285+
applicationResource.get().getLabelsOrThrow(APPLICATION_RESOURCE_PROJECT_ID));
295286

296287
gceOrGkeFuture = exportTimeSeries(projectName, timeSeries);
297288

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,20 @@ static List<TimeSeries> convertToApplicationResourceTimeSeries(
156156
}
157157

158158
@Nullable
159-
static MonitoredResource detectResource() {
159+
static MonitoredResource detectResourceSafe() {
160+
try {
161+
return detectResource();
162+
} catch (Exception e) {
163+
logger.log(
164+
Level.WARNING,
165+
"Failed to detect resource, will skip exporting application level metrics ",
166+
e);
167+
return null;
168+
}
169+
}
170+
171+
@Nullable
172+
private static MonitoredResource detectResource() {
160173
GCPPlatformDetector detector = GCPPlatformDetector.DEFAULT_INSTANCE;
161174
DetectedPlatform detectedPlatform = detector.detectPlatform();
162175
MonitoredResource monitoredResource = null;

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporterTest.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import com.google.api.gax.rpc.UnaryCallable;
3838
import com.google.cloud.monitoring.v3.MetricServiceClient;
3939
import com.google.cloud.monitoring.v3.stub.MetricServiceStub;
40+
import com.google.common.base.Suppliers;
4041
import com.google.common.collect.ImmutableList;
4142
import com.google.common.collect.ImmutableMap;
4243
import com.google.monitoring.v3.CreateTimeSeriesRequest;
@@ -95,7 +96,7 @@ public void setUp() {
9596

9697
exporter =
9798
new BigtableCloudMonitoringExporter(
98-
fakeMetricServiceClient, /* applicationResource= */ null, taskId);
99+
fakeMetricServiceClient, /* applicationResource= */ Suppliers.ofInstance(null), taskId);
99100

100101
attributes =
101102
Attributes.builder()
@@ -308,11 +309,12 @@ public void testTimeSeriesForMetricWithGceOrGkeResource() {
308309
BigtableCloudMonitoringExporter exporter =
309310
new BigtableCloudMonitoringExporter(
310311
fakeMetricServiceClient,
311-
MonitoredResource.newBuilder()
312-
.setType("gce-instance")
313-
.putLabels("some-gce-key", "some-gce-value")
314-
.putLabels("project_id", gceProjectId)
315-
.build(),
312+
Suppliers.ofInstance(
313+
MonitoredResource.newBuilder()
314+
.setType("gce-instance")
315+
.putLabels("some-gce-key", "some-gce-value")
316+
.putLabels("project_id", gceProjectId)
317+
.build()),
316318
taskId);
317319
ArgumentCaptor<CreateTimeSeriesRequest> argumentCaptor =
318320
ArgumentCaptor.forClass(CreateTimeSeriesRequest.class);

0 commit comments

Comments
 (0)