Skip to content

Commit

Permalink
Migrate HttpClientTest test server to Armeria (open-telemetry#3225)
Browse files Browse the repository at this point in the history
* Use Armeria for test HTTP server.

* Continue

* Migrate test http server to Armeria.

* Finish

* Use junit extension

* Remove unused.

* Use localhost for net peer name.

* Block for full response in recator-netty tests.

* Handle split responses in netty41 and akka

* Typo
  • Loading branch information
Anuraag Agrawal authored and robododge committed Jun 17, 2021
1 parent 83a5c88 commit e0e61cc
Show file tree
Hide file tree
Showing 21 changed files with 277 additions and 162 deletions.
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
apply from: "$rootDir/gradle/instrumentation.gradle"
apply from: "$rootDir/gradle/test-with-scala.gradle"
apply plugin: 'org.unbroken-dome.test-sets'

testSets {
version101Test {
dirName = 'test'
}
}

muzzle {
pass {
Expand Down Expand Up @@ -50,17 +43,6 @@ muzzle {
dependencies {
library "com.typesafe.akka:akka-http_2.11:10.0.0"
library "com.typesafe.akka:akka-stream_2.11:2.4.14"

// There are some internal API changes in 10.1 that we would like to test separately for
version101TestImplementation "com.typesafe.akka:akka-http_2.11:10.1.0"
version101TestImplementation "com.typesafe.akka:akka-stream_2.11:2.5.11"
}

test.dependsOn version101Test

compileVersion101TestGroovy {
classpath = classpath.plus(files(compileVersion101TestScala.destinationDir))
dependsOn compileVersion101TestScala
}

tasks.withType(Test).configureEach {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@ import akka.actor.ActorSystem
import akka.http.javadsl.Http
import akka.http.javadsl.model.HttpMethods
import akka.http.javadsl.model.HttpRequest
import akka.http.javadsl.model.HttpResponse
import akka.http.javadsl.model.headers.RawHeader
import akka.stream.ActorMaterializer
import io.opentelemetry.instrumentation.test.AgentTestTrait
import io.opentelemetry.instrumentation.test.base.HttpClientTest
import io.opentelemetry.instrumentation.test.base.SingleConnection
import java.util.concurrent.TimeUnit
import spock.lang.Shared

class AkkaHttpClientInstrumentationTest extends HttpClientTest<HttpRequest> implements AgentTestTrait {
Expand All @@ -33,17 +35,20 @@ class AkkaHttpClientInstrumentationTest extends HttpClientTest<HttpRequest> impl

@Override
int sendRequest(HttpRequest request, String method, URI uri, Map<String, String> headers) {
return Http.get(system)
HttpResponse response = Http.get(system)
.singleRequest(request, materializer)
.toCompletableFuture()
.get()
.status()
.intValue()
.get(10, TimeUnit.SECONDS)

response.discardEntityBytes(materializer)

return response.status().intValue()
}

@Override
void sendRequestWithCallback(HttpRequest request, String method, URI uri, Map<String, String> headers, RequestResult requestResult) {
Http.get(system).singleRequest(request, materializer).whenComplete {response, throwable ->
response.discardEntityBytes(materializer)
requestResult.complete({ response.status().intValue() }, throwable)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ abstract class AbstractGoogleHttpClientTest extends HttpClientTest<HttpRequest>

def "error traces when exception is not thrown"() {
given:
def uri = server.address.resolve("/error")
def uri = resolveAddress("/error")

when:
def responseCode = doRequest(method, uri)
Expand All @@ -85,7 +85,7 @@ abstract class AbstractGoogleHttpClientTest extends HttpClientTest<HttpRequest>
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
}
}
server.distributedRequestSpan(it, 1, span(0))
serverSpan(it, 1, span(0))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class HttpUrlConnectionTest extends HttpClientTest<HttpURLConnection> implements
@Unroll
def "trace request (useCaches: #useCaches)"() {
setup:
def url = server.address.resolve("/success").toURL()
def url = resolveAddress("/success").toURL()
runUnderTrace("someTrace") {
HttpURLConnection connection = url.openConnection()
connection.useCaches = useCaches
Expand Down Expand Up @@ -109,7 +109,7 @@ class HttpUrlConnectionTest extends HttpClientTest<HttpURLConnection> implements
attributes {
"${SemanticAttributes.NET_TRANSPORT.key}" IP_TCP
"${SemanticAttributes.NET_PEER_NAME.key}" "localhost"
"${SemanticAttributes.NET_PEER_PORT.key}" server.address.port
"${SemanticAttributes.NET_PEER_PORT.key}" server.httpPort()
"${SemanticAttributes.HTTP_URL.key}" "$url"
"${SemanticAttributes.HTTP_METHOD.key}" "GET"
"${SemanticAttributes.HTTP_STATUS_CODE.key}" STATUS
Expand All @@ -130,7 +130,7 @@ class HttpUrlConnectionTest extends HttpClientTest<HttpURLConnection> implements
attributes {
"${SemanticAttributes.NET_TRANSPORT.key}" IP_TCP
"${SemanticAttributes.NET_PEER_NAME.key}" "localhost"
"${SemanticAttributes.NET_PEER_PORT.key}" server.address.port
"${SemanticAttributes.NET_PEER_PORT.key}" server.httpPort()
"${SemanticAttributes.HTTP_URL.key}" "$url"
"${SemanticAttributes.HTTP_METHOD.key}" "GET"
"${SemanticAttributes.HTTP_STATUS_CODE.key}" STATUS
Expand All @@ -153,7 +153,7 @@ class HttpUrlConnectionTest extends HttpClientTest<HttpURLConnection> implements
def "test broken API usage"() {
setup:
def url = server.address.resolve("/success").toURL()
def url = resolveAddress("/success").toURL()
HttpURLConnection connection = runUnderTrace("someTrace") {
HttpURLConnection connection = url.openConnection()
connection.setRequestProperty("Connection", "close")
Expand All @@ -177,7 +177,7 @@ class HttpUrlConnectionTest extends HttpClientTest<HttpURLConnection> implements
childOf span(0)
attributes {
"${SemanticAttributes.NET_PEER_NAME.key}" "localhost"
"${SemanticAttributes.NET_PEER_PORT.key}" server.address.port
"${SemanticAttributes.NET_PEER_PORT.key}" server.httpPort()
"${SemanticAttributes.NET_TRANSPORT.key}" IP_TCP
"${SemanticAttributes.HTTP_URL.key}" "$url"
"${SemanticAttributes.HTTP_METHOD.key}" "GET"
Expand All @@ -198,7 +198,7 @@ class HttpUrlConnectionTest extends HttpClientTest<HttpURLConnection> implements
def "test post request"() {
setup:
def url = server.address.resolve("/success").toURL()
def url = resolveAddress("/success").toURL()
runUnderTrace("someTrace") {
HttpURLConnection connection = url.openConnection()
connection.setRequestMethod("POST")
Expand Down Expand Up @@ -236,7 +236,7 @@ class HttpUrlConnectionTest extends HttpClientTest<HttpURLConnection> implements
attributes {
"${SemanticAttributes.NET_TRANSPORT.key}" IP_TCP
"${SemanticAttributes.NET_PEER_NAME.key}" "localhost"
"${SemanticAttributes.NET_PEER_PORT.key}" server.address.port
"${SemanticAttributes.NET_PEER_PORT.key}" server.httpPort()
"${SemanticAttributes.HTTP_URL.key}" "$url"
"${SemanticAttributes.HTTP_METHOD.key}" "POST"
"${SemanticAttributes.HTTP_STATUS_CODE.key}" STATUS
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,23 @@
* SPDX-License-Identifier: Apache-2.0
*/

import static io.opentelemetry.api.trace.SpanKind.CLIENT
import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_TCP

import io.opentelemetry.instrumentation.test.AgentTestTrait
import io.opentelemetry.instrumentation.test.base.HttpClientTest
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
import java.net.http.HttpClient
import java.net.http.HttpRequest
import java.net.http.HttpResponse
import java.time.Duration
import java.time.temporal.ChronoUnit
import spock.lang.Requires
import spock.lang.Shared

class JdkHttpClientTest extends HttpClientTest<HttpRequest> implements AgentTestTrait {

@Shared
def client = HttpClient.newBuilder().connectTimeout(Duration.of(CONNECT_TIMEOUT_MS,
ChronoUnit.MILLIS)).followRedirects(HttpClient.Redirect.NORMAL).build()
def client = HttpClient.newBuilder()
.version(HttpClient.Version.HTTP_1_1)
.connectTimeout(Duration.of(CONNECT_TIMEOUT_MS, ChronoUnit.MILLIS))
.followRedirects(HttpClient.Redirect.NORMAL)
.build()

@Override
HttpRequest buildRequest(String method, URI uri, Map<String, String> headers) {
Expand Down Expand Up @@ -64,40 +62,4 @@ class JdkHttpClientTest extends HttpClientTest<HttpRequest> implements AgentTest
boolean testErrorWithCallback() {
return false
}

@Requires({ !System.getProperty("java.vm.name").contains("IBM J9 VM") })
def "test https request"() {
given:
def uri = new URI("https://www.google.com/")

when:
def responseCode = doRequest(method, uri)

then:
responseCode == 200
assertTraces(1) {
trace(0, 1) {
span(0) {
hasNoParent()
name expectedOperationName(method)
kind CLIENT
attributes {
"${SemanticAttributes.NET_TRANSPORT.key}" IP_TCP
"${SemanticAttributes.NET_PEER_NAME.key}" uri.host
"${SemanticAttributes.NET_PEER_IP.key}" { it == null || it == "127.0.0.1" }
// Optional
"${SemanticAttributes.NET_PEER_PORT.key}" uri.port > 0 ? uri.port : { it == null || it == 443 }
"${SemanticAttributes.HTTP_URL.key}" { it == "${uri}" || it == "${removeFragment(uri)}" }
"${SemanticAttributes.HTTP_METHOD.key}" method
"${SemanticAttributes.HTTP_FLAVOR.key}" "2.0"
"${SemanticAttributes.HTTP_STATUS_CODE.key}" responseCode
}
}
}
}

where:
method = "HEAD"
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ abstract class JaxRsClientTest extends HttpClientTest<Invocation.Builder> implem
def "should properly convert HTTP status #statusCode to span error status"() {
given:
def method = "GET"
def uri = server.address.resolve(path)
def uri = resolveAddress(path)

when:
def actualStatusCode = doRequest(method, uri)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class ResteasyProxyClientTest extends HttpClientTest<ResteasyProxyResource> impl
ResteasyProxyResource buildRequest(String method, URI uri, Map<String, String> headers) {
return new ResteasyClientBuilder()
.build()
.target(new ResteasyUriBuilder().uri(server.address))
.target(new ResteasyUriBuilder().uri(resolveAddress("")))
.proxy(ResteasyProxyResource)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@

import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.SimpleChannelInboundHandler;
import io.netty.handler.codec.http.FullHttpResponse;
import io.netty.handler.codec.http.HttpObject;
import io.netty.handler.codec.http.HttpResponse;
import io.netty.handler.codec.http.LastHttpContent;
import io.netty.util.AttributeKey;
import java.util.concurrent.CompletableFuture;

/*
Expand All @@ -15,6 +18,10 @@
future with response's status code.
*/
public class ClientHandler extends SimpleChannelInboundHandler<HttpObject> {

private static final AttributeKey<HttpResponse> HTTP_RESPONSE =
AttributeKey.valueOf(ClientHandler.class, "http-response");

private final CompletableFuture<Integer> responseCode;

public ClientHandler(CompletableFuture<Integer> responseCode) {
Expand All @@ -23,11 +30,16 @@ public ClientHandler(CompletableFuture<Integer> responseCode) {

@Override
public void channelRead0(ChannelHandlerContext ctx, HttpObject msg) {
if (msg instanceof HttpResponse) {
if (msg instanceof FullHttpResponse) {
ctx.pipeline().remove(this);

HttpResponse response = (HttpResponse) msg;
FullHttpResponse response = (FullHttpResponse) msg;
responseCode.complete(response.getStatus().code());
} else if (msg instanceof HttpResponse) {
// Headers before body have been received, store them to use when finishing the span.
ctx.channel().attr(HTTP_RESPONSE).set((HttpResponse) msg);
} else if (msg instanceof LastHttpContent) {
ctx.pipeline().remove(this);
responseCode.complete(ctx.channel().attr(HTTP_RESPONSE).get().getStatus().code());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,14 +145,14 @@ class Netty41ClientTest extends HttpClientTest<DefaultFullHttpRequest> implement
pipeline.addLast(new HttpClientCodec())
}
})
def request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, server.address.resolve("/success").toString(), Unpooled.EMPTY_BUFFER)
request.headers().set(HttpHeaderNames.HOST, server.address.host)
def request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, resolveAddress("/success").toString(), Unpooled.EMPTY_BUFFER)
request.headers().set(HttpHeaderNames.HOST, "localhost")
Channel ch = null

when:
// note that this is a purely asynchronous request
runUnderTrace("parent1") {
ch = b.connect(server.address.host, server.address.port).sync().channel()
ch = b.connect("localhost", server.httpPort()).sync().channel()
ch.write(request)
ch.flush()
}
Expand Down Expand Up @@ -285,7 +285,7 @@ class Netty41ClientTest extends HttpClientTest<DefaultFullHttpRequest> implement
}
}
clientSpan(it, 2, span(1), method)
server.distributedRequestSpan(it, 3, span(2))
serverSpan(it, 3, span(2))
}
}

Expand All @@ -295,8 +295,9 @@ class Netty41ClientTest extends HttpClientTest<DefaultFullHttpRequest> implement

class TracedClass {
int tracedMethod(String method) {
def uri = resolveAddress("/success")
runUnderTrace("tracedMethod") {
doRequest(method, server.address.resolve("/success"))
doRequest(method, uri)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class RatpackHttpClientTest extends HttpClientTest<Void> implements AgentTestTra
return new SingleConnection() {
@Override
int doRequest(String path, Map<String, String> headers) throws ExecutionException, InterruptedException, TimeoutException {
def uri = server.address.resolve(path)
def uri = resolveAddress(path)
return exec.yield {
internalSendRequest(singleConnectionClient, "GET", uri, headers)
}.valueOrThrow
Expand Down
Loading

0 comments on commit e0e61cc

Please sign in to comment.