Closed kylixs closed 1 month ago
raphw
commented
1 year ago That's non trivial, as normal transformations might be conflicting. For example, if a given value was supposed to be returned, what of the two transformers should win?
Decorating transformers, mainly Advice are however already additive. You should use those if the same method should be transformed twice.
kylixs
commented
1 year ago @raphw
That's non trivial, as normal transformations might be conflicting. For example, if a given value was supposed to be returned, what of the two transformers should win?
Do you mean to modify the return value of the method in the transformer? I don't think we need to change the behavior of the transformer, just execute them sequentially.
Decorating transformers, mainly
Adviceare however already additive. You should use those if the same method should be transformed twice.
We have many plugins, and maybe some plugins enhance same class methods, which transformers to use is determined by the matcher, which is not fixed. It seems that the use of Decorating transformers does not match very well. More tips?
kylixs
commented
1 year ago @raphw I found that Advice can do transform many times on same method, but there's a problem:
How to skip the execution of the original method according to the condition and return the custom value directly, replacing the function of the following interceptor.
public Object intercept(@Origin Class<?> clazz, @AllArguments Object[] allArguments, @Origin Method method,
@SuperCall Callable<?> zuper) throws Throwable {
StaticMethodsAroundInterceptor interceptor = InterceptorInstanceLoader.load(staticMethodsAroundInterceptorClassName, clazz
.getClassLoader());
MethodInterceptResult result = new MethodInterceptResult();
try {
interceptor.beforeMethod(clazz, method, allArguments, method.getParameterTypes(), result);
} catch (Throwable t) {
LOGGER.error(t, "class[{}] before static method[{}] intercept failure", clazz, method.getName());
}
Object ret = null;
try {
if (!result.isContinue()) {
ret = result._ret();
} else {
ret = zuper.call();
}
} catch (Throwable t) {
try {
interceptor.handleMethodException(clazz, method, allArguments, method.getParameterTypes(), t);
} catch (Throwable t2) {
LOGGER.error(t2, "class[{}] handle static method[{}] exception failure", clazz, method.getName(), t2.getMessage());
}
throw t;
} finally {
try {
ret = interceptor.afterMethod(clazz, method, allArguments, method.getParameterTypes(), ret);
} catch (Throwable t) {
LOGGER.error(t, "class[{}] after static method[{}] intercept failure:{}", clazz, method.getName(), t.getMessage());
}
}
return ret;
}I came across a code snippet that lacks elegance and appears rather peculiar to do it. @Advice.OnMethodEnter(skipOn=...)
seems peculiar, or using it incorrectly?
public class AdviceSkipOnTest {
public static void main(String[] args) throws Exception {
Instrumentation instrumentation = ByteBuddyAgent.install();
new AgentBuilder.Default()
.type(ElementMatchers.nameEndsWith("MyClass"))
.transform((builder, typeDescription, classLoader, module, protectionDomain) ->
builder.method(ElementMatchers.named("sayHello"))
.intercept(Advice.to(MyAdvice.class))
)
.with(createListener())
.installOn(instrumentation);
//instrumentation.retransformClasses(MyClass.class);
MyClass instance = new MyClass();
String str = instance.sayHello("Tom");
System.out.println("result: " + str);
System.out.println();
str = instance.sayHello("Joe");
System.out.println("result: " + str);
}
public static class MyClass {
public String sayHello(String message) {
System.out.println("Execute origin method, arg: " + message);
return "Hi, " + message;
}
}
public static class MyAdvice {
@Advice.OnMethodEnter(inline = false, skipOn = Advice.OnDefaultValue.class, skipOnIndex = 1)
public static Object[] enter(@Advice.This Object target,
@Advice.AllArguments Object[] allArguments,
@Advice.Origin Method method) {
String message = (String) allArguments[0];
System.out.println("Before method, arg:" + message);
MyContext context = new MyContext();
// Do something special ..
if (message.contains("Tom")) {
context.setSkip(true);
context.setValue("Reject");
}
if (context.isSkip()) {
return new Object[]{context, null};
} else {
return new Object[]{context, true};
}
}
@Advice.OnMethodExit
public static void exit(@Advice.This Object target,
@Advice.AllArguments Object[] allArguments,
@Advice.Return(readOnly = false, typing = Assigner.Typing.DYNAMIC) Object returnObj,
@Advice.Enter(readOnly = false, typing = Assigner.Typing.DYNAMIC) Object[] contexts) {
MyContext context = (MyContext) contexts[0];
if (context.isSkip()) {
returnObj = context.value;
System.out.println("Skip origin method and set return value: " + returnObj);
} else {
System.out.println("Exit method with return value: " + returnObj);
}
}
public static Object onExit(Object target, Object[] allArguments, Object returnObj, MyContext context) {
return context.value;
}
}
public static class MyContext {
boolean skip;
Object value;
}
}Output:
Before method, arg:Tom
Skip origin method and set return value: Reject
result: Reject
Before method, arg:Joe
Execute origin method, arg: Joe
Exit method with return value: Hi, Joe
result: Hi, Joe
Background: We know that adding multiple transformers to the AgentBuilder will eventually merge together and only enhance the bytecode once. If two Transformers enhance the same class method, only the final Transformer will take effect.
What we exptect: Multiple Transformers are isolated, that is, if both Transformers enhance the same class method, both will take effect.
I made some rough changes to the code to achieve the above requirements.
How to add this feature to
ByteBuddy? May need better design or add support extensions. Look forward to your kind advice.ByteBuddy changes: https://github.com/kylixs/byte-buddy/commit/999aeeab266dc8b9952ba653de3a469f4ab9fba9 Testcase: https://github.com/kylixs/bytebuddy-intercept-demo/blob/main/src/test/java/com/demo/case1/MultipleTransformersTest.java Related Issues: https://github.com/apache/skywalking-java/pull/521#issuecomment-1586097262