Add initial Ruby support #1933
Merged
Add initial Ruby support #1933
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
javierhonduco
force-pushed
the
add-rbperf-v0
branch
3 times, most recently
from
a1d49bd
to
e5a1dc8
Compare
kakkoyun
reviewed
Aug 22, 2023
There was a problem hiding this comment.
Awesome 🏆 🥇 💯
I liked the additions to the rbperf way of sending data for adapting to the parca agent way.
BPF code looks pretty solid to me.
I have left some comments for user space and as you mentioned in your comments there is a lot of room for optimizations.
I think it's sure close to be merged.
javierhonduco
force-pushed
the
add-rbperf-v0
branch
5 times, most recently
from
147cc10
to
7a63e3b
Compare
kakkoyun
approved these changes
Aug 29, 2023
javierhonduco
force-pushed
the
add-rbperf-v0
branch
from
7a63e3b
to
a1babf1
Compare
This commit adds initial Ruby support by integrating rbperf's unwinder and adapting it to this Agent. There is more work to do, but in the spirit of keeping this commit a bit more contained, right now: - only Ruby 3.0.4 is supported; - no focus on efficiency; - missing clean-ups (such as clearing the interpreter stack maps); - the Ruby stack is fetched even if the Ruby thread is sleeping as long as the native stack is on-CPU. This shows up confusingly as the Ruby stack in many native stacks that aren't actually running the Ruby code; - there are no integration tests, this will change soon; All the shortcomings above, and then some, will be addressed in forthcoming commits. Architecture ============ The current BPF unwinding architecture as well as the changes that this commit makes can be seen here: https://excalidraw.com/#json=E-W6lSLZ8rTOcri7HZdAl,4AjIUmb-olsQkdbwCqyxqg Test Plan ========= All current tests pass + Signed-off-by: Francisco Javier Honduvilla Coto <javierhonduco@gmail.com>
javierhonduco
force-pushed
the
add-rbperf-v0
branch
from
a1babf1
to
a366b3f
Compare
|
I'll retrigger the CI soon. |
|
Fix for container step #1964 |
kakkoyun
reviewed
Sep 6, 2023
|
|
||
| p.RawSamples = append(p.RawSamples, profile.RawSample{ | ||
| TID: profile.PID(pKey.tid), |
kakkoyun
added a commit
that referenced
this pull request
Sep 13, 2023
This PR adds initial Python support (CPython) using a dedicated unwinder to the Agent. There are still many things to do (like testing against various versions even though it technically should work). - Only Python `3.11` is thoroughly tested - No focus on performance - The targeted Python interpreter needs symbols to capture required addresses to initialize unwinding. That being said, this shortcoming will be addressed in upcoming versions. It has the same architecture as #1933, and it has similar shortcomings. All the shortcomings above, and then some, will be taken care of in the future. Additionally, it removes individual ruby and python metadata providers and introduces an interpreter metadata provider. You can find interpreterswith `intepreter="Python|Ruby"` ## TODO - [x] Fix the issue that causing failing Kernel tests ### Test Plan - [x] Local tests - [x] Test against Ruby - [x] Test against .pex files - [x] Test against [multiprocessing](https://docs.python.org/3/library/multiprocessing.html) - [ ] Local Kubernetes test --- ### Single-threaded  ### With Threadpool  ### Using .pex (probably need further investigation. I don't have enough knowledge on details yet)   ### With [multiprocessing](https://docs.python.org/3/library/multiprocessing.html) 
](https://cdn.discordapp.com/attachments/963045444332695562/1148621510827835433/CleanShot_2023-09-05_at_16.11.28.png](https://cdn.discordapp.com/attachments/963045444332695562/1148621510827835433/CleanShot_2023-09-05_at_16.11.28.png)){kind=link}
](https://cdn.discordapp.com/attachments/963045444332695562/1149044705158496256/CleanShot_2023-09-06_at_20.12.42.png](https://cdn.discordapp.com/attachments/963045444332695562/1149044705158496256/CleanShot_2023-09-06_at_20.12.42.png)){kind=link}
Open
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit adds initial Ruby support by integrating rbperf's unwinder
and adapting it to this Agent.
There is more work to do, but in the spirit of keeping this commit a bit
more contained, right now:
as the native stack is on-CPU. This shows up confusingly as the Ruby stack in
many native stacks that aren't actually running the Ruby code;
All the shortcomings above, and then some, will be addressed in
forthcoming commits.
Architecture
The current BPF unwinding architecture as well as the changes that this
commit makes can be seen here: https://excalidraw.com/#json=E-W6lSLZ8rTOcri7HZdAl,4AjIUmb-olsQkdbwCqyxqg
Test Plan
All current tests pass + running https://github.com/javierhonduco/rbperf/blob/main/tests/programs/simple_two_stacks.rb
Signed-off-by: Francisco Javier Honduvilla Coto javierhonduco@gmail.com