chore: Refactor DatastoreImplMetricsTest to check the attributes

Author: lqiu96

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

@@ -131,9 +131,10 @@ public Transaction newTransaction() {
   }
 
   /**
-   * A wrapper around {@link ReadWriteTransactionCallable} that adds OpenTelemetry tracing context
-   * propagation. All transaction logic (begin, run, commit, rollback, metrics recording) is
-   * delegated to the underlying {@link ReadWriteTransactionCallable}.
+   * A Tracing callable that adds OpenTelemetry tracing context. Intended to be used for
+   * transactions and wraps {@link ReadWriteTransactionCallable} as the delegate. All transaction
+   * logic (begin, run, commit, rollback, metrics recording) is handled in the delegate (this solely
+   * handles tracing).
    */
   static class TracedReadWriteTransactionCallable<T> implements Callable<T> {
     private final ReadWriteTransactionCallable<T> delegate;
@@ -146,10 +147,6 @@ static class TracedReadWriteTransactionCallable<T> implements Callable<T> {
       this.parentSpan = parentSpan;
     }
 
-    ReadWriteTransactionCallable<T> getDelegate() {
-      return delegate;
-    }
-
     @Override
     public T call() throws DatastoreException {
       try (io.opentelemetry.context.Scope ignored =
@@ -192,18 +189,6 @@ static class ReadWriteTransactionCallable<T> implements Callable<T> {
       this.transaction = null;
     }
 
-    Datastore getDatastore() {
-      return datastore;
-    }
-
-    TransactionOptions getOptions() {
-      return options;
-    }
-
-    Transaction getTransaction() {
-      return transaction;
-    }
-
     void setPrevTransactionId(ByteString transactionId) {
       TransactionOptions.ReadWrite readWrite =
           TransactionOptions.ReadWrite.newBuilder().setPreviousTransaction(transactionId).build();
@@ -221,19 +206,19 @@ public T call() throws DatastoreException {
         return value;
       } catch (Exception ex) {
         attemptStatus = DatastoreException.extractStatusCode(ex);
-        // An exception here can originate from either callable.run() (before commit was attempted)
-        // or from transaction.commit() itself. In both cases the transaction is still active.
-        // isActive() returns false if the transaction was already committed or rolled back, so
-        // it is safe to use as the sole guard here without tracking a separate committed flag.
+        // An exception here can stem from either `callable.run()` (before commit was attempted)
+        // or from `transaction.commit()`. If there is an exception thrown from either call site,
+        // then the transaction is still active. Check if it is still active (e.g. not commited)
+        // and roll back the transaction.
         if (transaction.isActive()) {
           transaction.rollback();
         }
         throw DatastoreException.propagateUserException(ex);
       } finally {
         recordAttempt(attemptStatus);
-        // transaction.isActive() returns false after both a successful commit or a completed
-        // rollback, so it already guards against rolling back a committed transaction or
-        // rolling back a transaction that has already been rolled back.
+        // 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).
         if (transaction.isActive()) {
           transaction.rollback();
         }

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

@@ -26,8 +26,9 @@
 /**
  * Interface to record specific metric operations.
  *
- * <p><b>Warning:</b> This is an internal API and is not intended for external use. Do not implement
- * or extend this interface.
+ * <p><b>Warning:</b> This is intended to be an internal API and is not intended for external use.
+ * This is public solely for implementation purposes and does not promise any backwards
+ * compatibility.
  */
 @InternalExtensionOnly
 public interface MetricsRecorder {

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

@@ -41,14 +41,14 @@ class OpenTelemetryMetricsRecorder implements MetricsRecorder {
 
     this.transactionLatency =
         meter
-            .histogramBuilder(TelemetryConstants.SERVICE_NAME + "/transaction_latency")
+            .histogramBuilder(TelemetryConstants.METRIC_NAME_TRANSACTION_LATENCY)
             .setDescription("Total latency of transaction operations")
             .setUnit("ms")
             .build();
 
     this.transactionAttemptCount =
         meter
-            .counterBuilder(TelemetryConstants.SERVICE_NAME + "/transaction_attempt_count")
+            .counterBuilder(TelemetryConstants.METRIC_NAME_TRANSACTION_ATTEMPT_COUNT)
             .setDescription("Number of attempts to commit a transaction")
             .build();
   }

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

@@ -45,6 +45,14 @@ public class TelemetryConstants {
   /** Attribute key for the Datastore database ID. */
   public static final String ATTRIBUTES_KEY_DATABASE_ID = "database_id";
 
+  /** Metric name for the total latency of a transaction. */
+  public static final String METRIC_NAME_TRANSACTION_LATENCY =
+      SERVICE_NAME + "/transaction_latency";
+
+  /** Metric name for the number of attempts a transaction took. */
+  public static final String METRIC_NAME_TRANSACTION_ATTEMPT_COUNT =
+      SERVICE_NAME + "/transaction_attempt_count";
+
   /* TODO(lawrenceqiu): For now, these are a duplicate of method names in TraceUtil. Those will use these eventually */
   // Format is not SnakeCase to keep backward compatibility with the existing values TraceUtil spans
   public static final String METHOD_ALLOCATE_IDS = "AllocateIds";