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/05/07 04:31:15 UTC
[GitHub] [skywalking] xuanyu66 opened a new pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
xuanyu66 opened a new pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910
<!--
⚠️ Please make sure to read this template first, pull requests that don't accord with this template
maybe closed without notice.
Texts surrounded by `<` and `>` are meant to be replaced by you, e.g. <framework name>, <issue number>.
Put an `x` in the `[ ]` to mark the item as CHECKED. `[x]`
-->
### Fix <bug description or the bug issue number or bug issue link>
- [ ] Add a unit test to verify that the fix works.
- [x] Explain briefly why the bug exists and how to fix it.
When transport is't enhanced by plugin, TServiceClient will fail to set SkyWalkingDynamicField, which make create exit span failed.
I validate transport, if it is not enhanced, set the SkyWalkingDynamicField with "UNKNOWN"
<!-- ==== 🔌 Remove this line WHEN AND ONLY WHEN you're adding a new plugin, follow the checklist 👇 ====
### Add an agent plugin to support <framework name>
- [ ] Add a test case for the new plugin, refer to [the doc](https://github.com/apache/skywalking/blob/master/docs/en/guides/Plugin-test.md)
- [ ] Add a component id in [the component-libraries.yml](https://github.com/apache/skywalking/blob/master/oap-server/server-bootstrap/src/main/resources/component-libraries.yml)
- [ ] Add a logo in [the UI repo](https://github.com/apache/skywalking-rocketbot-ui/tree/master/src/views/components/topology/assets)
==== 🔌 Remove this line WHEN AND ONLY WHEN you're adding a new plugin, follow the checklist 👆 ==== -->
<!-- ==== 📈 Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist 👇 ====
### Improve the performance of <class or module or ...>
- [ ] Add a benchmark for the improvement, refer to [the existing ones](https://github.com/apache/skywalking/blob/master/apm-commons/apm-datacarrier/src/test/java/org/apache/skywalking/apm/commons/datacarrier/LinkedArrayBenchmark.java)
- [ ] The benchmark result.
```text
<Paste the benchmark results here>
```
- [ ] Links/URLs to the theory proof or discussion articles/blogs. <links/URLs here>
==== 📈 Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist 👆 ==== -->
<!-- ==== 🆕 Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist 👇 ====
### <Feature description>
- [ ] If this is non-trivial feature, paste the links/URLs to the design doc.
- [ ] Update the documentation to include this new feature.
- [ ] Tests(including UT, IT, E2E) are added to verify the new feature.
- [ ] If it's UI related, attach the screenshots below.
==== 🆕 Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist 👆 ==== -->
- [ ] If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #<issue number>.
- [ ] 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] xuanyu66 commented on pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
xuanyu66 commented on pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910#issuecomment-834113981
> > > > > Do you mean customizing transport?
> > > >
> > > >
> > > > yes, some package maybe customize transport , we need some safeguard
> > >
> > >
> > > This is a known issue. In this scenario, there are two things maybe we need to consider.
> > >
> > > 1. whether we can get the "peer" from transport
> > > 2. can we cover all customized cases
> >
> >
> > I think we just set UNKNOWN, if someone want to enhance some customized transport, they can contribute themselves, we should only support official transport.
> > What's your opinion?
>
> My question would be, if this is not an official transport implementation, why should SkyWalking take care of? User could remove the thrift plugin directly.
we want to trace thrift rpc call, and it work properly. But other dependency with customized transport cause IllegalStateException.
--
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] xuanyu66 commented on pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
xuanyu66 commented on pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910#issuecomment-834081783
> Could you explain about new transport? Why wouldn't we enhance that?
It's jaeger that implement ThriftUdpTransport for UdpSender
```
public class UdpSender extends ThriftSender {
public static final String DEFAULT_AGENT_UDP_HOST = "localhost";
public static final int DEFAULT_AGENT_UDP_COMPACT_PORT = 6831;
@ToString.Exclude private Agent.Client agentClient;
@ToString.Exclude private ThriftUdpTransport udpTransport;
/**
* This constructor expects Jaeger running running on {@value #DEFAULT_AGENT_UDP_HOST}
* and port {@value #DEFAULT_AGENT_UDP_COMPACT_PORT}
*/
public UdpSender() {
this(DEFAULT_AGENT_UDP_HOST, DEFAULT_AGENT_UDP_COMPACT_PORT, 0);
}
/**
* @param host host e.g. {@value DEFAULT_AGENT_UDP_HOST}
* @param port port e.g. {@value DEFAULT_AGENT_UDP_COMPACT_PORT}
* @param maxPacketSize if 0 it will use {@value ThriftUdpTransport#MAX_PACKET_SIZE}
*/
public UdpSender(String host, int port, int maxPacketSize) {
super(ProtocolType.Compact, maxPacketSize);
if (host == null || host.length() == 0) {
host = DEFAULT_AGENT_UDP_HOST;
}
if (port == 0) {
port = DEFAULT_AGENT_UDP_COMPACT_PORT;
}
udpTransport = ThriftUdpTransport.newThriftUdpClient(host, port);
agentClient = new Agent.Client(protocolFactory.getProtocol(udpTransport));
}
```
--
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] dmsolr commented on pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
dmsolr commented on pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910#issuecomment-834086468
Do you mean customizing transport?
--
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 pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910#issuecomment-834056804
I think the question is, why not enhanced? Which should not happen.
--
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] xuanyu66 commented on pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
xuanyu66 commented on pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910#issuecomment-834097582
> > > Do you mean customizing transport?
> >
> >
> > yes, some package maybe customize transport , we need some safeguard
>
> This is a known issue. In this scenario, there are two things maybe we need to consider.
>
> 1. whether we can get the "peer" from transport
> 2. can we cover all customized cases
I think we just set UNKNOWN, if someone want to enhance some customized transport, they can contribute themselves, we should only support official transport
--
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 pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910#issuecomment-834090774
Please resolve conflict.
--
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 pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910#issuecomment-834102936
> > > > Do you mean customizing transport?
> > >
> > >
> > > yes, some package maybe customize transport , we need some safeguard
> >
> >
> > This is a known issue. In this scenario, there are two things maybe we need to consider.
> >
> > 1. whether we can get the "peer" from transport
> > 2. can we cover all customized cases
>
> I think we just set UNKNOWN, if someone want to enhance some customized transport, they can contribute themselves, we should only support official transport.
> What's your opinion?
My question would be, if this is not an official transport implementation, why should SkyWalking take care of? User could remove the thrift plugin directly.
--
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] xuanyu66 edited a comment on pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
xuanyu66 edited a comment on pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910#issuecomment-834097582
--
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] xuanyu66 commented on pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
xuanyu66 commented on pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910#issuecomment-834138108
> > > > > > > > > Do you mean customizing transport?
> > > > > > > >
> > > > > > > >
> > > > > > > > yes, some package maybe customize transport , we need some safeguard
> > > > > > >
> > > > > > >
> > > > > > > This is a known issue. In this scenario, there are two things maybe we need to consider.
> > > > > > >
> > > > > > > 1. whether we can get the "peer" from transport
> > > > > > > 2. can we cover all customized cases
> > > > > >
> > > > > >
> > > > > > I think we just set UNKNOWN, if someone want to enhance some customized transport, they can contribute themselves, we should only support official transport.
> > > > > > What's your opinion?
> > > > >
> > > > >
> > > > > My question would be, if this is not an official transport implementation, why should SkyWalking take care of? User could remove the thrift plugin directly.
> > > >
> > > >
> > > > we want to trace thrift rpc call, and it work properly. But other dependency with customized transport cause IllegalStateException.
> > >
> > >
> > > It seems strange. Are you saying your thrift is using 2 kinds of transport implementations in one service? Why?
> >
> >
> > We use official transport to call rpc, jaeger use its customized transport to report trace span.
> > there are two different thrift client
>
> That is strange, why SkyWalking needs to work with Jaeger? What is the use case?
Which I want to say is that other package may implement their own customized transport to send message at the same time we use official transport to call rpc between services. Should we take this situation into account.
--
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 pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910#issuecomment-834140870
> > > > > > > > > > Do you mean customizing transport?
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > yes, some package maybe customize transport , we need some safeguard
> > > > > > > >
> > > > > > > >
> > > > > > > > This is a known issue. In this scenario, there are two things maybe we need to consider.
> > > > > > > >
> > > > > > > > 1. whether we can get the "peer" from transport
> > > > > > > > 2. can we cover all customized cases
> > > > > > >
> > > > > > >
> > > > > > > I think we just set UNKNOWN, if someone want to enhance some customized transport, they can contribute themselves, we should only support official transport.
> > > > > > > What's your opinion?
> > > > > >
> > > > > >
> > > > > > My question would be, if this is not an official transport implementation, why should SkyWalking take care of? User could remove the thrift plugin directly.
> > > > >
> > > > >
> > > > > we want to trace thrift rpc call, and it work properly. But other dependency with customized transport cause IllegalStateException.
> > > >
> > > >
> > > > It seems strange. Are you saying your thrift is using 2 kinds of transport implementations in one service? Why?
> > >
> > >
> > > We use official transport to call rpc, jaeger use its customized transport to report trace span.
> > > there are two different thrift client
> >
> >
> > That is strange, why SkyWalking needs to work with Jaeger? What is the use case?
>
> Which I want to say is that other package may implement their own customized transport to send message at the same time we use official transport to call rpc between services. Should we take this situation into account.
I have to repeat the point I said before. `MAY` is not a thing the community should take care of. Some real and reasonable use cases are. It is really rare to see a service using 2 RPC protocols but sharing the same framework. And also, you mentioned, SkyWalking and Jaeger are being deployed at the same time. We have no intention to compete, but also I have no vision why you need both at the same time, especially you are talking in Java stack.
Maybe you have something that can't be shared in public, but at the same time, I am afraid, open-source community and project are responding to open information only.
--
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] xuanyu66 commented on pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
xuanyu66 commented on pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910#issuecomment-834088992
> Do you mean customizing transport?
yes, some package maybe customize transport , we need some safeguard
--
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] dmsolr commented on pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
dmsolr commented on pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910#issuecomment-834093980
> > Do you mean customizing transport?
>
> yes, some package maybe customize transport , we need some safeguard
This is a known issue. In this scenario, there are two things maybe we need to consider.
1. whether we can get the "peer" from transport
2. can we cover all customized cases
--
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] xuanyu66 commented on pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
xuanyu66 commented on pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910#issuecomment-835282138
> Should we close this? Or you want to add more context?
yes, close this
--
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 pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910#issuecomment-834223980
Should we close this? Or you want to add more context?
--
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] xuanyu66 commented on pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
xuanyu66 commented on pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910#issuecomment-834222498
> > > > > > > > > > > Do you mean customizing transport?
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > yes, some package maybe customize transport , we need some safeguard
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > This is a known issue. In this scenario, there are two things maybe we need to consider.
> > > > > > > > >
> > > > > > > > > 1. whether we can get the "peer" from transport
> > > > > > > > > 2. can we cover all customized cases
> > > > > > > >
> > > > > > > >
> > > > > > > > I think we just set UNKNOWN, if someone want to enhance some customized transport, they can contribute themselves, we should only support official transport.
> > > > > > > > What's your opinion?
> > > > > > >
> > > > > > >
> > > > > > > My question would be, if this is not an official transport implementation, why should SkyWalking take care of? User could remove the thrift plugin directly.
> > > > > >
> > > > > >
> > > > > > we want to trace thrift rpc call, and it work properly. But other dependency with customized transport cause IllegalStateException.
> > > > >
> > > > >
> > > > > It seems strange. Are you saying your thrift is using 2 kinds of transport implementations in one service? Why?
> > > >
> > > >
> > > > We use official transport to call rpc, jaeger use its customized transport to report trace span.
> > > > there are two different thrift client
> > >
> > >
> > > That is strange, why SkyWalking needs to work with Jaeger? What is the use case?
> >
> >
> > Which I want to say is that other package may implement their own customized transport to send message at the same time we use official transport to call rpc between services. Should we take this situation into account.
>
> I have to repeat the point I said before. `MAY` is not a thing the community should take care of. Some real and reasonable use cases are. It is really rare to see a service using 2 RPC protocols but sharing the same framework. And also, you mentioned, SkyWalking and Jaeger are being deployed at the same time. We have no intention to compete, but also I have no vision why you need both at the same time, especially you are talking in Java stack.
> Maybe you have something that can't be shared in public, but at the same time, I am afraid, open-source community and project are responding to open information only.
OK,I get your point
--
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] dmsolr commented on pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
dmsolr commented on pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910#issuecomment-834107354
I agree with @wu-sheng said. Thrift plugin is complex enough and easy to break other cases. I am worried about this.
--
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 pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910#issuecomment-834133283
> > > > > > > > Do you mean customizing transport?
> > > > > > >
> > > > > > >
> > > > > > > yes, some package maybe customize transport , we need some safeguard
> > > > > >
> > > > > >
> > > > > > This is a known issue. In this scenario, there are two things maybe we need to consider.
> > > > > >
> > > > > > 1. whether we can get the "peer" from transport
> > > > > > 2. can we cover all customized cases
> > > > >
> > > > >
> > > > > I think we just set UNKNOWN, if someone want to enhance some customized transport, they can contribute themselves, we should only support official transport.
> > > > > What's your opinion?
> > > >
> > > >
> > > > My question would be, if this is not an official transport implementation, why should SkyWalking take care of? User could remove the thrift plugin directly.
> > >
> > >
> > > we want to trace thrift rpc call, and it work properly. But other dependency with customized transport cause IllegalStateException.
> >
> >
> > It seems strange. Are you saying your thrift is using 2 kinds of transport implementations in one service? Why?
>
> We use official transport to call rpc, jaeger use its customized transport to report trace span.
> there are two different thrift client
That is strange, why SkyWalking needs to work with Jaeger? What is the use case?
--
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 pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910#issuecomment-834058913
Could you explain about new transport? Why wouldn't we enhance that?
--
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 pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
wu-sheng closed pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910
--
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] xuanyu66 commented on pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
xuanyu66 commented on pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910#issuecomment-834092534
> Please resolve conflict.
done
--
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] xuanyu66 commented on pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
xuanyu66 commented on pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910#issuecomment-834131844
> > > > > > > Do you mean customizing transport?
> > > > > >
> > > > > >
> > > > > > yes, some package maybe customize transport , we need some safeguard
> > > > >
> > > > >
> > > > > This is a known issue. In this scenario, there are two things maybe we need to consider.
> > > > >
> > > > > 1. whether we can get the "peer" from transport
> > > > > 2. can we cover all customized cases
> > > >
> > > >
> > > > I think we just set UNKNOWN, if someone want to enhance some customized transport, they can contribute themselves, we should only support official transport.
> > > > What's your opinion?
> > >
> > >
> > > My question would be, if this is not an official transport implementation, why should SkyWalking take care of? User could remove the thrift plugin directly.
> >
> >
> > we want to trace thrift rpc call, and it work properly. But other dependency with customized transport cause IllegalStateException.
>
> It seems strange. Are you saying your thrift is using 2 kinds of transport implementations in one service? Why?
We use official transport to call rpc, jaeger use its customized transport to report trace span
--
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] xuanyu66 edited a comment on pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
xuanyu66 edited a comment on pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910#issuecomment-834131844
> > > > > > > Do you mean customizing transport?
> > > > > >
> > > > > >
> > > > > > yes, some package maybe customize transport , we need some safeguard
> > > > >
> > > > >
> > > > > This is a known issue. In this scenario, there are two things maybe we need to consider.
> > > > >
> > > > > 1. whether we can get the "peer" from transport
> > > > > 2. can we cover all customized cases
> > > >
> > > >
> > > > I think we just set UNKNOWN, if someone want to enhance some customized transport, they can contribute themselves, we should only support official transport.
> > > > What's your opinion?
> > >
> > >
> > > My question would be, if this is not an official transport implementation, why should SkyWalking take care of? User could remove the thrift plugin directly.
> >
> >
> > we want to trace thrift rpc call, and it work properly. But other dependency with customized transport cause IllegalStateException.
>
> It seems strange. Are you saying your thrift is using 2 kinds of transport implementations in one service? Why?
We use official transport to call rpc, jaeger use its customized transport to report trace span.
there are two different thrift client
--
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 pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910#issuecomment-834116199
> > > > > > Do you mean customizing transport?
> > > > >
> > > > >
> > > > > yes, some package maybe customize transport , we need some safeguard
> > > >
> > > >
> > > > This is a known issue. In this scenario, there are two things maybe we need to consider.
> > > >
> > > > 1. whether we can get the "peer" from transport
> > > > 2. can we cover all customized cases
> > >
> > >
> > > I think we just set UNKNOWN, if someone want to enhance some customized transport, they can contribute themselves, we should only support official transport.
> > > What's your opinion?
> >
> >
> > My question would be, if this is not an official transport implementation, why should SkyWalking take care of? User could remove the thrift plugin directly.
>
> we want to trace thrift rpc call, and it work properly. But other dependency with customized transport cause IllegalStateException.
It seems strange. Are you saying your thrift is using 2 kinds of transport implementations in one service? Why?
--
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] xuanyu66 commented on pull request #6910: FIX: Failed to create ExitSpan when thrift transport is not enhanced
Posted by GitBox <gi...@apache.org>.
xuanyu66 commented on pull request #6910:
URL: https://github.com/apache/skywalking/pull/6910#issuecomment-834057590
> I think the question is, why not enhanced? Which should not happen.
If Someone import some package which implement new transport, which is situation we met.
I think it can not avoid，we need some Safeguard
--
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