From 0344cff386b19528c8f64078be0250a97ea45cf0 Mon Sep 17 00:00:00 2001 From: Anubhaw Arya Date: Fri, 13 Mar 2020 10:45:43 -0700 Subject: [PATCH 1/2] Remove hostname and port from HttpClientDecorator --- .../decorator/HttpClientDecorator.java | 28 ++++------ .../decorator/HttpClientDecoratorTest.groovy | 55 +++++++++---------- .../akkahttp/AkkaHttpClientDecorator.java | 10 ---- .../ApacheHttpAsyncClientDecorator.java | 29 +--------- .../ApacheHttpClientDecorator.java | 20 ------- .../aws/v0/AwsSdkClientDecorator.java | 10 ---- .../src/test/groovy/AWSClientTest.groovy | 10 +++- .../groovy/AWSClientTest.groovy | 7 +++ .../aws/v2/AwsSdkClientDecorator.java | 10 ---- .../CommonsHttpClientDecorator.java | 18 ------ .../GoogleHttpClientDecorator.java | 10 ---- .../HttpUrlConnectionDecorator.java | 19 +------ .../jaxrs/v1/JaxRsClientV1Decorator.java | 10 ---- .../jaxrs/JaxRsClientDecorator.java | 10 ---- .../client/NettyHttpClientDecorator.java | 29 ---------- .../client/NettyHttpClientDecorator.java | 29 ---------- .../okhttp3/OkHttpClientDecorator.java | 10 ---- .../playws/PlayWSClientDecorator.java | 10 ---- .../SpringWebfluxHttpClientDecorator.java | 10 ---- 19 files changed, 51 insertions(+), 283 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecorator.java index 21f26c5574..6030d6324f 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecorator.java @@ -16,10 +16,6 @@ public abstract class HttpClientDecorator extends ClientDecor protected abstract URI url(REQUEST request) throws URISyntaxException; - protected abstract String hostname(REQUEST request); - - protected abstract Integer port(REQUEST request); - protected abstract Integer status(RESPONSE response); @Override @@ -48,9 +44,16 @@ public abstract class HttpClientDecorator extends ClientDecor } if (url.getHost() != null) { urlNoParams.append(url.getHost()); - if (url.getPort() > 0 && url.getPort() != 80 && url.getPort() != 443) { - urlNoParams.append(":"); - urlNoParams.append(url.getPort()); + span.setTag(Tags.PEER_HOSTNAME, url.getHost()); + if (Config.get().isHttpClientSplitByDomain()) { + span.setTag(DDTags.SERVICE_NAME, url.getHost()); + } + if (url.getPort() > 0) { + span.setTag(Tags.PEER_PORT, url.getPort()); + if (url.getPort() != 80 && url.getPort() != 443) { + urlNoParams.append(":"); + urlNoParams.append(url.getPort()); + } } } final String path = url.getPath(); @@ -70,17 +73,6 @@ public abstract class HttpClientDecorator extends ClientDecor } catch (final Exception e) { log.debug("Error tagging url", e); } - - span.setTag(Tags.PEER_HOSTNAME, hostname(request)); - final Integer port = port(request); - // Negative or Zero ports might represent an unset/null value for an int type. Skip setting. - if (port != null && port > 0) { - span.setTag(Tags.PEER_PORT, port); - } - - if (Config.get().isHttpClientSplitByDomain()) { - span.setTag(DDTags.SERVICE_NAME, hostname(request)); - } } return span; } diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecoratorTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecoratorTest.groovy index 6d5ad6e530..adf3b1a21c 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecoratorTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecoratorTest.groovy @@ -11,7 +11,7 @@ import static datadog.trace.agent.test.utils.ConfigUtils.withConfigOverride class HttpClientDecoratorTest extends ClientDecoratorTest { @Shared - def testUrl = new URI("http://myhost/somepath") + def testUrl = new URI("http://myhost:123/somepath") def span = Mock(AgentSpan) @@ -28,10 +28,10 @@ class HttpClientDecoratorTest extends ClientDecoratorTest { if (req) { 1 * span.setTag(Tags.HTTP_METHOD, req.method) 1 * span.setTag(Tags.HTTP_URL, "$req.url") - 1 * span.setTag(Tags.PEER_HOSTNAME, req.host) - 1 * span.setTag(Tags.PEER_PORT, req.port) + 1 * span.setTag(Tags.PEER_HOSTNAME, req.url.host) + 1 * span.setTag(Tags.PEER_PORT, req.url.port) if (renameService) { - 1 * span.setTag(DDTags.SERVICE_NAME, req.host) + 1 * span.setTag(DDTags.SERVICE_NAME, req.url.host) } } 0 * _ @@ -40,8 +40,8 @@ class HttpClientDecoratorTest extends ClientDecoratorTest { renameService | req false | null true | null - false | [method: "test-method", url: testUrl, host: "test-host", port: 555] - true | [method: "test-method", url: testUrl, host: "test-host", port: 555] + false | [method: "test-method", url: testUrl] + true | [method: "test-method", url: testUrl] } def "test url handling for #url"() { @@ -62,23 +62,28 @@ class HttpClientDecoratorTest extends ClientDecoratorTest { 1 * span.setTag(DDTags.HTTP_FRAGMENT, expectedFragment) } 1 * span.setTag(Tags.HTTP_METHOD, null) - 1 * span.setTag(Tags.PEER_HOSTNAME, null) + if (hostname) { + 1 * span.setTag(Tags.PEER_HOSTNAME, hostname) + } + if (port) { + 1 * span.setTag(Tags.PEER_PORT, port) + } 0 * _ where: - tagQueryString | url | expectedUrl | expectedQuery | expectedFragment - false | null | null | null | null - false | "" | "/" | "" | null - false | "/path?query" | "/path" | "" | null - false | "https://host:0" | "https://host/" | "" | null - false | "https://host/path" | "https://host/path" | "" | null - false | "http://host:99/path?query#fragment" | "http://host:99/path" | "" | null - true | null | null | null | null - true | "" | "/" | null | null - true | "/path?encoded+%28query%29%3F" | "/path" | "encoded+(query)?" | null - true | "https://host:0" | "https://host/" | null | null - true | "https://host/path" | "https://host/path" | null | null - true | "http://host:99/path?query#encoded+%28fragment%29%3F" | "http://host:99/path" | "query" | "encoded+(fragment)?" + tagQueryString | url | expectedUrl | expectedQuery | expectedFragment | hostname | port + false | null | null | null | null | null | null + false | "" | "/" | "" | null | null | null + false | "/path?query" | "/path" | "" | null | null | null + false | "https://host:0" | "https://host/" | "" | null | "host" | null + false | "https://host/path" | "https://host/path" | "" | null | "host" | null + false | "http://host:99/path?query#fragment" | "http://host:99/path" | "" | null | "host" | 99 + true | null | null | null | null | null | null + true | "" | "/" | null | null | null | null + true | "/path?encoded+%28query%29%3F" | "/path" | "encoded+(query)?" | null | null | null + true | "https://host:0" | "https://host/" | null | null | "host" | null + true | "https://host/path" | "https://host/path" | null | null | "host" | null + true | "http://host:99/path?query#encoded+%28fragment%29%3F" | "http://host:99/path" | "query" | "encoded+(fragment)?" | "host" | 99 req = [url: url == null ? null : new URI(url)] } @@ -160,16 +165,6 @@ class HttpClientDecoratorTest extends ClientDecoratorTest { return m.url } - @Override - protected String hostname(Map m) { - return m.host - } - - @Override - protected Integer port(Map m) { - return m.port - } - @Override protected Integer status(Map m) { return m.status diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/AkkaHttpClientDecorator.java b/dd-java-agent/instrumentation/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/AkkaHttpClientDecorator.java index 49d9dde4a3..0986a4bd47 100644 --- a/dd-java-agent/instrumentation/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/AkkaHttpClientDecorator.java +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/AkkaHttpClientDecorator.java @@ -29,16 +29,6 @@ public class AkkaHttpClientDecorator extends HttpClientDecorator { + public static final ApacheHttpAsyncClientDecorator DECORATE = new ApacheHttpAsyncClientDecorator(); @@ -50,34 +51,6 @@ public class ApacheHttpAsyncClientDecorator extends HttpClientDecorator { + public static final HttpUrlConnectionDecorator DECORATE = new HttpUrlConnectionDecorator(); @Override @@ -29,23 +29,6 @@ public class HttpUrlConnectionDecorator extends HttpClientDecorator 0) { - return port; - } else if (connection instanceof HttpsURLConnection) { - return 443; - } else { - return 80; - } - } - @Override protected Integer status(final Integer status) { return status; diff --git a/dd-java-agent/instrumentation/jax-rs-client-1.1/src/main/java/datadog/trace/instrumentation/jaxrs/v1/JaxRsClientV1Decorator.java b/dd-java-agent/instrumentation/jax-rs-client-1.1/src/main/java/datadog/trace/instrumentation/jaxrs/v1/JaxRsClientV1Decorator.java index 81eb892c7a..70b6558106 100644 --- a/dd-java-agent/instrumentation/jax-rs-client-1.1/src/main/java/datadog/trace/instrumentation/jaxrs/v1/JaxRsClientV1Decorator.java +++ b/dd-java-agent/instrumentation/jax-rs-client-1.1/src/main/java/datadog/trace/instrumentation/jaxrs/v1/JaxRsClientV1Decorator.java @@ -28,16 +28,6 @@ public class JaxRsClientV1Decorator extends HttpClientDecorator Date: Fri, 13 Mar 2020 11:06:00 -0700 Subject: [PATCH 2/2] oops --- .../aws-java-sdk-1.11.0/src/test/groovy/AWSClientTest.groovy | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test/groovy/AWSClientTest.groovy b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test/groovy/AWSClientTest.groovy index 2d83a4bc35..0e66a21c8e 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test/groovy/AWSClientTest.groovy +++ b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test/groovy/AWSClientTest.groovy @@ -1,6 +1,9 @@ +import com.amazonaws.AmazonClientException import com.amazonaws.AmazonWebServiceClient import com.amazonaws.ClientConfiguration import com.amazonaws.Request +import com.amazonaws.SDKGlobalConfiguration +import com.amazonaws.SdkClientException import com.amazonaws.auth.AWSCredentialsProviderChain import com.amazonaws.auth.AWSStaticCredentialsProvider import com.amazonaws.auth.AnonymousAWSCredentials