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

server: import godeltaprof/http/pprof #450

Merged
merged 2 commits into from
Dec 13, 2023
Merged

server: import godeltaprof/http/pprof #450

merged 2 commits into from
Dec 13, 2023

Conversation

korniltsev
Copy link
Contributor

What this PR does:
This PR imports github.com/grafana/pyroscope-go/godeltaprof/http/pprof in addition to net/http/pprof

When pyroscope is scraping cumulative profiles (alloc_{space,objects}, mutex, block) it needs to compute delta (difference / substraction from the previous profile). This is currently happening at grafana-agent after scraping. This requires keeping state of previous profiles requiring much memory and also spends a bunch of cpu time (decompress,deserialize,substract,serialize,compress).

godeltaprof is an efficient delta profiler for memory, mutex, and block.

It does compute this delta on the target side instead of the agent side before any compression/serialization, so a lot of work is avoided. Using godeltaprof removes the need to compute the delta on the grafna-agent side - this saves cpu and memory. Also the scraping godeltaprof itself is more efficient compared to scraping net/http/pprof because after delta is computed a lot of samples are rejected and the output profile is much smaller. So it is a win-win for grafana-agent <-> targets. Some benchmarks .

This PR does not change net/http/pprof endpoints, just makes the server ready to scrape godeltaprof profiles. Additional changes to scraping configuration is required.

Which issue(s) this PR fixes:

Fixes #

Checklist

  • Tests updated
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@korniltsev korniltsev marked this pull request as ready for review December 12, 2023 12:20
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.

Lgtm

CHANGELOG.md Outdated
@@ -1,5 +1,6 @@
## Changelog

* [CHANGE] server: import godeltaprof/http/pprof
Copy link
Member

@pstibrany pstibrany Dec 12, 2023

Choose a reason for hiding this comment

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

Can you please specify which endpoints this adds? Also can you please add PR number?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

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

Thanks.

@korniltsev korniltsev merged commit bdc225d into main Dec 13, 2023
3 checks passed
@korniltsev korniltsev deleted the godeltaprof branch December 13, 2023 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants