feat: support JSON-RPC 2.0 over http in golang's uprobe #161
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #161 +/- ##
==========================================
- Coverage 71.02% 63.27% -7.75%
==========================================
Files 177 178 +1
Lines 19121 19161 +40
==========================================
- Hits 13580 12125 -1455
- Misses 4815 6355 +1540
+ Partials 726 681 -45
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:
|
|
Thanks for this @titaneric ! I checked the 5.15 kernel failure and it appears that there's some code that the old verifier complains about: |
|
If you need help debugging this let us know. |
|
Ohh, I would check it in the old kernel version. I develop this in relatively new version. |
titaneric
force-pushed
the
feat/otel-bpf-patch-4
branch
2 times, most recently
from
7799536 to
2bdba15
Compare
|
ok, I could reproduce this verifier error ran on Ubuntu 22.04, and I could try to resolve it. |
|
I fix it in kernel 5.15, and it also works on newer kernel such as 6.11 as well! If you find any verifier error on different kernel, please let me know! |
|
@titaneric do you think it would be possible to add an integration test as well? Similar to the example you mentioned in the review description. Happy to help with that as well if you prefer to share the burden. |
|
Yes! I would like to add the integration test as well. |
|
I have hit verifier error (max instrution limit) in kernel 5.15 in 2976d5e . I would try to resolve it first and update everything you suggest and test in both kernel. |
|
@titaneric whatever is easier for you - the max instruction limit error may warrant a tail call (feel free to paste the error - I checked the logs for the failing VM 5.15 integration test but I couldn't spot it there - for some reason the otel collector appears to be crashing?) |
|
Thank you! The verifier error on kernel 5.15 is attached as follows. |
|
Thanks @titaneric - yeah it seems we will have to tailcall. The fact that it didn't happen earlier is sheer luck - the compiler is generating a number of instructions that sits close to the allowed limit - so any changes in the code end up triggering it. If you need a reference implementation, have a look at: If you need help with this, let me know and I can try and have a go at it tomorrow. |
|
I approximately know how tail call work, the preparation of if (read_go_str_n(
"http body", (void *)invocation->body_addr, n, body_buf, sizeof(body_buf))) {
bpf_dbg_printk("body is %s", body_buf);
if (is_jsonrpc2_body((const unsigned char *)body_buf, sizeof(body_buf))) {
char method_buf[JSONRPC_METHOD_BUF_SIZE] = {};
u32 method_len = extract_jsonrpc2_method(
(const unsigned char *)body_buf, sizeof(body_buf), method_buf);
if (method_len > 0) {
bpf_dbg_printk("JSON-RPC method: %s", method_buf);
read_go_str_n("JSON-RPC method",
(void *)method_buf,
method_len,
invocation->method,
sizeof(invocation->method));
}
}
} |
My pleasure - just ping me if you need anything, I'm looking forward to merging this change :) |
|
It does not work by simply adopting the tail code titaneric@8b5e12f , I might need some time to investigate it in the tomorrow. |
|
@titaneric no worries - feel free to share updated logs, or the actual kernel version you are testing if you want me to give it a go as well |
bpf/gotracer/go_nethttp.c
Outdated
| if (read_go_str_n( | ||
| "http body", (void *)invocation->body_addr, n, body_buf, sizeof(body_buf))) { | ||
| bpf_dbg_printk("body is %s", body_buf); | ||
| if (is_jsonrpc2_body((const unsigned char *)body_buf, sizeof(body_buf))) { |
There was a problem hiding this comment.
you can try to tail here instead - i.e.
if (!is_json_content_type(...)) {
return ...
}
if (!read_go_str_n()) {
return ...
}}
// all good, tail
bpf_tailcall_....
this will split the program complexity into two different bpf programs
There was a problem hiding this comment.
Thank you so much for your changes @titaneric - let me know if you need help rebasing main (there's a conflict) - we can then re-trigger the tests, and everything looking good, go ahead and merge this! :)
titaneric
force-pushed
the
feat/otel-bpf-patch-4
branch
from
3d5c004 to
b3dc7d4
Compare
|
Thank you for your information, I would try it in the tomorrow! |
| // get content-type from readContinuedLineSlice | ||
| if (header_inv && header_inv->content_type[0]) { | ||
| bpf_dbg_printk("Found content type in ongoing request: %s", header_inv->content_type); | ||
| // __builtin_memcpy(invocation.content_type, |
There was a problem hiding this comment.
Should we copy the previously set content type into new invocation?
titaneric
force-pushed
the
feat/otel-bpf-patch-4
branch
from
d8a395e to
eb938aa
Compare
|
I run the mcp-go server example, and it works! go run server.go -t httpgo run client.go --http http://localhost:8080/mcpResult log: Partial result: F.Y.I. MCP spec |
|
@rafaelroquetto, I have added the tail call, stack-based array replacement, and integration test. Please take your time to review it! |
I try to capture http request body by introducing new hook for
net/http.(*body).Readfunction.The body struct is initialized in the readTransfer in readRequest.
readTransfer prepare the body reader, and body's
io.Readerimplementation is here.Note that I capture partial http request body and retrieve the
Content-Typein bpf, and send them into userspace.Test JSON-RPC code:
Sample request
curl -X POST http://localhost:8080/jsonrpc \ -H "Content-Type: application/json" \ -d '{ "jsonrpc": "2.0", "method": "Arith.Multiply", "params": [{"A": 7, "B": 8}], "id": 1 }'Output:
make dev sudo -E ./bin/ebpf-instrument --config ~/beyla-config.yamlI hope that present direction is correct for the implementation. I would like to hear more discussion and feedback, and I would continue the work for response body capturing, error code handling, etc.
P.S. The patch is came from grafana/beyla#2008