feat(datastore): Enable Otel metrics for custom Otel (#12969)

Mark `setMetricsEnabled(boolean enabled)` as public in
DatastoreOpenTelemetryOptions.

Some additional clean up about the Otel object and small fixes

Author: lqiu96

java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOpenTelemetryOptions.java

@@ -17,9 +17,9 @@
 package com.google.cloud.datastore;
 
 import com.google.api.core.BetaApi;
+import com.google.common.base.Preconditions;
 import io.opentelemetry.api.OpenTelemetry;
 import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
 
 /**
  * Represents the options that are used to configure the use of OpenTelemetry for telemetry
@@ -29,14 +29,15 @@ public class DatastoreOpenTelemetryOptions {
   private final boolean tracingEnabled;
   private final boolean metricsEnabled;
   private final boolean exportBuiltinMetricsToGoogleCloudMonitoring;
-  private final @Nullable OpenTelemetry openTelemetry;
+  private final OpenTelemetry openTelemetry;
 
   DatastoreOpenTelemetryOptions(Builder builder) {
     this.tracingEnabled = builder.tracingEnabled;
     this.metricsEnabled = builder.metricsEnabled;
     this.exportBuiltinMetricsToGoogleCloudMonitoring =
         builder.exportBuiltinMetricsToGoogleCloudMonitoring;
-    this.openTelemetry = builder.openTelemetry;
+    this.openTelemetry =
+        builder.openTelemetry == null ? OpenTelemetry.noop() : builder.openTelemetry;
   }
 
   /**
@@ -88,11 +89,13 @@ public boolean isExportBuiltinMetricsToGoogleCloudMonitoring() {
   }
 
   /**
-   * Returns the custom {@link OpenTelemetry} instance, if one was provided.
+   * Returns the configured custom {@link OpenTelemetry} instance.
    *
-   * @return the custom {@link OpenTelemetry} instance, or {@code null} if none was provided.
+   * @return the configured {@link OpenTelemetry} instance, or the global instance if a custom one
+   *     was not provided. If there is no global instance, then {@code OpenTelemetry.noop()} is
+   *     returned.
    */
-  @Nullable
+  @Nonnull
   public OpenTelemetry getOpenTelemetry() {
     return openTelemetry;
   }
@@ -123,7 +126,7 @@ public static class Builder {
     private boolean metricsEnabled;
     private boolean exportBuiltinMetricsToGoogleCloudMonitoring;
 
-    @Nullable private OpenTelemetry openTelemetry;
+    private OpenTelemetry openTelemetry;
 
     private Builder() {
       tracingEnabled = false;
@@ -159,8 +162,8 @@ public DatastoreOpenTelemetryOptions.Builder setTracingEnabled(boolean enabled)
      * @param enabled Whether metrics should be enabled.
      * @return this builder instance.
      */
-    @Nonnull
-    DatastoreOpenTelemetryOptions.Builder setMetricsEnabled(boolean enabled) {
+    @BetaApi
+    public DatastoreOpenTelemetryOptions.Builder setMetricsEnabled(boolean enabled) {
       this.metricsEnabled = enabled;
       return this;
     }
@@ -193,6 +196,7 @@ public DatastoreOpenTelemetryOptions.Builder setExportBuiltinMetricsToGoogleClou
     @Nonnull
     public DatastoreOpenTelemetryOptions.Builder setOpenTelemetry(
         @Nonnull OpenTelemetry openTelemetry) {
+      Preconditions.checkNotNull(openTelemetry, "OpenTelemetry instance cannot be null");
       this.openTelemetry = openTelemetry;
       return this;
     }

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

@@ -122,7 +122,7 @@ static DatastoreMetricsRecorder getInstance(
     // Note: Metrics will not be sent if an emulator is enabled.
     if (otelOptions.isMetricsEnabled()) {
       OpenTelemetry customOtel = otelOptions.getOpenTelemetry();
-      if (customOtel == null) {
+      if (customOtel.getMeterProvider() == OpenTelemetry.noop().getMeterProvider()) {
         customOtel = GlobalOpenTelemetry.get();
       }
       recorders.add(

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

@@ -22,9 +22,6 @@
 import com.google.api.core.ApiFutures;
 import com.google.api.core.InternalApi;
 import com.google.cloud.datastore.DatastoreOptions;
-import com.google.cloud.datastore.telemetry.TraceUtil.Context;
-import com.google.cloud.datastore.telemetry.TraceUtil.Scope;
-import com.google.cloud.datastore.telemetry.TraceUtil.Span;
 import com.google.common.base.Throwables;
 import io.grpc.ManagedChannelBuilder;
 import io.opentelemetry.api.GlobalOpenTelemetry;
@@ -51,16 +48,12 @@ public class EnabledTraceUtil implements TraceUtil {
   private final DatastoreOptions datastoreOptions;
 
   EnabledTraceUtil(DatastoreOptions datastoreOptions) {
-    OpenTelemetry openTelemetry = datastoreOptions.getOpenTelemetryOptions().getOpenTelemetry();
-
-    // If tracing is enabled, but an OpenTelemetry instance is not provided, fall back
-    // to using GlobalOpenTelemetry.
-    if (openTelemetry == null) {
-      openTelemetry = GlobalOpenTelemetry.get();
-    }
-
     this.datastoreOptions = datastoreOptions;
-    this.openTelemetry = openTelemetry;
+    OpenTelemetry otel = datastoreOptions.getOpenTelemetryOptions().getOpenTelemetry();
+    if (otel.getTracerProvider() == TracerProvider.noop()) {
+      otel = GlobalOpenTelemetry.get();
+    }
+    this.openTelemetry = otel;
     this.tracer = openTelemetry.getTracer(LIBRARY_NAME);
   }
 

java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOpenTelemetryOptionsTestHelper.java

@@ -22,6 +22,7 @@
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import com.google.api.gax.grpc.ChannelPoolSettings;
 import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider;
@@ -159,6 +160,45 @@ public void testTelemetrySignalsMixedEnabled() {
     assertTrue(o2.isEnabled());
   }
 
+  @Test
+  public void testOpenTelemetryMetricsAndCloudMonitoringMixed() {
+    DatastoreOpenTelemetryOptions o1 =
+        DatastoreOpenTelemetryOptions.newBuilder()
+            .setMetricsEnabled(true)
+            .setExportBuiltinMetricsToGoogleCloudMonitoring(false)
+            .build();
+    assertTrue(o1.isMetricsEnabled());
+    assertFalse(o1.isExportBuiltinMetricsToGoogleCloudMonitoring());
+    assertTrue(o1.isEnabled());
+
+    DatastoreOpenTelemetryOptions o2 =
+        DatastoreOpenTelemetryOptions.newBuilder()
+            .setMetricsEnabled(false)
+            .setExportBuiltinMetricsToGoogleCloudMonitoring(true)
+            .build();
+    assertFalse(o2.isMetricsEnabled());
+    assertTrue(o2.isExportBuiltinMetricsToGoogleCloudMonitoring());
+    assertFalse(o2.isEnabled());
+  }
+
+  @Test
+  public void testOpenTelemetryOptionsDefaultInstance() {
+    DatastoreOpenTelemetryOptions telemetryOptions =
+        DatastoreOpenTelemetryOptions.newBuilder().build();
+    assertThat(telemetryOptions.getOpenTelemetry())
+        .isSameInstanceAs(io.opentelemetry.api.OpenTelemetry.noop());
+  }
+
+  @Test
+  public void testOpenTelemetryOptionsSetNullThrowsNPE() {
+    try {
+      DatastoreOpenTelemetryOptions.newBuilder().setOpenTelemetry(null);
+      fail("Expected NullPointerException");
+    } catch (NullPointerException e) {
+      assertThat(e.getMessage()).isEqualTo("OpenTelemetry instance cannot be null");
+    }
+  }
+
   @Test
   public void testNamespace() {
     assertTrue(options.build().getNamespace().isEmpty());

java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java

@@ -14,13 +14,18 @@
  * limitations under the License.
  */
 
-package com.google.cloud.datastore;
+package com.google.cloud.datastore.it;
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
 import static org.junit.Assume.assumeNotNull;
 
 import com.google.cloud.TransportOptions;
+import com.google.cloud.datastore.Datastore;
+import com.google.cloud.datastore.DatastoreOpenTelemetryOptions;
+import com.google.cloud.datastore.DatastoreOptions;
+import com.google.cloud.datastore.Entity;
+import com.google.cloud.datastore.Key;
 import com.google.cloud.datastore.telemetry.TelemetryConstants;
 import com.google.cloud.grpc.GrpcTransportOptions;
 import com.google.cloud.http.HttpTransportOptions;
@@ -59,16 +64,15 @@
  */
 @RunWith(Parameterized.class)
 @SuppressWarnings("checkstyle:abbreviationaswordinname")
-public class ITDatastoreBuiltInAndCustomMetrics {
+public class ITDatastoreClientSideMetrics {
 
   private static final String PROJECT_ID = System.getenv("GOOGLE_CLOUD_PROJECT");
   private static final String DATABASE_ID =
       System.getenv().getOrDefault("DATASTORE_DATABASE_ID", "");
-  private boolean isDatastoreClosed = false;
 
   private final TransportOptions transportOptions;
 
-  public ITDatastoreBuiltInAndCustomMetrics(TransportOptions transportOptions) {
+  public ITDatastoreClientSideMetrics(TransportOptions transportOptions) {
     this.transportOptions = transportOptions;
   }
 
@@ -134,7 +138,7 @@ public void setUp() {
 
   @After
   public void tearDown() throws Exception {
-    if (datastore != null && !isDatastoreClosed) {
+    if (datastore != null) {
       Key key = datastore.newKeyFactory().setKind(kind).newKey("metrics-it-entity");
       try {
         datastore.delete(key);

…/ITDatastoreBuiltInAndCustomMetrics.java …ore/it/ITDatastoreClientSideMetrics.javajava-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/ITDatastoreBuiltInAndCustomMetrics.java renamed to java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreClientSideMetrics.java java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/ITDatastoreBuiltInAndCustomMetrics.java renamed to java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreClientSideMetrics.java

@@ -109,4 +109,45 @@ public void openTelemetryRecorderCreatedWithExplicitOpenTelemetry() {
 
     assertThat(recorder.getOpenTelemetry()).isSameInstanceAs(openTelemetry);
   }
+
+  @Test
+  public void builtInDisabledAndCustomEnabled_returnsOneRecorder() {
+    DatastoreOptions options =
+        baseOptions()
+            .setOpenTelemetryOptions(
+                DatastoreOpenTelemetryOptions.newBuilder()
+                    .setExportBuiltinMetricsToGoogleCloudMonitoring(false)
+                    .setMetricsEnabled(true)
+                    .setOpenTelemetry(OpenTelemetry.noop())
+                    .build())
+            .build();
+    OpenTelemetry builtInOtel = EasyMock.createMock(OpenTelemetry.class);
+    EasyMock.replay(builtInOtel);
+
+    DatastoreMetricsRecorder recorder = DatastoreMetricsRecorder.getInstance(options, builtInOtel);
+    assertThat(recorder).isInstanceOf(CompositeDatastoreMetricsRecorder.class);
+    CompositeDatastoreMetricsRecorder compositeRecorder =
+        (CompositeDatastoreMetricsRecorder) recorder;
+    assertThat(compositeRecorder.getMetricRecorders().size()).isEqualTo(1);
+  }
+
+  @Test
+  public void bothEnabled_returnsTwoRecorders() {
+    DatastoreOptions options =
+        baseOptions()
+            .setOpenTelemetryOptions(
+                DatastoreOpenTelemetryOptions.newBuilder()
+                    .setExportBuiltinMetricsToGoogleCloudMonitoring(true)
+                    .setMetricsEnabled(true)
+                    .setOpenTelemetry(OpenTelemetry.noop())
+                    .build())
+            .build();
+    OpenTelemetry builtInOtel = OpenTelemetry.noop();
+
+    DatastoreMetricsRecorder recorder = DatastoreMetricsRecorder.getInstance(options, builtInOtel);
+    assertThat(recorder).isInstanceOf(CompositeDatastoreMetricsRecorder.class);
+    CompositeDatastoreMetricsRecorder compositeRecorder =
+        (CompositeDatastoreMetricsRecorder) recorder;
+    assertThat(compositeRecorder.getMetricRecorders().size()).isEqualTo(2);
+  }
 }