Skip to content

Commit 529d6d7

Browse files
tjuwangningtjuwangning
authored
fix parent class enhance issue (#757)
1 parent 9e80a4c commit 529d6d7

File tree

8 files changed

+157
-2
lines changed

8 files changed

+157
-2
lines changed

‎CHANGES.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ Release Notes.
1111
* Agent kernel services could be not-booted-yet as ServiceManager#INSTANCE#boot executed after agent transfer
1212
initialization. Delay so11y metrics#build when the services are not ready to avoid MeterService status is not
1313
initialized.
14+
* Fix retransform failure when enhancing both parent and child classes.
1415

1516
All issues and pull requests are [here](https://github.com/apache/skywalking/milestone/236?closed=1)
1617

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,14 @@ public abstract class AbstractClassEnhancePluginDefine {
5151
* New field name.
5252
*/
5353
public static final String CONTEXT_ATTR_NAME = "_$EnhancedClassField_ws";
54+
/**
55+
* Getter method name.
56+
*/
57+
public static final String CONTEXT_GETTER_NAME = "getSkyWalkingDynamicField";
58+
/**
59+
* Setter method name.
60+
*/
61+
public static final String CONTEXT_SETTER_NAME = "setSkyWalkingDynamicField";
5462

5563
/**
5664
* 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‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance;
2020

2121
import net.bytebuddy.description.method.MethodDescription;
22+
import net.bytebuddy.description.modifier.Visibility;
2223
import net.bytebuddy.description.type.TypeDescription;
2324
import net.bytebuddy.dynamic.DynamicType;
2425
import net.bytebuddy.implementation.FieldAccessor;
@@ -102,6 +103,9 @@ protected DynamicType.Builder<?> enhanceInstance(TypeDescription typeDescription
102103
newClassBuilder = newClassBuilder.defineField(
103104
CONTEXT_ATTR_NAME, Object.class, ACC_PRIVATE | ACC_VOLATILE)
104105
.implement(EnhancedInstance.class)
106+
.defineMethod(CONTEXT_GETTER_NAME, Object.class, Visibility.PUBLIC)
107+
.intercept(FieldAccessor.ofField(CONTEXT_ATTR_NAME))
108+
.defineMethod(CONTEXT_SETTER_NAME, void.class, Visibility.PUBLIC).withParameters(Object.class)
105109
.intercept(FieldAccessor.ofField(CONTEXT_ATTR_NAME));
106110
context.extendObjectCompleted();
107111
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.v2;
1919

2020
import net.bytebuddy.description.method.MethodDescription;
21+
import net.bytebuddy.description.modifier.Visibility;
2122
import net.bytebuddy.description.type.TypeDescription;
2223
import net.bytebuddy.dynamic.DynamicType;
2324
import net.bytebuddy.implementation.FieldAccessor;
@@ -135,6 +136,9 @@ protected DynamicType.Builder<?> enhanceInstance(TypeDescription typeDescription
135136
newClassBuilder = newClassBuilder.defineField(
136137
CONTEXT_ATTR_NAME, Object.class, ACC_PRIVATE | ACC_VOLATILE)
137138
.implement(EnhancedInstance.class)
139+
.defineMethod(CONTEXT_GETTER_NAME, Object.class, Visibility.PUBLIC)
140+
.intercept(FieldAccessor.ofField(CONTEXT_ATTR_NAME))
141+
.defineMethod(CONTEXT_SETTER_NAME, void.class, Visibility.PUBLIC).withParameters(Object.class)
138142
.intercept(FieldAccessor.ofField(CONTEXT_ATTR_NAME));
139143
context.extendObjectCompleted();
140144
}

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*
17+
*/
18+
19+
package org.apache.skywalking.apm.agent.bytebuddy.biz;
20+
21+
public class ChildBar extends ParentBar {
22+
23+
public String sayHelloChild() {
24+
return "Joe";
25+
}
26+
27+
}

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*
17+
*/
18+
19+
package org.apache.skywalking.apm.agent.bytebuddy.biz;
20+
21+
public class ParentBar {
22+
23+
public String sayHelloParent() {
24+
return "Joe";
25+
}
26+
}

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

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import net.bytebuddy.agent.builder.SWAgentBuilderDefault;
2525
import net.bytebuddy.agent.builder.SWDescriptionStrategy;
2626
import net.bytebuddy.agent.builder.SWNativeMethodStrategy;
27+
import net.bytebuddy.description.modifier.Visibility;
2728
import net.bytebuddy.implementation.FieldAccessor;
2829
import net.bytebuddy.implementation.MethodDelegation;
2930
import net.bytebuddy.implementation.SWImplementationContextFactory;
@@ -38,6 +39,7 @@
3839
import org.apache.skywalking.apm.agent.bytebuddy.SWAuxiliaryTypeNamingStrategy;
3940
import org.apache.skywalking.apm.agent.bytebuddy.SWClassFileLocator;
4041
import org.apache.skywalking.apm.agent.bytebuddy.biz.BizFoo;
42+
import org.apache.skywalking.apm.agent.bytebuddy.biz.ChildBar;
4143
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
4244
import org.junit.Assert;
4345
import org.junit.BeforeClass;
@@ -58,11 +60,15 @@
5860
import static net.bytebuddy.jar.asm.Opcodes.ACC_PRIVATE;
5961
import static net.bytebuddy.jar.asm.Opcodes.ACC_VOLATILE;
6062
import static org.apache.skywalking.apm.agent.core.plugin.AbstractClassEnhancePluginDefine.CONTEXT_ATTR_NAME;
63+
import static org.apache.skywalking.apm.agent.core.plugin.AbstractClassEnhancePluginDefine.CONTEXT_GETTER_NAME;
64+
import static org.apache.skywalking.apm.agent.core.plugin.AbstractClassEnhancePluginDefine.CONTEXT_SETTER_NAME;
6165

6266
public class AbstractInterceptTest {
6367
public static final String BIZ_FOO_CLASS_NAME = "org.apache.skywalking.apm.agent.bytebuddy.biz.BizFoo";
6468
public static final String PROJECT_SERVICE_CLASS_NAME = "org.apache.skywalking.apm.agent.bytebuddy.biz.ProjectService";
6569
public static final String DOC_SERVICE_CLASS_NAME = "org.apache.skywalking.apm.agent.bytebuddy.biz.DocService";
70+
public static final String PARENT_BAR_CLASS_NAME = "org.apache.skywalking.apm.agent.bytebuddy.biz.ParentBar";
71+
public static final String CHILD_BAR_CLASS_NAME = "org.apache.skywalking.apm.agent.bytebuddy.biz.ChildBar";
6672
public static final String SAY_HELLO_METHOD = "sayHello";
6773
public static final int BASE_INT_VALUE = 100;
6874
public static final String CONSTRUCTOR_INTERCEPTOR_CLASS = "constructorInterceptorClass";
@@ -85,6 +91,20 @@ protected void failed(Throwable e, Description description) {
8591
}
8692
};
8793

94+
protected static void callBar(int round) {
95+
Log.info("-------------");
96+
Log.info("callChildBar: " + round);
97+
// load target class
98+
String strResultChild = new ChildBar().sayHelloChild();
99+
Log.info("result: " + strResultChild);
100+
101+
String strResultParent = new ChildBar().sayHelloParent();
102+
Log.info("result: " + strResultParent);
103+
104+
Assert.assertEquals("String value is unexpected", "John", strResultChild);
105+
Assert.assertEquals("String value is unexpected", "John", strResultParent);
106+
}
107+
88108
protected static void callBizFoo(int round) {
89109
Log.info("-------------");
90110
Log.info("callBizFoo: " + round);
@@ -115,7 +135,7 @@ protected static void checkConstructorInterceptor(String className, int round) {
115135

116136
protected static void checkInterface(Class testClass, Class interfaceCls) {
117137
Assert.assertTrue("Check interface failure, the test class: " + testClass + " does not implement the expected interface: " + interfaceCls,
118-
EnhancedInstance.class.isAssignableFrom(BizFoo.class));
138+
EnhancedInstance.class.isAssignableFrom(testClass));
119139
}
120140

121141
protected static void checkErrors() {
@@ -195,6 +215,9 @@ protected void installInterface(String className) {
195215
builder = builder.defineField(
196216
CONTEXT_ATTR_NAME, Object.class, ACC_PRIVATE | ACC_VOLATILE)
197217
.implement(EnhancedInstance.class)
218+
.defineMethod(CONTEXT_GETTER_NAME, Object.class, Visibility.PUBLIC)
219+
.intercept(FieldAccessor.ofField(CONTEXT_ATTR_NAME))
220+
.defineMethod(CONTEXT_SETTER_NAME, void.class, Visibility.PUBLIC).withParameters(Object.class)
198221
.intercept(FieldAccessor.ofField(CONTEXT_ATTR_NAME));
199222
}
200223
return builder;
@@ -223,7 +246,7 @@ protected void installTraceClassTransformer(String msg) {
223246
ClassFileTransformer classFileTransformer = new ClassFileTransformer() {
224247
@Override
225248
public byte[] transform(ClassLoader loader, String className, Class<?> classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer) throws IllegalClassFormatException {
226-
if (className.endsWith("BizFoo") || className.endsWith("ProjectService") || className.endsWith("DocService")) {
249+
if (className.endsWith("BizFoo") || className.endsWith("ProjectService") || className.endsWith("DocService") || className.endsWith("ChildBar") || className.endsWith("ParentBar")) {
227250
Log.error(msg + className);
228251
ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
229252
ClassReader cr = new ClassReader(classfileBuffer);

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*
17+
*/
18+
19+
package org.apache.skywalking.apm.agent.bytebuddy.cases;
20+
21+
import net.bytebuddy.agent.ByteBuddyAgent;
22+
import org.apache.skywalking.apm.agent.bytebuddy.biz.ChildBar;
23+
import org.apache.skywalking.apm.agent.bytebuddy.biz.ParentBar;
24+
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
25+
import org.junit.Test;
26+
27+
import java.lang.instrument.Instrumentation;
28+
29+
public class ReTransform4Test extends AbstractReTransformTest {
30+
31+
@Test
32+
public void testInterceptConstructor() throws Exception {
33+
Instrumentation instrumentation = ByteBuddyAgent.install();
34+
35+
// install transformer
36+
installMethodInterceptor(PARENT_BAR_CLASS_NAME, "sayHelloParent", 1);
37+
installMethodInterceptor(CHILD_BAR_CLASS_NAME, "sayHelloChild", 1);
38+
// implement EnhancedInstance
39+
installInterface(PARENT_BAR_CLASS_NAME);
40+
installInterface(CHILD_BAR_CLASS_NAME);
41+
42+
// call target class
43+
callBar(1);
44+
45+
// check interceptors
46+
checkInterface(ParentBar.class, EnhancedInstance.class);
47+
checkInterface(ChildBar.class, EnhancedInstance.class);
48+
checkErrors();
49+
50+
installTraceClassTransformer("Trace class: ");
51+
52+
// do retransform
53+
reTransform(instrumentation, ChildBar.class);
54+
55+
// check interceptors
56+
checkInterface(ParentBar.class, EnhancedInstance.class);
57+
checkInterface(ChildBar.class, EnhancedInstance.class);
58+
checkErrors();
59+
}
60+
61+
}
62+

0 commit comments

Comments
 (0)