You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@skywalking.apache.org by "Li BingLong (智能平台)" <bi...@sohu-inc.com> on 2021/01/04 06:12:59 UTC

Cross Thread plugin

Hi!
I have seen the pr.
https://github.com/apache/skywalking/pull/845
Frankly, this plugin is invasive compared to other plugins.
We must write hard code with RunnableWrapper to use it.
Why not enhance the Runnable class? This way we need to support lambda expression.

Re: Cross Thread plugin

Posted by "Li BingLong (智能平台)" <bi...@sohu-inc.com>.
When I wrote the blog, I find another thing I missed.

When I use the RunnableWrapper in the maven artifact org.apache.skywalking:apm-toolkit-trace to wrap the Runnable param in ThreadPoolExecutor#execute method. The RunnableWrapper is not instrumented because the org.apache.skywalking.apm.agent.core.plugin.match.ProtectiveShieldMatcher#matches return false. If there is some log, it will save me a lot time. The pr is https://github.com/apache/skywalking/pull/6330/files

Re: Cross Thread plugin

Posted by "Li BingLong (智能平台)" <bi...@sohu-inc.com>.
The skywalking pr is https://github.com/apache/skywalking/pull/6309


I have also write a blod. The pr is https://github.com/apache/skywalking-website/pull/209.


Re: Cross Thread plugin

Posted by Sheng Wu <wu...@gmail.com>.
For the blog, here, https://github.com/apache/skywalking-website.
You could send a pull request to add yours. Once it gets merged, it will be
posted on the website.

Sheng Wu 吴晟
Twitter, wusheng1108


Li BingLong(智能平台) <bi...@sohu-inc.com> 于2021年2月1日周一 下午4:35写道:

> > About your new project, could you fill the readme and doc for the public?
>
> I will add some details in the project README.MD soon.
>
>
>
> > And write a blog(better in English) to post on skywalking.apache.org for
> > sharing your codes? We are glad to host and promote your ideas.
>
> How do I post a blog? I can not find the entry.
>
>
> > And to show more error details if there is some RETRANSFORMATION error,
> we
> > think it is a good addition for the upstream if you want to send a pull
> > request and share what it could log out.
>
> I will request a pr.
>

Re: Cross Thread plugin

Posted by "Li BingLong (智能平台)" <bi...@sohu-inc.com>.
> About your new project, could you fill the readme and doc for the public?

I will add some details in the project README.MD soon.



> And write a blog(better in English) to post on skywalking.apache.org for
> sharing your codes? We are glad to host and promote your ideas.

How do I post a blog? I can not find the entry.


> And to show more error details if there is some RETRANSFORMATION error, we
> think it is a good addition for the upstream if you want to send a pull
> request and share what it could log out.

I will request a pr.

Re: Cross Thread plugin

Posted by Sheng Wu <wu...@gmail.com>.
Hi

Thanks again for keeping us updated on the progress.

About your new project, could you fill the readme and doc for the public?
And write a blog(better in English) to post on skywalking.apache.org for
sharing your codes? We are glad to host and promote your ideas.
And to show more error details if there is some RETRANSFORMATION error, we
think it is a good addition for the upstream if you want to send a pull
request and share what it could log out.

Sheng Wu 吴晟
Twitter, wusheng1108


Li BingLong(智能平台) <bi...@sohu-inc.com> 于2021年2月1日周一 下午3:59写道:

> I have finally got another idea to do this thing.
> So it’ time to end this topic.
>
>
> For People who want to enhance the ThreadPoolExecuter or other classes
> used in the skywalking java agent without changing the code, the following
> project is for you!
> https://github.com/libinglong/skywalking-threadpool-agent
>
> It’s also an agent.
>
>
> By the way, I test the disableClassFormatChanges(), but it does not work
> well in some scene.
>
> If we want to find more error message, there is another type of listener
> net.bytebuddy.agent.builder.AgentBuilder.RedefinitionStrategy.Listener.
>
> For example:
>
> agentBuilder.type(pluginFinder.buildMatch())
>             .transform(new Transformer(pluginFinder))
>
>
>            .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION)
>         // this listener is different from the next listener
>             .with(new RedefineListener())
>             .with(new Listener())
>
>
>
>
>
>
>

