diff --git a/dd-java-agent-ittests/src/test/java/datadog/trace/agent/test/IntegrationTestUtils.java b/dd-java-agent-ittests/src/test/java/datadog/trace/agent/test/IntegrationTestUtils.java index 2456609a02..c953a1c801 100644 --- a/dd-java-agent-ittests/src/test/java/datadog/trace/agent/test/IntegrationTestUtils.java +++ b/dd-java-agent-ittests/src/test/java/datadog/trace/agent/test/IntegrationTestUtils.java @@ -70,7 +70,7 @@ public class IntegrationTestUtils { * @throws IOException */ public static URL createJarWithClasses(final Class... classes) throws IOException { - final File tmpJar = File.createTempFile(UUID.randomUUID().toString() + "", ".jar"); + final File tmpJar = File.createTempFile(UUID.randomUUID().toString() + "-", ".jar"); tmpJar.deleteOnExit(); final Manifest manifest = new Manifest(); diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/HelperInjector.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/HelperInjector.java index 9b91fce16c..9df00ceadf 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/HelperInjector.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/HelperInjector.java @@ -76,14 +76,15 @@ public class HelperInjector implements Transformer { } } } + log.debug("Injecting classes onto classloader {} -> {}", classLoader, helperClassNames); if (classLoader == BOOTSTRAP_CLASSLOADER) { - Map> injected = + final Map> injected = ClassInjector.UsingInstrumentation.of( new File(System.getProperty("java.io.tmpdir")), ClassInjector.UsingInstrumentation.Target.BOOTSTRAP, AgentInstaller.getInstrumentation()) .inject(helperMap); - for (TypeDescription desc : injected.keySet()) { + for (final TypeDescription desc : injected.keySet()) { Class.forName(desc.getName(), false, Utils.getBootstrapProxy()); } } else { diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-2/elasticsearch-transport-2.gradle b/dd-java-agent/instrumentation/elasticsearch-transport-2/elasticsearch-transport-2.gradle index 8371b6d5bb..bad09746b7 100644 --- a/dd-java-agent/instrumentation/elasticsearch-transport-2/elasticsearch-transport-2.gradle +++ b/dd-java-agent/instrumentation/elasticsearch-transport-2/elasticsearch-transport-2.gradle @@ -38,10 +38,13 @@ dependencies { // TODO: add netty instrumentation when that is complete. testCompile group: 'org.elasticsearch', name: 'elasticsearch', version: '2.0.0' - testCompile group: 'net.java.dev.jna', name: 'jna', version: '4.5.1' + testCompile group: 'org.springframework.data', name: 'spring-data-elasticsearch', version: '2.0.0.RELEASE' + + testCompile group: 'net.java.dev.jna', name: 'jna', version: '4.5.1' testCompile group: 'org.apache.logging.log4j', name: 'log4j-core', version: '2.11.0' testCompile group: 'org.apache.logging.log4j', name: 'log4j-api', version: '2.11.0' latestDepTestCompile group: 'org.elasticsearch', name: 'elasticsearch', version: '2.+' + latestDepTestCompile group: 'org.springframework.data', name: 'spring-data-elasticsearch', version: '2.+' } diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-2/src/main/java/datadog/trace/instrumentation/elasticsearch2/Elasticsearch2TransportClientInstrumentation.java b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/main/java/datadog/trace/instrumentation/elasticsearch2/Elasticsearch2TransportClientInstrumentation.java index c9e8ffe5f9..3b8eb12605 100644 --- a/dd-java-agent/instrumentation/elasticsearch-transport-2/src/main/java/datadog/trace/instrumentation/elasticsearch2/Elasticsearch2TransportClientInstrumentation.java +++ b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/main/java/datadog/trace/instrumentation/elasticsearch2/Elasticsearch2TransportClientInstrumentation.java @@ -50,6 +50,9 @@ public class Elasticsearch2TransportClientInstrumentation extends Instrumenter.C new HelperInjector( "com.google.common.base.Preconditions", "com.google.common.base.Joiner", + "com.google.common.base.Joiner$1", + "com.google.common.base.Joiner$2", + "com.google.common.base.Joiner$MapJoiner", "datadog.trace.instrumentation.elasticsearch2.TransportActionListener")) .transform(DDTransformers.defaultTransformers()) .transform( diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-2/src/main/java/datadog/trace/instrumentation/elasticsearch2/TransportActionListener.java b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/main/java/datadog/trace/instrumentation/elasticsearch2/TransportActionListener.java index 4c59638a2d..a9dbe072d8 100644 --- a/dd-java-agent/instrumentation/elasticsearch-transport-2/src/main/java/datadog/trace/instrumentation/elasticsearch2/TransportActionListener.java +++ b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/main/java/datadog/trace/instrumentation/elasticsearch2/TransportActionListener.java @@ -13,6 +13,7 @@ import org.elasticsearch.action.DocumentRequest; import org.elasticsearch.action.IndicesRequest; import org.elasticsearch.action.bulk.BulkShardResponse; import org.elasticsearch.action.get.GetResponse; +import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.support.broadcast.BroadcastResponse; import org.elasticsearch.action.support.nodes.BaseNodesResponse; @@ -35,6 +36,10 @@ public class TransportActionListener implements Action span.setTag("elasticsearch.request.indices", Joiner.on(",").join(req.indices())); } } + if (request instanceof SearchRequest) { + final SearchRequest req = (SearchRequest) request; + span.setTag("elasticsearch.request.search.types", Joiner.on(",").join(req.types())); + } if (request instanceof DocumentRequest) { final DocumentRequest req = (DocumentRequest) request; span.setTag("elasticsearch.request.write.type", req.type()); diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/Elasticsearch2NodeClientTest.groovy b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/Elasticsearch2NodeClientTest.groovy index cf96507b5f..2c47ea3cb3 100644 --- a/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/Elasticsearch2NodeClientTest.groovy +++ b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/Elasticsearch2NodeClientTest.groovy @@ -37,7 +37,7 @@ class Elasticsearch2NodeClientTest extends AgentTestRunner { .put("http.port", HTTP_PORT) .put("transport.tcp.port", TCP_PORT) .build() - testNode = NodeBuilder.newInstance().clusterName("test-cluster").settings(settings).build() + testNode = NodeBuilder.newInstance().local(true).clusterName("test-cluster").settings(settings).build() testNode.start() TEST_WRITER.clear() testNode.client().admin().cluster().prepareHealth().setWaitForYellowStatus().execute().actionGet(5000) @@ -118,10 +118,13 @@ class Elasticsearch2NodeClientTest extends AgentTestRunner { def "test elasticsearch get"() { setup: + assert TEST_WRITER == [] def indexResult = client.admin().indices().prepareCreate(indexName).get() + TEST_WRITER.waitForTraces(1) expect: indexResult.acknowledged + TEST_WRITER.size() == 1 when: client.admin().cluster().prepareHealth().setWaitForYellowStatus().execute().actionGet(5000) @@ -192,6 +195,9 @@ class Elasticsearch2NodeClientTest extends AgentTestRunner { tags { "$Tags.COMPONENT.key" "elasticsearch-java" "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$Tags.PEER_HOSTNAME.key" "local" + "$Tags.PEER_HOST_IPV4.key" "0.0.0.0" + "$Tags.PEER_PORT.key" 0 "elasticsearch.action" "GetAction" "elasticsearch.request" "GetRequest" "elasticsearch.request.indices" indexName @@ -227,6 +233,9 @@ class Elasticsearch2NodeClientTest extends AgentTestRunner { tags { "$Tags.COMPONENT.key" "elasticsearch-java" "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$Tags.PEER_HOSTNAME.key" "local" + "$Tags.PEER_HOST_IPV4.key" "0.0.0.0" + "$Tags.PEER_PORT.key" 0 "elasticsearch.action" "IndexAction" "elasticsearch.request" "IndexRequest" "elasticsearch.request.indices" indexName @@ -244,6 +253,9 @@ class Elasticsearch2NodeClientTest extends AgentTestRunner { tags { "$Tags.COMPONENT.key" "elasticsearch-java" "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$Tags.PEER_HOSTNAME.key" "local" + "$Tags.PEER_HOST_IPV4.key" "0.0.0.0" + "$Tags.PEER_PORT.key" 0 "elasticsearch.action" "GetAction" "elasticsearch.request" "GetRequest" "elasticsearch.request.indices" indexName diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/Elasticsearch2SpringTemplateTest.groovy b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/Elasticsearch2SpringTemplateTest.groovy new file mode 100644 index 0000000000..b8326bd0b5 --- /dev/null +++ b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/Elasticsearch2SpringTemplateTest.groovy @@ -0,0 +1,328 @@ +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.TestUtils +import io.opentracing.tag.Tags +import org.elasticsearch.action.search.SearchResponse +import org.elasticsearch.common.io.FileSystemUtils +import org.elasticsearch.common.settings.Settings +import org.elasticsearch.index.IndexNotFoundException +import org.elasticsearch.node.Node +import org.elasticsearch.node.NodeBuilder +import org.elasticsearch.search.aggregations.bucket.nested.InternalNested +import org.elasticsearch.search.aggregations.bucket.terms.Terms +import org.springframework.data.elasticsearch.core.ElasticsearchTemplate +import org.springframework.data.elasticsearch.core.ResultsExtractor +import org.springframework.data.elasticsearch.core.query.IndexQueryBuilder +import org.springframework.data.elasticsearch.core.query.NativeSearchQuery +import org.springframework.data.elasticsearch.core.query.NativeSearchQueryBuilder +import spock.lang.Shared +import springdata.Doc + +import java.util.concurrent.atomic.AtomicLong + +import static datadog.trace.agent.test.ListWriterAssert.assertTraces + +class Elasticsearch2SpringTemplateTest extends AgentTestRunner { + static { + System.setProperty("dd.integration.elasticsearch.enabled", "true") + } + + static final int HTTP_PORT = TestUtils.randomOpenPort() + static final int TCP_PORT = TestUtils.randomOpenPort() + + @Shared + static Node testNode + static File esWorkingDir + + @Shared + static ElasticsearchTemplate template + + def setupSpec() { + esWorkingDir = File.createTempFile("test-es-working-dir-", "") + esWorkingDir.delete() + esWorkingDir.mkdir() + esWorkingDir.deleteOnExit() + println "ES work dir: $esWorkingDir" + + def settings = Settings.builder() + .put("path.home", esWorkingDir.path) + .put("http.port", HTTP_PORT) + .put("transport.tcp.port", TCP_PORT) + .build() + testNode = NodeBuilder.newInstance().local(true).clusterName("test-cluster").settings(settings).build() + testNode.start() + + template = new ElasticsearchTemplate(testNode.client()) + } + + def cleanupSpec() { + testNode?.close() + if (esWorkingDir != null) { + FileSystemUtils.deleteSubDirectories(esWorkingDir.toPath()) + esWorkingDir.delete() + } + } + + def "test elasticsearch error"() { + when: + template.refresh(indexName) + + then: + thrown IndexNotFoundException + + and: + assertTraces(TEST_WRITER, 1) { + trace(0, 1) { + span(0) { + serviceName "elasticsearch" + resourceName "RefreshAction" + operationName "elasticsearch.query" + spanType null + errored true + tags { + "$Tags.COMPONENT.key" "elasticsearch-java" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "elasticsearch.action" "RefreshAction" + "elasticsearch.request" "RefreshRequest" + "elasticsearch.request.indices" indexName + errorTags IndexNotFoundException, "no such index" + defaultTags() + } + } + } + } + + where: + indexName = "invalid-index" + } + + def "test elasticsearch get"() { + expect: + template.createIndex(indexName) + template.getClient().admin().cluster().prepareHealth().setWaitForYellowStatus().execute().actionGet(5000) + + when: + NativeSearchQuery query = new NativeSearchQueryBuilder() + .withIndices(indexName) + .withTypes(indexType) + .withIds([id]) + .build() + + then: + template.queryForIds(query) == [] + + when: + def result = template.index(IndexQueryBuilder.newInstance() + .withObject(new Doc()) + .withIndexName(indexName) + .withType(indexType) + .withId(id) + .build()) + template.refresh(Doc) + + then: + result == id + template.queryForList(query, Doc) == [new Doc()] + + and: + assertTraces(TEST_WRITER, 7) { + trace(0, 1) { + span(0) { + serviceName "elasticsearch" + resourceName "CreateIndexAction" + operationName "elasticsearch.query" + spanType null + tags { + "$Tags.COMPONENT.key" "elasticsearch-java" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "elasticsearch.action" "CreateIndexAction" + "elasticsearch.request" "CreateIndexRequest" + "elasticsearch.request.indices" indexName + defaultTags() + } + } + } + trace(1, 1) { + span(0) { + serviceName "elasticsearch" + resourceName "ClusterHealthAction" + operationName "elasticsearch.query" + spanType null + tags { + "$Tags.COMPONENT.key" "elasticsearch-java" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "elasticsearch.action" "ClusterHealthAction" + "elasticsearch.request" "ClusterHealthRequest" + defaultTags() + } + } + } + trace(2, 1) { + span(0) { + serviceName "elasticsearch" + resourceName "SearchAction" + operationName "elasticsearch.query" + spanType null + tags { + "$Tags.COMPONENT.key" "elasticsearch-java" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "elasticsearch.action" "SearchAction" + "elasticsearch.request" "SearchRequest" + "elasticsearch.request.indices" indexName + "elasticsearch.request.search.types" indexType + defaultTags() + } + } + } + trace(3, 1) { + span(0) { + serviceName "elasticsearch" + resourceName "PutMappingAction" + operationName "elasticsearch.query" + spanType null + tags { + "$Tags.COMPONENT.key" "elasticsearch-java" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "elasticsearch.action" "PutMappingAction" + "elasticsearch.request" "PutMappingRequest" + "elasticsearch.request.indices" indexName + defaultTags() + } + } + } + trace(4, 1) { + span(0) { + serviceName "elasticsearch" + resourceName "IndexAction" + operationName "elasticsearch.query" + spanType null + tags { + "$Tags.COMPONENT.key" "elasticsearch-java" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$Tags.PEER_HOSTNAME.key" "local" + "$Tags.PEER_HOST_IPV4.key" "0.0.0.0" + "$Tags.PEER_PORT.key" 0 + "elasticsearch.action" "IndexAction" + "elasticsearch.request" "IndexRequest" + "elasticsearch.request.indices" indexName + "elasticsearch.request.write.type" indexType + defaultTags() + } + } + } + trace(5, 1) { + span(0) { + serviceName "elasticsearch" + resourceName "RefreshAction" + operationName "elasticsearch.query" + spanType null + tags { + "$Tags.COMPONENT.key" "elasticsearch-java" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "elasticsearch.action" "RefreshAction" + "elasticsearch.request" "RefreshRequest" + "elasticsearch.request.indices" indexName + "elasticsearch.shard.broadcast.failed" 0 + "elasticsearch.shard.broadcast.successful" 5 + "elasticsearch.shard.broadcast.total" 10 + defaultTags() + } + } + } + trace(6, 1) { + span(0) { + serviceName "elasticsearch" + resourceName "SearchAction" + operationName "elasticsearch.query" + spanType null + tags { + "$Tags.COMPONENT.key" "elasticsearch-java" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "elasticsearch.action" "SearchAction" + "elasticsearch.request" "SearchRequest" + "elasticsearch.request.indices" indexName + "elasticsearch.request.search.types" indexType + defaultTags() + } + } + } + } + + where: + indexName = "test-index" + indexType = "test-type" + id = "1" + } + + def "test results extractor"() { + setup: + template.createIndex(indexName) + testNode.client().admin().cluster().prepareHealth().setWaitForYellowStatus().execute().actionGet(5000) + template.index(IndexQueryBuilder.newInstance() + .withObject(new Doc(id: 1, data: "doc a")) + .withIndexName(indexName) + .withId("a") + .build()) + template.index(IndexQueryBuilder.newInstance() + .withObject(new Doc(id: 2, data: "doc b")) + .withIndexName(indexName) + .withId("b") + .build()) + template.refresh(indexName) + TEST_WRITER.waitForTraces(6) + TEST_WRITER.clear() + + and: + def query = new NativeSearchQueryBuilder().withIndices(indexName).build() + def hits = new AtomicLong() + List> results = [] + def bucketTags = [:] + + when: + template.query(query, new ResultsExtractor() { + + @Override + Doc extract(SearchResponse response) { + hits.addAndGet(response.getHits().totalHits()) + results.addAll(response.hits.collect { it.source }) + if (response.getAggregations() != null) { + InternalNested internalNested = response.getAggregations().get("tag") + if (internalNested != null) { + Terms terms = internalNested.getAggregations().get("count_agg") + Collection buckets = terms.getBuckets() + for (Terms.Bucket bucket : buckets) { + bucketTags.put(Integer.valueOf(bucket.getKeyAsString()), bucket.getDocCount()) + } + } + } + return null + } + }) + + then: + hits.get() == 2 + results[0] == [id: "2", data: "doc b"] + results[1] == [id: "1", data: "doc a"] + bucketTags == [:] + + assertTraces(TEST_WRITER, 1) { + trace(0, 1) { + span(0) { + serviceName "elasticsearch" + resourceName "SearchAction" + operationName "elasticsearch.query" + tags { + "$Tags.COMPONENT.key" "elasticsearch-java" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "elasticsearch.action" "SearchAction" + "elasticsearch.request" "SearchRequest" + "elasticsearch.request.indices" indexName + defaultTags() + } + } + } + } + + where: + indexName = "test-index-extract" + } +} diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/Elasticsearch2TransportClientTest.groovy b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/Elasticsearch2TransportClientTest.groovy index 0886d2c694..60b95d9c1d 100644 --- a/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/Elasticsearch2TransportClientTest.groovy +++ b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/Elasticsearch2TransportClientTest.groovy @@ -132,10 +132,13 @@ class Elasticsearch2TransportClientTest extends AgentTestRunner { def "test elasticsearch get"() { setup: + assert TEST_WRITER == [] def indexResult = client.admin().indices().prepareCreate(indexName).get() + TEST_WRITER.waitForTraces(1) expect: indexResult.acknowledged + TEST_WRITER.size() == 1 when: client.admin().cluster().prepareHealth().setWaitForYellowStatus().execute().actionGet(5000) diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/springdata/Config.groovy b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/springdata/Config.groovy new file mode 100644 index 0000000000..0af1eaa061 --- /dev/null +++ b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/springdata/Config.groovy @@ -0,0 +1,49 @@ +package springdata + +import org.elasticsearch.common.io.FileSystemUtils +import org.elasticsearch.common.settings.Settings +import org.elasticsearch.node.NodeBuilder +import org.springframework.context.annotation.Bean +import org.springframework.context.annotation.ComponentScan +import org.springframework.context.annotation.Configuration +import org.springframework.data.elasticsearch.core.ElasticsearchOperations +import org.springframework.data.elasticsearch.core.ElasticsearchTemplate +import org.springframework.data.elasticsearch.repository.config.EnableElasticsearchRepositories + +@Configuration +@EnableElasticsearchRepositories(basePackages = "springdata") +@ComponentScan(basePackages = "springdata") +class Config { + + @Bean + NodeBuilder nodeBuilder() { + return new NodeBuilder() + } + + @Bean + ElasticsearchOperations elasticsearchTemplate() { + + def tmpDir = File.createTempFile("test-es-working-dir-", "") + tmpDir.delete() + tmpDir.mkdir() + tmpDir.deleteOnExit() + + System.addShutdownHook { + if (tmpDir != null) { + FileSystemUtils.deleteSubDirectories(esWorkingDir.toPath()) + tmpDir.delete() + } + } + + final Settings.Builder elasticsearchSettings = + Settings.settingsBuilder() + .put("http.enabled", "false") + .put("path.data", tmpDir.toString()) + .put("path.home", tmpDir.toString()) + + println "ES work dir: $tmpDir" + + return new ElasticsearchTemplate(nodeBuilder().local(true) + .settings(elasticsearchSettings.build()).node().client()) + } +} diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/springdata/Doc.groovy b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/springdata/Doc.groovy new file mode 100644 index 0000000000..b00adf02c8 --- /dev/null +++ b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/springdata/Doc.groovy @@ -0,0 +1,29 @@ +package springdata + +import groovy.transform.EqualsAndHashCode +import org.springframework.data.annotation.Id +import org.springframework.data.elasticsearch.annotations.Document + +@Document(indexName = "test-index") +@EqualsAndHashCode +class Doc { + @Id + private String id = "1" + private String data = "some data" + + String getId() { + return id + } + + void setId(String id) { + this.id = id + } + + String getData() { + return data + } + + void setData(String data) { + this.data = data + } +} diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/springdata/DocRepository.groovy b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/springdata/DocRepository.groovy new file mode 100644 index 0000000000..ab41d9513b --- /dev/null +++ b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/springdata/DocRepository.groovy @@ -0,0 +1,5 @@ +package springdata + +import org.springframework.data.elasticsearch.repository.ElasticsearchRepository + +interface DocRepository extends ElasticsearchRepository {} diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/springdata/Elasticsearch2SpringRepositoryTest.groovy b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/springdata/Elasticsearch2SpringRepositoryTest.groovy new file mode 100644 index 0000000000..e0b5dc1253 --- /dev/null +++ b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/test/groovy/springdata/Elasticsearch2SpringRepositoryTest.groovy @@ -0,0 +1,283 @@ +package springdata + +import datadog.trace.agent.test.AgentTestRunner +import io.opentracing.tag.Tags +import org.springframework.context.ApplicationContext +import org.springframework.context.annotation.AnnotationConfigApplicationContext +import spock.lang.Shared + +import static datadog.trace.agent.test.ListWriterAssert.assertTraces + +class Elasticsearch2SpringRepositoryTest extends AgentTestRunner { + static { + System.setProperty("dd.integration.elasticsearch.enabled", "true") + } + + @Shared + ApplicationContext applicationContext = new AnnotationConfigApplicationContext(Config) + + @Shared + DocRepository repo = applicationContext.getBean(DocRepository) + + def setup() { + repo.deleteAll() + TEST_WRITER.waitForTraces(4) + TEST_WRITER.clear() + } + + def "test empty repo"() { + when: + def result = repo.findAll() + + then: + !result.iterator().hasNext() + + and: + assertTraces(TEST_WRITER, 1) { + trace(0, 1) { + span(0) { + serviceName "elasticsearch" + resourceName "SearchAction" + operationName "elasticsearch.query" + spanType null + errored false + tags { + "$Tags.COMPONENT.key" "elasticsearch-java" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "elasticsearch.action" "SearchAction" + "elasticsearch.request" "SearchRequest" + "elasticsearch.request.indices" indexName + "elasticsearch.request.search.types" "doc" + defaultTags() + } + } + } + } + + where: + indexName = "test-index" + } + + def "test CRUD"() { + when: + def doc = new Doc() + + then: + repo.index(doc) == doc + + and: + assertTraces(TEST_WRITER, 3) { + trace(0, 1) { + span(0) { + resourceName "PutMappingAction" + operationName "elasticsearch.query" + tags { + "$Tags.COMPONENT.key" "elasticsearch-java" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "elasticsearch.action" "PutMappingAction" + "elasticsearch.request" "PutMappingRequest" + "elasticsearch.request.indices" indexName + defaultTags() + } + } + } + trace(1, 1) { + span(0) { + resourceName "IndexAction" + operationName "elasticsearch.query" + tags { + "$Tags.COMPONENT.key" "elasticsearch-java" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$Tags.PEER_HOSTNAME.key" "local" + "$Tags.PEER_HOST_IPV4.key" "0.0.0.0" + "$Tags.PEER_PORT.key" 0 + "elasticsearch.action" "IndexAction" + "elasticsearch.request" "IndexRequest" + "elasticsearch.request.indices" indexName + "elasticsearch.request.write.type" "doc" + defaultTags() + } + } + } + trace(2, 1) { + span(0) { + resourceName "RefreshAction" + operationName "elasticsearch.query" + tags { + "$Tags.COMPONENT.key" "elasticsearch-java" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "elasticsearch.action" "RefreshAction" + "elasticsearch.request" "RefreshRequest" + "elasticsearch.request.indices" indexName + "elasticsearch.shard.broadcast.failed" 0 + "elasticsearch.shard.broadcast.successful" 5 + "elasticsearch.shard.broadcast.total" 10 + defaultTags() + } + } + } + } + TEST_WRITER.clear() + + and: + repo.findOne("1") == doc + + and: + assertTraces(TEST_WRITER, 1) { + trace(0, 1) { + span(0) { + serviceName "elasticsearch" + resourceName "GetAction" + operationName "elasticsearch.query" + tags { + "$Tags.COMPONENT.key" "elasticsearch-java" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$Tags.PEER_HOSTNAME.key" "local" + "$Tags.PEER_HOST_IPV4.key" "0.0.0.0" + "$Tags.PEER_PORT.key" 0 + "elasticsearch.action" "GetAction" + "elasticsearch.request" "GetRequest" + "elasticsearch.request.indices" indexName + "elasticsearch.type" "doc" + "elasticsearch.id" "1" + "elasticsearch.version" 1 + defaultTags() + } + } + } + } + TEST_WRITER.clear() + + when: + doc.data == "other data" + + then: + repo.index(doc) == doc + repo.findOne("1") == doc + + and: + assertTraces(TEST_WRITER, 3) { + trace(0, 1) { + span(0) { + resourceName "IndexAction" + operationName "elasticsearch.query" + tags { + "$Tags.COMPONENT.key" "elasticsearch-java" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$Tags.PEER_HOSTNAME.key" "local" + "$Tags.PEER_HOST_IPV4.key" "0.0.0.0" + "$Tags.PEER_PORT.key" 0 + "elasticsearch.action" "IndexAction" + "elasticsearch.request" "IndexRequest" + "elasticsearch.request.indices" indexName + "elasticsearch.request.write.type" "doc" + defaultTags() + } + } + } + trace(1, 1) { + span(0) { + resourceName "RefreshAction" + operationName "elasticsearch.query" + tags { + "$Tags.COMPONENT.key" "elasticsearch-java" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "elasticsearch.action" "RefreshAction" + "elasticsearch.request" "RefreshRequest" + "elasticsearch.request.indices" indexName + "elasticsearch.shard.broadcast.failed" 0 + "elasticsearch.shard.broadcast.successful" 5 + "elasticsearch.shard.broadcast.total" 10 + defaultTags() + } + } + } + trace(2, 1) { + span(0) { + serviceName "elasticsearch" + resourceName "GetAction" + operationName "elasticsearch.query" + tags { + "$Tags.COMPONENT.key" "elasticsearch-java" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$Tags.PEER_HOSTNAME.key" "local" + "$Tags.PEER_HOST_IPV4.key" "0.0.0.0" + "$Tags.PEER_PORT.key" 0 + "elasticsearch.action" "GetAction" + "elasticsearch.request" "GetRequest" + "elasticsearch.request.indices" indexName + "elasticsearch.type" "doc" + "elasticsearch.id" "1" + "elasticsearch.version" 2 + defaultTags() + } + } + } + } + TEST_WRITER.clear() + + when: + repo.delete("1") + + then: + !repo.findAll().iterator().hasNext() + + and: + assertTraces(TEST_WRITER, 3) { + trace(0, 1) { + span(0) { + resourceName "DeleteAction" + operationName "elasticsearch.query" + tags { + "$Tags.COMPONENT.key" "elasticsearch-java" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$Tags.PEER_HOSTNAME.key" "local" + "$Tags.PEER_HOST_IPV4.key" "0.0.0.0" + "$Tags.PEER_PORT.key" 0 + "elasticsearch.action" "DeleteAction" + "elasticsearch.request" "DeleteRequest" + "elasticsearch.request.indices" indexName + "elasticsearch.request.write.type" "doc" + defaultTags() + } + } + } + trace(1, 1) { + span(0) { + resourceName "RefreshAction" + operationName "elasticsearch.query" + tags { + "$Tags.COMPONENT.key" "elasticsearch-java" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "elasticsearch.action" "RefreshAction" + "elasticsearch.request" "RefreshRequest" + "elasticsearch.request.indices" indexName + "elasticsearch.shard.broadcast.failed" 0 + "elasticsearch.shard.broadcast.successful" 5 + "elasticsearch.shard.broadcast.total" 10 + defaultTags() + } + } + } + trace(2, 1) { + span(0) { + serviceName "elasticsearch" + resourceName "SearchAction" + operationName "elasticsearch.query" + tags { + "$Tags.COMPONENT.key" "elasticsearch-java" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "elasticsearch.action" "SearchAction" + "elasticsearch.request" "SearchRequest" + "elasticsearch.request.indices" indexName + "elasticsearch.request.search.types" "doc" + defaultTags() + } + } + } + } + + where: + indexName = "test-index" + } +} diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-5/src/main/java/datadog/trace/instrumentation/elasticsearch5/Elasticsearch5TransportClientInstrumentation.java b/dd-java-agent/instrumentation/elasticsearch-transport-5/src/main/java/datadog/trace/instrumentation/elasticsearch5/Elasticsearch5TransportClientInstrumentation.java index 1dbafa4561..5554787e0b 100644 --- a/dd-java-agent/instrumentation/elasticsearch-transport-5/src/main/java/datadog/trace/instrumentation/elasticsearch5/Elasticsearch5TransportClientInstrumentation.java +++ b/dd-java-agent/instrumentation/elasticsearch-transport-5/src/main/java/datadog/trace/instrumentation/elasticsearch5/Elasticsearch5TransportClientInstrumentation.java @@ -50,6 +50,9 @@ public class Elasticsearch5TransportClientInstrumentation extends Instrumenter.C new HelperInjector( "com.google.common.base.Preconditions", "com.google.common.base.Joiner", + "com.google.common.base.Joiner$1", + "com.google.common.base.Joiner$2", + "com.google.common.base.Joiner$MapJoiner", "datadog.trace.instrumentation.elasticsearch5.TransportActionListener")) .transform(DDTransformers.defaultTransformers()) .transform( diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-5/src/main/java/datadog/trace/instrumentation/elasticsearch5/TransportActionListener.java b/dd-java-agent/instrumentation/elasticsearch-transport-5/src/main/java/datadog/trace/instrumentation/elasticsearch5/TransportActionListener.java index 17fecbbc8e..9345c1c652 100644 --- a/dd-java-agent/instrumentation/elasticsearch-transport-5/src/main/java/datadog/trace/instrumentation/elasticsearch5/TransportActionListener.java +++ b/dd-java-agent/instrumentation/elasticsearch-transport-5/src/main/java/datadog/trace/instrumentation/elasticsearch5/TransportActionListener.java @@ -14,6 +14,7 @@ import org.elasticsearch.action.IndicesRequest; import org.elasticsearch.action.bulk.BulkShardResponse; import org.elasticsearch.action.get.GetResponse; import org.elasticsearch.action.index.IndexResponse; +import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.support.broadcast.BroadcastResponse; import org.elasticsearch.action.support.nodes.BaseNodesResponse; import org.elasticsearch.action.support.replication.ReplicationResponse; @@ -40,6 +41,10 @@ public class TransportActionListener implements Action span.setTag("elasticsearch.request.indices", Joiner.on(",").join(req.indices())); } } + if (request instanceof SearchRequest) { + final SearchRequest req = (SearchRequest) request; + span.setTag("elasticsearch.request.search.types", Joiner.on(",").join(req.types())); + } if (request instanceof DocumentRequest) { final DocumentRequest req = (DocumentRequest) request; span.setTag("elasticsearch.request.write.type", req.type()); diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-5/src/test/groovy/Elasticsearch5NodeClientTest.groovy b/dd-java-agent/instrumentation/elasticsearch-transport-5/src/test/groovy/Elasticsearch5NodeClientTest.groovy index 19f1a9cf2c..ee2062b867 100644 --- a/dd-java-agent/instrumentation/elasticsearch-transport-5/src/test/groovy/Elasticsearch5NodeClientTest.groovy +++ b/dd-java-agent/instrumentation/elasticsearch-transport-5/src/test/groovy/Elasticsearch5NodeClientTest.groovy @@ -124,10 +124,13 @@ class Elasticsearch5NodeClientTest extends AgentTestRunner { def "test elasticsearch get"() { setup: + assert TEST_WRITER == [] def indexResult = client.admin().indices().prepareCreate(indexName).get() + TEST_WRITER.waitForTraces(1) expect: indexResult.acknowledged + TEST_WRITER.size() == 1 when: client.admin().cluster().prepareHealth().setWaitForYellowStatus().execute().actionGet(5000) diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-5/src/test/groovy/Elasticsearch5TransportClientTest.groovy b/dd-java-agent/instrumentation/elasticsearch-transport-5/src/test/groovy/Elasticsearch5TransportClientTest.groovy index 6e506d8aa1..9fd5084e0d 100644 --- a/dd-java-agent/instrumentation/elasticsearch-transport-5/src/test/groovy/Elasticsearch5TransportClientTest.groovy +++ b/dd-java-agent/instrumentation/elasticsearch-transport-5/src/test/groovy/Elasticsearch5TransportClientTest.groovy @@ -139,10 +139,13 @@ class Elasticsearch5TransportClientTest extends AgentTestRunner { def "test elasticsearch get"() { setup: + assert TEST_WRITER == [] def indexResult = client.admin().indices().prepareCreate(indexName).get() + TEST_WRITER.waitForTraces(1) expect: indexResult.acknowledged + TEST_WRITER.size() == 1 when: def emptyResult = client.prepareGet(indexName, indexType, id).get() diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-6/src/main/java/datadog/trace/instrumentation/elasticsearch6/Elasticsearch6TransportClientInstrumentation.java b/dd-java-agent/instrumentation/elasticsearch-transport-6/src/main/java/datadog/trace/instrumentation/elasticsearch6/Elasticsearch6TransportClientInstrumentation.java index f1e80adac4..d82515f0b3 100644 --- a/dd-java-agent/instrumentation/elasticsearch-transport-6/src/main/java/datadog/trace/instrumentation/elasticsearch6/Elasticsearch6TransportClientInstrumentation.java +++ b/dd-java-agent/instrumentation/elasticsearch-transport-6/src/main/java/datadog/trace/instrumentation/elasticsearch6/Elasticsearch6TransportClientInstrumentation.java @@ -54,6 +54,9 @@ public class Elasticsearch6TransportClientInstrumentation extends Instrumenter.C new HelperInjector( "com.google.common.base.Preconditions", "com.google.common.base.Joiner", + "com.google.common.base.Joiner$1", + "com.google.common.base.Joiner$2", + "com.google.common.base.Joiner$MapJoiner", "datadog.trace.instrumentation.elasticsearch6.TransportActionListener")) .transform(DDTransformers.defaultTransformers()) .transform( diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-6/src/main/java/datadog/trace/instrumentation/elasticsearch6/TransportActionListener.java b/dd-java-agent/instrumentation/elasticsearch-transport-6/src/main/java/datadog/trace/instrumentation/elasticsearch6/TransportActionListener.java index 488f0160c7..e170f5bbe3 100644 --- a/dd-java-agent/instrumentation/elasticsearch-transport-6/src/main/java/datadog/trace/instrumentation/elasticsearch6/TransportActionListener.java +++ b/dd-java-agent/instrumentation/elasticsearch-transport-6/src/main/java/datadog/trace/instrumentation/elasticsearch6/TransportActionListener.java @@ -14,6 +14,7 @@ import org.elasticsearch.action.IndicesRequest; import org.elasticsearch.action.bulk.BulkShardResponse; import org.elasticsearch.action.get.GetResponse; import org.elasticsearch.action.index.IndexResponse; +import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.support.broadcast.BroadcastResponse; import org.elasticsearch.action.support.nodes.BaseNodesResponse; import org.elasticsearch.action.support.replication.ReplicationResponse; @@ -44,6 +45,10 @@ public class TransportActionListener implements Action span.setTag("elasticsearch.request.indices", Joiner.on(",").join(req.indices())); } } + if (request instanceof SearchRequest) { + final SearchRequest req = (SearchRequest) request; + span.setTag("elasticsearch.request.search.types", Joiner.on(",").join(req.types())); + } if (request instanceof DocWriteRequest) { final DocWriteRequest req = (DocWriteRequest) request; span.setTag("elasticsearch.request.write.type", req.type()); diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-6/src/test/groovy/Elasticsearch6NodeClientTest.groovy b/dd-java-agent/instrumentation/elasticsearch-transport-6/src/test/groovy/Elasticsearch6NodeClientTest.groovy index 90ef3d13fa..4fa588a492 100644 --- a/dd-java-agent/instrumentation/elasticsearch-transport-6/src/test/groovy/Elasticsearch6NodeClientTest.groovy +++ b/dd-java-agent/instrumentation/elasticsearch-transport-6/src/test/groovy/Elasticsearch6NodeClientTest.groovy @@ -120,10 +120,13 @@ class Elasticsearch6NodeClientTest extends AgentTestRunner { def "test elasticsearch get"() { setup: + assert TEST_WRITER == [] def indexResult = client.admin().indices().prepareCreate(indexName).get() + TEST_WRITER.waitForTraces(1) expect: indexResult.index() == indexName + TEST_WRITER.size() == 1 when: def emptyResult = client.prepareGet(indexName, indexType, id).get() diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-6/src/test/groovy/Elasticsearch6TransportClientTest.groovy b/dd-java-agent/instrumentation/elasticsearch-transport-6/src/test/groovy/Elasticsearch6TransportClientTest.groovy index ddbd8ed953..fc7fe80bc8 100644 --- a/dd-java-agent/instrumentation/elasticsearch-transport-6/src/test/groovy/Elasticsearch6TransportClientTest.groovy +++ b/dd-java-agent/instrumentation/elasticsearch-transport-6/src/test/groovy/Elasticsearch6TransportClientTest.groovy @@ -135,10 +135,13 @@ class Elasticsearch6TransportClientTest extends AgentTestRunner { def "test elasticsearch get"() { setup: + assert TEST_WRITER == [] def indexResult = client.admin().indices().prepareCreate(indexName).get() + TEST_WRITER.waitForTraces(1) expect: indexResult.index() == indexName + TEST_WRITER.size() == 1 when: def emptyResult = client.prepareGet(indexName, indexType, id).get() diff --git a/dd-java-agent/instrumentation/url-connection/url-connection.gradle b/dd-java-agent/instrumentation/http-url-connection/http-url-connection.gradle similarity index 100% rename from dd-java-agent/instrumentation/url-connection/url-connection.gradle rename to dd-java-agent/instrumentation/http-url-connection/http-url-connection.gradle diff --git a/dd-java-agent/instrumentation/url-connection/src/main/java/datadog/trace/instrumentation/url_connection/UrlConnectionInstrumentation.java b/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/HttpUrlConnectionInstrumentation.java similarity index 64% rename from dd-java-agent/instrumentation/url-connection/src/main/java/datadog/trace/instrumentation/url_connection/UrlConnectionInstrumentation.java rename to dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/HttpUrlConnectionInstrumentation.java index a88a50ec21..b743b018f6 100644 --- a/dd-java-agent/instrumentation/url-connection/src/main/java/datadog/trace/instrumentation/url_connection/UrlConnectionInstrumentation.java +++ b/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/HttpUrlConnectionInstrumentation.java @@ -1,13 +1,11 @@ -package datadog.trace.instrumentation.url_connection; +package datadog.trace.instrumentation.http_url_connection; import static io.opentracing.log.Fields.ERROR_OBJECT; -import static net.bytebuddy.matcher.ElementMatchers.is; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.isSubTypeOf; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; 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.DDAdvice; @@ -23,22 +21,18 @@ import io.opentracing.Tracer; import io.opentracing.propagation.Format; import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; -import java.io.IOException; import java.net.HttpURLConnection; import java.net.URL; -import java.net.URLConnection; import java.util.Collections; import javax.net.ssl.HttpsURLConnection; import net.bytebuddy.agent.builder.AgentBuilder; import net.bytebuddy.asm.Advice; -import sun.net.www.protocol.ftp.FtpURLConnection; -import sun.net.www.protocol.mailto.MailToURLConnection; @AutoService(Instrumenter.class) -public class UrlConnectionInstrumentation extends Instrumenter.Configurable { +public class HttpUrlConnectionInstrumentation extends Instrumenter.Configurable { - public UrlConnectionInstrumentation() { - super("urlconnection", "httpurlconnection"); + public HttpUrlConnectionInstrumentation() { + super("httpurlconnection"); } @Override @@ -49,14 +43,10 @@ public class UrlConnectionInstrumentation extends Instrumenter.Configurable { @Override public AgentBuilder apply(final AgentBuilder agentBuilder) { return agentBuilder - .type( - not( - is(sun.net.www.protocol.jar.JarURLConnection.class) - .or(is(sun.net.www.protocol.file.FileURLConnection.class))) - .and(isSubTypeOf(URLConnection.class))) + .type(isSubTypeOf(HttpURLConnection.class)) .transform( new HelperInjector( - "datadog.trace.instrumentation.url_connection.MessageHeadersInjectAdapter")) + "datadog.trace.instrumentation.http_url_connection.MessageHeadersInjectAdapter")) .transform(DDTransformers.defaultTransformers()) .transform( DDAdvice.create() @@ -68,38 +58,36 @@ public class UrlConnectionInstrumentation extends Instrumenter.Configurable { .or(named("getOutputStream")) .or(named("getInputStream")) .or(nameStartsWith("getHeaderField"))), - UrlConnectionAdvice.class.getName())) + HttpUrlConnectionAdvice.class.getName())) .asDecorator(); } - public static class UrlConnectionAdvice { + public static class HttpUrlConnectionAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static Scope startSpan( - @Advice.This final URLConnection connection, @Advice.Origin("#m") final String methodName) { + @Advice.This final HttpURLConnection connection, + @Advice.FieldValue("connected") final boolean connected, + @Advice.Origin("#m") final String methodName) { + + final boolean isTraceRequest = + Thread.currentThread().getName().equals("dd-agent-writer") + || connection.getRequestProperty("Datadog-Meta-Lang") != null; + if (isTraceRequest) { + return null; + } + + final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(HttpURLConnection.class); + if (callDepth > 0) { + return null; + } + String protocol = "url"; if (connection != null) { final URL url = connection.getURL(); protocol = url.getProtocol(); } - final boolean isValidProtocol = - protocol.equals("http") - || protocol.equals("https") - || protocol.equals("ftp") - || protocol.equals("mailto"); - final boolean isTraceRequest = - Thread.currentThread().getName().equals("dd-agent-writer") - || (connection != null && connection.getRequestProperty("Datadog-Meta-Lang") != null); - if (!isValidProtocol || isTraceRequest) { - return null; - } - - final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(URLConnection.class); - if (callDepth > 0) { - return null; - } - String command = ".request.response_code"; if (methodName.equals("getOutputStream")) { command = ".request.output_stream"; @@ -125,22 +113,17 @@ public class UrlConnectionInstrumentation extends Instrumenter.Configurable { Tags.PEER_PORT.set(span, url.getPort()); } else if (connection instanceof HttpsURLConnection) { Tags.PEER_PORT.set(span, 443); - } else if (connection instanceof HttpURLConnection) { + } else { Tags.PEER_PORT.set(span, 80); - } else if (connection instanceof FtpURLConnection) { - Tags.PEER_PORT.set(span, 21); - } else if (connection instanceof MailToURLConnection) { - Tags.PEER_PORT.set(span, 25); } - if (connection instanceof HttpURLConnection) { - - Tags.HTTP_METHOD.set(span, ((HttpURLConnection) connection).getRequestMethod()); + Tags.HTTP_METHOD.set(span, connection.getRequestMethod()); + if (!connected) { tracer.inject( span.context(), Format.Builtin.HTTP_HEADERS, - new MessageHeadersInjectAdapter((HttpURLConnection) connection)); + new MessageHeadersInjectAdapter(connection)); } } return scope; @@ -148,7 +131,7 @@ public class UrlConnectionInstrumentation extends Instrumenter.Configurable { @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void stopSpan( - @Advice.This final URLConnection connection, + @Advice.FieldValue("responseCode") final int responseCode, @Advice.Enter final Scope scope, @Advice.Thrown final Throwable throwable) { @@ -161,14 +144,11 @@ public class UrlConnectionInstrumentation extends Instrumenter.Configurable { if (throwable != null) { Tags.ERROR.set(span, true); span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); - } else if (connection instanceof HttpURLConnection) { - try { - Tags.HTTP_STATUS.set(span, ((HttpURLConnection) connection).getResponseCode()); - } catch (final IOException e) { - } + } else if (responseCode > 0) { + Tags.HTTP_STATUS.set(span, responseCode); } scope.close(); - CallDepthThreadLocalMap.reset(URLConnection.class); + CallDepthThreadLocalMap.reset(HttpURLConnection.class); } } } diff --git a/dd-java-agent/instrumentation/url-connection/src/main/java/datadog/trace/instrumentation/url_connection/MessageHeadersInjectAdapter.java b/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/MessageHeadersInjectAdapter.java similarity index 67% rename from dd-java-agent/instrumentation/url-connection/src/main/java/datadog/trace/instrumentation/url_connection/MessageHeadersInjectAdapter.java rename to dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/MessageHeadersInjectAdapter.java index 642ed8c2b5..22aa124e2b 100644 --- a/dd-java-agent/instrumentation/url-connection/src/main/java/datadog/trace/instrumentation/url_connection/MessageHeadersInjectAdapter.java +++ b/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/MessageHeadersInjectAdapter.java @@ -1,4 +1,4 @@ -package datadog.trace.instrumentation.url_connection; +package datadog.trace.instrumentation.http_url_connection; import io.opentracing.propagation.TextMap; import java.net.HttpURLConnection; @@ -15,8 +15,12 @@ public class MessageHeadersInjectAdapter implements TextMap { @Override public void put(final String key, final String value) { - if (connection.getRequestProperty(key) == null) { - connection.setRequestProperty(key, value); + try { + if (connection.getRequestProperty(key) == null) { + connection.setRequestProperty(key, value); + } + } catch (final IllegalStateException e) { + // Connection is already established. Too late to set headers. } } diff --git a/dd-java-agent/instrumentation/url-connection/src/main/java/datadog/trace/instrumentation/url_connection/UrlInstrumentation.java b/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/UrlInstrumentation.java similarity index 97% rename from dd-java-agent/instrumentation/url-connection/src/main/java/datadog/trace/instrumentation/url_connection/UrlInstrumentation.java rename to dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/UrlInstrumentation.java index c20a659b6c..8ddada8d54 100644 --- a/dd-java-agent/instrumentation/url-connection/src/main/java/datadog/trace/instrumentation/url_connection/UrlInstrumentation.java +++ b/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/UrlInstrumentation.java @@ -1,4 +1,4 @@ -package datadog.trace.instrumentation.url_connection; +package datadog.trace.instrumentation.http_url_connection; import static io.opentracing.log.Fields.ERROR_OBJECT; import static net.bytebuddy.matcher.ElementMatchers.is; diff --git a/dd-java-agent/instrumentation/url-connection/src/test/groovy/HttpUrlConnectionTest.groovy b/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/HttpUrlConnectionTest.groovy similarity index 83% rename from dd-java-agent/instrumentation/url-connection/src/test/groovy/HttpUrlConnectionTest.groovy rename to dd-java-agent/instrumentation/http-url-connection/src/test/groovy/HttpUrlConnectionTest.groovy index 4f09fb5d5b..ccd337a576 100644 --- a/dd-java-agent/instrumentation/url-connection/src/test/groovy/HttpUrlConnectionTest.groovy +++ b/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/HttpUrlConnectionTest.groovy @@ -18,14 +18,16 @@ import static ratpack.http.HttpMethod.POST class HttpUrlConnectionTest extends AgentTestRunner { static { - System.setProperty("dd.integration.urlconnection.enabled", "true") + System.setProperty("dd.integration.httpurlconnection.enabled", "true") } + static final RESPONSE = "

Hello test.

" + static final STATUS = 202 + @Shared def server = ratpack { handlers { all { - String msg = "

Hello test.

\n" boolean isDDServer = true if (context.request.getHeaders().contains("is-dd-server")) { isDDServer = Boolean.parseBoolean(context.request.getHeaders().get("is-dd-server")) @@ -42,7 +44,15 @@ class HttpUrlConnectionTest extends AgentTestRunner { scope.close() } - response.status(201).send(msg) + response.status(STATUS) + // Ratpack seems to be sending body with HEAD requests - RFC specifically forbids this. + // This becomes a major problem with keep-alived requests - client seems to fail to parse + // such response properly messing up following requests. + if (request.method.isHead()) { + response.send() + } else { + response.send(RESPONSE) + } } } } @@ -55,17 +65,17 @@ class HttpUrlConnectionTest extends AgentTestRunner { def stream = connection.inputStream def lines = stream.readLines() stream.close() - assert connection.getResponseCode() == 201 - assert lines == ["

Hello test.

"] + assert connection.getResponseCode() == STATUS + assert lines == [RESPONSE] // call again to ensure the cycling is ok connection = server.getAddress().toURL().openConnection() assert GlobalTracer.get().scopeManager().active() != null + assert connection.getResponseCode() == STATUS // call before input stream to test alternate behavior stream = connection.inputStream lines = stream.readLines() stream.close() - assert connection.getResponseCode() == 201 - assert lines == ["

Hello test.

"] + assert lines == [RESPONSE] } expect: @@ -100,22 +110,6 @@ class HttpUrlConnectionTest extends AgentTestRunner { } } span(1) { - operationName "http.request.response_code" - 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" "GET" - "$Tags.HTTP_STATUS.key" 201 - "$Tags.PEER_HOSTNAME.key" "localhost" - "$Tags.PEER_PORT.key" server.address.port - defaultTags() - } - } - span(2) { operationName "http.request.input_stream" childOf span(0) errored false @@ -125,7 +119,23 @@ class HttpUrlConnectionTest extends AgentTestRunner { "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT "$Tags.HTTP_URL.key" "$server.address" "$Tags.HTTP_METHOD.key" "GET" - "$Tags.HTTP_STATUS.key" 201 + "$Tags.HTTP_STATUS.key" STATUS + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" server.address.port + defaultTags() + } + } + span(2) { + operationName "http.request.response_code" + 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" "GET" + "$Tags.HTTP_STATUS.key" STATUS "$Tags.PEER_HOSTNAME.key" "localhost" "$Tags.PEER_PORT.key" server.address.port defaultTags() @@ -141,7 +151,7 @@ class HttpUrlConnectionTest extends AgentTestRunner { "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT "$Tags.HTTP_URL.key" "$server.address" "$Tags.HTTP_METHOD.key" "GET" - "$Tags.HTTP_STATUS.key" 201 + "$Tags.HTTP_STATUS.key" STATUS "$Tags.PEER_HOSTNAME.key" "localhost" "$Tags.PEER_PORT.key" server.address.port defaultTags() @@ -157,7 +167,7 @@ class HttpUrlConnectionTest extends AgentTestRunner { "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT "$Tags.HTTP_URL.key" "$server.address" "$Tags.HTTP_METHOD.key" "GET" - "$Tags.HTTP_STATUS.key" 201 + "$Tags.HTTP_STATUS.key" STATUS "$Tags.PEER_HOSTNAME.key" "localhost" "$Tags.PEER_PORT.key" server.address.port defaultTags() @@ -176,18 +186,18 @@ class HttpUrlConnectionTest extends AgentTestRunner { def stream = connection.inputStream def lines = stream.readLines() stream.close() - assert connection.getResponseCode() == 201 - assert lines == ["

Hello test.

"] + assert connection.getResponseCode() == STATUS + assert lines == [RESPONSE] // call again to ensure the cycling is ok connection = server.getAddress().toURL().openConnection() connection.addRequestProperty("is-dd-server", "false") assert GlobalTracer.get().scopeManager().active() != null + assert connection.getResponseCode() == STATUS // call before input stream to test alternate behavior stream = connection.inputStream lines = stream.readLines() stream.close() - assert connection.getResponseCode() == 201 - assert lines == ["

Hello test.

"] + assert lines == [RESPONSE] } expect: @@ -202,22 +212,6 @@ class HttpUrlConnectionTest extends AgentTestRunner { } } span(1) { - operationName "http.request.response_code" - 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" "GET" - "$Tags.HTTP_STATUS.key" 201 - "$Tags.PEER_HOSTNAME.key" "localhost" - "$Tags.PEER_PORT.key" server.address.port - defaultTags() - } - } - span(2) { operationName "http.request.input_stream" childOf span(0) errored false @@ -227,7 +221,23 @@ class HttpUrlConnectionTest extends AgentTestRunner { "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT "$Tags.HTTP_URL.key" "$server.address" "$Tags.HTTP_METHOD.key" "GET" - "$Tags.HTTP_STATUS.key" 201 + "$Tags.HTTP_STATUS.key" STATUS + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" server.address.port + defaultTags() + } + } + span(2) { + operationName "http.request.response_code" + 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" "GET" + "$Tags.HTTP_STATUS.key" STATUS "$Tags.PEER_HOSTNAME.key" "localhost" "$Tags.PEER_PORT.key" server.address.port defaultTags() @@ -243,7 +253,7 @@ class HttpUrlConnectionTest extends AgentTestRunner { "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT "$Tags.HTTP_URL.key" "$server.address" "$Tags.HTTP_METHOD.key" "GET" - "$Tags.HTTP_STATUS.key" 201 + "$Tags.HTTP_STATUS.key" STATUS "$Tags.PEER_HOSTNAME.key" "localhost" "$Tags.PEER_PORT.key" server.address.port defaultTags() @@ -259,7 +269,7 @@ class HttpUrlConnectionTest extends AgentTestRunner { "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT "$Tags.HTTP_URL.key" "$server.address" "$Tags.HTTP_METHOD.key" "GET" - "$Tags.HTTP_STATUS.key" 201 + "$Tags.HTTP_STATUS.key" STATUS "$Tags.PEER_HOSTNAME.key" "localhost" "$Tags.PEER_PORT.key" server.address.port defaultTags() @@ -276,7 +286,7 @@ class HttpUrlConnectionTest extends AgentTestRunner { connection.setRequestMethod(HEAD.name) connection.addRequestProperty("is-dd-server", "false") assert GlobalTracer.get().scopeManager().active() != null - assert connection.getResponseCode() == 201 + assert connection.getResponseCode() == STATUS } expect: @@ -300,7 +310,7 @@ class HttpUrlConnectionTest extends AgentTestRunner { "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT "$Tags.HTTP_URL.key" "$server.address" "$Tags.HTTP_METHOD.key" "HEAD" - "$Tags.HTTP_STATUS.key" 201 + "$Tags.HTTP_STATUS.key" STATUS "$Tags.PEER_HOSTNAME.key" "localhost" "$Tags.PEER_PORT.key" server.address.port defaultTags() @@ -325,7 +335,12 @@ class HttpUrlConnectionTest extends AgentTestRunner { wr.flush() wr.close() - assert connection.getResponseCode() == 201 + assert connection.getResponseCode() == STATUS + + def stream = connection.inputStream + def lines = stream.readLines() + stream.close() + assert lines == [RESPONSE] } expect: @@ -333,14 +348,14 @@ class HttpUrlConnectionTest extends AgentTestRunner { trace(0, 1) { span(0) { operationName "test-http-server" - childOf(TEST_WRITER[1][2]) + childOf(TEST_WRITER[1][3]) errored false tags { defaultTags() } } } - trace(1, 3) { + trace(1, 4) { span(0) { operationName "someTrace" parent() @@ -350,6 +365,22 @@ class HttpUrlConnectionTest extends AgentTestRunner { } } span(1) { + operationName "http.request.input_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.HTTP_STATUS.key" STATUS + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" server.address.port + defaultTags() + } + } + span(2) { operationName "http.request.response_code" childOf span(0) errored false @@ -359,13 +390,13 @@ class HttpUrlConnectionTest extends AgentTestRunner { "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT "$Tags.HTTP_URL.key" "$server.address" "$Tags.HTTP_METHOD.key" "POST" - "$Tags.HTTP_STATUS.key" 201 + "$Tags.HTTP_STATUS.key" STATUS "$Tags.PEER_HOSTNAME.key" "localhost" "$Tags.PEER_PORT.key" server.address.port defaultTags() } } - span(2) { + span(3) { operationName "http.request.output_stream" childOf span(0) errored false @@ -375,7 +406,6 @@ class HttpUrlConnectionTest extends AgentTestRunner { "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT "$Tags.HTTP_URL.key" "$server.address" "$Tags.HTTP_METHOD.key" "POST" - "$Tags.HTTP_STATUS.key" 201 "$Tags.PEER_HOSTNAME.key" "localhost" "$Tags.PEER_PORT.key" server.address.port defaultTags() @@ -394,8 +424,8 @@ class HttpUrlConnectionTest extends AgentTestRunner { def stream = connection.inputStream def lines = stream.readLines() stream.close() - assert connection.getResponseCode() == 201 - assert lines == ["

Hello test.

"] + assert connection.getResponseCode() == STATUS + assert lines == [RESPONSE] } expect: diff --git a/dd-java-agent/instrumentation/url-connection/src/test/groovy/UrlConnectionTest.groovy b/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy similarity index 90% rename from dd-java-agent/instrumentation/url-connection/src/test/groovy/UrlConnectionTest.groovy rename to dd-java-agent/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy index 06004279ea..efe36877a5 100644 --- a/dd-java-agent/instrumentation/url-connection/src/test/groovy/UrlConnectionTest.groovy +++ b/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy @@ -10,7 +10,7 @@ import static datadog.trace.agent.test.TestUtils.runUnderTrace class UrlConnectionTest extends AgentTestRunner { static { - System.setProperty("dd.integration.urlconnection.enabled", "true") + System.setProperty("dd.integration.httpurlconnection.enabled", "true") } private static final int INVALID_PORT = TestUtils.randomOpenPort() @@ -49,9 +49,7 @@ class UrlConnectionTest extends AgentTestRunner { "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT "$Tags.HTTP_URL.key" "$url" - if (scheme.startsWith("http")) { - "$Tags.HTTP_METHOD.key" "GET" - } + "$Tags.HTTP_METHOD.key" "GET" "$Tags.PEER_HOSTNAME.key" "localhost" "$Tags.PEER_PORT.key" INVALID_PORT errorTags ConnectException, "Connection refused (Connection refused)" @@ -63,7 +61,6 @@ class UrlConnectionTest extends AgentTestRunner { where: scheme | component - "ftp" | "FtpURLConnection" "http" | "HttpURLConnection" "https" | "HttpsURLConnectionImpl" diff --git a/settings.gradle b/settings.gradle index 3c9761353e..f77a40d1db 100644 --- a/settings.gradle +++ b/settings.gradle @@ -18,6 +18,7 @@ include ':dd-java-agent:instrumentation:elasticsearch-rest-5' include ':dd-java-agent:instrumentation:elasticsearch-transport-2' include ':dd-java-agent:instrumentation:elasticsearch-transport-5' include ':dd-java-agent:instrumentation:elasticsearch-transport-6' +include ':dd-java-agent:instrumentation:http-url-connection' include ':dd-java-agent:instrumentation:hystrix-1.4' include ':dd-java-agent:instrumentation:jax-rs-annotations' include ':dd-java-agent:instrumentation:jax-rs-client' @@ -43,7 +44,6 @@ include ':dd-java-agent:instrumentation:servlet-3' include ':dd-java-agent:instrumentation:sparkjava-2.4' include ':dd-java-agent:instrumentation:spring-web' include ':dd-java-agent:instrumentation:trace-annotation' -include ':dd-java-agent:instrumentation:url-connection' // benchmark include ':dd-java-agent:benchmark'