NEWRELIC-4204 Fix lettuce instrumentation for clustering URLs

instrumentation/lettuce-4.3/build.gradle

@@ -2,9 +2,14 @@ dependencies {
     implementation(project(":newrelic-weaver-api"))
     implementation(project(":agent-bridge"))
     implementation 'biz.paluch.redis:lettuce:4.4.1.Final'
+
     testImplementation('org.testcontainers:testcontainers:1.17.1')
-    testImplementation('junit:junit:4.13.1')
+    testImplementation 'org.junit.jupiter:junit-jupiter-api:5.8.1'
+
+    testCompileOnly 'junit:junit:4.13.1'
 
+    testRuntimeOnly 'org.junit.vintage:junit-vintage-engine:5.8.1'
+    testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.8.1'
 }
 
 jar {

instrumentation/lettuce-4.3/src/main/java/com/lambdaworks/redis/AbstractRedisAsyncCommands_Instrumentation.java

@@ -17,6 +17,7 @@
 import com.newrelic.api.agent.weaver.Weave;
 import com.newrelic.api.agent.weaver.Weaver;
 import com.nr.lettuce43.instrumentation.NRBiConsumer;
+import com.nr.lettuce43.instrumentation.RedisDatastoreParameters;
 
 @Weave(originalName = "com.lambdaworks.redis.AbstractRedisAsyncCommands")
 public abstract class AbstractRedisAsyncCommands_Instrumentation<K, V> {
@@ -42,15 +43,7 @@ public <T> AsyncCommand<K, V, T> dispatch(RedisCommand<K, V, T> cmd) {
         if (t != null && t.name() != null && !t.name().isEmpty()) {
             operation = t.name();
         }
-        DatastoreParameters params = null;
-        if (uri != null) {
-
-            params = DatastoreParameters.product("Redis").collection(collName).operation(operation)
-                    .instance(uri.getHost(), uri.getPort()).noDatabaseName().build();
-        } else {
-            params = DatastoreParameters.product("Redis").collection(collName).operation("").noInstance()
-                    .noDatabaseName().noSlowQuery().build();
-        }
+        DatastoreParameters params = RedisDatastoreParameters.from(uri, operation);
         Segment segment = NewRelic.getAgent().getTransaction().startSegment("Lettuce", operation);
 
         NRBiConsumer<T> nrBiConsumer = new NRBiConsumer<T>(segment, params);

instrumentation/lettuce-4.3/src/main/java/com/lambdaworks/redis/RedisClient_Instrumentation.java

@@ -27,7 +27,7 @@ public static RedisClient_Instrumentation create(String uri) {
     public abstract StatefulRedisConnection<String, String> connect();
 
     protected <K, V> StatefulRedisConnectionImpl_Instrumentation<K, V> newStatefulRedisConnection(CommandHandler<K, V> commandHandler,
-            RedisCodec<K, V> codec, long timeout, TimeUnit unit) {
+                                                                                                  RedisCodec<K, V> codec, long timeout, TimeUnit unit) {
         StatefulRedisConnectionImpl_Instrumentation<K, V> connection = Weaver.callOriginal();
         connection.redisURI = redisURI;
         return connection;

instrumentation/lettuce-4.3/src/main/java/com/lambdaworks/redis/StatefulRedisConnectionImpl_Instrumentation.java

@@ -20,7 +20,7 @@ public abstract class StatefulRedisConnectionImpl_Instrumentation<K, V> implemen
     public RedisURI redisURI = null;
 
     public StatefulRedisConnectionImpl_Instrumentation(RedisChannelWriter<K, V> writer, RedisCodec<K, V> codec, long timeout,
-            TimeUnit unit) {
+                                                       TimeUnit unit) {
 
     }
 }

instrumentation/lettuce-4.3/src/main/java/com/nr/lettuce43/instrumentation/RedisDatastoreParameters.java

@@ -0,0 +1,19 @@
+package com.nr.lettuce43.instrumentation;
+
+import com.lambdaworks.redis.RedisURI;
+import com.newrelic.api.agent.DatastoreParameters;
+
+public class RedisDatastoreParameters {
+    public static DatastoreParameters from(RedisURI uri, String operation) {
+        DatastoreParameters params;
+        if (uri != null) {
+
+            params = DatastoreParameters.product("Redis").collection(null).operation(operation)
+                    .instance(uri.getHost(), uri.getPort()).noDatabaseName().build();
+        } else {
+            params = DatastoreParameters.product("Redis").collection(null).operation(operation).noInstance()
+                    .noDatabaseName().noSlowQuery().build();
+        }
+        return params;
+    }
+}

instrumentation/lettuce-4.3/src/test/java/com/nr/lettuce43/instrumentation/Lettuce43InstrumentationTest.java

@@ -27,7 +27,7 @@
 import static org.junit.Assert.assertTrue;
 
 @RunWith(InstrumentationTestRunner.class)
-@InstrumentationTestConfig(includePrefixes = { "com.lambdaworks.redis" })
+@InstrumentationTestConfig(includePrefixes = {"com.lambdaworks.redis"})
 public class Lettuce43InstrumentationTest {
 
     @Rule

instrumentation/lettuce-4.3/src/test/java/com/nr/lettuce43/instrumentation/RedisDatastoreParametersTest.java

@@ -0,0 +1,45 @@
+package com.nr.lettuce43.instrumentation;
+
+import com.lambdaworks.redis.RedisURI;
+import com.newrelic.api.agent.DatastoreParameters;
+import junit.framework.TestCase;
+import org.junit.jupiter.api.Test;
+
+public class RedisDatastoreParametersTest extends TestCase {
+
+    @Test
+    public void testDatastoreParametersForUri() {
+        // Given
+        Integer expectedPort = 12345;
+        RedisURI uri = RedisURI.create("redis://localhost:" + expectedPort);
+        String operation = "GET";
+
+        // When
+        DatastoreParameters dbParams = RedisDatastoreParameters.from(uri, operation);
+
+        // Then url values are correct
+        assertEquals(expectedPort, dbParams.getPort());
+        assertEquals("localhost", dbParams.getHost());
+
+        // And database info is populated correctly
+        assertEquals("Redis", dbParams.getProduct());
+        assertEquals(operation, dbParams.getOperation());
+    }
+
+    @Test
+    public void testDatastoreParametersForClustering_noUrl() {
+        // Given
+        String operation = "GET";
+
+        // When
+        DatastoreParameters dbParams = RedisDatastoreParameters.from(null, operation);
+
+        // Then there are no url values
+        assertNull(dbParams.getPort());
+        assertNull(dbParams.getHost());
+
+        // And database info is STILL populated correctly
+        assertEquals("Redis", dbParams.getProduct());
+        assertEquals(operation, dbParams.getOperation());
+    }
+}

instrumentation/lettuce-5.0/build.gradle

@@ -1,8 +1,14 @@
 dependencies {
     implementation(project(":agent-bridge"))
     implementation group: 'io.lettuce', name: 'lettuce-core', version: '5.0.3.RELEASE'
+
     testImplementation('org.testcontainers:testcontainers:1.17.1')
-    testImplementation('junit:junit:4.13.1')
+    testImplementation 'org.junit.jupiter:junit-jupiter-api:5.8.1'
+
+    testCompileOnly 'junit:junit:4.13.1'
+
+    testRuntimeOnly 'org.junit.vintage:junit-vintage-engine:5.8.1'
+    testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.8.1'
 }
 
 jar {

instrumentation/lettuce-5.0/src/main/java/com/nr/lettuce5/instrumentation/RedisDatastoreParameters.java

@@ -0,0 +1,19 @@
+package com.nr.lettuce5.instrumentation;
+
+import com.newrelic.api.agent.DatastoreParameters;
+import io.lettuce.core.RedisURI;
+
+public class RedisDatastoreParameters {
+    public static DatastoreParameters from(RedisURI uri, String operation) {
+        DatastoreParameters params;
+        if (uri != null) {
+
+            params = DatastoreParameters.product("Redis").collection(null).operation(operation)
+                    .instance(uri.getHost(), uri.getPort()).noDatabaseName().build();
+        } else {
+            params = DatastoreParameters.product("Redis").collection(null).operation(operation).noInstance()
+                    .noDatabaseName().noSlowQuery().build();
+        }
+        return params;
+    }
+}

instrumentation/lettuce-5.0/src/main/java/io/lettuce/core/AbstractRedisAsyncCommands_Instrumentation.java

@@ -13,6 +13,7 @@
 import com.newrelic.api.agent.weaver.Weave;
 import com.newrelic.api.agent.weaver.Weaver;
 import com.nr.lettuce5.instrumentation.NRBiConsumer;
+import com.nr.lettuce5.instrumentation.RedisDatastoreParameters;
 import io.lettuce.core.api.StatefulConnection;
 import io.lettuce.core.protocol.AsyncCommand;
 import io.lettuce.core.protocol.ProtocolKeyword;
@@ -44,15 +45,7 @@ public <T> AsyncCommand<K, V, T> dispatch(RedisCommand<K, V, T> cmd) {
         if (operation.equalsIgnoreCase("expire")) {
             return acmd;
         }
-        DatastoreParameters params;
-        if (uri != null) {
-
-            params = DatastoreParameters.product("Redis").collection(null).operation(operation)
-                    .instance(uri.getHost(), uri.getPort()).noDatabaseName().build();
-        } else {
-            params = DatastoreParameters.product("Redis").collection(null).operation("").noInstance()
-                    .noDatabaseName().noSlowQuery().build();
-        }
+        DatastoreParameters params = RedisDatastoreParameters.from(uri, operation);
         Segment segment = NewRelic.getAgent().getTransaction().startSegment("Redis", operation);
 
         NRBiConsumer<T> nrBiConsumer = new NRBiConsumer<T>(segment, params);

instrumentation/lettuce-5.0/src/main/java/io/lettuce/core/AbstractRedisReactiveCommands_Instrumentation.java

@@ -9,10 +9,7 @@
 import com.newrelic.api.agent.DatastoreParameters;
 import com.newrelic.api.agent.weaver.Weave;
 import com.newrelic.api.agent.weaver.Weaver;
-import com.nr.lettuce5.instrumentation.NRErrorConsumer;
-import com.nr.lettuce5.instrumentation.NRHolder;
-import com.nr.lettuce5.instrumentation.NRSignalTypeConsumer;
-import com.nr.lettuce5.instrumentation.NRSubscribeConsumer;
+import com.nr.lettuce5.instrumentation.*;
 import io.lettuce.core.api.StatefulConnection;
 import io.lettuce.core.protocol.ProtocolKeyword;
 import io.lettuce.core.protocol.RedisCommand;
@@ -47,17 +44,7 @@ public <T> Mono<T> createMono(Supplier<RedisCommand<K, V, T>> commandSupplier) {
             if ((t != null) && (t.name() != null) && (!t.name().isEmpty())) {
                 operation = t.name();
             }
-            DatastoreParameters params = null;
-            if (uri != null) {
-                params = DatastoreParameters.product("Redis")
-                        .collection(collName)
-                        .operation(operation)
-                        .instance(uri.getHost(), Integer.valueOf(uri.getPort()))
-                        .noDatabaseName()
-                        .build();
-            } else {
-                params = DatastoreParameters.product("Redis").collection(collName).operation(operation).noInstance().noDatabaseName().noSlowQuery().build();
-            }
+            DatastoreParameters params = RedisDatastoreParameters.from(uri, operation);
             NRHolder holder = new NRHolder(name, params);
             NRSubscribeConsumer subscriberConsumer = new NRSubscribeConsumer(holder);
 

instrumentation/lettuce-5.0/src/main/java/io/lettuce/core/RedisClient_Instrumentation.java

@@ -30,7 +30,7 @@ public static RedisClient_Instrumentation create(String uri) {
     public abstract StatefulRedisConnection<String, String> connect();
 
     protected <K, V> StatefulRedisConnectionImpl_Instrumentation<K, V> newStatefulRedisConnection(RedisChannelWriter channelWriter,
-            RedisCodec<K, V> codec, Duration timeout) {
+                                                                                                  RedisCodec<K, V> codec, Duration timeout) {
         StatefulRedisConnectionImpl_Instrumentation<K, V> connection = Weaver.callOriginal();
         connection.redisURI = redisURI;
         return connection;

instrumentation/lettuce-5.0/src/test/java/com/nr/lettuce5/instrumentation/Lettuce5InstrumentationTest.java

@@ -25,12 +25,10 @@
 import java.util.List;
 import java.util.concurrent.ExecutionException;
 
-import static org.junit.Assert.assertArrayEquals;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;
 
 @RunWith(InstrumentationTestRunner.class)
-@InstrumentationTestConfig(includePrefixes = { "io.lettuce.core", "io.lettuce.core.protocol", "io.netty.channel" })
+@InstrumentationTestConfig(includePrefixes = {"io.lettuce.core", "io.lettuce.core.protocol", "io.netty.channel"})
 public class Lettuce5InstrumentationTest {
 
     @Rule
@@ -139,14 +137,14 @@ public void testReactive() {
 
         // then all 'OK'
         assertArrayEquals("All responses should be 'OK'",
-                new String[] { "OK", "OK", "OK" }, ids.toArray());
+                new String[]{"OK", "OK", "OK"}, ids.toArray());
 
         // when reactive 'get' called
         List<String> values = redisDataService
                 .reactiveGet(Flux.just(data1.key, data2.key, data3.key));
 
         // then 3 values returned
-        String[] expectedValues = new String[] { data1.value, data2.value, data3.value };
+        String[] expectedValues = new String[]{data1.value, data2.value, data3.value};
         assertEquals("Get values size did not math the amount set", 3, values.size());
         assertArrayEquals("Values returned should equal sent", expectedValues, values.toArray());
 

instrumentation/lettuce-5.0/src/test/java/com/nr/lettuce5/instrumentation/RedisDatastoreParametersTest.java

@@ -0,0 +1,45 @@
+package com.nr.lettuce5.instrumentation;
+
+import com.newrelic.api.agent.DatastoreParameters;
+import io.lettuce.core.RedisURI;
+import junit.framework.TestCase;
+import org.junit.jupiter.api.Test;
+
+public class RedisDatastoreParametersTest extends TestCase {
+
+    @Test
+    public void testDatastoreParametersForUri() {
+        // Given
+        Integer expectedPort = 12345;
+        RedisURI uri = RedisURI.create("redis://localhost:" + expectedPort);
+        String operation = "GET";
+
+        // When
+        DatastoreParameters dbParams = RedisDatastoreParameters.from(uri, operation);
+
+        // Then url values are correct
+        assertEquals(expectedPort, dbParams.getPort());
+        assertEquals("localhost", dbParams.getHost());
+
+        // And database info is populated correctly
+        assertEquals("Redis", dbParams.getProduct());
+        assertEquals(operation, dbParams.getOperation());
+    }
+
+    @Test
+    public void testDatastoreParametersForClustering_noUrl() {
+        // Given
+        String operation = "GET";
+
+        // When
+        DatastoreParameters dbParams = RedisDatastoreParameters.from(null, operation);
+
+        // Then there are no url values
+        assertNull(dbParams.getPort());
+        assertNull(dbParams.getHost());
+
+        // And database info is STILL populated correctly
+        assertEquals("Redis", dbParams.getProduct());
+        assertEquals(operation, dbParams.getOperation());
+    }
+}

instrumentation/lettuce-6.0/build.gradle

@@ -3,8 +3,14 @@ apply plugin: 'java'
 dependencies {
     implementation(project(":agent-bridge"))
     implementation group: 'io.lettuce', name: 'lettuce-core', version: '6.1.8.RELEASE'
+
     testImplementation('org.testcontainers:testcontainers:1.17.1')
-    testImplementation('junit:junit:4.13.1')
+    testImplementation 'org.junit.jupiter:junit-jupiter-api:5.8.1'
+
+    testCompileOnly 'junit:junit:4.13.1'
+
+    testRuntimeOnly 'org.junit.vintage:junit-vintage-engine:5.8.1'
+    testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.8.1'
 }
 
 jar {

instrumentation/lettuce-6.0/src/main/java/com/nr/lettuce6/instrumentation/RedisDatastoreParameters.java

@@ -0,0 +1,19 @@
+package com.nr.lettuce6.instrumentation;
+
+import com.newrelic.api.agent.DatastoreParameters;
+import io.lettuce.core.RedisURI;
+
+public class RedisDatastoreParameters {
+    public static DatastoreParameters from(RedisURI uri, String operation) {
+        DatastoreParameters params;
+        if (uri != null) {
+
+            params = DatastoreParameters.product("Redis").collection(null).operation(operation)
+                    .instance(uri.getHost(), uri.getPort()).noDatabaseName().build();
+        } else {
+            params = DatastoreParameters.product("Redis").collection(null).operation(operation).noInstance()
+                    .noDatabaseName().noSlowQuery().build();
+        }
+        return params;
+    }
+}