Re: Cross Thread plugin

Posted by "Li BingLong (智能平台)" <bi...@sohu-inc.com>.
I have finally got another idea to do this thing.
So it’ time to end this topic.


For People who want to enhance the ThreadPoolExecuter or other classes used in the skywalking java agent without changing the code, the following project is for you!
https://github.com/libinglong/skywalking-threadpool-agent

It’s also an agent.


By the way, I test the disableClassFormatChanges(), but it does not work well in some scene.

If we want to find more error message, there is another type of listener net.bytebuddy.agent.builder.AgentBuilder.RedefinitionStrategy.Listener.

For example:

agentBuilder.type(pluginFinder.buildMatch())
            .transform(new Transformer(pluginFinder))
           

           .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION)
	// this listener is different from the next listener
            .with(new RedefineListener())
	    .with(new Listener())







Re: Cross Thread plugin

Posted by "Li BingLong (智能平台)" <bi...@sohu-inc.com>.
Basically, we still don't do RETRANSFORMATION.


The code in master:

agentBuilder.type(pluginFinder.buildMatch())
    .transform(new Transformer(pluginFinder))
    .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION)
    .with(new Listener())
    .installOn(instrumentation);


Just suggesting:

If not using RETRANSFORMATION, the config should be

1.
AgentBuilder.RedefinitionStrategy.REDEFINITION
AgentBuilder.RedefinitionStrategy.DISABLED

This code explicit disabled RETRANSFORMATION so that no log about retransforming a class loaded is acceptable.


2.
AgentBuilder.RedefinitionStrategy.RETRANSFORMATION
.disableClassFormatChanges()

This code enable RETRANSFORMATION.we should also activate .disableClassFormatChanges() so that we can know why the RETRANSFORMATION fails in the listener#onError
It is useful for plugin developer for troubleshooting.


See https://github.com/raphw/byte-buddy/issues/237#issuecomment-269348271






Re: Cross Thread plugin

Posted by Sheng Wu <wu...@gmail.com>.
Oh, I see your point. It was added by another PMC member, to make the
SkyWalking could work with other agents.
See https://github.com/apache/skywalking/issues/3155.
Basically, we still don't do RETRANSFORMATION.

Sheng Wu 吴晟
Twitter, wusheng1108


Li BingLong(智能平台) <bi...@sohu-inc.com> 于2021年1月23日周六 下午11:00写道:

> OK, I don't remember we enable RETRANSFORMATION.
>
>
>
> We are blocking adding this feature for a very long time, due to make the
> agent installed service stable.
>
>
>
>
> I am a little confused about what you said.
>
>
> agentBuilder.type(pluginFinder.buildMatch())
>     .transform(new Transformer(pluginFinder))
>     .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION)
>     .with(new Listener())
>     .installOn(instrumentation);
>
>
>
> The former code is in master.
>
>
> https://github.com/apache/skywalking/blob/master/apm-sniffer/apm-agent/src/main/java/org/apache/skywalking/apm/agent/SkyWalkingAgent.java
>
>

Re: Cross Thread plugin

Posted by "Li BingLong (智能平台)" <bi...@sohu-inc.com>.
OK, I don't remember we enable RETRANSFORMATION.


We are blocking adding this feature for a very long time, due to make the
agent installed service stable.



I am a little confused about what you said.


agentBuilder.type(pluginFinder.buildMatch())
    .transform(new Transformer(pluginFinder))
    .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION)
    .with(new Listener())
    .installOn(instrumentation);



The former code is in master.

https://github.com/apache/skywalking/blob/master/apm-sniffer/apm-agent/src/main/java/org/apache/skywalking/apm/agent/SkyWalkingAgent.java


Re: Cross Thread plugin

Posted by Sheng Wu <wu...@gmail.com>.
There is not a replacement. That is why `RETRANSFORMATION` is a JDK level
concept. Sorry.
We are blocking adding this feature for a very long time, due to make the
agent installed service stable.
Too many production environment incidents reported because of this.

Sheng Wu 吴晟
Twitter, wusheng1108


Li BingLong(智能平台) <bi...@sohu-inc.com> 于2021年1月23日周六 下午10:31写道:

