From 3ceab08ec1d042ba8c6d59e9bc1a019fbdab2333 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Thu, 12 Dec 2024 15:27:49 -0500 Subject: [PATCH 1/5] fix: move resource detection to the first export to avoid slow start --- .../BigtableCloudMonitoringExporter.java | 41 +++++++++++-------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java index ff5bcd81c1..de4a641813 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java @@ -98,8 +98,9 @@ public final class BigtableCloudMonitoringExporter implements MetricExporter { private final String taskId; // The resource the client application is running on - private final MonitoredResource applicationResource; + private MonitoredResource applicationResource; + private final AtomicBoolean initializedAppResource = new AtomicBoolean(false); private final AtomicBoolean isShutdown = new AtomicBoolean(false); private CompletableResultCode lastExportCode; @@ -149,22 +150,9 @@ public static BigtableCloudMonitoringExporter create( // it as not retried for now. settingsBuilder.createServiceTimeSeriesSettings().setSimpleTimeoutNoRetries(timeout); - // Detect the resource that the client application is running on. For example, - // this could be a GCE instance or a GKE pod. Currently, we only support GCE instance and - // GKE pod. This method will return null for everything else. - MonitoredResource applicationResource = null; - try { - applicationResource = BigtableExporterUtils.detectResource(); - } catch (Exception e) { - logger.log( - Level.WARNING, - "Failed to detect resource, will skip exporting application level metrics ", - e); - } - return new BigtableCloudMonitoringExporter( MetricServiceClient.create(settingsBuilder.build()), - applicationResource, + null, BigtableExporterUtils.getDefaultTaskValue()); } @@ -173,7 +161,11 @@ public static BigtableCloudMonitoringExporter create( MetricServiceClient client, @Nullable MonitoredResource applicationResource, String taskId) { this.client = client; this.taskId = taskId; - this.applicationResource = applicationResource; + if (applicationResource != null) { + // for test to set a fake resource + initializedAppResource.set(true); + this.applicationResource = applicationResource; + } } @Override @@ -258,6 +250,23 @@ public void onSuccess(List emptyList) { /** Export metrics associated with the resource the Application is running on. */ private CompletableResultCode exportApplicationResourceMetrics( Collection collection) { + // Initialize the application resource on the first export which runs on a background thread + // to avoid slowness when starting the client. + if (initializedAppResource.compareAndSet(false, true)) { + // Detect the resource that the client application is running on. For example, + // this could be a GCE instance or a GKE pod. Currently, we only support GCE instance and + // GKE pod. This method will return null for everything else. + applicationResource = null; + try { + applicationResource = BigtableExporterUtils.detectResource(); + } catch (Exception e) { + logger.log( + Level.WARNING, + "Failed to detect resource, will skip exporting application level metrics ", + e); + } + } + if (applicationResource == null) { return CompletableResultCode.ofSuccess(); } From 2daf6980d5432ec2f26044f2ab51ae6571b0ffe5 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Fri, 13 Dec 2024 12:49:39 -0500 Subject: [PATCH 2/5] use supplier --- .../BigtableCloudMonitoringExporter.java | 34 +++++++------------ .../stub/metrics/BigtableExporterUtils.java | 12 +++++++ 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java index de4a641813..6e0684c3dd 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java @@ -40,6 +40,8 @@ import com.google.cloud.monitoring.v3.MetricServiceClient; import com.google.cloud.monitoring.v3.MetricServiceSettings; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; @@ -97,10 +99,10 @@ public final class BigtableCloudMonitoringExporter implements MetricExporter { private final String taskId; - // The resource the client application is running on - private MonitoredResource applicationResource; + // Application resource is initialized on the first export, which runs on a background thread + // to avoid slowness when starting the client. + private Supplier applicationResource; - private final AtomicBoolean initializedAppResource = new AtomicBoolean(false); private final AtomicBoolean isShutdown = new AtomicBoolean(false); private CompletableResultCode lastExportCode; @@ -162,9 +164,7 @@ public static BigtableCloudMonitoringExporter create( this.client = client; this.taskId = taskId; if (applicationResource != null) { - // for test to set a fake resource - initializedAppResource.set(true); - this.applicationResource = applicationResource; + this.applicationResource = Suppliers.ofInstance(applicationResource); } } @@ -250,24 +250,15 @@ public void onSuccess(List emptyList) { /** Export metrics associated with the resource the Application is running on. */ private CompletableResultCode exportApplicationResourceMetrics( Collection collection) { - // Initialize the application resource on the first export which runs on a background thread - // to avoid slowness when starting the client. - if (initializedAppResource.compareAndSet(false, true)) { + // applicationResource will only not be null when this class is initialized by test + if (applicationResource == null) { // Detect the resource that the client application is running on. For example, // this could be a GCE instance or a GKE pod. Currently, we only support GCE instance and // GKE pod. This method will return null for everything else. - applicationResource = null; - try { - applicationResource = BigtableExporterUtils.detectResource(); - } catch (Exception e) { - logger.log( - Level.WARNING, - "Failed to detect resource, will skip exporting application level metrics ", - e); - } + applicationResource = Suppliers.memoize(BigtableExporterUtils::detectResourceSafe); } - if (applicationResource == null) { + if (applicationResource.get() == null) { return CompletableResultCode.ofSuccess(); } @@ -286,7 +277,7 @@ private CompletableResultCode exportApplicationResourceMetrics( try { timeSeries = BigtableExporterUtils.convertToApplicationResourceTimeSeries( - metricData, taskId, applicationResource); + metricData, taskId, applicationResource.get()); } catch (Throwable e) { logger.log( Level.WARNING, @@ -301,7 +292,8 @@ private CompletableResultCode exportApplicationResourceMetrics( CompletableResultCode exportCode = new CompletableResultCode(); try { ProjectName projectName = - ProjectName.of(applicationResource.getLabelsOrThrow(APPLICATION_RESOURCE_PROJECT_ID)); + ProjectName.of( + applicationResource.get().getLabelsOrThrow(APPLICATION_RESOURCE_PROJECT_ID)); gceOrGkeFuture = exportTimeSeries(projectName, timeSeries); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java index 821c2295e0..f803053c15 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java @@ -155,6 +155,18 @@ static List convertToApplicationResourceTimeSeries( return allTimeSeries; } + static MonitoredResource detectResourceSafe() { + try { + return detectResource(); + } catch (Exception e) { + logger.log( + Level.WARNING, + "Failed to detect resource, will skip exporting application level metrics ", + e); + return null; + } + } + @Nullable static MonitoredResource detectResource() { GCPPlatformDetector detector = GCPPlatformDetector.DEFAULT_INSTANCE; From fe25319ea01a5e566d4e182bee3e3cca76e48edd Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Fri, 13 Dec 2024 16:42:07 -0500 Subject: [PATCH 3/5] use supplier correctly --- .../BigtableCloudMonitoringExporter.java | 19 +++++++------------ .../stub/metrics/BigtableExporterUtils.java | 3 ++- .../BigtableCloudMonitoringExporterTest.java | 12 +++++++----- 3 files changed, 16 insertions(+), 18 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java index 6e0684c3dd..6b0af32284 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java @@ -160,12 +160,15 @@ public static BigtableCloudMonitoringExporter create( @VisibleForTesting BigtableCloudMonitoringExporter( - MetricServiceClient client, @Nullable MonitoredResource applicationResource, String taskId) { + MetricServiceClient client, + @Nullable Supplier applicationResource, + String taskId) { this.client = client; this.taskId = taskId; - if (applicationResource != null) { - this.applicationResource = Suppliers.ofInstance(applicationResource); - } + this.applicationResource = + applicationResource != null + ? applicationResource + : Suppliers.memoize(BigtableExporterUtils::detectResourceSafe); } @Override @@ -250,14 +253,6 @@ public void onSuccess(List emptyList) { /** Export metrics associated with the resource the Application is running on. */ private CompletableResultCode exportApplicationResourceMetrics( Collection collection) { - // applicationResource will only not be null when this class is initialized by test - if (applicationResource == null) { - // Detect the resource that the client application is running on. For example, - // this could be a GCE instance or a GKE pod. Currently, we only support GCE instance and - // GKE pod. This method will return null for everything else. - applicationResource = Suppliers.memoize(BigtableExporterUtils::detectResourceSafe); - } - if (applicationResource.get() == null) { return CompletableResultCode.ofSuccess(); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java index f803053c15..95df887f0d 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java @@ -155,6 +155,7 @@ static List convertToApplicationResourceTimeSeries( return allTimeSeries; } + @Nullable static MonitoredResource detectResourceSafe() { try { return detectResource(); @@ -168,7 +169,7 @@ static MonitoredResource detectResourceSafe() { } @Nullable - static MonitoredResource detectResource() { + private static MonitoredResource detectResource() { GCPPlatformDetector detector = GCPPlatformDetector.DEFAULT_INSTANCE; DetectedPlatform detectedPlatform = detector.detectPlatform(); MonitoredResource monitoredResource = null; diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporterTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporterTest.java index 657db7d8ae..d1306dfdef 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporterTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporterTest.java @@ -37,6 +37,7 @@ import com.google.api.gax.rpc.UnaryCallable; import com.google.cloud.monitoring.v3.MetricServiceClient; import com.google.cloud.monitoring.v3.stub.MetricServiceStub; +import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.monitoring.v3.CreateTimeSeriesRequest; @@ -308,11 +309,12 @@ public void testTimeSeriesForMetricWithGceOrGkeResource() { BigtableCloudMonitoringExporter exporter = new BigtableCloudMonitoringExporter( fakeMetricServiceClient, - MonitoredResource.newBuilder() - .setType("gce-instance") - .putLabels("some-gce-key", "some-gce-value") - .putLabels("project_id", gceProjectId) - .build(), + Suppliers.ofInstance( + MonitoredResource.newBuilder() + .setType("gce-instance") + .putLabels("some-gce-key", "some-gce-value") + .putLabels("project_id", gceProjectId) + .build()), taskId); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(CreateTimeSeriesRequest.class); From 3f9f960b2c6095fc6cd6784976fb00b1bbc9da95 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Mon, 16 Dec 2024 11:43:23 -0500 Subject: [PATCH 4/5] remove null check and make it final --- .../metrics/BigtableCloudMonitoringExporter.java | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java index 565018399b..a829c3f719 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java @@ -100,7 +100,7 @@ public final class BigtableCloudMonitoringExporter implements MetricExporter { // Application resource is initialized on the first export, which runs on a background thread // to avoid slowness when starting the client. - private Supplier applicationResource; + private final Supplier applicationResource; private final AtomicBoolean isShutdown = new AtomicBoolean(false); @@ -153,21 +153,16 @@ public static BigtableCloudMonitoringExporter create( return new BigtableCloudMonitoringExporter( MetricServiceClient.create(settingsBuilder.build()), - null, + Suppliers.memoize(BigtableExporterUtils::detectResourceSafe), BigtableExporterUtils.getDefaultTaskValue()); } @VisibleForTesting BigtableCloudMonitoringExporter( - MetricServiceClient client, - @Nullable Supplier applicationResource, - String taskId) { + MetricServiceClient client, Supplier applicationResource, String taskId) { this.client = client; this.taskId = taskId; - this.applicationResource = - applicationResource != null - ? applicationResource - : Suppliers.memoize(BigtableExporterUtils::detectResourceSafe); + this.applicationResource = applicationResource; } @Override From d4d4747f580a39647eb6fc8b0a766986deab82c2 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Mon, 16 Dec 2024 12:13:03 -0500 Subject: [PATCH 5/5] remove null checks --- .../v2/stub/metrics/BigtableCloudMonitoringExporterTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporterTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporterTest.java index d1306dfdef..e471b19a20 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporterTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporterTest.java @@ -96,7 +96,7 @@ public void setUp() { exporter = new BigtableCloudMonitoringExporter( - fakeMetricServiceClient, /* applicationResource= */ null, taskId); + fakeMetricServiceClient, /* applicationResource= */ Suppliers.ofInstance(null), taskId); attributes = Attributes.builder()