Visitar URL original
fix(datastore): disable built-in OpenTelemetry SDK when metrics expor… · googleapis/google-cloud-java@449fd02 · GitHub
Skip to content

Commit 449fd02

Browse files
lqiu96gemini-code-assist[bot]
lqiu96
and
gemini-code-assist[bot]
committed
fix(datastore): disable built-in OpenTelemetry SDK when metrics export is disabled (#13543)
Fixes: #13469 --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 parent 49cbbbd commit 449fd02

7 files changed

Lines changed: 85 additions & 12 deletions

File tree

‎java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/BuiltInDatastoreMetricsProvider.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
import java.util.logging.Level;
3737
import java.util.logging.Logger;
3838
import javax.annotation.Nonnull;
39-
import javax.annotation.Nullable;
4039

4140
/**
4241
* Provides a built-in {@link OpenTelemetry} instance for Datastore client-side metrics.
@@ -116,10 +115,15 @@ private static String hashClientUId(String uuid) {
116115
* the lifetime of their Datastore client.
117116
*
118117
* @param options Datastore Client Options
119-
* @return a new {@link OpenTelemetry} instance, or {@code null} if it could not be created.
118+
* @return a new {@link OpenTelemetry} instance.
120119
*/
121-
@Nullable
122120
public OpenTelemetry createOpenTelemetry(@Nonnull DatastoreOptions options) {
121+
// If built-in metrics export is disabled, return no-op OpenTelemetry to avoid instantiating
122+
// a real SdkMeterProvider. Otherwise, the SDK-internal PeriodicMetricReader will start
123+
// and attempt to export diagnostic metrics, leading to log spam if the exporter fails.
124+
if (!options.getOpenTelemetryOptions().isExportBuiltinMetricsToGoogleCloudMonitoring()) {
125+
return OpenTelemetry.noop();
126+
}
123127
Credentials credentials =
124128
Preconditions.checkNotNull(
125129
options.getCredentials(), "Credentials cannot be null for built in metrics");

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,10 @@ public CompletableResultCode export(@Nonnull Collection<MetricData> collection)
235235
return CompletableResultCode.ofFailure();
236236
}
237237

238+
if (datastoreTimeSeries.isEmpty()) {
239+
return CompletableResultCode.ofSuccess();
240+
}
241+
238242
ProjectName projectName = ProjectName.of(projectId);
239243

240244
// Perform the actual network call to Cloud Monitoring.

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,12 @@ static List<TimeSeries> convertToDatastoreTimeSeries(
8282
List<MetricData> collection, Map<String, String> clientAttributes) {
8383
List<TimeSeries> allTimeSeries = new ArrayList<>();
8484

85-
// Metrics should already been filtered for Gax and Datastore related ones
85+
// Only convert metrics that belong to Datastore/GAX (starting with METRIC_PREFIX)
86+
// to filter out OpenTelemetry internal diagnostic metrics.
8687
for (MetricData metricData : collection) {
88+
if (!metricData.getName().startsWith(TelemetryConstants.METRIC_PREFIX)) {
89+
continue;
90+
}
8791
// TODO(b/405457573): The monitored resource is currently written to `global` because the
8892
// Firestore namespace in Cloud Monitoring has not been deployed yet. Once the namespace
8993
// is available, database_id and location labels should be added here using

‎java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/DatastoreMetricsRecorder.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ static DatastoreMetricsRecorder getInstance(
103103
// When using a local emulator, there is no need to configure a built-in Otel instance
104104
if (otelOptions.isExportBuiltinMetricsToGoogleCloudMonitoring()) {
105105
try {
106-
if (builtInOtel != null) {
106+
if (builtInOtel != null && builtInOtel != OpenTelemetry.noop()) {
107107
recorders.add(
108108
new OpenTelemetryDatastoreMetricsRecorder(
109109
builtInOtel, TelemetryConstants.METRIC_PREFIX));

‎java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/BuiltInDatastoreMetricsProviderTest.java‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.google.auth.CredentialTypeForMetrics;
2222
import com.google.auth.Credentials;
2323
import com.google.cloud.NoCredentials;
24+
import com.google.cloud.datastore.DatastoreOpenTelemetryOptions;
2425
import com.google.cloud.datastore.DatastoreOptions;
2526
import io.opentelemetry.api.OpenTelemetry;
2627
import io.opentelemetry.sdk.OpenTelemetrySdk;
@@ -54,7 +55,7 @@ public void testCreateOpenTelemetry_returnsNoOp() {
5455
.setCredentials(NoCredentials.getInstance())
5556
.build();
5657
OpenTelemetry otel = BuiltInDatastoreMetricsProvider.INSTANCE.createOpenTelemetry(options);
57-
assertThat(otel).isInstanceOf(OpenTelemetry.noop().getClass());
58+
assertThat(otel).isSameInstanceAs(OpenTelemetry.noop());
5859
}
5960

6061
@Test
@@ -69,6 +70,10 @@ public void testCreateOpenTelemetry_returnsNonNull() {
6970
.setProjectId(PROJECT_ID)
7071
.setDatabaseId("test-db")
7172
.setCredentials(credentials)
73+
.setOpenTelemetryOptions(
74+
DatastoreOpenTelemetryOptions.newBuilder()
75+
.setExportBuiltinMetricsToGoogleCloudMonitoring(true)
76+
.build())
7277
.build();
7378

7479
OpenTelemetry otel = BuiltInDatastoreMetricsProvider.INSTANCE.createOpenTelemetry(options);
@@ -96,13 +101,21 @@ public void testCreateOpenTelemetry_eachCallReturnsDistinctInstance() {
96101
.setProjectId(PROJECT_ID)
97102
.setDatabaseId("test-db")
98103
.setCredentials(credentials1)
104+
.setOpenTelemetryOptions(
105+
DatastoreOpenTelemetryOptions.newBuilder()
106+
.setExportBuiltinMetricsToGoogleCloudMonitoring(true)
107+
.build())
99108
.build();
100109

101110
DatastoreOptions options2 =
102111
DatastoreOptions.newBuilder()
103112
.setProjectId(PROJECT_ID)
104113
.setDatabaseId("test-db")
105114
.setCredentials(credentials2)
115+
.setOpenTelemetryOptions(
116+
DatastoreOpenTelemetryOptions.newBuilder()
117+
.setExportBuiltinMetricsToGoogleCloudMonitoring(true)
118+
.build())
106119
.build();
107120

108121
OpenTelemetry otel1 = BuiltInDatastoreMetricsProvider.INSTANCE.createOpenTelemetry(options1);
@@ -132,9 +145,26 @@ public void testCreateOpenTelemetry_withEmulatorHostProperty_returnsNoOp() {
132145
.setCredentials(NoCredentials.getInstance())
133146
.build();
134147
OpenTelemetry otel = BuiltInDatastoreMetricsProvider.INSTANCE.createOpenTelemetry(options);
135-
assertThat(otel).isInstanceOf(OpenTelemetry.noop().getClass());
148+
assertThat(otel).isSameInstanceAs(OpenTelemetry.noop());
136149
} finally {
137150
System.clearProperty(DatastoreOptions.LOCAL_HOST_ENV_VAR);
138151
}
139152
}
153+
154+
@Test
155+
public void testCreateOpenTelemetry_exportDisabled_returnsNoOp() {
156+
Credentials credentials = EasyMock.mock(Credentials.class);
157+
EasyMock.replay(credentials);
158+
159+
DatastoreOptions options =
160+
DatastoreOptions.newBuilder()
161+
.setProjectId(PROJECT_ID)
162+
.setDatabaseId("test-db")
163+
.setCredentials(credentials)
164+
// export is disabled by default
165+
.build();
166+
167+
OpenTelemetry otel = BuiltInDatastoreMetricsProvider.INSTANCE.createOpenTelemetry(options);
168+
assertThat(otel).isSameInstanceAs(OpenTelemetry.noop());
169+
}
140170
}

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import com.google.monitoring.v3.TimeSeries;
4343
import com.google.protobuf.Empty;
4444
import io.opentelemetry.api.common.Attributes;
45+
import io.opentelemetry.sdk.common.CompletableResultCode;
4546
import io.opentelemetry.sdk.common.InstrumentationScopeInfo;
4647
import io.opentelemetry.sdk.metrics.data.AggregationTemporality;
4748
import io.opentelemetry.sdk.metrics.data.LongPointData;
@@ -190,6 +191,35 @@ public void testClientCacheReferenceCounting() {
190191
verify(mockClient);
191192
}
192193

194+
@Test
195+
public void testExportingIgnoredMetrics() {
196+
// No expectations on mockMetricServiceStub means we expect NO calls to it.
197+
replay(mockMetricServiceStub);
198+
199+
long fakeValue = 11L;
200+
long startEpoch = 10;
201+
long endEpoch = 15;
202+
LongPointData longPointData =
203+
ImmutableLongPointData.create(startEpoch, endEpoch, attributes, fakeValue);
204+
205+
String ignoredMetricName = "otel.sdk.metric_reader.collection.duration";
206+
MetricData ignoredData =
207+
ImmutableMetricData.createLongSum(
208+
resource,
209+
scope,
210+
ignoredMetricName,
211+
"description",
212+
"1",
213+
ImmutableSumData.create(
214+
true, AggregationTemporality.CUMULATIVE, ImmutableList.of(longPointData)));
215+
216+
CompletableResultCode result = exporter.export(Collections.singletonList(ignoredData));
217+
218+
assertThat(result.isSuccess()).isTrue();
219+
220+
verify(mockMetricServiceStub);
221+
}
222+
193223
private static class FakeMetricServiceClient extends MetricServiceClient {
194224
protected FakeMetricServiceClient(MetricServiceStub stub) {
195225
super(stub);

‎java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/DatastoreMetricsRecorderTest.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.google.cloud.datastore.DatastoreOpenTelemetryOptions;
2222
import com.google.cloud.datastore.DatastoreOptions;
2323
import io.opentelemetry.api.OpenTelemetry;
24+
import io.opentelemetry.sdk.OpenTelemetrySdk;
2425
import org.easymock.EasyMock;
2526
import org.junit.Test;
2627
import org.junit.runner.RunWith;
@@ -79,7 +80,7 @@ public void tracingEnabledButMetricsDisabledAndBuiltInDisabled_returnsNoRecorder
7980
}
8081

8182
@Test
82-
public void defaultOptionsWithBuiltInMetricsEnabled_butNoCredentials_returnsOneRecorder() {
83+
public void defaultOptionsWithBuiltInMetricsEnabled_butNoCredentials_returnsNoRecorders() {
8384
// Explicitly enable built-in metrics export
8485
DatastoreOptions options =
8586
baseOptions() // Uses NoCredentials by default
@@ -92,12 +93,12 @@ public void defaultOptionsWithBuiltInMetricsEnabled_butNoCredentials_returnsOneR
9293
DatastoreMetricsRecorder.getInstance(options, OpenTelemetry.noop());
9394

9495
// Since baseOptions() uses NoCredentials, the provider returns OpenTelemetry.noop().
95-
// This NoOp instance is passed to getInstance, which adds it to the recorders list.
96-
// So we have 1 recorder (the NoOp one).
96+
// This NoOp instance is passed to getInstance, which should NOT add it to the recorders list.
97+
// So we have 0 recorders.
9798
assertThat(recorder).isInstanceOf(CompositeDatastoreMetricsRecorder.class);
9899
CompositeDatastoreMetricsRecorder compositeRecorder =
99100
(CompositeDatastoreMetricsRecorder) recorder;
100-
assertThat(compositeRecorder.getMetricRecorders().size()).isEqualTo(1);
101+
assertThat(compositeRecorder.getMetricRecorders().size()).isEqualTo(0);
101102
}
102103

103104
@Test
@@ -142,7 +143,7 @@ public void bothEnabled_returnsTwoRecorders() {
142143
.setOpenTelemetry(OpenTelemetry.noop())
143144
.build())
144145
.build();
145-
OpenTelemetry builtInOtel = OpenTelemetry.noop();
146+
OpenTelemetry builtInOtel = OpenTelemetrySdk.builder().build();
146147

147148
DatastoreMetricsRecorder recorder = DatastoreMetricsRecorder.getInstance(options, builtInOtel);
148149
assertThat(recorder).isInstanceOf(CompositeDatastoreMetricsRecorder.class);

0 commit comments

Comments
 (0)