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

Improve the docs of pyroscope.scrape #5847

Merged
merged 6 commits into from
Jan 10, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Update docs/sources/flow/reference/components/pyroscope.scrape.md
Co-authored-by: Ryan Perry <Rperry2174@gmail.com>
  • Loading branch information
ptodev and Rperry2174 committed Jan 9, 2024
commit f0db8e2702e40fc47470c48f4e5239d41890fdcd
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@clayton-cornell - The changes in the PR make this page more in line with our Writing documentation for Flow components guide, but I have taken a few liberties which are noted in comments.

@knylander-grafana - there is a Go (pull mode) page on the Pyroscope website which contains lots of duplicate information. Should we remove the duplications?

Copy link
Contributor

Choose a reason for hiding this comment

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

We created that page in response to feedback that this page was too verbose and hard to get started quickly. We will definitely keep the other page for now and I'll just manage the duplicate work of making sure changes made here make it over there

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could look into converting Go (pull mode) and eBPF into new task-based docs under Agent's Getting Started section called "Profiling Go Applications" and "Profiling with eBPF".

Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ title: pyroscope.scrape

`pyroscope.scrape` collects [pprof] performance profiles for a given set of HTTP `targets`.

Copy link
Contributor

@Rperry2174 Rperry2174 Nov 24, 2023

Choose a reason for hiding this comment

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

I usually share this picture to show visually what the grafana agent does... may be worth adding in here. we could update this image to have more detail (i.e. showing multiple endpoints for scraping)

image

Copy link
Contributor Author

@ptodev ptodev Dec 7, 2023

Choose a reason for hiding this comment

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

Since the picture shows the interactions between different components, I'd prefer not to place it in the reference docs for pyroscope.scrape. It'd be great to have a new "Profiling Go Applications" task-based doc in the Getting Started section. That doc can walk people through installing the Agent for profiling and we could put the picture there.

I think having such an architecture picture would be great for most task-based docs, and it would be even better if they are all consistent.
cc @clayton-cornell

`pyroscope.scrape` borrows the scraping behavior of `prometheus.scrape`.
`pyroscope.scrape` mimcks the scraping behavior of `prometheus.scrape`.
Similarly to how Prometheus scrapes metrics via HTTP, `pyroscope.scrape` collects profiles via HTTP requests.

Unlike Prometheus, which usually only scrapes one `/metrics` endpoint per target,
Expand Down