fix(datastore): disable built-in OpenTelemetry SDK when metrics export is disabled

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,10 +115,15 @@ 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
+    // 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");

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

@@ -235,6 +235,10 @@ public CompletableResultCode export(@Nonnull Collection<MetricData> collection)
       return CompletableResultCode.ofFailure();
     }
 
+    if (datastoreTimeSeries.isEmpty()) {
+      return CompletableResultCode.ofSuccess();
+    }
+
     ProjectName projectName = ProjectName.of(projectId);
 
     // Perform the actual network call to Cloud Monitoring.

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

@@ -82,8 +82,12 @@ static List<TimeSeries> convertToDatastoreTimeSeries(
       List<MetricData> collection, Map<String, String> clientAttributes) {
     List<TimeSeries> 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

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));

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;
@@ -54,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
@@ -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,13 +101,21 @@ public void testCreateOpenTelemetry_eachCallReturnsDistinctInstance() {
             .setProjectId(PROJECT_ID)
             .setDatabaseId("test-db")
             .setCredentials(credentials1)
+            .setOpenTelemetryOptions(
+                DatastoreOpenTelemetryOptions.newBuilder()
+                    .setExportBuiltinMetricsToGoogleCloudMonitoring(true)
+                    .build())
             .build();
 
     DatastoreOptions options2 =
         DatastoreOptions.newBuilder()
             .setProjectId(PROJECT_ID)
             .setDatabaseId("test-db")
             .setCredentials(credentials2)
+            .setOpenTelemetryOptions(
+                DatastoreOpenTelemetryOptions.newBuilder()
+                    .setExportBuiltinMetricsToGoogleCloudMonitoring(true)
+                    .build())
             .build();
 
     OpenTelemetry otel1 = BuiltInDatastoreMetricsProvider.INSTANCE.createOpenTelemetry(options1);
@@ -132,9 +145,26 @@ 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);
     }
   }
+
+  @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).isSameInstanceAs(OpenTelemetry.noop());
+  }
 }

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);

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);