Merge branch 'main' into feat/actionable-errors-logging-utils

Author: westarle

.github/workflows/sonar.yaml

@@ -8,6 +8,7 @@ on:
 jobs:
   build:
     name: Build
+    if: github.event.pull_request.head.repo.full_name == github.repository || github.event_name != 'pull_request'
     runs-on: ubuntu-22.04
     steps:
       - uses: actions/checkout@v3

gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java

@@ -693,13 +693,8 @@ ChannelCredentials createS2ASecuredChannelCredentials() {
     return s2aChannelCredentials;
   }
 
-  private ManagedChannel createSingleChannel() throws IOException {
-    GrpcHeaderInterceptor headerInterceptor =
-        new GrpcHeaderInterceptor(headersWithDuplicatesRemoved);
-
-    GrpcMetadataHandlerInterceptor metadataHandlerInterceptor =
-        new GrpcMetadataHandlerInterceptor();
-
+  @InternalApi("For internal use by google-cloud-java clients only")
+  public ManagedChannelBuilder<?> createChannelBuilder() throws IOException {
     int colon = endpoint.lastIndexOf(':');
     if (colon < 0) {
       throw new IllegalStateException("invalid endpoint - should have been validated: " + endpoint);
@@ -775,8 +770,19 @@ private ManagedChannel createSingleChannel() throws IOException {
       // See https://github.com/googleapis/gapic-generator/issues/2816
       builder.disableServiceConfigLookUp();
     }
-    builder =
-        builder
+    return builder;
+  }
+
+  @InternalApi("For internal use by google-cloud-java clients only")
+  public ManagedChannelBuilder<?> createDecoratedChannelBuilder() throws IOException {
+    GrpcHeaderInterceptor headerInterceptor =
+        new GrpcHeaderInterceptor(headersWithDuplicatesRemoved);
+
+    GrpcMetadataHandlerInterceptor metadataHandlerInterceptor =
+        new GrpcMetadataHandlerInterceptor();
+
+    ManagedChannelBuilder<?> builder =
+        createChannelBuilder()
             .intercept(new GrpcChannelUUIDInterceptor())
             .intercept(new GrpcLoggingInterceptor())
             .intercept(headerInterceptor)
@@ -806,6 +812,12 @@ private ManagedChannel createSingleChannel() throws IOException {
       builder = channelConfigurator.apply(builder);
     }
 
+    return builder;
+  }
+
+  private ManagedChannel createSingleChannel() throws IOException {
+    ManagedChannelBuilder<?> builder = createDecoratedChannelBuilder();
+
     ManagedChannel managedChannel = builder.build();
     if (channelPrimer != null) {
       channelPrimer.primeChannel(managedChannel);
@@ -1422,7 +1434,7 @@ public ApiFunction<ManagedChannelBuilder, ManagedChannelBuilder> getChannelConfi
   }
 
   private static ImmutableMap<String, ?> getDefaultDirectPathServiceConfig() {
-    // When channel pooling is enabled, force the pick_first grpclb strategy.
+    // When channel pooling is enabled, force the pick_first strategy.
     // This is necessary to avoid the multiplicative effect of creating channel pool with
     // `poolSize` number of `ManagedChannel`s, each with a `subSetting` number of number of
     // subchannels.
@@ -1431,13 +1443,8 @@ public ApiFunction<ManagedChannelBuilder, ManagedChannelBuilder> getChannelConfi
     ImmutableMap<String, Object> pickFirstStrategy =
         ImmutableMap.<String, Object>of("pick_first", ImmutableMap.of());
 
-    ImmutableMap<String, Object> childPolicy =
-        ImmutableMap.<String, Object>of("childPolicy", ImmutableList.of(pickFirstStrategy));
-
-    ImmutableMap<String, Object> grpcLbPolicy =
-        ImmutableMap.<String, Object>of("grpclb", childPolicy);
-
-    return ImmutableMap.<String, Object>of("loadBalancingConfig", ImmutableList.of(grpcLbPolicy));
+    return ImmutableMap.<String, Object>of(
+        "loadBalancingConfig", ImmutableList.of(pickFirstStrategy));
   }
 
   private static void validateEndpoint(String endpoint) {

gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java

@@ -235,7 +235,7 @@ void testToBuilder() {
         builder -> {
           throw new UnsupportedOperationException();
         };
-    Map<String, ?> directPathServiceConfig = ImmutableMap.of("loadbalancingConfig", "grpclb");
+    Map<String, ?> directPathServiceConfig = ImmutableMap.of("loadBalancingConfig", "pick_first");
     List<InstantiatingGrpcChannelProvider.HardBoundTokenTypes> hardBoundTokenTypes =
         new ArrayList<>();
     hardBoundTokenTypes.add(InstantiatingGrpcChannelProvider.HardBoundTokenTypes.ALTS);
@@ -549,11 +549,7 @@ void testWithDefaultDirectPathServiceConfig() {
     List<Map<String, ?>> lbConfigs = getAsObjectList(defaultServiceConfig, "loadBalancingConfig");
     assertThat(lbConfigs).hasSize(1);
     Map<String, ?> lbConfig = lbConfigs.get(0);
-    Map<String, ?> grpclb = getAsObject(lbConfig, "grpclb");
-    List<Map<String, ?>> childPolicies = getAsObjectList(grpclb, "childPolicy");
-    assertThat(childPolicies).hasSize(1);
-    Map<String, ?> childPolicy = childPolicies.get(0);
-    assertThat(childPolicy.keySet()).containsExactly("pick_first");
+    assertThat(lbConfig.keySet()).containsExactly("pick_first");
   }
 
   @Nullable
@@ -599,10 +595,10 @@ void testWithCustomDirectPathServiceConfig() {
     ImmutableMap<String, Object> childPolicy =
         ImmutableMap.<String, Object>of(
             "childPolicy", ImmutableList.of(pickFirstStrategy), "foo", "bar");
-    ImmutableMap<String, Object> grpcLbPolicy =
-        ImmutableMap.<String, Object>of("grpclb", childPolicy);
+    ImmutableMap<String, Object> customLbPolicy =
+        ImmutableMap.<String, Object>of("my_custom_lb", childPolicy);
     Map<String, Object> passedServiceConfig = new HashMap<>();
-    passedServiceConfig.put("loadBalancingConfig", ImmutableList.of(grpcLbPolicy));
+    passedServiceConfig.put("loadBalancingConfig", ImmutableList.of(customLbPolicy));
 
     InstantiatingGrpcChannelProvider provider =
         InstantiatingGrpcChannelProvider.newBuilder()

gax-java/gax/src/main/java/com/google/api/gax/rpc/LibraryMetadata.java

@@ -65,6 +65,9 @@ public abstract class LibraryMetadata {
   @Nullable
   public abstract String artifactName();
 
+  @Nullable
+  public abstract String version();
+
   public static LibraryMetadata empty() {
     return newBuilder().build();
   }
@@ -83,6 +86,8 @@ public abstract static class Builder {
 
     public abstract Builder setArtifactName(@Nullable String artifactName);
 
+    public abstract Builder setVersion(@Nullable String version);
+
     public abstract LibraryMetadata build();
   }
 }

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

@@ -136,6 +136,18 @@ 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();
+
   /**
    * @return a map of attributes to be included in attempt-level spans
    */
@@ -165,6 +177,34 @@ public Map<String, Object> getAttemptAttributes() {
     return attributes;
   }
 
+  Map<String, Object> getMetricsAttributes() {
+    Map<String, Object> attributes = new HashMap<>();
+    if (!Strings.isNullOrEmpty(serverAddress())) {
+      attributes.put(ObservabilityAttributes.SERVER_ADDRESS_ATTRIBUTE, serverAddress());
+    }
+    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 (!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 (!Strings.isNullOrEmpty(urlTemplate())) {
+        attributes.put(ObservabilityAttributes.URL_TEMPLATE_ATTRIBUTE, urlTemplate());
+      }
+    }
+    return attributes;
+  }
+
   /**
    * Merges this context with another context. The values in the other context take precedence.
    *
@@ -191,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();
   }
 
@@ -218,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/GoldenSignalsMetricsRecorder.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/GoldenSignalsMetricsTracer.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/GoldenSignalsMetricsTracerFactory.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/ObservabilityAttributes.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/ObservabilityUtils.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();
   }
 }