From c17acbe55257a74be0cef2ca02fb2ee19fff83e3 Mon Sep 17 00:00:00 2001 From: Jiajing LU Date: Thu, 28 Oct 2021 09:33:41 +0800 Subject: [PATCH] Support Jedis' Transaction and fix duplicated enhancement (#57) --- CHANGES.md | 2 +- .../v2/TransactionConstructorInterceptor.java | 32 ++++++++++ .../MultiKeyPipelineBaseInstrumentation.java | 3 +- ...TransactionConstructorInstrumentation.java | 63 +++++++++++++++++++ .../src/main/resources/skywalking-plugin.def | 1 + ...TransactionConstructorInterceptorTest.java | 52 +++++++++++++++ .../jedis-scenario/config/expectedData.yaml | 28 +++++++++ .../jedis/controller/CaseController.java | 5 ++ .../RedisTransactionCommandExecutor.java | 41 ++++++++++++ 9 files changed, 225 insertions(+), 2 deletions(-) create mode 100644 apm-sniffer/apm-sdk-plugin/jedis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v2/TransactionConstructorInterceptor.java create mode 100644 apm-sniffer/apm-sdk-plugin/jedis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v2/define/TransactionConstructorInstrumentation.java create mode 100644 apm-sniffer/apm-sdk-plugin/jedis-2.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/jedis/v2/TransactionConstructorInterceptorTest.java create mode 100644 test/plugin/scenarios/jedis-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisTransactionCommandExecutor.java diff --git a/CHANGES.md b/CHANGES.md index 5dbe6bbb5..47b2fef1e 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -5,7 +5,7 @@ Release Notes. 8.9.0 ------------------ - +* Support `Transaction` and fix duplicated methods enhancements for `jedis-2.x` plugin. #### Documentation diff --git a/apm-sniffer/apm-sdk-plugin/jedis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v2/TransactionConstructorInterceptor.java b/apm-sniffer/apm-sdk-plugin/jedis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v2/TransactionConstructorInterceptor.java new file mode 100644 index 000000000..189b02b75 --- /dev/null +++ b/apm-sniffer/apm-sdk-plugin/jedis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v2/TransactionConstructorInterceptor.java @@ -0,0 +1,32 @@ +/* + * 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.v2; + +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.Client; + +public class TransactionConstructorInterceptor implements InstanceConstructorInterceptor { + @Override + public void onConstruct(EnhancedInstance objInst, Object[] allArguments) throws Throwable { + Client client = (Client) allArguments[0]; + + objInst.setSkyWalkingDynamicField(client.getHost() + ":" + client.getPort()); + } +} diff --git a/apm-sniffer/apm-sdk-plugin/jedis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v2/define/MultiKeyPipelineBaseInstrumentation.java b/apm-sniffer/apm-sdk-plugin/jedis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v2/define/MultiKeyPipelineBaseInstrumentation.java index d7c4c008c..de23912de 100644 --- a/apm-sniffer/apm-sdk-plugin/jedis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v2/define/MultiKeyPipelineBaseInstrumentation.java +++ b/apm-sniffer/apm-sdk-plugin/jedis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v2/define/MultiKeyPipelineBaseInstrumentation.java @@ -21,6 +21,7 @@ package org.apache.skywalking.apm.plugin.jedis.v2.define; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.matcher.ElementMatcher; 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.interceptor.enhance.ClassInstanceMethodsEnhancePluginDefine; import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch; @@ -46,7 +47,7 @@ public class MultiKeyPipelineBaseInstrumentation extends ClassInstanceMethodsEnh @Override public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() { return new InstanceMethodsInterceptPoint[] { - new InstanceMethodsInterceptPoint() { + new DeclaredInstanceMethodsInterceptPoint() { @Override public ElementMatcher getMethodsMatcher() { return RedisMethodMatch.INSTANCE.getJedisMethodMatcher(); diff --git a/apm-sniffer/apm-sdk-plugin/jedis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v2/define/TransactionConstructorInstrumentation.java b/apm-sniffer/apm-sdk-plugin/jedis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v2/define/TransactionConstructorInstrumentation.java new file mode 100644 index 000000000..4d57056d9 --- /dev/null +++ b/apm-sniffer/apm-sdk-plugin/jedis-2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v2/define/TransactionConstructorInstrumentation.java @@ -0,0 +1,63 @@ +/* + * 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.v2.define; + +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.matcher.ElementMatcher; +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.interceptor.enhance.ClassInstanceMethodsEnhancePluginDefine; +import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch; + +import static org.apache.skywalking.apm.agent.core.plugin.bytebuddy.ArgumentTypeNameMatch.takesArgumentWithType; +import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName; + +public class TransactionConstructorInstrumentation extends ClassInstanceMethodsEnhancePluginDefine { + + private static final String ENHANCE_CLASS = "redis.clients.jedis.Transaction"; + private static final String TRANSACTION_CONSTRUCTION_INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.jedis.v2.TransactionConstructorInterceptor"; + private static final String ARGUMENT_TYPE_NAME = "redis.clients.jedis.Client"; + + @Override + protected ClassMatch enhanceClass() { + return byName(ENHANCE_CLASS); + } + + @Override + public ConstructorInterceptPoint[] getConstructorsInterceptPoints() { + return new ConstructorInterceptPoint[]{ + new ConstructorInterceptPoint() { + @Override + public ElementMatcher getConstructorMatcher() { + return takesArgumentWithType(0, ARGUMENT_TYPE_NAME); + } + + @Override + public String getConstructorInterceptor() { + return TRANSACTION_CONSTRUCTION_INTERCEPT_CLASS; + } + } + }; + } + + @Override + public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() { + return new InstanceMethodsInterceptPoint[0]; + } +} diff --git a/apm-sniffer/apm-sdk-plugin/jedis-2.x-plugin/src/main/resources/skywalking-plugin.def b/apm-sniffer/apm-sdk-plugin/jedis-2.x-plugin/src/main/resources/skywalking-plugin.def index fcff84cbe..f771f56b2 100644 --- a/apm-sniffer/apm-sdk-plugin/jedis-2.x-plugin/src/main/resources/skywalking-plugin.def +++ b/apm-sniffer/apm-sdk-plugin/jedis-2.x-plugin/src/main/resources/skywalking-plugin.def @@ -19,5 +19,6 @@ jedis-2.x=org.apache.skywalking.apm.plugin.jedis.v2.define.JedisInstrumentation jedis-2.x=org.apache.skywalking.apm.plugin.jedis.v2.define.PipelineInstrumentation jedis-2.x=org.apache.skywalking.apm.plugin.jedis.v2.define.PipelineBaseInstrumentation jedis-2.x=org.apache.skywalking.apm.plugin.jedis.v2.define.MultiKeyPipelineBaseInstrumentation +jedis-2.x=org.apache.skywalking.apm.plugin.jedis.v2.define.TransactionConstructorInstrumentation diff --git a/apm-sniffer/apm-sdk-plugin/jedis-2.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/jedis/v2/TransactionConstructorInterceptorTest.java b/apm-sniffer/apm-sdk-plugin/jedis-2.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/jedis/v2/TransactionConstructorInterceptorTest.java new file mode 100644 index 000000000..2f5f77096 --- /dev/null +++ b/apm-sniffer/apm-sdk-plugin/jedis-2.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/jedis/v2/TransactionConstructorInterceptorTest.java @@ -0,0 +1,52 @@ +/* + * 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.v2; + +import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.powermock.modules.junit4.PowerMockRunner; +import redis.clients.jedis.Client; + +import static org.mockito.Mockito.verify; + +@RunWith(PowerMockRunner.class) +public class TransactionConstructorInterceptorTest { + + private TransactionConstructorInterceptor interceptor; + + @Mock + private EnhancedInstance enhancedInstance; + + @Before + public void setUp() throws Exception { + interceptor = new TransactionConstructorInterceptor(); + } + + @Test + public void onConstruct() throws Throwable { + interceptor.onConstruct(enhancedInstance, new Object[]{ + new Client("127.0.0.1", 6379) + }); + + verify(enhancedInstance).setSkyWalkingDynamicField("127.0.0.1:6379"); + } +} \ No newline at end of file diff --git a/test/plugin/scenarios/jedis-scenario/config/expectedData.yaml b/test/plugin/scenarios/jedis-scenario/config/expectedData.yaml index 8ef5c8aa7..8c83ed879 100644 --- a/test/plugin/scenarios/jedis-scenario/config/expectedData.yaml +++ b/test/plugin/scenarios/jedis-scenario/config/expectedData.yaml @@ -117,6 +117,34 @@ segmentItems: - {key: db.type, value: Redis} - {key: db.statement, value: hdel a} skipAnalysis: 'false' + - operationName: Jedis/set + parentSpanId: 0 + spanId: 8 + spanLayer: Cache + startTime: nq 0 + endTime: nq 0 + componentId: 30 + isError: false + spanType: Exit + peer: redis-server:6379 + tags: + - { key: db.type, value: Redis } + - { key: db.statement, value: set key } + skipAnalysis: 'false' + - operationName: Jedis/expire + parentSpanId: 0 + spanId: 9 + spanLayer: Cache + startTime: nq 0 + endTime: nq 0 + componentId: 30 + isError: false + spanType: Exit + peer: redis-server:6379 + tags: + - { key: db.type, value: Redis } + - { key: db.statement, value: expire key } + skipAnalysis: 'false' - operationName: GET:/jedis-scenario/case/jedis-scenario parentSpanId: -1 spanId: 0 diff --git a/test/plugin/scenarios/jedis-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/CaseController.java b/test/plugin/scenarios/jedis-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/CaseController.java index 53a0f6c79..5c07c61a2 100644 --- a/test/plugin/scenarios/jedis-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/CaseController.java +++ b/test/plugin/scenarios/jedis-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/CaseController.java @@ -47,6 +47,11 @@ public class CaseController { try (RedisPipelineCommandExecutor command = new RedisPipelineCommandExecutor(redisHost, redisPort)) { command.pipelineExecute(); } + + try (RedisTransactionCommandExecutor command = new RedisTransactionCommandExecutor(redisHost, redisPort)) { + command.multiExecute(); + } + return SUCCESS; } diff --git a/test/plugin/scenarios/jedis-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisTransactionCommandExecutor.java b/test/plugin/scenarios/jedis-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisTransactionCommandExecutor.java new file mode 100644 index 000000000..10c029ef2 --- /dev/null +++ b/test/plugin/scenarios/jedis-scenario/src/main/java/org/apache/skywalking/apm/testcase/jedis/controller/RedisTransactionCommandExecutor.java @@ -0,0 +1,41 @@ +/* + * 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.testcase.jedis.controller; + +import redis.clients.jedis.Jedis; +import redis.clients.jedis.Transaction; + +public class RedisTransactionCommandExecutor implements AutoCloseable { + private Jedis jedis; + + public RedisTransactionCommandExecutor(String host, Integer port) { + jedis = new Jedis(host, port); + } + + public void multiExecute() { + Transaction pipeline = jedis.multi(); + pipeline.set("key", "a"); + pipeline.expire("key", 5); + pipeline.exec(); + } + + public void close() throws Exception { + jedis.close(); + } +}