impl(o11y): introduce http.request.method and span name for HTTP requests

gax-java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonCallableFactory.java

@@ -45,7 +45,6 @@
 import com.google.api.gax.rpc.UnaryCallSettings;
 import com.google.api.gax.rpc.UnaryCallable;
 import com.google.api.gax.tracing.ApiTracerContext;
-import com.google.api.gax.tracing.SpanName;
 import com.google.api.gax.tracing.TracedUnaryCallable;
 import javax.annotation.Nonnull;
 
@@ -83,7 +82,7 @@ static <RequestT, ResponseT> UnaryCallable<RequestT, ResponseT> createUnaryCalla
         new TracedUnaryCallable<>(
             callable,
             clientContext.getTracerFactory(),
-            getSpanName(httpJsonCallSettings.getMethodDescriptor()));
+            getApiTracerContext(httpJsonCallSettings.getMethodDescriptor()));
     return callable.withDefaultCallContext(clientContext.getDefaultCallContext());
   }
 
@@ -222,13 +221,13 @@ ServerStreamingCallable<RequestT, ResponseT> createServerStreamingCallable(
   }
 
   @InternalApi("Visible for testing")
-  static SpanName getSpanName(@Nonnull ApiMethodDescriptor<?, ?> methodDescriptor) {
-    ApiTracerContext apiTracerContext =
-        ApiTracerContext.newBuilder()
-            .setFullMethodName(methodDescriptor.getFullMethodName())
-            .setTransport(ApiTracerContext.Transport.HTTP)
-            .setLibraryMetadata(LibraryMetadata.empty())
-            .build();
-    return SpanName.of(apiTracerContext);
+  static ApiTracerContext getApiTracerContext(@Nonnull ApiMethodDescriptor<?, ?> methodDescriptor) {
+    return ApiTracerContext.newBuilder()
+        .setFullMethodName(methodDescriptor.getFullMethodName())
+        .setHttpMethod(methodDescriptor.getHttpMethod())
+        .setHttpPathTemplate(methodDescriptor.getRequestFormatter().getPathTemplate().toRawString())
+        .setTransport(ApiTracerContext.Transport.HTTP)
+        .setLibraryMetadata(LibraryMetadata.empty())
+        .build();
   }
 }

gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonCallableFactoryTest.java

@@ -33,7 +33,9 @@
 import static com.google.common.truth.Truth.assertWithMessage;
 
 import com.google.api.client.http.HttpMethods;
+import com.google.api.gax.tracing.ApiTracerContext;
 import com.google.api.gax.tracing.SpanName;
