polish code

apache · Jun 23, 2023 · 731d85c · 731d85c
1 parent 3d9c851
commit 731d85c
Show file tree

Hide file tree

Showing 16 changed files with 163 additions and 187 deletions.
diff --git a/...apache/skywalking/apm/agent/core/plugin/interceptor/enhance/ClassEnhancePluginDefine.java b/...apache/skywalking/apm/agent/core/plugin/interceptor/enhance/ClassEnhancePluginDefine.java
@@ -78,7 +78,7 @@ protected DynamicType.Builder<?> enhanceInstance(TypeDescription typeDescription
  if (instanceMethodsInterceptPoints != null && instanceMethodsInterceptPoints.length > 0) {
  existedMethodsInterceptPoints = true;
  }
- DelegateNamingResolver delegateNamingResolver = DelegateNamingResolver.get(typeDescription.getTypeName(), this);
+ DelegateNamingResolver delegateNamingResolver = new DelegateNamingResolver(typeDescription.getTypeName(), this);
 
  /**
  * nothing need to be enhanced in class instance, maybe need enhance static methods.
@@ -184,7 +184,7 @@ protected DynamicType.Builder<?> enhanceClass(TypeDescription typeDescription, D
  if (staticMethodsInterceptPoints == null || staticMethodsInterceptPoints.length == 0) {
  return newClassBuilder;
  }
- DelegateNamingResolver delegateNamingResolver = DelegateNamingResolver.get(typeDescription.getTypeName(), this);
+ DelegateNamingResolver delegateNamingResolver = new DelegateNamingResolver(typeDescription.getTypeName(), this);
 
  for (StaticMethodsInterceptPoint staticMethodsInterceptPoint : staticMethodsInterceptPoints) {
  String interceptor = staticMethodsInterceptPoint.getMethodsInterceptor();

diff --git a/...g/apache/skywalking/apm/agent/core/plugin/interceptor/enhance/DelegateNamingResolver.java b/...g/apache/skywalking/apm/agent/core/plugin/interceptor/enhance/DelegateNamingResolver.java
@@ -34,16 +34,12 @@
  */
 public class DelegateNamingResolver {
  private static final String PREFIX = "delegate$";
- private final String className;
- private final int identifier;
  private final String fieldNamePrefix;
 
- public DelegateNamingResolver(String className, int identifier) {
- this.className = className;
- this.identifier = identifier;
+ public DelegateNamingResolver(String className, AbstractClassEnhancePluginDefine pluginDefine) {
  // Interceptor delegate field name pattern: <name_trait>$delegate$<class_name_hash>$<plugin_define_hash>$<intercept_point_hash>
  // something like: InstMethodsInter sw$delegate$td03673$sib0lj0$5n874b1;
- this.fieldNamePrefix = Constants.NAME_TRAIT + PREFIX + RandomString.hashOf(className.hashCode()) + "$" + RandomString.hashOf(identifier) + "$";
+ this.fieldNamePrefix = Constants.NAME_TRAIT + PREFIX + RandomString.hashOf(className.hashCode()) + "$" + RandomString.hashOf(pluginDefine.hashCode()) + "$";
  }
 
  public String resolve(Object interceptPoint) {
@@ -72,8 +68,4 @@ private int computeHashCode(Object interceptPoint) {
  return interceptPoint.hashCode();
  }
 
- public static DelegateNamingResolver get(String className, AbstractClassEnhancePluginDefine pluginDefine) {
- return new DelegateNamingResolver(className, pluginDefine.hashCode());
- }
-
 }
diff --git a/...e/skywalking/apm/agent/core/plugin/interceptor/enhance/v2/ClassEnhancePluginDefineV2.java b/...e/skywalking/apm/agent/core/plugin/interceptor/enhance/v2/ClassEnhancePluginDefineV2.java
@@ -66,7 +66,7 @@ protected DynamicType.Builder<?> enhanceClass(TypeDescription typeDescription,
  if (staticMethodsInterceptV2Points == null || staticMethodsInterceptV2Points.length == 0) {
  return newClassBuilder;
  }
- DelegateNamingResolver delegateNamingResolver = DelegateNamingResolver.get(typeDescription.getTypeName(), this);
+ DelegateNamingResolver delegateNamingResolver = new DelegateNamingResolver(typeDescription.getTypeName(), this);
 
  for (StaticMethodsInterceptV2Point staticMethodsInterceptV2Point : staticMethodsInterceptV2Points) {
  String interceptor = staticMethodsInterceptV2Point.getMethodsInterceptorV2();
@@ -115,7 +115,7 @@ protected DynamicType.Builder<?> enhanceInstance(TypeDescription typeDescription
  ConstructorInterceptPoint[] constructorInterceptPoints = getConstructorsInterceptPoints();
  InstanceMethodsInterceptV2Point[] instanceMethodsInterceptV2Points = getInstanceMethodsInterceptV2Points();
  String enhanceOriginClassName = typeDescription.getTypeName();
- DelegateNamingResolver fieldNamingResolver = DelegateNamingResolver.get(typeDescription.getTypeName(), this);
+ DelegateNamingResolver fieldNamingResolver = new DelegateNamingResolver(typeDescription.getTypeName(), this);
 
  boolean existedConstructorInterceptPoint = false;
  if (constructorInterceptPoints != null && constructorInterceptPoints.length > 0) {

diff --git a/...ytebuddy-patch/src/test/java/org/apache/skywalking/apm/agent/bytebuddy/EnhanceHelper.java b/...ytebuddy-patch/src/test/java/org/apache/skywalking/apm/agent/bytebuddy/EnhanceHelper.java
@@ -24,11 +24,11 @@
 
 public class EnhanceHelper {
 
- private static List<String> INTERCEPTORS = new ArrayList<>();
- private static List<Map.Entry<String, Throwable>> ERRORS = new ArrayList<>();
+ private static final List<String> INTERCEPTORS = new ArrayList<>();
+ private static final List<Map.Entry<String, Throwable>> ERRORS = new ArrayList<>();
 
  public static void onError(String message, Throwable error) {
- ERRORS.add(new MapEntry(message, error));
+ ERRORS.add(new MapEntry<>(message, error));
  }
 
  public static void addInterceptor(String interceptor) {
@@ -49,7 +49,7 @@ public static void clear() {
  }
 
  private static class MapEntry<T, P> implements Map.Entry<T, P> {
- private T key;
+ private final T key;
  private P value;
 
  public MapEntry(T key, P value) {

diff --git a/apm-sniffer/bytebuddy-patch/src/test/java/org/apache/skywalking/apm/agent/bytebuddy/Log.java b/apm-sniffer/bytebuddy-patch/src/test/java/org/apache/skywalking/apm/agent/bytebuddy/Log.java
@@ -18,25 +18,42 @@
 
 package org.apache.skywalking.apm.agent.bytebuddy;
 
+import java.io.ByteArrayOutputStream;
 import java.io.PrintStream;
 
 public class Log {
- private static PrintStream PRINT = System.out;
- private static PrintStream ERROR_PRINT = System.err;
+ private static ByteArrayOutputStream OUTPUT;
+ private static ByteArrayOutputStream ERR_OUTPUT;
+ private static PrintStream PRINT;
+ private static PrintStream ERROR_PRINT;
 
- public static void info(String msg) {
+ public static void info(String msg, Object... args) {
+ msg = formatLog(msg, args);
  PRINT.println(msg);
  }
 
- public static void info(int msg) {
- PRINT.println(msg);
+ public static void error(String msg, Object... args) {
+ msg = formatLog(msg, args);
+ ERROR_PRINT.println(msg);
  }
 
- public static void info(Object obj) {
- PRINT.println(obj);
+ private static String formatLog(String msg, Object[] args) {
+ msg = msg.replaceAll("\\{}", "%s");
+ msg = String.format(msg, args);
+ return msg;
  }
 
- public static void error(String msg) {
- ERROR_PRINT.println(msg);
+ public static void clear() {
+ OUTPUT = new ByteArrayOutputStream(128);
+ ERR_OUTPUT = new ByteArrayOutputStream(128);
+ PRINT = new PrintStream(OUTPUT, true);
+ ERROR_PRINT = new PrintStream(ERR_OUTPUT, true);
+ }
+
+ public static void printToConsole() {
+ PrintStream out = System.out;
+ PrintStream err = System.err;
+ out.println(OUTPUT.toString());
+ err.println(ERR_OUTPUT.toString());
  }
 }
diff --git a/...ytebuddy/case1/AbstractInterceptTest.java → ...ytebuddy/cases/AbstractInterceptTest.java b/...ytebuddy/case1/AbstractInterceptTest.java → ...ytebuddy/cases/AbstractInterceptTest.java
@@ -16,7 +16,7 @@
  *
  */
 
-package org.apache.skywalking.apm.agent.bytebuddy.case1;
+package org.apache.skywalking.apm.agent.bytebuddy.cases;
 
 import net.bytebuddy.ByteBuddy;
 import net.bytebuddy.agent.ByteBuddyAgent;
@@ -37,6 +37,10 @@
 import org.apache.skywalking.apm.agent.bytebuddy.SWClassFileLocator;
 import org.apache.skywalking.apm.agent.bytebuddy.biz.BizFoo;
 import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.rules.TestWatcher;
+import org.junit.runner.Description;
 import org.objectweb.asm.ClassReader;
 import org.objectweb.asm.util.TraceClassVisitor;
 
@@ -59,12 +63,27 @@ public class AbstractInterceptTest {
  protected List<String> nameTraits = Arrays.asList("sw2023", "sw2024");
  protected boolean deleteDuplicatedFields = false;
 
+ @BeforeClass
+ public static void setUp() {
+ EnhanceHelper.clear();
+ Log.clear();
+ }
+
+ @Rule
+ public TestWatcher watcher = new TestWatcher() {
+ @Override
+ protected void failed(Throwable e, Description description) {
+ Log.error("Test failure: {}.{}(), error: {}", description.getTestClass(), description.getMethodName(), e);
+ Log.printToConsole();
+ }
+ };
+
  protected static void callBizFoo(int round) {
  Log.info("-------------");
  Log.info("callBizFoo: " + round);
  // load target class
  int intResult = new BizFoo().sayHello(BASE_INT_VALUE);
- Log.info(intResult);
+ Log.info("result: " + intResult);
 
  String result = new BizFoo("Smith").sayHello("Joe");
  Log.info(result);
@@ -87,6 +106,10 @@ protected static void checkConstructorInterceptor(String className, int round) {
  Log.info("Found interceptor: " + interceptorName);
  }
 
+ protected static void checkErrors() {
+ Assert.assertEquals("Error occurred in transform", 0, EnhanceHelper.getErrors().size());
+ }
+
  protected void installMethodInterceptor(String className, String methodName, int round) {
  this.installMethodInterceptorWithMethodDelegation(className, methodName, round);
  }
@@ -179,7 +202,9 @@ private static AgentBuilder.Listener.Adapter getListener(String interceptorClass
  return new AgentBuilder.Listener.Adapter() {
  @Override
  public void onError(String typeName, ClassLoader classLoader, JavaModule module, boolean loaded, Throwable throwable) {
- System.err.println(String.format("Transform Error: interceptorClassName: %s, typeName: %s, classLoader: %s, module: %s, loaded: %s", interceptorClassName, typeName, classLoader, module, loaded));
+ String msg = String.format("Transform Error: interceptorClassName: %s, typeName: %s, classLoader: %s, module: %s, loaded: %s", interceptorClassName, typeName, classLoader, module, loaded);
+ EnhanceHelper.onError(msg, throwable);
+ System.err.println(msg);
  throwable.printStackTrace();
  }
  };

diff --git a/...ebuddy/case1/AbstractRetransformTest.java → ...ebuddy/cases/AbstractReTransformTest.java b/...ebuddy/case1/AbstractRetransformTest.java → ...ebuddy/cases/AbstractReTransformTest.java
@@ -16,21 +16,20 @@
  *
  */
 
-package org.apache.skywalking.apm.agent.bytebuddy.case1;
+package org.apache.skywalking.apm.agent.bytebuddy.cases;
 
 import org.apache.skywalking.apm.agent.bytebuddy.Log;
 
 import java.lang.instrument.ClassFileTransformer;
 import java.lang.instrument.IllegalClassFormatException;
 import java.lang.instrument.Instrumentation;
-import java.lang.instrument.UnmodifiableClassException;
 import java.security.ProtectionDomain;
 
-public class AbstractRetransformTest extends AbstractInterceptTest {
+public class AbstractReTransformTest extends AbstractInterceptTest {
 
- protected static void reTransform(Instrumentation instrumentation, Class clazz) throws UnmodifiableClassException {
+ protected static void reTransform(Instrumentation instrumentation, Class clazz) throws Exception {
  Log.info("-------------");
- Log.info("Begin to retransform class: " + clazz.getName() + " ..");
+ Log.info("Begin to re-transform class: " + clazz.getName() + " ..");
  ClassFileTransformer transformer = new ClassFileTransformer() {
  public byte[] transform(ClassLoader loader, String className, Class<?> classBeingRedefined,
  ProtectionDomain protectionDomain, byte[] classfileBuffer) throws IllegalClassFormatException {
@@ -42,10 +41,10 @@ public byte[] transform(ClassLoader loader, String className, Class<?> classBein
  try {
  instrumentation.addTransformer(transformer, true);
  instrumentation.retransformClasses(clazz);
- Log.info("Retransform class " + clazz.getName() + " successful.");
+ Log.info("ReTransform class " + clazz.getName() + " successful.");
  Log.info("-------------");
  } catch (Throwable e) {
- Log.info("Retransform class " + clazz.getName() + " failure: " + e);
+ Log.info("ReTransform class " + clazz.getName() + " failure: " + e);
  Log.info("-------------");
  throw e;
  } finally {

diff --git a/...agent/bytebuddy/case1/Intercept1Test.java → ...agent/bytebuddy/cases/Intercept1Test.java b/...agent/bytebuddy/case1/Intercept1Test.java → ...agent/bytebuddy/cases/Intercept1Test.java
@@ -16,32 +16,27 @@
  *
  */
 
-package org.apache.skywalking.apm.agent.bytebuddy.case1;
+package org.apache.skywalking.apm.agent.bytebuddy.cases;
 
 import net.bytebuddy.agent.ByteBuddyAgent;
 import org.junit.Test;
 
-import java.lang.instrument.UnmodifiableClassException;
-
 public class Intercept1Test extends AbstractInterceptTest {
 
  @Test
- public void test1() throws UnmodifiableClassException {
+ public void test1() {
  ByteBuddyAgent.install();
 
  // install transformer
  installMethodInterceptor(BIZ_FOO_CLASS_NAME, SAY_HELLO_METHOD, 1);
  installConstructorInterceptor(BIZ_FOO_CLASS_NAME, 1);
 
- try {
- callBizFoo(1);
- } catch (Throwable e) {
- e.printStackTrace();
- } finally {
- // check interceptors
- checkMethodInterceptor(SAY_HELLO_METHOD, 1);
- checkConstructorInterceptor(BIZ_FOO_CLASS_NAME, 1);
- }
+ callBizFoo(1);
+
+ // check interceptors
+ checkMethodInterceptor(SAY_HELLO_METHOD, 1);
+ checkConstructorInterceptor(BIZ_FOO_CLASS_NAME, 1);
+ checkErrors();
  }
 }
 
diff --git a/...agent/bytebuddy/case1/Intercept2Test.java → ...agent/bytebuddy/cases/Intercept2Test.java b/...agent/bytebuddy/case1/Intercept2Test.java → ...agent/bytebuddy/cases/Intercept2Test.java
@@ -16,17 +16,15 @@
  *
  */
 
-package org.apache.skywalking.apm.agent.bytebuddy.case1;
+package org.apache.skywalking.apm.agent.bytebuddy.cases;
 
 import net.bytebuddy.agent.ByteBuddyAgent;
 import org.junit.Test;
 
-import java.lang.instrument.UnmodifiableClassException;
-
 public class Intercept2Test extends AbstractInterceptTest {
 
  @Test
- public void test2() throws UnmodifiableClassException {
+ public void test2() {
  ByteBuddyAgent.install();
 
  // install transformer
@@ -35,16 +33,13 @@ public void test2() throws UnmodifiableClassException {
  installConstructorInterceptor(BIZ_FOO_CLASS_NAME, 1);
 
  // load target class
- try {
- callBizFoo(2);
- } catch (Throwable e) {
- e.printStackTrace();
- } finally {
- // check interceptors
- checkMethodInterceptor(SAY_HELLO_METHOD, 1);
- checkMethodInterceptor(SAY_HELLO_METHOD, 2);
- checkConstructorInterceptor(BIZ_FOO_CLASS_NAME, 1);
- }
+ callBizFoo(2);
+
+ // check interceptors
+ checkMethodInterceptor(SAY_HELLO_METHOD, 1);
+ checkMethodInterceptor(SAY_HELLO_METHOD, 2);
+ checkConstructorInterceptor(BIZ_FOO_CLASS_NAME, 1);
+ checkErrors();
  }
 }
 
diff --git a/...agent/bytebuddy/case1/Intercept3Test.java → ...agent/bytebuddy/cases/Intercept3Test.java b/...agent/bytebuddy/case1/Intercept3Test.java → ...agent/bytebuddy/cases/Intercept3Test.java
@@ -16,17 +16,15 @@
  *
  */
 
-package org.apache.skywalking.apm.agent.bytebuddy.case1;
+package org.apache.skywalking.apm.agent.bytebuddy.cases;
 
 import net.bytebuddy.agent.ByteBuddyAgent;
 import org.junit.Test;
 
-import java.lang.instrument.UnmodifiableClassException;
-
 public class Intercept3Test extends AbstractInterceptTest {
 
  @Test
- public void test3() throws UnmodifiableClassException {
+ public void test3() {
  ByteBuddyAgent.install();
 
  // install transformer
@@ -35,17 +33,13 @@ public void test3() throws UnmodifiableClassException {
  installMethodInterceptor(BIZ_FOO_CLASS_NAME, SAY_HELLO_METHOD, 2);
 
  // load target class
- try {
- callBizFoo(2);
- } catch (Throwable e) {
- e.printStackTrace();
- } finally {
- // check interceptors
- checkMethodInterceptor(SAY_HELLO_METHOD, 1);
- checkConstructorInterceptor(BIZ_FOO_CLASS_NAME, 1);
- checkMethodInterceptor(SAY_HELLO_METHOD, 2);
- }
+ callBizFoo(2);
 
+ // check interceptors
+ checkMethodInterceptor(SAY_HELLO_METHOD, 1);
+ checkConstructorInterceptor(BIZ_FOO_CLASS_NAME, 1);
+ checkMethodInterceptor(SAY_HELLO_METHOD, 2);
+ checkErrors();
  }
 
 }