Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discussion about multi-interceptors supporting in SkyWalking #7440

Closed
daimingzhi opened this issue Aug 11, 2021 · 4 comments
Closed

Discussion about multi-interceptors supporting in SkyWalking #7440

daimingzhi opened this issue Aug 11, 2021 · 4 comments
Labels
discussion Discussions

Comments

@daimingzhi
Copy link

  • Discussion : multi-interceptors supporting in SkyWalking

As I have mentioned in https://github.com/apache/skywalking/issues/6932#issuecomment-895825310

For extensibility of SkyWaling, we should support multi-interceptors。

I have two ways of doing this:

the first one

We can construct multiple ClassFileTransformer instances. In each ClassFileTransformer, there will only be one pluginDefine for each class.To do this, we should group the plugindefine,and we will construct a ClassFileTransformer instance for each group.
The code might look something like this

List<PluginFinder> pluginFinders = groupByPluginDefine();
for (PluginFinder finder : pluginFinders) {
    agentBuilder.type(finder.buildMatch())
        .transform(new Transformer(finder))
        .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION)
        .with(new Listener())
        .installOn(instrumentation);
}

What the method named groupByPluginDefine method does is group the pluginDefine, and promise only one pluginDefine for each class per group.

the second one

I have debugged code, and found the reason is every time a class is enhanced, it will delegate the original method to a new InstMethodsInter instance. So, the previous is overridden.
image-20210811135321186

To solve it, maybe we can cache the interceptor and merge them.

Finally, I also looked for new apis in ByteBuddy as an alternative to MethodDelegation. but,I am newer for it.This will be a long time.
This is my whole idea.I hope some seniors can give me some advice. Thanks.

@wu-sheng
Copy link
Member

The first solution may not work. Because the delegate mechanism only supports to do once.

Finally, I also looked for new apis in ByteBuddy as an alternative to MethodDelegation. but,I am newer for it.This will be a long time.

This should not work, I checked with the author before.

I have debugged code, and found the reason is every time a class is enhanced, it will delegate the original method to a new InstMethodsInter instance. So, the previous is overridden.

Yes, this is the reason.

You missed one important port, EnhancedInstance#dynamicField, which has only one value. But different interceptors could access(set/get) with or without other plugins. In this case, you will face ·race condition, visibility of memory operations across threads, even type cast exception` in the runtime.

@wu-sheng wu-sheng added the discussion Discussions label Aug 11, 2021
@daimingzhi daimingzhi reopened this Aug 11, 2021
@daimingzhi
Copy link
Author

Because the delegate mechanism only supports to do once

I didn't fully grasp the meaning of this。but,I tested the byteBuddy API like this

  • code of agent
public class ByteBuddyAgent {

    public static void premain(String agentArgs, Instrumentation inst) {
        // install first time
        new AgentBuilder.Default()
            .type(named("com.easy4coding.bytebuddy.Bar"))
            .transform(new AgentBuilder.Transformer() {
                @Override
                public DynamicType.Builder transform(DynamicType.Builder builder,
                                                     TypeDescription typeDescription,
                                                     ClassLoader classloader,
                                                     JavaModule javaModule) {
                    return builder.method(named("m"))
                        .intercept(MethodDelegation.to(new BarInterceptorFirst()));
                }
            }).installOn(inst);
		
        // install second time
        new AgentBuilder.Default()
            .type(named("com.easy4coding.bytebuddy.Bar"))
            .transform(new AgentBuilder.Transformer() {
                @Override
                public DynamicType.Builder transform(DynamicType.Builder builder,
                                                     TypeDescription typeDescription,
                                                     ClassLoader classloader,
                                                     JavaModule javaModule) {
                    return builder.method(named("m"))
                        .intercept(MethodDelegation.to(new BarInterceptorSecond()));
                }
            }).installOn(inst);
    }
}

public class BarInterceptorFirst {

	@RuntimeType
	public Object intercept(@This Object obj, @AllArguments Object[] allArguments, @SuperCall Callable<?> zuper,
	                        @Origin Method method) {

		System.out.println("before method first");
		Object ret = null;
		try {
			ret = zuper.call();
		} catch (Exception e) {
			e.printStackTrace();
		}
		System.out.println("after method first");

		return ret;
	}

}

public class BarInterceptorSecond {

	@RuntimeType
	public Object intercept(@This Object obj, @AllArguments Object[] allArguments, @SuperCall Callable<?> zuper,
	                        @Origin Method method) {

		System.out.println("before method Second");
		Object ret = null;
		try {
			ret = zuper.call();
		} catch (Exception e) {
			e.printStackTrace();
		}
		System.out.println("after method Second");

		return ret;
	}

}
  • testDemo
public class DuplicateInstallDemo {
	public static void main(String[] args) {
		new Bar().m();
	}
}

The program output is as follows:

before method 2
before method
method named m invoked
after method
after method 2

What puzzles me most about the first one is, how do I group pluginDefine?

What I did in my company was adjust the directory structure of the project and extend the AgentClassLoader。

like this:

image-20210812130038336

But, this way is not suitable for skywaling obviously。

You missed one important port, EnhancedInstance#dynamicField, which has only one value.

I'm not really familiar with this piece of logic now.I will spend more time learning about it,Also, I might have to learn more about ByteBuddy

@wu-sheng
Copy link
Member

wu-sheng commented Aug 12, 2021

This is not about bytebuddy. SkyWalking agent is not bytebuddy agent. I think you mixed these.
I never said Bytebuddy can't do this, it definitely can. I am from SkyWalking perspective.

@wu-sheng
Copy link
Member

I think you skipped the details, and assume SkyWalking agent is as simple as Bytebuddy. If so, you wouldn't see why there are so many codes for SkyWalking agent core. We are building on the top of Bytebuddy.

@apache apache locked and limited conversation to collaborators Aug 19, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
discussion Discussions
Projects
None yet
Development

No branches or pull requests

2 participants