+import com.google.api.pathtemplate.PathTemplate;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import java.util.List;
@@ -43,8 +45,15 @@
 import org.mockito.Mockito;
 
 class HttpJsonCallableFactoryTest {
+  private HttpRequestFormatter createMockRequestFormatter() {
+    HttpRequestFormatter formatter = Mockito.mock(HttpRequestFormatter.class);
+    PathTemplate template = PathTemplate.create("/test/path/template");
+    Mockito.when(formatter.getPathTemplate()).thenReturn(template);
+    return formatter;
+  }
+
   @Test
-  void testGetSpanName() {
+  void testGetApiTracerContext() {
     Map<String, SpanName> validNames =
         ImmutableMap.of(
             "google.cloud.service.v1.CoolService/CoolRPC", SpanName.of("CoolService", "CoolRPC"),
@@ -56,17 +65,19 @@ void testGetSpanName() {
           ApiMethodDescriptor.newBuilder()
               .setFullMethodName(entry.getKey())
               .setHttpMethod(HttpMethods.POST)
-              .setRequestFormatter(Mockito.mock(HttpRequestFormatter.class))
+              .setRequestFormatter(createMockRequestFormatter())
               .setResponseParser(Mockito.mock(HttpResponseParser.class))
               .build();
 
-      SpanName actualSpanName = HttpJsonCallableFactory.getSpanName(descriptor);
-      assertThat(actualSpanName).isEqualTo(entry.getValue());
+      ApiTracerContext context = HttpJsonCallableFactory.getApiTracerContext(descriptor);
+      SpanName actual = SpanName.of(context);
+      assertThat(actual.getClientName()).isEqualTo(entry.getValue().getClientName());
+      assertThat(actual.getMethodName()).isEqualTo(entry.getValue().getMethodName());
     }
   }
 
   @Test
-  void testGetSpanNameInvalid() {
+  void testGetApiTracerContextInvalid() {
     List<String> invalidNames = ImmutableList.of("no_split", ".no_client");
 
     for (String invalidName : invalidNames) {
@@ -75,16 +86,16 @@ void testGetSpanNameInvalid() {
           ApiMethodDescriptor.newBuilder()
               .setFullMethodName(invalidName)
               .setHttpMethod(HttpMethods.POST)
-              .setRequestFormatter(Mockito.mock(HttpRequestFormatter.class))
+              .setRequestFormatter(createMockRequestFormatter())
               .setResponseParser(Mockito.mock(HttpResponseParser.class))
               .build();
 
       IllegalArgumentException actualError = null;
       try {
-        SpanName spanName = HttpJsonCallableFactory.getSpanName(descriptor);
+        ApiTracerContext context = HttpJsonCallableFactory.getApiTracerContext(descriptor);
+        SpanName.of(context);
         assertWithMessage(
-                "Invalid method descriptor should not have a valid span name: %s should not generate the spanName: %s",
-                invalidName, spanName)
+                "Invalid method descriptor should not have a valid client name: %s", invalidName)
             .fail();
       } catch (IllegalArgumentException e) {
         actualError = e;

gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/RetryingTest.java

@@ -55,6 +55,7 @@
 import com.google.api.gax.rpc.UnaryCallSettings;
 import com.google.api.gax.rpc.UnaryCallable;
 import com.google.api.gax.rpc.UnknownException;
+import com.google.api.pathtemplate.PathTemplate;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.UncheckedExecutionException;
@@ -75,7 +76,7 @@ class RetryingTest {
       ApiMethodDescriptor.newBuilder()
           .setFullMethodName("google.cloud.v1.Fake/FakeMethodForRequestMutator")
           .setHttpMethod(HttpMethods.POST)
-          .setRequestFormatter(Mockito.mock(HttpRequestFormatter.class))
+          .setRequestFormatter(createMockRequestFormatter())
           .setResponseParser(Mockito.mock(HttpResponseParser.class))
           .build();
 
@@ -113,6 +114,13 @@ class RetryingTest {
           .setTotalTimeoutDuration(java.time.Duration.ofMillis(10L))
           .build();
 
+  private HttpRequestFormatter createMockRequestFormatter() {
+    HttpRequestFormatter formatter = Mockito.mock(HttpRequestFormatter.class);
+    PathTemplate template = PathTemplate.create("/test/path/template");
+    Mockito.when(formatter.getPathTemplate()).thenReturn(template);
+    return formatter;
+  }
+
   @BeforeEach
   void resetClock() {
     fakeClock = new FakeApiClock(System.nanoTime());

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

@@ -136,6 +136,26 @@ String rpcSystemName() {
   @Nullable
   public abstract OperationType operationType();
 
+  /**
+   * Returns the HTTP method used for the RPC, in case the RPC is an HttpJson method.
+   *
+   * <p>Example: {@code PATCH}.
+   *
+   * @return the HTTP method, or {@code null} if not set
+   */
+  @Nullable
+  abstract String httpMethod();
+
+  /**
+   * Returns the HTTP path template used for the RPC, in case the RPC is an HttpJson method.
+   *
+   * <p>Example: {@code /users/{user_id}/get}.
+   *
+   * @return the HTTP path template, or {@code null} if not set
+   */
+  @Nullable
+  abstract String httpPathTemplate();
+
   /**
    * @return a map of attributes to be included in attempt-level spans
    */
@@ -159,8 +179,16 @@ public Map<String, Object> getAttemptAttributes() {
             ObservabilityAttributes.ARTIFACT_ATTRIBUTE, libraryMetadata().artifactName());
       }
     }
-    if (transport() == Transport.GRPC && !Strings.isNullOrEmpty(fullMethodName())) {
-      attributes.put(ObservabilityAttributes.GRPC_RPC_METHOD_ATTRIBUTE, fullMethodName());
+    if (transport() == Transport.GRPC) {
+      if (!Strings.isNullOrEmpty(fullMethodName())) {
+        attributes.put(ObservabilityAttributes.GRPC_RPC_METHOD_ATTRIBUTE, fullMethodName());
+      }
+      if (Strings.isNullOrEmpty(fullMethodName())) {
+        attributes.put(ObservabilityAttributes.GRPC_RPC_METHOD_ATTRIBUTE, fullMethodName());
+      }
+    }
+    if (transport() == Transport.HTTP && !Strings.isNullOrEmpty(httpMethod())) {
+      attributes.put(ObservabilityAttributes.HTTP_METHOD_ATTRIBUTE, httpMethod());
     }
     return attributes;
   }
@@ -188,6 +216,12 @@ ApiTracerContext merge(ApiTracerContext other) {
     if (other.transport() != null) {
       builder.setTransport(other.transport());
     }
+    if (!Strings.isNullOrEmpty(other.httpMethod())) {
+      builder.setHttpMethod(other.httpMethod());
+    }
+    if (!Strings.isNullOrEmpty(other.httpPathTemplate())) {
+      builder.setHttpPathTemplate(other.httpPathTemplate());
+    }
     if (other.operationType() != null) {
       builder.setOperationType(other.operationType());
     }
@@ -218,6 +252,10 @@ public abstract static class Builder {
 
     public abstract Builder setServerPort(@Nullable Integer serverPort);
 
+    public abstract Builder setHttpMethod(@Nullable String httpMethod);
+
+    public abstract Builder setHttpPathTemplate(@Nullable String rawString);
+
     public abstract ApiTracerContext build();
   }
 }

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

@@ -59,6 +59,9 @@ public class ObservabilityAttributes {
   /** The RPC system name, e.g. 'grpc' or 'http'. */
   public static final String RPC_SYSTEM_NAME_ATTRIBUTE = "rpc.system.name";
 
+  /** The HTTP method of the request (e.g., "GET"). Only used in HTTP transport. */
+  public static final String HTTP_METHOD_ATTRIBUTE = "http.request.method";
+
   /**
    * The error codes of the request. The value will be the string representation of the canonical
    * gRPC status code (e.g., "OK", "INTERNAL").

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

@@ -33,6 +33,7 @@
 import com.google.api.core.BetaApi;
 import com.google.api.core.InternalApi;
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
 
 /**
  * A {@link ApiTracerFactory} to build instances of {@link SpanTracer}.
@@ -77,19 +78,20 @@ public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType op
 
   @Override
   public ApiTracer newTracer(ApiTracer parent, ApiTracerContext apiTracerContext) {
-    ApiTracerContext context = this.apiTracerContext.merge(apiTracerContext);
+    ApiTracerContext mergedContext = this.apiTracerContext.merge(apiTracerContext);
 
     String attemptSpanName;
-    if (context.transport() == ApiTracerContext.Transport.GRPC) {
-      attemptSpanName = context.fullMethodName();
+    if (mergedContext.transport() == ApiTracerContext.Transport.GRPC) {
+      attemptSpanName = mergedContext.fullMethodName();
     } else {
-      // TODO(diegomarquezp): this is a placeholder for the HTTP span name and will be adjusted as
-      // the
-      // feature is developed.
-      attemptSpanName = context.fullMethodName() + "/attempt";
+      Preconditions.checkNotNull(mergedContext.httpMethod(), "HTTP method cannot be null");
+      Preconditions.checkNotNull(
+          mergedContext.httpPathTemplate(), "HTTP path template cannot be null");
+      attemptSpanName =
+          String.format("%s %s", mergedContext.httpMethod(), mergedContext.httpPathTemplate());
     }
 
-    return new SpanTracer(traceManager, context, attemptSpanName);
+    return new SpanTracer(traceManager, mergedContext, attemptSpanName);
   }
 
   @Override

gax-java/gax/src/test/java/com/google/api/gax/tracing/ApiTracerContextTest.java

@@ -74,6 +74,32 @@ void testGetAttemptAttributes_artifact() {
         .containsEntry(ObservabilityAttributes.ARTIFACT_ATTRIBUTE, "test-artifact");
   }
 
+  @Test
+  void testGetAttemptAttributes_httpMethod() {
+    ApiTracerContext context =
+        ApiTracerContext.newBuilder()
+            .setLibraryMetadata(LibraryMetadata.empty())
+            .setTransport(ApiTracerContext.Transport.HTTP)
+            .setHttpMethod("POST")
+            .build();
+    Map<String, Object> attributes = context.getAttemptAttributes();
+
+    assertThat(attributes).containsEntry(ObservabilityAttributes.HTTP_METHOD_ATTRIBUTE, "POST");
+  }
+
+  @Test
+  void testGetAttemptAttributes_httpMethod_notHttpTransport() {
+    ApiTracerContext context =
+        ApiTracerContext.newBuilder()
+            .setLibraryMetadata(LibraryMetadata.empty())
+            .setTransport(ApiTracerContext.Transport.GRPC)
+            .setHttpMethod("POST")
+            .build();
+    Map<String, Object> attributes = context.getAttemptAttributes();
+
+    assertThat(attributes).doesNotContainKey(ObservabilityAttributes.HTTP_METHOD_ATTRIBUTE);
+  }
+
   @Test
   void testGetAttemptAttributes_fullMethodName_noTransport_notPresent() {
     ApiTracerContext context =
@@ -181,4 +207,22 @@ void testMerge_emptyOther() {
 
     assertThat(merged).isEqualTo(context1);
   }
+
+  @Test
+  void testMerge_httpFields() {
+    ApiTracerContext context1 =
+        ApiTracerContext.newBuilder()
+            .setLibraryMetadata(LibraryMetadata.empty())
+            .setHttpMethod("GET")
+            .build();
+    ApiTracerContext context2 =
+        ApiTracerContext.newBuilder()
+            .setLibraryMetadata(LibraryMetadata.empty())
+            .setHttpPathTemplate("v1/projects/{project}")
+            .build();
+
+    ApiTracerContext merged = context1.merge(context2);
+    assertThat(merged.httpMethod()).isEqualTo("GET");
+    assertThat(merged.httpPathTemplate()).isEqualTo("v1/projects/{project}");
+  }
 }

gax-java/gax/src/test/java/com/google/api/gax/tracing/SpanTracerFactoryTest.java

@@ -46,6 +46,7 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 import org.junit.jupiter.params.provider.ValueSource;
 import org.mockito.ArgumentCaptor;
 
@@ -196,12 +197,21 @@ void testNewTracer_withContext_grpc_usesFullMethodName() {
     verify(traceManager).createSpan(eq("google.cloud.v1.Service/Method"), anyMap());
   }
 
-  @Test
-  void testNewTracer_withContext_http_usesPlaceholder() {
+  @ParameterizedTest
+  @CsvSource({
+    "POST, v1/projects/{project}/methods, POST v1/projects/{project}/methods",
+    "GET, v1/projects/{project}/methods/{method}, GET v1/projects/{project}/methods/{method}",
+    "DELETE, v1/projects/{project}/methods/{method}, DELETE v1/projects/{project}/methods/{method}",
+    "PATCH, v1/projects/{project}/methods/{method}, PATCH v1/projects/{project}/methods/{method}"
+  })
+  void testNewTracer_withContext_http_usesHttpMethodAndPathTemplate(
+      String httpMethod, String httpPathTemplate, String expectedSpanName) {
     ApiTracerContext context =
         ApiTracerContext.newBuilder()
             .setFullMethodName("google.cloud.v1.Service.Method")
             .setTransport(Transport.HTTP)
+            .setHttpMethod(httpMethod)
+            .setHttpPathTemplate(httpPathTemplate)
             .setLibraryMetadata(LibraryMetadata.empty())
             .build();
 
@@ -210,7 +220,7 @@ void testNewTracer_withContext_http_usesPlaceholder() {
 
     tracer.attemptStarted(null, 1);
 
-    verify(traceManager).createSpan(eq("google.cloud.v1.Service.Method/attempt"), anyMap());
+    verify(traceManager).createSpan(eq(expectedSpanName), anyMap());
   }
 
   @Test

java-showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelTracing.java

@@ -154,9 +154,10 @@ void testTracing_successfulEcho_httpjson() throws Exception {
 
       SpanData attemptSpan =
           spans.stream()
-              .filter(span -> span.getName().equals("Echo/Echo/attempt"))
+              .filter(span -> span.getName().equals("POST v1beta1/echo:echo"))
               .findFirst()
-              .orElseThrow(() -> new AssertionError("Attempt span 'Echo/Echo/attempt' not found"));
+              .orElseThrow(
+                  () -> new AssertionError("Attempt span 'POST v1beta1/echo:echo' not found"));
       assertThat(attemptSpan.getKind()).isEqualTo(SpanKind.CLIENT);
       assertThat(
               attemptSpan
@@ -183,6 +184,11 @@ void testTracing_successfulEcho_httpjson() throws Exception {
                   .getAttributes()
                   .get(AttributeKey.stringKey(ObservabilityAttributes.ARTIFACT_ATTRIBUTE)))
           .isEqualTo(SHOWCASE_ARTIFACT);
+      assertThat(
+              attemptSpan
+                  .getAttributes()
+                  .get(AttributeKey.stringKey(ObservabilityAttributes.HTTP_METHOD_ATTRIBUTE)))
+          .isEqualTo("POST");
     }
   }
 }