fix: no data receive from grpc#103
Conversation
LinuxSuRen
changed the title
|
I can see gRPC plugin exists in dev only with tests. Have you cross verified with that? |
|
FYI @Alipebt as you added gRPC plugin. |
Could you be more specific? I just checked the result before and after with this PR. By the way, I checked the interface name of gRPC client. It is https://github.com/grpc/grpc-go/blob/fe0dc2275d7dfdd61c6915bd3a4ec5bff37963e3/clientconn.go#L618 Any way, please let me know if anyone konw the difference. |
LinuxSuRen
changed the title
|
I mean this is the test, All contributed plugins, including the ones made by the maintainers, require tests with various declared versions, as well as codes to help further users/contributora know what are tested. This doc could help you to understand the tests. https://skywalking.apache.org/docs/skywalking-go/next/en/development-and-contribution/write-plugin-testing/ All these make maintainers don't debug the plugin for users and contributors. You could cross check, do local verification, and submit a patch or an enhancement with new test codes. |
| "github.com/dave/dst/dstutil" | ||
| ) | ||
|
|
||
| type Buildin struct { |
There was a problem hiding this comment.
Why you need to add this? It's not related to the grpc plugin.
There was a problem hiding this comment.
Sorry about this. I forgot to remove the unrelated lines.
| }, | ||
| { | ||
| PackagePath: "", | ||
| At: instrument.NewMethodEnhance("*ClientConnInterface", "Invoke", |
There was a problem hiding this comment.
The interface cannot be intercepted, and I think intercepting ClientConnInterface is not effective.
ClientConn implements this interface, and when user make RPC calls, the Invoke and NewStream methods implemented by ClientConn are executed.
There was a problem hiding this comment.
Let me do more tests. I didn't get the tracing data of gRPC before. Below is my usage.
There was a problem hiding this comment.
If you mean you need more intercepting points to support more gRPC usage, that is good.
You need to update the tests accordingly, with new usage of the gRPC APIs, and new expected segments/spans.
There was a problem hiding this comment.
Sure. I need more time to make sure my use case is correct. I will add the corresponding tests after that.
There was a problem hiding this comment.
I feel very confused. The e2e test seem correct, but it cannot work in my case. So, I tend to close this PR until I can figure it out.
By the way, I didn't find a way to control if sending the data to SK server or not. For some use cases (at least in my cas), SkyWalking is not required for the installation. So, I'm wonderfing if there is a way to control it.
I found a possible solution is that. Read an environment variable SW_AGENT_ENABLED, return false if its value is false.
There was a problem hiding this comment.
By the way, I didn't find a way to control if sending the data to SK server or not
No, you can't. That is kernel responsibility, including connection management, TLS, traffic limitation, etc.
For some use cases (at least in my cas), SkyWalking is not required for the installation
What do you mean some cases? Do you mean some URL or protocol? Or something else? You want to compile the agent into your binary, but not using it?
There was a problem hiding this comment.
What do you mean some cases? Do you mean some URL or protocol? Or something else? You want to compile the agent into your binary, but not using it?
Yes. Users might only want to install the tool instead of other optional component. For example my test tool, the following could start a server:
atest server
It will output the following errors if no SK server given. I'm thinking about if it's possible to avoid printing the errors.
skywalking-go 2023/09/25 14:58:14 open stream error rpc error: code = Unavailable desc = connection error: desc = "transport: Error while dialing: dial tcp 0.0.0.0:11800: connect: connection refused"
There was a problem hiding this comment.
In the previous discussion, we were not considering a general use tool would adopt this agent. So, the old proposal is going to package two binaries, one with the agent, the other is not. This should be good enough for a business service.
@mrproliu Do we have the mechanism for the runtime configuration for now? If so, maybe we could consider add one to disable the agent.
There was a problem hiding this comment.
Thanks for your explanation. Consider my project is very small and simple, having mutiple distribution is not a good choice. As you mentioned, it's not problem for those business vendors.
There was a problem hiding this comment.
Let's focus on the feature at here.
|
What data are you referring to? I think the picture you provided is gRPC related data. |
I am wondering that too. @LinuxSuRen Could you explain what is your point of asking? The agent would not intercept your all codes, or not have to be the codes(APIs) you are calling. The meaning of exit spans(your screenshots) measures the performance of remote peer(server side and the network) as much as possible. |
|
This is a background you should learn about distributed tracing, https://research.google/pubs/pub36356/. |
I can see the data from that picture when I build hi @wu-sheng , for the background of my project. The workflow is below: A -> HTTP API -> call the extension which is a gRPC server. So, sk could get the gRPC tracing data if I understand correctly. Currently, I only inject sk into the gRPC client side. |
|
Follow this if that is your point, #103 (comment) |
|
Close this PR until I could figure it out. |
|
Finally, I figured out the reason. It's too embarrassing that I used |
4 participants
What happend
I cannot see the gRPC related data from SkyWaling native UI.
Test
See the following screenshots with the changes:
I read part code lines of this project. But I'm still not familar with the picture. Please let me know if I didn't understand the related codes correctly.
By the way, I still try to figure out how to add the corresponding unit tests for these changes.