> > OK, I don't remember we enable RETRANSFORMATION.
> > And I remember we had a discussion before, and feedback from developers
> of
> > APM vendors, RETRANSFORMATION is a super bad idea in the product
> > environment. (Sorry I can't tell you the source)
>
> I will add this code and request a pr soon when I have tested it.
>
>
>
> Is there a good replacement of RETRANSFORMATION? I don’t know much about
> RETRANSFORMATION.
>
>
>

Re: Cross Thread plugin

Posted by "Li BingLong (智能平台)" <bi...@sohu-inc.com>.
> OK, I don't remember we enable RETRANSFORMATION.
> And I remember we had a discussion before, and feedback from developers of
> APM vendors, RETRANSFORMATION is a super bad idea in the product
> environment. (Sorry I can't tell you the source)

I will add this code and request a pr soon when I have tested it.



Is there a good replacement of RETRANSFORMATION? I don’t know much about RETRANSFORMATION.



Re: Cross Thread plugin

Posted by Sheng Wu <wu...@gmail.com>.
OK, I don't remember we enable RETRANSFORMATION.
And I remember we had a discussion before, and feedback from developers of
APM vendors, RETRANSFORMATION is a super bad idea in the product
environment. (Sorry I can't tell you the source)

Sheng Wu 吴晟
Twitter, wusheng1108


Li BingLong(智能平台) <bi...@sohu-inc.com> 于2021年1月23日周六 下午9:53写道:

> > If you want to show the transform failure, it is fine to open that always
> > as an error level log, meanwhile, don't crash the monitored service.
> > And you don't need a config to open this, we should warn the users about
> > this always.
>
>
>
> I mean the following code
>
> agentBuilder.type(pluginFinder.buildMatch())
>             .transform(new Transformer(pluginFinder))
>             .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION)
>             .with(new Listener())
>             // if we don’t add this code, the listener does not show any
> error
>             .disableClassFormatChanges()
>             .installOn(instrumentation);
>
> https://github.com/raphw/byte-buddy/issues/871#issuecomment-635204898
>
>

Re: Cross Thread plugin

Posted by "Li BingLong (智能平台)" <bi...@sohu-inc.com>.
> If you want to show the transform failure, it is fine to open that always
> as an error level log, meanwhile, don't crash the monitored service.
> And you don't need a config to open this, we should warn the users about
> this always.



I mean the following code

agentBuilder.type(pluginFinder.buildMatch())
            .transform(new Transformer(pluginFinder))
            .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION)
            .with(new Listener())
	    // if we don’t add this code, the listener does not show any error
            .disableClassFormatChanges()
            .installOn(instrumentation);

https://github.com/raphw/byte-buddy/issues/871#issuecomment-635204898


Re: Cross Thread plugin

Posted by Sheng Wu <wu...@gmail.com>.
If you want to show the transform failure, it is fine to open that always
as an error level log, meanwhile, don't crash the monitored service.
And you don't need a config to open this, we should warn the users about
this always.

Sheng Wu 吴晟
Twitter, wusheng1108


Li BingLong(智能平台) <bi...@sohu-inc.com> 于2021年1月23日周六 下午9:13写道:

> > When need EnhancedInstance, there are many cases, such as propagate the
> > context, cache the data.
> > You change any core level interface will break many things.
> > We don't expect that.
>
> If we can not change this, I think we should add the config
> disableClassFormatChanges so that we can know why the instrument fails.
>
> Hope I can find another way to do something about this topic. :)

Re: Cross Thread plugin

Posted by "Li BingLong (智能平台)" <bi...@sohu-inc.com>.
> When need EnhancedInstance, there are many cases, such as propagate the
> context, cache the data.
> You change any core level interface will break many things.
> We don't expect that.

If we can not change this, I think we should add the config disableClassFormatChanges so that we can know why the instrument fails.

Hope I can find another way to do something about this topic. :)

Re: Cross Thread plugin

Posted by Sheng Wu <wu...@gmail.com>.
When need EnhancedInstance, there are many cases, such as propagate the
context, cache the data.
You change any core level interface will break many things.
We don't expect that.

