address review comments

Author: laurit

instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/SpringRestTemplateTest.java

@@ -6,15 +6,18 @@
 package io.opentelemetry.javaagent.instrumentation.spring.web.v6_0;
 
 import static java.util.Collections.singletonList;
+import static org.assertj.core.api.Assertions.assertThat;
 
 import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
 import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest;
 import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension;
 import io.opentelemetry.instrumentation.testing.junit.http.HttpClientResult;
 import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions;
+import io.opentelemetry.sdk.trace.data.StatusData;
 import java.io.InputStream;
 import java.net.URI;
 import java.util.Map;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 import org.springframework.http.HttpEntity;
 import org.springframework.http.HttpHeaders;
@@ -100,6 +103,41 @@ protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
 
     // no enum value for TEST
     optionsBuilder.disableTestNonStandardHttpMethod();
-    optionsBuilder.setExpectedClientSpanNameMapper((uri, method) -> method + " " + uri.getPath());
+    optionsBuilder.setExpectedClientSpanNameMapper(
+        (uri, method) -> method + " " + getTemplate(uri));
+    optionsBuilder.setExpectedUrlTemplateMapper(SpringRestTemplateTest::getTemplate);
+    optionsBuilder.setHasUrlTemplate(true);
+  }
+
+  private static String getTemplate(URI uri) {
+    String path = uri.getPath();
+    if (path.startsWith("/hello/")) {
+      return "/hello/{name}";
+    }
+    return path;
+  }
+
+  @Test
+  void requestWithTemplate() {
+    URI rootUri = resolveAddress("/");
+    URI uri = resolveAddress("/hello/world");
+    String method = "GET";
+    int responseCode =
+        restTemplate
+            .exchange(
+                rootUri + "hello/{name}", HttpMethod.valueOf(method), null, String.class, "world")
+            .getStatusCode()
+            .value();
+
+    assertThat(responseCode).isEqualTo(200);
+
+    testing.waitAndAssertTraces(
+        trace ->
+            trace.hasSpansSatisfyingExactly(
+                span ->
+                    assertClientSpan(span, uri, method, responseCode, null)
+                        .hasNoParent()
+                        .hasStatus(StatusData.unset()),
+                span -> assertServerSpan(span).hasParent(trace.getSpan(0))));
   }
 }

testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java

@@ -1059,8 +1059,7 @@ void spanEndsAfterHeadersReceived() throws Exception {
         });
   }
 
