diff --git a/CHANGES.md b/CHANGES.md index c0f267073..1cd619979 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -11,6 +11,7 @@ Release Notes. * Move the baseline to JDK 17 for development, the runtime baseline is still Java 8 compatible. * Remove Powermock entirely from the test cases. * Fix H2 instrumentation point +* Refactor pipeline in jedis-plugin. #### Documentation * Update docs of Tracing APIs, reorganize the API docs into six parts diff --git a/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-2.x-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v3/JedisMethodInterceptor.java b/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-2.x-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v3/JedisMethodInterceptor.java index c4a386008..0a72214a6 100644 --- a/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-2.x-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v3/JedisMethodInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-2.x-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v3/JedisMethodInterceptor.java @@ -26,6 +26,8 @@ import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceM import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult; import org.apache.skywalking.apm.network.trace.component.ComponentsDefine; import org.apache.skywalking.apm.util.StringUtil; +import redis.clients.jedis.Pipeline; +import redis.clients.jedis.Transaction; import java.lang.reflect.Method; import java.util.Optional; @@ -41,9 +43,13 @@ public class JedisMethodInterceptor implements InstanceMethodsAroundInterceptor SpanLayer.asCache(span); String methodName = method.getName(); Tags.CACHE_TYPE.set(span, "Redis"); - Tags.CACHE_CMD.set(span, methodName); - getKey(allArguments).ifPresent(key -> Tags.CACHE_KEY.set(span, key)); - parseOperation(methodName).ifPresent(op -> Tags.CACHE_OP.set(span, op)); + if (objInst instanceof Pipeline || objInst instanceof Transaction) { + Tags.CACHE_CMD.set(span, "BATCH_EXECUTE"); + } else { + Tags.CACHE_CMD.set(span, methodName); + getKey(allArguments).ifPresent(key -> Tags.CACHE_KEY.set(span, key)); + parseOperation(methodName).ifPresent(op -> Tags.CACHE_OP.set(span, op)); + } } private Optional getKey(Object[] allArguments) { diff --git a/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-2.x-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v3/define/PipelineInstrumentation.java b/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-2.x-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v3/define/PipelineInstrumentation.java index 9a41706c9..b304dd8bc 100644 --- a/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-2.x-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v3/define/PipelineInstrumentation.java +++ b/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-2.x-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v3/define/PipelineInstrumentation.java @@ -30,6 +30,7 @@ public class PipelineInstrumentation extends AbstractWitnessInstrumentation { private static final String ENHANCE_CLASS = "redis.clients.jedis.Pipeline"; private static final String PIPELINE_SET_CLIENT_METHOD_INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.jedis.v3.PipelineSetClientMethodInterceptor"; + private static final String JEDIS_METHOD_INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.jedis.v3.JedisMethodInterceptor"; @Override public ClassMatch enhanceClass() { @@ -56,6 +57,23 @@ public class PipelineInstrumentation extends AbstractWitnessInstrumentation { return PIPELINE_SET_CLIENT_METHOD_INTERCEPT_CLASS; } + @Override + public boolean isOverrideArgs() { + return false; + } + }, + + new InstanceMethodsInterceptPoint() { + @Override + public ElementMatcher getMethodsMatcher() { + return named("sync").or(named("syncAndReturnAll")).or(named("discard")); + } + + @Override + public String getMethodsInterceptor() { + return JEDIS_METHOD_INTERCEPT_CLASS; + } + @Override public boolean isOverrideArgs() { return false; diff --git a/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-2.x-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v3/define/TransactionConstructorInstrumentation.java b/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-2.x-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v3/define/TransactionConstructorInstrumentation.java index b6dad2c25..1db2a774b 100644 --- a/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-2.x-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v3/define/TransactionConstructorInstrumentation.java +++ b/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-2.x-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v3/define/TransactionConstructorInstrumentation.java @@ -23,6 +23,7 @@ import org.apache.skywalking.apm.agent.core.plugin.interceptor.ConstructorInterc import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint; import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch; +import static net.bytebuddy.matcher.ElementMatchers.named; import static org.apache.skywalking.apm.agent.core.plugin.bytebuddy.ArgumentTypeNameMatch.takesArgumentWithType; import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName; @@ -30,6 +31,7 @@ public class TransactionConstructorInstrumentation extends AbstractWitnessInstru private static final String ENHANCE_CLASS = "redis.clients.jedis.Transaction"; private static final String TRANSACTION_CONSTRUCTION_INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.jedis.v3.TransactionConstructorInterceptor"; + private static final String JEDIS_METHOD_INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.jedis.v3.JedisMethodInterceptor"; private static final String ARGUMENT_TYPE_NAME = "redis.clients.jedis.Client"; @Override @@ -56,6 +58,23 @@ public class TransactionConstructorInstrumentation extends AbstractWitnessInstru @Override public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() { - return new InstanceMethodsInterceptPoint[0]; + return new InstanceMethodsInterceptPoint[]{ + new InstanceMethodsInterceptPoint() { + @Override + public ElementMatcher getMethodsMatcher() { + return named("exec").or(named("execGetResponse")).or(named("discard")); + } + + @Override + public String getMethodsInterceptor() { + return JEDIS_METHOD_INTERCEPT_CLASS; + } + + @Override + public boolean isOverrideArgs() { + return false; + } + } + }; } } diff --git a/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-2.x-3.x-plugin/src/main/resources/skywalking-plugin.def b/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-2.x-3.x-plugin/src/main/resources/skywalking-plugin.def index 84476efe8..c7566ce07 100644 --- a/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-2.x-3.x-plugin/src/main/resources/skywalking-plugin.def +++ b/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-2.x-3.x-plugin/src/main/resources/skywalking-plugin.def @@ -17,8 +17,6 @@ jedis-2.x-3.x=org.apache.skywalking.apm.plugin.jedis.v3.define.JedisClusterInstrumentation jedis-2.x-3.x=org.apache.skywalking.apm.plugin.jedis.v3.define.JedisInstrumentation jedis-2.x-3.x=org.apache.skywalking.apm.plugin.jedis.v3.define.PipelineInstrumentation -jedis-2.x-3.x=org.apache.skywalking.apm.plugin.jedis.v3.define.PipelineBaseInstrumentation -jedis-2.x-3.x=org.apache.skywalking.apm.plugin.jedis.v3.define.MultiKeyPipelineBaseInstrumentation jedis-2.x-3.x=org.apache.skywalking.apm.plugin.jedis.v3.define.TransactionConstructorInstrumentation diff --git a/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/ConnectionSendCmdInterceptor.java b/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/JedisConstructorInterceptor.java similarity index 50% rename from apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/ConnectionSendCmdInterceptor.java rename to apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/JedisConstructorInterceptor.java index 447ebdfd7..b318317f8 100644 --- a/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/ConnectionSendCmdInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/JedisConstructorInterceptor.java @@ -17,16 +17,22 @@ package org.apache.skywalking.apm.plugin.jedis.v4; -import redis.clients.jedis.CommandArguments; -import redis.clients.jedis.args.Rawable; - -import java.util.Iterator; - -public class ConnectionSendCmdInterceptor extends AbstractConnectionInterceptor { +import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance; +import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceConstructorInterceptor; +import redis.clients.jedis.Connection; +import redis.clients.jedis.Jedis; +public class JedisConstructorInterceptor implements InstanceConstructorInterceptor { @Override - protected Iterator getCommands(Object[] allArguments) { - CommandArguments commandArguments = (CommandArguments) allArguments[0]; - return commandArguments.iterator(); + public void onConstruct(EnhancedInstance objInst, Object[] allArguments) throws Throwable { + Connection connection = null; + if (allArguments[0] instanceof Jedis) { + connection = ((Jedis) allArguments[0]).getConnection(); + } else if (allArguments[0] instanceof Connection) { + connection = (Connection) allArguments[0]; + } + if (connection instanceof EnhancedInstance) { + objInst.setSkyWalkingDynamicField(((EnhancedInstance) connection).getSkyWalkingDynamicField()); + } } -} +} \ No newline at end of file diff --git a/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/JedisMethodInterceptor.java b/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/JedisMethodInterceptor.java new file mode 100644 index 000000000..26175616e --- /dev/null +++ b/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/JedisMethodInterceptor.java @@ -0,0 +1,57 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF 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 org.apache.skywalking.apm.plugin.jedis.v4; + +import org.apache.skywalking.apm.agent.core.context.ContextManager; +import org.apache.skywalking.apm.agent.core.context.tag.Tags; +import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan; +import org.apache.skywalking.apm.agent.core.context.trace.SpanLayer; +import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance; +import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor; +import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult; +import org.apache.skywalking.apm.network.trace.component.ComponentsDefine; + +import java.lang.reflect.Method; + +public class JedisMethodInterceptor implements InstanceMethodsAroundInterceptor { + + @Override + public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, + MethodInterceptResult result) throws Throwable { + String peer = String.valueOf(objInst.getSkyWalkingDynamicField()); + AbstractSpan span = ContextManager.createExitSpan("Jedis/" + method.getName(), peer); + span.setComponent(ComponentsDefine.JEDIS); + SpanLayer.asCache(span); + Tags.CACHE_TYPE.set(span, "Redis"); + Tags.CACHE_CMD.set(span, "BATCH_EXECUTE"); + } + + @Override + public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, + Object ret) throws Throwable { + ContextManager.stopSpan(); + return ret; + } + + @Override + public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments, + Class[] argumentsTypes, Throwable t) { + AbstractSpan span = ContextManager.activeSpan(); + span.log(t); + } +} diff --git a/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/define/ConnectionInstrumentation.java b/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/define/ConnectionInstrumentation.java index 3e23346b7..f43fb8f66 100644 --- a/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/define/ConnectionInstrumentation.java +++ b/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/define/ConnectionInstrumentation.java @@ -33,7 +33,6 @@ public class ConnectionInstrumentation extends AbstractWitnessInstrumentation { private static final String ENHANCE_CLASS = "redis.clients.jedis.Connection"; private static final String CONNECTION_CONSTRUCTOR_INTERCEPTOR = "org.apache.skywalking.apm.plugin.jedis.v4.ConnectionConstructorInterceptor"; private static final String CONNECTION_EXECUTE_INTERCEPTOR = "org.apache.skywalking.apm.plugin.jedis.v4.ConnectionExecuteInterceptor"; - private static final String CONNECTION_SEND_INTERCEPTOR = "org.apache.skywalking.apm.plugin.jedis.v4.ConnectionSendCmdInterceptor"; @Override protected ClassMatch enhanceClass() { @@ -72,22 +71,6 @@ public class ConnectionInstrumentation extends AbstractWitnessInstrumentation { return CONNECTION_EXECUTE_INTERCEPTOR; } - @Override - public boolean isOverrideArgs() { - return false; - } - }, - new InstanceMethodsInterceptPoint() { - @Override - public ElementMatcher getMethodsMatcher() { - return named("sendCommand").and(takesArgumentWithType(0, "redis.clients.jedis.CommandArguments")); - } - - @Override - public String getMethodsInterceptor() { - return CONNECTION_SEND_INTERCEPTOR; - } - @Override public boolean isOverrideArgs() { return false; diff --git a/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-2.x-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v3/define/PipelineBaseInstrumentation.java b/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/define/PipelineInstrumentation.java similarity index 65% rename from apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-2.x-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v3/define/PipelineBaseInstrumentation.java rename to apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/define/PipelineInstrumentation.java index 4bccd225d..d96aa7937 100644 --- a/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-2.x-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v3/define/PipelineBaseInstrumentation.java +++ b/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/define/PipelineInstrumentation.java @@ -15,21 +15,23 @@ * limitations under the License. */ -package org.apache.skywalking.apm.plugin.jedis.v3.define; +package org.apache.skywalking.apm.plugin.jedis.v4.define; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.matcher.ElementMatcher; +import net.bytebuddy.matcher.ElementMatchers; import org.apache.skywalking.apm.agent.core.plugin.interceptor.ConstructorInterceptPoint; import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint; import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch; -import org.apache.skywalking.apm.plugin.jedis.v3.RedisMethodMatch; +import static net.bytebuddy.matcher.ElementMatchers.named; import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName; -public class PipelineBaseInstrumentation extends AbstractWitnessInstrumentation { +public class PipelineInstrumentation extends AbstractWitnessInstrumentation { - private static final String ENHANCE_CLASS = "redis.clients.jedis.PipelineBase"; - private static final String JEDIS_METHOD_INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.jedis.v3.JedisMethodInterceptor"; + private static final String ENHANCE_CLASS = "redis.clients.jedis.Pipeline"; + private static final String JEDIS_CONSTRUCTION_INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.jedis.v4.JedisConstructorInterceptor"; + private static final String JEDIS_METHOD_INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.jedis.v4.JedisMethodInterceptor"; @Override public ClassMatch enhanceClass() { @@ -38,7 +40,20 @@ public class PipelineBaseInstrumentation extends AbstractWitnessInstrumentation @Override public ConstructorInterceptPoint[] getConstructorsInterceptPoints() { - return new ConstructorInterceptPoint[0]; + return new ConstructorInterceptPoint[]{ + new ConstructorInterceptPoint() { + @Override + public ElementMatcher getConstructorMatcher() { + return ElementMatchers.takesArgument(0, named("redis.clients.jedis.Jedis")) + .or(ElementMatchers.takesArgument(0, named("redis.clients.jedis.Connection"))); + } + + @Override + public String getConstructorInterceptor() { + return JEDIS_CONSTRUCTION_INTERCEPT_CLASS; + } + } + }; } @Override @@ -47,7 +62,7 @@ public class PipelineBaseInstrumentation extends AbstractWitnessInstrumentation new InstanceMethodsInterceptPoint() { @Override public ElementMatcher getMethodsMatcher() { - return RedisMethodMatch.INSTANCE.getJedisMethodMatcher(); + return named("sync").or(named("syncAndReturnAll")); } @Override diff --git a/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-2.x-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v3/define/MultiKeyPipelineBaseInstrumentation.java b/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/define/TransactionConstructorInstrumentation.java similarity index 61% rename from apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-2.x-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v3/define/MultiKeyPipelineBaseInstrumentation.java rename to apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/define/TransactionConstructorInstrumentation.java index 5159658a6..e2229b9d4 100644 --- a/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-2.x-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v3/define/MultiKeyPipelineBaseInstrumentation.java +++ b/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/define/TransactionConstructorInstrumentation.java @@ -15,40 +15,54 @@ * limitations under the License. */ -package org.apache.skywalking.apm.plugin.jedis.v3.define; +package org.apache.skywalking.apm.plugin.jedis.v4.define; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.matcher.ElementMatcher; +import net.bytebuddy.matcher.ElementMatchers; import org.apache.skywalking.apm.agent.core.plugin.interceptor.ConstructorInterceptPoint; -import org.apache.skywalking.apm.agent.core.plugin.interceptor.DeclaredInstanceMethodsInterceptPoint; import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint; import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch; -import org.apache.skywalking.apm.plugin.jedis.v3.RedisMethodMatch; +import static net.bytebuddy.matcher.ElementMatchers.named; import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName; -public class MultiKeyPipelineBaseInstrumentation extends AbstractWitnessInstrumentation { +public class TransactionConstructorInstrumentation extends AbstractWitnessInstrumentation { - private static final String ENHANCE_CLASS = "redis.clients.jedis.MultiKeyPipelineBase"; - private static final String JEDIS_METHOD_INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.jedis.v3.JedisMethodInterceptor"; + private static final String ENHANCE_CLASS = "redis.clients.jedis.Transaction"; + private static final String JEDIS_CONSTRUCTION_INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.jedis.v4.JedisConstructorInterceptor"; + private static final String JEDIS_METHOD_INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.jedis.v4.JedisMethodInterceptor"; @Override - public ClassMatch enhanceClass() { + protected ClassMatch enhanceClass() { return byName(ENHANCE_CLASS); } @Override public ConstructorInterceptPoint[] getConstructorsInterceptPoints() { - return new ConstructorInterceptPoint[0]; + return new ConstructorInterceptPoint[]{ + new ConstructorInterceptPoint() { + @Override + public ElementMatcher getConstructorMatcher() { + return ElementMatchers.takesArgument(0, named("redis.clients.jedis.Jedis")) + .or(ElementMatchers.takesArgument(0, named("redis.clients.jedis.Connection"))); + } + + @Override + public String getConstructorInterceptor() { + return JEDIS_CONSTRUCTION_INTERCEPT_CLASS; + } + } + }; } @Override public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() { - return new InstanceMethodsInterceptPoint[] { - new DeclaredInstanceMethodsInterceptPoint() { + return new InstanceMethodsInterceptPoint[]{ + new InstanceMethodsInterceptPoint() { @Override public ElementMatcher getMethodsMatcher() { - return RedisMethodMatch.INSTANCE.getJedisMethodMatcher(); + return named("exec").or(named("discard")); } @Override diff --git a/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/resources/skywalking-plugin.def b/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/resources/skywalking-plugin.def index 67fe1ca04..a226a135b 100644 --- a/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/resources/skywalking-plugin.def +++ b/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/resources/skywalking-plugin.def @@ -14,4 +14,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -jedis-4.x=org.apache.skywalking.apm.plugin.jedis.v4.define.ConnectionInstrumentation \ No newline at end of file +jedis-4.x=org.apache.skywalking.apm.plugin.jedis.v4.define.ConnectionInstrumentation +jedis-4.x=org.apache.skywalking.apm.plugin.jedis.v4.define.PipelineInstrumentation +jedis-4.x=org.apache.skywalking.apm.plugin.jedis.v4.define.TransactionConstructorInstrumentation \ No newline at end of file diff --git a/test/plugin/scenarios/jedis-2.x-scenario/config/expectedData.yaml b/test/plugin/scenarios/jedis-2.x-scenario/config/expectedData.yaml index 1543775d2..3ee66ae31 100644 --- a/test/plugin/scenarios/jedis-2.x-scenario/config/expectedData.yaml +++ b/test/plugin/scenarios/jedis-2.x-scenario/config/expectedData.yaml @@ -86,7 +86,7 @@ segmentItems: - {key: cache.cmd, value: del} - {key: cache.key, value: a} - {key: cache.op, value: write} - - operationName: Jedis/hset + - operationName: Jedis/syncAndReturnAll operationId: 0 parentSpanId: 0 spanId: 5 @@ -100,10 +100,8 @@ segmentItems: skipAnalysis: false tags: - {key: cache.type, value: Redis} - - {key: cache.cmd, value: hset} - - {key: cache.key, value: a} - - {key: cache.op, value: write} - - operationName: Jedis/hget + - {key: cache.cmd, value: BATCH_EXECUTE} + - operationName: Jedis/discard operationId: 0 parentSpanId: 0 spanId: 6 @@ -117,10 +115,8 @@ segmentItems: skipAnalysis: false tags: - {key: cache.type, value: Redis} - - {key: cache.cmd, value: hget} - - {key: cache.key, value: a} - - {key: cache.op, value: read} - - operationName: Jedis/hdel + - {key: cache.cmd, value: BATCH_EXECUTE} + - operationName: Jedis/exec operationId: 0 parentSpanId: 0 spanId: 7 @@ -134,10 +130,8 @@ segmentItems: skipAnalysis: false tags: - {key: cache.type, value: Redis} - - {key: cache.cmd, value: hdel} - - {key: cache.key, value: a} - - {key: cache.op, value: write} - - operationName: Jedis/set + - {key: cache.cmd, value: BATCH_EXECUTE} + - operationName: Jedis/discard operationId: 0 parentSpanId: 0 spanId: 8 @@ -151,25 +145,7 @@ segmentItems: skipAnalysis: false tags: - {key: cache.type, value: Redis} - - {key: cache.cmd, value: set} - - {key: cache.key, value: key} - - {key: cache.op, value: write} - - operationName: Jedis/expire - operationId: 0 - parentSpanId: 0 - spanId: 9 - spanLayer: Cache - startTime: gt 0 - endTime: gt 0 - componentId: 30 - isError: false - spanType: Exit - peer: redis-server:6379 - skipAnalysis: false - tags: - - {key: cache.type, value: Redis} - - {key: cache.cmd, value: expire} - - {key: cache.key, value: key} + - {key: cache.cmd, value: BATCH_EXECUTE} - operationName: GET:/jedis-scenario/case/jedis-scenario operationId: 0 parentSpanId: -1 diff --git a/test/plugin/scenarios/jedis-2.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/CaseController.java b/test/plugin/scenarios/jedis-2.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/CaseController.java index 5c07c61a2..90be6c430 100644 --- a/test/plugin/scenarios/jedis-2.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/CaseController.java +++ b/test/plugin/scenarios/jedis-2.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/CaseController.java @@ -46,10 +46,12 @@ public class CaseController { try (RedisPipelineCommandExecutor command = new RedisPipelineCommandExecutor(redisHost, redisPort)) { command.pipelineExecute(); + command.pipelineDiscard(); } try (RedisTransactionCommandExecutor command = new RedisTransactionCommandExecutor(redisHost, redisPort)) { command.multiExecute(); + command.multiDiscard(); } return SUCCESS; diff --git a/test/plugin/scenarios/jedis-2.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisPipelineCommandExecutor.java b/test/plugin/scenarios/jedis-2.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisPipelineCommandExecutor.java index f90a270ef..99fcf791f 100644 --- a/test/plugin/scenarios/jedis-2.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisPipelineCommandExecutor.java +++ b/test/plugin/scenarios/jedis-2.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisPipelineCommandExecutor.java @@ -36,7 +36,22 @@ public class RedisPipelineCommandExecutor implements AutoCloseable { pipeline.syncAndReturnAll(); } + public void pipelineDiscard() { + Pipeline pipeline = jedis.pipelined(); + pipeline.multi(); + pipeline.hset("a", "a", "a"); + pipeline.hget("a", "a"); + pipeline.hdel("a", "a"); + pipeline.discard(); + } + public void close() throws Exception { - jedis.close(); + // In the lower version(2.4.2, 2.5.2) of the test, jedis will close twice. + // Here, catch exceptions to prevent the impact on the test results. + try { + jedis.close(); + } catch (Exception e) { + // ignore + } } } diff --git a/test/plugin/scenarios/jedis-2.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisTransactionCommandExecutor.java b/test/plugin/scenarios/jedis-2.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisTransactionCommandExecutor.java index 10c029ef2..f4ddea599 100644 --- a/test/plugin/scenarios/jedis-2.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisTransactionCommandExecutor.java +++ b/test/plugin/scenarios/jedis-2.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisTransactionCommandExecutor.java @@ -35,6 +35,13 @@ public class RedisTransactionCommandExecutor implements AutoCloseable { pipeline.exec(); } + public void multiDiscard() { + Transaction pipeline = jedis.multi(); + pipeline.set("key", "a"); + pipeline.expire("key", 5); + pipeline.discard(); + } + public void close() throws Exception { jedis.close(); } diff --git a/test/plugin/scenarios/jedis-3.1.x-plus-scenario/config/expectedData.yaml b/test/plugin/scenarios/jedis-3.1.x-plus-scenario/config/expectedData.yaml index ab5696b47..2c3ae1b4e 100644 --- a/test/plugin/scenarios/jedis-3.1.x-plus-scenario/config/expectedData.yaml +++ b/test/plugin/scenarios/jedis-3.1.x-plus-scenario/config/expectedData.yaml @@ -87,7 +87,7 @@ segmentItems: - {key: cache.cmd, value: del} - {key: cache.key, value: a} - {key: cache.op, value: write} - - operationName: Jedis/hset + - operationName: Jedis/syncAndReturnAll operationId: 0 parentSpanId: 0 spanId: 5 @@ -101,10 +101,8 @@ segmentItems: skipAnalysis: false tags: - {key: cache.type, value: Redis} - - {key: cache.cmd, value: hset} - - {key: cache.key, value: a} - - {key: cache.op, value: write} - - operationName: Jedis/hget + - {key: cache.cmd, value: BATCH_EXECUTE} + - operationName: Jedis/discard operationId: 0 parentSpanId: 0 spanId: 6 @@ -117,11 +115,9 @@ segmentItems: peer: redis-server:6379 skipAnalysis: false tags: - - {key: cache.type, value: Redis} - - {key: cache.cmd, value: hget} - - {key: cache.key, value: a} - - {key: cache.op, value: read} - - operationName: Jedis/hdel + - { key: cache.type, value: Redis } + - { key: cache.cmd, value: BATCH_EXECUTE } + - operationName: Jedis/exec operationId: 0 parentSpanId: 0 spanId: 7 @@ -135,10 +131,8 @@ segmentItems: skipAnalysis: false tags: - {key: cache.type, value: Redis} - - {key: cache.cmd, value: hdel} - - {key: cache.key, value: a} - - {key: cache.op, value: write} - - operationName: Jedis/set + - {key: cache.cmd, value: BATCH_EXECUTE} + - operationName: Jedis/discard operationId: 0 parentSpanId: 0 spanId: 8 @@ -151,30 +145,12 @@ segmentItems: peer: redis-server:6379 skipAnalysis: false tags: - - {key: cache.type, value: Redis} - - {key: cache.cmd, value: set} - - {key: cache.key, value: key} - - {key: cache.op, value: write} - - operationName: Jedis/expire - operationId: 0 - parentSpanId: 0 - spanId: 9 - spanLayer: Cache - startTime: gt 0 - endTime: gt 0 - componentId: 30 - isError: false - spanType: Exit - peer: redis-server:6379 - skipAnalysis: false - tags: - - {key: cache.type, value: Redis} - - {key: cache.cmd, value: expire} - - {key: cache.key, value: key} + - { key: cache.type, value: Redis } + - { key: cache.cmd, value: BATCH_EXECUTE } - operationName: Jedis/xadd operationId: 0 parentSpanId: 0 - spanId: 10 + spanId: 9 spanLayer: Cache startTime: gt 0 endTime: gt 0 @@ -191,7 +167,7 @@ segmentItems: - operationName: Jedis/xread operationId: 0 parentSpanId: 0 - spanId: 11 + spanId: 10 spanLayer: Cache startTime: gt 0 endTime: gt 0 @@ -207,7 +183,7 @@ segmentItems: - operationName: Jedis/xdel operationId: 0 parentSpanId: 0 - spanId: 12 + spanId: 11 spanLayer: Cache startTime: gt 0 endTime: gt 0 @@ -236,4 +212,4 @@ segmentItems: tags: - {key: url, value: 'http://localhost:8080/jedis-scenario/case/jedis-scenario'} - {key: http.method, value: GET} - - {key: http.status_code, value: '200'} \ No newline at end of file + - {key: http.status_code, value: '200'} diff --git a/test/plugin/scenarios/jedis-3.1.x-plus-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/CaseController.java b/test/plugin/scenarios/jedis-3.1.x-plus-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/CaseController.java index 4fd97c037..a831a90c2 100644 --- a/test/plugin/scenarios/jedis-3.1.x-plus-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/CaseController.java +++ b/test/plugin/scenarios/jedis-3.1.x-plus-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/CaseController.java @@ -46,10 +46,12 @@ public class CaseController { try (RedisPipelineCommandExecutor command = new RedisPipelineCommandExecutor(redisHost, redisPort)) { command.pipelineExecute(); + command.pipelineDiscard(); } try (RedisTransactionCommandExecutor command = new RedisTransactionCommandExecutor(redisHost, redisPort)) { command.multiExecute(); + command.multiDiscard(); } try (RedisStreamCommandExecutor c = new RedisStreamCommandExecutor(redisHost, redisPort)) { c.exec(); diff --git a/test/plugin/scenarios/jedis-3.1.x-plus-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisPipelineCommandExecutor.java b/test/plugin/scenarios/jedis-3.1.x-plus-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisPipelineCommandExecutor.java index f90a270ef..1ac97e4a0 100644 --- a/test/plugin/scenarios/jedis-3.1.x-plus-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisPipelineCommandExecutor.java +++ b/test/plugin/scenarios/jedis-3.1.x-plus-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisPipelineCommandExecutor.java @@ -36,6 +36,15 @@ public class RedisPipelineCommandExecutor implements AutoCloseable { pipeline.syncAndReturnAll(); } + public void pipelineDiscard() { + Pipeline pipeline = jedis.pipelined(); + pipeline.multi(); + pipeline.hset("a", "a", "a"); + pipeline.hget("a", "a"); + pipeline.hdel("a", "a"); + pipeline.discard(); + } + public void close() throws Exception { jedis.close(); } diff --git a/test/plugin/scenarios/jedis-3.1.x-plus-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisTransactionCommandExecutor.java b/test/plugin/scenarios/jedis-3.1.x-plus-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisTransactionCommandExecutor.java index 10c029ef2..f4ddea599 100644 --- a/test/plugin/scenarios/jedis-3.1.x-plus-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisTransactionCommandExecutor.java +++ b/test/plugin/scenarios/jedis-3.1.x-plus-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisTransactionCommandExecutor.java @@ -35,6 +35,13 @@ public class RedisTransactionCommandExecutor implements AutoCloseable { pipeline.exec(); } + public void multiDiscard() { + Transaction pipeline = jedis.multi(); + pipeline.set("key", "a"); + pipeline.expire("key", 5); + pipeline.discard(); + } + public void close() throws Exception { jedis.close(); } diff --git a/test/plugin/scenarios/jedis-4.x-scenario/config/expectedData.yaml b/test/plugin/scenarios/jedis-4.x-scenario/config/expectedData.yaml index 281a0fcb3..6be161a8b 100644 --- a/test/plugin/scenarios/jedis-4.x-scenario/config/expectedData.yaml +++ b/test/plugin/scenarios/jedis-4.x-scenario/config/expectedData.yaml @@ -20,26 +20,10 @@ segmentItems: segments: - segmentId: not null spans: - - operationName: Jedis/echo - operationId: 0 - parentSpanId: 0 - spanId: 1 - spanLayer: Cache - startTime: gt 0 - endTime: gt 0 - componentId: 30 - isError: false - spanType: Exit - peer: redis-server:6379 - skipAnalysis: false - tags: - - {key: cache.key, value: Test} - - {key: cache.cmd, value: echo} - - {key: cache.type, value: Redis} - operationName: Jedis/set operationId: 0 parentSpanId: 0 - spanId: 2 + spanId: 1 spanLayer: Cache startTime: gt 0 endTime: gt 0 @@ -56,7 +40,7 @@ segmentItems: - operationName: Jedis/get operationId: 0 parentSpanId: 0 - spanId: 3 + spanId: 2 spanLayer: Cache startTime: gt 0 endTime: gt 0 @@ -73,7 +57,7 @@ segmentItems: - operationName: Jedis/del operationId: 0 parentSpanId: 0 - spanId: 4 + spanId: 3 spanLayer: Cache startTime: gt 0 endTime: gt 0 @@ -87,7 +71,22 @@ segmentItems: - {key: cache.cmd, value: del} - {key: cache.type, value: Redis} - {key: cache.op, value: write} - - operationName: Jedis/hset + - operationName: Jedis/syncAndReturnAll + operationId: 0 + parentSpanId: 0 + spanId: 4 + spanLayer: Cache + startTime: gt 0 + endTime: gt 0 + componentId: 30 + isError: false + spanType: Exit + peer: redis-server:6379 + skipAnalysis: false + tags: + - {key: cache.type, value: Redis} + - {key: cache.cmd, value: BATCH_EXECUTE} + - operationName: Jedis/exec operationId: 0 parentSpanId: 0 spanId: 5 @@ -100,11 +99,9 @@ segmentItems: peer: redis-server:6379 skipAnalysis: false tags: - - {key: cache.key, value: a} - - {key: cache.cmd, value: hset} - {key: cache.type, value: Redis} - - {key: cache.op, value: write} - - operationName: Jedis/hget + - {key: cache.cmd, value: BATCH_EXECUTE} + - operationName: Jedis/discard operationId: 0 parentSpanId: 0 spanId: 6 @@ -117,94 +114,12 @@ segmentItems: peer: redis-server:6379 skipAnalysis: false tags: - - {key: cache.key, value: a} - - {key: cache.cmd, value: hget} - - {key: cache.type, value: Redis} - - {key: cache.op, value: read} - - operationName: Jedis/hdel - operationId: 0 - parentSpanId: 0 - spanId: 7 - spanLayer: Cache - startTime: gt 0 - endTime: gt 0 - componentId: 30 - isError: false - spanType: Exit - peer: redis-server:6379 - skipAnalysis: false - tags: - - {key: cache.key, value: a} - - {key: cache.cmd, value: hdel} - - {key: cache.type, value: Redis} - - {key: cache.op, value: write} - - operationName: Jedis/multi - operationId: 0 - parentSpanId: 0 - spanId: 8 - spanLayer: Cache - startTime: gt 0 - endTime: gt 0 - componentId: 30 - isError: false - spanType: Exit - peer: redis-server:6379 - skipAnalysis: false - tags: - - {key: cache.cmd, value: multi} - - {key: cache.type, value: Redis} - - operationName: Jedis/set - operationId: 0 - parentSpanId: 0 - spanId: 9 - spanLayer: Cache - startTime: gt 0 - endTime: gt 0 - componentId: 30 - isError: false - spanType: Exit - peer: redis-server:6379 - skipAnalysis: false - tags: - - {key: cache.key, value: key} - - {key: cache.cmd, value: set} - - {key: cache.type, value: Redis} - - {key: cache.op, value: write} - - operationName: Jedis/expire - operationId: 0 - parentSpanId: 0 - spanId: 10 - spanLayer: Cache - startTime: gt 0 - endTime: gt 0 - componentId: 30 - isError: false - spanType: Exit - peer: redis-server:6379 - skipAnalysis: false - tags: - - {key: cache.key, value: key} - - {key: cache.cmd, value: expire} - - {key: cache.type, value: Redis} - - operationName: Jedis/exec - operationId: 0 - parentSpanId: 0 - spanId: 11 - spanLayer: Cache - startTime: gt 0 - endTime: gt 0 - componentId: 30 - isError: false - spanType: Exit - peer: redis-server:6379 - skipAnalysis: false - tags: - - {key: cache.cmd, value: exec} - {key: cache.type, value: Redis} + - {key: cache.cmd, value: BATCH_EXECUTE} - operationName: Jedis/xadd operationId: 0 parentSpanId: 0 - spanId: 12 + spanId: 7 spanLayer: Cache startTime: gt 0 endTime: gt 0 @@ -221,7 +136,7 @@ segmentItems: - operationName: Jedis/xread operationId: 0 parentSpanId: 0 - spanId: 13 + spanId: 8 spanLayer: Cache startTime: gt 0 endTime: gt 0 @@ -237,7 +152,7 @@ segmentItems: - operationName: Jedis/xdel operationId: 0 parentSpanId: 0 - spanId: 14 + spanId: 9 spanLayer: Cache startTime: gt 0 endTime: gt 0 diff --git a/test/plugin/scenarios/jedis-4.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/CaseController.java b/test/plugin/scenarios/jedis-4.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/CaseController.java index 8ae473c33..a7f570960 100644 --- a/test/plugin/scenarios/jedis-4.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/CaseController.java +++ b/test/plugin/scenarios/jedis-4.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/CaseController.java @@ -50,6 +50,7 @@ public class CaseController { try (RedisTransactionCommandExecutor command = new RedisTransactionCommandExecutor(redisHost, redisPort)) { command.multiExecute(); + command.multiDiscard(); } try (RedisStreamCommandExecutor executor = new RedisStreamCommandExecutor(redisHost, redisPort)) { executor.exec(); diff --git a/test/plugin/scenarios/jedis-4.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisTransactionCommandExecutor.java b/test/plugin/scenarios/jedis-4.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisTransactionCommandExecutor.java index 10c029ef2..f4ddea599 100644 --- a/test/plugin/scenarios/jedis-4.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisTransactionCommandExecutor.java +++ b/test/plugin/scenarios/jedis-4.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisTransactionCommandExecutor.java @@ -35,6 +35,13 @@ public class RedisTransactionCommandExecutor implements AutoCloseable { pipeline.exec(); } + public void multiDiscard() { + Transaction pipeline = jedis.multi(); + pipeline.set("key", "a"); + pipeline.expire("key", 5); + pipeline.discard(); + } + public void close() throws Exception { jedis.close(); }