> Yes,I do.
> Up to now, our service is normal.

I don't mean for your env only. I mean for wide cases, especially complex
webserver cases, such as WebLogic, Websphere, etc.


Sheng Wu 吴晟
Twitter, wusheng1108


Li BingLong(智能平台) <bi...@sohu-inc.com> 于2021年1月23日周六 下午6:27写道:

> > You don't get my point. take a look at InstMethodsInter. It requires the
> > EnhancedInstance as parameter.
>
> The interface EnhancedInstance is a limit just like the jvm reloading(can
> not reload different strcture of a class).
> Now I have no idea unless changing it.
> I am not sure in which scene we must use enhancedInstance.
>
> > Do you have experience of doing this in the product env? It is very easy
> to
> > cause a stop-world event. That is why we don't support re-transfer.
>
> Yes,I do.
> Up to now, our service is normal.

Re: Cross Thread plugin

Posted by "Li BingLong (智能平台)" <bi...@sohu-inc.com>.
> You don't get my point. take a look at InstMethodsInter. It requires the
> EnhancedInstance as parameter.

The interface EnhancedInstance is a limit just like the jvm reloading(can not reload different strcture of a class).
Now I have no idea unless changing it.
I am not sure in which scene we must use enhancedInstance.

> Do you have experience of doing this in the product env? It is very easy to
> cause a stop-world event. That is why we don't support re-transfer.

Yes,I do.
Up to now, our service is normal.

Re: Cross Thread plugin

Posted by Sheng Wu <wu...@gmail.com>.
> If you don't add a field, this class isn't `EnhancedInstance`. All
> interceptors would not work.

> We will have a EnhancedInstance of Runnable that is the param of
ThreadPoolExecutor#execute.

You don't get my point. take a look at InstMethodsInter. It requires the
EnhancedInstance as parameter.

> The instrumentation target can be used as long as we don’t change the
class structure,such as adding field, changing method signature.

Do you have experience of doing this in the product env? It is very easy to
cause a stop-world event. That is why we don't support re-transfer.

Sheng Wu 吴晟
Twitter, wusheng1108


Li BingLong(智能平台) <bi...@sohu-inc.com> 于2021年1月23日周六 下午5:43写道:

> > Yes, this rule always exists. So, the instrumentation target can't be
> used
> > in the agent core booting stage.
>
>
> The instrumentation target can be used as long as we don’t change the
> class structure,such as adding field, changing method signature.
>
>
> > If you don't add a field, this class isn't `EnhancedInstance`. All
> > interceptors would not work.
>
> We will have a EnhancedInstance of Runnable that is the param of
> ThreadPoolExecutor#execute.
>
>
> > In my mind, I still don't like the idea of
> > `ThreadPoolExecutor` instrumentation, it is really widely used, and
> > manipulating it is a high-risk operation.
>
> It’s high-risk but less Invasive for user code.We can give the ability but
> user choose to use it or not.

Re: Cross Thread plugin

Posted by "Li BingLong (智能平台)" <bi...@sohu-inc.com>.
> Yes, this rule always exists. So, the instrumentation target can't be used
> in the agent core booting stage.


The instrumentation target can be used as long as we don’t change the class structure,such as adding field, changing method signature.


> If you don't add a field, this class isn't `EnhancedInstance`. All
> interceptors would not work.

We will have a EnhancedInstance of Runnable that is the param of ThreadPoolExecutor#execute.


> In my mind, I still don't like the idea of
> `ThreadPoolExecutor` instrumentation, it is really widely used, and
> manipulating it is a high-risk operation.

It’s high-risk but less Invasive for user code.We can give the ability but user choose to use it or not.

Re: Cross Thread plugin

Posted by Sheng Wu <wu...@gmail.com>.
Hi,

Thanks for investing. I want to be clear about 2 things.

> Jvm can not change the class structure when a class has already been
loaded.

Yes, this rule always exists. So, the instrumentation target can't be used
in the agent core booting stage.

> If we just want to make a aspect for ThreadPoolExecutor#execute method,
we don't need to add a field.

If you don't add a field, this class isn't `EnhancedInstance`. All
interceptors would not work.

