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

Add mutex profiling support #660

Closed
lizthegrey opened this issue Dec 25, 2021 · 4 comments
Closed

Add mutex profiling support #660

lizthegrey opened this issue Dec 25, 2021 · 4 comments
Assignees
Labels
enhancement New feature or request 🐿. golang Related to Golang profiler integration integrations

Comments

@lizthegrey
Copy link
Contributor

https://github.com/pyroscope-io/pyroscope/blob/9e6022b95e935e81ba6a3e6c028495d271a510bf/pkg/scrape/config/config.go#L40

needs an entry for mutex profiling.

@kolesnikovae
Copy link
Collaborator

Default configuration only includes Go cpu and mem profiles but eventually we will extend the list. Although this part is not yet fully supported (and documented), it is possible to scrape arbitrary profiles – including mutex, goroutines, and block:

scrape-configs:
- job-name: my-job
   enabled-profiles: [cpu, mem, mutex]
   profiles:
     mutex:
       path: /debug/pprof/mutex
       sample-types:
         mutex:
           units: objects
           aggregation: average
           cumulative: true

We will add support for all of them soon!

@kolesnikovae kolesnikovae self-assigned this Dec 25, 2021
@kolesnikovae kolesnikovae added the enhancement New feature or request label Dec 25, 2021
@eh-am
Copy link
Collaborator

eh-am commented Jan 13, 2022

@kolesnikovae Just out of curiosity, how far are we from supporting those?

@kolesnikovae
Copy link
Collaborator

@eh-am basically, we need a couple of little changes:

  • Default profile configuration: we need to extend the default configuration of profiles as per my comment above (the exact list of supported profiles is TBD). I don't think the new profiles should be enabled by default, therefore it is a pretty safe change. On the other hand, we learned a lot about cpu and heap and know what to expect; meanwhile, new profiles can bring new issues, so we need to be very careful.
  • Frontend support: add proper annotations for the new profile types (like "Frame width represents CPU time per function" for cpu). AFAIK, it's hardcoded, which is not great, but I don't think we have to change it (at least right now).
  • Documentation: it's time to update docs for pull mode and scrape configuration, add a section explaining how profiles are configured and how it can be used. In addition, it may make sense to add an article/blog post on how to analyse the profiling data: for example, mutex and block profiles can be empty – in some cases it can be confusing.
  • [Optional] Go push client: for consistency, the same profile types should be supported in both push/pull modes. I think that we should abandon support of the old client.
  • [Optional] Self-profiling: I believe it is time to switch self-profiling to the new Go client.

@eh-am
Copy link
Collaborator

eh-am commented Jul 6, 2022

Implemented in #1178

@eh-am eh-am closed this as completed Jul 6, 2022
korniltsev pushed a commit that referenced this issue Jul 18, 2023
Adds new client docs mostly copied over from pyroscope.io
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request 🐿. golang Related to Golang profiler integration integrations
Projects
None yet
Development

No branches or pull requests

4 participants