-
Notifications
You must be signed in to change notification settings - Fork 110
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
Add support for tracking http2 client and server requests #568
Conversation
@@ -252,7 +252,7 @@ static __always_inline void get_or_create_trace_info(http_connection_metadata_t | |||
#ifdef BPF_TRACEPARENT | |||
// The below buffer scan can be expensive on high volume of requests. We make it optional | |||
// for customers to enable it. Off by default. | |||
if (!capture_header_buffer || !bpf_core_enum_value_exists(enum bpf_func_id, BPF_FUNC_loop)) { | |||
if (!capture_header_buffer) { |
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.
We ifdef around this support, so the runtime check wasn't needed. Plus it was causing compile issues on ARM64 Linux.
@@ -128,12 +128,24 @@ var structMembers = map[string]structInfo{ | |||
"nextID": "http2_client_next_id_pos", | |||
}, | |||
}, | |||
"golang.org/x/net/http2/hpack.Encoder": { |
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.
I noticed that sometimes hpack.Encode doesn't have to be the vendor version in Go. Added it just to be on the safe side.
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! Once integration tests are fixed, I'm fine with merging.
configs/offsets/tracker_input.json
Outdated
@@ -1,7 +1,7 @@ | |||
{ | |||
"go": { | |||
"versions": ">= 1.17", | |||
"inspect": "./configs/offsets/std_inspect.go", | |||
"versions": ">= 1.18", |
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.
Reminder: We will need to document this and state it clearly in the release notes.
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.
Adding the breaking change label, as I understand that we won't support Go 1.17 anymore, right?
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.
I fixed the go-offsets tracker to support custom files for modules, so we can now bring back Go 1.17, no issues.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #568 +/- ##
==========================================
- Coverage 45.02% 44.85% -0.17%
==========================================
Files 67 67
Lines 5664 5678 +14
==========================================
- Hits 2550 2547 -3
- Misses 2964 2981 +17
Partials 150 150
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
This PR adds support for tracking http2 client and server requests. Couple of things to note: