Skip to content
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

Merged
merged 81 commits into from
Jan 16, 2024
Merged

discovery.process and pyroscope.java #5858

merged 81 commits into from
Jan 16, 2024

Conversation

korniltsev
Copy link
Contributor

@korniltsev korniltsev commented Nov 27, 2023

PR Description

discovery.process is pretty simple
There 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

// asprof
// glibc / libasyncProfiler.so
// musl / libasyncProfiler.so

Extracted to this layout on the host. And shared library is copied to the target process at the same path via /proc/pid/root/...

./grafana-agent-asprof-musl-f97eb5b45a225752d7b26bb0fd4f2e37d1ddf5d5
./grafana-agent-asprof-musl-f97eb5b45a225752d7b26bb0fd4f2e37d1ddf5d5/bin
./grafana-agent-asprof-musl-f97eb5b45a225752d7b26bb0fd4f2e37d1ddf5d5/bin/asprof
./grafana-agent-asprof-musl-f97eb5b45a225752d7b26bb0fd4f2e37d1ddf5d5/lib
./grafana-agent-asprof-musl-f97eb5b45a225752d7b26bb0fd4f2e37d1ddf5d5/lib/libasyncProfiler.so
./grafana-agent-asprof-glibc-f97eb5b45a225752d7b26bb0fd4f2e37d1ddf5d5
./grafana-agent-asprof-glibc-f97eb5b45a225752d7b26bb0fd4f2e37d1ddf5d5/bin
./grafana-agent-asprof-glibc-f97eb5b45a225752d7b26bb0fd4f2e37d1ddf5d5/bin/asprof
./grafana-agent-asprof-glibc-f97eb5b45a225752d7b26bb0fd4f2e37d1ddf5d5/lib
./grafana-agent-asprof-glibc-f97eb5b45a225752d7b26bb0fd4f2e37d1ddf5d5/lib/libasyncProfiler.so

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

  • CHANGELOG.md updated
  • Documentation added
  • Tests updated
  • Config converters updated

Copy link
Contributor

@cyriltovena cyriltovena left a 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 !

Copy link
Contributor

github-actions bot commented Jan 1, 2024

This PR has not had any activity in the past 30 days, so the needs-attention label has been added to it.
If you do not have enough time to follow up on this PR or you think it's no longer relevant, consider closing it.
The needs-attention label signals to maintainers that something has fallen through the cracks. No action is needed by you; your PR will be kept open and you do not have to respond to this comment. The label will be removed the next time this job runs if there is new activity.
Thank you for your contributions!

@github-actions github-actions bot added the needs-attention An issue or PR has been sitting around and needs attention. label Jan 1, 2024
@github-actions github-actions bot removed the needs-attention An issue or PR has been sitting around and needs attention. label Jan 4, 2024
@korniltsev korniltsev changed the title WIP: discovery.process WIP: discovery.process and pyroscope.java Jan 8, 2024
@cyriltovena
Copy link
Contributor

Docs are good to me.

Copy link
Contributor

@clayton-cornell clayton-cornell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some doc suggestions

docs/sources/flow/reference/components/pyroscope.java.md Outdated Show resolved Hide resolved
docs/sources/flow/reference/components/pyroscope.java.md Outdated Show resolved Hide resolved
docs/sources/flow/reference/components/pyroscope.java.md Outdated Show resolved Hide resolved
docs/sources/flow/reference/components/pyroscope.java.md Outdated Show resolved Hide resolved
docs/sources/flow/reference/components/pyroscope.java.md Outdated Show resolved Hide resolved
korniltsev and others added 16 commits January 12, 2024 10:59
Copy link
Contributor

@clayton-cornell clayton-cornell left a 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 ;-)

@clayton-cornell clayton-cornell added the type/docs Docs Squad label across all Grafana Labs repos label Jan 15, 2024
@korniltsev korniltsev merged commit 1224b53 into main Jan 16, 2024
10 checks passed
@korniltsev korniltsev deleted the discovery_process branch January 16, 2024 05:57
BarunKGP pushed a commit to BarunKGP/grafana-agent that referenced this pull request Feb 20, 2024
@github-actions github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Feb 21, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. type/docs Docs Squad label across all Grafana Labs repos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants