open-telemetry
/
opentelemetry-java-instrumentation
mirror of https://github.com/open-telemetry/opentelemetry-java-instrumentation.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix some remote connection tests (#2933)

This commit is contained in:
Trask Stalnaker 2021-05-09 20:19:39 -07:00 committed by GitHub
parent 6a84c2b2ec
commit cd3dd581f9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 52 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/test/groovy/ApacheHttpClientTest.groovy
View File

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

18
instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/test/groovy/ApacheHttpClientTest.groovy
View File

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

6
instrumentation/armeria-1.3/testing/src/main/groovy/io/opentelemetry/instrumentation/armeria/v1_3/AbstractArmeriaHttpClientTest.groovy
View File

@ -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() {

6
instrumentation/java-http-client/javaagent/src/test/groovy/JdkHttpClientTest.groovy
View File

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

16
instrumentation/netty/netty-4.0/javaagent/src/test/groovy/Netty40ClientTest.groovy
View File

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

16
instrumentation/netty/netty-4.1/javaagent/src/test/groovy/Netty41ClientTest.groovy
View File

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

5
instrumentation/spring/spring-web-3.1/library/src/test/groovy/RestTemplateInstrumentationTest.groovy
View File

@ -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() {

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

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