Merge pull request #882 from DataDog/tyler/cassandra-testing

Update Cassandra Tests and more instance name cleanup

dd-java-agent/instrumentation/datastax-cassandra-3/src/main/java/datadog/trace/instrumentation/datastax/cassandra/CassandraClientDecorator.java

@@ -7,9 +7,6 @@ import datadog.trace.agent.decorator.DatabaseClientDecorator;
 import datadog.trace.api.DDSpanTypes;
 import io.opentracing.Span;
 import io.opentracing.tag.Tags;
-import java.net.Inet4Address;
-import java.net.InetAddress;
-import java.nio.ByteBuffer;
 
 public class CassandraClientDecorator extends DatabaseClientDecorator<Session> {
   public static final CassandraClientDecorator DECORATE = new CassandraClientDecorator();
@@ -53,15 +50,7 @@ public class CassandraClientDecorator extends DatabaseClientDecorator<Session> {
     if (result != null) {
       final Host host = result.getExecutionInfo().getQueriedHost();
       Tags.PEER_PORT.set(span, host.getSocketAddress().getPort());
-      Tags.PEER_HOSTNAME.set(span, host.getAddress().getHostName());
+      onPeerConnection(span, host.getSocketAddress().getAddress());
-
-      final InetAddress inetAddress = host.getSocketAddress().getAddress();
-      if (inetAddress instanceof Inet4Address) {
-        final byte[] address = inetAddress.getAddress();
-        Tags.PEER_HOST_IPV4.set(span, ByteBuffer.wrap(address).getInt());
-      } else {
-        Tags.PEER_HOST_IPV6.set(span, inetAddress.getHostAddress());
-      }
     }
     return span;
   }

dd-java-agent/instrumentation/datastax-cassandra-3/src/test/groovy/CassandraClientTest.groovy

@@ -2,15 +2,21 @@ import com.datastax.driver.core.Cluster
 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.DDSpanTypes
 import io.opentracing.tag.Tags
 import org.cassandraunit.utils.EmbeddedCassandraServerHelper
 import spock.lang.Shared
 
+import static datadog.trace.agent.test.utils.TraceUtils.basicSpan
+import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
+
 class CassandraClientTest extends AgentTestRunner {
 
   @Shared
   Cluster cluster
+  @Shared
+  int port = 9142
 
   def setupSpec() {
     /*
@@ -34,72 +40,91 @@ class CassandraClientTest extends AgentTestRunner {
     EmbeddedCassandraServerHelper.cleanEmbeddedCassandra()
   }
 
-  def "sync traces"() {
+  def "test sync"() {
     setup:
-    final Session session = cluster.newSession()
+    Session session = cluster.connect(keyspace)
 
-    session.execute("DROP KEYSPACE IF EXISTS sync_test")
+    session.execute(statement)
-    session.execute(
-      "CREATE KEYSPACE sync_test WITH REPLICATION = {'class':'SimpleStrategy', 'replication_factor':3}")
-    session.execute("CREATE TABLE sync_test.users ( id UUID PRIMARY KEY, name text )")
-    session.execute("INSERT INTO sync_test.users (id, name) values (uuid(), 'alice')")
-    session.execute("SELECT * FROM sync_test.users where name = 'alice' ALLOW FILTERING")
-
-    def query = "SELECT * FROM sync_test.users where name = 'alice' ALLOW FILTERING"
 
     expect:
-    session.getClass().getName().endsWith("cassandra.TracingSession")
+    assertTraces(keyspace ? 2 : 1) {
-    TEST_WRITER.size() == 5
+      if (keyspace) {
-    final DDSpan selectTrace = TEST_WRITER.get(TEST_WRITER.size() - 1).get(0)
+        trace(0, 1) {
-
+          cassandraSpan(it, 0, "USE $keyspace", null)
-    selectTrace.getServiceName() == "cassandra"
+        }
-    selectTrace.getOperationName() == "cassandra.query"
+      }
-    selectTrace.getResourceName() == query
+      trace(keyspace ? 1 : 0, 1) {
-    selectTrace.getSpanType() == DDSpanTypes.CASSANDRA
+        cassandraSpan(it, 0, statement, keyspace)
-
+      }
-    selectTrace.getTags().get(Tags.COMPONENT.getKey()) == "java-cassandra"
-    selectTrace.getTags().get(Tags.DB_TYPE.getKey()) == "cassandra"
-    selectTrace.getTags().get(Tags.PEER_HOSTNAME.getKey()) == "localhost"
-    // More info about IPv4 tag: https://trello.com/c/2el2IwkF/174-mongodb-ot-contrib-provides-a-wrong-peeripv4
-    selectTrace.getTags().get(Tags.PEER_HOST_IPV4.getKey()) == 2130706433
-    selectTrace.getTags().get(Tags.PEER_PORT.getKey()) == 9142
-    selectTrace.getTags().get(Tags.SPAN_KIND.getKey()) == "client"
     }
 
-  def "async traces"() {
+    cleanup:
+    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"
+  }
+
+  def "test async"() {
     setup:
-    final Session session = cluster.connectAsync().get()
+    Session session = cluster.connect(keyspace)
-
+    runUnderTrace("parent") {
-    session.executeAsync("DROP KEYSPACE IF EXISTS async_test").get()
+      session.executeAsync(statement)
-    session
+      blockUntilChildSpansFinished(1)
-      .executeAsync(
+    }
-      "CREATE KEYSPACE async_test WITH REPLICATION = {'class':'SimpleStrategy', 'replication_factor':3}")
-      .get()
-    session.executeAsync("CREATE TABLE async_test.users ( id UUID PRIMARY KEY, name text )").get()
-    session.executeAsync("INSERT INTO async_test.users (id, name) values (uuid(), 'alice')").get()
-    TEST_WRITER.waitForTraces(4)
-    session
-      .executeAsync("SELECT * FROM async_test.users where name = 'alice' ALLOW FILTERING")
-      .get()
-    TEST_WRITER.waitForTraces(5)
-
-    def query = "SELECT * FROM async_test.users where name = 'alice' ALLOW FILTERING"
 
     expect:
-    session.getClass().getName().endsWith("cassandra.TracingSession")
+    assertTraces(keyspace ? 2 : 1) {
-    final DDSpan selectTrace = TEST_WRITER.get(TEST_WRITER.size() - 1).get(0)
+      if (keyspace) {
-
+        trace(0, 1) {
-    selectTrace.getServiceName() == "cassandra"
+          cassandraSpan(it, 0, "USE $keyspace", null)
-    selectTrace.getOperationName() == "cassandra.query"
-    selectTrace.getResourceName() == query
-    selectTrace.getSpanType() == DDSpanTypes.CASSANDRA
-
-    selectTrace.getTags().get(Tags.COMPONENT.getKey()) == "java-cassandra"
-    selectTrace.getTags().get(Tags.DB_TYPE.getKey()) == "cassandra"
-    selectTrace.getTags().get(Tags.PEER_HOSTNAME.getKey()) == "localhost"
-    // More info about IPv4 tag: https://trello.com/c/2el2IwkF/174-mongodb-ot-contrib-provides-a-wrong-peeripv4
-    selectTrace.getTags().get(Tags.PEER_HOST_IPV4.getKey()) == 2130706433
-    selectTrace.getTags().get(Tags.PEER_PORT.getKey()) == 9142
-    selectTrace.getTags().get(Tags.SPAN_KIND.getKey()) == "client"
         }
       }
+      trace(keyspace ? 1 : 0, 2) {
+        basicSpan(it, 0, "parent")
+        cassandraSpan(it, 1, statement, keyspace, span(0))
+      }
+    }
+
+    cleanup:
+    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"
+  }
+
+  def cassandraSpan(TraceAssert trace, int index, String statement, String keyspace, Object parentSpan = null, Throwable exception = null) {
+    trace.span(index) {
+      serviceName "cassandra"
+      operationName "cassandra.query"
+      resourceName statement
+      spanType DDSpanTypes.CASSANDRA
+      if (parentSpan == null) {
+        parent()
+      } else {
+        childOf((DDSpan) parentSpan)
+      }
+      tags {
+        "$Tags.COMPONENT.key" "java-cassandra"
+        "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+        "$Tags.DB_INSTANCE.key" keyspace
+        "$Tags.DB_TYPE.key" "cassandra"
+        "$Tags.PEER_HOSTNAME.key" "localhost"
+        "$Tags.PEER_HOST_IPV4.key" "127.0.0.1"
+        "$Tags.PEER_PORT.key" port
+        defaultTags()
+      }
+    }
+  }
+
+}

dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java

@@ -47,7 +47,11 @@ public class JDBCDecorator extends DatabaseClientDecorator<DBInfo> {
 
   @Override
   protected String dbInstance(final DBInfo info) {
+    if (info.getInstance() != null) {
       return info.getInstance();
+    } else {
+      return info.getDb();
+    }
   }
 
   public Span onConnection(final Span span, final Connection connection) {

dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java

@@ -43,7 +43,7 @@ public class LettuceClientDecorator extends DatabaseClientDecorator<RedisURI> {
 
   @Override
   protected String dbInstance(final RedisURI connection) {
-    return connection.getHost() + ":" + connection.getPort() + "/" + connection.getDatabase();
+    return null;
   }
 
   @Override
@@ -53,7 +53,8 @@ public class LettuceClientDecorator extends DatabaseClientDecorator<RedisURI> {
       Tags.PEER_PORT.set(span, connection.getPort());
 
       span.setTag("db.redis.dbIndex", connection.getDatabase());
-      span.setTag(DDTags.RESOURCE_NAME, "CONNECT:" + dbInstance(connection));
+      span.setTag(DDTags.RESOURCE_NAME, "CONNECT:" + connection.getHost()
+        + ":" + connection.getPort() + "/" + connection.getDatabase());
     }
     return super.onConnection(span, connection);
   }

dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceAsyncClientTest.groovy

@@ -123,7 +123,6 @@ class LettuceAsyncClientTest extends AgentTestRunner {
           tags {
             defaultTags()
             "component" "redis-client"
-            "db.instance" dbAddr
             "db.redis.dbIndex" 0
             "db.type" "redis"
             "peer.hostname" HOST
@@ -163,7 +162,6 @@ class LettuceAsyncClientTest extends AgentTestRunner {
           tags {
             defaultTags()
             "component" "redis-client"
-            "db.instance" dbAddrNonExistent
             "db.redis.dbIndex" 0
             "db.type" "redis"
             errorTags CompletionException, String

dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceSyncClientTest.groovy

@@ -103,7 +103,6 @@ class LettuceSyncClientTest extends AgentTestRunner {
           tags {
             defaultTags()
             "component" "redis-client"
-            "db.instance" dbAddr
             "db.redis.dbIndex" 0
             "db.type" "redis"
             "peer.hostname" HOST
@@ -140,7 +139,6 @@ class LettuceSyncClientTest extends AgentTestRunner {
           tags {
             defaultTags()
             "component" "redis-client"
-            "db.instance" dbAddrNonExistent
             "db.redis.dbIndex" 0
             "db.type" "redis"
             errorTags CompletionException, String

dd-java-agent/instrumentation/netty-4.0/src/test/groovy/Netty40ClientTest.groovy

@@ -9,6 +9,7 @@ import java.util.concurrent.ExecutionException
 import java.util.concurrent.TimeUnit
 
 import static datadog.trace.agent.test.utils.PortUtils.UNUSABLE_PORT
+import static datadog.trace.agent.test.utils.TraceUtils.basicSpan
 import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
 import static org.asynchttpclient.Dsl.asyncHttpClient
 
@@ -65,7 +66,7 @@ class Netty40ClientTest extends HttpClientTest<NettyHttpClientDecorator> {
     and:
     assertTraces(1) {
       trace(0, 2) {
-        parentSpan(it, 0, thrownException)
+        basicSpan(it, 0, "parent", thrownException)
 
         span(1) {
           operationName "netty.connect"

dd-java-agent/instrumentation/netty-4.1/src/test/groovy/Netty41ClientTest.groovy

@@ -10,6 +10,7 @@ import java.util.concurrent.ExecutionException
 import java.util.concurrent.TimeUnit
 
 import static datadog.trace.agent.test.utils.PortUtils.UNUSABLE_PORT
+import static datadog.trace.agent.test.utils.TraceUtils.basicSpan
 import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
 import static org.asynchttpclient.Dsl.asyncHttpClient
 
@@ -67,7 +68,7 @@ class Netty41ClientTest extends HttpClientTest<NettyHttpClientDecorator> {
     and:
     assertTraces(1) {
       trace(0, 2) {
-        parentSpan(it, 0, thrownException)
+        basicSpan(it, 0, "parent", thrownException)
 
         span(1) {
           operationName "netty.connect"

dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpClientTest.groovy

@@ -16,6 +16,7 @@ import java.util.concurrent.ExecutionException
 import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer
 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.basicSpan
 import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
 import static org.junit.Assume.assumeTrue
 
@@ -108,7 +109,7 @@ abstract class HttpClientTest<T extends HttpClientDecorator> extends AgentTestRu
     assertTraces(2) {
       server.distributedRequestTrace(it, 0, trace(1).last())
       trace(1, size(2)) {
-        parentSpan(it, 0)
+        basicSpan(it, 0, "parent")
         clientSpan(it, 1, span(0), method, false)
       }
     }
@@ -150,7 +151,7 @@ abstract class HttpClientTest<T extends HttpClientDecorator> extends AgentTestRu
     // only one trace (client).
     assertTraces(1) {
       trace(0, size(2)) {
-        parentSpan(it, 0)
+        basicSpan(it, 0, "parent")
         clientSpan(it, 1, span(0), method, renameService)
       }
     }
@@ -173,7 +174,7 @@ abstract class HttpClientTest<T extends HttpClientDecorator> extends AgentTestRu
     // only one trace (client).
     assertTraces(1) {
       trace(0, size(3)) {
-        parentSpan(it, 0)
+        basicSpan(it, 0, "parent")
         span(1) {
           operationName "child"
           childOf span(0)
@@ -304,7 +305,7 @@ abstract class HttpClientTest<T extends HttpClientDecorator> extends AgentTestRu
     and:
     assertTraces(1) {
       trace(0, 2) {
-        parentSpan(it, 0, thrownException)
+        basicSpan(it, 0, "parent", thrownException)
         clientSpan(it, 1, span(0), method, false, false, uri, null, thrownException)
       }
     }
@@ -313,22 +314,6 @@ abstract class HttpClientTest<T extends HttpClientDecorator> extends AgentTestRu
     method = "GET"
   }
 
-  void parentSpan(TraceAssert trace, int index, Throwable exception = null) {
-    trace.span(index) {
-      parent()
-      serviceName "unnamed-java-app"
-      operationName "parent"
-      resourceName "parent"
-      errored exception != null
-      tags {
-        defaultTags()
-        if (exception) {
-          errorTags(exception.class, exception.message)
-        }
-      }
-    }
-  }
-
   // parent span must be cast otherwise it breaks debugging classloading (junit loads it early)
   void clientSpan(TraceAssert trace, int index, Object parentSpan, String method = "GET", boolean renameService = false, boolean tagQueryString = false, URI uri = server.address.resolve("/success"), Integer status = 200, Throwable exception = null) {
     trace.span(index) {

dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/TraceUtils.groovy

@@ -1,6 +1,7 @@
 package datadog.trace.agent.test.utils
 
 import datadog.trace.agent.decorator.BaseDecorator
+import datadog.trace.agent.test.asserts.TraceAssert
 import datadog.trace.context.TraceScope
 import io.opentracing.Scope
 import io.opentracing.util.GlobalTracer
@@ -40,4 +41,20 @@ class TraceUtils {
       scope.close()
     }
   }
+
+  static basicSpan(TraceAssert trace, int index, String spanName, Throwable exception = null) {
+    trace.span(index) {
+      parent()
+      serviceName "unnamed-java-app"
+      operationName spanName
+      resourceName spanName
+      errored exception != null
+      tags {
+        defaultTags()
+        if (exception) {
+          errorTags(exception.class, exception.message)
+        }
+      }
+    }
+  }
 }