eyalkoren · May 16, 2022 · May 10, 2022 · May 16, 2022 · May 16, 2022
@@ -33,6 +33,7 @@ updates:
       - dependency-name: "com.datastax.oss:java-driver-core"
       - dependency-name: "io.micrometer:*"
       - dependency-name: "jakarta.*:*"
+      - dependency-name: "redis.clients:*"
     ignore:
       - dependency-name: "net.bytebuddy:byte-buddy-agent"
         # We deliberately want to keep this older version of Byte Buddy for our runtime attach test

@@ -36,13 +36,15 @@ custom `service.name` field setting and either allow the agent to automatically
 HTTP 2 request handling, transactions are ended when the request end event occurs and are kept alive until response end, when they are allowed
 to recycle. This allows for spans representing asynchronous handling of requests for which the corresponding transaction has ended -
 {pull}2564[#2564]
+* Jedis clients instrumentation was changed
 
 [float]
 ===== Features
 * Set the service version when using the ECS reformatting of the application logs: {pull}2603[#2603]
 * Add ECS-reformatting support for `java.util.logging` - {pull}2591[#2591]
 * Added support for setting the service version on Log4j2's EcsLayout - {pull}2604[#2604]
 * Added support for AWS S3 and DynamoDB - {pull}2606[#2606]
+* Added support for Jedis 4.x clients
 
 [float]
 ===== Bug fixes

@@ -0,0 +1,47 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <artifactId>apm-redis-plugin</artifactId>
+        <groupId>co.elastic.apm</groupId>
+        <version>1.30.2-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>apm-jedis-4-plugin</artifactId>
+    <name>${project.groupId}:${project.artifactId}</name>
+
+    <properties>
+        <apm-agent-parent.base.dir>${project.basedir}/../../..</apm-agent-parent.base.dir>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>${project.groupId}</groupId>
+            <artifactId>apm-jedis-plugin</artifactId>
+            <version>${project.version}</version>
+        </dependency>
+
+        <dependency>
+            <groupId>${project.groupId}</groupId>
+            <artifactId>apm-redis-common</artifactId>
+            <type>test-jar</type>
+            <version>${project.version}</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>${project.groupId}</groupId>
+            <artifactId>apm-jedis-plugin</artifactId>
+            <type>test-jar</type>
+            <version>${project.version}</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>redis.clients</groupId>
+            <artifactId>jedis</artifactId>
+            <version>4.2.3</version>
+            <scope>provided</scope>
+        </dependency>
+    </dependencies>
+
+</project>
@@ -0,0 +1,46 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package co.elastic.apm.agent.jedis;
+
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+
+import static co.elastic.apm.agent.bci.bytebuddy.CustomElementMatchers.classLoaderCanLoadClass;
+import static net.bytebuddy.matcher.ElementMatchers.isBootstrapClassLoader;
+import static net.bytebuddy.matcher.ElementMatchers.named;
+import static net.bytebuddy.matcher.ElementMatchers.not;
+import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
+
+public class Jedis4ConnectionInstrumentation extends JedisConnectionInstrumentation {
+
+    @Override
+    public ElementMatcher.Junction<ClassLoader> getClassLoaderMatcher() {
+        return not(isBootstrapClassLoader()).and(classLoaderCanLoadClass("redis.clients.jedis.CommandArguments"));
+    }
+
+    @Override
+    public String getAdviceClassName() {
+        return "co.elastic.apm.agent.jedis.Jedis4SendCommandAdvice";
+    }
+
+    @Override
+    public ElementMatcher<? super MethodDescription> getMethodMatcher() {
+        return named("sendCommand").and(takesArgument(0, named("redis.clients.jedis.CommandArguments")));
+    }
+}
@@ -0,0 +1,56 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package co.elastic.apm.agent.jedis;
+
+import co.elastic.apm.agent.impl.transaction.Span;
+import co.elastic.apm.agent.redis.RedisSpanUtils;
+import net.bytebuddy.asm.Advice;
+import redis.clients.jedis.CommandArguments;
+import redis.clients.jedis.DefaultJedisSocketFactory;
+import redis.clients.jedis.HostAndPort;
+import redis.clients.jedis.JedisSocketFactory;
+
+import javax.annotation.Nullable;
+
+public class Jedis4SendCommandAdvice {
+
+    @Nullable
+    @Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
+    public static Object sendCommandEntry(@Advice.Argument(0) CommandArguments commandArguments) {
+        return RedisSpanUtils.createRedisSpan(commandArguments.getCommand().toString());
+    }
+
+    @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class, inline = false)
+    public static void sendCommandExit(@Advice.FieldValue("socketFactory") JedisSocketFactory socketFactory,
+                                       @Nullable @Advice.Enter Object spanObj,
+                                       @Nullable @Advice.Thrown Throwable thrown) {
+        Span span = (Span) spanObj;
+        if (span != null) {
+            if (socketFactory instanceof DefaultJedisSocketFactory) {
+                HostAndPort hostAndPort = ((DefaultJedisSocketFactory) socketFactory).getHostAndPort();
+                span.getContext().getDestination()
+                    .withAddress(hostAndPort.getHost())
+                    .withPort(hostAndPort.getPort());
+            }
+            span.captureException(thrown)
+                .deactivate()
+                .end();
+        }
+    }
+}
@@ -0,0 +1,22 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+@NonnullApi
+package co.elastic.apm.agent.jedis;
+
+import co.elastic.apm.agent.sdk.NonnullApi;
@@ -0,0 +1 @@
+co.elastic.apm.agent.jedis.Jedis4ConnectionInstrumentation
@@ -0,0 +1,63 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package co.elastic.apm.agent.jedis;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import redis.clients.jedis.HostAndPort;
+import redis.clients.jedis.JedisSharding;
+import redis.clients.jedis.UnifiedJedis;
+
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class Jedis4InstrumentationIT extends Jedis1InstrumentationIT {
+    private JedisSharding shardedJedis;
+    private UnifiedJedis binaryJedis;
+
+    @BeforeEach
+    void setUp() {
+        shardedJedis = new JedisSharding(List.of(new HostAndPort("localhost", redisPort)));
+        binaryJedis = new UnifiedJedis(new HostAndPort("localhost", redisPort));
+    }
+
+    @AfterEach
+    void tearDown() {
+        shardedJedis.close();
+        binaryJedis.close();
+    }
+
+    @Test
+    void testShardedJedis() {
+        shardedJedis.set("foo", "bar");
+        assertThat(shardedJedis.get("foo".getBytes())).isEqualTo("bar".getBytes());
+
+        assertTransactionWithRedisSpans("SET", "GET");
+    }
+
+    @Test
+    void testBinaryJedis() {
+        binaryJedis.set("foo".getBytes(), "bar".getBytes());
+        assertThat(binaryJedis.get("foo".getBytes())).isEqualTo("bar".getBytes());
+
+        assertTransactionWithRedisSpans("SET", "GET");
+    }
+}
@@ -19,54 +19,41 @@
 package co.elastic.apm.agent.jedis;
 
 import co.elastic.apm.agent.bci.TracerAwareInstrumentation;
-import co.elastic.apm.agent.impl.transaction.AbstractSpan;
-import co.elastic.apm.agent.impl.transaction.Span;
-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.Arrays;
 import java.util.Collection;
 
-import static net.bytebuddy.matcher.ElementMatchers.isPublic;
+import static co.elastic.apm.agent.bci.bytebuddy.CustomElementMatchers.classLoaderCanLoadClass;
+import static net.bytebuddy.matcher.ElementMatchers.isBootstrapClassLoader;
 import static net.bytebuddy.matcher.ElementMatchers.nameEndsWith;
 import static net.bytebuddy.matcher.ElementMatchers.named;
+import static net.bytebuddy.matcher.ElementMatchers.not;
 import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
 
-/**
- * Sets the Redis span name to the Redis protocol command.
- * Because of the way the instrumentation works for Jedis,
- * it would otherwise be set to the {@link redis.clients.jedis.Jedis} client method name.
- * This is good enough as a default but we want all Redis clients to produce the same span names.
- */
-public class JedisSpanNameInstrumentation extends TracerAwareInstrumentation {
+public class JedisConnectionInstrumentation extends TracerAwareInstrumentation {
 
-    public static class AdviceClass {
-        @Advice.OnMethodEnter(inline = false)
-        public static void setSpanNameToRedisProtocolCommand(@Advice.Argument(1) Object command) {
-            AbstractSpan<?> active = tracer.getActive();
-            if (active instanceof Span) {
-                Span activeSpan = (Span) active;
-                if ("redis".equals(activeSpan.getSubtype())) {
-                    activeSpan.withName(command.toString());
-                }
-            }
-        }
+    @Override
+    public ElementMatcher.Junction<ClassLoader> getClassLoaderMatcher() {
+        // This makes sure we do not apply this instrumentation to Jedis 4 Connection, for which we have dedicated Advice
+        return not(isBootstrapClassLoader()).and(not(classLoaderCanLoadClass("redis.clients.jedis.CommandArguments")));
+    }
+
+    @Override
+    public String getAdviceClassName() {
+        return "co.elastic.apm.agent.jedis.SendCommandAdvice";
     }
 
     @Override
     public ElementMatcher<? super TypeDescription> getTypeMatcher() {
-        return named("redis.clients.jedis.Protocol");
+        return named("redis.clients.jedis.Connection");
     }
 
     @Override
     public ElementMatcher<? super MethodDescription> getMethodMatcher() {
-        return named("sendCommand")
-            .and(isPublic())
-            .and(takesArgument(0, nameEndsWith("RedisOutputStream")))
-            .and(takesArgument(1, nameEndsWith("Command")))
-            .and(takesArgument(2, byte[][].class));
+        return named("sendCommand").and(takesArgument(0, nameEndsWith("Command")));
     }
 
     @Override
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		co.elastic.apm.agent.jedis.Jedis4ConnectionInstrumentation