fix parent class enhance issue

CHANGES.md

@@ -11,6 +11,7 @@ Release Notes.
 * Agent kernel services could be not-booted-yet as ServiceManager#INSTANCE#boot executed after agent transfer
   initialization. Delay so11y metrics#build when the services are not ready to avoid MeterService status is not
   initialized.
+* Fix retransform failure when enhancing both parent and child classes.
 
 All issues and pull requests are [here](https://github.com/apache/skywalking/milestone/236?closed=1)
 

apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/plugin/AbstractClassEnhancePluginDefine.java

@@ -51,6 +51,14 @@ public abstract class AbstractClassEnhancePluginDefine {
      * New field name.
      */
     public static final String CONTEXT_ATTR_NAME = "_$EnhancedClassField_ws";
+    /**
+     * Getter method name.
+     */
+    public static final String CONTEXT_GETTER_NAME = "getSkyWalkingDynamicField";
+    /**
+     * Setter method name.
+     */
+    public static final String CONTEXT_SETTER_NAME = "setSkyWalkingDynamicField";
 
     /**
      * Main entrance of enhancing the class.

apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/plugin/interceptor/enhance/ClassEnhancePluginDefine.java

@@ -19,6 +19,7 @@
 package org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance;
 
 import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.description.modifier.Visibility;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.dynamic.DynamicType;
 import net.bytebuddy.implementation.FieldAccessor;
@@ -102,6 +103,9 @@ protected DynamicType.Builder<?> enhanceInstance(TypeDescription typeDescription
                 newClassBuilder = newClassBuilder.defineField(
                     CONTEXT_ATTR_NAME, Object.class, ACC_PRIVATE | ACC_VOLATILE)
                                                  .implement(EnhancedInstance.class)
+                                                 .defineMethod(CONTEXT_GETTER_NAME, Object.class, Visibility.PUBLIC)
+                                                 .intercept(FieldAccessor.ofField(CONTEXT_ATTR_NAME))
+                                                 .defineMethod(CONTEXT_SETTER_NAME, void.class, Visibility.PUBLIC).withParameters(Object.class)
                                                  .intercept(FieldAccessor.ofField(CONTEXT_ATTR_NAME));
                 context.extendObjectCompleted();
             }

apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/plugin/interceptor/enhance/v2/ClassEnhancePluginDefineV2.java

@@ -18,6 +18,7 @@
 package org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.v2;
 
 import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.description.modifier.Visibility;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.dynamic.DynamicType;
 import net.bytebuddy.implementation.FieldAccessor;
@@ -135,6 +136,9 @@ protected DynamicType.Builder<?> enhanceInstance(TypeDescription typeDescription
                 newClassBuilder = newClassBuilder.defineField(
                     CONTEXT_ATTR_NAME, Object.class, ACC_PRIVATE | ACC_VOLATILE)
                                                  .implement(EnhancedInstance.class)
+                                                 .defineMethod(CONTEXT_GETTER_NAME, Object.class, Visibility.PUBLIC)
+                                                 .intercept(FieldAccessor.ofField(CONTEXT_ATTR_NAME))
+                                                 .defineMethod(CONTEXT_SETTER_NAME, void.class, Visibility.PUBLIC).withParameters(Object.class)
                                                  .intercept(FieldAccessor.ofField(CONTEXT_ATTR_NAME));
                 context.extendObjectCompleted();
             }

apm-sniffer/bytebuddy-patch/src/test/java/org/apache/skywalking/apm/agent/bytebuddy/biz/ChildBar.java

@@ -0,0 +1,27 @@
+/*
+ * 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.agent.bytebuddy.biz;
+
+public class ChildBar extends ParentBar {
+
+    public String sayHelloChild() {
+        return "Joe";
+    }
+
+}

apm-sniffer/bytebuddy-patch/src/test/java/org/apache/skywalking/apm/agent/bytebuddy/biz/ParentBar.java

@@ -0,0 +1,26 @@
+/*
+ * 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.agent.bytebuddy.biz;
+
+public class ParentBar {
+
+    public String sayHelloParent() {
+        return "Joe";
+    }
+}

apm-sniffer/bytebuddy-patch/src/test/java/org/apache/skywalking/apm/agent/bytebuddy/cases/AbstractInterceptTest.java

@@ -24,6 +24,7 @@
 import net.bytebuddy.agent.builder.SWAgentBuilderDefault;
 import net.bytebuddy.agent.builder.SWDescriptionStrategy;
 import net.bytebuddy.agent.builder.SWNativeMethodStrategy;
+import net.bytebuddy.description.modifier.Visibility;
 import net.bytebuddy.implementation.FieldAccessor;
 import net.bytebuddy.implementation.MethodDelegation;
 import net.bytebuddy.implementation.SWImplementationContextFactory;
@@ -38,6 +39,7 @@
 import org.apache.skywalking.apm.agent.bytebuddy.SWAuxiliaryTypeNamingStrategy;
 import org.apache.skywalking.apm.agent.bytebuddy.SWClassFileLocator;
 import org.apache.skywalking.apm.agent.bytebuddy.biz.BizFoo;
+import org.apache.skywalking.apm.agent.bytebuddy.biz.ChildBar;
 import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
 import org.junit.Assert;
 import org.junit.BeforeClass;
@@ -58,11 +60,15 @@
 import static net.bytebuddy.jar.asm.Opcodes.ACC_PRIVATE;
 import static net.bytebuddy.jar.asm.Opcodes.ACC_VOLATILE;
 import static org.apache.skywalking.apm.agent.core.plugin.AbstractClassEnhancePluginDefine.CONTEXT_ATTR_NAME;
+import static org.apache.skywalking.apm.agent.core.plugin.AbstractClassEnhancePluginDefine.CONTEXT_GETTER_NAME;
+import static org.apache.skywalking.apm.agent.core.plugin.AbstractClassEnhancePluginDefine.CONTEXT_SETTER_NAME;
 
 public class AbstractInterceptTest {
     public static final String BIZ_FOO_CLASS_NAME = "org.apache.skywalking.apm.agent.bytebuddy.biz.BizFoo";
     public static final String PROJECT_SERVICE_CLASS_NAME = "org.apache.skywalking.apm.agent.bytebuddy.biz.ProjectService";
     public static final String DOC_SERVICE_CLASS_NAME = "org.apache.skywalking.apm.agent.bytebuddy.biz.DocService";
+    public static final String PARENT_BAR_CLASS_NAME = "org.apache.skywalking.apm.agent.bytebuddy.biz.ParentBar";
+    public static final String CHILD_BAR_CLASS_NAME = "org.apache.skywalking.apm.agent.bytebuddy.biz.ChildBar";
     public static final String SAY_HELLO_METHOD = "sayHello";
     public static final int BASE_INT_VALUE = 100;
     public static final String CONSTRUCTOR_INTERCEPTOR_CLASS = "constructorInterceptorClass";
@@ -85,6 +91,20 @@ protected void failed(Throwable e, Description description) {
         }
     };
 
+    protected static void callBar(int round) {
+        Log.info("-------------");
+        Log.info("callChildBar: " + round);
+        // load target class
+        String strResultChild = new ChildBar().sayHelloChild();
+        Log.info("result: " + strResultChild);
+
+        String strResultParent = new ChildBar().sayHelloParent();
+        Log.info("result: " + strResultParent);
+
+        Assert.assertEquals("String value is unexpected", "John", strResultChild);
+        Assert.assertEquals("String value is unexpected", "John", strResultParent);
+    }
+
     protected static void callBizFoo(int round) {
         Log.info("-------------");
         Log.info("callBizFoo: " + round);
@@ -115,7 +135,7 @@ protected static void checkConstructorInterceptor(String className, int round) {
 
     protected static void checkInterface(Class testClass, Class interfaceCls) {
         Assert.assertTrue("Check interface failure, the test class: " + testClass + " does not implement the expected interface: " + interfaceCls,
-                EnhancedInstance.class.isAssignableFrom(BizFoo.class));
+                EnhancedInstance.class.isAssignableFrom(testClass));
     }
 
     protected static void checkErrors() {
@@ -195,6 +215,9 @@ protected void installInterface(String className) {
                                 builder = builder.defineField(
                                                 CONTEXT_ATTR_NAME, Object.class, ACC_PRIVATE | ACC_VOLATILE)
                                         .implement(EnhancedInstance.class)
+                                        .defineMethod(CONTEXT_GETTER_NAME, Object.class, Visibility.PUBLIC)
+                                        .intercept(FieldAccessor.ofField(CONTEXT_ATTR_NAME))
+                                        .defineMethod(CONTEXT_SETTER_NAME, void.class, Visibility.PUBLIC).withParameters(Object.class)
                                         .intercept(FieldAccessor.ofField(CONTEXT_ATTR_NAME));
                             }
                             return builder;
@@ -223,7 +246,7 @@ protected void installTraceClassTransformer(String msg) {
         ClassFileTransformer classFileTransformer = new ClassFileTransformer() {
             @Override
             public byte[] transform(ClassLoader loader, String className, Class<?> classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer) throws IllegalClassFormatException {
-                if (className.endsWith("BizFoo") || className.endsWith("ProjectService") || className.endsWith("DocService")) {
+                if (className.endsWith("BizFoo") || className.endsWith("ProjectService") || className.endsWith("DocService") || className.endsWith("ChildBar") || className.endsWith("ParentBar")) {
                     Log.error(msg + className);
                     ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
                     ClassReader cr = new ClassReader(classfileBuffer);

apm-sniffer/bytebuddy-patch/src/test/java/org/apache/skywalking/apm/agent/bytebuddy/cases/ReTransform4Test.java

@@ -0,0 +1,62 @@
+/*
+ * 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.agent.bytebuddy.cases;
+
+import net.bytebuddy.agent.ByteBuddyAgent;
+import org.apache.skywalking.apm.agent.bytebuddy.biz.ChildBar;
+import org.apache.skywalking.apm.agent.bytebuddy.biz.ParentBar;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+import org.junit.Test;
+
+import java.lang.instrument.Instrumentation;
+
+public class ReTransform4Test extends AbstractReTransformTest {
+
+    @Test
+    public void testInterceptConstructor() throws Exception {
+        Instrumentation instrumentation = ByteBuddyAgent.install();
+
+        // install transformer
+        installMethodInterceptor(PARENT_BAR_CLASS_NAME, "sayHelloParent", 1);
+        installMethodInterceptor(CHILD_BAR_CLASS_NAME, "sayHelloChild", 1);
+        // implement EnhancedInstance
+        installInterface(PARENT_BAR_CLASS_NAME);
+        installInterface(CHILD_BAR_CLASS_NAME);
+
+        // call target class
+        callBar(1);
+
+        // check interceptors
+        checkInterface(ParentBar.class, EnhancedInstance.class);
+        checkInterface(ChildBar.class, EnhancedInstance.class);
+        checkErrors();
+
+         installTraceClassTransformer("Trace class: ");
+
+        // do retransform
+        reTransform(instrumentation, ChildBar.class);
+
+        // check interceptors
+        checkInterface(ParentBar.class, EnhancedInstance.class);
+        checkInterface(ChildBar.class, EnhancedInstance.class);
+        checkErrors();
+    }
+
+}
+