chore: Refactor code and add comments

Author: lqiu96

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

@@ -128,7 +128,7 @@ public static class Builder {
     private Builder() {
       tracingEnabled = false;
       metricsEnabled = false;
-      // TODO: This is disabled by default until the Firestore namespace is deployed
+      // TODO(b/405457573): This is disabled by default until the Firestore namespace is deployed
       exportBuiltinMetricsToGoogleCloudMonitoring = false;
       openTelemetry = null;
     }
@@ -176,7 +176,6 @@ DatastoreOpenTelemetryOptions.Builder setMetricsEnabled(boolean enabled) {
      * @return this builder instance.
      */
     @BetaApi
-    @Nonnull
     public DatastoreOpenTelemetryOptions.Builder setExportBuiltinMetricsToGoogleCloudMonitoring(
         boolean exportBuiltinMetrics) {
       this.exportBuiltinMetricsToGoogleCloudMonitoring = exportBuiltinMetrics;

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

@@ -18,6 +18,7 @@
 
 import com.google.auth.Credentials;
 import com.google.cloud.NoCredentials;
+import com.google.common.base.Preconditions;
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.common.AttributesBuilder;
@@ -47,16 +48,15 @@
  * automated environment detection and resource attribute configuration for the {@link
  * TelemetryConstants#DATASTORE_RESOURCE_TYPE} monitored resource.
  */
-public final class BuiltInDatastoreMetricsProvider {
+class BuiltInDatastoreMetricsProvider {
 
   public static final BuiltInDatastoreMetricsProvider INSTANCE =
       new BuiltInDatastoreMetricsProvider();
 
   private static final Logger logger =
       Logger.getLogger(BuiltInDatastoreMetricsProvider.class.getName());
 
-  private static volatile String location;
-  private static final String DEFAULT_LOCATION = "global";
+  private String location;
 
   // Pre-computed once per JVM; hostname lookup can block, so we pay the cost at class-init time.
   private static final String PID_AND_HOSTNAME = getProcessId() + "@" + getHostnameSafely();
@@ -92,6 +92,7 @@ static Map<String, String> buildClientAttributes() {
   @Nullable
   public OpenTelemetry createOpenTelemetry(
       @Nonnull String projectId, @Nonnull String databaseId, @Nonnull Credentials credentials) {
+    Preconditions.checkNotNull(credentials, "Credentials cannot be null for built in metrics");
     SdkMeterProviderBuilder sdkMeterProviderBuilder = SdkMeterProvider.builder();
 
     Map<String, String> clientAttributes = buildClientAttributes();
@@ -127,9 +128,9 @@ public OpenTelemetry createOpenTelemetry(
    *
    * @return the detected location, or "global" if detection fails.
    */
-  public static String detectClientLocation() {
+  public String detectClientLocation() {
     if (location == null) {
-      location = DEFAULT_LOCATION;
+      location = "global";
     }
     return location;
   }

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

@@ -52,7 +52,7 @@ class OpenTelemetryDatastoreMetricsRecorder extends OpenTelemetryMetricsRecorder
   private final OpenTelemetry openTelemetry;
   // True when this recorder created the OpenTelemetry instance (built-in path) and therefore
   // owns its lifecycle. False when the instance was provided by the user.
-  private final boolean ownsOpenTelemetry;
+  private final boolean isBuiltInOpenTelemetryInstance;
 
   // Datastore-specific transaction metrics (registered under the Datastore meter).
   private final DoubleHistogram transactionLatency;
@@ -66,14 +66,16 @@ class OpenTelemetryDatastoreMetricsRecorder extends OpenTelemetryMetricsRecorder
   /**
    * Creates a recorder, specifying whether this instance owns the {@link OpenTelemetry} lifecycle.
    *
-   * @param ownsOpenTelemetry {@code true} if this recorder created the instance and should shut it
-   *     down on {@link #close()}; {@code false} if the user provided it.
+   * @param isBuiltInOpenTelemetryInstance {@code true} if this recorder created the instance and
+   *     should shut it down on {@link #close()}; {@code false} if the user provided it.
    */
   OpenTelemetryDatastoreMetricsRecorder(
-      @Nonnull OpenTelemetry openTelemetry, String metricPrefix, boolean ownsOpenTelemetry) {
+      @Nonnull OpenTelemetry openTelemetry,
+      String metricPrefix,
+      boolean isBuiltInOpenTelemetryInstance) {
     super(openTelemetry, metricPrefix);
     this.openTelemetry = openTelemetry;
-    this.ownsOpenTelemetry = ownsOpenTelemetry;
+    this.isBuiltInOpenTelemetryInstance = isBuiltInOpenTelemetryInstance;
 
     // Note: Standard GAX RPC metrics (operation_latency, attempt_latency, etc.) are handled by the
     // base OpenTelemetryMetricsRecorder class. Those metrics are inherited from the parent classes.
@@ -108,7 +110,7 @@ OpenTelemetry getOpenTelemetry() {
    */
   @Override
   public void close() {
-    if (ownsOpenTelemetry && openTelemetry instanceof OpenTelemetrySdk) {
+    if (isBuiltInOpenTelemetryInstance && openTelemetry instanceof OpenTelemetrySdk) {
       try {
         ((OpenTelemetrySdk) openTelemetry).close();
       } catch (Exception e) {

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

@@ -18,8 +18,12 @@
 
 import static com.google.common.truth.Truth.assertThat;
 
+import com.google.auth.CredentialTypeForMetrics;
+import com.google.auth.Credentials;
+import com.google.cloud.NoCredentials;
 import io.opentelemetry.api.OpenTelemetry;
 import java.util.Map;
+import org.easymock.EasyMock;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -39,27 +43,43 @@ public void testBuildClientAttributes() {
         .isEqualTo(TelemetryConstants.SERVICE_VALUE);
   }
 
+  @Test
+  public void testCreateOpenTelemetry_returnsNull() {
+    OpenTelemetry otel =
+        BuiltInDatastoreMetricsProvider.INSTANCE.createOpenTelemetry(
+            PROJECT_ID, "test-db", NoCredentials.getInstance());
+    assertThat(otel).isNull();
+  }
+
   @Test
   public void testCreateOpenTelemetry_returnsNonNull() {
+    Credentials credentials = EasyMock.mock(Credentials.class);
+    EasyMock.expect(credentials.getMetricsCredentialType())
+        .andReturn(CredentialTypeForMetrics.DO_NOT_SEND);
+    EasyMock.replay(credentials);
     OpenTelemetry otel =
-        BuiltInDatastoreMetricsProvider.INSTANCE.createOpenTelemetry(PROJECT_ID, "test-db", null);
+        BuiltInDatastoreMetricsProvider.INSTANCE.createOpenTelemetry(
+            PROJECT_ID, "test-db", credentials);
     assertThat(otel).isNotNull();
   }
 
   @Test
   public void testCreateOpenTelemetry_eachCallReturnsDistinctInstance() {
+    Credentials credentials1 = EasyMock.mock(Credentials.class);
+    EasyMock.expect(credentials1.getMetricsCredentialType())
+        .andReturn(CredentialTypeForMetrics.DO_NOT_SEND);
+    Credentials credentials2 = EasyMock.mock(Credentials.class);
+    EasyMock.expect(credentials2.getMetricsCredentialType())
+        .andReturn(CredentialTypeForMetrics.DO_NOT_SEND);
+    EasyMock.replay(credentials1, credentials2);
     OpenTelemetry otel1 =
-        BuiltInDatastoreMetricsProvider.INSTANCE.createOpenTelemetry(PROJECT_ID, "test-db", null);
+        BuiltInDatastoreMetricsProvider.INSTANCE.createOpenTelemetry(
+            PROJECT_ID, "test-db", credentials1);
     OpenTelemetry otel2 =
-        BuiltInDatastoreMetricsProvider.INSTANCE.createOpenTelemetry(PROJECT_ID, "test-db", null);
+        BuiltInDatastoreMetricsProvider.INSTANCE.createOpenTelemetry(
+            PROJECT_ID, "test-db", credentials2);
     assertThat(otel1).isNotNull();
     assertThat(otel2).isNotNull();
     assertThat(otel1).isNotSameInstanceAs(otel2);
   }
-
-  @Test
-  public void testDetectClientLocation() {
-    String location = BuiltInDatastoreMetricsProvider.detectClientLocation();
-    assertThat(location).isEqualTo("global");
-  }
 }