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/09/12 15:05:15 UTC
[GitHub] [skywalking-java] zifeihan opened a new pull request #22: Add thrift plugin support thrift TMultiplexedProcessor.
zifeihan opened a new pull request #22:
URL: https://github.com/apache/skywalking-java/pull/22
### Fix thrift plugin does not support thrift TMultiplexedProcessor, so NPE is triggered
- [ ] Add a unit test to verify that the fix works.
It has been tested through the demo submitted by the user. I have been too busy recently. I will submit the test scenario in two days.
- [x] Explain briefly why the bug exists and how to fix it.
Check [7571]( https://github.com/apache/skywalking/issues/7571) to understand the cause of the bug and how to fix it.
- [x] If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #7571 .
- [x] Update the [`CHANGES` log](https://github.com/apache/skywalking/blob/master/CHANGES.md).
--
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
[GitHub] [skywalking-java] wu-sheng merged pull request #22: Add thrift plugin support thrift TMultiplexedProcessor.
Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #22:
URL: https://github.com/apache/skywalking-java/pull/22
--
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
[GitHub] [skywalking-java] dmsolr commented on pull request #22: Add thrift plugin support thrift TMultiplexedProcessor.
Posted by GitBox <gi...@apache.org>.
dmsolr commented on pull request #22:
URL: https://github.com/apache/skywalking-java/pull/22#issuecomment-917684778
I think this is an enhancement. :)
We don't implement all `Processor`s.
--
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
[GitHub] [skywalking-java] zifeihan edited a comment on pull request #22: Add thrift plugin support thrift TMultiplexedProcessor.
Posted by GitBox <gi...@apache.org>.
zifeihan edited a comment on pull request #22:
URL: https://github.com/apache/skywalking-java/pull/22#issuecomment-918105775
The object here is new, so it does not go through ServerInProtocolWrapper, so special judgment is required in org.apache.skywalking.apm.plugin.thrift.TBaseProcessorInterceptor
--
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
[GitHub] [skywalking-java] zifeihan commented on pull request #22: Add thrift plugin support thrift TMultiplexedProcessor.
Posted by GitBox <gi...@apache.org>.
zifeihan commented on pull request #22:
URL: https://github.com/apache/skywalking-java/pull/22#issuecomment-918105775
The object here is new, so it does not go through ServerInProtocolWrapper, so special judgment is required
--
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
[GitHub] [skywalking-java] wu-sheng commented on pull request #22: Add thrift plugin support thrift TMultiplexedProcessor.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #22:
URL: https://github.com/apache/skywalking-java/pull/22#issuecomment-918694618
> LGTM, @zifeihan told me who is too busy to complete docs.
Hi, we don't have typically documents to host all logic of plugins, they are too complex to describe. AFAIK, only very few plugin added README.md in the root of plugin source codes only. @dmsolr If you want to, welcome to send a pull request later.
--
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
[GitHub] [skywalking-java] zifeihan commented on pull request #22: Add thrift plugin support thrift TMultiplexedProcessor.
Posted by GitBox <gi...@apache.org>.
zifeihan commented on pull request #22:
URL: https://github.com/apache/skywalking-java/pull/22#issuecomment-917735362
> @zifeihan maybe we need to list of `Processor`s in docs which we have enhanced, WDYT?
Ihink it is necessary. Currently thrift only has 3 processors, and we currently support all of them.
--
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
[GitHub] [skywalking-java] dmsolr commented on pull request #22: Add thrift plugin support thrift TMultiplexedProcessor.
Posted by GitBox <gi...@apache.org>.
dmsolr commented on pull request #22:
URL: https://github.com/apache/skywalking-java/pull/22#issuecomment-917685642
@zifeihan maybe we need to list of `Processor`s in docs which we have enhanced, WDYT?
--
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
[GitHub] [skywalking-java] dmsolr commented on pull request #22: Add thrift plugin support thrift TMultiplexedProcessor.
Posted by GitBox <gi...@apache.org>.
dmsolr commented on pull request #22:
URL: https://github.com/apache/skywalking-java/pull/22#issuecomment-917685205
Hi @lzdujing Please check it if you have 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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] dmsolr commented on pull request #22: Add thrift plugin support thrift TMultiplexedProcessor.
Posted by GitBox <gi...@apache.org>.
dmsolr commented on pull request #22:
URL: https://github.com/apache/skywalking-java/pull/22#issuecomment-917757495
> > @zifeihan maybe we need to list of `Processor`s in docs which we have enhanced, WDYT?
>
> I think it is necessary. Currently thrift only has 3 processors, and we currently support all of them.
Actually, not only these are 3. There are also many wrapped processors.
--
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
[GitHub] [skywalking-java] zifeihan edited a comment on pull request #22: Add thrift plugin support thrift TMultiplexedProcessor.
Posted by GitBox <gi...@apache.org>.
zifeihan edited a comment on pull request #22:
URL: https://github.com/apache/skywalking-java/pull/22#issuecomment-917735362
> @zifeihan maybe we need to list of `Processor`s in docs which we have enhanced, WDYT?
I think it is necessary. Currently thrift only has 3 processors, and we currently support all of them.
--
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