From a4d53b0dcd128fcb72f27923f570befde9069952 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Thu, 13 Jun 2019 14:18:51 -0700 Subject: [PATCH] Update Cassandra Tests and more instance name cleanup --- .../cassandra/CassandraClientDecorator.java | 13 +- .../test/groovy/CassandraClientTest.groovy | 133 +++++++++++------- .../instrumentation/jdbc/JDBCDecorator.java | 6 +- .../lettuce/LettuceClientDecorator.java | 5 +- .../test/groovy/LettuceAsyncClientTest.groovy | 2 - .../test/groovy/LettuceSyncClientTest.groovy | 2 - .../src/test/groovy/Netty40ClientTest.groovy | 3 +- .../src/test/groovy/Netty41ClientTest.groovy | 3 +- .../agent/test/base/HttpClientTest.groovy | 25 +--- .../trace/agent/test/utils/TraceUtils.groovy | 17 +++ 10 files changed, 114 insertions(+), 95 deletions(-) diff --git a/dd-java-agent/instrumentation/datastax-cassandra-3/src/main/java/datadog/trace/instrumentation/datastax/cassandra/CassandraClientDecorator.java b/dd-java-agent/instrumentation/datastax-cassandra-3/src/main/java/datadog/trace/instrumentation/datastax/cassandra/CassandraClientDecorator.java index 98eaab8e5e..e757311252 100644 --- a/dd-java-agent/instrumentation/datastax-cassandra-3/src/main/java/datadog/trace/instrumentation/datastax/cassandra/CassandraClientDecorator.java +++ b/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 { public static final CassandraClientDecorator DECORATE = new CassandraClientDecorator(); @@ -53,15 +50,7 @@ public class CassandraClientDecorator extends DatabaseClientDecorator { 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()); - - 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()); - } + onPeerConnection(span, host.getSocketAddress().getAddress()); } return span; } 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 14c55c407c..18968c1ea1 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 @@ -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( - "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" + session.execute(statement) expect: - session.getClass().getName().endsWith("cassandra.TracingSession") - TEST_WRITER.size() == 5 - final DDSpan selectTrace = TEST_WRITER.get(TEST_WRITER.size() - 1).get(0) + assertTraces(keyspace ? 2 : 1) { + if (keyspace) { + trace(0, 1) { + cassandraSpan(it, 0, "USE $keyspace", null) + } + } + trace(keyspace ? 1 : 0, 1) { + cassandraSpan(it, 0, statement, keyspace) + } + } - selectTrace.getServiceName() == "cassandra" - selectTrace.getOperationName() == "cassandra.query" - selectTrace.getResourceName() == query - selectTrace.getSpanType() == DDSpanTypes.CASSANDRA + cleanup: + session.close() - 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" + 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 "async traces"() { + def "test async"() { setup: - final Session session = cluster.connectAsync().get() - - session.executeAsync("DROP KEYSPACE IF EXISTS async_test").get() - session - .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" + Session session = cluster.connect(keyspace) + runUnderTrace("parent") { + session.executeAsync(statement) + blockUntilChildSpansFinished(1) + } expect: - session.getClass().getName().endsWith("cassandra.TracingSession") - final DDSpan selectTrace = TEST_WRITER.get(TEST_WRITER.size() - 1).get(0) + assertTraces(keyspace ? 2 : 1) { + if (keyspace) { + trace(0, 1) { + cassandraSpan(it, 0, "USE $keyspace", null) + } + } + trace(keyspace ? 1 : 0, 2) { + basicSpan(it, 0, "parent") + cassandraSpan(it, 1, statement, keyspace, span(0)) + } + } - selectTrace.getServiceName() == "cassandra" - selectTrace.getOperationName() == "cassandra.query" - selectTrace.getResourceName() == query - selectTrace.getSpanType() == DDSpanTypes.CASSANDRA + cleanup: + session.close() - 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" + 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() + } + } + } + } diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java index 8540f2ab67..8c5f12a7b0 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java @@ -47,7 +47,11 @@ public class JDBCDecorator extends DatabaseClientDecorator { @Override protected String dbInstance(final DBInfo info) { - return info.getInstance(); + if (info.getInstance() != null) { + return info.getInstance(); + } else { + return info.getDb(); + } } public Span onConnection(final Span span, final Connection connection) { diff --git a/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java b/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java index 491bdb6448..f05674b500 100644 --- a/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java +++ b/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java @@ -43,7 +43,7 @@ public class LettuceClientDecorator extends DatabaseClientDecorator { @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 { 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); } diff --git a/dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceAsyncClientTest.groovy b/dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceAsyncClientTest.groovy index 81ab5a9da5..5af20d06d0 100644 --- a/dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceAsyncClientTest.groovy +++ b/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 diff --git a/dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceSyncClientTest.groovy b/dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceSyncClientTest.groovy index 9bb25e2416..f328eb240b 100644 --- a/dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceSyncClientTest.groovy +++ b/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 diff --git a/dd-java-agent/instrumentation/netty-4.0/src/test/groovy/Netty40ClientTest.groovy b/dd-java-agent/instrumentation/netty-4.0/src/test/groovy/Netty40ClientTest.groovy index 0961415733..cdf7c74d77 100644 --- a/dd-java-agent/instrumentation/netty-4.0/src/test/groovy/Netty40ClientTest.groovy +++ b/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 { and: assertTraces(1) { trace(0, 2) { - parentSpan(it, 0, thrownException) + basicSpan(it, 0, "parent", thrownException) span(1) { operationName "netty.connect" diff --git a/dd-java-agent/instrumentation/netty-4.1/src/test/groovy/Netty41ClientTest.groovy b/dd-java-agent/instrumentation/netty-4.1/src/test/groovy/Netty41ClientTest.groovy index e8f722e899..0a76910b66 100644 --- a/dd-java-agent/instrumentation/netty-4.1/src/test/groovy/Netty41ClientTest.groovy +++ b/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 { and: assertTraces(1) { trace(0, 2) { - parentSpan(it, 0, thrownException) + basicSpan(it, 0, "parent", thrownException) span(1) { operationName "netty.connect" diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpClientTest.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpClientTest.groovy index 5bc4818091..cc949da32c 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpClientTest.groovy +++ b/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 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 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 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 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 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) { diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/TraceUtils.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/TraceUtils.groovy index db2dc132fc..c349504cb1 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/TraceUtils.groovy +++ b/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) + } + } + } + } }