Fix http.url handing in vert.x 3 http client (#4739)

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

* correct version

* fix build

* if https test is disabled use http for non routable aadress test

* if https test is disabled use http for non routable aadress test

* use StringBuilder
This commit is contained in:
Lauri Tulmin 2021-12-01 20:51:53 +02:00 committed by GitHub
parent 55e0d86d95
commit 313979a467
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 204 additions and 15 deletions

View File

@ -88,7 +88,7 @@ class Netty38ClientTest extends HttpClientTest<Request> implements AgentTestTrai
String expectedClientSpanName(URI uri, String method) {
switch (uri.toString()) {
case "http://localhost:61/": // unopened port
case "https://192.0.2.1/": // non routable address
case "http://192.0.2.1/": // non routable address
return "CONNECT"
default:
return super.expectedClientSpanName(uri, method)
@ -101,7 +101,7 @@ class Netty38ClientTest extends HttpClientTest<Request> implements AgentTestTrai
case "http://localhost:61/": // unopened port
exception = exception.getCause() != null ? exception.getCause() : new ConnectException("Connection refused: localhost/127.0.0.1:61")
break
case "https://192.0.2.1/": // non routable address
case "http://192.0.2.1/": // non routable address
exception = exception.getCause() != null ? exception.getCause() : new ClosedChannelException()
}
return exception
@ -111,7 +111,7 @@ class Netty38ClientTest extends HttpClientTest<Request> implements AgentTestTrai
Set<AttributeKey<?>> httpAttributes(URI uri) {
switch (uri.toString()) {
case "http://localhost:61/": // unopened port
case "https://192.0.2.1/": // non routable address
case "http://192.0.2.1/": // non routable address
return []
}
return super.httpAttributes(uri)

View File

@ -108,7 +108,7 @@ class Netty40ClientTest extends HttpClientTest<DefaultFullHttpRequest> implement
String expectedClientSpanName(URI uri, String method) {
switch (uri.toString()) {
case "http://localhost:61/": // unopened port
case "https://192.0.2.1/": // non routable address
case "http://192.0.2.1/": // non routable address
return "CONNECT"
default:
return super.expectedClientSpanName(uri, method)
@ -119,7 +119,7 @@ class Netty40ClientTest extends HttpClientTest<DefaultFullHttpRequest> implement
Set<AttributeKey<?>> httpAttributes(URI uri) {
switch (uri.toString()) {
case "http://localhost:61/": // unopened port
case "https://192.0.2.1/": // non routable address
case "http://192.0.2.1/": // non routable address
return []
}
return super.httpAttributes(uri)

View File

@ -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

View File

@ -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);
}
}
}

View File

@ -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));
}
}
}

View File

@ -89,8 +89,13 @@ public class HttpRequestInstrumentation implements TypeInstrumentation {
@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;
}

View File

@ -5,16 +5,43 @@
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) {
StringBuilder result = new StringBuilder();
result.append(requestInfo.isSsl() ? "https://" : "http://");
result.append(requestInfo.getHost());
if (requestInfo.getPort() != -1
&& (requestInfo.getPort() != 80 || requestInfo.isSsl())
&& (requestInfo.getPort() != 443 || !requestInfo.isSsl())) {
result.append(':').append(requestInfo.getPort());
}
result.append(uri);
return result.toString();
}
@Override

View File

@ -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 class VertxClientInstrumentationModule extends InstrumentationModule {
@Override
public List<TypeInstrumentation> typeInstrumentations() {
return singletonList(new HttpRequestInstrumentation());
return asList(
new HttpClientImplInstrumentation(),
new HttpRequestImplInstrumentation(),
new HttpRequestInstrumentation());
}
}

View File

@ -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();
}

View File

@ -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
}

View File

@ -90,7 +90,7 @@ class VertxHttpClientTest extends HttpClientTest<Future<HttpClientRequest>> impl
String expectedClientSpanName(URI uri, String method) {
switch (uri.toString()) {
case "http://localhost:61/": // unopened port
case "https://192.0.2.1/": // non routable address
case "http://192.0.2.1/": // non routable address
return "CONNECT"
default:
return super.expectedClientSpanName(uri, method)
@ -101,7 +101,7 @@ class VertxHttpClientTest extends HttpClientTest<Future<HttpClientRequest>> impl
Set<AttributeKey<?>> httpAttributes(URI uri) {
switch (uri.toString()) {
case "http://localhost:61/": // unopened port
case "https://192.0.2.1/": // non routable address
case "http://192.0.2.1/": // non routable address
return []
}
return super.httpAttributes(uri)

View File

@ -63,7 +63,7 @@ class VertxRxWebClientTest extends HttpClientTest<HttpRequest<Buffer>> implement
if (exception.class == RuntimeException) {
switch (uri.toString()) {
case "http://localhost:61/": // unopened port
case "https://192.0.2.1/": // non routable address
case "http://192.0.2.1/": // non routable address
exception = exception.getCause()
}
}

View File

@ -569,7 +569,7 @@ public abstract class AbstractHttpClientTest<REQUEST> {
assumeTrue(options.testRemoteConnection);
String method = "HEAD";
URI uri = URI.create("https://192.0.2.1/");
URI uri = URI.create(options.testHttps ? "https://192.0.2.1/" : "http://192.0.2.1/");
Throwable thrown =
catchThrowable(() -> testing.runWithSpan("parent", () -> doRequest(method, uri)));
@ -904,7 +904,7 @@ public abstract class AbstractHttpClientTest<REQUEST> {
port -> {
// Some instrumentation seem to set NET_PEER_PORT to -1 incorrectly.
if (port > 0) {
assertThat(port).isEqualTo(443);
assertThat(port).isEqualTo(options.testHttps ? 443 : 80);
}
});
}