diff --git a/dd-java-agent/instrumentation/apache-httpclient-4.3/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientInstrumentation.java b/dd-java-agent/instrumentation/apache-httpclient-4.3/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientInstrumentation.java deleted file mode 100644 index 2e4fde528c..0000000000 --- a/dd-java-agent/instrumentation/apache-httpclient-4.3/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientInstrumentation.java +++ /dev/null @@ -1,86 +0,0 @@ -package datadog.trace.instrumentation.apachehttpclient; - -import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; -import static io.opentracing.log.Fields.ERROR_OBJECT; -import static net.bytebuddy.matcher.ElementMatchers.isAbstract; -import static net.bytebuddy.matcher.ElementMatchers.isMethod; -import static net.bytebuddy.matcher.ElementMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.not; - -import com.google.auto.service.AutoService; -import datadog.trace.agent.tooling.Instrumenter; -import io.opentracing.Scope; -import io.opentracing.Span; -import io.opentracing.Tracer; -import io.opentracing.tag.Tags; -import io.opentracing.util.GlobalTracer; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; -import net.bytebuddy.asm.Advice; -import net.bytebuddy.description.type.TypeDescription; -import net.bytebuddy.matcher.ElementMatcher; -import org.apache.http.impl.execchain.ClientExecChain; - -@AutoService(Instrumenter.class) -public class ApacheHttpClientInstrumentation extends Instrumenter.Default { - - public ApacheHttpClientInstrumentation() { - super("httpclient"); - } - - @Override - public ElementMatcher typeMatcher() { - return named("org.apache.http.impl.client.HttpClientBuilder") - .or(safeHasSuperType(named("org.apache.http.impl.client.CloseableHttpClient"))); - } - - @Override - public String[] helperClassNames() { - return new String[] { - "datadog.trace.instrumentation.apachehttpclient.DDTracingClientExec", - "datadog.trace.instrumentation.apachehttpclient.DDTracingClientExec$HttpHeadersInjectAdapter" - }; - } - - @Override - public Map transformers() { - final Map transformers = new HashMap<>(); - transformers.put( - isMethod().and(not(isAbstract())).and(named("doExecute")), ClientAdvice.class.getName()); - transformers.put( - isMethod().and(named("decorateProtocolExec")), ClientExecAdvice.class.getName()); - return transformers; - } - - public static class ClientAdvice { - - @Advice.OnMethodEnter(suppress = Throwable.class) - public static Scope methodEnter() { - final Tracer.SpanBuilder spanBuilder = - GlobalTracer.get() - .buildSpan(DDTracingClientExec.OPERATION_NAME) - .withTag(Tags.COMPONENT.getKey(), DDTracingClientExec.COMPONENT_NAME); - return spanBuilder.startActive(true); - } - - @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) - public static void methodExit( - @Advice.Enter final Scope scope, @Advice.Thrown final Throwable throwable) { - final Span span = scope.span(); - if (throwable != null) { - Tags.ERROR.set(span, Boolean.TRUE); - span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); - span.finish(); - } - scope.close(); - } - } - - public static class ClientExecAdvice { - @Advice.OnMethodExit(suppress = Throwable.class) - public static void addTracingExec(@Advice.Return(readOnly = false) ClientExecChain execChain) { - execChain = new DDTracingClientExec(execChain, GlobalTracer.get()); - } - } -} diff --git a/dd-java-agent/instrumentation/apache-httpclient-4.3/src/main/java/datadog/trace/instrumentation/apachehttpclient/DDTracingClientExec.java b/dd-java-agent/instrumentation/apache-httpclient-4.3/src/main/java/datadog/trace/instrumentation/apachehttpclient/DDTracingClientExec.java deleted file mode 100644 index f8d3aef521..0000000000 --- a/dd-java-agent/instrumentation/apache-httpclient-4.3/src/main/java/datadog/trace/instrumentation/apachehttpclient/DDTracingClientExec.java +++ /dev/null @@ -1,132 +0,0 @@ -package datadog.trace.instrumentation.apachehttpclient; - -import static io.opentracing.log.Fields.ERROR_OBJECT; - -import datadog.trace.api.DDSpanTypes; -import datadog.trace.api.DDTags; -import io.opentracing.Scope; -import io.opentracing.Span; -import io.opentracing.Tracer; -import io.opentracing.propagation.Format; -import io.opentracing.propagation.TextMap; -import io.opentracing.tag.Tags; -import java.io.IOException; -import java.net.URI; -import java.util.Collections; -import java.util.Iterator; -import java.util.Map; -import lombok.extern.slf4j.Slf4j; -import org.apache.http.HttpException; -import org.apache.http.HttpRequest; -import org.apache.http.client.methods.CloseableHttpResponse; -import org.apache.http.client.methods.HttpExecutionAware; -import org.apache.http.client.methods.HttpRequestWrapper; -import org.apache.http.client.protocol.HttpClientContext; -import org.apache.http.conn.routing.HttpRoute; -import org.apache.http.impl.execchain.ClientExecChain; - -/** - * Tracing is added before {@link org.apache.http.impl.execchain.ProtocolExec} which is invoked as - * the next to last. Note that {@link org.apache.http.impl.execchain.RedirectExec} is invoked before - * so this exec has to deal with redirects. - */ -@Slf4j -public class DDTracingClientExec implements ClientExecChain { - static final String COMPONENT_NAME = "apache-httpclient"; - static final String OPERATION_NAME = "http.request"; - - private final ClientExecChain requestExecutor; - - private final Tracer tracer; - - public DDTracingClientExec(final ClientExecChain clientExecChain, final Tracer tracer) { - requestExecutor = clientExecChain; - this.tracer = tracer; - } - - @Override - public CloseableHttpResponse execute( - final HttpRoute route, - final HttpRequestWrapper request, - final HttpClientContext clientContext, - final HttpExecutionAware execAware) - throws IOException, HttpException { - Scope scope = null; - Span span = null; - try { - // This handlers runs untrapped in the client code - // so we must ensure any unexpected agent errors are caught. - try { - scope = - tracer - .buildSpan(OPERATION_NAME) - .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT) - .withTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_CLIENT) - .startActive(true); - span = scope.span(); - - final boolean awsClientCall = request.getHeaders("amz-sdk-invocation-id").length > 0; - // AWS calls are often signed, so we can't add headers without breaking the signature. - if (!awsClientCall) { - tracer.inject( - span.context(), Format.Builtin.HTTP_HEADERS, new HttpHeadersInjectAdapter(request)); - } - // request tags - Tags.HTTP_METHOD.set(span, request.getRequestLine().getMethod()); - Tags.HTTP_URL.set(span, request.getRequestLine().getUri()); - final URI uri = request.getURI(); - // zuul users have encountered cases where getURI returns null - if (null != uri) { - Tags.PEER_PORT.set(span, uri.getPort() == -1 ? 80 : uri.getPort()); - Tags.PEER_HOSTNAME.set(span, uri.getHost()); - } - } catch (final Exception e) { - log.debug("failed to create span", e); - } - - final CloseableHttpResponse response = - requestExecutor.execute(route, request, clientContext, execAware); - - try { - // response tags - if (null != span) { - Tags.HTTP_STATUS.set(span, response.getStatusLine().getStatusCode()); - } - } catch (final Exception e) { - log.debug("failed to set span status", e); - } - - return response; - } catch (final IOException | HttpException | RuntimeException e) { - // error tags - Tags.ERROR.set(span, Boolean.TRUE); - span.log(Collections.singletonMap(ERROR_OBJECT, e)); - - throw e; - } finally { - if (null != scope) { - scope.close(); - } - } - } - - public static class HttpHeadersInjectAdapter implements TextMap { - - private final HttpRequest httpRequest; - - public HttpHeadersInjectAdapter(final HttpRequest httpRequest) { - this.httpRequest = httpRequest; - } - - @Override - public void put(final String key, final String value) { - httpRequest.addHeader(key, value); - } - - @Override - public Iterator> iterator() { - throw new UnsupportedOperationException( - "This class should be used only with tracer#inject()"); - } - } -} diff --git a/dd-java-agent/instrumentation/apache-httpclient-4.3/apache-httpclient-4.3.gradle b/dd-java-agent/instrumentation/apache-httpclient-4/apache-httpclient-4.gradle similarity index 85% rename from dd-java-agent/instrumentation/apache-httpclient-4.3/apache-httpclient-4.3.gradle rename to dd-java-agent/instrumentation/apache-httpclient-4/apache-httpclient-4.gradle index 75abaff407..0750d65549 100644 --- a/dd-java-agent/instrumentation/apache-httpclient-4.3/apache-httpclient-4.3.gradle +++ b/dd-java-agent/instrumentation/apache-httpclient-4/apache-httpclient-4.gradle @@ -4,12 +4,12 @@ muzzle { fail { group = "commons-httpclient" module = "commons-httpclient" - versions = "[,4.3)" + versions = "[,4.0)" } pass { group = "org.apache.httpcomponents" module = "httpclient" - versions = "[4.3,)" + versions = "[4.0,)" assertInverse = true } } @@ -17,13 +17,11 @@ muzzle { apply plugin: 'org.unbroken-dome.test-sets' testSets { - latestDepTest { - dirName = 'test' - } + latestDepTest } dependencies { - compileOnly group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.3' + compileOnly group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.0' compile project(':dd-java-agent:agent-tooling') @@ -33,8 +31,7 @@ dependencies { implementation deps.autoservice testCompile project(':dd-java-agent:testing') - testCompile group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.3' - + testCompile group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.0' latestDepTestCompile group: 'org.apache.httpcomponents', name: 'httpclient', version: '+' } diff --git a/dd-java-agent/instrumentation/apache-httpclient-4.3/src/test/groovy/ApacheHttpClientTest.groovy b/dd-java-agent/instrumentation/apache-httpclient-4/src/latestDepTest/groovy/ApacheHttpClientTest.groovy similarity index 73% rename from dd-java-agent/instrumentation/apache-httpclient-4.3/src/test/groovy/ApacheHttpClientTest.groovy rename to dd-java-agent/instrumentation/apache-httpclient-4/src/latestDepTest/groovy/ApacheHttpClientTest.groovy index b1adf35895..b77e34d627 100644 --- a/dd-java-agent/instrumentation/apache-httpclient-4.3/src/test/groovy/ApacheHttpClientTest.groovy +++ b/dd-java-agent/instrumentation/apache-httpclient-4/src/latestDepTest/groovy/ApacheHttpClientTest.groovy @@ -9,11 +9,13 @@ import org.apache.http.client.ClientProtocolException import org.apache.http.client.HttpClient import org.apache.http.client.config.RequestConfig import org.apache.http.client.methods.HttpGet +import org.apache.http.impl.client.BasicResponseHandler import org.apache.http.impl.client.HttpClientBuilder import org.apache.http.message.BasicHeader import spock.lang.AutoCleanup import spock.lang.Shared +import static datadog.trace.agent.test.TestUtils.runUnderTrace import static datadog.trace.agent.test.asserts.ListWriterAssert.assertTraces import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer @@ -25,7 +27,7 @@ class ApacheHttpClientTest extends AgentTestRunner { handlers { prefix("success") { handleDistributedRequest() - String msg = "

Hello test.

\n" + String msg = "Hello." response.status(200).send(msg) } prefix("redirect") { @@ -46,24 +48,35 @@ class ApacheHttpClientTest extends AgentTestRunner { def redirectUrl = server.address.resolve("/redirect") @Shared def twoRedirectsUrl = server.address.resolve("/another-redirect") + @Shared + def handler = new BasicResponseHandler() final HttpClientBuilder builder = HttpClientBuilder.create() final HttpClient client = builder.build() def "trace request with propagation"() { when: - HttpResponse response = client.execute(new HttpGet(successUrl)) + String response = runUnderTrace("parent") { + if (responseHandler) { + client.execute(new HttpGet(successUrl), responseHandler) + } else { + client.execute(new HttpGet(successUrl)).entity.content.text + } + } then: - response.getStatusLine().getStatusCode() == 200 + response == "Hello." // one trace on the server, one trace on the client assertTraces(TEST_WRITER, 2) { server.distributedRequestTrace(it, 0, TEST_WRITER[1][1]) trace(1, 2) { - clientParentSpan(it, 0) + parentSpan(it, 0) successClientSpan(it, 1, span(0)) } } + + where: + responseHandler << [null, handler] } def "trace redirected request with propagation many redirects allowed"() { @@ -75,18 +88,19 @@ class ApacheHttpClientTest extends AgentTestRunner { request.setConfig(requestConfigBuilder.build()) when: - HttpResponse response = client.execute(request) + HttpResponse response = runUnderTrace("parent") { + client.execute(request) + } then: response.getStatusLine().getStatusCode() == 200 // two traces on the server, one trace on the client assertTraces(TEST_WRITER, 3) { - server.distributedRequestTrace(it, 0, TEST_WRITER[2][2]) + server.distributedRequestTrace(it, 0, TEST_WRITER[2][1]) server.distributedRequestTrace(it, 1, TEST_WRITER[2][1]) - trace(2, 3) { - clientParentSpan(it, 0) - successClientSpan(it, 1, span(0)) - redirectClientSpan(it, 2, span(0)) + trace(2, 2) { + parentSpan(it, 0) + successClientSpan(it, 1, span(0), 200, "redirect") } } } @@ -99,18 +113,19 @@ class ApacheHttpClientTest extends AgentTestRunner { request.setConfig(requestConfigBuilder.build()) when: - HttpResponse response = client.execute(request) + HttpResponse response = runUnderTrace("parent") { + client.execute(request) + } then: response.getStatusLine().getStatusCode() == 200 // two traces on the server, one trace on the client assertTraces(TEST_WRITER, 3) { - server.distributedRequestTrace(it, 0, TEST_WRITER[2][2]) + server.distributedRequestTrace(it, 0, TEST_WRITER[2][1]) server.distributedRequestTrace(it, 1, TEST_WRITER[2][1]) - trace(2, 3) { - clientParentSpan(it, 0) - successClientSpan(it, 1, span(0)) - redirectClientSpan(it, 2, span(0)) + trace(2, 2) { + parentSpan(it, 0) + successClientSpan(it, 1, span(0), 200, "redirect") } } } @@ -124,18 +139,19 @@ class ApacheHttpClientTest extends AgentTestRunner { request.setConfig(requestConfigBuilder.build()) when: - client.execute(request) + runUnderTrace("parent") { + client.execute(request) + } then: def exception = thrown(ClientProtocolException) // two traces on the server, one trace on the client assertTraces(TEST_WRITER, 3) { - server.distributedRequestTrace(it, 0, TEST_WRITER[2][2]) + server.distributedRequestTrace(it, 0, TEST_WRITER[2][1]) server.distributedRequestTrace(it, 1, TEST_WRITER[2][1]) - trace(2, 3) { - clientParentSpan(it, 0, exception) - redirectClientSpan(it, 1, span(0)) - redirectClientSpan(it, 2, span(0), "another-redirect") + trace(2, 2) { + parentSpan(it, 0, exception) + successClientSpan(it, 1, span(0), null, "another-redirect", exception) } } } @@ -146,29 +162,30 @@ class ApacheHttpClientTest extends AgentTestRunner { request.addHeader(new BasicHeader("is-dd-server", "false")) when: - HttpResponse response = client.execute(request) + HttpResponse response = runUnderTrace("parent") { + client.execute(request) + } then: response.getStatusLine().getStatusCode() == 200 // only one trace (client). assertTraces(TEST_WRITER, 1) { trace(0, 2) { - clientParentSpan(it, 0) + parentSpan(it, 0) successClientSpan(it, 1, span(0)) } } } - def clientParentSpan(TraceAssert trace, int index, Throwable exception = null) { + def parentSpan(TraceAssert trace, int index, Throwable exception = null) { trace.span(index) { parent() serviceName "unnamed-java-app" - operationName "apache.http" - resourceName "apache.http" + operationName "parent" + resourceName "parent" errored exception != null tags { defaultTags() - "$Tags.COMPONENT.key" "apache-httpclient" if (exception) { errorTags(exception.class) } @@ -176,15 +193,19 @@ class ApacheHttpClientTest extends AgentTestRunner { } } - def successClientSpan(TraceAssert trace, int index, DDSpan parent, status = 200, route = "success") { + def successClientSpan(TraceAssert trace, int index, DDSpan parent, status = 200, route = "success", Throwable exception = null) { trace.span(index) { childOf parent serviceName "unnamed-java-app" operationName "http.request" resourceName "GET /$route" - errored false + errored exception != null tags { defaultTags() + if (exception) { + errorTags(exception.class) + } + "$Tags.COMPONENT.key" "apache-httpclient" "$Tags.HTTP_STATUS.key" status "$Tags.HTTP_URL.key" "http://localhost:$port/$route" "$Tags.PEER_HOSTNAME.key" "localhost" @@ -195,8 +216,4 @@ class ApacheHttpClientTest extends AgentTestRunner { } } } - - def redirectClientSpan(TraceAssert trace, int index, DDSpan parent, route = "redirect") { - successClientSpan(trace, index, parent, 302, route) - } } diff --git a/dd-java-agent/instrumentation/apache-httpclient-4/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientInstrumentation.java b/dd-java-agent/instrumentation/apache-httpclient-4/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientInstrumentation.java new file mode 100644 index 0000000000..1c88f1128b --- /dev/null +++ b/dd-java-agent/instrumentation/apache-httpclient-4/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientInstrumentation.java @@ -0,0 +1,187 @@ +package datadog.trace.instrumentation.apachehttpclient; + +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; +import static io.opentracing.log.Fields.ERROR_OBJECT; +import static net.bytebuddy.matcher.ElementMatchers.isAbstract; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.api.DDSpanTypes; +import datadog.trace.api.DDTags; +import datadog.trace.bootstrap.CallDepthThreadLocalMap; +import io.opentracing.Scope; +import io.opentracing.Span; +import io.opentracing.Tracer; +import io.opentracing.propagation.Format; +import io.opentracing.propagation.TextMap; +import io.opentracing.tag.Tags; +import io.opentracing.util.GlobalTracer; +import java.io.IOException; +import java.net.URI; +import java.util.Collections; +import java.util.Iterator; +import java.util.Map; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.implementation.bytecode.assign.Assigner; +import net.bytebuddy.matcher.ElementMatcher; +import org.apache.http.HttpRequest; +import org.apache.http.HttpResponse; +import org.apache.http.client.ClientProtocolException; +import org.apache.http.client.HttpClient; +import org.apache.http.client.ResponseHandler; +import org.apache.http.client.methods.HttpUriRequest; + +@AutoService(Instrumenter.class) +public class ApacheHttpClientInstrumentation extends Instrumenter.Default { + + public ApacheHttpClientInstrumentation() { + super("httpclient"); + } + + @Override + public ElementMatcher typeMatcher() { + return safeHasSuperType(named("org.apache.http.client.HttpClient")); + } + + @Override + public String[] helperClassNames() { + return new String[] { + getClass().getName() + "$HttpHeadersInjectAdapter", + getClass().getName() + "$WrappingStatusSettingResponseHandler", + }; + } + + @Override + public Map transformers() { + return Collections.singletonMap( + isMethod() + .and(not(isAbstract())) + .and(named("execute")) + .and(takesArgument(0, named("org.apache.http.client.methods.HttpUriRequest"))), + ClientAdvice.class.getName()); + } + + public static class ClientAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static Scope methodEnter( + @Advice.Argument(0) final HttpUriRequest request, + // ResponseHandler could be either slot, but not both. + @Advice.Argument( + value = 1, + optional = true, + typing = Assigner.Typing.DYNAMIC, + readOnly = false) + Object handler1, + @Advice.Argument( + value = 2, + optional = true, + typing = Assigner.Typing.DYNAMIC, + readOnly = false) + Object handler2) { + final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(HttpClient.class); + if (callDepth > 0) { + return null; + } + final Tracer tracer = GlobalTracer.get(); + final Scope scope = + tracer + .buildSpan("http.request") + .withTag(Tags.COMPONENT.getKey(), "apache-httpclient") + .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT) + .withTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_CLIENT) + .withTag(Tags.HTTP_METHOD.getKey(), request.getRequestLine().getMethod()) + .withTag(Tags.HTTP_URL.getKey(), request.getRequestLine().getUri()) + .startActive(true); + + final Span span = scope.span(); + + // Wrap the handler so we capture the status code + if (handler1 instanceof ResponseHandler) { + handler1 = new WrappingStatusSettingResponseHandler(span, (ResponseHandler) handler1); + } else if (handler2 instanceof ResponseHandler) { + handler2 = new WrappingStatusSettingResponseHandler(span, (ResponseHandler) handler2); + } + + final boolean awsClientCall = request.getHeaders("amz-sdk-invocation-id").length > 0; + // AWS calls are often signed, so we can't add headers without breaking the signature. + if (!awsClientCall) { + tracer.inject( + span.context(), Format.Builtin.HTTP_HEADERS, new HttpHeadersInjectAdapter(request)); + } + final URI uri = request.getURI(); + // zuul users have encountered cases where getURI returns null + if (null != uri) { + Tags.PEER_PORT.set(span, uri.getPort() == -1 ? 80 : uri.getPort()); + Tags.PEER_HOSTNAME.set(span, uri.getHost()); + } + return scope; + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void methodExit( + @Advice.Enter final Scope scope, + @Advice.Return final Object result, + @Advice.Thrown final Throwable throwable) { + if (scope != null) { + final Span span = scope.span(); + + if (result instanceof HttpResponse) { + Tags.HTTP_STATUS.set(span, ((HttpResponse) result).getStatusLine().getStatusCode()); + } // else they probably provided a ResponseHandler. + + if (throwable != null) { + Tags.ERROR.set(span, Boolean.TRUE); + span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); + span.finish(); + } + scope.close(); + CallDepthThreadLocalMap.reset(HttpClient.class); + } + } + } + + public static class WrappingStatusSettingResponseHandler implements ResponseHandler { + final Span span; + final ResponseHandler handler; + + public WrappingStatusSettingResponseHandler(final Span span, final ResponseHandler handler) { + this.span = span; + this.handler = handler; + } + + @Override + public Object handleResponse(final HttpResponse response) + throws ClientProtocolException, IOException { + if (null != span) { + Tags.HTTP_STATUS.set(span, response.getStatusLine().getStatusCode()); + } + return handler.handleResponse(response); + } + } + + public static class HttpHeadersInjectAdapter implements TextMap { + + private final HttpRequest httpRequest; + + public HttpHeadersInjectAdapter(final HttpRequest httpRequest) { + this.httpRequest = httpRequest; + } + + @Override + public void put(final String key, final String value) { + httpRequest.addHeader(key, value); + } + + @Override + public Iterator> iterator() { + throw new UnsupportedOperationException( + "This class should be used only with tracer#inject()"); + } + } +} diff --git a/dd-java-agent/instrumentation/apache-httpclient-4/src/test/groovy/ApacheHttpClientTest.groovy b/dd-java-agent/instrumentation/apache-httpclient-4/src/test/groovy/ApacheHttpClientTest.groovy new file mode 100644 index 0000000000..7eb08c933b --- /dev/null +++ b/dd-java-agent/instrumentation/apache-httpclient-4/src/test/groovy/ApacheHttpClientTest.groovy @@ -0,0 +1,139 @@ +import datadog.opentracing.DDSpan +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.asserts.TraceAssert +import datadog.trace.api.DDSpanTypes +import datadog.trace.api.DDTags +import io.opentracing.tag.Tags +import org.apache.http.HttpResponse +import org.apache.http.client.HttpClient +import org.apache.http.client.methods.HttpGet +import org.apache.http.impl.client.BasicResponseHandler +import org.apache.http.impl.client.DefaultHttpClient +import org.apache.http.message.BasicHeader +import spock.lang.AutoCleanup +import spock.lang.Shared + +import static datadog.trace.agent.test.TestUtils.runUnderTrace +import static datadog.trace.agent.test.asserts.ListWriterAssert.assertTraces +import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer + +class ApacheHttpClientTest extends AgentTestRunner { + + @AutoCleanup + @Shared + def server = httpServer { + handlers { + prefix("success") { + handleDistributedRequest() + String msg = "Hello." + response.status(200).send(msg) + } + prefix("redirect") { + handleDistributedRequest() + redirect(server.address.resolve("/success").toURL().toString()) + } + prefix("another-redirect") { + handleDistributedRequest() + redirect(server.address.resolve("/redirect").toURL().toString()) + } + } + } + @Shared + int port = server.address.port + @Shared + def successUrl = server.address.resolve("/success") + @Shared + def redirectUrl = server.address.resolve("/redirect") + @Shared + def twoRedirectsUrl = server.address.resolve("/another-redirect") + @Shared + def handler = new BasicResponseHandler() + + final HttpClient client = new DefaultHttpClient() + + def "trace request with propagation"() { + when: + String response = runUnderTrace("parent") { + if (responseHandler) { + client.execute(new HttpGet(successUrl), responseHandler) + } else { + client.execute(new HttpGet(successUrl)).entity.content.text + } + } + + then: + response == "Hello." + // one trace on the server, one trace on the client + assertTraces(TEST_WRITER, 2) { + server.distributedRequestTrace(it, 0, TEST_WRITER[1][1]) + trace(1, 2) { + parentSpan(it, 0) + successClientSpan(it, 1, span(0)) + } + } + + where: + responseHandler << [null, handler] + } + + def "trace request without propagation"() { + setup: + HttpGet request = new HttpGet(successUrl) + request.addHeader(new BasicHeader("is-dd-server", "false")) + + when: + HttpResponse response = runUnderTrace("parent") { + client.execute(request) + } + + then: + response.getStatusLine().getStatusCode() == 200 + // only one trace (client). + assertTraces(TEST_WRITER, 1) { + trace(0, 2) { + parentSpan(it, 0) + successClientSpan(it, 1, span(0)) + } + } + } + + def parentSpan(TraceAssert trace, int index, Throwable exception = null) { + trace.span(index) { + parent() + serviceName "unnamed-java-app" + operationName "parent" + resourceName "parent" + errored exception != null + tags { + defaultTags() + if (exception) { + errorTags(exception.class) + } + } + } + } + + def successClientSpan(TraceAssert trace, int index, DDSpan parent, status = 200, route = "success", Throwable exception = null) { + trace.span(index) { + childOf parent + serviceName "unnamed-java-app" + operationName "http.request" + resourceName "GET /$route" + errored exception != null + tags { + defaultTags() + if (exception) { + errorTags(exception.class) + } + "$Tags.COMPONENT.key" "apache-httpclient" + "$Tags.HTTP_STATUS.key" status + "$Tags.HTTP_URL.key" "http://localhost:$port/$route" + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" server.address.port + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT + } + } + } +} diff --git a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/aws-java-sdk-1.11.0.gradle b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/aws-java-sdk-1.11.0.gradle index b2c670c6e4..dfe601732e 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/aws-java-sdk-1.11.0.gradle +++ b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/aws-java-sdk-1.11.0.gradle @@ -58,7 +58,7 @@ dependencies { testCompile project(':dd-java-agent:testing') // Include httpclient instrumentation for testing because it is a dependency for aws-sdk. - testCompile project(':dd-java-agent:instrumentation:apache-httpclient-4.3') + testCompile project(':dd-java-agent:instrumentation:apache-httpclient-4') testCompile group: 'com.amazonaws', name: 'aws-java-sdk', version: '1.11.0' } diff --git a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/SpanDecorator.java b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/SpanDecorator.java index 7551bdb1e9..6727fcc2f5 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/SpanDecorator.java +++ b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/SpanDecorator.java @@ -13,21 +13,16 @@ */ package datadog.trace.instrumentation.aws.v0; -import static io.opentracing.log.Fields.ERROR_KIND; import static io.opentracing.log.Fields.ERROR_OBJECT; -import static io.opentracing.log.Fields.EVENT; -import static io.opentracing.log.Fields.MESSAGE; -import static io.opentracing.log.Fields.STACK; import com.amazonaws.AmazonWebServiceResponse; import com.amazonaws.Request; import com.amazonaws.Response; +import datadog.trace.api.DDSpanTypes; import datadog.trace.api.DDTags; import io.opentracing.Span; import io.opentracing.tag.Tags; -import java.io.PrintWriter; -import java.io.StringWriter; -import java.util.HashMap; +import java.util.Collections; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -50,9 +45,11 @@ class SpanDecorator { span.setTag("aws.operation", awsOperation.getSimpleName()); span.setTag("aws.endpoint", request.getEndpoint().toString()); + span.setTag(DDTags.SERVICE_NAME, COMPONENT_NAME); span.setTag( DDTags.RESOURCE_NAME, remapServiceName(awsServiceName) + "." + remapOperationName(awsOperation)); + span.setTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_CLIENT); } static void onResponse(final Response response, final Span span) { @@ -65,22 +62,7 @@ class SpanDecorator { static void onError(final Throwable throwable, final Span span) { Tags.ERROR.set(span, Boolean.TRUE); - span.log(errorLogs(throwable)); - } - - private static Map errorLogs(final Throwable throwable) { - final Map errorLogs = new HashMap<>(4); - errorLogs.put(EVENT, Tags.ERROR.getKey()); - errorLogs.put(ERROR_KIND, throwable.getClass().getName()); - errorLogs.put(ERROR_OBJECT, throwable); - - errorLogs.put(MESSAGE, throwable.getMessage()); - - final StringWriter sw = new StringWriter(); - throwable.printStackTrace(new PrintWriter(sw)); - errorLogs.put(STACK, sw.toString()); - - return errorLogs; + span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); } private static String remapServiceName(final String serviceName) { 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 919f3455d6..4a91492656 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 @@ -7,6 +7,7 @@ import com.amazonaws.services.rds.model.DeleteOptionGroupRequest import com.amazonaws.services.s3.AmazonS3Client import com.amazonaws.services.s3.S3ClientOptions import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.api.DDSpanTypes import datadog.trace.api.DDTags import io.opentracing.tag.Tags import spock.lang.AutoCleanup @@ -14,6 +15,7 @@ import spock.lang.Shared import java.util.concurrent.atomic.AtomicReference +import static datadog.trace.agent.test.asserts.ListWriterAssert.assertTraces import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer class AWSClientTest extends AgentTestRunner { @@ -72,81 +74,52 @@ class AWSClientTest extends AgentTestRunner { client.requestHandler2s.size() == handlerCount client.requestHandler2s.get(0).getClass().getSimpleName() == "TracingRequestHandler" - TEST_WRITER.size() == 2 - - def trace = TEST_WRITER.get(0) - trace.size() == 2 - - and: // span 0 - from apache-httpclient instrumentation - def span1 = trace[0] - - span1.context().operationName == "apache.http" - span1.serviceName == "unnamed-java-app" - span1.resourceName == "apache.http" - span1.type == null - !span1.context().getErrorFlag() - span1.context().parentId == "0" - - - def tags1 = span1.context().tags - tags1["component"] == "apache-httpclient" - tags1["thread.name"] != null - tags1["thread.id"] != null - tags1.size() == 3 - - and: // span 1 - from apache-httpclient instrumentation - def span2 = trace[1] - - span2.context().operationName == "http.request" - span2.serviceName == "unnamed-java-app" - span2.resourceName == "$method /$url" - span2.type == "http" - !span2.context().getErrorFlag() - span2.context().parentId == span1.spanId - - - def tags2 = span2.context().tags - tags2[Tags.SPAN_KIND.key] == Tags.SPAN_KIND_CLIENT - tags2[Tags.HTTP_METHOD.key] == "$method" - tags2[Tags.HTTP_URL.key] == "http://localhost:$server.address.port/$url" - tags2[Tags.PEER_HOSTNAME.key] == "localhost" - tags2[Tags.PEER_PORT.key] == server.address.port - tags2[DDTags.THREAD_NAME] != null - tags2[DDTags.THREAD_ID] != null - tags2.size() == 9 - - and: - - def trace2 = TEST_WRITER.get(1) - trace2.size() == 1 - - and: // span 0 - from aws instrumentation - def span = trace2[0] - - span.context().operationName == "aws.http" - span.serviceName == "java-aws-sdk" - span.resourceName == "$service.$operation" - span.type == "web" - !span.context().getErrorFlag() - span.context().parentId == "0" - - def tags = span.context().tags - tags[Tags.COMPONENT.key] == "java-aws-sdk" - tags[Tags.SPAN_KIND.key] == Tags.SPAN_KIND_CLIENT - tags[Tags.HTTP_METHOD.key] == "$method" - tags[Tags.HTTP_URL.key] == "http://localhost:$server.address.port" - tags[Tags.HTTP_STATUS.key] == 200 - tags["aws.service"] == "Amazon $service" || tags["aws.service"] == "Amazon$service" - tags["aws.endpoint"] == "http://localhost:$server.address.port" - tags["aws.operation"] == "${operation}Request" - tags["aws.agent"] == "java-aws-sdk" - tags["span.type"] == "web" - tags["thread.name"] != null - tags["thread.id"] != null - tags.size() == 12 - - server.lastRequest.headers.get("x-datadog-trace-id") == "$span.traceId" - server.lastRequest.headers.get("x-datadog-parent-id") == "$span.spanId" + assertTraces(TEST_WRITER, 2) { + trace(0, 1) { + span(0) { + operationName "http.request" + resourceName "$method /$url" + errored false + parent() // FIXME: This should be a child of the aws.http call. + tags { + "$Tags.COMPONENT.key" "apache-httpclient" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "$server.address/$url" + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" server.address.port + "$Tags.HTTP_METHOD.key" "$method" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT + defaultTags() + } + } + } + trace(1, 1) { + span(0) { + serviceName "java-aws-sdk" + operationName "aws.http" + resourceName "$service.$operation" + errored false + parent() + tags { + "$Tags.COMPONENT.key" "java-aws-sdk" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "$server.address" + "$Tags.HTTP_METHOD.key" "$method" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT + "aws.service" String + "aws.endpoint" "$server.address" + "aws.operation" "${operation}Request" + "aws.agent" "java-aws-sdk" + defaultTags() + } + } + } + } + // Not sure why these are children of the aws.http span: + server.lastRequest.headers.get("x-datadog-trace-id") == TEST_WRITER[1][0].traceId + server.lastRequest.headers.get("x-datadog-parent-id") == TEST_WRITER[1][0].spanId where: service | operation | method | url | handlerCount | call | body | client diff --git a/dd-java-agent/instrumentation/aws-java-sdk-1.11.106/aws-java-sdk-1.11.106.gradle b/dd-java-agent/instrumentation/aws-java-sdk-1.11.106/aws-java-sdk-1.11.106.gradle index f83c29be0b..e5b01afdb3 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-1.11.106/aws-java-sdk-1.11.106.gradle +++ b/dd-java-agent/instrumentation/aws-java-sdk-1.11.106/aws-java-sdk-1.11.106.gradle @@ -31,7 +31,7 @@ dependencies { testCompile project(':dd-java-agent:testing') // Include httpclient instrumentation for testing because it is a dependency for aws-sdk. - testCompile project(':dd-java-agent:instrumentation:apache-httpclient-4.3') + testCompile project(':dd-java-agent:instrumentation:apache-httpclient-4') testCompile group: 'com.amazonaws', name: 'aws-java-sdk', version: '1.11.106' } diff --git a/dd-java-agent/instrumentation/aws-java-sdk-1.11.106/src/main/java/datadog/trace/instrumentation/aws/v106/SpanDecorator.java b/dd-java-agent/instrumentation/aws-java-sdk-1.11.106/src/main/java/datadog/trace/instrumentation/aws/v106/SpanDecorator.java index a827ae97a9..851ad369f8 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-1.11.106/src/main/java/datadog/trace/instrumentation/aws/v106/SpanDecorator.java +++ b/dd-java-agent/instrumentation/aws-java-sdk-1.11.106/src/main/java/datadog/trace/instrumentation/aws/v106/SpanDecorator.java @@ -13,21 +13,16 @@ */ package datadog.trace.instrumentation.aws.v106; -import static io.opentracing.log.Fields.ERROR_KIND; import static io.opentracing.log.Fields.ERROR_OBJECT; -import static io.opentracing.log.Fields.EVENT; -import static io.opentracing.log.Fields.MESSAGE; -import static io.opentracing.log.Fields.STACK; import com.amazonaws.AmazonWebServiceResponse; import com.amazonaws.Request; import com.amazonaws.Response; +import datadog.trace.api.DDSpanTypes; import datadog.trace.api.DDTags; import io.opentracing.Span; import io.opentracing.tag.Tags; -import java.io.PrintWriter; -import java.io.StringWriter; -import java.util.HashMap; +import java.util.Collections; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -50,9 +45,11 @@ class SpanDecorator { span.setTag("aws.operation", awsOperation.getSimpleName()); span.setTag("aws.endpoint", request.getEndpoint().toString()); + span.setTag(DDTags.SERVICE_NAME, COMPONENT_NAME); span.setTag( DDTags.RESOURCE_NAME, remapServiceName(awsServiceName) + "." + remapOperationName(awsOperation)); + span.setTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_CLIENT); } static void onResponse(final Response response, final Span span) { @@ -65,22 +62,7 @@ class SpanDecorator { static void onError(final Throwable throwable, final Span span) { Tags.ERROR.set(span, Boolean.TRUE); - span.log(errorLogs(throwable)); - } - - private static Map errorLogs(final Throwable throwable) { - final Map errorLogs = new HashMap<>(4); - errorLogs.put(EVENT, Tags.ERROR.getKey()); - errorLogs.put(ERROR_KIND, throwable.getClass().getName()); - errorLogs.put(ERROR_OBJECT, throwable); - - errorLogs.put(MESSAGE, throwable.getMessage()); - - final StringWriter sw = new StringWriter(); - throwable.printStackTrace(new PrintWriter(sw)); - errorLogs.put(STACK, sw.toString()); - - return errorLogs; + span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); } private static String remapServiceName(final String serviceName) { diff --git a/dd-java-agent/instrumentation/aws-java-sdk-1.11.106/src/test/groovy/AWSClientTest.groovy b/dd-java-agent/instrumentation/aws-java-sdk-1.11.106/src/test/groovy/AWSClientTest.groovy index 4cd8b7da7f..88dac306c3 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-1.11.106/src/test/groovy/AWSClientTest.groovy +++ b/dd-java-agent/instrumentation/aws-java-sdk-1.11.106/src/test/groovy/AWSClientTest.groovy @@ -12,6 +12,7 @@ import com.amazonaws.services.rds.model.DeleteOptionGroupRequest import com.amazonaws.services.s3.AmazonS3Client import com.amazonaws.services.s3.AmazonS3ClientBuilder import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.api.DDSpanTypes import datadog.trace.api.DDTags import io.opentracing.tag.Tags import spock.lang.AutoCleanup @@ -19,6 +20,7 @@ import spock.lang.Shared import java.util.concurrent.atomic.AtomicReference +import static datadog.trace.agent.test.asserts.ListWriterAssert.assertTraces import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer class AWSClientTest extends AgentTestRunner { @@ -101,81 +103,52 @@ class AWSClientTest extends AgentTestRunner { client.requestHandler2s.size() == handlerCount client.requestHandler2s.get(0).getClass().getSimpleName() == "TracingRequestHandler" - TEST_WRITER.size() == 2 - - def trace = TEST_WRITER.get(0) - trace.size() == 2 - - and: // span 0 - from apache-httpclient instrumentation - def span1 = trace[0] - - span1.context().operationName == "apache.http" - span1.serviceName == "unnamed-java-app" - span1.resourceName == "apache.http" - span1.type == null - !span1.context().getErrorFlag() - span1.context().parentId == "0" - - - def tags1 = span1.context().tags - tags1["component"] == "apache-httpclient" - tags1["thread.name"] != null - tags1["thread.id"] != null - tags1.size() == 3 - - and: // span 1 - from apache-httpclient instrumentation - def span2 = trace[1] - - span2.context().operationName == "http.request" - span2.serviceName == "unnamed-java-app" - span2.resourceName == "$method /$url" - span2.type == "http" - !span2.context().getErrorFlag() - span2.context().parentId == span1.spanId - - - def tags2 = span2.context().tags - tags2[Tags.SPAN_KIND.key] == Tags.SPAN_KIND_CLIENT - tags2[Tags.HTTP_METHOD.key] == "$method" - tags2[Tags.HTTP_URL.key] == "http://localhost:$server.address.port/$url" - tags2[Tags.PEER_HOSTNAME.key] == "localhost" - tags2[Tags.PEER_PORT.key] == server.address.port - tags2[DDTags.THREAD_NAME] != null - tags2[DDTags.THREAD_ID] != null - tags2.size() == 9 - - and: - - def trace2 = TEST_WRITER.get(1) - trace2.size() == 1 - - and: // span 0 - from aws instrumentation - def span = trace2[0] - - span.context().operationName == "aws.http" - span.serviceName == "java-aws-sdk" - span.resourceName == "$service.$operation" - span.type == "web" - !span.context().getErrorFlag() - span.context().parentId == "0" - - def tags = span.context().tags - tags[Tags.COMPONENT.key] == "java-aws-sdk" - tags[Tags.SPAN_KIND.key] == Tags.SPAN_KIND_CLIENT - tags[Tags.HTTP_METHOD.key] == "$method" - tags[Tags.HTTP_URL.key] == "http://localhost:$server.address.port" - tags[Tags.HTTP_STATUS.key] == 200 - tags["aws.service"] == "Amazon $service" || tags["aws.service"] == "Amazon$service" - tags["aws.endpoint"] == "http://localhost:$server.address.port" - tags["aws.operation"] == "${operation}Request" - tags["aws.agent"] == "java-aws-sdk" - tags["span.type"] == "web" - tags["thread.name"] != null - tags["thread.id"] != null - tags.size() == 12 - - server.lastRequest.headers.get("x-datadog-trace-id") == "$span.traceId" - server.lastRequest.headers.get("x-datadog-parent-id") == "$span.spanId" + assertTraces(TEST_WRITER, 2) { + trace(0, 1) { + span(0) { + operationName "http.request" + resourceName "$method /$url" + errored false + parent() // FIXME: This should be a child of the aws.http call. + tags { + "$Tags.COMPONENT.key" "apache-httpclient" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "$server.address/$url" + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" server.address.port + "$Tags.HTTP_METHOD.key" "$method" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT + defaultTags() + } + } + } + trace(1, 1) { + span(0) { + serviceName "java-aws-sdk" + operationName "aws.http" + resourceName "$service.$operation" + errored false + parent() + tags { + "$Tags.COMPONENT.key" "java-aws-sdk" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "$server.address" + "$Tags.HTTP_METHOD.key" "$method" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT + "aws.service" String + "aws.endpoint" "$server.address" + "aws.operation" "${operation}Request" + "aws.agent" "java-aws-sdk" + defaultTags() + } + } + } + } + // Not sure why these are children of the aws.http span: + server.lastRequest.headers.get("x-datadog-trace-id") == TEST_WRITER[1][0].traceId + server.lastRequest.headers.get("x-datadog-parent-id") == TEST_WRITER[1][0].spanId where: service | operation | method | url | handlerCount | call | body | client diff --git a/dd-java-agent/instrumentation/elasticsearch-rest-5/elasticsearch-rest-5.gradle b/dd-java-agent/instrumentation/elasticsearch-rest-5/elasticsearch-rest-5.gradle index cc842bc0b5..68117fe433 100644 --- a/dd-java-agent/instrumentation/elasticsearch-rest-5/elasticsearch-rest-5.gradle +++ b/dd-java-agent/instrumentation/elasticsearch-rest-5/elasticsearch-rest-5.gradle @@ -34,7 +34,7 @@ dependencies { // Include httpclient instrumentation for testing because it is a dependency for elasticsearch-rest-client. // It doesn't actually work though. They use HttpAsyncClient, which isn't currently instrumented. // TODO: add HttpAsyncClient instrumentation when that is complete. - testCompile project(':dd-java-agent:instrumentation:apache-httpclient-4.3') + testCompile project(':dd-java-agent:instrumentation:apache-httpclient-4') // TODO: add netty instrumentation when that is complete. testCompile group: 'org.apache.logging.log4j', name: 'log4j-core', version: '2.11.0' diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-5/elasticsearch-transport-5.gradle b/dd-java-agent/instrumentation/elasticsearch-transport-5/elasticsearch-transport-5.gradle index 18d74a5587..cd64732832 100644 --- a/dd-java-agent/instrumentation/elasticsearch-transport-5/elasticsearch-transport-5.gradle +++ b/dd-java-agent/instrumentation/elasticsearch-transport-5/elasticsearch-transport-5.gradle @@ -41,7 +41,7 @@ dependencies { // Include httpclient instrumentation for testing because it is a dependency for elasticsearch-rest-client. // It doesn't actually work though. They use HttpAsyncClient, which isn't currently instrumented. // TODO: add HttpAsyncClient instrumentation when that is complete. - testCompile project(':dd-java-agent:instrumentation:apache-httpclient-4.3') + testCompile project(':dd-java-agent:instrumentation:apache-httpclient-4') // TODO: add netty instrumentation when that is complete. testCompile group: 'org.apache.logging.log4j', name: 'log4j-core', version: '2.11.0' diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-6/elasticsearch-transport-6.gradle b/dd-java-agent/instrumentation/elasticsearch-transport-6/elasticsearch-transport-6.gradle index f2a334f327..77843ad81e 100644 --- a/dd-java-agent/instrumentation/elasticsearch-transport-6/elasticsearch-transport-6.gradle +++ b/dd-java-agent/instrumentation/elasticsearch-transport-6/elasticsearch-transport-6.gradle @@ -41,7 +41,7 @@ dependencies { // Include httpclient instrumentation for testing because it is a dependency for elasticsearch-rest-client. // It doesn't actually work though. They use HttpAsyncClient, which isn't currently instrumented. // TODO: add HttpAsyncClient instrumentation when that is complete. - testCompile project(':dd-java-agent:instrumentation:apache-httpclient-4.3') + testCompile project(':dd-java-agent:instrumentation:apache-httpclient-4') // TODO: add netty instrumentation when that is complete. testCompile group: 'org.elasticsearch.plugin', name: 'transport-netty4-client', version: '6.0.0' diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DDDecoratorsFactory.java b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DDDecoratorsFactory.java index 61bd648e1b..9578b5e27d 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DDDecoratorsFactory.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DDDecoratorsFactory.java @@ -6,15 +6,11 @@ import java.util.List; /** Create DDSpanDecorators */ public class DDDecoratorsFactory { public static List createBuiltinDecorators() { - final HTTPComponent httpDecorator = new HTTPComponent(); - httpDecorator.setMatchingTag("component"); - httpDecorator.setMatchingValue("java-aws-sdk"); return Arrays.asList( new DBStatementAsResourceName(), new DBTypeDecorator(), new ErrorFlag(), - httpDecorator, new OperationDecorator(), new PeerServiceDecorator(), new ResourceNameDecorator(), diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/HTTPComponent.java b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/HTTPComponent.java deleted file mode 100644 index 612e9cb29b..0000000000 --- a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/HTTPComponent.java +++ /dev/null @@ -1,29 +0,0 @@ -package datadog.opentracing.decorators; - -import datadog.opentracing.DDSpanContext; -import datadog.trace.api.DDTags; -import io.opentracing.tag.Tags; - -/** - * This span decorator leverages HTTP tags. It allows the dev to define a custom service name and - * retrieves some HTTP meta such as the request path - */ -public class HTTPComponent extends AbstractDecorator { - - public HTTPComponent() { - super(); - this.setMatchingTag(Tags.COMPONENT.getKey()); - this.setReplacementTag(DDTags.SERVICE_NAME); - } - - @Override - public boolean shouldSetTag(final DDSpanContext context, final String tag, final Object value) { - if (getMatchingValue().equals(value)) { - // Assign service name - super.shouldSetTag(context, tag, value); - // Assign span type to WEB - context.setSpanType("web"); - } - return true; - } -} diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/OperationDecorator.java b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/OperationDecorator.java index 439103e499..445f1ea68d 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/OperationDecorator.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/OperationDecorator.java @@ -17,7 +17,6 @@ public class OperationDecorator extends AbstractDecorator { static { final Map mappings = new HashMap<>(); // Component name <> Operation name - mappings.put("apache-httpclient", "apache.http"); mappings.put("java-aws-sdk", "aws.http"); // FIXME: JMS ops card is low (jms-send or jms-receive), may be this mapping is useless mappings.put("java-jms", "jms"); @@ -28,7 +27,7 @@ public class OperationDecorator extends AbstractDecorator { public OperationDecorator() { super(); - this.setMatchingTag(Tags.COMPONENT.getKey()); + setMatchingTag(Tags.COMPONENT.getKey()); } @Override diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/URLAsResourceName.java b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/URLAsResourceName.java index 06f25a1e96..5852bb1137 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/URLAsResourceName.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/URLAsResourceName.java @@ -17,8 +17,8 @@ public class URLAsResourceName extends AbstractDecorator { public URLAsResourceName() { super(); - this.setMatchingTag(Tags.HTTP_URL.getKey()); - this.setReplacementTag(DDTags.RESOURCE_NAME); + setMatchingTag(Tags.HTTP_URL.getKey()); + setReplacementTag(DDTags.RESOURCE_NAME); } @Override @@ -59,6 +59,10 @@ public class URLAsResourceName extends AbstractDecorator { norm = QUERYSTRING.matcher(norm).replaceAll(""); norm = PATH_MIXED_ALPHANUMERICS.matcher(norm).replaceAll("?"); + if (norm.trim().isEmpty()) { + norm = "/"; + } + return norm; } } diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/URLAsResourceNameTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/URLAsResourceNameTest.groovy index 1aa4402133..6b777056e2 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/URLAsResourceNameTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/URLAsResourceNameTest.groovy @@ -25,6 +25,9 @@ class URLAsResourceNameTest extends Specification { where: input | output + "" | "/" + " " | "/" + "\t" | "/" "/" | "/" "/?asdf" | "/" "/search" | "/search" diff --git a/settings.gradle b/settings.gradle index fb91efd0e6..1370f11b61 100644 --- a/settings.gradle +++ b/settings.gradle @@ -10,7 +10,7 @@ include ':dd-trace-api' // instrumentation: include ':dd-java-agent:instrumentation:akka-http-10.0' -include ':dd-java-agent:instrumentation:apache-httpclient-4.3' +include ':dd-java-agent:instrumentation:apache-httpclient-4' include ':dd-java-agent:instrumentation:aws-java-sdk-1.11.0' include ':dd-java-agent:instrumentation:aws-java-sdk-1.11.106' include ':dd-java-agent:instrumentation:couchbase-2.0'