Fix some remote connection tests
MODIFY
instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/test/groovy/ApacheHttpClientTest.groovy
instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/test/groovy/ApacheHttpClientTest.groovy
@@ -92,20 +92,15 @@ class ApacheClientHostRequest extends ApacheHttpClientTest<BasicHttpRequest> {
@Override
HttpResponse executeRequest(BasicHttpRequest request, URI uri) {
- return client.execute(new HttpHost(uri.getHost(), uri.getPort()), request)
+ return client.execute(new HttpHost(uri.getHost(), uri.getPort(), uri.getScheme()), request)
}
@Override
void executeRequestWithCallback(BasicHttpRequest request, URI uri, Consumer<HttpResponse> callback) {
- client.execute(new HttpHost(uri.getHost(), uri.getPort()), request) {
+ client.execute(new HttpHost(uri.getHost(), uri.getPort(), uri.getScheme()), request) {
callback.accept(it)
}
}
-
- @Override
- boolean testRemoteConnection() {
- return false
- }
}
class ApacheClientHostRequestContext extends ApacheHttpClientTest<BasicHttpRequest> {
@@ -116,20 +111,15 @@ class ApacheClientHostRequestContext extends ApacheHttpClientTest<BasicHttpReque
@Override
HttpResponse executeRequest(BasicHttpRequest request, URI uri) {
- return client.execute(new HttpHost(uri.getHost(), uri.getPort()), request, new BasicHttpContext())
+ return client.execute(new HttpHost(uri.getHost(), uri.getPort(), uri.getScheme()), request, new BasicHttpContext())
}
@Override
void executeRequestWithCallback(BasicHttpRequest request, URI uri, Consumer<HttpResponse> callback) {
- client.execute(new HttpHost(uri.getHost(), uri.getPort()), request, {
+ client.execute(new HttpHost(uri.getHost(), uri.getPort(), uri.getScheme()), request, {
callback.accept(it)
}, new BasicHttpContext())
}
-
- @Override
- boolean testRemoteConnection() {
- return false
- }
}
class ApacheClientUriRequest extends ApacheHttpClientTest<HttpUriRequest> {

MODIFY
instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/test/groovy/ApacheHttpClientTest.groovy
instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/test/groovy/ApacheHttpClientTest.groovy
@@ -97,20 +97,15 @@ class ApacheClientHostRequest extends ApacheHttpClientTest<ClassicHttpRequest> {
@Override
ClassicHttpResponse executeRequest(ClassicHttpRequest request, URI uri) {
- return client.execute(new HttpHost(uri.getHost(), uri.getPort()), request)
+ return client.execute(new HttpHost(uri.getScheme(), uri.getHost(), uri.getPort()), request)
}
@Override
void executeRequestWithCallback(ClassicHttpRequest request, URI uri, Consumer<ClassicHttpResponse> callback) {
- client.execute(new HttpHost(uri.getHost(), uri.getPort()), request) {
+ client.execute(new HttpHost(uri.getScheme(), uri.getHost(), uri.getPort()), request) {
callback.accept(it)
}
}
-
- @Override
- boolean testRemoteConnection() {
- return false
- }
}
class ApacheClientHostRequestContext extends ApacheHttpClientTest<ClassicHttpRequest> {
@@ -121,20 +116,15 @@ class ApacheClientHostRequestContext extends ApacheHttpClientTest<ClassicHttpReq
@Override
ClassicHttpResponse executeRequest(ClassicHttpRequest request, URI uri) {
- return client.execute(new HttpHost(uri.getHost(), uri.getPort()), request, new BasicHttpContext())
+ return client.execute(new HttpHost(uri.getScheme(), uri.getHost(), uri.getPort()), request, new BasicHttpContext())
}
@Override
void executeRequestWithCallback(ClassicHttpRequest request, URI uri, Consumer<ClassicHttpResponse> callback) {
- client.execute(new HttpHost(uri.getHost(), uri.getPort()), request, new BasicHttpContext()) {
+ client.execute(new HttpHost(uri.getScheme(), uri.getHost(), uri.getPort()), request, new BasicHttpContext()) {
callback.accept(it)
}
}
-
- @Override
- boolean testRemoteConnection() {
- return false
- }
}
class ApacheClientUriRequest extends ApacheHttpClientTest<ClassicHttpRequest> {

MODIFY
instrumentation/armeria-1.3/testing/src/main/groovy/io/opentelemetry/instrumentation/armeria/v1_3/AbstractArmeriaHttpClientTest.groovy
instrumentation/armeria-1.3/testing/src/main/groovy/io/opentelemetry/instrumentation/armeria/v1_3/AbstractArmeriaHttpClientTest.groovy
@@ -63,12 +63,6 @@ abstract class AbstractArmeriaHttpClientTest extends HttpClientTest<HttpRequest>
false
}
- // TODO(anuraaga): Enable after fixing the test https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/2344
- @Override
- boolean testRemoteConnection() {
- false
- }
-
// TODO: context not propagated to callback
@Override
boolean testErrorWithCallback() {

MODIFY
instrumentation/java-http-client/javaagent/src/test/groovy/JdkHttpClientTest.groovy
instrumentation/java-http-client/javaagent/src/test/groovy/JdkHttpClientTest.groovy
@@ -51,12 +51,6 @@ class JdkHttpClientTest extends HttpClientTest<HttpRequest> implements AgentTest
return false
}
- //We override this test below because it produces somewhat different attributes
- @Override
- boolean testRemoteConnection() {
- return false
- }
-
// TODO nested client span is not created, but context is still injected
// which is not what the test expects
@Override

MODIFY
instrumentation/netty/netty-4.0/javaagent/src/test/groovy/Netty40ClientTest.groovy
instrumentation/netty/netty-4.0/javaagent/src/test/groovy/Netty40ClientTest.groovy
@@ -60,7 +60,7 @@ class Netty40ClientTest extends HttpClientTest<DefaultFullHttpRequest> implement
@Override
int sendRequest(DefaultFullHttpRequest request, String method, URI uri, Map<String, String> headers) {
- def channel = bootstrap.connect(uri.host, uri.port).sync().channel()
+ def channel = bootstrap.connect(uri.host, getPort(uri)).sync().channel()
def result = new CompletableFuture<Integer>()
channel.pipeline().addLast(new ClientHandler(result))
channel.writeAndFlush(request).get()
@@ -69,7 +69,7 @@ class Netty40ClientTest extends HttpClientTest<DefaultFullHttpRequest> implement
@Override
void sendRequestWithCallback(DefaultFullHttpRequest request, String method, URI uri, Map<String, String> headers, RequestResult requestResult) {
- Channel ch = bootstrap.connect(uri.host, uri.port).sync().channel()
+ Channel ch = bootstrap.connect(uri.host, getPort(uri)).sync().channel()
def result = new CompletableFuture<Integer>()
result.whenComplete { status, throwable ->
requestResult.complete({ status }, throwable)
@@ -78,6 +78,18 @@ class Netty40ClientTest extends HttpClientTest<DefaultFullHttpRequest> implement
ch.writeAndFlush(request)
}
+ private static int getPort(URI uri) {
+ if (uri.port != -1) {
+ return uri.port
+ } else if (uri.scheme == "http") {
+ return 80
+ } else if (uri.scheme == "https") {
+ 443
+ } else {
+ throw new IllegalArgumentException("Unexpected uri: $uri")
+ }
+ }
+
@Override
String userAgent() {
return "Netty"

MODIFY
instrumentation/netty/netty-4.1/javaagent/src/test/groovy/Netty41ClientTest.groovy
instrumentation/netty/netty-4.1/javaagent/src/test/groovy/Netty41ClientTest.groovy
@@ -68,7 +68,7 @@ class Netty41ClientTest extends HttpClientTest<DefaultFullHttpRequest> implement
@Override
int sendRequest(DefaultFullHttpRequest request, String method, URI uri, Map<String, String> headers) {
- def channel = bootstrap.connect(uri.host, uri.port).sync().channel()
+ def channel = bootstrap.connect(uri.host, getPort(uri)).sync().channel()
def result = new CompletableFuture<Integer>()
channel.pipeline().addLast(new ClientHandler(result))
channel.writeAndFlush(request).get()
@@ -77,7 +77,7 @@ class Netty41ClientTest extends HttpClientTest<DefaultFullHttpRequest> implement
@Override
void sendRequestWithCallback(DefaultFullHttpRequest request, String method, URI uri, Map<String, String> headers, RequestResult requestResult) {
- Channel ch = bootstrap.connect(uri.host, uri.port).sync().channel()
+ Channel ch = bootstrap.connect(uri.host, getPort(uri)).sync().channel()
def result = new CompletableFuture<Integer>()
result.whenComplete { status, throwable ->
requestResult.complete({ status }, throwable)
@@ -86,6 +86,18 @@ class Netty41ClientTest extends HttpClientTest<DefaultFullHttpRequest> implement
ch.writeAndFlush(request)
}
+ private static int getPort(URI uri) {
+ if (uri.port != -1) {
+ return uri.port
+ } else if (uri.scheme == "http") {
+ return 80
+ } else if (uri.scheme == "https") {
+ 443
+ } else {
+ throw new IllegalArgumentException("Unexpected uri: $uri")
+ }
+ }
+
@Override
boolean testRedirects() {
false

MODIFY
instrumentation/spring/spring-web-3.1/library/src/test/groovy/RestTemplateInstrumentationTest.groovy
instrumentation/spring/spring-web-3.1/library/src/test/groovy/RestTemplateInstrumentationTest.groovy
@@ -60,11 +60,6 @@ class RestTemplateInstrumentationTest extends HttpClientTest<HttpEntity<String>>
false
}
- @Override
- boolean testRemoteConnection() {
- false
- }
-
// library instrumentation doesn't have a good way of suppressing nested CLIENT spans yet
@Override
boolean testWithClientParent() {

MODIFY
testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpClientTest.groovy
testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpClientTest.groovy
@@ -797,20 +797,29 @@ abstract class HttpClientTest<REQUEST> extends InstrumentationSpecification {
}
attributes {
"${SemanticAttributes.NET_TRANSPORT.key}" "IP.TCP"
- if (uri.port == UNUSABLE_PORT) {
- // TODO(anuraaga): For the unusable port, there isn't actually a peer so we shouldn't be
+ if (uri.port == UNUSABLE_PORT || uri.host == "192.0.2.1" || (uri.host == "www.google.com" && uri.port == 81)) {
+ // TODO(anuraaga): For theses cases, there isn't actually a peer so we shouldn't be
// filling in peer information but some instrumentation does so based on the URL itself
// which is present in HTTP attributes. We should fix this.
"${SemanticAttributes.NET_PEER_NAME.key}" { it == null || it == uri.host }
- "${SemanticAttributes.NET_PEER_PORT.key}" { it == null || it == UNUSABLE_PORT }
+ "${SemanticAttributes.NET_PEER_PORT.key}" { it == null || it == uri.port }
} else {
"${SemanticAttributes.NET_PEER_NAME.key}" uri.host
"${SemanticAttributes.NET_PEER_PORT.key}" uri.port > 0 ? uri.port : { it == null || it == 443 }
}
+ if (uri.host == "www.google.com") {
+ // unpredictable IP address (or can be none if no connection is made, see comment above)
+ "${SemanticAttributes.NET_PEER_IP.key}" { it == null || it instanceof String }
+ } else {
"${SemanticAttributes.NET_PEER_IP.key}" { it == null || it == "127.0.0.1" } // Optional
+ }
"${SemanticAttributes.HTTP_URL.key}" { it == "${uri}" || it == "${removeFragment(uri)}" }
"${SemanticAttributes.HTTP_METHOD.key}" method
+ if (uri.host == "www.google.com") {
+ "${SemanticAttributes.HTTP_FLAVOR.key}" { it == httpFlavor || it == "2.0" } // google https request can be http 2.0
+ } else {
"${SemanticAttributes.HTTP_FLAVOR.key}" httpFlavor
+ }
if (userAgent) {
"${SemanticAttributes.HTTP_USER_AGENT.key}" { it.startsWith(userAgent) }
}
@@ -819,7 +828,7 @@ abstract class HttpClientTest<REQUEST> extends InstrumentationSpecification {
}
if (extraAttributes.contains(SemanticAttributes.HTTP_HOST)) {
- "${SemanticAttributes.HTTP_HOST}" "localhost:${uri.port}"
+ "${SemanticAttributes.HTTP_HOST}" { it == uri.host || it == "${uri.host}:${uri.port}" }
}
if (extraAttributes.contains(SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH)) {
"${SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH}" Long
@@ -828,7 +837,7 @@ abstract class HttpClientTest<REQUEST> extends InstrumentationSpecification {
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH}" Long
}
if (extraAttributes.contains(SemanticAttributes.HTTP_SCHEME)) {
- "${SemanticAttributes.HTTP_SCHEME}" "http"
+ "${SemanticAttributes.HTTP_SCHEME}" uri.scheme
}
if (extraAttributes.contains(SemanticAttributes.HTTP_TARGET)) {
"${SemanticAttributes.HTTP_TARGET}" uri.path + "${uri.query != null ? "?${uri.query}" : ""}"

Fix some remote connection tests (#2933)