Fix http.url handing in vert.x 3 http client

instrumentation/vertx-http-client/vertx-http-client-3.0/javaagent/build.gradle.kts

@@ -14,6 +14,9 @@ muzzle {
 dependencies {
   library("io.vertx:vertx-core:3.0.0")
 
+  compileOnly("com.google.auto.value:auto-value-annotations")
+  annotationProcessor("com.google.auto.value:auto-value")
+
   implementation(project(":instrumentation:vertx-http-client:vertx-http-client-common:javaagent"))
 
   // We need both version as different versions of Vert.x use different versions of Netty

instrumentation/vertx-http-client/vertx-http-client-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/v3_0/client/HttpClientImplInstrumentation.java

@@ -0,0 +1,41 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.vertx.v3_0.client;
+
+import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
+import static net.bytebuddy.matcher.ElementMatchers.named;
+
+import io.opentelemetry.instrumentation.api.field.VirtualField;
+import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
+import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
+import io.vertx.core.http.HttpClientOptions;
+import io.vertx.core.http.impl.HttpClientImpl;
+import net.bytebuddy.asm.Advice;
+import net.bytebuddy.description.type.TypeDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+
+public class HttpClientImplInstrumentation implements TypeInstrumentation {
+
+  @Override
+  public ElementMatcher<TypeDescription> typeMatcher() {
+    return named("io.vertx.core.http.impl.HttpClientImpl");
+  }
+
+  @Override
+  public void transform(TypeTransformer transformer) {
+    transformer.applyAdviceToMethod(
+        isConstructor(), HttpClientImplInstrumentation.class.getName() + "$AttachStateAdvice");
+  }
+
+  public static class AttachStateAdvice {
+    @Advice.OnMethodExit(suppress = Throwable.class)
+    public static void attachHttpClientOptions(
+        @Advice.This HttpClientImpl client,
+        @Advice.FieldValue("options") HttpClientOptions options) {
+      VirtualField.find(HttpClientImpl.class, HttpClientOptions.class).set(client, options);
+    }
+  }
+}

instrumentation/vertx-http-client/vertx-http-client-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/v3_0/client/HttpRequestImplInstrumentation.java

@@ -0,0 +1,88 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.vertx.v3_0.client;
+
+import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
+import static net.bytebuddy.matcher.ElementMatchers.named;
+import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
+
+import io.opentelemetry.instrumentation.api.field.VirtualField;
+import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
+import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
+import io.vertx.core.http.HttpClientOptions;
+import io.vertx.core.http.HttpClientRequest;
+import io.vertx.core.http.impl.HttpClientImpl;
+import net.bytebuddy.asm.Advice;
+import net.bytebuddy.description.type.TypeDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+
+public class HttpRequestImplInstrumentation implements TypeInstrumentation {
+
+  @Override
+  public ElementMatcher<TypeDescription> typeMatcher() {
+    return named("io.vertx.core.http.impl.HttpClientRequestImpl");
+  }
+
+  @Override
+  public void transform(TypeTransformer transformer) {
+    transformer.applyAdviceToMethod(
+        isConstructor().and(takesArgument(2, String.class)).and(takesArgument(3, int.class)),
+        HttpRequestImplInstrumentation.class.getName() + "$Vertx30Advice");
+    transformer.applyAdviceToMethod(
+        isConstructor()
+            .and(takesArgument(1, boolean.class))
+            .and(takesArgument(3, String.class))
+            .and(takesArgument(4, int.class)),
+        HttpRequestImplInstrumentation.class.getName() + "$Vertx34Advice");
+    transformer.applyAdviceToMethod(
+        isConstructor()
+            .and(takesArgument(1, boolean.class))
+            .and(takesArgument(4, String.class))
+            .and(takesArgument(5, int.class)),
+        HttpRequestImplInstrumentation.class.getName() + "$Vertx37Advice");
+  }
+
+  public static class Vertx30Advice {
+    @Advice.OnMethodExit(suppress = Throwable.class)
+    public static void attachRequestInfo(
+        @Advice.This HttpClientRequest request,
+        @Advice.Argument(0) HttpClientImpl client,
+        @Advice.Argument(2) String host,
+        @Advice.Argument(3) int port) {
+      HttpClientOptions httpClientOptions =
+          VirtualField.find(HttpClientImpl.class, HttpClientOptions.class).get(client);
+      VirtualField.find(HttpClientRequest.class, VertxRequestInfo.class)
+          .set(
+              request,
+              VertxRequestInfo.create(
+                  httpClientOptions != null ? httpClientOptions.isSsl() : false, host, port));
+    }
+  }
+
+  public static class Vertx34Advice {
+    @Advice.OnMethodExit(suppress = Throwable.class)
+    public static void attachRequestInfo(
+        @Advice.This HttpClientRequest request,
+        @Advice.Argument(1) boolean ssl,
+        @Advice.Argument(3) String host,
+        @Advice.Argument(4) int port) {
+      VirtualField.find(HttpClientRequest.class, VertxRequestInfo.class)
+          .set(request, VertxRequestInfo.create(ssl, host, port));
+    }
+  }
+
+  public static class Vertx37Advice {
+    @Advice.OnMethodExit(suppress = Throwable.class)
+    public static void attachRequestInfo(
+        @Advice.This HttpClientRequest request,
+        @Advice.Argument(1) boolean ssl,
+        @Advice.Argument(4) String host,
+        @Advice.Argument(5) int port) {
+      VirtualField.find(HttpClientRequest.class, VertxRequestInfo.class)
+          .set(request, VertxRequestInfo.create(ssl, host, port));
+    }
+  }
+}

instrumentation/vertx-http-client/vertx-http-client-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/v3_0/client/HttpRequestInstrumentation.java

@@ -89,8 +89,13 @@ public static void attachContext(
         @Advice.Local("otelContext") Context context,
         @Advice.Local("otelScope") Scope scope) {
 
-      Context parentContext = Java8BytecodeBridge.currentContext();
+      VertxRequestInfo requestInfo =
+          VirtualField.find(HttpClientRequest.class, VertxRequestInfo.class).get(request);
+      if (requestInfo == null) {
+        return;
+      }
 
+      Context parentContext = Java8BytecodeBridge.currentContext();
       if (!instrumenter().shouldStart(parentContext, request)) {
         return;
       }

instrumentation/vertx-http-client/vertx-http-client-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/v3_0/client/Vertx3HttpAttributesExtractor.java

@@ -5,16 +5,42 @@
 
 package io.opentelemetry.javaagent.instrumentation.vertx.v3_0.client;
 
+import io.opentelemetry.instrumentation.api.field.VirtualField;
 import io.opentelemetry.javaagent.instrumentation.vertx.client.AbstractVertxHttpAttributesExtractor;
 import io.vertx.core.http.HttpClientRequest;
 import javax.annotation.Nullable;
 
 final class Vertx3HttpAttributesExtractor extends AbstractVertxHttpAttributesExtractor {
+  private static final VirtualField<HttpClientRequest, VertxRequestInfo> requestInfoField =
+      VirtualField.find(HttpClientRequest.class, VertxRequestInfo.class);
 
   @Nullable
   @Override
   protected String url(HttpClientRequest request) {
-    return request.uri();
+    String uri = request.uri();
+    // Uri should be relative, but it is possible to misuse vert.x api and pass an absolute uri
+    // where relative is expected.
+    if (!isAbsolute(uri)) {
+      VertxRequestInfo requestInfo = requestInfoField.get(request);
+      uri = absoluteUri(requestInfo, uri);
+    }
+    return uri;
+  }
+
+  private static boolean isAbsolute(String uri) {
+    return uri.startsWith("http://") || uri.startsWith("https://");
+  }
+
+  private static String absoluteUri(VertxRequestInfo requestInfo, String uri) {
+    String result = requestInfo.isSsl() ? "https://" : "http://";
+    result += requestInfo.getHost();
+    if (requestInfo.getPort() != -1
+        && (requestInfo.getPort() != 80 || requestInfo.isSsl())
+        && (requestInfo.getPort() != 443 || !requestInfo.isSsl())) {
+      result += ":" + requestInfo.getPort();
+    }
+    result += uri;
+    return result;
   }
 
   @Override

instrumentation/vertx-http-client/vertx-http-client-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/v3_0/client/VertxClientInstrumentationModule.java

@@ -6,7 +6,7 @@
 package io.opentelemetry.javaagent.instrumentation.vertx.v3_0.client;
 
 import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
-import static java.util.Collections.singletonList;
+import static java.util.Arrays.asList;
 
 import com.google.auto.service.AutoService;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
@@ -29,6 +29,9 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
 
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
-    return singletonList(new HttpRequestInstrumentation());
+    return asList(
+        new HttpClientImplInstrumentation(),
+        new HttpRequestImplInstrumentation(),
+        new HttpRequestInstrumentation());
   }
 }

instrumentation/vertx-http-client/vertx-http-client-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/v3_0/client/VertxRequestInfo.java

@@ -0,0 +1,22 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.vertx.v3_0.client;
+
+import com.google.auto.value.AutoValue;
+
+@AutoValue
+public abstract class VertxRequestInfo {
+
+  public static VertxRequestInfo create(boolean ssl, String host, int port) {
+    return new AutoValue_VertxRequestInfo(ssl, host, port);
+  }
+
+  public abstract boolean isSsl();
+
+  public abstract String getHost();
+
+  public abstract int getPort();
+}

instrumentation/vertx-http-client/vertx-http-client-3.0/javaagent/src/test/groovy/client/VertxHttpClientTest.groovy

@@ -31,7 +31,7 @@ class VertxHttpClientTest extends HttpClientTest<HttpClientRequest> implements A
 
   @Override
   HttpClientRequest buildRequest(String method, URI uri, Map<String, String> headers) {
-    def request = httpClient.request(HttpMethod.valueOf(method), getPort(uri), uri.host, "$uri")
+    def request = httpClient.requestAbs(HttpMethod.valueOf(method), "$uri")
     headers.each { request.putHeader(it.key, it.value) }
     return request
   }
@@ -78,6 +78,11 @@ class VertxHttpClientTest extends HttpClientTest<HttpClientRequest> implements A
     false
   }
 
+  @Override
+  String nonRoutableAddress() {
+    "http://192.0.2.1/"
+  }
+
   @Override
   Set<AttributeKey<?>> httpAttributes(URI uri) {
     def attributes = super.httpAttributes(uri)

testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpClientTest.groovy

@@ -225,6 +225,11 @@ abstract class HttpClientTest<REQUEST> extends InstrumentationSpecification {
     protected boolean testErrorWithCallback() {
       return HttpClientTest.this.testErrorWithCallback()
     }
+
+    @Override
+    protected String nonRoutableAddress() {
+      return HttpClientTest.this.nonRoutableAddress()
+    }
   }
 
   @Shared
@@ -501,6 +506,10 @@ abstract class HttpClientTest<REQUEST> extends InstrumentationSpecification {
     return true
   }
 
+  String nonRoutableAddress() {
+    HttpClientTestOptions.DEFAULT_NON_ROUTABLE_ADDRESS
+  }
+
   Throwable clientSpanError(URI uri, Throwable exception) {
     return exception
   }

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

@@ -5,6 +5,7 @@
 
 package io.opentelemetry.instrumentation.testing.junit.http;
 
+import static io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions.DEFAULT_NON_ROUTABLE_ADDRESS;
 import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
 import static org.assertj.core.api.Assertions.catchThrowable;
 import static org.junit.jupiter.api.Assumptions.assumeTrue;
@@ -191,6 +192,7 @@ void setupOptions() {
     if (!testErrorWithCallback()) {
       options.disableTestErrorWithCallback();
     }
+    options.setNonRoutableAddress(nonRoutableAddress());
 
     configure(options);
   }
@@ -569,7 +571,7 @@ void connectionErrorNonRoutableAddress() {
     assumeTrue(options.testRemoteConnection);
 
     String method = "HEAD";
-    URI uri = URI.create("https://192.0.2.1/");
+    URI uri = URI.create(options.nonRoutableAddress);
 
     Throwable thrown =
         catchThrowable(() -> testing.runWithSpan("parent", () -> doRequest(method, uri)));
@@ -922,10 +924,11 @@ SpanDataAssert assertClientSpan(
               // TODO(anuraaga): Move to test knob rather than always treating
               // as optional
               if (attrs.asMap().containsKey(SemanticAttributes.NET_PEER_IP)) {
-                if (uri.getHost().equals("192.0.2.1")) {
+                String nonRoutableHost = URI.create(options.nonRoutableAddress).getHost();
+                if (uri.getHost().equals(nonRoutableHost)) {
                   // NB(anuraaga): This branch seems to currently only be exercised on Java 15.
                   // It would be good to understand how the JVM version is impacting this check.
-                  assertThat(attrs).containsEntry(SemanticAttributes.NET_PEER_IP, "192.0.2.1");
+                  assertThat(attrs).containsEntry(SemanticAttributes.NET_PEER_IP, nonRoutableHost);
                 } else {
                   assertThat(attrs).containsEntry(SemanticAttributes.NET_PEER_IP, "127.0.0.1");
                 }
@@ -1071,6 +1074,10 @@ protected boolean testErrorWithCallback() {
     return true;
   }
 
+  protected String nonRoutableAddress() {
+    return DEFAULT_NON_ROUTABLE_ADDRESS;
+  }
+
   protected void configure(HttpClientTestOptions options) {}
 
   private int doRequest(String method, URI uri) throws Exception {

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

@@ -31,6 +31,8 @@ public final class HttpClientTestOptions {
   public static final BiFunction<URI, String, String> DEFAULT_EXPECTED_CLIENT_SPAN_NAME_MAPPER =
       (uri, method) -> method != null ? "HTTP " + method : "HTTP request";
 
+  public static final String DEFAULT_NON_ROUTABLE_ADDRESS = "https://192.0.2.1/";
+
   Function<URI, Set<AttributeKey<?>>> httpAttributes = unused -> DEFAULT_HTTP_ATTRIBUTES;
 
   BiFunction<URI, String, String> expectedClientSpanNameMapper =
@@ -57,6 +59,7 @@ public final class HttpClientTestOptions {
   boolean testCallback = true;
   boolean testCallbackWithParent = true;
   boolean testErrorWithCallback = true;
+  String nonRoutableAddress = DEFAULT_NON_ROUTABLE_ADDRESS;
 
   HttpClientTestOptions() {}
 
@@ -163,4 +166,9 @@ public HttpClientTestOptions disableTestErrorWithCallback() {
     testErrorWithCallback = false;
     return this;
   }
+
+  public HttpClientTestOptions setNonRoutableAddress(String nonRoutableAddress) {
+    this.nonRoutableAddress = nonRoutableAddress;
+    return this;
+  }
 }