About what we can do,
I still think unless this wouldn't be used in the core services, we don't
have many options.
In my mind, I still don't like the idea of
`ThreadPoolExecutor` instrumentation, it is really widely used, and
manipulating it is a high-risk operation.


Sheng Wu 吴晟
Twitter, wusheng1108


Li BingLong(智能平台) <bi...@sohu-inc.com> 于2021年1月23日周六 下午1:15写道:

> After a few days learning byte buddy, I finally know why we can not
> instrument ThreadPoolExecutor when we have used it in the agent.
>
> See the following code that byte buddy generates:
>
> public class ThreadPoolExecutor extends AbstractExecutorService implements
> EnhancedInstance {
> //...
>     private volatile Object _$EnhancedClassField_ws;
> //...
> }
>
> The agent add a field in the ThreadPoolExecutor class.
>
> When config is AgentBuilder.RedefinitionStrategy.RETRANSFORMATION and
> disableClassFormatChanges, we will see the error:
>
> java.lang.IllegalStateException: Cannot define field for frozen type:
> class java.util.concurrent.ThreadPoolExecutor
>
> Now the problem is clear.
> Jvm can not change the class structure when a class has already been
> loaded.
>
> If we just want to make a aspect for ThreadPoolExecutor#execute method, we
> don't need to add a field.
> So if we can do something here?
>
>
>
>
>

Re: Cross Thread plugin

Posted by "Li BingLong (智能平台)" <bi...@sohu-inc.com>.
After a few days learning byte buddy, I finally know why we can not instrument ThreadPoolExecutor when we have used it in the agent.

See the following code that byte buddy generates:

public class ThreadPoolExecutor extends AbstractExecutorService implements EnhancedInstance {
//...
    private volatile Object _$EnhancedClassField_ws;
//...
}

The agent add a field in the ThreadPoolExecutor class.

When config is AgentBuilder.RedefinitionStrategy.RETRANSFORMATION and disableClassFormatChanges, we will see the error:

java.lang.IllegalStateException: Cannot define field for frozen type: class java.util.concurrent.ThreadPoolExecutor

Now the problem is clear.
Jvm can not change the class structure when a class has already been loaded.

If we just want to make a aspect for ThreadPoolExecutor#execute method, we don't need to add a field.
So if we can do something here?





Re: Cross Thread plugin

Posted by Sheng Wu <wu...@gmail.com>.
Your understanding about the classloader has a little wrong,
ThreadPoolExecutor is in the rt.jar, which means only AppClassLoader(JVM's)
could load it no matter what we do.

Sheng Wu 吴晟
Twitter, wusheng1108


Li BingLong(智能平台) <bi...@sohu-inc.com> 于2021年1月4日周一 下午4:57写道:

> Yes, I have known the point.
> In FileWriter, it’s the code "Executors.newSingleThreadScheduledExecutor"
> introduce the ThreadPoolExecutor. We even can not see the
> ThreadPoolExecutor code.
>
> So, the different classloader that I mentioned is like a sandbox.
> It's just like that maven-plugin has it's own classloader.
> The agent should has own classloader, too.The agent code should run in the
> classloader.They don't need to care about when the user classloader load
> ThreadPoolExecutor or any other class.
>
> 在 2021/1/4 下午4:32,“Sheng Wu”<wu...@gmail.com> 写入:
>
>     I don't mean that. The thing is, this class(specific for
>     ThreadPoolExecutor), could be introduced into the classloader by many
>     things, we even don't know it is added.
>
>     > Or can we use different classloader when running agent core code?
>
>     It is already, but think about this, who loads ThreadPoolExecutor? This
>     isn't the SkyWalking classloader's responsibility.
>
>     Sheng Wu 吴晟
>     Twitter, wusheng1108
>
>
>
>

Re: Cross Thread plugin

Posted by "Li BingLong (智能平台)" <bi...@sohu-inc.com>.
Yes, I have known the point.
In FileWriter, it’s the code "Executors.newSingleThreadScheduledExecutor" introduce the ThreadPoolExecutor. We even can not see the ThreadPoolExecutor code.

So, the different classloader that I mentioned is like a sandbox.
It's just like that maven-plugin has it's own classloader.
The agent should has own classloader, too.The agent code should run in the classloader.They don't need to care about when the user classloader load ThreadPoolExecutor or any other class.

