-
Notifications
You must be signed in to change notification settings - Fork 127
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
Allow sockmsg to run independently from tctracer #1799
Conversation
050e84c
to
75dda82
Compare
Hey @grafsean, do you have any ideas what may be going with the documentation checks? |
08f9e42
to
00ac3a7
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1799 +/- ##
==========================================
+ Coverage 65.36% 67.23% +1.87%
==========================================
Files 203 221 +18
Lines 20884 22693 +1809
==========================================
+ Hits 13650 15258 +1608
- Misses 6360 6659 +299
+ Partials 874 776 -98
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
00ac3a7
to
e78b5ab
Compare
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! I really like how you've split this now. I left a few comments, to not block you I'm approving the PR, but I think we should fix the handling of the two variables now. Changing to enum is perhaps a breaking change, perhaps the trick is to choose a better name for the IP side. Essentially IP propagation can't work without context_propagation. Maybe something along the lines of disable_ip_options_context_propagation
. This way we won't explicitly have to name it in the code. Alternatively, we can introduce a secondary enum, rather than replacing the enable_context_propagation
boolean. The enum can be context_propagation_mode
or something like that, with values: All, headersOnly,ipOptionsOnly
7107912
to
5785f74
Compare
Rebased + introduced enum. |
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! Thanks for addressing the feedback!
Allow the sockmsg and sockops programs to run independently from the tctracer. This allows the HTTP header trace context propagation to continue to operate in case the TC programs fail for whatever reason.
A new option,
BEYLA_BPF_CONTEXT_PROPAGATION
_ which lets you select what kind of context propagation you want to use has been introduced and supersedesBEYLA_BPF_ENABLE_CONTEXT_PROPAGATION