fix: Add more attributes. Refactor attributes from String to Object for metrics.

Author: blakeli0

gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerContext.java

@@ -38,17 +38,6 @@
 import java.util.HashMap;
 import java.util.Map;
 import javax.annotation.Nullable;
-//gcp.client.service [consistent across T4 spans]
-//gcp.client.version [consistent across T4 spans]
-//        rpc.system.name (e.g., grpc, http) [consistent across T4 spans]
-//        rpc.response.status_code (for gRPC and HTTP) [from last T4 span]
-//        rpc.method (for gRPC and HTTP) [from the last T4 span]
-//url.domain [consistent across T4 spans]
-//        url.template (for HTTP) [from the first T4 span]
-//        http.response.status_code (for HTTP) [from the last T4 span]
-//server.address [from the last T4 span]
-//server.port [from the last T4 span]
-//        error.type (if the overall T3 operation failed)
 
 /**
  * A context object that contains information used to infer attributes that are common for all
@@ -147,12 +136,15 @@ String rpcSystemName() {
   @Nullable
   public abstract OperationType operationType();
 
+  /** The service name of a client (e.g. "bigtable", "spanner"). */
   @Nullable
   public abstract String serviceName();
 
+  /** The url domain of the request (e.g. "pubsub.googleapis.com"). */
   @Nullable
   public abstract String urlDomain();
 
+  /** The url template of the request (e.g. /v1/{name}:access). */
   @Nullable
   public abstract String urlTemplate();
 
@@ -185,20 +177,29 @@ public Map<String, Object> getAttemptAttributes() {
     return attributes;
   }
 
