Hi Zhenxu,
It is mostly good to me. But I don’t understand how to intercept multi-methods
in the New API. I think the new api to support to intercept multi-methods when
use `.interceptMethod()` after `.with()`. It is very like ByteBuddy API style.
But I prefer the following.
```java
public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() {
return
Intercepts.addInterceptor(Intercept.instanceMethod(named(ENHANCE_METHOD)
.and(takesArgumentWithType(0, "javax.jms.Destination"))
.and(takesArgumentWithType(1, "javax.jms.Message")))
.with(INTERCEPTOR_CLASS)
.build())
.addIntercceptor(Intercept.instanceMethod(name(ENHANCE_METHOD).and(takesArguments(1)))
.with(INTERCEPTOR_CLASS)
.build())
.build();
}
```
———————
Daming(@dmsolr)
Apache SkyWalking
> 在 2020年5月16日，下午6:04，kezhenxu94@apache <kezhenx...@apache.org> 写道：
>
> Hi the SkyWalking Community
>
> I’m writing to propose some changes in the Java agent instrumentation APIs,
> nowadays, we’re using anonymous inner classes (AIC) as follows:
>
>
> ```java
> public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
> return new ConstructorInterceptPoint[] {
> new ConstructorInterceptPoint() {
> @Override
> public ElementMatcher<MethodDescription> getConstructorMatcher() {
> return any();
> }
>
> @Override
> public String getConstructorInterceptor() {
> return
> "org.apache.skywalking.apm.plugin.elasticsearch.v5.GenericActionConstructorInterceptor";
> }
> }
> };
> }
> ```
>
>
> There’re some disadvantages with AIC:
>
> 1. Every AIC (`new ConstructorInterceptPoint()`) in every plugin definition
> is a completely unique class, as they are compiled into something like
> `ActiveMQConsumerInstrumentation$1.class` and
> `ActiveMQConsumerInstrumentation$2.class`, and thus loading them requires
> more metaspace, small though.
>
> 2. The API is somewhat verbose, and exposes too many concepts, like
> `ConstructorsInterceptPoint`, `StaticMethodsInterceptPoint`,
> `InstanceMethodsInterceptPoint`, we can have a unified API to provide all of
> these.
>
> Hereby I propose the new APIs, which eliminates the unnecessary classes, and
> should be more semantic, like this:
>
>
> ```java
> public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
> return Intercept.constructor(takesArgumentWithType(0,
> CONSTRUCTOR_INTERCEPT_TYPE))
> .with(CONSTRUCTOR_INTERCEPTOR_CLASS)
> .build();
> }
>
> public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() {
> return Intercept.instanceMethod(named(ENHANCE_METHOD)
> .and(takesArgumentWithType(0,
> "javax.jms.Destination"))
> .and(takesArgumentWithType(1,
> "javax.jms.Message")))
> .with(INTERCEPTOR_CLASS)
> .build();
> }
>
> public StaticMethodsInterceptPoint[] getStaticMethodsInterceptPoints() {
> return Intercept.staticMethod(named("makeWrapper"))
>
> .with("org.apache.skywalking.apm.plugin.dubbo.patch.MakeWrapperInterceptor")
> .build();
> }
> ```
>
>
> We could keep the old APIs there and mark them as deprecated for
> compatibility, or simply replace them with the new APIs, what do you think?
>
>
> If there is no objection from the community, I can open an issue to track
> this.
>
>
>
>
> GitHub @kezhenxu94
> Apache SkyWalking, Apache Dubbo
>
>
