From 1fbb33d182382d41206b2cfe148536f9a3a9ca5a Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Fri, 22 Jun 2018 16:21:13 -0400 Subject: [PATCH] More fixes to Lettuce tests Lettuce intrumentatio is implemented in a way that after operation has been performed `span` is not closed syncronously - in fact this happens on separate thread. This means `spans` for even syncronous operations may be closed on opposite order. This means that writing tests that pefrom two operations and expect two traces is slightly more complicated. In many places we can just avoid doing that by preparing necessary data in `setup`. This fixes some of the false negatives in tests. --- .../test/groovy/LettuceAsyncClientTest.groovy | 56 +++----------- .../groovy/LettuceReactiveClientTest.groovy | 47 ++---------- .../test/groovy/LettuceSyncClientTest.groovy | 73 +++---------------- 3 files changed, 30 insertions(+), 146 deletions(-) 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 c98f99a43a..d384151c2b 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 @@ -72,7 +72,10 @@ class LettuceAsyncClientTest extends AgentTestRunner { asyncCommands = connection.async() syncCommands = connection.sync() - TEST_WRITER.waitForTraces(1) + syncCommands.set("TESTKEY", "TESTVAL") + + // 1 set + 1 connect trace + TEST_WRITER.waitForTraces(2) TEST_WRITER.clear() } @@ -162,7 +165,7 @@ class LettuceAsyncClientTest extends AgentTestRunner { def "set command using Future get with timeout"() { setup: - RedisFuture redisFuture = asyncCommands.set("TESTKEY", "TESTVAL") + RedisFuture redisFuture = asyncCommands.set("TESTSETKEY", "TESTSETVAL") String res = redisFuture.get(3, TimeUnit.SECONDS) expect: @@ -190,8 +193,6 @@ class LettuceAsyncClientTest extends AgentTestRunner { def "get command chained with thenAccept"() { setup: - syncCommands.set("TESTKEY", "TESTVAL") - def conds = new AsyncConditions() Consumer consumer = new Consumer() { @Override @@ -208,25 +209,8 @@ class LettuceAsyncClientTest extends AgentTestRunner { then: conds.await() - assertTraces(TEST_WRITER, 2) { + assertTraces(TEST_WRITER, 1) { trace(0, 1) { - span(0) { - serviceName "redis" - operationName "redis.query" - spanType "redis" - resourceName "SET" - errored false - - tags { - defaultTags() - "component" "redis-client" - "db.type" "redis" - "span.kind" "client" - "span.type" "redis" - } - } - } - trace(1, 1) { span(0) { serviceName "redis" operationName "redis.query" @@ -301,14 +285,12 @@ class LettuceAsyncClientTest extends AgentTestRunner { def "command with no arguments using a biconsumer"() { setup: - syncCommands.set("TESTKEY", "TESTVAL") - def conds = new AsyncConditions() BiConsumer biConsumer = new BiConsumer() { @Override void accept(String keyRetrieved, Throwable throwable) { conds.evaluate{ - assert keyRetrieved == "TESTKEY" + assert keyRetrieved != null } } } @@ -319,25 +301,8 @@ class LettuceAsyncClientTest extends AgentTestRunner { then: conds.await() - assertTraces(TEST_WRITER, 2) { + assertTraces(TEST_WRITER, 1) { trace(0, 1) { - span(0) { - serviceName "redis" - operationName "redis.query" - spanType "redis" - resourceName "SET" - errored false - - tags { - defaultTags() - "component" "redis-client" - "db.type" "redis" - "span.kind" "client" - "span.type" "redis" - } - } - } - trace(1, 1) { span(0) { serviceName "redis" operationName "redis.query" @@ -362,14 +327,15 @@ class LettuceAsyncClientTest extends AgentTestRunner { def conds = new AsyncConditions() when: - RedisFuture hmsetFuture = asyncCommands.hmset("user", testHashMap) + RedisFuture hmsetFuture = asyncCommands.hmset("TESTHM", testHashMap) hmsetFuture.thenApplyAsync(new Function() { @Override Object apply(String setResult) { + TEST_WRITER.waitForTraces(1) // Wait for 'hmset' trace to get written conds.evaluate { assert setResult == "OK" } - RedisFuture> hmGetAllFuture = asyncCommands.hgetall("user") + RedisFuture> hmGetAllFuture = asyncCommands.hgetall("TESTHM") hmGetAllFuture.exceptionally(new Function>() { @Override Map apply(Throwable throwable) { diff --git a/dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceReactiveClientTest.groovy b/dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceReactiveClientTest.groovy index c11f7cbaf1..0a7afdbf61 100644 --- a/dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceReactiveClientTest.groovy +++ b/dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceReactiveClientTest.groovy @@ -49,7 +49,10 @@ class LettuceReactiveClientTest extends AgentTestRunner { reactiveCommands = connection.reactive() syncCommands = connection.sync() - TEST_WRITER.waitForTraces(1) + syncCommands.set("TESTKEY", "TESTVAL") + + // 1 set + 1 connect trace + TEST_WRITER.waitForTraces(2) TEST_WRITER.clear() } @@ -71,7 +74,7 @@ class LettuceReactiveClientTest extends AgentTestRunner { } when: - reactiveCommands.set("TESTKEY", "TESTVAL").subscribe(consumer) + reactiveCommands.set("TESTSETKEY", "TESTSETVAL").subscribe(consumer) then: conds.await() @@ -98,7 +101,6 @@ class LettuceReactiveClientTest extends AgentTestRunner { def "get command with lambda function"() { setup: - syncCommands.set("TESTKEY", "TESTVAL") def conds = new AsyncConditions() when: @@ -106,25 +108,8 @@ class LettuceReactiveClientTest extends AgentTestRunner { then: conds.await() - assertTraces(TEST_WRITER, 2) { + assertTraces(TEST_WRITER, 1) { trace(0, 1) { - span(0) { - serviceName "redis" - operationName "redis.query" - spanType "redis" - resourceName "SET" - errored false - - tags { - defaultTags() - "component" "redis-client" - "db.type" "redis" - "span.kind" "client" - "span.type" "redis" - } - } - } - trace(1, 1) { span(0) { serviceName "redis" operationName "redis.query" @@ -184,7 +169,6 @@ class LettuceReactiveClientTest extends AgentTestRunner { def "command with no arguments"() { setup: - syncCommands.set("TESTKEY", "TESTVAL") def conds = new AsyncConditions() when: @@ -196,25 +180,8 @@ class LettuceReactiveClientTest extends AgentTestRunner { then: conds.await() - assertTraces(TEST_WRITER, 2) { + assertTraces(TEST_WRITER, 1) { trace(0, 1) { - span(0) { - serviceName "redis" - operationName "redis.query" - spanType "redis" - resourceName "SET" - errored false - - tags { - defaultTags() - "component" "redis-client" - "db.type" "redis" - "span.kind" "client" - "span.type" "redis" - } - } - } - trace(1, 1) { span(0) { serviceName "redis" operationName "redis.query" 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 963d0eb3e1..16f14ccfc3 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 @@ -53,7 +53,12 @@ class LettuceSyncClientTest extends AgentTestRunner { redisServer.start() connection = redisClient.connect() syncCommands = connection.sync() - TEST_WRITER.waitForTraces(1) + + syncCommands.set("TESTKEY", "TESTVAL") + syncCommands.hmset("TESTHM", testHashMap) + + // 2 sets + 1 connect trace + TEST_WRITER.waitForTraces(3) TEST_WRITER.clear() } @@ -137,7 +142,7 @@ class LettuceSyncClientTest extends AgentTestRunner { def "set command"() { setup: - String res = syncCommands.set("TESTKEY", "TESTVAL") + String res = syncCommands.set("TESTSETKEY", "TESTSETVAL") expect: res == "OK" @@ -164,30 +169,12 @@ class LettuceSyncClientTest extends AgentTestRunner { def "get command"() { setup: - syncCommands.set("TESTKEY", "TESTVAL") String res = syncCommands.get("TESTKEY") expect: res == "TESTVAL" - assertTraces(TEST_WRITER, 2) { + assertTraces(TEST_WRITER, 1) { trace(0, 1) { - span(0) { - serviceName "redis" - operationName "redis.query" - spanType "redis" - resourceName "SET" - errored false - - tags { - defaultTags() - "component" "redis-client" - "db.type" "redis" - "span.kind" "client" - "span.type" "redis" - } - } - } - trace(1, 1) { span(0) { serviceName "redis" operationName "redis.query" @@ -236,30 +223,12 @@ class LettuceSyncClientTest extends AgentTestRunner { def "command with no arguments"() { setup: - syncCommands.set("TESTKEY", "TESTVAL") def keyRetrieved = syncCommands.randomkey() expect: - keyRetrieved == "TESTKEY" - assertTraces(TEST_WRITER, 2) { + keyRetrieved != null + assertTraces(TEST_WRITER, 1) { trace(0, 1) { - span(0) { - serviceName "redis" - operationName "redis.query" - spanType "redis" - resourceName "SET" - errored false - - tags { - defaultTags() - "component" "redis-client" - "db.type" "redis" - "span.kind" "client" - "span.type" "redis" - } - } - } - trace(1, 1) { span(0) { serviceName "redis" operationName "redis.query" @@ -335,30 +304,12 @@ class LettuceSyncClientTest extends AgentTestRunner { def "hash getall command"() { setup: - syncCommands.hmset("user", testHashMap) - Map res = syncCommands.hgetall("user") + Map res = syncCommands.hgetall("TESTHM") expect: res == testHashMap - assertTraces(TEST_WRITER, 2) { + assertTraces(TEST_WRITER, 1) { trace(0, 1) { - span(0) { - serviceName "redis" - operationName "redis.query" - spanType "redis" - resourceName "HMSET" - errored false - - tags { - defaultTags() - "component" "redis-client" - "db.type" "redis" - "span.kind" "client" - "span.type" "redis" - } - } - } - trace(1, 1) { span(0) { serviceName "redis" operationName "redis.query"