在 2021/1/4 下午4:32,“Sheng Wu”<wu...@gmail.com> 写入:

    I don't mean that. The thing is, this class(specific for
    ThreadPoolExecutor), could be introduced into the classloader by many
    things, we even don't know it is added.

    > Or can we use different classloader when running agent core code?

    It is already, but think about this, who loads ThreadPoolExecutor? This
    isn't the SkyWalking classloader's responsibility.

    Sheng Wu 吴晟
    Twitter, wusheng1108




Re: Cross Thread plugin

Posted by Sheng Wu <wu...@gmail.com>.
I don't mean that. The thing is, this class(specific for
ThreadPoolExecutor), could be introduced into the classloader by many
things, we even don't know it is added.

> Or can we use different classloader when running agent core code?

It is already, but think about this, who loads ThreadPoolExecutor? This
isn't the SkyWalking classloader's responsibility.

Sheng Wu 吴晟
Twitter, wusheng1108


Li BingLong(智能平台) <bi...@sohu-inc.com> 于2021年1月4日周一 下午4:28写道:

> The plugin developer should let core develop know what the problem they
> are facing because this scene is scant.
> And the booting sequence do not to be unexpected random.
>
> Or can we use different classloader when running agent core code?
>
>
>
> 在 2021/1/4 下午4:18,“Li BingLong(智能平台)”<bi...@sohu-inc.com> 写入:
>
>     So is it supported or valuable to add a ThreadPoolExecutor plugin?
>     I have tried this, some log not important will appear in stdout.
>
>
>
>     在 2021/1/4 下午3:15,“Sheng Wu”<wu...@gmail.com> 写入:
>
>         True, we can't instrument the class the agent core is using
> already.
>         This is a logic conflict. If we allow the instrumentation
> activated before
>         the configuration initialization and core service booting, you
> have a
>         chance to face runtime NPE or race condition.
>         The core developer and plugin developer would expect the booting
> sequence
>         will be unexpected random, and determined by the monitored app
> codes.
>
>         Sheng Wu 吴晟
>         Twitter, wusheng1108
>
>
>
>
>
>
>

Re: Cross Thread plugin

Posted by "Li BingLong (智能平台)" <bi...@sohu-inc.com>.
The plugin developer should let core develop know what the problem they are facing because this scene is scant.
And the booting sequence do not to be unexpected random.

Or can we use different classloader when running agent core code?



在 2021/1/4 下午4:18,“Li BingLong(智能平台)”<bi...@sohu-inc.com> 写入:

    So is it supported or valuable to add a ThreadPoolExecutor plugin?
    I have tried this, some log not important will appear in stdout.



    在 2021/1/4 下午3:15,“Sheng Wu”<wu...@gmail.com> 写入:

        True, we can't instrument the class the agent core is using already.
        This is a logic conflict. If we allow the instrumentation activated before
        the configuration initialization and core service booting, you have a
        chance to face runtime NPE or race condition.
        The core developer and plugin developer would expect the booting sequence
        will be unexpected random, and determined by the monitored app codes.

        Sheng Wu 吴晟
        Twitter, wusheng1108







Re: Cross Thread plugin

Posted by Sheng Wu <wu...@gmail.com>.
I know there is a chance to support this.
The tricky thing would be, how carefully you need to be when writing this
plugin.
Because, the core at that moment could be not ready.

Sheng Wu 吴晟
Twitter, wusheng1108


Li BingLong(智能平台) <bi...@sohu-inc.com> 于2021年1月4日周一 下午4:18写道:

> So is it supported or valuable to add a ThreadPoolExecutor plugin?
> I have tried this, some log not important will appear in stdout.
>
>
>
> 在 2021/1/4 下午3:15,“Sheng Wu”<wu...@gmail.com> 写入:
>
>     True, we can't instrument the class the agent core is using already.
>     This is a logic conflict. If we allow the instrumentation activated
> before
>     the configuration initialization and core service booting, you have a
>     chance to face runtime NPE or race condition.
>     The core developer and plugin developer would expect the booting
> sequence
>     will be unexpected random, and determined by the monitored app codes.
>
>     Sheng Wu 吴晟
>     Twitter, wusheng1108
>
>
>
>
>
>

