Merge pull request #1316 from aarya123/hostAndPort

Remove hostname and port from HttpClientDecorator
This commit is contained in:
Tyler Benson 2020-03-13 11:27:25 -07:00 committed by GitHub
commit a1fdfa51a6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
19 changed files with 51 additions and 280 deletions

View File

@ -16,10 +16,6 @@ public abstract class HttpClientDecorator<REQUEST, RESPONSE> 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<REQUEST, RESPONSE> 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<REQUEST, RESPONSE> 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;
}

View File

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

View File

@ -29,16 +29,6 @@ public class AkkaHttpClientDecorator extends HttpClientDecorator<HttpRequest, Ht
return new URI(httpRequest.uri().toString());
}
@Override
protected String hostname(final HttpRequest httpRequest) {
return httpRequest.getUri().host().address();
}
@Override
protected Integer port(final HttpRequest httpRequest) {
return httpRequest.getUri().port();
}
@Override
protected Integer status(final HttpResponse httpResponse) {
return httpResponse.status().intValue();

View File

@ -12,6 +12,7 @@ import org.apache.http.protocol.HttpContext;
import org.apache.http.protocol.HttpCoreContext;
public class ApacheHttpAsyncClientDecorator extends HttpClientDecorator<HttpRequest, HttpContext> {
public static final ApacheHttpAsyncClientDecorator DECORATE =
new ApacheHttpAsyncClientDecorator();
@ -50,34 +51,6 @@ public class ApacheHttpAsyncClientDecorator extends HttpClientDecorator<HttpRequ
}
}
@Override
protected String hostname(final HttpRequest request) {
try {
final URI uri = url(request);
if (uri != null) {
return uri.getHost();
} else {
return null;
}
} catch (final URISyntaxException e) {
return null;
}
}
@Override
protected Integer port(final HttpRequest request) {
try {
final URI uri = url(request);
if (uri != null) {
return uri.getPort();
} else {
return null;
}
} catch (final URISyntaxException e) {
return null;
}
}
@Override
protected Integer status(final HttpContext context) {
final Object responseObject = context.getAttribute(HttpCoreContext.HTTP_RESPONSE);

View File

@ -28,26 +28,6 @@ public class ApacheHttpClientDecorator extends HttpClientDecorator<HttpUriReques
return request.getURI();
}
@Override
protected String hostname(final HttpUriRequest httpRequest) {
final URI uri = httpRequest.getURI();
if (uri != null) {
return uri.getHost();
} else {
return null;
}
}
@Override
protected Integer port(final HttpUriRequest httpRequest) {
final URI uri = httpRequest.getURI();
if (uri != null) {
return uri.getPort();
} else {
return null;
}
}
@Override
protected Integer status(final HttpResponse httpResponse) {
return httpResponse.getStatusLine().getStatusCode();

View File

@ -105,16 +105,6 @@ public class AwsSdkClientDecorator extends HttpClientDecorator<Request, Response
return request.getEndpoint();
}
@Override
protected String hostname(final Request request) {
return null;
}
@Override
protected Integer port(final Request request) {
return null;
}
@Override
protected Integer status(final Response response) {
return response.getHttpResponse().getStatusCode();

View File

@ -146,6 +146,8 @@ class AWSClientTest extends AgentTestRunner {
"$Tags.HTTP_URL" "$server.address/"
"$Tags.HTTP_METHOD" "$method"
"$Tags.HTTP_STATUS" 200
"$Tags.PEER_PORT" server.address.port
"$Tags.PEER_HOSTNAME" "localhost"
"aws.service" { it.contains(service) }
"aws.endpoint" "$server.address"
"aws.operation" "${operation}Request"
@ -240,6 +242,8 @@ class AWSClientTest extends AgentTestRunner {
"$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT
"$Tags.HTTP_URL" "http://localhost:${UNUSABLE_PORT}/"
"$Tags.HTTP_METHOD" "$method"
"$Tags.PEER_HOSTNAME" "localhost"
"$Tags.PEER_PORT" 61
"aws.service" { it.contains(service) }
"aws.endpoint" "http://localhost:${UNUSABLE_PORT}"
"aws.operation" "${operation}Request"
@ -306,6 +310,7 @@ class AWSClientTest extends AgentTestRunner {
"$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT
"$Tags.HTTP_URL" "https://s3.amazonaws.com/"
"$Tags.HTTP_METHOD" "HEAD"
"$Tags.PEER_HOSTNAME" "s3.amazonaws.com"
"aws.service" "Amazon S3"
"aws.endpoint" "https://s3.amazonaws.com"
"aws.operation" "HeadBucketRequest"
@ -352,6 +357,8 @@ class AWSClientTest extends AgentTestRunner {
"$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT
"$Tags.HTTP_URL" "$server.address/"
"$Tags.HTTP_METHOD" "GET"
"$Tags.PEER_PORT" server.address.port
"$Tags.PEER_HOSTNAME" "localhost"
"aws.service" "Amazon S3"
"aws.endpoint" "$server.address"
"aws.operation" "GetObjectRequest"

View File

@ -109,6 +109,8 @@ class AWSClientTest extends AgentTestRunner {
"$Tags.HTTP_URL" "$server.address/"
"$Tags.HTTP_METHOD" "$method"
"$Tags.HTTP_STATUS" 200
"$Tags.PEER_PORT" server.address.port
"$Tags.PEER_HOSTNAME" "localhost"
"aws.service" { it.contains(service) }
"aws.endpoint" "$server.address"
"aws.operation" "${operation}Request"
@ -185,6 +187,8 @@ class AWSClientTest extends AgentTestRunner {
"$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT
"$Tags.HTTP_URL" "http://localhost:${UNUSABLE_PORT}/"
"$Tags.HTTP_METHOD" "$method"
"$Tags.PEER_PORT" 61
"$Tags.PEER_HOSTNAME" "localhost"
"aws.service" { it.contains(service) }
"aws.endpoint" "http://localhost:${UNUSABLE_PORT}"
"aws.operation" "${operation}Request"
@ -251,6 +255,7 @@ class AWSClientTest extends AgentTestRunner {
"$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT
"$Tags.HTTP_URL" "https://s3.amazonaws.com/"
"$Tags.HTTP_METHOD" "GET"
"$Tags.PEER_HOSTNAME" "s3.amazonaws.com"
"aws.service" "Amazon S3"
"aws.endpoint" "https://s3.amazonaws.com"
"aws.operation" "GetObjectRequest"
@ -298,6 +303,8 @@ class AWSClientTest extends AgentTestRunner {
"$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT
"$Tags.HTTP_URL" "$server.address/"
"$Tags.HTTP_METHOD" "GET"
"$Tags.PEER_PORT" server.address.port
"$Tags.PEER_HOSTNAME" "localhost"
"aws.service" "Amazon S3"
"aws.endpoint" "http://localhost:$server.address.port"
"aws.operation" "GetObjectRequest"

View File

@ -88,16 +88,6 @@ public class AwsSdkClientDecorator extends HttpClientDecorator<SdkHttpRequest, S
return request.getUri();
}
@Override
protected String hostname(final SdkHttpRequest request) {
return request.host();
}
@Override
protected Integer port(final SdkHttpRequest request) {
return request.port();
}
@Override
protected Integer status(final SdkHttpResponse response) {
return response.statusCode();

View File

@ -35,24 +35,6 @@ public class CommonsHttpClientDecorator extends HttpClientDecorator<HttpMethod,
}
}
@Override
protected String hostname(final HttpMethod httpMethod) {
try {
return httpMethod.getURI().getHost();
} catch (final URIException e) {
return null;
}
}
@Override
protected Integer port(final HttpMethod httpMethod) {
try {
return httpMethod.getURI().getPort();
} catch (final URIException e) {
return null;
}
}
@Override
protected Integer status(final HttpMethod httpMethod) {
final StatusLine statusLine = httpMethod.getStatusLine();

View File

@ -23,16 +23,6 @@ public class GoogleHttpClientDecorator extends HttpClientDecorator<HttpRequest,
return new URI(fixedUrl);
}
@Override
protected String hostname(final HttpRequest httpRequest) {
return httpRequest.getUrl().getHost();
}
@Override
protected Integer port(final HttpRequest httpRequest) {
return httpRequest.getUrl().getPort();
}
@Override
protected Integer status(final HttpResponse httpResponse) {
return httpResponse.getStatusCode();

View File

@ -4,9 +4,9 @@ import datadog.trace.bootstrap.instrumentation.decorator.HttpClientDecorator;
import java.net.HttpURLConnection;
import java.net.URI;
import java.net.URISyntaxException;
import javax.net.ssl.HttpsURLConnection;
public class HttpUrlConnectionDecorator extends HttpClientDecorator<HttpURLConnection, Integer> {
public static final HttpUrlConnectionDecorator DECORATE = new HttpUrlConnectionDecorator();
@Override
@ -29,23 +29,6 @@ public class HttpUrlConnectionDecorator extends HttpClientDecorator<HttpURLConne
return connection.getURL().toURI();
}
@Override
protected String hostname(final HttpURLConnection connection) {
return connection.getURL().getHost();
}
@Override
protected Integer port(final HttpURLConnection connection) {
final int port = connection.getURL().getPort();
if (port > 0) {
return port;
} else if (connection instanceof HttpsURLConnection) {
return 443;
} else {
return 80;
}
}
@Override
protected Integer status(final Integer status) {
return status;

View File

@ -28,16 +28,6 @@ public class JaxRsClientV1Decorator extends HttpClientDecorator<ClientRequest, C
return httpRequest.getURI();
}
@Override
protected String hostname(final ClientRequest httpRequest) {
return httpRequest.getURI().getHost();
}
@Override
protected Integer port(final ClientRequest httpRequest) {
return httpRequest.getURI().getPort();
}
@Override
protected Integer status(final ClientResponse clientResponse) {
return clientResponse.getStatus();

View File

@ -29,16 +29,6 @@ public class JaxRsClientDecorator
return httpRequest.getUri();
}
@Override
protected String hostname(final ClientRequestContext httpRequest) {
return httpRequest.getUri().getHost();
}
@Override
protected Integer port(final ClientRequestContext httpRequest) {
return httpRequest.getUri().getPort();
}
@Override
protected Integer status(final ClientResponseContext httpResponse) {
return httpResponse.getStatus();

View File

@ -38,35 +38,6 @@ public class NettyHttpClientDecorator extends HttpClientDecorator<HttpRequest, H
}
}
@Override
protected String hostname(final HttpRequest request) {
try {
final URI uri = new URI(request.getUri());
if ((uri.getHost() == null || uri.getHost().equals("")) && request.headers().contains(HOST)) {
return request.headers().get(HOST).split(":")[0];
} else {
return uri.getHost();
}
} catch (final Exception e) {
return null;
}
}
@Override
protected Integer port(final HttpRequest request) {
try {
final URI uri = new URI(request.getUri());
if ((uri.getHost() == null || uri.getHost().equals("")) && request.headers().contains(HOST)) {
final String[] hostPort = request.headers().get(HOST).split(":");
return hostPort.length == 2 ? Integer.parseInt(hostPort[1]) : null;
} else {
return uri.getPort();
}
} catch (final Exception e) {
return null;
}
}
@Override
protected Integer status(final HttpResponse httpResponse) {
return httpResponse.getStatus().code();

View File

@ -38,35 +38,6 @@ public class NettyHttpClientDecorator extends HttpClientDecorator<HttpRequest, H
}
}
@Override
protected String hostname(final HttpRequest request) {
try {
final URI uri = new URI(request.uri());
if ((uri.getHost() == null || uri.getHost().equals("")) && request.headers().contains(HOST)) {
return request.headers().get(HOST).split(":")[0];
} else {
return uri.getHost();
}
} catch (final Exception e) {
return null;
}
}
@Override
protected Integer port(final HttpRequest request) {
try {
final URI uri = new URI(request.uri());
if ((uri.getHost() == null || uri.getHost().equals("")) && request.headers().contains(HOST)) {
final String[] hostPort = request.headers().get(HOST).split(":");
return hostPort.length == 2 ? Integer.parseInt(hostPort[1]) : null;
} else {
return uri.getPort();
}
} catch (final Exception e) {
return null;
}
}
@Override
protected Integer status(final HttpResponse httpResponse) {
return httpResponse.status().code();

View File

@ -33,16 +33,6 @@ public class OkHttpClientDecorator extends HttpClientDecorator<Request, Response
return httpRequest.url().uri();
}
@Override
protected String hostname(final Request httpRequest) {
return httpRequest.url().host();
}
@Override
protected Integer port(final Request httpRequest) {
return httpRequest.url().port();
}
@Override
protected Integer status(final Response httpResponse) {
return httpResponse.code();

View File

@ -19,16 +19,6 @@ public class PlayWSClientDecorator extends HttpClientDecorator<Request, Response
return request.getUri().toJavaNetURI();
}
@Override
protected String hostname(final Request request) {
return request.getUri().getHost();
}
@Override
protected Integer port(final Request request) {
return request.getUri().getPort();
}
@Override
protected Integer status(final Response response) {
return response.getStatusCode();

View File

@ -39,16 +39,6 @@ public class SpringWebfluxHttpClientDecorator
return httpRequest.url();
}
@Override
protected String hostname(final ClientRequest httpRequest) {
return httpRequest.url().getHost();
}
@Override
protected Integer port(final ClientRequest httpRequest) {
return httpRequest.url().getPort();
}
@Override
protected Integer status(final ClientResponse httpResponse) {
return httpResponse.statusCode().value();