Skip to content

Commit b37ca37

Browse files
committed
chore: Address PR feedback
1 parent 6e0b04d commit b37ca37

3 files changed

Lines changed: 9 additions & 27 deletions

File tree

java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/DatastoreCloudMonitoringExporter.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,10 @@ static class CachedMetricsClient {
108108
// Flag to prevent log spam of any export failures
109109
private final AtomicBoolean datastoreExportFailureLogged = new AtomicBoolean(false);
110110

111+
// Flag to prevent double shutdown of this exporter instance
112+
private final AtomicBoolean isExporterShutDown = new AtomicBoolean(false);
113+
111114
private final String projectId;
112-
private final String databaseId;
113115

114116
/**
115117
* Creates a new instance of the exporter.
@@ -163,7 +165,7 @@ static DatastoreCloudMonitoringExporter create(
163165
}
164166

165167
return new DatastoreCloudMonitoringExporter(
166-
key, projectId, databaseId, cachedMetricsClient.client, clientAttributes);
168+
key, projectId, cachedMetricsClient.client, clientAttributes);
167169
}
168170

169171
private static MetricServiceClient createMetricServiceClient(Credentials credentials)
@@ -189,13 +191,11 @@ private static MetricServiceClient createMetricServiceClient(Credentials credent
189191
DatastoreCloudMonitoringExporter(
190192
String cacheKey,
191193
String projectId,
192-
String databaseId,
193194
MetricServiceClient client,
194195
Map<String, String> clientAttributes) {
195196
this.cacheKey = cacheKey;
196197
this.client = client;
197198
this.projectId = projectId;
198-
this.databaseId = databaseId;
199199
this.clientAttributes = clientAttributes;
200200
}
201201

@@ -207,7 +207,7 @@ private static MetricServiceClient createMetricServiceClient(Credentials credent
207207
*/
208208
@Override
209209
public CompletableResultCode export(@Nonnull Collection<MetricData> collection) {
210-
if (client.isShutdown()) {
210+
if (isExporterShutDown.get()) {
211211
logger.log(Level.WARNING, "Exporter is shut down");
212212
return CompletableResultCode.ofFailure();
213213
}
@@ -305,7 +305,7 @@ public CompletableResultCode flush() {
305305
/** Shuts down the exporter and the underlying {@link MetricServiceClient}. */
306306
@Override
307307
public CompletableResultCode shutdown() {
308-
if (client.isShutdown()) {
308+
if (!isExporterShutDown.compareAndSet(false, true)) {
309309
logger.log(Level.WARNING, "shutdown is called multiple times");
310310
return CompletableResultCode.ofSuccess();
311311
}

java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/DatastoreCloudMonitoringExporterUtils.java

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import static com.google.api.MetricDescriptor.ValueType.DOUBLE;
2424
import static com.google.api.MetricDescriptor.ValueType.INT64;
2525
import static com.google.cloud.datastore.telemetry.TelemetryConstants.DATASTORE_RESOURCE_TYPE;
26-
import static com.google.cloud.datastore.telemetry.TelemetryConstants.PROJECT_ID_KEY;
2726

2827
import com.google.api.Distribution;
2928
import com.google.api.Distribution.BucketOptions;
@@ -48,7 +47,6 @@
4847
import io.opentelemetry.sdk.metrics.data.MetricDataType;
4948
import io.opentelemetry.sdk.metrics.data.PointData;
5049
import io.opentelemetry.sdk.metrics.data.SumData;
51-
import io.opentelemetry.sdk.resources.Resource;
5250
import java.util.ArrayList;
5351
import java.util.List;
5452
import java.util.Map;
@@ -72,16 +70,6 @@ class DatastoreCloudMonitoringExporterUtils {
7270

7371
private DatastoreCloudMonitoringExporterUtils() {}
7472

75-
/**
76-
* Extracts the project ID from the OpenTelemetry {@link Resource}.
77-
*
78-
* @param resource the OTel resource.
79-
* @return the project ID string, or null if not present.
80-
*/
81-
static String getProjectId(Resource resource) {
82-
return resource.getAttributes().get(PROJECT_ID_KEY);
83-
}
84-
8573
/**
8674
* Converts a list of {@link MetricData} to Cloud Monitoring {@link TimeSeries}.
8775
*

java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/DatastoreCloudMonitoringExporterTest.java

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,7 @@ public void setUp() {
8383

8484
exporter =
8585
new DatastoreCloudMonitoringExporter(
86-
PROJECT_ID + ":" + DATABASE_ID,
87-
PROJECT_ID,
88-
DATABASE_ID,
89-
fakeMetricServiceClient,
90-
clientAttributes);
86+
PROJECT_ID + ":" + DATABASE_ID, PROJECT_ID, fakeMetricServiceClient, clientAttributes);
9187

9288
attributes =
9389
Attributes.builder()
@@ -173,17 +169,15 @@ public void testClientCacheReferenceCounting() {
173169
DatastoreCloudMonitoringExporter.METRICS_CLIENT_CACHE.put(key, cachedMetricsClient);
174170

175171
DatastoreCloudMonitoringExporter exporter1 =
176-
new DatastoreCloudMonitoringExporter(
177-
key, PROJECT_ID, DATABASE_ID, mockClient, Collections.emptyMap());
172+
new DatastoreCloudMonitoringExporter(key, PROJECT_ID, mockClient, Collections.emptyMap());
178173

179174
// First shutdown should decrement refCount to 1, but not close client
180175
exporter1.shutdown();
181176
assertThat(cachedMetricsClient.refCount.get()).isEqualTo(1);
182177
assertThat(DatastoreCloudMonitoringExporter.METRICS_CLIENT_CACHE.containsKey(key)).isTrue();
183178

184179
DatastoreCloudMonitoringExporter exporter2 =
185-
new DatastoreCloudMonitoringExporter(
186-
key, PROJECT_ID, DATABASE_ID, mockClient, Collections.emptyMap());
180+
new DatastoreCloudMonitoringExporter(key, PROJECT_ID, mockClient, Collections.emptyMap());
187181

188182
// Second shutdown should decrement refCount to 0, close client, and remove from cache
189183
exporter2.shutdown();

0 commit comments

Comments
 (0)