Re: Cross Thread plugin

Posted by "Li BingLong (智能平台)" <bi...@sohu-inc.com>.
So is it supported or valuable to add a ThreadPoolExecutor plugin?
I have tried this, some log not important will appear in stdout.



在 2021/1/4 下午3:15,“Sheng Wu”<wu...@gmail.com> 写入:

    True, we can't instrument the class the agent core is using already.
    This is a logic conflict. If we allow the instrumentation activated before
    the configuration initialization and core service booting, you have a
    chance to face runtime NPE or race condition.
    The core developer and plugin developer would expect the booting sequence
    will be unexpected random, and determined by the monitored app codes.

    Sheng Wu 吴晟
    Twitter, wusheng1108


 



Re: Cross Thread plugin

Posted by Sheng Wu <wu...@gmail.com>.
True, we can't instrument the class the agent core is using already.
This is a logic conflict. If we allow the instrumentation activated before
the configuration initialization and core service booting, you have a
chance to face runtime NPE or race condition.
The core developer and plugin developer would expect the booting sequence
will be unexpected random, and determined by the monitored app codes.

Sheng Wu 吴晟
Twitter, wusheng1108


Li BingLong(智能平台) <bi...@sohu-inc.com> 于2021年1月4日周一 下午3:04写道:

> We can not carry the tracing context if we do not change the parameter.
> We must change the Runnable parameter of
> java.util.concurrent.Executor#execute to RunnableWrapper.
> But please note than it is different from that We use the RunnableWrapper
> in the user code.
> People do not need to change their code.
>
> Also, The FileWriter have used ThreadPoolExecutor.
> If we want to enhance the ThreadPoolExecutor,make sure
> org.apache.skywalking.apm.agent.core.conf.SnifferConfigInitializer#IS_INIT_COMPLETED
> is false.
>
>
>
>
> 在 2021/1/4 下午2:44,“Sheng Wu”<wu...@gmail.com> 写入:
>
>     About ThreadPoolExecutor, yes, you can instrument it. But as no change
> made
>     on the parameter, where do you expect to carry the tracing
>     context(Snapshot)?
>
>     Sheng Wu 吴晟
>     Twitter, wusheng1108
>
>
>
>
>
>

Re: Cross Thread plugin

Posted by "Li BingLong (智能平台)" <bi...@sohu-inc.com>.
Also, The FileWriter has used ThreadPoolExecutor.
If we want to enhance the ThreadPoolExecutor,make sure org.apache.skywalking.apm.agent.core.conf.SnifferConfigInitializer#IS_INIT_COMPLETED is false before we have enhanced the ThreadPoolExecutor.




在 2021/1/4 下午3:04,“Li BingLong(智能平台)”<bi...@sohu-inc.com> 写入:

    We can not carry the tracing context if we do not change the parameter.
    We must change the Runnable parameter of java.util.concurrent.Executor#execute to RunnableWrapper.
    But please note than it is different from that We use the RunnableWrapper in the user code.
    People do not need to change their code.

    Also, The FileWriter have used ThreadPoolExecutor.
    If we want to enhance the ThreadPoolExecutor,make sure org.apache.skywalking.apm.agent.core.conf.SnifferConfigInitializer#IS_INIT_COMPLETED is false.




    在 2021/1/4 下午2:44,“Sheng Wu”<wu...@gmail.com> 写入:

        About ThreadPoolExecutor, yes, you can instrument it. But as no change made
        on the parameter, where do you expect to carry the tracing
        context(Snapshot)?

        Sheng Wu 吴晟
        Twitter, wusheng1108







Re: Cross Thread plugin

Posted by "Li BingLong (智能平台)" <bi...@sohu-inc.com>.
We can not carry the tracing context if we do not change the parameter.
We must change the Runnable parameter of java.util.concurrent.Executor#execute to RunnableWrapper.
But please note than it is different from that We use the RunnableWrapper in the user code.
People do not need to change their code.

Also, The FileWriter have used ThreadPoolExecutor.
If we want to enhance the ThreadPoolExecutor,make sure org.apache.skywalking.apm.agent.core.conf.SnifferConfigInitializer#IS_INIT_COMPLETED is false.




