bpf: gotracer: set grpc server context in shared map#1298
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1298 +/- ##
=======================================
Coverage 43.62% 43.63%
=======================================
Files 307 307
Lines 32959 32968 +9
=======================================
+ Hits 14379 14386 +7
- Misses 17658 17659 +1
- Partials 922 923 +1
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:
|
a278c7d to
056ce38
Compare
|
In 8c21536 I changed the hardcoded offsets to be fetched from DWARF (I previously misinterpreted an error in OBI logs) but this comes with additional overhead: We could evaluate reverting it to the hardcoded ones for this path |
|
Test is failing, putting back as draft |
|
This is great! it's crazy that checking for the offsets is that expensive, perhaps we need to rethink that and choose another approach? I would keep them hardcoded, since with hardcoded values the delta was minimal for such a great added functionality. |
|
The VM tests have been flaky, I investigated one, but all looked right in the logs, I couldn't tell why the test was failing. This multiprocess with chained calls from various languages. |
|
update: |
I've been thinking about the way we handle the namespaced pids currently, it's very expensive. We only need the namespaced pids if the userspace component in Beyla doesn't have host pid. I've been wondering if it's possible to process them once after they are supplied by the userspace and then just use simple host pid comparison everywhere. |
I've been wondering the same thing. Perhaps this has already been proposed, but perhaps we could:
|
202659e to
cb01a37
Compare
cb01a37 to
4b1da3c
Compare
|
@grcevski @rafaelroquetto agree on riducing complexity by removing the handling of nss pids |
|
newest benchmark: new baseline on OBI(main) after VM restart: |
grcevski
left a comment
There was a problem hiding this comment.
LGTM! Amazing, that is acceptable overhead IMO.
This PR adds a hook to Go's
runtime.casgstatusin order to set the current running goroutine context in the shared map. As of now, only gRPC server traces have been added.This is a hot path in the Go runtime so I made some benchmarks:
Without OBI
With OBI (including runtime.casgstatus hook)
With OBI (main)
AI summary