-
Notifications
You must be signed in to change notification settings - Fork 487
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
discovery.process and pyroscope.java #5858
Conversation
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.
Really simple, love it so far !
This PR has not had any activity in the past 30 days, so the |
Docs are good to me. |
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.
Some doc suggestions
Co-authored-by: Clayton Cornell <[email protected]>
Co-authored-by: Clayton Cornell <[email protected]>
Co-authored-by: Clayton Cornell <[email protected]>
Co-authored-by: Clayton Cornell <[email protected]>
Co-authored-by: Clayton Cornell <[email protected]>
Co-authored-by: Clayton Cornell <[email protected]>
Co-authored-by: Clayton Cornell <[email protected]>
Co-authored-by: Clayton Cornell <[email protected]>
Co-authored-by: Clayton Cornell <[email protected]>
Co-authored-by: Clayton Cornell <[email protected]>
Co-authored-by: Clayton Cornell <[email protected]>
Co-authored-by: Clayton Cornell <[email protected]>
Co-authored-by: Clayton Cornell <[email protected]>
Co-authored-by: Clayton Cornell <[email protected]>
Co-authored-by: Clayton Cornell <[email protected]>
Co-authored-by: Clayton Cornell <[email protected]>
Co-authored-by: Clayton Cornell <[email protected]>
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.
Docs look good to me now ;-)
PR Description
discovery.process
is pretty simpleThere are couple of things worth noting for
pyroscope.java
We bundle two versions of async-profiler one for musl, second for glibc. The musl one is compiled with static stdc++ because alpine often does not have stdc++ installed by default.
asprof
is compiled statically, so that we have 1 binary instead of 2.libAsyncProfiler.so
absolute path should be exactly the same on host as inside container.This is the layout of the bundled
.tar.gz
Extracted to this layout on the host. And shared library is copied to the target process at the same path via
/proc/pid/root/...
It is loaded from
bin/apsorf
binary as 'bin/../lib/libasyncProfiler.so`asprof is built from upstream main branch with these changes to makefile https://gist.github.com/korniltsev/972f8ee37f3280af6757d7b8fec445d0
I had one interesting bug when my unpacking code was overwriting shared libraries in the target process root fs causing SIGSEGV in
jvm->dlopen("libAsyncProfiler.so")
so now it does not overwrite files.Which issue(s) this PR fixes
Notes to the Reviewer
PR Checklist