-
Notifications
You must be signed in to change notification settings - Fork 108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Various fixes for corrupt gRPC URLs and wrong request directions #930
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #930 +/- ##
==========================================
+ Coverage 72.28% 79.34% +7.06%
==========================================
Files 130 131 +1
Lines 10026 10125 +99
==========================================
+ Hits 7247 8034 +787
+ Misses 2166 1582 -584
+ Partials 613 509 -104
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow! so many changes 🚀🚀🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR fixes a few edge cases in how we were handling gRPC requests:
a. For kprobes, we keep connections sortted, but now I track the original destination port when we parse the connection information. Before we send the traces to userspace we validate if this original port matches what we expect for client/server, and if not we swap the connection values.
b. For Go, since we already know what's server and what's client, we don't sort the connection info and only sort it before we store it in the
trace_map
(which we share with kprobes for black-box context propagation). We swap the original connection tuples before we send the Go server traces (HTTP, HTTP2 or gRPC) to userspace, because userspace expects the destination port to be the server port.