diff --git a/dd-java-agent-ittests/src/test/java/datadog/trace/agent/integration/MongoAsyncClientInstrumentationTest.java b/dd-java-agent-ittests/src/test/java/datadog/trace/agent/integration/MongoAsyncClientInstrumentationTest.java index 857f738638..b86aa7d4c7 100644 --- a/dd-java-agent-ittests/src/test/java/datadog/trace/agent/integration/MongoAsyncClientInstrumentationTest.java +++ b/dd-java-agent-ittests/src/test/java/datadog/trace/agent/integration/MongoAsyncClientInstrumentationTest.java @@ -29,6 +29,10 @@ public class MongoAsyncClientInstrumentationTest { public static void setup() throws Exception { IntegrationTestUtils.registerOrReplaceGlobalTracer(tracer); MongoClientInstrumentationTest.startLocalMongo(); + // Embeded Mongo uses HttpUrlConnection to download things so we have to clear traces before + // going to tests + writer.clear(); + client = MongoClients.create("mongodb://" + MONGO_HOST + ":" + MONGO_PORT); } @@ -50,7 +54,7 @@ public class MongoAsyncClientInstrumentationTest { } @Test - public void insertOperation() throws InterruptedException, Exception { + public void insertOperation() throws Exception { final MongoDatabase db = client.getDatabase(MONGO_DB_NAME); final String collectionName = "asyncCollection"; final AtomicBoolean done = new AtomicBoolean(false); diff --git a/dd-java-agent-ittests/src/test/java/datadog/trace/agent/integration/MongoClientInstrumentationTest.java b/dd-java-agent-ittests/src/test/java/datadog/trace/agent/integration/MongoClientInstrumentationTest.java index 2ecd189624..32adafa674 100644 --- a/dd-java-agent-ittests/src/test/java/datadog/trace/agent/integration/MongoClientInstrumentationTest.java +++ b/dd-java-agent-ittests/src/test/java/datadog/trace/agent/integration/MongoClientInstrumentationTest.java @@ -16,7 +16,7 @@ import de.flapdoodle.embed.mongo.config.Net; import de.flapdoodle.embed.mongo.distribution.Version; import de.flapdoodle.embed.process.runtime.Network; import io.opentracing.tag.Tags; -import java.net.UnknownHostException; +import java.util.concurrent.TimeoutException; import org.bson.Document; import org.junit.AfterClass; import org.junit.Assert; @@ -36,7 +36,6 @@ public class MongoClientInstrumentationTest { public static void startLocalMongo() throws Exception { final MongodStarter starter = MongodStarter.getDefaultInstance(); - final IMongodConfig mongodConfig = new MongodConfigBuilder() .version(Version.Main.PRODUCTION) @@ -62,6 +61,9 @@ public class MongoClientInstrumentationTest { public static void setup() throws Exception { IntegrationTestUtils.registerOrReplaceGlobalTracer(tracer); startLocalMongo(); + // Embeded Mongo uses HttpUrlConnection to download things so we have to clear traces before + // going to tests + writer.clear(); client = new MongoClient(MONGO_HOST, MONGO_PORT); } @@ -84,7 +86,7 @@ public class MongoClientInstrumentationTest { } @Test - public void insertOperation() throws UnknownHostException { + public void insertOperation() throws TimeoutException, InterruptedException { final MongoDatabase db = client.getDatabase(MONGO_DB_NAME); final String collectionName = "testCollection"; db.createCollection(collectionName); diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-2/src/latestDepTest/groovy/springdata/Elasticsearch2SpringTemplateTest.groovy b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/latestDepTest/groovy/springdata/Elasticsearch2SpringTemplateTest.groovy index 6a53d40c14..4de2b62dba 100644 --- a/dd-java-agent/instrumentation/elasticsearch-transport-2/src/latestDepTest/groovy/springdata/Elasticsearch2SpringTemplateTest.groovy +++ b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/latestDepTest/groovy/springdata/Elasticsearch2SpringTemplateTest.groovy @@ -105,7 +105,9 @@ class Elasticsearch2SpringTemplateTest extends AgentTestRunner { def "test elasticsearch get"() { expect: template.createIndex(indexName) + TEST_WRITER.waitForTraces(1) template.getClient().admin().cluster().prepareHealth().setWaitForYellowStatus().execute().actionGet(TIMEOUT) + TEST_WRITER.waitForTraces(2) when: NativeSearchQuery query = new NativeSearchQueryBuilder() @@ -277,7 +279,10 @@ class Elasticsearch2SpringTemplateTest extends AgentTestRunner { def "test results extractor"() { setup: template.createIndex(indexName) + TEST_WRITER.waitForTraces(1) testNode.client().admin().cluster().prepareHealth().setWaitForYellowStatus().execute().actionGet(TIMEOUT) + TEST_WRITER.waitForTraces(2) + template.index(IndexQueryBuilder.newInstance() .withObject(new Doc(id: 1, data: "doc a")) .withIndexName(indexName) diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/springdata/Elasticsearch2SpringTemplateTest.groovy b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/springdata/Elasticsearch2SpringTemplateTest.groovy index 690dbfcb51..d476c79d3c 100644 --- a/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/springdata/Elasticsearch2SpringTemplateTest.groovy +++ b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/springdata/Elasticsearch2SpringTemplateTest.groovy @@ -105,7 +105,9 @@ class Elasticsearch2SpringTemplateTest extends AgentTestRunner { def "test elasticsearch get"() { expect: template.createIndex(indexName) + TEST_WRITER.waitForTraces(1) template.getClient().admin().cluster().prepareHealth().setWaitForYellowStatus().execute().actionGet(TIMEOUT) + TEST_WRITER.waitForTraces(2) when: NativeSearchQuery query = new NativeSearchQueryBuilder() diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-5.3/src/test/groovy/springdata/Elasticsearch53SpringTemplateTest.groovy b/dd-java-agent/instrumentation/elasticsearch-transport-5.3/src/test/groovy/springdata/Elasticsearch53SpringTemplateTest.groovy index 5d9871f79d..f368c75a02 100644 --- a/dd-java-agent/instrumentation/elasticsearch-transport-5.3/src/test/groovy/springdata/Elasticsearch53SpringTemplateTest.groovy +++ b/dd-java-agent/instrumentation/elasticsearch-transport-5.3/src/test/groovy/springdata/Elasticsearch53SpringTemplateTest.groovy @@ -125,7 +125,9 @@ class Elasticsearch53SpringTemplateTest extends AgentTestRunner { def "test elasticsearch get"() { expect: template.createIndex(indexName) + TEST_WRITER.waitForTraces(1) template.getClient().admin().cluster().prepareHealth().setWaitForYellowStatus().execute().actionGet(TIMEOUT) + TEST_WRITER.waitForTraces(2) when: NativeSearchQuery query = new NativeSearchQueryBuilder() @@ -305,7 +307,10 @@ class Elasticsearch53SpringTemplateTest extends AgentTestRunner { def "test results extractor"() { setup: template.createIndex(indexName) + TEST_WRITER.waitForTraces(1) testNode.client().admin().cluster().prepareHealth().setWaitForYellowStatus().execute().actionGet(TIMEOUT) + TEST_WRITER.waitForTraces(2) + template.index(IndexQueryBuilder.newInstance() .withObject(new Doc(id: 1, data: "doc a")) .withIndexName(indexName) diff --git a/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/HttpUrlConnectionInstrumentation.java b/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/HttpUrlConnectionInstrumentation.java index 6a22d6507d..f8a2d65be7 100644 --- a/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/HttpUrlConnectionInstrumentation.java +++ b/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/HttpUrlConnectionInstrumentation.java @@ -17,11 +17,14 @@ import datadog.trace.bootstrap.InstrumentationContext; 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.net.HttpURLConnection; import java.net.URL; import java.util.Collections; +import java.util.Iterator; import java.util.Map; import javax.net.ssl.HttpsURLConnection; import net.bytebuddy.asm.Advice; @@ -35,11 +38,6 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default { super("httpurlconnection"); } - @Override - protected boolean defaultEnabled() { - return false; - } - @Override public ElementMatcher typeMatcher() { return safeHasSuperType(named("java.net.HttpURLConnection")) @@ -50,15 +48,16 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default { @Override public String[] helperClassNames() { return new String[] { - HttpUrlConnectionInstrumentation.class.getName() + "$HttpURLState", - HttpUrlConnectionInstrumentation.class.getName() + "$HttpURLState$1" + HttpUrlConnectionInstrumentation.class.getName() + "$HeadersInjectAdapter", + HttpUrlConnectionInstrumentation.class.getName() + "$HttpUrlState", + HttpUrlConnectionInstrumentation.class.getName() + "$HttpUrlState$1" }; } @Override public Map contextStore() { return Collections.singletonMap( - "java.net.HttpURLConnection", getClass().getName() + "$HttpURLState"); + "java.net.HttpURLConnection", getClass().getName() + "$HttpUrlState"); } @Override @@ -73,131 +72,170 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default { public static class HttpUrlConnectionAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) - public static Scope startSpan( + public static HttpUrlState methodEnter( @Advice.This final HttpURLConnection thiz, - @Advice.FieldValue("connected") final boolean connected, - @Advice.Origin("#m") final String methodName) { - final ContextStore contextStore = - InstrumentationContext.get(HttpURLConnection.class, HttpURLState.class); - final HttpURLState state = contextStore.putIfAbsent(thiz, HttpURLState.FACTORY); + @Advice.FieldValue("connected") final boolean connected) { - String operationName = "http.request"; + final ContextStore contextStore = + InstrumentationContext.get(HttpURLConnection.class, HttpUrlState.class); + final HttpUrlState state = contextStore.putIfAbsent(thiz, HttpUrlState.FACTORY); - switch (methodName) { - case "connect": - if (connected) { - return null; + synchronized (state) { + /* + * AgentWriter uses HttpURLConnection to report to the trace-agent. We don't want to trace + * those requests. Check after the connected test above because getRequestProperty will + * throw an exception if already connected. + */ + final boolean isTraceRequest = + Thread.currentThread().getName().equals("dd-agent-writer") + || (!connected && thiz.getRequestProperty("Datadog-Meta-Lang") != null); + if (isTraceRequest) { + state.finish(); + return null; + } + + final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(HttpURLConnection.class); + if (callDepth > 0) { + return null; + } + + if (!state.hasSpan() && !state.isFinished()) { + final Span span = state.startSpan(thiz); + if (!connected) { + GlobalTracer.get() + .inject( + span.context(), Format.Builtin.HTTP_HEADERS, new HeadersInjectAdapter(thiz)); } - /* - * Ideally, we would like to only have a single span for each of the output and input streams, - * but since headers are also sent on connect(), there wouldn't be a span to mark as parent if - * we don't create a span here. - */ - operationName += ".connect"; - break; - - case "getOutputStream": - if (state.calledOutputStream) { - return null; - } - state.calledOutputStream = true; - operationName += ".output-stream"; - break; - - case "getInputStream": - if (state.calledInputStream) { - return null; - } - state.calledInputStream = true; - operationName += ".input-stream"; - break; + } + return state; } - - /* - * AgentWriter uses HttpURLConnection to report to the trace-agent. We don't want to trace - * those requests. Check after the connected test above because getRequestProperty will - * throw an exception if already connected. - */ - final boolean isTraceRequest = - Thread.currentThread().getName().equals("dd-agent-writer") - || (!connected && thiz.getRequestProperty("Datadog-Meta-Lang") != null); - if (isTraceRequest) { - return null; - } - - final Tracer tracer = GlobalTracer.get(); - if (tracer.activeSpan() == null) { - // We don't want this as a top level span. - return null; - } - - final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(HttpURLConnection.class); - if (callDepth > 0) { - return null; - } - - final Scope scope = - tracer - .buildSpan(operationName) - .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT) - .withTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_CLIENT) - .startActive(true); - - final URL url = thiz.getURL(); - final Span span = scope.span(); - Tags.COMPONENT.set(scope.span(), thiz.getClass().getSimpleName()); - Tags.HTTP_URL.set(span, url.toString()); - Tags.PEER_HOSTNAME.set(span, url.getHost()); - if (url.getPort() > 0) { - Tags.PEER_PORT.set(span, url.getPort()); - } else if (thiz instanceof HttpsURLConnection) { - Tags.PEER_PORT.set(span, 443); - } else { - Tags.PEER_PORT.set(span, 80); - } - Tags.HTTP_METHOD.set(span, thiz.getRequestMethod()); - - return scope; } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) - public static void stopSpan( + public static void methodExit( + @Advice.Enter final HttpUrlState state, @Advice.FieldValue("responseCode") final int responseCode, - @Advice.Enter final Scope scope, - @Advice.Thrown final Throwable throwable) { + @Advice.Thrown final Throwable throwable, + @Advice.Origin("#m") final String methodName) { - if (scope == null) { + if (state == null) { return; } - final Span span = scope.span(); - if (throwable != null) { - Tags.ERROR.set(span, true); - span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); - } else if (responseCode > 0) { - /* - * responseCode field cache is sometimes not populated. - * We can't call getResponseCode() due to some unwanted side-effects - * (e.g. breaks getOutputStream). - */ - Tags.HTTP_STATUS.set(span, responseCode); + synchronized (state) { + if (state.hasSpan() && !state.isFinished()) { + if (throwable != null) { + state.finishSpan(throwable); + } else if ("getInputStream".equals(methodName)) { + state.finishSpan(responseCode); + } + } } - scope.close(); + CallDepthThreadLocalMap.reset(HttpURLConnection.class); } } - public static class HttpURLState { + public static class HeadersInjectAdapter implements TextMap { - public static final ContextStore.Factory FACTORY = - new ContextStore.Factory() { + private final HttpURLConnection connection; + + public HeadersInjectAdapter(final HttpURLConnection connection) { + this.connection = connection; + } + + @Override + public void put(final String key, final String value) { + try { + connection.setRequestProperty(key, value); + } catch (final IllegalStateException e) { + // There are cases when this can through an exception. E.g. some implementations have + // 'connecting' state. Just guard against that here, there is not much we can do at this + // point. + } + } + + @Override + public Iterator> iterator() { + throw new UnsupportedOperationException( + "This class should be used only with tracer#inject()"); + } + } + + public static class HttpUrlState { + + public static final String OPERATION_NAME = "http.request"; + public static final String COMPONENT_NAME = "http-url-connection"; + + public static final ContextStore.Factory FACTORY = + new ContextStore.Factory() { @Override - public HttpURLState create() { - return new HttpURLState(); + public HttpUrlState create() { + return new HttpUrlState(); } }; - public boolean calledOutputStream = false; - public boolean calledInputStream = false; + private volatile Span span = null; + private volatile boolean finished = false; + + public Span startSpan(final HttpURLConnection connection) { + final Tracer.SpanBuilder builder = + GlobalTracer.get() + .buildSpan(OPERATION_NAME) + .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT) + .withTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_CLIENT); + try (final Scope scope = builder.startActive(false)) { + span = scope.span(); + final URL url = connection.getURL(); + Tags.COMPONENT.set(span, COMPONENT_NAME); + Tags.HTTP_URL.set(span, url.toString()); + Tags.PEER_HOSTNAME.set(span, url.getHost()); + if (url.getPort() > 0) { + Tags.PEER_PORT.set(span, url.getPort()); + } else if (connection instanceof HttpsURLConnection) { + Tags.PEER_PORT.set(span, 443); + } else { + Tags.PEER_PORT.set(span, 80); + } + Tags.HTTP_METHOD.set(span, connection.getRequestMethod()); + return span; + } + } + + public boolean hasSpan() { + return span != null; + } + + public boolean isFinished() { + return finished; + } + + public void finish() { + finished = true; + } + + public void finishSpan(final Throwable throwable) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + Tags.ERROR.set(span, true); + span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); + } + span = null; + finished = true; + } + + public void finishSpan(final int responseCode) { + /* + * responseCode field is sometimes not populated. + * We can't call getResponseCode() due to some unwanted side-effects + * (e.g. breaks getOutputStream). + */ + if (responseCode > 0) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + Tags.HTTP_STATUS.set(span, responseCode); + span = null; + finished = true; + } + } + } } } diff --git a/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/SunHttpClientInstrumentation.java b/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/SunHttpClientInstrumentation.java deleted file mode 100644 index 71f83c2a09..0000000000 --- a/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/SunHttpClientInstrumentation.java +++ /dev/null @@ -1,95 +0,0 @@ -package datadog.trace.instrumentation.http_url_connection; - -import static net.bytebuddy.matcher.ElementMatchers.isMethod; -import static net.bytebuddy.matcher.ElementMatchers.isPublic; -import static net.bytebuddy.matcher.ElementMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.takesArgument; -import static net.bytebuddy.matcher.ElementMatchers.takesArguments; - -import com.google.auto.service.AutoService; -import datadog.trace.agent.tooling.Instrumenter; -import io.opentracing.Span; -import io.opentracing.Tracer; -import io.opentracing.propagation.Format; -import io.opentracing.propagation.TextMap; -import io.opentracing.util.GlobalTracer; -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.matcher.ElementMatcher; -import sun.net.www.MessageHeader; -import sun.net.www.http.HttpClient; - -@AutoService(Instrumenter.class) -public class SunHttpClientInstrumentation extends Instrumenter.Default { - - public SunHttpClientInstrumentation() { - super("httpurlconnection"); - } - - @Override - protected boolean defaultEnabled() { - return false; - } - - @Override - public ElementMatcher typeMatcher() { - return named("sun.net.www.http.HttpClient"); - } - - @Override - public String[] helperClassNames() { - return new String[] { - SunHttpClientInstrumentation.class.getName() + "$MessageHeadersInjectAdapter" - }; - } - - @Override - public Map transformers() { - return Collections.singletonMap( - isMethod() - .and(isPublic()) - .and(named("writeRequests")) - .and(takesArgument(0, named("sun.net.www.MessageHeader"))) - // exclude the delegating method: - .and(takesArguments(1).or(takesArguments(2))), - InjectAdvice.class.getName()); - } - - public static class InjectAdvice { - - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void inject( - @Advice.Argument(0) final MessageHeader header, @Advice.This final HttpClient client) { - final Tracer tracer = GlobalTracer.get(); - final Span span = tracer.activeSpan(); - - if (span != null) { - tracer.inject( - span.context(), Format.Builtin.HTTP_HEADERS, new MessageHeadersInjectAdapter(header)); - } - } - } - - public static class MessageHeadersInjectAdapter implements TextMap { - - private final MessageHeader header; - - public MessageHeadersInjectAdapter(final MessageHeader header) { - this.header = header; - } - - @Override - public void put(final String key, final String value) { - header.setIfNotSet(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/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/UrlInstrumentation.java b/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/UrlInstrumentation.java index 397a49e94b..23c782fb48 100644 --- a/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/UrlInstrumentation.java +++ b/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/UrlInstrumentation.java @@ -31,11 +31,6 @@ public class UrlInstrumentation extends Instrumenter.Default { super("urlconnection", "httpurlconnection"); } - @Override - protected boolean defaultEnabled() { - return false; - } - @Override public ElementMatcher typeMatcher() { return is(URL.class); diff --git a/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/HttpUrlConnectionTest.groovy b/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/HttpUrlConnectionTest.groovy index cce7b4c6dd..ed38f23a24 100644 --- a/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/HttpUrlConnectionTest.groovy +++ b/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/HttpUrlConnectionTest.groovy @@ -9,11 +9,10 @@ import spock.lang.Shared import static datadog.trace.agent.test.TestUtils.runUnderTrace import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer +import static datadog.trace.instrumentation.http_url_connection.HttpUrlConnectionInstrumentation.HttpUrlState.COMPONENT_NAME +import static datadog.trace.instrumentation.http_url_connection.HttpUrlConnectionInstrumentation.HttpUrlState.OPERATION_NAME class HttpUrlConnectionTest extends AgentTestRunner { - static { - System.setProperty("dd.integration.httpurlconnection.enabled", "true") - } static final RESPONSE = "

Hello test.

" static final STATUS = 202 @@ -68,11 +67,11 @@ class HttpUrlConnectionTest extends AgentTestRunner { } } span(1) { - operationName "http.request.input-stream" + operationName OPERATION_NAME childOf span(0) errored false tags { - "$Tags.COMPONENT.key" "HttpURLConnection" + "$Tags.COMPONENT.key" COMPONENT_NAME "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT "$Tags.HTTP_URL.key" "$server.address" @@ -84,11 +83,11 @@ class HttpUrlConnectionTest extends AgentTestRunner { } } span(2) { - operationName "http.request.input-stream" + operationName OPERATION_NAME childOf span(0) errored false tags { - "$Tags.COMPONENT.key" "HttpURLConnection" + "$Tags.COMPONENT.key" COMPONENT_NAME "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT "$Tags.HTTP_URL.key" "$server.address" @@ -144,11 +143,11 @@ class HttpUrlConnectionTest extends AgentTestRunner { } } span(1) { - operationName "http.request.input-stream" + operationName OPERATION_NAME childOf span(0) errored false tags { - "$Tags.COMPONENT.key" "HttpURLConnection" + "$Tags.COMPONENT.key" COMPONENT_NAME "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT "$Tags.HTTP_URL.key" "$server.address" @@ -160,11 +159,11 @@ class HttpUrlConnectionTest extends AgentTestRunner { } } span(2) { - operationName "http.request.input-stream" + operationName OPERATION_NAME childOf span(0) errored false tags { - "$Tags.COMPONENT.key" "HttpURLConnection" + "$Tags.COMPONENT.key" COMPONENT_NAME "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT "$Tags.HTTP_URL.key" "$server.address" @@ -204,11 +203,11 @@ class HttpUrlConnectionTest extends AgentTestRunner { } } span(1) { - operationName "http.request.input-stream" + operationName OPERATION_NAME childOf span(0) errored false tags { - "$Tags.COMPONENT.key" "HttpURLConnection" + "$Tags.COMPONENT.key" COMPONENT_NAME "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT "$Tags.HTTP_URL.key" "$server.address" @@ -246,11 +245,11 @@ class HttpUrlConnectionTest extends AgentTestRunner { } } span(1) { - operationName "http.request.input-stream" + operationName OPERATION_NAME childOf span(0) errored false tags { - "$Tags.COMPONENT.key" "HttpURLConnection" + "$Tags.COMPONENT.key" COMPONENT_NAME "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT "$Tags.HTTP_URL.key" "$server.address" @@ -297,7 +296,7 @@ class HttpUrlConnectionTest extends AgentTestRunner { expect: assertTraces(2) { server.distributedRequestTrace(it, 0, TEST_WRITER[1][1]) - trace(1, 3) { + trace(1, 2) { span(0) { operationName "someTrace" parent() @@ -307,11 +306,11 @@ class HttpUrlConnectionTest extends AgentTestRunner { } } span(1) { - operationName "http.request.input-stream" + operationName OPERATION_NAME childOf span(0) errored false tags { - "$Tags.COMPONENT.key" "HttpURLConnection" + "$Tags.COMPONENT.key" COMPONENT_NAME "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT "$Tags.HTTP_URL.key" "$server.address" @@ -322,21 +321,6 @@ class HttpUrlConnectionTest extends AgentTestRunner { defaultTags() } } - span(2) { - operationName "http.request.output-stream" - childOf span(0) - errored false - tags { - "$Tags.COMPONENT.key" "HttpURLConnection" - "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT - "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT - "$Tags.HTTP_URL.key" "$server.address" - "$Tags.HTTP_METHOD.key" "POST" - "$Tags.PEER_HOSTNAME.key" "localhost" - "$Tags.PEER_PORT.key" server.address.port - defaultTags() - } - } } } } @@ -380,7 +364,26 @@ class HttpUrlConnectionTest extends AgentTestRunner { assert lines == [RESPONSE] expect: - assertTraces(0) {} + assertTraces(1) { + trace(0, 1) { + span(0) { + operationName OPERATION_NAME + parent() + errored false + tags { + "$Tags.COMPONENT.key" COMPONENT_NAME + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT + "$Tags.HTTP_URL.key" "$server.address" + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" STATUS + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" server.address.port + defaultTags() + } + } + } + } } def "rest template"() { @@ -393,8 +396,8 @@ class HttpUrlConnectionTest extends AgentTestRunner { expect: assertTraces(2) { - server.distributedRequestTrace(it, 0, TEST_WRITER[1][2]) - trace(1, 4) { + server.distributedRequestTrace(it, 0, TEST_WRITER[1][1]) + trace(1, 2) { span(0) { operationName "someTrace" parent() @@ -404,11 +407,11 @@ class HttpUrlConnectionTest extends AgentTestRunner { } } span(1) { - operationName "http.request.input-stream" + operationName OPERATION_NAME childOf span(0) errored false tags { - "$Tags.COMPONENT.key" "HttpURLConnection" + "$Tags.COMPONENT.key" COMPONENT_NAME "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT "$Tags.HTTP_URL.key" "$server.address" @@ -419,36 +422,6 @@ class HttpUrlConnectionTest extends AgentTestRunner { defaultTags() } } - span(2) { - operationName "http.request.output-stream" - childOf span(0) - errored false - tags { - "$Tags.COMPONENT.key" "HttpURLConnection" - "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT - "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT - "$Tags.HTTP_URL.key" "$server.address" - "$Tags.HTTP_METHOD.key" "POST" - "$Tags.PEER_HOSTNAME.key" "localhost" - "$Tags.PEER_PORT.key" server.address.port - defaultTags() - } - } - span(3) { - operationName "http.request.connect" - childOf span(0) - errored false - tags { - "$Tags.COMPONENT.key" "HttpURLConnection" - "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT - "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT - "$Tags.HTTP_URL.key" "$server.address" - "$Tags.HTTP_METHOD.key" "POST" - "$Tags.PEER_HOSTNAME.key" "localhost" - "$Tags.PEER_PORT.key" server.address.port - defaultTags() - } - } } } } diff --git a/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy b/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy index 83f3a410e7..24919644fe 100644 --- a/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy +++ b/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy @@ -7,15 +7,14 @@ import io.opentracing.tag.Tags import io.opentracing.util.GlobalTracer import static datadog.trace.agent.test.TestUtils.runUnderTrace +import static datadog.trace.instrumentation.http_url_connection.HttpUrlConnectionInstrumentation.HttpUrlState.COMPONENT_NAME +import static datadog.trace.instrumentation.http_url_connection.HttpUrlConnectionInstrumentation.HttpUrlState.OPERATION_NAME class UrlConnectionTest extends AgentTestRunner { - static { - System.setProperty("dd.integration.httpurlconnection.enabled", "true") - } private static final int INVALID_PORT = TestUtils.randomOpenPort() - def "trace request with connection failure"() { + def "trace request with connection failure #scheme"() { when: runUnderTrace("someTrace") { URLConnection connection = url.openConnection() @@ -41,11 +40,11 @@ class UrlConnectionTest extends AgentTestRunner { } } span(1) { - operationName "http.request.input-stream" + operationName OPERATION_NAME childOf span(0) errored true tags { - "$Tags.COMPONENT.key" component + "$Tags.COMPONENT.key" COMPONENT_NAME "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT "$Tags.HTTP_URL.key" "$url" @@ -60,9 +59,9 @@ class UrlConnectionTest extends AgentTestRunner { } where: - scheme | component - "http" | "HttpURLConnection" - "https" | "DelegateHttpsURLConnection" + scheme | _ + "http" | _ + "https" | _ url = new URI("$scheme://localhost:$INVALID_PORT").toURL() }