From 2c240754fc4988026a8a559ad531682f94f6ba4e Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Fri, 14 Jun 2019 10:08:17 -0700 Subject: [PATCH 01/24] Additional testing for split-by-instance config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some DB’s don’t define an instance, so verify the setting has no effect for them. --- .../groovy/util/AbstractCouchbaseTest.groovy | 6 +++ .../test/groovy/CassandraClientTest.groovy | 46 +++++++++++-------- .../groovy/JDBCInstrumentationTest.groovy | 40 ++++++++-------- .../src/test/groovy/JedisClientTest.groovy | 6 +++ .../src/test/groovy/MongoClientTest.groovy | 15 ++++-- .../spymemcached/SpymemcachedTest.groovy | 6 +++ 6 files changed, 76 insertions(+), 43 deletions(-) diff --git a/dd-java-agent/instrumentation/couchbase-2.0/src/test/groovy/util/AbstractCouchbaseTest.groovy b/dd-java-agent/instrumentation/couchbase-2.0/src/test/groovy/util/AbstractCouchbaseTest.groovy index 9d25162423..b5e3f25087 100644 --- a/dd-java-agent/instrumentation/couchbase-2.0/src/test/groovy/util/AbstractCouchbaseTest.groovy +++ b/dd-java-agent/instrumentation/couchbase-2.0/src/test/groovy/util/AbstractCouchbaseTest.groovy @@ -16,6 +16,7 @@ import com.couchbase.mock.CouchbaseMock import com.couchbase.mock.http.query.QueryServer import datadog.trace.agent.test.AgentTestRunner import datadog.trace.agent.test.utils.PortUtils +import datadog.trace.api.Config import spock.lang.Shared import java.util.concurrent.RejectedExecutionException @@ -87,6 +88,9 @@ abstract class AbstractCouchbaseTest extends AgentTestRunner { // Cache buckets: couchbaseCluster.openBucket(bucketCouchbase.name(), bucketCouchbase.password()) memcacheCluster.openBucket(bucketMemcache.name(), bucketMemcache.password()) + + // This setting should have no effect since decorator returns null for the instance. + System.setProperty(Config.PREFIX + Config.DB_CLIENT_HOST_SPLIT_BY_INSTANCE, "true") } private static BucketConfiguration convert(BucketSettings bucketSettings) { @@ -112,6 +116,8 @@ abstract class AbstractCouchbaseTest extends AgentTestRunner { } mock?.stop() + + System.clearProperty(Config.PREFIX + Config.DB_CLIENT_HOST_SPLIT_BY_INSTANCE) } private DefaultCouchbaseEnvironment.Builder envBuilder(BucketSettings bucketSettings) { diff --git a/dd-java-agent/instrumentation/datastax-cassandra-3/src/test/groovy/CassandraClientTest.groovy b/dd-java-agent/instrumentation/datastax-cassandra-3/src/test/groovy/CassandraClientTest.groovy index 18968c1ea1..007d00cfb6 100644 --- a/dd-java-agent/instrumentation/datastax-cassandra-3/src/test/groovy/CassandraClientTest.groovy +++ b/dd-java-agent/instrumentation/datastax-cassandra-3/src/test/groovy/CassandraClientTest.groovy @@ -3,11 +3,13 @@ import com.datastax.driver.core.Session import datadog.opentracing.DDSpan import datadog.trace.agent.test.AgentTestRunner import datadog.trace.agent.test.asserts.TraceAssert +import datadog.trace.api.Config import datadog.trace.api.DDSpanTypes import io.opentracing.tag.Tags import org.cassandraunit.utils.EmbeddedCassandraServerHelper import spock.lang.Shared +import static datadog.trace.agent.test.utils.ConfigUtils.withConfigOverride import static datadog.trace.agent.test.utils.TraceUtils.basicSpan import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace @@ -44,17 +46,19 @@ class CassandraClientTest extends AgentTestRunner { setup: Session session = cluster.connect(keyspace) - session.execute(statement) + withConfigOverride(Config.DB_CLIENT_HOST_SPLIT_BY_INSTANCE, "$renameService") { + session.execute(statement) + } expect: assertTraces(keyspace ? 2 : 1) { if (keyspace) { trace(0, 1) { - cassandraSpan(it, 0, "USE $keyspace", null) + cassandraSpan(it, 0, "USE $keyspace", null, false) } } trace(keyspace ? 1 : 0, 1) { - cassandraSpan(it, 0, statement, keyspace) + cassandraSpan(it, 0, statement, keyspace, renameService) } } @@ -62,19 +66,21 @@ class CassandraClientTest extends AgentTestRunner { session.close() where: - statement | keyspace - "DROP KEYSPACE IF EXISTS sync_test" | null - "CREATE KEYSPACE sync_test WITH REPLICATION = {'class':'SimpleStrategy', 'replication_factor':3}" | null - "CREATE TABLE sync_test.users ( id UUID PRIMARY KEY, name text )" | "sync_test" - "INSERT INTO sync_test.users (id, name) values (uuid(), 'alice')" | "sync_test" - "SELECT * FROM users where name = 'alice' ALLOW FILTERING" | "sync_test" + statement | keyspace | renameService + "DROP KEYSPACE IF EXISTS sync_test" | null | false + "CREATE KEYSPACE sync_test WITH REPLICATION = {'class':'SimpleStrategy', 'replication_factor':3}" | null | true + "CREATE TABLE sync_test.users ( id UUID PRIMARY KEY, name text )" | "sync_test" | false + "INSERT INTO sync_test.users (id, name) values (uuid(), 'alice')" | "sync_test" | false + "SELECT * FROM users where name = 'alice' ALLOW FILTERING" | "sync_test" | true } def "test async"() { setup: Session session = cluster.connect(keyspace) runUnderTrace("parent") { - session.executeAsync(statement) + withConfigOverride(Config.DB_CLIENT_HOST_SPLIT_BY_INSTANCE, "$renameService") { + session.executeAsync(statement) + } blockUntilChildSpansFinished(1) } @@ -82,12 +88,12 @@ class CassandraClientTest extends AgentTestRunner { assertTraces(keyspace ? 2 : 1) { if (keyspace) { trace(0, 1) { - cassandraSpan(it, 0, "USE $keyspace", null) + cassandraSpan(it, 0, "USE $keyspace", null, false) } } trace(keyspace ? 1 : 0, 2) { basicSpan(it, 0, "parent") - cassandraSpan(it, 1, statement, keyspace, span(0)) + cassandraSpan(it, 1, statement, keyspace, renameService, span(0)) } } @@ -95,17 +101,17 @@ class CassandraClientTest extends AgentTestRunner { session.close() where: - statement | keyspace - "DROP KEYSPACE IF EXISTS async_test" | null - "CREATE KEYSPACE async_test WITH REPLICATION = {'class':'SimpleStrategy', 'replication_factor':3}" | null - "CREATE TABLE async_test.users ( id UUID PRIMARY KEY, name text )" | "async_test" - "INSERT INTO async_test.users (id, name) values (uuid(), 'alice')" | "async_test" - "SELECT * FROM users where name = 'alice' ALLOW FILTERING" | "async_test" + statement | keyspace | renameService + "DROP KEYSPACE IF EXISTS async_test" | null | false + "CREATE KEYSPACE async_test WITH REPLICATION = {'class':'SimpleStrategy', 'replication_factor':3}" | null | true + "CREATE TABLE async_test.users ( id UUID PRIMARY KEY, name text )" | "async_test" | false + "INSERT INTO async_test.users (id, name) values (uuid(), 'alice')" | "async_test" | false + "SELECT * FROM users where name = 'alice' ALLOW FILTERING" | "async_test" | true } - def cassandraSpan(TraceAssert trace, int index, String statement, String keyspace, Object parentSpan = null, Throwable exception = null) { + def cassandraSpan(TraceAssert trace, int index, String statement, String keyspace, boolean renameService, Object parentSpan = null, Throwable exception = null) { trace.span(index) { - serviceName "cassandra" + serviceName renameService && keyspace ? keyspace : "cassandra" operationName "cassandra.query" resourceName statement spanType DDSpanTypes.CASSANDRA diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTest.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTest.groovy index 9ecc358c19..2211ae4458 100644 --- a/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTest.groovy @@ -2,6 +2,7 @@ import com.mchange.v2.c3p0.ComboPooledDataSource import com.zaxxer.hikari.HikariConfig import com.zaxxer.hikari.HikariDataSource import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.api.Config import datadog.trace.api.DDSpanTypes import io.opentracing.tag.Tags import javax.sql.DataSource @@ -17,6 +18,7 @@ import java.sql.PreparedStatement import java.sql.ResultSet import java.sql.Statement +import static datadog.trace.agent.test.utils.ConfigUtils.withConfigOverride import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace class JDBCInstrumentationTest extends AgentTestRunner { @@ -154,7 +156,9 @@ class JDBCInstrumentationTest extends AgentTestRunner { setup: Statement statement = connection.createStatement() ResultSet resultSet = runUnderTrace("parent") { - return statement.executeQuery(query) + withConfigOverride(Config.DB_CLIENT_HOST_SPLIT_BY_INSTANCE, "$renameService") { + return statement.executeQuery(query) + } } expect: @@ -167,8 +171,8 @@ class JDBCInstrumentationTest extends AgentTestRunner { parent() } span(1) { + serviceName renameService ? dbName.toLowerCase() : driver operationName "${driver}.query" - serviceName driver resourceName query spanType DDSpanTypes.SQL childOf span(0) @@ -193,22 +197,22 @@ class JDBCInstrumentationTest extends AgentTestRunner { connection.close() where: - driver | connection | username | query - "h2" | new Driver().connect(jdbcUrls.get("h2"), null) | null | "SELECT 3" - "derby" | new EmbeddedDriver().connect(jdbcUrls.get("derby"), null) | "APP" | "SELECT 3 FROM SYSIBM.SYSDUMMY1" - "hsqldb" | new JDBCDriver().connect(jdbcUrls.get("hsqldb"), null) | "SA" | "SELECT 3 FROM INFORMATION_SCHEMA.SYSTEM_USERS" - "h2" | new Driver().connect(jdbcUrls.get("h2"), connectionProps) | null | "SELECT 3" - "derby" | new EmbeddedDriver().connect(jdbcUrls.get("derby"), connectionProps) | "APP" | "SELECT 3 FROM SYSIBM.SYSDUMMY1" - "hsqldb" | new JDBCDriver().connect(jdbcUrls.get("hsqldb"), connectionProps) | "SA" | "SELECT 3 FROM INFORMATION_SCHEMA.SYSTEM_USERS" - "h2" | cpDatasources.get("tomcat").get("h2").getConnection() | null | "SELECT 3" - "derby" | cpDatasources.get("tomcat").get("derby").getConnection() | "APP" | "SELECT 3 FROM SYSIBM.SYSDUMMY1" - "hsqldb" | cpDatasources.get("tomcat").get("hsqldb").getConnection() | "SA" | "SELECT 3 FROM INFORMATION_SCHEMA.SYSTEM_USERS" - "h2" | cpDatasources.get("hikari").get("h2").getConnection() | null | "SELECT 3" - "derby" | cpDatasources.get("hikari").get("derby").getConnection() | "APP" | "SELECT 3 FROM SYSIBM.SYSDUMMY1" - "hsqldb" | cpDatasources.get("hikari").get("hsqldb").getConnection() | "SA" | "SELECT 3 FROM INFORMATION_SCHEMA.SYSTEM_USERS" - "h2" | cpDatasources.get("c3p0").get("h2").getConnection() | null | "SELECT 3" - "derby" | cpDatasources.get("c3p0").get("derby").getConnection() | "APP" | "SELECT 3 FROM SYSIBM.SYSDUMMY1" - "hsqldb" | cpDatasources.get("c3p0").get("hsqldb").getConnection() | "SA" | "SELECT 3 FROM INFORMATION_SCHEMA.SYSTEM_USERS" + driver | connection | username | renameService | query + "h2" | new Driver().connect(jdbcUrls.get("h2"), null) | null | false | "SELECT 3" + "derby" | new EmbeddedDriver().connect(jdbcUrls.get("derby"), null) | "APP" | false | "SELECT 3 FROM SYSIBM.SYSDUMMY1" + "hsqldb" | new JDBCDriver().connect(jdbcUrls.get("hsqldb"), null) | "SA" | false | "SELECT 3 FROM INFORMATION_SCHEMA.SYSTEM_USERS" + "h2" | new Driver().connect(jdbcUrls.get("h2"), connectionProps) | null | true | "SELECT 3" + "derby" | new EmbeddedDriver().connect(jdbcUrls.get("derby"), connectionProps) | "APP" | true | "SELECT 3 FROM SYSIBM.SYSDUMMY1" + "hsqldb" | new JDBCDriver().connect(jdbcUrls.get("hsqldb"), connectionProps) | "SA" | true | "SELECT 3 FROM INFORMATION_SCHEMA.SYSTEM_USERS" + "h2" | cpDatasources.get("tomcat").get("h2").getConnection() | null | false | "SELECT 3" + "derby" | cpDatasources.get("tomcat").get("derby").getConnection() | "APP" | false | "SELECT 3 FROM SYSIBM.SYSDUMMY1" + "hsqldb" | cpDatasources.get("tomcat").get("hsqldb").getConnection() | "SA" | true | "SELECT 3 FROM INFORMATION_SCHEMA.SYSTEM_USERS" + "h2" | cpDatasources.get("hikari").get("h2").getConnection() | null | false | "SELECT 3" + "derby" | cpDatasources.get("hikari").get("derby").getConnection() | "APP" | true | "SELECT 3 FROM SYSIBM.SYSDUMMY1" + "hsqldb" | cpDatasources.get("hikari").get("hsqldb").getConnection() | "SA" | false | "SELECT 3 FROM INFORMATION_SCHEMA.SYSTEM_USERS" + "h2" | cpDatasources.get("c3p0").get("h2").getConnection() | null | true | "SELECT 3" + "derby" | cpDatasources.get("c3p0").get("derby").getConnection() | "APP" | false | "SELECT 3 FROM SYSIBM.SYSDUMMY1" + "hsqldb" | cpDatasources.get("c3p0").get("hsqldb").getConnection() | "SA" | false | "SELECT 3 FROM INFORMATION_SCHEMA.SYSTEM_USERS" } @Unroll diff --git a/dd-java-agent/instrumentation/jedis-1.4/src/test/groovy/JedisClientTest.groovy b/dd-java-agent/instrumentation/jedis-1.4/src/test/groovy/JedisClientTest.groovy index a28d642a2a..42041c647b 100644 --- a/dd-java-agent/instrumentation/jedis-1.4/src/test/groovy/JedisClientTest.groovy +++ b/dd-java-agent/instrumentation/jedis-1.4/src/test/groovy/JedisClientTest.groovy @@ -1,4 +1,5 @@ import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.api.Config import datadog.trace.api.DDSpanTypes import io.opentracing.tag.Tags import redis.clients.jedis.Jedis @@ -22,11 +23,16 @@ class JedisClientTest extends AgentTestRunner { def setupSpec() { println "Using redis: $redisServer.args" redisServer.start() + + // This setting should have no effect since decorator returns null for the instance. + System.setProperty(Config.PREFIX + Config.DB_CLIENT_HOST_SPLIT_BY_INSTANCE, "true") } def cleanupSpec() { redisServer.stop() // jedis.close() // not available in the early version we're using. + + System.clearProperty(Config.PREFIX + Config.DB_CLIENT_HOST_SPLIT_BY_INSTANCE) } def setup() { diff --git a/dd-java-agent/instrumentation/mongo/driver-3.1/src/test/groovy/MongoClientTest.groovy b/dd-java-agent/instrumentation/mongo/driver-3.1/src/test/groovy/MongoClientTest.groovy index 66ded8bb60..f21f85546f 100644 --- a/dd-java-agent/instrumentation/mongo/driver-3.1/src/test/groovy/MongoClientTest.groovy +++ b/dd-java-agent/instrumentation/mongo/driver-3.1/src/test/groovy/MongoClientTest.groovy @@ -6,6 +6,7 @@ import com.mongodb.client.MongoCollection import com.mongodb.client.MongoDatabase import datadog.opentracing.DDSpan import datadog.trace.agent.test.asserts.TraceAssert +import datadog.trace.api.Config import datadog.trace.api.DDSpanTypes import io.opentracing.tag.Tags import org.bson.BsonDocument @@ -13,6 +14,7 @@ import org.bson.BsonString import org.bson.Document import spock.lang.Shared +import static datadog.trace.agent.test.utils.ConfigUtils.withConfigOverride import static datadog.trace.agent.test.utils.PortUtils.UNUSABLE_PORT import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace @@ -38,18 +40,21 @@ class MongoClientTest extends MongoBaseTest { MongoDatabase db = client.getDatabase(dbName) when: - db.createCollection(collectionName) + withConfigOverride(Config.DB_CLIENT_HOST_SPLIT_BY_INSTANCE, "$renameService") { + db.createCollection(collectionName) + } then: assertTraces(1) { trace(0, 1) { - mongoSpan(it, 0, "{\"create\":\"$collectionName\",\"capped\":\"?\"}") + mongoSpan(it, 0, "{\"create\":\"$collectionName\",\"capped\":\"?\"}", renameService) } } where: dbName = "test_db" collectionName = "testCollection" + renameService << [false, true] } def "test create collection no description"() { @@ -62,7 +67,7 @@ class MongoClientTest extends MongoBaseTest { then: assertTraces(1) { trace(0, 1) { - mongoSpan(it, 0, "{\"create\":\"$collectionName\",\"capped\":\"?\"}", dbName) + mongoSpan(it, 0, "{\"create\":\"$collectionName\",\"capped\":\"?\"}", false, dbName) } } @@ -228,9 +233,9 @@ class MongoClientTest extends MongoBaseTest { collectionName = "testCollection" } - def mongoSpan(TraceAssert trace, int index, String statement, String instance = "some-description", Object parentSpan = null, Throwable exception = null) { + def mongoSpan(TraceAssert trace, int index, String statement, boolean renameService = false, String instance = "some-description", Object parentSpan = null, Throwable exception = null) { trace.span(index) { - serviceName "mongo" + serviceName renameService ? instance : "mongo" operationName "mongo.query" resourceName { assert it.replace(" ", "") == statement diff --git a/dd-java-agent/instrumentation/spymemcached-2.12/src/test/groovy/datadog/trace/instrumentation/spymemcached/SpymemcachedTest.groovy b/dd-java-agent/instrumentation/spymemcached-2.12/src/test/groovy/datadog/trace/instrumentation/spymemcached/SpymemcachedTest.groovy index 2f02de169e..7fba808b7d 100644 --- a/dd-java-agent/instrumentation/spymemcached-2.12/src/test/groovy/datadog/trace/instrumentation/spymemcached/SpymemcachedTest.groovy +++ b/dd-java-agent/instrumentation/spymemcached-2.12/src/test/groovy/datadog/trace/instrumentation/spymemcached/SpymemcachedTest.groovy @@ -3,6 +3,7 @@ package datadog.trace.instrumentation.spymemcached import com.google.common.util.concurrent.MoreExecutors import datadog.trace.agent.test.AgentTestRunner import datadog.trace.agent.test.asserts.TraceAssert +import datadog.trace.api.Config import datadog.trace.api.DDSpanTypes import io.opentracing.tag.Tags import net.spy.memcached.CASResponse @@ -71,12 +72,17 @@ class SpymemcachedTest extends AgentTestRunner { memcachedContainer.getMappedPort(defaultMemcachedPort) ) } + + // This setting should have no effect since decorator returns null for the instance. + System.setProperty(Config.PREFIX + Config.DB_CLIENT_HOST_SPLIT_BY_INSTANCE, "true") } def cleanupSpec() { if (memcachedContainer) { memcachedContainer.stop() } + + System.clearProperty(Config.PREFIX + Config.DB_CLIENT_HOST_SPLIT_BY_INSTANCE) } ReentrantLock queueLock From 2027027e4635b61fce70f462d21e46ab4fca2825 Mon Sep 17 00:00:00 2001 From: Luca Abbati Date: Wed, 3 Jul 2019 15:24:51 +0200 Subject: [PATCH 02/24] Explitely run JmxFetch app as a daemon --- dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle | 2 +- .../src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle b/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle index c075f69f72..5caf2e4730 100644 --- a/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle +++ b/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle @@ -4,7 +4,7 @@ plugins { apply from: "${rootDir}/gradle/java.gradle" dependencies { - compile('com.datadoghq:jmxfetch:0.29.0'){ + compile('com.datadoghq:jmxfetch:0.30.0--------------yet-to-be-released'){ exclude group: 'org.slf4j', module: 'slf4j-log4j12' exclude group: 'log4j', module: 'log4j' } diff --git a/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java b/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java index 321e626314..b432aba41b 100644 --- a/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java +++ b/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java @@ -74,6 +74,8 @@ public class JMXFetch { final AppConfig.AppConfigBuilder configBuilder = AppConfig.builder() .action(ImmutableList.of(ACTION_COLLECT)) + // App should be run as daemon otherwise CLI apps would not exit once main method exits. + .daemon(true) .confdDirectory(jmxFetchConfigDir) .yamlFileList(jmxFetchConfigs) .targetDirectInstances(true) From 2e41e94774a3eff38ac9a448d566fcd15f6f1a35 Mon Sep 17 00:00:00 2001 From: Luca Abbati Date: Mon, 8 Jul 2019 14:43:29 -0400 Subject: [PATCH 03/24] Update jmxfetch to version 0.30.0 --- dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle b/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle index 5caf2e4730..b2f5f55862 100644 --- a/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle +++ b/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle @@ -4,7 +4,7 @@ plugins { apply from: "${rootDir}/gradle/java.gradle" dependencies { - compile('com.datadoghq:jmxfetch:0.30.0--------------yet-to-be-released'){ + compile('com.datadoghq:jmxfetch:0.30.0'){ exclude group: 'org.slf4j', module: 'slf4j-log4j12' exclude group: 'log4j', module: 'log4j' } From f53b14bd0fcf60b3ec9f74bcd28de28ad8b31660 Mon Sep 17 00:00:00 2001 From: Laplie Anderson Date: Mon, 8 Jul 2019 14:44:56 -0500 Subject: [PATCH 04/24] Stub google http client integration --- .../google-http-client.gradle | 32 +++++++++++++ .../GoogleHttpClientDecorator.java | 46 +++++++++++++++++++ .../GoogleHttpClientInstrumentation.java | 25 ++++++++++ settings.gradle | 1 + 4 files changed, 104 insertions(+) create mode 100644 dd-java-agent/instrumentation/google-http-client/google-http-client.gradle create mode 100644 dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/apachehttpclient/GoogleHttpClientDecorator.java create mode 100644 dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/apachehttpclient/GoogleHttpClientInstrumentation.java diff --git a/dd-java-agent/instrumentation/google-http-client/google-http-client.gradle b/dd-java-agent/instrumentation/google-http-client/google-http-client.gradle new file mode 100644 index 0000000000..d5ffe59130 --- /dev/null +++ b/dd-java-agent/instrumentation/google-http-client/google-http-client.gradle @@ -0,0 +1,32 @@ +muzzle { + pass { + group = "com.google.http-client" + module = "google-http-client" + versions = "[1.2.0,)" + } +} + +apply from: "${rootDir}/gradle/java.gradle" +apply plugin: 'org.unbroken-dome.test-sets' + +testSets { + latestDepTest { + dirName = 'test' + } +} + +dependencies { + compileOnly group: 'com.google.http-client', name: 'google-http-client', version: '1.20.0' + + compile project(':dd-java-agent:agent-tooling') + + compile deps.bytebuddy + compile deps.opentracing + annotationProcessor deps.autoservice + implementation deps.autoservice + + testCompile project(':dd-java-agent:testing') + testCompile group: 'com.google.http-client', name: 'google-http-client', version: '1.20.0' + + latestDepTestCompile group: 'com.google.http-client', name: 'google-http-client', version: '+' +} diff --git a/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/apachehttpclient/GoogleHttpClientDecorator.java b/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/apachehttpclient/GoogleHttpClientDecorator.java new file mode 100644 index 0000000000..d9371e0f42 --- /dev/null +++ b/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/apachehttpclient/GoogleHttpClientDecorator.java @@ -0,0 +1,46 @@ +package datadog.trace.instrumentation.apachehttpclient; + +import com.google.api.client.http.HttpRequest; +import com.google.api.client.http.HttpResponse; +import datadog.trace.agent.decorator.HttpClientDecorator; +import java.net.URI; +import java.net.URISyntaxException; + +public class GoogleHttpClientDecorator extends HttpClientDecorator { + public static final GoogleHttpClientDecorator DECORATE = new GoogleHttpClientDecorator(); + + @Override + protected String method(final HttpRequest httpRequest) { + return httpRequest.getRequestMethod(); + } + + @Override + protected URI url(final HttpRequest httpRequest) throws URISyntaxException { + return httpRequest.getUrl().toURI(); + } + + @Override + protected String hostname(final HttpRequest httpRequest) { + return httpRequest.getUrl().getHost(); + } + + @Override + protected Integer port(final HttpRequest httpRequest) { + return httpRequest.getUrl().getPort(); + } + + @Override + protected Integer status(final HttpResponse httpResponse) { + return httpResponse.getStatusCode(); + } + + @Override + protected String[] instrumentationNames() { + return new String[] {"google-http-client"}; + } + + @Override + protected String component() { + return "google-http-client"; + } +} diff --git a/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/apachehttpclient/GoogleHttpClientInstrumentation.java b/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/apachehttpclient/GoogleHttpClientInstrumentation.java new file mode 100644 index 0000000000..de8781eb80 --- /dev/null +++ b/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/apachehttpclient/GoogleHttpClientInstrumentation.java @@ -0,0 +1,25 @@ +package datadog.trace.instrumentation.apachehttpclient; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; +import java.util.Map; + +@AutoService(Instrumenter.class) +public class GoogleHttpClientInstrumentation extends Instrumenter.Default { + public GoogleHttpClientInstrumentation() { + super("google-http-client"); + } + + @Override + public ElementMatcher typeMatcher() { + return null; + } + + @Override + public Map, String> transformers() { + return null; + } +} diff --git a/settings.gradle b/settings.gradle index 23b68dd31f..7fa89c512e 100644 --- a/settings.gradle +++ b/settings.gradle @@ -41,6 +41,7 @@ include ':dd-java-agent:instrumentation:elasticsearch:transport-5' include ':dd-java-agent:instrumentation:elasticsearch:transport-5.3' include ':dd-java-agent:instrumentation:elasticsearch:transport-6' include ':dd-java-agent:instrumentation:glassfish' +include ':dd-java-agent:instrumentation:google-http-client' include ':dd-java-agent:instrumentation:grpc-1.5' include ':dd-java-agent:instrumentation:hibernate' include ':dd-java-agent:instrumentation:hibernate:core-3.3' From 30916ac5d7424d45f43b6f5c27c177cebe83f370 Mon Sep 17 00:00:00 2001 From: Laplie Anderson Date: Mon, 8 Jul 2019 17:37:14 -0400 Subject: [PATCH 05/24] Implementation --- .../GoogleHttpClientInstrumentation.java | 102 +++++++++++++++++- 1 file changed, 100 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/apachehttpclient/GoogleHttpClientInstrumentation.java b/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/apachehttpclient/GoogleHttpClientInstrumentation.java index de8781eb80..b54b2147eb 100644 --- a/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/apachehttpclient/GoogleHttpClientInstrumentation.java +++ b/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/apachehttpclient/GoogleHttpClientInstrumentation.java @@ -1,10 +1,29 @@ package datadog.trace.instrumentation.apachehttpclient; +import static datadog.trace.instrumentation.apachehttpclient.GoogleHttpClientDecorator.DECORATE; +import static java.util.Collections.singletonMap; +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.takesArguments; + +import com.google.api.client.http.HttpRequest; +import com.google.api.client.http.HttpResponse; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.bootstrap.CallDepthThreadLocalMap; +import io.opentracing.Scope; +import io.opentracing.Span; +import io.opentracing.log.Fields; +import io.opentracing.propagation.Format; +import io.opentracing.propagation.TextMap; +import io.opentracing.tag.Tags; +import io.opentracing.util.GlobalTracer; +import net.bytebuddy.asm.Advice; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; +import java.util.Iterator; import java.util.Map; @AutoService(Instrumenter.class) @@ -15,11 +34,90 @@ public class GoogleHttpClientInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return null; + // HttpRequest is a final class. Only need to instrument it exactly + return named("com.google.api.client.http.HttpRequest"); + } + + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ClientDecorator", + "datadog.trace.agent.decorator.HttpClientDecorator", + packageName + ".GoogleHttpClientClientDecorator", + getClass().getName() + "$GoogleHttpClientAdvice", + getClass().getName() + "$HeadersInjectAdapter" + }; } @Override public Map, String> transformers() { - return null; + return singletonMap( + isMethod().and(isPublic()).and(named("execute").and(takesArguments(0))), + GoogleHttpClientAdvice.class.getName()); + } + + public static class GoogleHttpClientAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static Scope methodEnter(@Advice.This final HttpRequest request) { + final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(HttpRequest.class); + if (callDepth > 0) { + return null; + } + + final Span span = GlobalTracer.get().buildSpan("http.request").start(); + final Scope scope = GlobalTracer.get().scopeManager().activate(span, false); + DECORATE.afterStart(span); + DECORATE.onRequest(span, request); + GlobalTracer.get() + .inject(span.context(), Format.Builtin.HTTP_HEADERS, new HeadersInjectAdapter(request)); + return scope; + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void methodExit( + @Advice.Enter final Scope scope, + @Advice.Return final HttpResponse response, + @Advice.Thrown final Throwable throwable) { + + if (scope != null) { + try { + final Span span = scope.span(); + DECORATE.onResponse(span, response); + + // If HttpRequest.setThrowExceptionOnExecuteError is set to false, there are no exceptions + // for a failed request. Thus, check the response code + if (!response.isSuccessStatusCode()) { + Tags.ERROR.set(span, true); + span.log(singletonMap(Fields.MESSAGE, response.getStatusMessage())); + } + DECORATE.onError(span, throwable); + + DECORATE.beforeFinish(span); + } finally { + scope.close(); + CallDepthThreadLocalMap.reset(HttpRequest.class); + } + } + } + } + + public static class HeadersInjectAdapter implements TextMap { + private final HttpRequest request; + + public HeadersInjectAdapter(final HttpRequest request) { + this.request = request; + } + + @Override + public Iterator> iterator() { + throw new UnsupportedOperationException( + "This class should be used only with tracer#inject()"); + } + + @Override + public void put(final String key, final String value) { + request.getHeaders().put(key, value); + } } } From d97b1c2d536abe5a96d2819facde506bd700f7ed Mon Sep 17 00:00:00 2001 From: Laplie Anderson Date: Mon, 8 Jul 2019 17:58:42 -0400 Subject: [PATCH 06/24] Fix package --- .../GoogleHttpClientDecorator.java | 2 +- .../GoogleHttpClientInstrumentation.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) rename dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/{apachehttpclient => googlehttpclient}/GoogleHttpClientDecorator.java (95%) rename dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/{apachehttpclient => googlehttpclient}/GoogleHttpClientInstrumentation.java (97%) diff --git a/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/apachehttpclient/GoogleHttpClientDecorator.java b/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/googlehttpclient/GoogleHttpClientDecorator.java similarity index 95% rename from dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/apachehttpclient/GoogleHttpClientDecorator.java rename to dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/googlehttpclient/GoogleHttpClientDecorator.java index d9371e0f42..bebdddcf8c 100644 --- a/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/apachehttpclient/GoogleHttpClientDecorator.java +++ b/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/googlehttpclient/GoogleHttpClientDecorator.java @@ -1,4 +1,4 @@ -package datadog.trace.instrumentation.apachehttpclient; +package datadog.trace.instrumentation.googlehttpclient; import com.google.api.client.http.HttpRequest; import com.google.api.client.http.HttpResponse; diff --git a/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/apachehttpclient/GoogleHttpClientInstrumentation.java b/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/googlehttpclient/GoogleHttpClientInstrumentation.java similarity index 97% rename from dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/apachehttpclient/GoogleHttpClientInstrumentation.java rename to dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/googlehttpclient/GoogleHttpClientInstrumentation.java index b54b2147eb..b0e692d6b4 100644 --- a/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/apachehttpclient/GoogleHttpClientInstrumentation.java +++ b/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/googlehttpclient/GoogleHttpClientInstrumentation.java @@ -1,6 +1,6 @@ -package datadog.trace.instrumentation.apachehttpclient; +package datadog.trace.instrumentation.googlehttpclient; -import static datadog.trace.instrumentation.apachehttpclient.GoogleHttpClientDecorator.DECORATE; +import static datadog.trace.instrumentation.googlehttpclient.GoogleHttpClientDecorator.DECORATE; import static java.util.Collections.singletonMap; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isPublic; From caa7e4426a7a15d6133b01c24f06a55e1be4d7b5 Mon Sep 17 00:00:00 2001 From: Laplie Anderson Date: Tue, 9 Jul 2019 15:08:08 -0400 Subject: [PATCH 07/24] Passing tests. Modify escaping of spaces in urls --- .../GoogleHttpClientDecorator.java | 6 +- .../GoogleHttpClientInstrumentation.java | 9 +-- .../test/groovy/GoogleHttpClientTest.groovy | 67 +++++++++++++++++++ 3 files changed, 77 insertions(+), 5 deletions(-) create mode 100644 dd-java-agent/instrumentation/google-http-client/src/test/groovy/GoogleHttpClientTest.groovy diff --git a/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/googlehttpclient/GoogleHttpClientDecorator.java b/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/googlehttpclient/GoogleHttpClientDecorator.java index bebdddcf8c..4a6d03c1e5 100644 --- a/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/googlehttpclient/GoogleHttpClientDecorator.java +++ b/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/googlehttpclient/GoogleHttpClientDecorator.java @@ -16,7 +16,11 @@ public class GoogleHttpClientDecorator extends HttpClientDecorator { + + @Shared + def requestFactory = new NetHttpTransport().createRequestFactory(); + + @Override + int doRequest(String method, URI uri, Map headers, Closure callback) { + doRequest(method, uri, headers, callback, false); + } + + int doRequest(String method, URI uri, Map headers, Closure callback, boolean throwExceptionOnError) { + GenericUrl genericUrl = new GenericUrl(uri) + + HttpRequest request = requestFactory.buildRequest(method, genericUrl, null) + request.getHeaders().putAll(headers) + request.setThrowExceptionOnExecuteError(throwExceptionOnError) + + HttpResponse response = request.execute() + callback?.call() + + return response.getStatusCode() + } + + @Override + GoogleHttpClientDecorator decorator() { + return GoogleHttpClientDecorator.DECORATE + } + + @Override + boolean testRedirects() { + // Circular redirects don't throw an exception with Google Http Client + return false + } + + def "error traces when exception is not thrown"() { + given: + def uri = server.address.resolve("/error") + + when: + def status = doRequest(method, uri) + + then: + status == 500 + assertTraces(2) { + server.distributedRequestTrace(it, 0, trace(1).last()) + trace(1, size(1)) { + span(0) { + resourceName "$method $uri.path" + spanType DDSpanTypes.HTTP_CLIENT + errored true + } + } + } + + where: + method = "GET" + } +} From 82ee01cadf61cc8f72022e8a24054ca4904e88cc Mon Sep 17 00:00:00 2001 From: Laplie Anderson Date: Fri, 12 Jul 2019 15:41:14 -0400 Subject: [PATCH 08/24] Implement instrumentation for async requests --- .../GoogleHttpClientInstrumentation.java | 113 ++++++++++++++---- .../googlehttpclient/RequestState.java | 10 ++ .../AbstractGoogleHttpClientTest.groovy | 69 +++++++++++ .../groovy/GoogleHttpClientAsyncTest.groovy | 9 ++ .../test/groovy/GoogleHttpClientTest.groovy | 64 +--------- 5 files changed, 182 insertions(+), 83 deletions(-) create mode 100644 dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/googlehttpclient/RequestState.java create mode 100644 dd-java-agent/instrumentation/google-http-client/src/test/groovy/AbstractGoogleHttpClientTest.groovy create mode 100644 dd-java-agent/instrumentation/google-http-client/src/test/groovy/GoogleHttpClientAsyncTest.groovy diff --git a/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/googlehttpclient/GoogleHttpClientInstrumentation.java b/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/googlehttpclient/GoogleHttpClientInstrumentation.java index dba8cae9da..d587ab0aba 100644 --- a/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/googlehttpclient/GoogleHttpClientInstrumentation.java +++ b/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/googlehttpclient/GoogleHttpClientInstrumentation.java @@ -5,13 +5,15 @@ import static java.util.Collections.singletonMap; 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.api.client.http.HttpRequest; import com.google.api.client.http.HttpResponse; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; -import datadog.trace.bootstrap.CallDepthThreadLocalMap; +import datadog.trace.bootstrap.ContextStore; +import datadog.trace.bootstrap.InstrumentationContext; import io.opentracing.Scope; import io.opentracing.Span; import io.opentracing.log.Fields; @@ -19,6 +21,8 @@ import io.opentracing.propagation.Format; import io.opentracing.propagation.TextMap; import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; +import java.util.Collections; +import java.util.HashMap; import java.util.Iterator; import java.util.Map; import net.bytebuddy.asm.Advice; @@ -38,6 +42,12 @@ public class GoogleHttpClientInstrumentation extends Instrumenter.Default { return named("com.google.api.client.http.HttpRequest"); } + @Override + public Map contextStore() { + return Collections.singletonMap( + "com.google.api.client.http.HttpRequest", RequestState.class.getName()); + } + @Override public String[] helperClassNames() { return new String[] { @@ -45,45 +55,71 @@ public class GoogleHttpClientInstrumentation extends Instrumenter.Default { "datadog.trace.agent.decorator.ClientDecorator", "datadog.trace.agent.decorator.HttpClientDecorator", packageName + ".GoogleHttpClientDecorator", + packageName + ".RequestState", getClass().getName() + "$GoogleHttpClientAdvice", + getClass().getName() + "$GoogleHttpClientAsyncAdvice", getClass().getName() + "$HeadersInjectAdapter" }; } @Override public Map, String> transformers() { - return singletonMap( - isMethod().and(isPublic()).and(named("execute").and(takesArguments(0))), + final Map, String> transformers = new HashMap<>(); + transformers.put( + isMethod().and(isPublic()).and(named("execute")).and(takesArguments(0)), GoogleHttpClientAdvice.class.getName()); + + transformers.put( + isMethod() + .and(isPublic()) + .and(named("executeAsync")) + .and(takesArguments(1)) + .and(takesArgument(0, (named("java.util.concurrent.Executor")))), + GoogleHttpClientAsyncAdvice.class.getName()); + + return transformers; } public static class GoogleHttpClientAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) - public static Scope methodEnter(@Advice.This final HttpRequest request) { - final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(HttpRequest.class); - if (callDepth > 0) { - return null; + public static void methodEnter(@Advice.This final HttpRequest request) { + + final ContextStore contextStore = + InstrumentationContext.get(HttpRequest.class, RequestState.class); + + RequestState state = contextStore.get(request); + + if (state == null) { + state = new RequestState(GlobalTracer.get().buildSpan("http.request").start()); + contextStore.put(request, state); } - final Span span = GlobalTracer.get().buildSpan("http.request").start(); - final Scope scope = GlobalTracer.get().scopeManager().activate(span, false); - DECORATE.afterStart(span); - DECORATE.onRequest(span, request); - GlobalTracer.get() - .inject(span.context(), Format.Builtin.HTTP_HEADERS, new HeadersInjectAdapter(request)); - return scope; + final Span span = state.getSpan(); + + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { + DECORATE.afterStart(span); + DECORATE.onRequest(span, request); + GlobalTracer.get() + .inject(span.context(), Format.Builtin.HTTP_HEADERS, new HeadersInjectAdapter(request)); + } } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void methodExit( - @Advice.Enter final Scope scope, + @Advice.This final HttpRequest request, @Advice.Return final HttpResponse response, @Advice.Thrown final Throwable throwable) { - if (scope != null) { - try { - final Span span = scope.span(); + final ContextStore contextStore = + InstrumentationContext.get(HttpRequest.class, RequestState.class); + final RequestState state = contextStore.get(request); + + if (state != null) { + final Span span = state.getSpan(); + + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { DECORATE.onResponse(span, response); + DECORATE.onError(span, throwable); // If HttpRequest.setThrowExceptionOnExecuteError is set to false, there are no exceptions // for a failed request. Thus, check the response code @@ -91,13 +127,46 @@ public class GoogleHttpClientInstrumentation extends Instrumenter.Default { Tags.ERROR.set(span, true); span.log(singletonMap(Fields.MESSAGE, response.getStatusMessage())); } - DECORATE.onError(span, throwable); DECORATE.beforeFinish(span); span.finish(); - } finally { - scope.close(); - CallDepthThreadLocalMap.reset(HttpRequest.class); + } + } + } + } + + public static class GoogleHttpClientAsyncAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void methodEnter(@Advice.This final HttpRequest request) { + final Span span = GlobalTracer.get().buildSpan("http.request").start(); + + final ContextStore contextStore = + InstrumentationContext.get(HttpRequest.class, RequestState.class); + + final RequestState state = new RequestState(span); + contextStore.put(request, state); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void methodExit( + @Advice.This final HttpRequest request, @Advice.Thrown final Throwable throwable) { + + if (throwable != null) { + + final ContextStore contextStore = + InstrumentationContext.get(HttpRequest.class, RequestState.class); + final RequestState state = contextStore.get(request); + + if (state != null) { + final Span span = state.getSpan(); + + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { + DECORATE.onError(span, throwable); + + DECORATE.beforeFinish(span); + span.finish(); + } } } } diff --git a/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/googlehttpclient/RequestState.java b/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/googlehttpclient/RequestState.java new file mode 100644 index 0000000000..8e52df3b72 --- /dev/null +++ b/dd-java-agent/instrumentation/google-http-client/src/main/java/datadog/trace/instrumentation/googlehttpclient/RequestState.java @@ -0,0 +1,10 @@ +package datadog.trace.instrumentation.googlehttpclient; + +import io.opentracing.Span; +import lombok.Data; +import lombok.NonNull; + +@Data +public class RequestState { + @NonNull public Span span; +} diff --git a/dd-java-agent/instrumentation/google-http-client/src/test/groovy/AbstractGoogleHttpClientTest.groovy b/dd-java-agent/instrumentation/google-http-client/src/test/groovy/AbstractGoogleHttpClientTest.groovy new file mode 100644 index 0000000000..2dce2bdb81 --- /dev/null +++ b/dd-java-agent/instrumentation/google-http-client/src/test/groovy/AbstractGoogleHttpClientTest.groovy @@ -0,0 +1,69 @@ +import com.google.api.client.http.GenericUrl +import com.google.api.client.http.HttpRequest +import com.google.api.client.http.HttpResponse +import com.google.api.client.http.javanet.NetHttpTransport +import datadog.trace.agent.test.base.HttpClientTest +import datadog.trace.api.DDSpanTypes +import datadog.trace.instrumentation.googlehttpclient.GoogleHttpClientDecorator +import spock.lang.Shared + +abstract class AbstractGoogleHttpClientTest extends HttpClientTest { + + @Shared + def requestFactory = new NetHttpTransport().createRequestFactory(); + + @Override + int doRequest(String method, URI uri, Map headers, Closure callback) { + doRequest(method, uri, headers, callback, false); + } + + int doRequest(String method, URI uri, Map headers, Closure callback, boolean throwExceptionOnError) { + GenericUrl genericUrl = new GenericUrl(uri) + + HttpRequest request = requestFactory.buildRequest(method, genericUrl, null) + request.getHeaders().putAll(headers) + request.setThrowExceptionOnExecuteError(throwExceptionOnError) + + HttpResponse response = executeRequest(request); + callback?.call() + + return response.getStatusCode() + } + + abstract HttpResponse executeRequest(HttpRequest request); + + @Override + GoogleHttpClientDecorator decorator() { + return GoogleHttpClientDecorator.DECORATE + } + + @Override + boolean testRedirects() { + // Circular redirects don't throw an exception with Google Http Client + return false + } + + def "error traces when exception is not thrown"() { + given: + def uri = server.address.resolve("/error") + + when: + def status = doRequest(method, uri) + + then: + status == 500 + assertTraces(2) { + server.distributedRequestTrace(it, 0, trace(1).last()) + trace(1, size(1)) { + span(0) { + resourceName "$method $uri.path" + spanType DDSpanTypes.HTTP_CLIENT + errored true + } + } + } + + where: + method = "GET" + } +} diff --git a/dd-java-agent/instrumentation/google-http-client/src/test/groovy/GoogleHttpClientAsyncTest.groovy b/dd-java-agent/instrumentation/google-http-client/src/test/groovy/GoogleHttpClientAsyncTest.groovy new file mode 100644 index 0000000000..934e31dc54 --- /dev/null +++ b/dd-java-agent/instrumentation/google-http-client/src/test/groovy/GoogleHttpClientAsyncTest.groovy @@ -0,0 +1,9 @@ +import com.google.api.client.http.HttpRequest +import com.google.api.client.http.HttpResponse + +class GoogleHttpClientAsyncTest extends AbstractGoogleHttpClientTest { + @Override + HttpResponse executeRequest(HttpRequest request) { + return request.executeAsync().get() + } +} diff --git a/dd-java-agent/instrumentation/google-http-client/src/test/groovy/GoogleHttpClientTest.groovy b/dd-java-agent/instrumentation/google-http-client/src/test/groovy/GoogleHttpClientTest.groovy index 944b72dfaf..70272d34e7 100644 --- a/dd-java-agent/instrumentation/google-http-client/src/test/groovy/GoogleHttpClientTest.groovy +++ b/dd-java-agent/instrumentation/google-http-client/src/test/groovy/GoogleHttpClientTest.groovy @@ -1,67 +1,9 @@ -import com.google.api.client.http.GenericUrl import com.google.api.client.http.HttpRequest import com.google.api.client.http.HttpResponse -import com.google.api.client.http.javanet.NetHttpTransport -import datadog.trace.agent.test.base.HttpClientTest -import datadog.trace.api.DDSpanTypes -import datadog.trace.instrumentation.googlehttpclient.GoogleHttpClientDecorator -import spock.lang.Shared - -class GoogleHttpClientTest extends HttpClientTest { - - @Shared - def requestFactory = new NetHttpTransport().createRequestFactory(); +class GoogleHttpClientTest extends AbstractGoogleHttpClientTest { @Override - int doRequest(String method, URI uri, Map headers, Closure callback) { - doRequest(method, uri, headers, callback, false); - } - - int doRequest(String method, URI uri, Map headers, Closure callback, boolean throwExceptionOnError) { - GenericUrl genericUrl = new GenericUrl(uri) - - HttpRequest request = requestFactory.buildRequest(method, genericUrl, null) - request.getHeaders().putAll(headers) - request.setThrowExceptionOnExecuteError(throwExceptionOnError) - - HttpResponse response = request.execute() - callback?.call() - - return response.getStatusCode() - } - - @Override - GoogleHttpClientDecorator decorator() { - return GoogleHttpClientDecorator.DECORATE - } - - @Override - boolean testRedirects() { - // Circular redirects don't throw an exception with Google Http Client - return false - } - - def "error traces when exception is not thrown"() { - given: - def uri = server.address.resolve("/error") - - when: - def status = doRequest(method, uri) - - then: - status == 500 - assertTraces(2) { - server.distributedRequestTrace(it, 0, trace(1).last()) - trace(1, size(1)) { - span(0) { - resourceName "$method $uri.path" - spanType DDSpanTypes.HTTP_CLIENT - errored true - } - } - } - - where: - method = "GET" + HttpResponse executeRequest(HttpRequest request) { + return request.execute(); } } From 8a7336a017d7b44238ed0f6c2233003da9932045 Mon Sep 17 00:00:00 2001 From: Laplie Anderson Date: Fri, 12 Jul 2019 16:06:19 -0400 Subject: [PATCH 09/24] Use 1.19.0 as first supported version --- .../google-http-client/google-http-client.gradle | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/dd-java-agent/instrumentation/google-http-client/google-http-client.gradle b/dd-java-agent/instrumentation/google-http-client/google-http-client.gradle index d5ffe59130..b2631fab38 100644 --- a/dd-java-agent/instrumentation/google-http-client/google-http-client.gradle +++ b/dd-java-agent/instrumentation/google-http-client/google-http-client.gradle @@ -2,7 +2,9 @@ muzzle { pass { group = "com.google.http-client" module = "google-http-client" - versions = "[1.2.0,)" + + // 1.19.0 is the first release. The versions before are betas and RCs + versions = "[1.19.0,)" } } @@ -16,7 +18,7 @@ testSets { } dependencies { - compileOnly group: 'com.google.http-client', name: 'google-http-client', version: '1.20.0' + compileOnly group: 'com.google.http-client', name: 'google-http-client', version: '1.19.0' compile project(':dd-java-agent:agent-tooling') @@ -26,7 +28,7 @@ dependencies { implementation deps.autoservice testCompile project(':dd-java-agent:testing') - testCompile group: 'com.google.http-client', name: 'google-http-client', version: '1.20.0' + testCompile group: 'com.google.http-client', name: 'google-http-client', version: '1.19.0' - latestDepTestCompile group: 'com.google.http-client', name: 'google-http-client', version: '+' + latestDepTestCompile group: 'com.google.http-client', name: 'google-http-client', version: '+' } From 0eab4688cd0aca15c8a634a38a98e8be0a69740c Mon Sep 17 00:00:00 2001 From: Laplie Anderson Date: Fri, 12 Jul 2019 16:36:02 -0400 Subject: [PATCH 10/24] Remove unnecessary semicolons --- .../src/test/groovy/AbstractGoogleHttpClientTest.groovy | 8 ++++---- .../src/test/groovy/GoogleHttpClientTest.groovy | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/dd-java-agent/instrumentation/google-http-client/src/test/groovy/AbstractGoogleHttpClientTest.groovy b/dd-java-agent/instrumentation/google-http-client/src/test/groovy/AbstractGoogleHttpClientTest.groovy index 2dce2bdb81..d255ed2642 100644 --- a/dd-java-agent/instrumentation/google-http-client/src/test/groovy/AbstractGoogleHttpClientTest.groovy +++ b/dd-java-agent/instrumentation/google-http-client/src/test/groovy/AbstractGoogleHttpClientTest.groovy @@ -10,11 +10,11 @@ import spock.lang.Shared abstract class AbstractGoogleHttpClientTest extends HttpClientTest { @Shared - def requestFactory = new NetHttpTransport().createRequestFactory(); + def requestFactory = new NetHttpTransport().createRequestFactory() @Override int doRequest(String method, URI uri, Map headers, Closure callback) { - doRequest(method, uri, headers, callback, false); + doRequest(method, uri, headers, callback, false) } int doRequest(String method, URI uri, Map headers, Closure callback, boolean throwExceptionOnError) { @@ -24,13 +24,13 @@ abstract class AbstractGoogleHttpClientTest extends HttpClientTest Date: Mon, 15 Jul 2019 10:01:34 -0400 Subject: [PATCH 11/24] Avoid Jetty 10 Alpha version of Jetty 10 was released and it seems to be compiled for java versions above 8 so tests fail on java8. Limit latest dep Jetty tests to Jetty 9 for now. --- dd-java-agent/instrumentation/jetty-8/jetty-8.gradle | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/dd-java-agent/instrumentation/jetty-8/jetty-8.gradle b/dd-java-agent/instrumentation/jetty-8/jetty-8.gradle index be5a2d8373..1a7c354b3c 100644 --- a/dd-java-agent/instrumentation/jetty-8/jetty-8.gradle +++ b/dd-java-agent/instrumentation/jetty-8/jetty-8.gradle @@ -34,7 +34,9 @@ dependencies { testCompile group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '8.0.0.v20110901' testCompile group: 'org.eclipse.jetty', name: 'jetty-continuation', version: '8.0.0.v20110901' - latestDepTestCompile group: 'org.eclipse.jetty', name: 'jetty-server', version: '+' - latestDepTestCompile group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '+' - latestDepTestCompile group: 'org.eclipse.jetty', name: 'jetty-continuation', version: '+' + // Jetty 10 seems to refuse to run on java8. + // TODO: we need to setup separate test for Jetty 10 when that is released. + latestDepTestCompile group: 'org.eclipse.jetty', name: 'jetty-server', version: '9.+' + latestDepTestCompile group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '9.+' + latestDepTestCompile group: 'org.eclipse.jetty', name: 'jetty-continuation', version: '9.+' } From 1fbe3955865ff895691f9bc501595ec87cbb16c6 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Mon, 15 Jul 2019 10:14:50 -0400 Subject: [PATCH 12/24] Avoid Jetty 10 in servlet-3 test Alpha version of Jetty 10 was released and it seems to be compiled for java versions above 8 so tests fail on java8. Limit latest dep Jetty tests to Jetty 9 for now. --- dd-java-agent/instrumentation/servlet-3/servlet-3.gradle | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/instrumentation/servlet-3/servlet-3.gradle b/dd-java-agent/instrumentation/servlet-3/servlet-3.gradle index 3ce592b6c8..7ed27b3c41 100644 --- a/dd-java-agent/instrumentation/servlet-3/servlet-3.gradle +++ b/dd-java-agent/instrumentation/servlet-3/servlet-3.gradle @@ -41,8 +41,10 @@ dependencies { testCompile group: 'org.apache.tomcat.embed', name: 'tomcat-embed-core', version: '8.0.41' testCompile group: 'org.apache.tomcat.embed', name: 'tomcat-embed-jasper', version: '8.0.41' - latestDepTestCompile group: 'org.eclipse.jetty', name: 'jetty-server', version: '+' - latestDepTestCompile group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '+' + // Jetty 10 seems to refuse to run on java8. + // TODO: we need to setup separate test for Jetty 10 when that is released. + latestDepTestCompile group: 'org.eclipse.jetty', name: 'jetty-server', version: '9.+' + latestDepTestCompile group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '9.+' latestDepTestCompile group: 'org.apache.tomcat.embed', name: 'tomcat-embed-core', version: '+' latestDepTestCompile group: 'org.apache.tomcat.embed', name: 'tomcat-embed-jasper', version: '+' } From 747f758a708d94987e7bf907867b070995265bf7 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Mon, 15 Jul 2019 12:52:57 -0700 Subject: [PATCH 13/24] Remove default instance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s not very interesting and breaks the definition of “instance” when we want to see the db name when no instance name is defined. --- .../instrumentation/jdbc/JDBCConnectionUrlParser.java | 4 ---- .../src/test/groovy/JDBCConnectionUrlParserTest.groovy | 8 ++++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCConnectionUrlParser.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCConnectionUrlParser.java index 8f5124ad79..a8000ba357 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCConnectionUrlParser.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCConnectionUrlParser.java @@ -295,7 +295,6 @@ public enum JDBCConnectionUrlParser { MSSQLSERVER("microsoft", "sqlserver") { private static final String DEFAULT_HOST = "localhost"; private static final int DEFAULT_PORT = 1433; - private static final String DEFAULT_INSTANCE = "MSSQLSERVER"; @Override DBInfo.Builder doParse(String jdbcUrl, final DBInfo.Builder builder) { @@ -307,9 +306,6 @@ public enum JDBCConnectionUrlParser { } builder.type("sqlserver"); final DBInfo dbInfo = builder.build(); - if (dbInfo.getInstance() == null) { - builder.instance(DEFAULT_INSTANCE); - } if (dbInfo.getHost() == null) { builder.host(DEFAULT_HOST); } diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCConnectionUrlParserTest.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCConnectionUrlParserTest.groovy index e0fb31854a..b3a662ef55 100644 --- a/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCConnectionUrlParserTest.groovy +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCConnectionUrlParserTest.groovy @@ -88,12 +88,12 @@ class JDBCConnectionUrlParserTest extends Specification { "address=(host=anotherhost)(port=3306)(user=wrong)(password=PW)/mdbdb?user=mdbuser&password=PW" | null | "mysql" | "replication" | "mdbuser" | "mdb.host" | 3306 | null | "mdbdb" //https://docs.microsoft.com/en-us/sql/connect/jdbc/building-the-connection-url - "jdbc:microsoft:sqlserver://;" | null | "sqlserver" | null | null | "localhost" | 1433 | "MSSQLSERVER" | null - "jdbc:microsoft:sqlserver://;" | stdProps | "sqlserver" | null | "stdUserName" | "stdServerName" | 9999 | "MSSQLSERVER" | "stdDatabaseName" + "jdbc:microsoft:sqlserver://;" | null | "sqlserver" | null | null | "localhost" | 1433 | null | null + "jdbc:microsoft:sqlserver://;" | stdProps | "sqlserver" | null | "stdUserName" | "stdServerName" | 9999 | null | "stdDatabaseName" "jdbc:sqlserver://ss.host\\ssinstance:44;databaseName=ssdb;user=ssuser;password=pw" | null | "sqlserver" | null | "ssuser" | "ss.host" | 44 | "ssinstance" | "ssdb" "jdbc:sqlserver://;serverName=ss.host\\ssinstance:44;DatabaseName=;" | null | "sqlserver" | null | null | "ss.host" | 44 | "ssinstance" | null - "jdbc:sqlserver://ss.host;serverName=althost;DatabaseName=ssdb;" | null | "sqlserver" | null | null | "ss.host" | 1433 | "MSSQLSERVER" | "ssdb" - "jdbc:microsoft:sqlserver://ss.host:44;DatabaseName=ssdb;user=ssuser;password=pw;user=ssuser2;" | null | "sqlserver" | null | "ssuser" | "ss.host" | 44 | "MSSQLSERVER" | "ssdb" + "jdbc:sqlserver://ss.host;serverName=althost;DatabaseName=ssdb;" | null | "sqlserver" | null | null | "ss.host" | 1433 | null | "ssdb" + "jdbc:microsoft:sqlserver://ss.host:44;DatabaseName=ssdb;user=ssuser;password=pw;user=ssuser2;" | null | "sqlserver" | null | "ssuser" | "ss.host" | 44 | null | "ssdb" // https://docs.oracle.com/cd/B28359_01/java.111/b31224/urls.htm // https://docs.oracle.com/cd/B28359_01/java.111/b31224/jdbcthin.htm From e08e3a787ce53ff27f6155b09dcd4ce3796dbe30 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Mon, 15 Jul 2019 19:11:51 -0700 Subject: [PATCH 14/24] Allow null to be set in Scope for Span This is useful to temporarily remove a trace from scope for a defined period. --- .../java/datadog/opentracing/DDTracer.java | 2 +- .../scopemanager/ContinuableScope.java | 2 +- .../opentracing/scopemanager/SimpleScope.java | 4 +-- .../opentracing/DDSpanBuilderTest.groovy | 28 +++++++++++++++++++ .../scopemanager/ScopeManagerTest.groovy | 22 +++++++++------ 5 files changed, 45 insertions(+), 13 deletions(-) diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java index 74bf6129e7..487f53be66 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java @@ -619,7 +619,7 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace if (parentContext == null && !ignoreScope) { // use the Scope as parent unless overridden or ignored. final Scope scope = scopeManager.active(); - if (scope != null) { + if (scope != null && scope.span() != null) { parentContext = scope.span().context(); } } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContinuableScope.java b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContinuableScope.java index a40126b343..53a788dd9e 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContinuableScope.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContinuableScope.java @@ -48,7 +48,7 @@ public class ContinuableScope implements Scope, TraceScope { this.openCount = openCount; this.continuation = continuation; this.spanUnderScope = spanUnderScope; - this.finishOnClose = finishOnClose; + this.finishOnClose = finishOnClose && spanUnderScope != null; toRestore = scopeManager.tlsScope.get(); scopeManager.tlsScope.set(this); for (final ScopeListener listener : scopeManager.scopeListeners) { diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/SimpleScope.java b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/SimpleScope.java index fc43e92fe8..d11194a9c9 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/SimpleScope.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/SimpleScope.java @@ -17,8 +17,8 @@ public class SimpleScope implements Scope { final boolean finishOnClose) { this.scopeManager = scopeManager; this.spanUnderScope = spanUnderScope; - this.finishOnClose = finishOnClose; - this.toRestore = scopeManager.tlsScope.get(); + this.finishOnClose = finishOnClose && spanUnderScope != null; + toRestore = scopeManager.tlsScope.get(); scopeManager.tlsScope.set(this); for (final ScopeListener listener : scopeManager.scopeListeners) { listener.afterScopeActivated(); diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy index f158c3845b..e7a6da24e2 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy @@ -6,6 +6,7 @@ import datadog.trace.api.Config import datadog.trace.api.DDTags import datadog.trace.api.sampling.PrioritySampling import datadog.trace.common.writer.ListWriter +import io.opentracing.Scope import spock.lang.Specification import static datadog.opentracing.DDSpanContext.ORIGIN_KEY @@ -173,6 +174,33 @@ class DDSpanBuilderTest extends Specification { actualContext.getTraceId() == spanId } + def "should link to parent span implicitly"() { + setup: + final Scope parent = nullParent ? + tracer.scopeManager().activate(null, false) : + tracer.buildSpan("parent") + .startActive(false) + + final String expectedParentId = nullParent ? "0" : parent.span().context().getSpanId() + + final String expectedName = "fakeName" + + final DDSpan span = tracer + .buildSpan(expectedName) + .start() + + final DDSpanContext actualContext = span.context() + + expect: + actualContext.getParentId() == expectedParentId + + cleanup: + parent.close() + + where: + nullParent << [false, true] + } + def "should inherit the DD parent attributes"() { setup: def expectedName = "fakeName" diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/scopemanager/ScopeManagerTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/scopemanager/ScopeManagerTest.groovy index 4ff52ea087..54afaade40 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/scopemanager/ScopeManagerTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/scopemanager/ScopeManagerTest.groovy @@ -96,19 +96,19 @@ class ScopeManagerTest extends Specification { def "sets parent as current upon close"() { setup: def parentScope = tracer.buildSpan("parent").startActive(finishSpan) - def childScope = tracer.buildSpan("parent").startActive(finishSpan) + def childScope = nullChild ? tracer.scopeManager().activate(null, finishSpan) : tracer.buildSpan("parent").startActive(finishSpan) expect: scopeManager.active() == childScope - childScope.span().context().parentId == parentScope.span().context().spanId - childScope.span().context().trace == parentScope.span().context().trace + nullChild || childScope.span().context().parentId == parentScope.span().context().spanId + nullChild || childScope.span().context().trace == parentScope.span().context().trace when: childScope.close() then: scopeManager.active() == parentScope - spanFinished(childScope.span()) == finishSpan + spanFinished(childScope.span()) == (nullChild ? null : finishSpan) !spanFinished(parentScope.span()) writer == [] @@ -116,13 +116,17 @@ class ScopeManagerTest extends Specification { parentScope.close() then: - spanFinished(childScope.span()) == finishSpan + spanFinished(childScope.span()) == (nullChild ? null : finishSpan) spanFinished(parentScope.span()) == finishSpan - writer == [[parentScope.span(), childScope.span()]] || !finishSpan + writer == [[parentScope.span(), childScope.span()]] || !finishSpan || nullChild scopeManager.active() == null where: - finishSpan << [true, false] + finishSpan | nullChild + true | false + false | false + true | true + false | true } def "ContinuableScope only creates continuations when propagation is set"() { @@ -567,8 +571,8 @@ class ScopeManagerTest extends Specification { closedCount.get() == 4 } - boolean spanFinished(Span span) { - return ((DDSpan) span).isFinished() + Boolean spanFinished(Span span) { + return ((DDSpan) span)?.isFinished() } class AtomicReferenceScope extends AtomicReference implements ScopeContext, Scope { From 36487a045654959e115a89973e5a8edab636d551 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Tue, 16 Jul 2019 10:33:07 -0400 Subject: [PATCH 15/24] Remove version from gradle wrapper definition It doesn't look like it is doing anything useful but instead it seems to set version in resulting build jar which is not expected. --- dd-trace-java.gradle | 1 - 1 file changed, 1 deletion(-) diff --git a/dd-trace-java.gradle b/dd-trace-java.gradle index 0de18a9544..16a39866f7 100644 --- a/dd-trace-java.gradle +++ b/dd-trace-java.gradle @@ -52,7 +52,6 @@ buildScan { wrapper { distributionType = Wrapper.DistributionType.ALL - version = '5.5.1' } allprojects { From 369658c2008e19cba5bcf8613ca3796466b17414 Mon Sep 17 00:00:00 2001 From: Laplie Anderson Date: Tue, 16 Jul 2019 10:53:07 -0400 Subject: [PATCH 16/24] Initial cgroups parsing implementation --- .../datadog/opentracing/ContainerInfo.java | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 dd-trace-ot/src/main/java/datadog/opentracing/ContainerInfo.java diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/ContainerInfo.java b/dd-trace-ot/src/main/java/datadog/opentracing/ContainerInfo.java new file mode 100644 index 0000000000..09c848edfc --- /dev/null +++ b/dd-trace-ot/src/main/java/datadog/opentracing/ContainerInfo.java @@ -0,0 +1,102 @@ +package datadog.opentracing; + +import lombok.Data; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.text.ParseException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +@Data +public class ContainerInfo { + private static final Path CGROUP_DEFAULT_PROCFILE = Paths.get("/proc/self/cgroup"); + private static final String UUID_REGEX = + "[0-9a-f]{8}[-_][0-9a-f]{4}[-_][0-9a-f]{4}[-_][0-9a-f]{4}[-_][0-9a-f]{12}"; + private static final String CONTAINER_REGEX = "[0-9a-f]{64}"; + private static final Pattern LINE_PATTERN = Pattern.compile("(\\d+):([^:]*):(.+)$"); + private static final Pattern POD_PATTERN = Pattern.compile("pod(" + UUID_REGEX + ")(?:.slice)?$"); + private static final Pattern CONTAINER_PATTERN = + Pattern.compile("(" + UUID_REGEX + "|" + CONTAINER_REGEX + ")(?:.scope)?$"); + + public String containerId; + public String podId; + public List cGroups; + + @Data + public static class CGroupInfo { + public int id; + public String path; + public List controllers; + public String containerId; + public String podId; + } + + public static boolean isRunningInContainer() { + return Files.isReadable(CGROUP_DEFAULT_PROCFILE); + } + + public static ContainerInfo fromDefaultProcFile() throws IOException, ParseException { + final String content = new String(Files.readAllBytes(CGROUP_DEFAULT_PROCFILE)); + return parse(content); + } + + public static ContainerInfo parse(final String cgroupsContent) throws ParseException { + final ContainerInfo containerInfo = new ContainerInfo(); + + final String[] lines = cgroupsContent.split("\n"); + final List parsedCGroups = new ArrayList<>(); + for (final String line : lines) { + final CGroupInfo cGroupInfo = parseLine(line); + + parsedCGroups.add(cGroupInfo); + + if (cGroupInfo.getPodId() != null) { + containerInfo.setPodId(cGroupInfo.getPodId()); + } + + if (cGroupInfo.getContainerId() != null) { + containerInfo.setContainerId(cGroupInfo.getContainerId()); + } + } + + containerInfo.setCGroups(parsedCGroups); + + return containerInfo; + } + + static CGroupInfo parseLine(final String line) throws ParseException { + final Matcher matcher = LINE_PATTERN.matcher(line); + + if (!matcher.matches()) { + throw new ParseException("Unable to match cgroup", 0); + } + + final CGroupInfo cGroupInfo = new CGroupInfo(); + cGroupInfo.setId(Integer.parseInt(matcher.group(1))); + cGroupInfo.setControllers(Arrays.asList(matcher.group(2).split(","))); + + final String path = matcher.group(3); + final String[] pathParts = path.split("/"); + + cGroupInfo.setPath(path); + + if (pathParts.length >= 1) { + final Matcher containerIdMatcher = CONTAINER_PATTERN.matcher(pathParts[0]); + final String containerId = containerIdMatcher.matches() ? containerIdMatcher.group(1) : null; + cGroupInfo.setContainerId(containerId); + } + + if (pathParts.length >= 2) { + final Matcher podIdMatcher = POD_PATTERN.matcher(pathParts[1]); + final String podId = podIdMatcher.matches() ? podIdMatcher.group(1) : null; + cGroupInfo.setPodId(podId); + } + + return cGroupInfo; + } +} From d74ffc3db02f83b14e9b1acbc61716ab1ed68335 Mon Sep 17 00:00:00 2001 From: Laplie Anderson Date: Tue, 16 Jul 2019 11:44:35 -0400 Subject: [PATCH 17/24] Parsing tests from RFC and reference impl --- .../opentracing/ContainerInfoTest.groovy | 148 ++++++++++++++++++ 1 file changed, 148 insertions(+) create mode 100644 dd-trace-ot/src/test/groovy/datadog/opentracing/ContainerInfoTest.groovy diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/ContainerInfoTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/ContainerInfoTest.groovy new file mode 100644 index 0000000000..e5d014e594 --- /dev/null +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/ContainerInfoTest.groovy @@ -0,0 +1,148 @@ +package datadog.opentracing + +import spock.lang.Specification +import spock.lang.Unroll + + +class ContainerInfoTest extends Specification { + + @Unroll + def "CGroupInfo is parsed from individual lines"() { + when: + ContainerInfo.CGroupInfo cGroupInfo = ContainerInfo.parseLine(line) + + then: + cGroupInfo.getId() == id + cGroupInfo.getPath() == path + cGroupInfo.getControllers() == controllers + cGroupInfo.getContainerId() == containerId + cGroupInfo.podId == podId + + // Examples from DataDog/architecture/rfcs/apm/agent/containers-tagging/rfc.md and Qard/container-info + where: + id | controllers | path | containerId | podId | line + + // Docker examples + 13 | ["name=systemd"] | "/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | "3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | null | "13:name=systemd:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" + 12 | ["pids"] | "/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | "3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | null | "12:pids:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" + 11 | ["hugetlb"] | "/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | "3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | null | "11:hugetlb:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" + 10 | ["net_prio"] | "/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | "3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | null | "10:net_prio:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" + 9 | ["perf_event"] | "/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | "3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | null | "9:perf_event:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" + 8 | ["net_cls"] | "/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | "3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | null | "8:net_cls:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" + 7 | ["freezer"] | "/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | "3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | null | "7:freezer:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" + 6 | ["devices"] | "/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | "3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | null | "6:devices:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" + 5 | ["memory"] | "/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | "3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | null | "5:memory:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" + 4 | ["blkio"] | "/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | "3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | null | "4:blkio:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" + 3 | ["cpuacct"] | "/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | "3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | null | "3:cpuacct:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" + 2 | ["cpu"] | "/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | "3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | null | "2:cpu:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" + 1 | ["cpuset"] | "/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | "3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | null | "1:cpuset:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" + + // Kubernates examples + 11 | ["perf_event"] | "/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3d274242-8ee0-11e9-a8a6-1e68d864ef1a" | "11:perf_event:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" + 10 | ["pids"] | "/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3d274242-8ee0-11e9-a8a6-1e68d864ef1a" | "10:pids:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" + 9 | ["memory"] | "/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3d274242-8ee0-11e9-a8a6-1e68d864ef1a" | "9:memory:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" + 8 | ["cpu", "cpuacct"] | "/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3d274242-8ee0-11e9-a8a6-1e68d864ef1a" | "8:cpu,cpuacct:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" + 7 | ["blkio"] | "/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3d274242-8ee0-11e9-a8a6-1e68d864ef1a" | "7:blkio:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" + 6 | ["cpuset"] | "/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3d274242-8ee0-11e9-a8a6-1e68d864ef1a" | "6:cpuset:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" + 5 | ["devices"] | "/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3d274242-8ee0-11e9-a8a6-1e68d864ef1a" | "5:devices:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" + 4 | ["freezer"] | "/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3d274242-8ee0-11e9-a8a6-1e68d864ef1a" | "4:freezer:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" + 3 | ["net_cls", "net_prop"] | "/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3d274242-8ee0-11e9-a8a6-1e68d864ef1a" | "3:net_cls,net_prio:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" + 2 | ["hugetlb"] | "/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3d274242-8ee0-11e9-a8a6-1e68d864ef1a" | "2:hugetlb:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" + 1 | ["name=systemd"] | "/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3d274242-8ee0-11e9-a8a6-1e68d864ef1a" | "1:name=systemd:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" + + //ECS examples + 9 | ["perf_event"] | "/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" | "38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" | null | "9:perf_event:/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" + 8 | ["memory"] | "/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" | "38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" | null | "8:memory:/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" + 7 | ["hugetlb"] | "/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" | "38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" | null | "7:hugetlb:/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" + 6 | ["freezer"] | "/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" | "38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" | null | "6:freezer:/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" + 5 | ["devices"] | "/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" | "38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" | null | "5:devices:/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" + 4 | ["cpuset"] | "/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" | "38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" | null | "4:cpuset:/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" + 3 | ["cpuacct"] | "/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" | "38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" | null | "3:cpuacct:/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" + 2 | ["cpu"] | "/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" | "38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" | null | "2:cpu:/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" + 1 | ["blkio"] | "/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" | "38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" | null | "1:blkio:/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" + + //Fargate Examples + 11 | ["hugetlb"] | "/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" | "432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" | null | "11:hugetlb:/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" + 10 | ["pids"] | "/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" | "432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" | null | "10:pids:/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" + 9 | ["cpuset"] | "/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" | "432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" | null | "9:cpuset:/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" + 8 | ["net_cls", "net_prio"] | "/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" | "432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" | null | "8:net_cls,net_prio:/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" + 7 | ["cpu", "cpuacct"] | "/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" | "432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" | null | "7:cpu,cpuacct:/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" + 6 | ["perf_event"] | "/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" | "432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" | null | "6:perf_event:/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" + 5 | ["freezer"] | "/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" | "432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" | null | "5:freezer:/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" + 4 | ["devices"] | "/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" | "432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" | null | "4:devices:/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" + 3 | ["blkio"] | "/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" | "432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" | null | "3:blkio:/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" + 2 | ["memory"] | "/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" | "432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" | null | "2:memory:/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" + 1 | ["name=systemd"] | "/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" | "432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" | null | "1:name=systemd:/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" + + //Reference impl examples + 1 | ["name=systemd"] | "/system.slice/docker-cde7c2bab394630a42d73dc610b9c57415dced996106665d427f6d0566594411.scope" | "cde7c2bab394630a42d73dc610b9c57415dced996106665d427f6d0566594411" | null | "1:name=systemd:/system.slice/docker-cde7c2bab394630a42d73dc610b9c57415dced996106665d427f6d0566594411.scope" + 1 | ["name=systemd"] | "/docker/051e2ee0bce99116029a13df4a9e943137f19f957f38ac02d6bad96f9b700f76/not_hex" | null | null | "1:name=systemd:/docker/051e2ee0bce99116029a13df4a9e943137f19f957f38ac02d6bad96f9b700f76/not_hex" + 1 | ["name=systemd"] | "/kubepods.slice/kubepods-burstable.slice/kubepods-burstable-pod90d81341_92de_11e7_8cf2_507b9d4141fa.slice/crio-2227daf62df6694645fee5df53c1f91271546a9560e8600a525690ae252b7f63.scope" | "2227daf62df6694645fee5df53c1f91271546a9560e8600a525690ae252b7f63" | "90d81341_92de_11e7_8cf2_507b9d4141fa" | "1:name=systemd:/kubepods.slice/kubepods-burstable.slice/kubepods-burstable-pod90d81341_92de_11e7_8cf2_507b9d4141fa.slice/crio-2227daf62df6694645fee5df53c1f91271546a9560e8600a525690ae252b7f63.scope" + + } + + @Unroll + def "Container info parsed from file content"() { + when: + ContainerInfo containerInfo = ContainerInfo.parse(content) + + then: + containerInfo.getContainerId() == containerId + containerInfo.getPodId() == podId + containerInfo.getCGroups().size() == size + + where: + containerId | podId | size | content + // Docker + "3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860" | null | 13 | """13:name=systemd:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860 +12:pids:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860 +11:hugetlb:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860 +10:net_prio:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860 +9:perf_event:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860 +8:net_cls:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860 +7:freezer:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860 +6:devices:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860 +5:memory:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860 +4:blkio:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860 +3:cpuacct:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860 +2:cpu:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860 +1:cpuset:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860""" + + // Kubernetes + "3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3d274242-8ee0-11e9-a8a6-1e68d864ef1a" | 11 | """11:perf_event:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1 +10:pids:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1 +9:memory:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1 +8:cpu,cpuacct:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1 +7:blkio:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1 +6:cpuset:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1 +5:devices:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1 +4:freezer:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1 +3:net_cls,net_prio:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1 +2:hugetlb:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1 +1:name=systemd:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1""" + + // ECS + "38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce" | null | 9 | """9:perf_event:/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce +8:memory:/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce +7:hugetlb:/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce +6:freezer:/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce +5:devices:/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce +4:cpuset:/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce +3:cpuacct:/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce +2:cpu:/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce +1:blkio:/ecs/haissam-ecs-classic/5a0d5ceddf6c44c1928d367a815d890f/38fac3e99302b3622be089dd41e7ccf38aff368a86cc339972075136ee2710ce""" + + // Fargate + "432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da" | null | 11 | """11:hugetlb:/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da +10:pids:/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da +9:cpuset:/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da +8:net_cls,net_prio:/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da +7:cpu,cpuacct:/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da +6:perf_event:/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da +5:freezer:/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da +4:devices:/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da +3:blkio:/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da +2:memory:/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da +1:name=systemd:/ecs/55091c13-b8cf-4801-b527-f4601742204d/432624d2150b349fe35ba397284dea788c2bf66b885d14dfc1569b01890ca7da""" + } +} From 9dbe3a0a17b19b34301cab28172ddbd89e186641 Mon Sep 17 00:00:00 2001 From: Laplie Anderson Date: Tue, 16 Jul 2019 15:31:15 -0400 Subject: [PATCH 18/24] Fix part of path to match --- .../src/main/java/datadog/opentracing/ContainerInfo.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/ContainerInfo.java b/dd-trace-ot/src/main/java/datadog/opentracing/ContainerInfo.java index 09c848edfc..2cbb7d2ace 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/ContainerInfo.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/ContainerInfo.java @@ -12,6 +12,10 @@ import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; +/** + * Parses container information from /proc/self/cgroup. Implementation based largely on + * Qard/container-info + */ @Data public class ContainerInfo { private static final Path CGROUP_DEFAULT_PROCFILE = Paths.get("/proc/self/cgroup"); @@ -86,13 +90,13 @@ public class ContainerInfo { cGroupInfo.setPath(path); if (pathParts.length >= 1) { - final Matcher containerIdMatcher = CONTAINER_PATTERN.matcher(pathParts[0]); + final Matcher containerIdMatcher = CONTAINER_PATTERN.matcher(pathParts[pathParts.length - 1]); final String containerId = containerIdMatcher.matches() ? containerIdMatcher.group(1) : null; cGroupInfo.setContainerId(containerId); } if (pathParts.length >= 2) { - final Matcher podIdMatcher = POD_PATTERN.matcher(pathParts[1]); + final Matcher podIdMatcher = POD_PATTERN.matcher(pathParts[pathParts.length - 2]); final String podId = podIdMatcher.matches() ? podIdMatcher.group(1) : null; cGroupInfo.setPodId(podId); } From 48538018133b675e829b8492219c84493cb49a6f Mon Sep 17 00:00:00 2001 From: Laplie Anderson Date: Tue, 16 Jul 2019 17:21:59 -0400 Subject: [PATCH 19/24] Add container id headers to request builder --- .../datadog/opentracing/ContainerInfo.java | 7 +++--- .../datadog/opentracing/DDTraceOTInfo.java | 16 ++++++++++++++ .../datadog/trace/common/writer/DDApi.java | 22 +++++++++++++------ .../opentracing/ContainerInfoTest.groovy | 2 +- 4 files changed, 36 insertions(+), 11 deletions(-) diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/ContainerInfo.java b/dd-trace-ot/src/main/java/datadog/opentracing/ContainerInfo.java index 2cbb7d2ace..5ec79a0b24 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/ContainerInfo.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/ContainerInfo.java @@ -1,6 +1,5 @@ package datadog.opentracing; -import lombok.Data; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -11,6 +10,7 @@ import java.util.Arrays; import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; +import lombok.Data; /** * Parses container information from /proc/self/cgroup. Implementation based largely on @@ -23,9 +23,10 @@ public class ContainerInfo { "[0-9a-f]{8}[-_][0-9a-f]{4}[-_][0-9a-f]{4}[-_][0-9a-f]{4}[-_][0-9a-f]{12}"; private static final String CONTAINER_REGEX = "[0-9a-f]{64}"; private static final Pattern LINE_PATTERN = Pattern.compile("(\\d+):([^:]*):(.+)$"); - private static final Pattern POD_PATTERN = Pattern.compile("pod(" + UUID_REGEX + ")(?:.slice)?$"); + private static final Pattern POD_PATTERN = + Pattern.compile("(?:.+)?pod(" + UUID_REGEX + ")(?:.slice)?$"); private static final Pattern CONTAINER_PATTERN = - Pattern.compile("(" + UUID_REGEX + "|" + CONTAINER_REGEX + ")(?:.scope)?$"); + Pattern.compile("(?:.+)?(" + UUID_REGEX + "|" + CONTAINER_REGEX + ")(?:.scope)?$"); public String containerId; public String podId; diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DDTraceOTInfo.java b/dd-trace-ot/src/main/java/datadog/opentracing/DDTraceOTInfo.java index 28840ad8e1..20ce98c057 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDTraceOTInfo.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDTraceOTInfo.java @@ -1,7 +1,9 @@ package datadog.opentracing; import java.io.BufferedReader; +import java.io.IOException; import java.io.InputStreamReader; +import java.text.ParseException; import lombok.extern.slf4j.Slf4j; @Slf4j @@ -13,6 +15,8 @@ public class DDTraceOTInfo { public static final String VERSION; + public static final String CONTAINER_ID; + static { String v; try { @@ -30,6 +34,18 @@ public class DDTraceOTInfo { } VERSION = v; log.info("dd-trace - version: {}", v); + + ContainerInfo containerInfo = null; + + if (ContainerInfo.isRunningInContainer()) { + try { + containerInfo = ContainerInfo.fromDefaultProcFile(); + } catch (final IOException | ParseException e) { + log.error("Unable to parse proc file"); + } + } + + CONTAINER_ID = containerInfo == null ? null : containerInfo.getContainerId(); } public static void main(final String... args) { diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDApi.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDApi.java index 8fa1bd170a..d81ac79a2a 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDApi.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDApi.java @@ -35,6 +35,7 @@ public class DDApi { private static final String DATADOG_META_LANG_INTERPRETER_VENDOR = "Datadog-Meta-Lang-Interpreter-Vendor"; private static final String DATADOG_META_TRACER_VERSION = "Datadog-Meta-Tracer-Version"; + private static final String DATADOG_CONTAINER_ID = "Datadog-Container-ID"; private static final String X_DATADOG_TRACE_COUNT = "X-Datadog-Trace-Count"; private static final int HTTP_TIMEOUT = 1; // 1 second for conenct/read/write operations @@ -261,13 +262,20 @@ public class DDApi { } private static Request.Builder prepareRequest(final HttpUrl url) { - return new Request.Builder() - .url(url) - .addHeader(DATADOG_META_LANG, "java") - .addHeader(DATADOG_META_LANG_VERSION, DDTraceOTInfo.JAVA_VERSION) - .addHeader(DATADOG_META_LANG_INTERPRETER, DDTraceOTInfo.JAVA_VM_NAME) - .addHeader(DATADOG_META_LANG_INTERPRETER_VENDOR, DDTraceOTInfo.JAVA_VM_VENDOR) - .addHeader(DATADOG_META_TRACER_VERSION, DDTraceOTInfo.VERSION); + final Request.Builder builder = + new Request.Builder() + .url(url) + .addHeader(DATADOG_META_LANG, "java") + .addHeader(DATADOG_META_LANG_VERSION, DDTraceOTInfo.JAVA_VERSION) + .addHeader(DATADOG_META_LANG_INTERPRETER, DDTraceOTInfo.JAVA_VM_NAME) + .addHeader(DATADOG_META_LANG_INTERPRETER_VENDOR, DDTraceOTInfo.JAVA_VM_VENDOR) + .addHeader(DATADOG_META_TRACER_VERSION, DDTraceOTInfo.VERSION); + + if (DDTraceOTInfo.CONTAINER_ID == null) { + return builder; + } else { + return builder.addHeader(DATADOG_CONTAINER_ID, DDTraceOTInfo.CONTAINER_ID); + } } @Override diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/ContainerInfoTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/ContainerInfoTest.groovy index e5d014e594..61190f5bc3 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/ContainerInfoTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/ContainerInfoTest.groovy @@ -46,7 +46,7 @@ class ContainerInfoTest extends Specification { 6 | ["cpuset"] | "/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3d274242-8ee0-11e9-a8a6-1e68d864ef1a" | "6:cpuset:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" 5 | ["devices"] | "/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3d274242-8ee0-11e9-a8a6-1e68d864ef1a" | "5:devices:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" 4 | ["freezer"] | "/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3d274242-8ee0-11e9-a8a6-1e68d864ef1a" | "4:freezer:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" - 3 | ["net_cls", "net_prop"] | "/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3d274242-8ee0-11e9-a8a6-1e68d864ef1a" | "3:net_cls,net_prio:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" + 3 | ["net_cls", "net_prio"] | "/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3d274242-8ee0-11e9-a8a6-1e68d864ef1a" | "3:net_cls,net_prio:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" 2 | ["hugetlb"] | "/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3d274242-8ee0-11e9-a8a6-1e68d864ef1a" | "2:hugetlb:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" 1 | ["name=systemd"] | "/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" | "3d274242-8ee0-11e9-a8a6-1e68d864ef1a" | "1:name=systemd:/kubepods/besteffort/pod3d274242-8ee0-11e9-a8a6-1e68d864ef1a/3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" From d03a2a95e2f4ba0a457b022451346b9bea736e7e Mon Sep 17 00:00:00 2001 From: Laplie Anderson Date: Tue, 16 Jul 2019 19:09:00 -0400 Subject: [PATCH 20/24] Change to @Getter/@Setter to be above coverage threshold Since toString(), equals(), and hashCode() are never called, the code coverage for the classes are too low. In the future, we should ignore code coverage of lombok generated methods --- .../src/main/java/datadog/opentracing/ContainerInfo.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/ContainerInfo.java b/dd-trace-ot/src/main/java/datadog/opentracing/ContainerInfo.java index 5ec79a0b24..47c0b9894a 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/ContainerInfo.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/ContainerInfo.java @@ -10,13 +10,15 @@ import java.util.Arrays; import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; -import lombok.Data; +import lombok.Getter; +import lombok.Setter; /** * Parses container information from /proc/self/cgroup. Implementation based largely on * Qard/container-info */ -@Data +@Getter +@Setter public class ContainerInfo { private static final Path CGROUP_DEFAULT_PROCFILE = Paths.get("/proc/self/cgroup"); private static final String UUID_REGEX = @@ -32,7 +34,8 @@ public class ContainerInfo { public String podId; public List cGroups; - @Data + @Getter + @Setter public static class CGroupInfo { public int id; public String path; From 719bf0ad76b08dd7980d861792629aeb8e6a0637 Mon Sep 17 00:00:00 2001 From: Laplie Anderson Date: Wed, 17 Jul 2019 12:00:31 -0400 Subject: [PATCH 21/24] Remove reference to private repo --- .../test/groovy/datadog/opentracing/ContainerInfoTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/ContainerInfoTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/ContainerInfoTest.groovy index 61190f5bc3..051ab2c9c1 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/ContainerInfoTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/ContainerInfoTest.groovy @@ -18,7 +18,7 @@ class ContainerInfoTest extends Specification { cGroupInfo.getContainerId() == containerId cGroupInfo.podId == podId - // Examples from DataDog/architecture/rfcs/apm/agent/containers-tagging/rfc.md and Qard/container-info + // Examples from container tagging rfc and Qard/container-info where: id | controllers | path | containerId | podId | line From 5956f51e07a2d9d44a174542e46456d361121e6d Mon Sep 17 00:00:00 2001 From: Laplie Anderson Date: Wed, 17 Jul 2019 13:01:08 -0400 Subject: [PATCH 22/24] Change container info to a singleton ContainerInfo DDTraceOTInfo wasn't the best place for container information. Changed ContainerInfo.java to have a static instance --- .../datadog/opentracing/ContainerInfo.java | 28 +++++++++++++++---- .../datadog/opentracing/DDTraceOTInfo.java | 16 ----------- .../datadog/trace/common/writer/DDApi.java | 6 ++-- 3 files changed, 27 insertions(+), 23 deletions(-) diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/ContainerInfo.java b/dd-trace-ot/src/main/java/datadog/opentracing/ContainerInfo.java index 47c0b9894a..455bb34969 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/ContainerInfo.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/ContainerInfo.java @@ -12,6 +12,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import lombok.Getter; import lombok.Setter; +import lombok.extern.slf4j.Slf4j; /** * Parses container information from /proc/self/cgroup. Implementation based largely on @@ -19,6 +20,7 @@ import lombok.Setter; */ @Getter @Setter +@Slf4j public class ContainerInfo { private static final Path CGROUP_DEFAULT_PROCFILE = Paths.get("/proc/self/cgroup"); private static final String UUID_REGEX = @@ -30,9 +32,24 @@ public class ContainerInfo { private static final Pattern CONTAINER_PATTERN = Pattern.compile("(?:.+)?(" + UUID_REGEX + "|" + CONTAINER_REGEX + ")(?:.scope)?$"); + private static final ContainerInfo INSTANCE; + public String containerId; public String podId; - public List cGroups; + public List cGroups = new ArrayList<>(); + + static { + ContainerInfo containerInfo = new ContainerInfo(); + if (ContainerInfo.isRunningInContainer()) { + try { + containerInfo = ContainerInfo.fromDefaultProcFile(); + } catch (final IOException | ParseException e) { + log.error("Unable to parse proc file"); + } + } + + INSTANCE = containerInfo; + } @Getter @Setter @@ -44,6 +61,10 @@ public class ContainerInfo { public String podId; } + public static ContainerInfo get() { + return INSTANCE; + } + public static boolean isRunningInContainer() { return Files.isReadable(CGROUP_DEFAULT_PROCFILE); } @@ -57,11 +78,10 @@ public class ContainerInfo { final ContainerInfo containerInfo = new ContainerInfo(); final String[] lines = cgroupsContent.split("\n"); - final List parsedCGroups = new ArrayList<>(); for (final String line : lines) { final CGroupInfo cGroupInfo = parseLine(line); - parsedCGroups.add(cGroupInfo); + containerInfo.getCGroups().add(cGroupInfo); if (cGroupInfo.getPodId() != null) { containerInfo.setPodId(cGroupInfo.getPodId()); @@ -72,8 +92,6 @@ public class ContainerInfo { } } - containerInfo.setCGroups(parsedCGroups); - return containerInfo; } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DDTraceOTInfo.java b/dd-trace-ot/src/main/java/datadog/opentracing/DDTraceOTInfo.java index 20ce98c057..28840ad8e1 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDTraceOTInfo.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDTraceOTInfo.java @@ -1,9 +1,7 @@ package datadog.opentracing; import java.io.BufferedReader; -import java.io.IOException; import java.io.InputStreamReader; -import java.text.ParseException; import lombok.extern.slf4j.Slf4j; @Slf4j @@ -15,8 +13,6 @@ public class DDTraceOTInfo { public static final String VERSION; - public static final String CONTAINER_ID; - static { String v; try { @@ -34,18 +30,6 @@ public class DDTraceOTInfo { } VERSION = v; log.info("dd-trace - version: {}", v); - - ContainerInfo containerInfo = null; - - if (ContainerInfo.isRunningInContainer()) { - try { - containerInfo = ContainerInfo.fromDefaultProcFile(); - } catch (final IOException | ParseException e) { - log.error("Unable to parse proc file"); - } - } - - CONTAINER_ID = containerInfo == null ? null : containerInfo.getContainerId(); } public static void main(final String... args) { diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDApi.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDApi.java index d81ac79a2a..e3acc6ed5b 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDApi.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDApi.java @@ -4,6 +4,7 @@ import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import datadog.opentracing.ContainerInfo; import datadog.opentracing.DDSpan; import datadog.opentracing.DDTraceOTInfo; import datadog.trace.common.writer.unixdomainsockets.UnixDomainSocketFactory; @@ -271,10 +272,11 @@ public class DDApi { .addHeader(DATADOG_META_LANG_INTERPRETER_VENDOR, DDTraceOTInfo.JAVA_VM_VENDOR) .addHeader(DATADOG_META_TRACER_VERSION, DDTraceOTInfo.VERSION); - if (DDTraceOTInfo.CONTAINER_ID == null) { + final String containerId = ContainerInfo.get().getContainerId(); + if (containerId == null) { return builder; } else { - return builder.addHeader(DATADOG_CONTAINER_ID, DDTraceOTInfo.CONTAINER_ID); + return builder.addHeader(DATADOG_CONTAINER_ID, containerId); } } From ab7786d352b6bee1df49981bcf84934559a35f95 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Wed, 17 Jul 2019 16:58:29 -0400 Subject: [PATCH 23/24] Minor javadoc fix --- .../java/datadog/trace/agent/tooling/ClassLoaderMatcher.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ClassLoaderMatcher.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ClassLoaderMatcher.java index 59c05548f9..d8275b233a 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ClassLoaderMatcher.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ClassLoaderMatcher.java @@ -95,7 +95,7 @@ public class ClassLoaderMatcher { /** * TODO: this turns out to be useless with OSGi: {@code - * }org.eclipse.osgi.internal.loader.BundleLoader#isRequestFromVM} returns {@code true} when + * org.eclipse.osgi.internal.loader.BundleLoader#isRequestFromVM} returns {@code true} when * class loading is issued from this check and {@code false} for 'real' class loads. We should * come up with some sort of hack to avoid this problem. */ From 17d26a49346cd3c64ea0025d681f591e5aead2c5 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Thu, 18 Jul 2019 12:38:49 -0700 Subject: [PATCH 24/24] Assert span not null and use NoopSpan instead. --- .../main/java/datadog/opentracing/DDTracer.java | 2 +- .../scopemanager/ContinuableScope.java | 3 ++- .../opentracing/scopemanager/SimpleScope.java | 3 ++- .../datadog/opentracing/DDSpanBuilderTest.groovy | 9 +++++---- .../scopemanager/ScopeManagerTest.groovy | 16 ++++++++-------- 5 files changed, 18 insertions(+), 15 deletions(-) diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java index 487f53be66..74bf6129e7 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java @@ -619,7 +619,7 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace if (parentContext == null && !ignoreScope) { // use the Scope as parent unless overridden or ignored. final Scope scope = scopeManager.active(); - if (scope != null && scope.span() != null) { + if (scope != null) { parentContext = scope.span().context(); } } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContinuableScope.java b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContinuableScope.java index 53a788dd9e..049a14ad77 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContinuableScope.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContinuableScope.java @@ -44,11 +44,12 @@ public class ContinuableScope implements Scope, TraceScope { final Continuation continuation, final DDSpan spanUnderScope, final boolean finishOnClose) { + assert spanUnderScope != null : "span must not be null"; this.scopeManager = scopeManager; this.openCount = openCount; this.continuation = continuation; this.spanUnderScope = spanUnderScope; - this.finishOnClose = finishOnClose && spanUnderScope != null; + this.finishOnClose = finishOnClose; toRestore = scopeManager.tlsScope.get(); scopeManager.tlsScope.set(this); for (final ScopeListener listener : scopeManager.scopeListeners) { diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/SimpleScope.java b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/SimpleScope.java index d11194a9c9..3f97c8a2d7 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/SimpleScope.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/SimpleScope.java @@ -15,9 +15,10 @@ public class SimpleScope implements Scope { final ContextualScopeManager scopeManager, final Span spanUnderScope, final boolean finishOnClose) { + assert spanUnderScope != null : "span must not be null"; this.scopeManager = scopeManager; this.spanUnderScope = spanUnderScope; - this.finishOnClose = finishOnClose && spanUnderScope != null; + this.finishOnClose = finishOnClose; toRestore = scopeManager.tlsScope.get(); scopeManager.tlsScope.set(this); for (final ScopeListener listener : scopeManager.scopeListeners) { diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy index e7a6da24e2..86452c433d 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy @@ -7,6 +7,7 @@ import datadog.trace.api.DDTags import datadog.trace.api.sampling.PrioritySampling import datadog.trace.common.writer.ListWriter import io.opentracing.Scope +import io.opentracing.noop.NoopSpan import spock.lang.Specification import static datadog.opentracing.DDSpanContext.ORIGIN_KEY @@ -176,12 +177,12 @@ class DDSpanBuilderTest extends Specification { def "should link to parent span implicitly"() { setup: - final Scope parent = nullParent ? - tracer.scopeManager().activate(null, false) : + final Scope parent = noopParent ? + tracer.scopeManager().activate(NoopSpan.INSTANCE, false) : tracer.buildSpan("parent") .startActive(false) - final String expectedParentId = nullParent ? "0" : parent.span().context().getSpanId() + final String expectedParentId = noopParent ? "0" : parent.span().context().getSpanId() final String expectedName = "fakeName" @@ -198,7 +199,7 @@ class DDSpanBuilderTest extends Specification { parent.close() where: - nullParent << [false, true] + noopParent << [false, true] } def "should inherit the DD parent attributes"() { diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/scopemanager/ScopeManagerTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/scopemanager/ScopeManagerTest.groovy index 54afaade40..80e2eeb879 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/scopemanager/ScopeManagerTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/scopemanager/ScopeManagerTest.groovy @@ -96,19 +96,19 @@ class ScopeManagerTest extends Specification { def "sets parent as current upon close"() { setup: def parentScope = tracer.buildSpan("parent").startActive(finishSpan) - def childScope = nullChild ? tracer.scopeManager().activate(null, finishSpan) : tracer.buildSpan("parent").startActive(finishSpan) + def childScope = noopChild ? tracer.scopeManager().activate(NoopSpan.INSTANCE, finishSpan) : tracer.buildSpan("parent").startActive(finishSpan) expect: scopeManager.active() == childScope - nullChild || childScope.span().context().parentId == parentScope.span().context().spanId - nullChild || childScope.span().context().trace == parentScope.span().context().trace + noopChild || childScope.span().context().parentId == parentScope.span().context().spanId + noopChild || childScope.span().context().trace == parentScope.span().context().trace when: childScope.close() then: scopeManager.active() == parentScope - spanFinished(childScope.span()) == (nullChild ? null : finishSpan) + noopChild || spanFinished(childScope.span()) == finishSpan !spanFinished(parentScope.span()) writer == [] @@ -116,13 +116,13 @@ class ScopeManagerTest extends Specification { parentScope.close() then: - spanFinished(childScope.span()) == (nullChild ? null : finishSpan) + noopChild || spanFinished(childScope.span()) == finishSpan spanFinished(parentScope.span()) == finishSpan - writer == [[parentScope.span(), childScope.span()]] || !finishSpan || nullChild + writer == [[parentScope.span(), childScope.span()]] || !finishSpan || noopChild scopeManager.active() == null where: - finishSpan | nullChild + finishSpan | noopChild true | false false | false true | true @@ -571,7 +571,7 @@ class ScopeManagerTest extends Specification { closedCount.get() == 4 } - Boolean spanFinished(Span span) { + boolean spanFinished(Span span) { return ((DDSpan) span)?.isFinished() }