chore: Update to remove the extra attributes

Author: lqiu96

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

@@ -45,7 +45,6 @@
 import com.google.cloud.RetryHelper;
 import com.google.cloud.RetryHelper.RetryHelperException;
 import com.google.cloud.ServiceOptions;
-import com.google.cloud.TransportOptions;
 import com.google.cloud.datastore.execution.AggregationQueryExecutor;
 import com.google.cloud.datastore.spi.v1.DatastoreRpc;
 import com.google.cloud.datastore.telemetry.DatastoreMetricsRecorder;
@@ -74,7 +73,6 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
@@ -223,7 +221,7 @@ public T call() throws DatastoreException {
         }
         throw DatastoreException.propagateUserException(ex);
       } finally {
-        recordAttempt(attemptStatus, datastore.getOptions().getTransportOptions());
+        recordAttempt(attemptStatus);
         // If the transaction is active, then commit the rollback. If it was already successfully
         // rolled back, the transaction is inactive (prevents rolling back an already rolled back
         // transaction).
@@ -242,18 +240,10 @@ public T call() throws DatastoreException {
      * Records a single transaction commit attempt with the given status code. This is called once
      * per invocation of {@link #call()}, capturing the outcome of each individual commit attempt.
      */
-    private void recordAttempt(String status, TransportOptions transportOptions) {
-      Map<String, String> attributes = new HashMap<>();
-      attributes.put(TelemetryConstants.ATTRIBUTES_KEY_STATUS, status);
-      attributes.put(
-          TelemetryConstants.ATTRIBUTES_KEY_METHOD, TelemetryConstants.METHOD_TRANSACTION_COMMIT);
-      attributes.put(
-          TelemetryConstants.ATTRIBUTES_KEY_PROJECT_ID, datastore.getOptions().getProjectId());
-      attributes.put(
-          TelemetryConstants.ATTRIBUTES_KEY_DATABASE_ID, datastore.getOptions().getDatabaseId());
-      attributes.put(
-          TelemetryConstants.ATTRIBUTES_KEY_TRANSPORT,
-          TelemetryConstants.getTransportName(transportOptions));
+    private void recordAttempt(String status) {
+      Map<String, String> attributes =
+          TelemetryUtils.buildMetricAttributes(
+              TelemetryConstants.METHOD_TRANSACTION_COMMIT, status);
       metricsRecorder.recordTransactionAttemptCount(1, attributes);
     }
   }
@@ -290,15 +280,8 @@ public <T> T runInTransaction(
       throw DatastoreException.translateAndThrow(e);
     } finally {
       long latencyMs = stopwatch.elapsed(TimeUnit.MILLISECONDS);
-      Map<String, String> attributes = new HashMap<>();
-      attributes.put(TelemetryConstants.ATTRIBUTES_KEY_STATUS, status);
-      attributes.put(
-          TelemetryConstants.ATTRIBUTES_KEY_METHOD, TelemetryConstants.METHOD_TRANSACTION_RUN);
-      attributes.put(TelemetryConstants.ATTRIBUTES_KEY_PROJECT_ID, getOptions().getProjectId());
-      attributes.put(TelemetryConstants.ATTRIBUTES_KEY_DATABASE_ID, getOptions().getDatabaseId());
-      attributes.put(
-          TelemetryConstants.ATTRIBUTES_KEY_TRANSPORT,
-          TelemetryConstants.getTransportName(getOptions().getTransportOptions()));
+      Map<String, String> attributes =
+          TelemetryUtils.buildMetricAttributes(TelemetryConstants.METHOD_TRANSACTION_RUN, status);
       metricsRecorder.recordTransactionLatency(latencyMs, attributes);
       span.end();
     }
@@ -807,7 +790,7 @@ private <T> T runWithObservability(
 
     DatastoreOptions options = getOptions();
     Callable<T> attemptCallable =
-        TelemetryUtils.attemptMetricsCallable(callable, metricsRecorder, options, methodName);
+        TelemetryUtils.attemptMetricsCallable(callable, metricsRecorder, methodName);
     try (TraceUtil.Scope ignored = span.makeCurrent()) {
       return RetryHelper.runWithRetries(
           attemptCallable, retrySettings, exceptionHandler, options.getClock());
@@ -817,7 +800,7 @@ private <T> T runWithObservability(
       throw DatastoreException.translateAndThrow(e);
     } finally {
       TelemetryUtils.recordOperationMetrics(
-          metricsRecorder, options, operationStopwatch, methodName, operationStatus);
+          metricsRecorder, operationStopwatch, methodName, operationStatus);
       span.end();
     }
   }

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

@@ -205,8 +205,7 @@ <O> O invokeRpc(Callable<O> block, String startSpan, String methodName) {
     String operationStatus = StatusCode.Code.UNKNOWN.toString();
     try (TraceUtil.Scope ignored = span.makeCurrent()) {
       Callable<O> callable =
-          TelemetryUtils.attemptMetricsCallable(
-              block, metricsRecorder, datastoreOptions, methodName);
+          TelemetryUtils.attemptMetricsCallable(block, metricsRecorder, methodName);
       O result =
           RetryHelper.runWithRetries(
               callable, this.retrySettings, EXCEPTION_HANDLER, this.datastoreOptions.getClock());
@@ -218,7 +217,7 @@ <O> O invokeRpc(Callable<O> block, String startSpan, String methodName) {
       throw DatastoreException.translateAndThrow(e);
     } finally {
       TelemetryUtils.recordOperationMetrics(
-          metricsRecorder, datastoreOptions, stopwatch, methodName, operationStatus);
+          metricsRecorder, stopwatch, methodName, operationStatus);
       span.end();
     }
   }

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

@@ -45,9 +45,16 @@ class OpenTelemetryDatastoreMetricsRecorder extends OpenTelemetryMetricsRecorder
 
   private final OpenTelemetry openTelemetry;
 
+  // Datastore-specific transaction metrics (registered under the Datastore meter).
   private final DoubleHistogram transactionLatency;
   private final LongCounter transactionAttemptCount;
 
+  // GAX operation/attempt latency metrics re-registered under the Datastore meter with the
+  // plural names required by the internal Cloud Monitoring descriptor. These override the
+  // singular-named histograms registered by the parent GAX class.
+  private final DoubleHistogram operationLatency;
+  private final DoubleHistogram attemptLatency;
+
   OpenTelemetryDatastoreMetricsRecorder(@Nonnull OpenTelemetry openTelemetry, String metricPrefix) {
     super(openTelemetry, metricPrefix);
     this.openTelemetry = openTelemetry;
@@ -66,12 +73,37 @@ class OpenTelemetryDatastoreMetricsRecorder extends OpenTelemetryMetricsRecorder
             .counterBuilder(TelemetryConstants.METRIC_NAME_TRANSACTION_ATTEMPT_COUNT)
             .setDescription("Number of attempts to commit a transaction")
             .build();
+
+    this.operationLatency =
+        meter
+            .histogramBuilder(TelemetryConstants.METRIC_NAME_OPERATION_LATENCY)
+            .setDescription(
+                "Total time until final operation success or failure, including retries and backoff.")
+            .setUnit("ms")
+            .build();
+
+    this.attemptLatency =
+        meter
+            .histogramBuilder(TelemetryConstants.METRIC_NAME_ATTEMPT_LATENCY)
+            .setDescription("Time an individual attempt took")
+            .setUnit("ms")
+            .build();
   }
 
   OpenTelemetry getOpenTelemetry() {
     return openTelemetry;
   }
 
+  @Override
+  public void recordOperationLatency(double latencyMs, Map<String, String> attributes) {
+    operationLatency.record(latencyMs, toOtelAttributes(attributes));
+  }
+
+  @Override
+  public void recordAttemptLatency(double latencyMs, Map<String, String> attributes) {
+    attemptLatency.record(latencyMs, toOtelAttributes(attributes));
+  }
+
   @Override
   public void recordTransactionLatency(double latencyMs, Map<String, String> attributes) {
     transactionLatency.record(latencyMs, toOtelAttributes(attributes));

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

@@ -17,9 +17,6 @@
 package com.google.cloud.datastore.telemetry;
 
 import com.google.api.core.InternalApi;
-import com.google.api.gax.tracing.OpenTelemetryMetricsRecorder;
-import com.google.cloud.TransportOptions;
-import com.google.cloud.grpc.GrpcTransportOptions;
 import com.google.common.collect.ImmutableSet;
 import io.opentelemetry.api.common.AttributeKey;
 import java.util.Set;
@@ -33,80 +30,24 @@
  */
 @InternalApi
 public class TelemetryConstants {
-  // Built-in metrics constants for Cloud Monitoring export
+
+  // The Firestore namespace has not been deployed yet. Must target the custom namespace
+  // until this is implemented.
   public static final String METRIC_PREFIX = "custom.googleapis.com/internal/client";
-  public static final String GAX_METER_NAME = OpenTelemetryMetricsRecorder.GAX_METER_NAME;
   public static final String DATASTORE_METER_NAME = "java-datastore";
 
   // Monitored resource type for Cloud Monitoring
   public static final String DATASTORE_RESOURCE_TYPE = "global";
 
   // Resource label keys for the monitored resource
+  // The Firestore namespace has not been deployed yet. Must target the global
+  // Monitored Resource until this is implemented.
   public static final String RESOURCE_LABEL_PROJECT_ID = "project_id";
   public static final String RESOURCE_LABEL_DATABASE_ID = "database_id";
   public static final String RESOURCE_LABEL_LOCATION = "location";
   public static final Set<String> DATASTORE_RESOURCE_LABELS =
-      ImmutableSet.of(RESOURCE_LABEL_PROJECT_ID);
-
-  // Resource attribute keys (used on OTel Resource)
-  public static final AttributeKey<String> PROJECT_ID_KEY = AttributeKey.stringKey("project_id");
-  public static final AttributeKey<String> DATABASE_ID_KEY = AttributeKey.stringKey("database_id");
-  public static final AttributeKey<String> LOCATION_ID_KEY = AttributeKey.stringKey("location");
-
-  // Metric attribute keys (used on metric data points)
-  public static final AttributeKey<String> CLIENT_UID_KEY = AttributeKey.stringKey("client_uid");
-  public static final AttributeKey<String> CLIENT_NAME_KEY = AttributeKey.stringKey("client_name");
-  public static final AttributeKey<String> METHOD_KEY = AttributeKey.stringKey("method");
-  public static final AttributeKey<String> STATUS_KEY = AttributeKey.stringKey("status");
-  public static final AttributeKey<String> DATABASE_KEY = AttributeKey.stringKey("database_id");
-  public static final AttributeKey<String> LIBRARY_VERSION_KEY =
-      AttributeKey.stringKey("library_version");
-  public static final AttributeKey<String> TRANSPORT_KEY = AttributeKey.stringKey("transport");
-  public static final AttributeKey<String> SERVICE_KEY = AttributeKey.stringKey("service");
-
-  public static final String SERVICE_VALUE = "datastore.googleapis.com";
-
-  /**
-   * The allowlist of metric attributes that are permitted on every exported data point.
-   *
-   * <p>Cloud Monitoring is strict about label schemas: exporting a label that was not present when
-   * the metric descriptor was first created will cause the entire {@code createTimeSeries} call to
-   * fail.
-   */
-  public static final Set<AttributeKey<?>> COMMON_ATTRIBUTES =
-      ImmutableSet.of(
-          CLIENT_UID_KEY,
-          CLIENT_NAME_KEY,
-          METHOD_KEY,
-          STATUS_KEY,
-          DATABASE_KEY,
-          LIBRARY_VERSION_KEY,
-          TRANSPORT_KEY,
-          SERVICE_KEY);
-
-  // Metric names (short names, without prefix)
-  public static final String METRIC_NAME_SHORT_OPERATION_LATENCY = "operation_latency";
-  public static final String METRIC_NAME_SHORT_ATTEMPT_LATENCY = "attempt_latency";
-  public static final String METRIC_NAME_SHORT_OPERATION_COUNT = "operation_count";
-  public static final String METRIC_NAME_SHORT_ATTEMPT_COUNT = "attempt_count";
-  public static final String METRIC_NAME_SHORT_TRANSACTION_LATENCY = "transaction_latency";
-  public static final String METRIC_NAME_SHORT_TRANSACTION_ATTEMPT_COUNT =
-      "transaction_attempt_count";
-
-  // Metrics collected at the GAX layer vs Datastore SDK layer
-  public static final Set<String> GAX_METRICS =
-      ImmutableSet.of(
-          METRIC_NAME_SHORT_OPERATION_LATENCY,
-          METRIC_NAME_SHORT_ATTEMPT_LATENCY,
-          METRIC_NAME_SHORT_OPERATION_COUNT,
-          METRIC_NAME_SHORT_ATTEMPT_COUNT);
-
-  public static final Set<String> DATASTORE_METRICS =
       ImmutableSet.of(
-          METRIC_NAME_SHORT_TRANSACTION_LATENCY, METRIC_NAME_SHORT_TRANSACTION_ATTEMPT_COUNT);
-
-  // Environment variable to enable/disable built-in metrics
-  public static final String ENABLE_METRICS_ENV_VAR = "DATASTORE_ENABLE_METRICS";
+          RESOURCE_LABEL_PROJECT_ID, RESOURCE_LABEL_DATABASE_ID, RESOURCE_LABEL_LOCATION);
 
   // Existing attribute key constants (string-based, used by MetricsHelper/TelemetryUtils)
   public static final String ATTRIBUTES_KEY_DOCUMENT_COUNT = "doc_count";
@@ -130,23 +71,58 @@ public class TelemetryConstants {
   /** Attribute key for the Datastore database ID. */
   public static final String ATTRIBUTES_KEY_DATABASE_ID = "database_id";
 
-  public static final String ATTRIBUTES_KEY_LIBRARY_VERSION = "library_version";
+  // Resource attribute keys (used on OTel Resource)
+  public static final AttributeKey<String> PROJECT_ID_KEY = AttributeKey.stringKey("project_id");
+  public static final AttributeKey<String> DATABASE_ID_KEY = AttributeKey.stringKey("database_id");
+  public static final AttributeKey<String> LOCATION_ID_KEY = AttributeKey.stringKey("location");
+
+  // Metric attribute keys (used on metric data points)
+  public static final AttributeKey<String> CLIENT_UID_KEY = AttributeKey.stringKey("client_uid");
+  public static final AttributeKey<String> METHOD_KEY = AttributeKey.stringKey("method");
+  public static final AttributeKey<String> STATUS_KEY = AttributeKey.stringKey("status");
+  public static final AttributeKey<String> SERVICE_KEY = AttributeKey.stringKey("service");
+
+  public static final String SERVICE_VALUE = "datastore.googleapis.com";
 
-  public static final String ATTRIBUTES_KEY_TRANSPORT = "transport";
+  /** String key for the {@code service} metric attribute (value: {@code "service"}). */
+  public static final String ATTRIBUTES_KEY_SERVICE = SERVICE_KEY.getKey();
+
+  /**
+   * The allowlist of metric attributes that are permitted on every exported data point.
+   *
+   * <p>Cloud Monitoring is strict about label schemas: exporting a label that was not present when
+   * the metric descriptor was first created will cause the entire {@code createTimeSeries} call to
+   * fail. Only {@code status}, {@code method}, {@code service}, and {@code client_uid} are
+   * accepted; all other attributes must be omitted from every {@code record*()} call.
+   */
+  public static final Set<AttributeKey<?>> COMMON_ATTRIBUTES =
+      ImmutableSet.of(CLIENT_UID_KEY, METHOD_KEY, STATUS_KEY, SERVICE_KEY);
 
   /** Metric name for the total latency of a transaction. */
   public static final String METRIC_NAME_TRANSACTION_LATENCY =
-      METRIC_PREFIX + "/transaction_latency";
+      METRIC_PREFIX + "/transaction_latencies";
 
   /** Metric name for the number of attempts a transaction took. */
   public static final String METRIC_NAME_TRANSACTION_ATTEMPT_COUNT =
       METRIC_PREFIX + "/transaction_attempt_count";
 
-  /** Metric name for the total latency of an operation (one full RPC call including retries). */
-  public static final String METRIC_NAME_OPERATION_LATENCY = METRIC_PREFIX + "/operation_latency";
+  /**
+   * Metric name for the total latency of an operation (one full RPC call including retries).
+   *
+   * <p>The plural form ({@code operation_latencies}) is intentional: it matches the internal Cloud
+   * Monitoring metric descriptor name. {@link OpenTelemetryDatastoreMetricsRecorder} overrides the
+   * inherited GAX method to record to this name rather than the singular GAX default.
+   */
+  public static final String METRIC_NAME_OPERATION_LATENCY = METRIC_PREFIX + "/operation_latencies";
 
-  /** Metric name for the latency of a single RPC attempt. */
-  public static final String METRIC_NAME_ATTEMPT_LATENCY = METRIC_PREFIX + "/attempt_latency";
+  /**
+   * Metric name for the latency of a single RPC attempt.
+   *
+   * <p>The plural form ({@code attempt_latencies}) is intentional: it matches the internal Cloud
+   * Monitoring metric descriptor name. {@link OpenTelemetryDatastoreMetricsRecorder} overrides the
+   * inherited GAX method to record to this name rather than the singular GAX default.
+   */
+  public static final String METRIC_NAME_ATTEMPT_LATENCY = METRIC_PREFIX + "/attempt_latencies";
 
   /** Metric name for the count of operations. */
   public static final String METRIC_NAME_OPERATION_COUNT = METRIC_PREFIX + "/operation_count";
@@ -191,13 +167,5 @@ public String getTransport() {
     }
   }
 
-  public static String getTransportName(TransportOptions transportOptions) {
-    if (transportOptions instanceof GrpcTransportOptions) {
-      return Transport.GRPC.getTransport();
-    } else {
-      return Transport.HTTP.getTransport();
-    }
-  }
-
   private TelemetryConstants() {}
 }