在 2021/1/4 下午2:44,“Sheng Wu”<wu...@gmail.com> 写入:

    About ThreadPoolExecutor, yes, you can instrument it. But as no change made
    on the parameter, where do you expect to carry the tracing
    context(Snapshot)?

    Sheng Wu 吴晟
    Twitter, wusheng1108


    



Re: Cross Thread plugin

Posted by Sheng Wu <wu...@gmail.com>.
About ThreadPoolExecutor, yes, you can instrument it. But as no change made
on the parameter, where do you expect to carry the tracing
context(Snapshot)?

Sheng Wu 吴晟
Twitter, wusheng1108


Li BingLong(智能平台) <bi...@sohu-inc.com> 于2021年1月4日周一 下午2:40写道:

> How about enhance an Executor(ThreadPoolExecutor or something similar) if
> we can not enhance Runnable.
>
>
>
>
>
>
>
> *发件人**: *Sheng Wu <wu...@gmail.com>
> *日期**: *2021年1月4日 星期一 下午2:30
> *收件人**: *"Li BingLong(智能平台)" <bi...@sohu-inc.com>
> *抄送**: *"dev@skywalking.apache.org" <de...@skywalking.apache.org>
> *主题**: *Re: Cross Thread plugin
>
>
>
> First, you could learn what is lambda really in Java :) You will get the
> conclusion that is insane :P
>
> And about enhancing the Runnable, it is also very radical, as it is widely
> used in many places, even not in cross-thread scenarios. That is why we
> don't want to mass up the whole JVM
>
>
>
> Sheng Wu 吴晟
>
> Twitter, wusheng1108
>
>
>
>
>
> Li BingLong(智能平台) <bi...@sohu-inc.com> 于2021年1月4日周一 下午2:13写道:
>
> Hi!
>
> I have seen the pr.
> https://github.com/apache/skywalking/pull/845
>
> Frankly, this plugin is invasive compared to other plugins.
>
> We must write hard code with RunnableWrapper to use it.
> Why not enhance the Runnable class? This way we need to support lambda
> expression.
>
>

Re: Cross Thread plugin

Posted by "Li BingLong (智能平台)" <bi...@sohu-inc.com>.
How about enhance an Executor(ThreadPoolExecutor or something similar) if we can not enhance Runnable.



发件人: Sheng Wu <wu...@gmail.com>
日期: 2021年1月4日 星期一 下午2:30
收件人: "Li BingLong(智能平台)" <bi...@sohu-inc.com>
抄送: "dev@skywalking.apache.org" <de...@skywalking.apache.org>
主题: Re: Cross Thread plugin

First, you could learn what is lambda really in Java :) You will get the conclusion that is insane :P
And about enhancing the Runnable, it is also very radical, as it is widely used in many places, even not in cross-thread scenarios. That is why we don't want to mass up the whole JVM

Sheng Wu 吴晟
Twitter, wusheng1108


Li BingLong(智能平台) <bi...@sohu-inc.com>> 于2021年1月4日周一 下午2:13写道:
Hi!
I have seen the pr.
https://github.com/apache/skywalking/pull/845
Frankly, this plugin is invasive compared to other plugins.
We must write hard code with RunnableWrapper to use it.
Why not enhance the Runnable class? This way we need to support lambda expression.

Re: Cross Thread plugin

Posted by Sheng Wu <wu...@gmail.com>.
First, you could learn what is lambda really in Java :) You will get the
conclusion that is insane :P
And about enhancing the Runnable, it is also very radical, as it is widely
used in many places, even not in cross-thread scenarios. That is why we
don't want to mass up the whole JVM

Sheng Wu 吴晟
Twitter, wusheng1108


Li BingLong(智能平台) <bi...@sohu-inc.com> 于2021年1月4日周一 下午2:13写道:

> Hi!
>
> I have seen the pr.
> https://github.com/apache/skywalking/pull/845
>
> Frankly, this plugin is invasive compared to other plugins.
>
> We must write hard code with RunnableWrapper to use it.
> Why not enhance the Runnable class? This way we need to support lambda
> expression.
>