You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2021/02/02 06:26:16 UTC
[GitHub] [skywalking] whfjam opened a new issue #6305: about httpasyncclient4.x enhancement
whfjam opened a new issue #6305:
URL: https://github.com/apache/skywalking/issues/6305
Please answer these questions before submitting your issue.
- Why do you submit this issue?
- [*] Question or discussion
- [ ] Bug
- [ ] Requirement
- [ ] Feature or performance improvement
___
### Question
In the enhancement of httpasyncclient4.x because of the class SessionRequestImpl was not always be created（when reuse the connection），so the enhancement was failed when resuse the httpconnection。
I have found a way to correct this problem by enhancing the class “org.apache.http.impl.nio.client.DefaultClientExchangeHandlerImpl” as it was always being created new，and i could get it from HttpAsyncRequestExecutor.requestReady so that i could use it to deliver the skywalkingdynamicfield。
But the most important thing was that the DefaultClientExchangeHandlerImpl is not “public”，i wonder if it is a right thing to enhance the class which is not “public” in skywalking。
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #6305: about httpasyncclient4.x enhancement
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #6305:
URL: https://github.com/apache/skywalking/issues/6305#issuecomment-771502238
Thanks for providing the details.
> In the enhancement of httpasyncclient4.x because of the class SessionRequestImpl was not always be created（when reuse the connection），so the enhancement was failed when resuse the httpconnection。
In this case, then it is a bug for sure.
> But the most important thing was that the DefaultClientExchangeHandlerImpl is not “public”，i wonder if it is a right thing to enhance the class which is not “public” in skywalking。
Not a public class is fine and acceptable for SkyWalking. We don't care how the original codes open classes, we change it based on how they works and make sure the context propagation works.
As you already have the idea, feel free to submit a pull request to fix, and if you could enhance the plugin test to verify it, it is better.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #6305: about httpasyncclient4.x enhancement
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #6305:
URL: https://github.com/apache/skywalking/issues/6305#issuecomment-771502238
Thanks for providing the details.
> In the enhancement of httpasyncclient4.x because of the class SessionRequestImpl was not always be created（when reuse the connection），so the enhancement was failed when resuse the httpconnection。
In this case, then it is a bug for sure.
> But the most important thing was that the DefaultClientExchangeHandlerImpl is not “public”，i wonder if it is a right thing to enhance the class which is not “public” in skywalking。
Not a public class is fine and acceptable for SkyWalking. We don't care how the original codes open classes, we change it based on how they works and make sure the context propagation works.
As you already have the idea, feel free to submit a pull request to fix, and if you could enhance the plugin test to verify it, it is better.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #6305: about httpasyncclient4.x enhancement
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #6305:
URL: https://github.com/apache/skywalking/issues/6305#issuecomment-774635327
Don't hurry, take your time.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] whfjam commented on issue #6305: about httpasyncclient4.x enhancement
Posted by GitBox <gi...@apache.org>.
whfjam commented on issue #6305:
URL: https://github.com/apache/skywalking/issues/6305#issuecomment-774615798
>
>
> Thanks for providing the details.
>
> > In the enhancement of httpasyncclient4.x because of the class SessionRequestImpl was not always be created（when reuse the connection），so the enhancement was failed when resuse the httpconnection。
>
> In this case, then it is a bug for sure.
>
> > But the most important thing was that the DefaultClientExchangeHandlerImpl is not “public”，i wonder if it is a right thing to enhance the class which is not “public” in skywalking。
>
> Not a public class is fine and acceptable for SkyWalking. We don't care how the original codes open classes, we change it based on how they works and make sure the context propagation works.
>
> As you already have the idea, feel free to submit a pull request to fix, and if you could enhance the plugin test to verify it, it is better.
I'm doing this ,and i found there were lots of test cases to be done（such as disconnect，read timeout， connect timed out），so I need more time to complete it
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng closed issue #6305: about httpasyncclient4.x enhancement
Posted by GitBox <gi...@apache.org>.
wu-sheng closed issue #6305:
URL: https://github.com/apache/skywalking/issues/6305
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org