-  Map<String, String> getMetricsAttributes() {
-    Map<String, String> attributes = new HashMap<>();
-    if (serverAddress() != null) {
+  Map<String, Object> getMetricsAttributes() {
+    Map<String, Object> attributes = new HashMap<>();
+    if (!Strings.isNullOrEmpty(serverAddress())) {
       attributes.put(ObservabilityAttributes.SERVER_ADDRESS_ATTRIBUTE, serverAddress());
     }
-    if (serviceName() != null) {
-      attributes.put("gcp.client.service", serviceName());
+    if (serverPort() != null) {
+      attributes.put(ObservabilityAttributes.SERVER_PORT_ATTRIBUTE, serverPort());
+    }
+    if (!Strings.isNullOrEmpty(serviceName())) {
+      attributes.put(ObservabilityAttributes.GCP_CLIENT_SERVICE_ATTRIBUTE, serviceName());
+    }
+    if (!Strings.isNullOrEmpty(rpcSystemName())) {
+      attributes.put(ObservabilityAttributes.RPC_SYSTEM_NAME_ATTRIBUTE, rpcSystemName());
     }
-    if (transport() == HTTP) {
-      if (urlDomain() != null) {
-        attributes.put("url.domain", serviceName());
+    if (!Strings.isNullOrEmpty(fullMethodName())) {
+      attributes.put(ObservabilityAttributes.GRPC_RPC_METHOD_ATTRIBUTE, fullMethodName());
+    }
+    if (transport() == Transport.HTTP) {
+      if (!Strings.isNullOrEmpty(urlDomain())) {
+        attributes.put(ObservabilityAttributes.URL_DOMAIN_ATTRIBUTE, urlDomain());
       }
-      if (serviceName() != null) {
-        attributes.put("url.template", serviceName());
+      if (!Strings.isNullOrEmpty(urlTemplate())) {
+        attributes.put(ObservabilityAttributes.URL_TEMPLATE_ATTRIBUTE, urlTemplate());
       }
     }
     return attributes;
@@ -230,6 +231,15 @@ ApiTracerContext merge(ApiTracerContext other) {
     if (other.operationType() != null) {
       builder.setOperationType(other.operationType());
     }
+    if (other.serviceName() != null) {
+      builder.setServiceName(other.serviceName());
+    }
+    if (other.urlDomain() != null) {
+      builder.setUrlDomain(other.urlDomain());
+    }
+    if (other.urlTemplate() != null) {
+      builder.setUrlTemplate(other.urlTemplate());
+    }
     return builder.build();
   }
 
@@ -257,6 +267,12 @@ public abstract static class Builder {
 
     public abstract Builder setServerPort(@Nullable Integer serverPort);
 
+    public abstract Builder setServiceName(@Nullable String serviceName);
+
+    public abstract Builder setUrlDomain(@Nullable String urlDomain);
+
+    public abstract Builder setUrlTemplate(@Nullable String urlTemplate);
+
     public abstract ApiTracerContext build();
   }
 }

gax-java/gax/src/main/java/com/google/api/gax/tracing/GoldenSignalMetricsTracer.java

@@ -65,7 +65,7 @@ class GoldenSignalsMetricsRecorder {
             .build();
   }
 
-  void recordOperationLatency(double operationLatency, Map<String, String> attributes) {
+  void recordOperationLatency(double operationLatency, Map<String, Object> attributes) {
     clientRequestDurationRecorder.record(
         operationLatency, ObservabilityUtils.toOtelAttributes(attributes));
   }

gax-java/gax/src/main/java/com/google/api/gax/tracing/GoldenSignalMetricsTracerFactory.java

@@ -48,26 +48,23 @@
 class GoldenSignalsMetricsTracer implements ApiTracer {
   private final Stopwatch clientRequestTimer;
   private final GoldenSignalsMetricsRecorder metricsRecorder;
-  private final Map<String, String> attributes = new HashMap<>();
+  private final Map<String, Object> attributes;
 
-  /**
-   * Creates the following instruments for the following metrics:
-   *
-   * <ul>
-   *   <li>Client Request Duration: Histogram
-   * </ul>
-   *
-   * @param metricsRecorder OpenTelemetry
-   */
-  GoldenSignalsMetricsTracer(GoldenSignalsMetricsRecorder metricsRecorder) {
+  GoldenSignalsMetricsTracer(
+      GoldenSignalsMetricsRecorder metricsRecorder, ApiTracerContext apiTracerContext) {
     this.clientRequestTimer = Stopwatch.createStarted();
     this.metricsRecorder = metricsRecorder;
+    this.attributes = apiTracerContext.getMetricsAttributes();
   }
 
   @VisibleForTesting
-  GoldenSignalsMetricsTracer(GoldenSignalsMetricsRecorder metricsRecorder, Ticker ticker) {
+  GoldenSignalsMetricsTracer(
+      GoldenSignalsMetricsRecorder metricsRecorder,
+      ApiTracerContext apiTracerContext,
+      Ticker ticker) {
     this.clientRequestTimer = Stopwatch.createStarted(ticker);
     this.metricsRecorder = metricsRecorder;
+    this.attributes = new HashMap<>(apiTracerContext.getMetricsAttributes());
   }
 
   /**

gax-java/gax/src/main/java/com/google/api/gax/tracing/GoldenSignalsMetricsRecorder.java

@@ -48,6 +48,7 @@ public class GoldenSignalsMetricsTracerFactory implements ApiTracerFactory {
 
   public GoldenSignalsMetricsTracerFactory(OpenTelemetry openTelemetry) {
     this.openTelemetry = openTelemetry;
+    this.apiTracerContext = ApiTracerContext.empty();
   }
 
   @Override
@@ -57,7 +58,7 @@ public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType op
       // regular requests.
       return new BaseApiTracer();
     }
-    return new GoldenSignalsMetricsTracer(metricsRecorder);
+    return new GoldenSignalsMetricsTracer(metricsRecorder, apiTracerContext);
   }
 
   @Override

gax-java/gax/src/main/java/com/google/api/gax/tracing/GoldenSignalsMetricsTracer.java

@@ -64,4 +64,13 @@ public class ObservabilityAttributes {
    * gRPC status code (e.g., "OK", "INTERNAL").
    */
   public static final String RPC_RESPONSE_STATUS_ATTRIBUTE = "rpc.response.status_code";
+
+  /** The service name of a client (e.g. "bigtable", "spanner"). */
+  public static final String GCP_CLIENT_SERVICE_ATTRIBUTE = "gcp.client.service";
+
+  /** The url domain of the request (e.g. "pubsub.googleapis.com"). */
+  public static final String URL_DOMAIN_ATTRIBUTE = "url.domain";
+
+  /** The url template of the request (e.g. /v1/{name}:access). */
+  public static final String URL_TEMPLATE_ATTRIBUTE = "url.template";
 }

gax-java/gax/src/main/java/com/google/api/gax/tracing/GoldenSignalsMetricsTracerFactory.java

@@ -31,7 +31,6 @@
 
 import com.google.api.gax.rpc.ApiException;
 import com.google.api.gax.rpc.StatusCode;
-import com.google.common.base.Preconditions;
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.common.AttributesBuilder;
 import java.util.Map;
@@ -57,10 +56,19 @@ static String extractStatus(@Nullable Throwable error) {
     return statusString;
   }
 
-  static Attributes toOtelAttributes(Map<String, String> attributes) {
-    Preconditions.checkNotNull(attributes, "Attributes map cannot be null");
+  static Attributes toOtelAttributes(Map<String, Object> attributes) {
     AttributesBuilder attributesBuilder = Attributes.builder();
-    attributes.forEach(attributesBuilder::put);
+    if (attributes == null) {
+      return attributesBuilder.build();
+    }
+    attributes.forEach(
+        (k, v) -> {
+          if (v instanceof String) {
+            attributesBuilder.put(k, (String) v);
+          } else if (v instanceof Integer) {
+            attributesBuilder.put(k, (long) (Integer) v);
+          }
+        });
     return attributesBuilder.build();
   }
 }