-  // Visible for spock bridge.
-  SpanDataAssert assertClientSpan(
+  protected SpanDataAssert assertClientSpan(
       SpanDataAssert span,
       URI uri,
       String method,
@@ -1120,9 +1119,11 @@ SpanDataAssert assertClientSpan(
               if (httpClientAttributes.contains(UrlAttributes.URL_FULL)) {
                 assertThat(attrs).containsEntry(UrlAttributes.URL_FULL, uri.toString());
               }
-              if (httpClientAttributes.contains(UrlIncubatingAttributes.URL_TEMPLATE)) {
+              if (options.getHasUrlTemplate()) {
                 assertThat(attrs)
-                    .containsEntry(UrlIncubatingAttributes.URL_TEMPLATE, uri.getPath());
+                    .containsEntry(
+                        UrlIncubatingAttributes.URL_TEMPLATE,
+                        options.getExpectedUrlTemplateMapper().apply(uri));
               }
               if (httpClientAttributes.contains(HttpAttributes.HTTP_REQUEST_METHOD)) {
                 assertThat(attrs).containsEntry(HttpAttributes.HTTP_REQUEST_METHOD, method);
@@ -1153,8 +1154,7 @@ SpanDataAssert assertClientSpan(
             });
   }
 
-  // Visible for spock bridge.
-  static SpanDataAssert assertServerSpan(SpanDataAssert span) {
+  protected static SpanDataAssert assertServerSpan(SpanDataAssert span) {
     return span.hasName("test-http-server").hasKind(SpanKind.SERVER);
   }
 

testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java

@@ -39,6 +39,8 @@ public abstract class HttpClientTestOptions {
   public static final BiFunction<URI, String, String> DEFAULT_EXPECTED_CLIENT_SPAN_NAME_MAPPER =
       (uri, method) -> HttpConstants._OTHER.equals(method) ? "HTTP" : method;
 
+  public static final Function<URI, String> DEFAULT_EXPECTED_URL_TEMPLATE_MAPPER = URI::getPath;
+
   public static final int FOUND_STATUS_CODE = HttpStatus.FOUND.code();
 
   public abstract Function<URI, Set<AttributeKey<?>>> getHttpAttributes();
@@ -56,6 +58,8 @@ public abstract class HttpClientTestOptions {
 
   public abstract BiFunction<URI, String, String> getExpectedClientSpanNameMapper();
 
+  public abstract Function<URI, String> getExpectedUrlTemplateMapper();
+
   abstract HttpClientInstrumentationType getInstrumentationType();
 
   public boolean isLowLevelInstrumentation() {
@@ -95,6 +99,8 @@ public boolean isLowLevelInstrumentation() {
 
   public abstract Function<URI, String> getHttpProtocolVersion();
 
+  public abstract boolean getHasUrlTemplate();
+
   @Nullable
   abstract SpanEndsAfterType getSpanEndsAfterType();
 
@@ -120,6 +126,7 @@ default Builder withDefaults() {
           .setClientSpanErrorMapper((uri, exception) -> exception)
           .setSingleConnectionFactory((host, port) -> null)
           .setExpectedClientSpanNameMapper(DEFAULT_EXPECTED_CLIENT_SPAN_NAME_MAPPER)
+          .setExpectedUrlTemplateMapper(DEFAULT_EXPECTED_URL_TEMPLATE_MAPPER)
           .setInstrumentationType(HttpClientInstrumentationType.HIGH_LEVEL)
           .setSpanEndsAfterType(SpanEndsAfterType.HEADERS)
           .setTestWithClientParent(true)
@@ -137,6 +144,7 @@ default Builder withDefaults() {
           .setTestNonStandardHttpMethod(true)
           .setTestCaptureHttpHeaders(true)
           .setHasSendRequest(true)
+          .setHasUrlTemplate(false)
           .setHttpProtocolVersion(uri -> "1.1");
     }
 
@@ -152,6 +160,8 @@ default Builder withDefaults() {
 
     Builder setExpectedClientSpanNameMapper(BiFunction<URI, String, String> value);
 
+    Builder setExpectedUrlTemplateMapper(Function<URI, String> value);
+
     Builder setInstrumentationType(HttpClientInstrumentationType instrumentationType);
 
     Builder setSpanEndsAfterType(SpanEndsAfterType spanEndsAfterType);
@@ -184,6 +194,8 @@ default Builder withDefaults() {
 
     Builder setHasSendRequest(boolean value);
 
+    Builder setHasUrlTemplate(boolean value);
+
     Builder setHttpProtocolVersion(Function<URI, String> value);
 
     @CanIgnoreReturnValue
@@ -261,6 +273,11 @@ default Builder spanEndsAfterBody() {
       return setSpanEndsAfterType(SpanEndsAfterType.BODY);
     }
 
+    @CanIgnoreReturnValue
+    default Builder enableUrlTemplate() {
+      return setHasUrlTemplate(true);
+    }
+
     HttpClientTestOptions build();
   }
 

testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestServer.java

@@ -124,6 +124,12 @@ protected void configure(ServerBuilder sb) throws Exception {
 
               return writer;
             })
+        .service(
+            "/hello/{name}",
+            (ctx, req) -> {
+              String name = ctx.pathParam("name");
+              return HttpResponse.of("Hello, %s!", name != null ? name : "unknown");
+            })
         .decorator(
             (delegate, ctx, req) -> {
               for (String field : openTelemetry.getPropagators().getTextMapPropagator().fields()) {