From 7301bce343475536904061753b980f0c8d7bafcd Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Tue, 23 Jun 2026 03:20:54 +0000 Subject: [PATCH 1/4] fix(datastore): disable built-in OpenTelemetry SDK when metrics export is disabled --- .../BuiltInDatastoreMetricsProvider.java | 6 ++++ .../BuiltInDatastoreMetricsProviderTest.java | 30 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/BuiltInDatastoreMetricsProvider.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/BuiltInDatastoreMetricsProvider.java index a538e1318135..4ddf8b3242f5 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/BuiltInDatastoreMetricsProvider.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/BuiltInDatastoreMetricsProvider.java @@ -120,6 +120,12 @@ private static String hashClientUId(String uuid) { */ @Nullable public OpenTelemetry createOpenTelemetry(@Nonnull DatastoreOptions options) { + // If built-in metrics export is disabled, return no-op OpenTelemetry to avoid instantiating + // a real SdkMeterProvider. Otherwise, the SDK-internal PeriodicMetricReader will start + // and attempt to export diagnostic metrics, leading to log spam if the exporter fails. + if (!options.getOpenTelemetryOptions().isExportBuiltinMetricsToGoogleCloudMonitoring()) { + return OpenTelemetry.noop(); + } Credentials credentials = Preconditions.checkNotNull( options.getCredentials(), "Credentials cannot be null for built in metrics"); diff --git a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/BuiltInDatastoreMetricsProviderTest.java b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/BuiltInDatastoreMetricsProviderTest.java index 655be0b6f84d..3d914d8813b2 100644 --- a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/BuiltInDatastoreMetricsProviderTest.java +++ b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/BuiltInDatastoreMetricsProviderTest.java @@ -21,6 +21,7 @@ import com.google.auth.CredentialTypeForMetrics; import com.google.auth.Credentials; import com.google.cloud.NoCredentials; +import com.google.cloud.datastore.DatastoreOpenTelemetryOptions; import com.google.cloud.datastore.DatastoreOptions; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.sdk.OpenTelemetrySdk; @@ -69,6 +70,10 @@ public void testCreateOpenTelemetry_returnsNonNull() { .setProjectId(PROJECT_ID) .setDatabaseId("test-db") .setCredentials(credentials) + .setOpenTelemetryOptions( + DatastoreOpenTelemetryOptions.newBuilder() + .setExportBuiltinMetricsToGoogleCloudMonitoring(true) + .build()) .build(); OpenTelemetry otel = BuiltInDatastoreMetricsProvider.INSTANCE.createOpenTelemetry(options); @@ -96,6 +101,10 @@ public void testCreateOpenTelemetry_eachCallReturnsDistinctInstance() { .setProjectId(PROJECT_ID) .setDatabaseId("test-db") .setCredentials(credentials1) + .setOpenTelemetryOptions( + DatastoreOpenTelemetryOptions.newBuilder() + .setExportBuiltinMetricsToGoogleCloudMonitoring(true) + .build()) .build(); DatastoreOptions options2 = @@ -103,6 +112,10 @@ public void testCreateOpenTelemetry_eachCallReturnsDistinctInstance() { .setProjectId(PROJECT_ID) .setDatabaseId("test-db") .setCredentials(credentials2) + .setOpenTelemetryOptions( + DatastoreOpenTelemetryOptions.newBuilder() + .setExportBuiltinMetricsToGoogleCloudMonitoring(true) + .build()) .build(); OpenTelemetry otel1 = BuiltInDatastoreMetricsProvider.INSTANCE.createOpenTelemetry(options1); @@ -137,4 +150,21 @@ public void testCreateOpenTelemetry_withEmulatorHostProperty_returnsNoOp() { System.clearProperty(DatastoreOptions.LOCAL_HOST_ENV_VAR); } } + + @Test + public void testCreateOpenTelemetry_exportDisabled_returnsNoOp() { + Credentials credentials = EasyMock.mock(Credentials.class); + EasyMock.replay(credentials); + + DatastoreOptions options = + DatastoreOptions.newBuilder() + .setProjectId(PROJECT_ID) + .setDatabaseId("test-db") + .setCredentials(credentials) + // export is disabled by default + .build(); + + OpenTelemetry otel = BuiltInDatastoreMetricsProvider.INSTANCE.createOpenTelemetry(options); + assertThat(otel).isInstanceOf(OpenTelemetry.noop().getClass()); + } } From cd365236321d12ac7f7f368a1c7be3b1ef725b85 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Mon, 22 Jun 2026 23:33:53 -0400 Subject: [PATCH 2/4] Update java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/BuiltInDatastoreMetricsProviderTest.java Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- .../telemetry/BuiltInDatastoreMetricsProviderTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/BuiltInDatastoreMetricsProviderTest.java b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/BuiltInDatastoreMetricsProviderTest.java index 3d914d8813b2..dd6d282c1805 100644 --- a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/BuiltInDatastoreMetricsProviderTest.java +++ b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/BuiltInDatastoreMetricsProviderTest.java @@ -165,6 +165,6 @@ public void testCreateOpenTelemetry_exportDisabled_returnsNoOp() { .build(); OpenTelemetry otel = BuiltInDatastoreMetricsProvider.INSTANCE.createOpenTelemetry(options); - assertThat(otel).isInstanceOf(OpenTelemetry.noop().getClass()); + assertThat(otel).isSameInstanceAs(OpenTelemetry.noop()); } } From a71d22a7e27bf15f204bf065707a2334443b007e Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Tue, 23 Jun 2026 14:32:23 +0000 Subject: [PATCH 3/4] fix(datastore): filter out non-datastore metrics in Cloud Monitoring exporter --- .../DatastoreCloudMonitoringExporter.java | 4 +++ ...DatastoreCloudMonitoringExporterUtils.java | 6 +++- .../DatastoreCloudMonitoringExporterTest.java | 30 +++++++++++++++++++ 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/DatastoreCloudMonitoringExporter.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/DatastoreCloudMonitoringExporter.java index c0e34a977b16..06f355f8bb69 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/DatastoreCloudMonitoringExporter.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/DatastoreCloudMonitoringExporter.java @@ -235,6 +235,10 @@ public CompletableResultCode export(@Nonnull Collection collection) return CompletableResultCode.ofFailure(); } + if (datastoreTimeSeries.isEmpty()) { + return CompletableResultCode.ofSuccess(); + } + ProjectName projectName = ProjectName.of(projectId); // Perform the actual network call to Cloud Monitoring. diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/DatastoreCloudMonitoringExporterUtils.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/DatastoreCloudMonitoringExporterUtils.java index 949bdff3c293..ce1dff566631 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/DatastoreCloudMonitoringExporterUtils.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/DatastoreCloudMonitoringExporterUtils.java @@ -82,8 +82,12 @@ static List convertToDatastoreTimeSeries( List collection, Map clientAttributes) { List allTimeSeries = new ArrayList<>(); - // Metrics should already been filtered for Gax and Datastore related ones + // Only convert metrics that belong to Datastore/GAX (starting with METRIC_PREFIX) + // to filter out OpenTelemetry internal diagnostic metrics. for (MetricData metricData : collection) { + if (!metricData.getName().startsWith(TelemetryConstants.METRIC_PREFIX)) { + continue; + } // TODO(b/405457573): The monitored resource is currently written to `global` because the // Firestore namespace in Cloud Monitoring has not been deployed yet. Once the namespace // is available, database_id and location labels should be added here using diff --git a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/DatastoreCloudMonitoringExporterTest.java b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/DatastoreCloudMonitoringExporterTest.java index 9391585a12a9..e310082de363 100644 --- a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/DatastoreCloudMonitoringExporterTest.java +++ b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/DatastoreCloudMonitoringExporterTest.java @@ -42,6 +42,7 @@ import com.google.monitoring.v3.TimeSeries; import com.google.protobuf.Empty; import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.sdk.common.CompletableResultCode; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; import io.opentelemetry.sdk.metrics.data.AggregationTemporality; import io.opentelemetry.sdk.metrics.data.LongPointData; @@ -190,6 +191,35 @@ public void testClientCacheReferenceCounting() { verify(mockClient); } + @Test + public void testExportingIgnoredMetrics() { + // No expectations on mockMetricServiceStub means we expect NO calls to it. + replay(mockMetricServiceStub); + + long fakeValue = 11L; + long startEpoch = 10; + long endEpoch = 15; + LongPointData longPointData = + ImmutableLongPointData.create(startEpoch, endEpoch, attributes, fakeValue); + + String ignoredMetricName = "otel.sdk.metric_reader.collection.duration"; + MetricData ignoredData = + ImmutableMetricData.createLongSum( + resource, + scope, + ignoredMetricName, + "description", + "1", + ImmutableSumData.create( + true, AggregationTemporality.CUMULATIVE, ImmutableList.of(longPointData))); + + CompletableResultCode result = exporter.export(Collections.singletonList(ignoredData)); + + assertThat(result.isSuccess()).isTrue(); + + verify(mockMetricServiceStub); + } + private static class FakeMetricServiceClient extends MetricServiceClient { protected FakeMetricServiceClient(MetricServiceStub stub) { super(stub); From 4abd4414cd956a3584a69f620329646293a11219 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Thu, 25 Jun 2026 19:07:07 +0000 Subject: [PATCH 4/4] chore(datastore): address review comments on built-in metrics Address comments regarding: - Removing @Nullable from createOpenTelemetry - Avoiding wrapping no-op OpenTelemetry in DatastoreMetricsRecorder - Using isSameInstanceAs in tests TAG=agy CONV=5a246380-6d14-49d4-b850-ec05f8f89bcb --- .../telemetry/BuiltInDatastoreMetricsProvider.java | 4 +--- .../datastore/telemetry/DatastoreMetricsRecorder.java | 2 +- .../BuiltInDatastoreMetricsProviderTest.java | 4 ++-- .../telemetry/DatastoreMetricsRecorderTest.java | 11 ++++++----- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/BuiltInDatastoreMetricsProvider.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/BuiltInDatastoreMetricsProvider.java index 4ddf8b3242f5..d8494a52258d 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/BuiltInDatastoreMetricsProvider.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/BuiltInDatastoreMetricsProvider.java @@ -36,7 +36,6 @@ import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nonnull; -import javax.annotation.Nullable; /** * Provides a built-in {@link OpenTelemetry} instance for Datastore client-side metrics. @@ -116,9 +115,8 @@ private static String hashClientUId(String uuid) { * the lifetime of their Datastore client. * * @param options Datastore Client Options - * @return a new {@link OpenTelemetry} instance, or {@code null} if it could not be created. + * @return a new {@link OpenTelemetry} instance. */ - @Nullable public OpenTelemetry createOpenTelemetry(@Nonnull DatastoreOptions options) { // If built-in metrics export is disabled, return no-op OpenTelemetry to avoid instantiating // a real SdkMeterProvider. Otherwise, the SDK-internal PeriodicMetricReader will start diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/DatastoreMetricsRecorder.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/DatastoreMetricsRecorder.java index ac755ddb0f81..16202efcb544 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/DatastoreMetricsRecorder.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/DatastoreMetricsRecorder.java @@ -103,7 +103,7 @@ static DatastoreMetricsRecorder getInstance( // When using a local emulator, there is no need to configure a built-in Otel instance if (otelOptions.isExportBuiltinMetricsToGoogleCloudMonitoring()) { try { - if (builtInOtel != null) { + if (builtInOtel != null && builtInOtel != OpenTelemetry.noop()) { recorders.add( new OpenTelemetryDatastoreMetricsRecorder( builtInOtel, TelemetryConstants.METRIC_PREFIX)); diff --git a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/BuiltInDatastoreMetricsProviderTest.java b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/BuiltInDatastoreMetricsProviderTest.java index dd6d282c1805..631c288a8c4f 100644 --- a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/BuiltInDatastoreMetricsProviderTest.java +++ b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/BuiltInDatastoreMetricsProviderTest.java @@ -55,7 +55,7 @@ public void testCreateOpenTelemetry_returnsNoOp() { .setCredentials(NoCredentials.getInstance()) .build(); OpenTelemetry otel = BuiltInDatastoreMetricsProvider.INSTANCE.createOpenTelemetry(options); - assertThat(otel).isInstanceOf(OpenTelemetry.noop().getClass()); + assertThat(otel).isSameInstanceAs(OpenTelemetry.noop()); } @Test @@ -145,7 +145,7 @@ public void testCreateOpenTelemetry_withEmulatorHostProperty_returnsNoOp() { .setCredentials(NoCredentials.getInstance()) .build(); OpenTelemetry otel = BuiltInDatastoreMetricsProvider.INSTANCE.createOpenTelemetry(options); - assertThat(otel).isInstanceOf(OpenTelemetry.noop().getClass()); + assertThat(otel).isSameInstanceAs(OpenTelemetry.noop()); } finally { System.clearProperty(DatastoreOptions.LOCAL_HOST_ENV_VAR); } diff --git a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/DatastoreMetricsRecorderTest.java b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/DatastoreMetricsRecorderTest.java index 71390957a04d..c19aea217334 100644 --- a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/DatastoreMetricsRecorderTest.java +++ b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/DatastoreMetricsRecorderTest.java @@ -21,6 +21,7 @@ import com.google.cloud.datastore.DatastoreOpenTelemetryOptions; import com.google.cloud.datastore.DatastoreOptions; import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.sdk.OpenTelemetrySdk; import org.easymock.EasyMock; import org.junit.Test; import org.junit.runner.RunWith; @@ -79,7 +80,7 @@ public void tracingEnabledButMetricsDisabledAndBuiltInDisabled_returnsNoRecorder } @Test - public void defaultOptionsWithBuiltInMetricsEnabled_butNoCredentials_returnsOneRecorder() { + public void defaultOptionsWithBuiltInMetricsEnabled_butNoCredentials_returnsNoRecorders() { // Explicitly enable built-in metrics export DatastoreOptions options = baseOptions() // Uses NoCredentials by default @@ -92,12 +93,12 @@ public void defaultOptionsWithBuiltInMetricsEnabled_butNoCredentials_returnsOneR DatastoreMetricsRecorder.getInstance(options, OpenTelemetry.noop()); // Since baseOptions() uses NoCredentials, the provider returns OpenTelemetry.noop(). - // This NoOp instance is passed to getInstance, which adds it to the recorders list. - // So we have 1 recorder (the NoOp one). + // This NoOp instance is passed to getInstance, which should NOT add it to the recorders list. + // So we have 0 recorders. assertThat(recorder).isInstanceOf(CompositeDatastoreMetricsRecorder.class); CompositeDatastoreMetricsRecorder compositeRecorder = (CompositeDatastoreMetricsRecorder) recorder; - assertThat(compositeRecorder.getMetricRecorders().size()).isEqualTo(1); + assertThat(compositeRecorder.getMetricRecorders().size()).isEqualTo(0); } @Test @@ -142,7 +143,7 @@ public void bothEnabled_returnsTwoRecorders() { .setOpenTelemetry(OpenTelemetry.noop()) .build()) .build(); - OpenTelemetry builtInOtel = OpenTelemetry.noop(); + OpenTelemetry builtInOtel = OpenTelemetrySdk.builder().build(); DatastoreMetricsRecorder recorder = DatastoreMetricsRecorder.getInstance(options, builtInOtel); assertThat(recorder).isInstanceOf(CompositeDatastoreMetricsRecorder.class);