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

Filter PIDs to profile #605

Closed
Jongy opened this issue Nov 17, 2022 · 2 comments
Closed

Filter PIDs to profile #605

Jongy opened this issue Nov 17, 2022 · 2 comments
Assignees
Labels
defined-and-prioritized Tickets that have fully defined the desired outcome & are prioritized to be developed. enhancement New feature or request

Comments

@Jongy
Copy link
Contributor

Jongy commented Nov 17, 2022

Filtering processes to profile is a feature that was requested in multiple cases.#482 #298 #274 #573. This ticket is for the first step - filter by PIDs (relevant only in cases where processes remain running).

I suggest:

  • --pids parameter that is a number or comma-separated list of numbers, and can also be given multiple times e.g --pids 1,2 --pids 5 --pids 76,76576. All are accumulated.
  • At this point https://github.com/Granulate/gprofiler/blob/914e05081bf90f96059f1a3b5995dec71b334b7c/gprofiler/profilers/profiler_base.py#L177
    PIDs are selected for runtime profilers. We can intersect the "selected PIDs" here with the PIDs passed via --pids. This class is extended by all runtime profilers classes, so the "easy" way to pass more parameters is to add it in all constructors of JavaProfiler etc. I prefer if we find a clean way to do so, so we don't need to modify all runtime profilers, this needs to be decoupled.
  • There are 3 profilers which select their PIDs "automatically" and not via gProfiler decision: perf, PyPerf and phpspy. They need to be modified. At the very least, perf and PyPerf support receiving target PID(s) as arguments. We need to check about phpspy. Their respective classes should receive the pids argument and make use of it if passed.
  • UX wise, I think gProfiler should be fine if it gets --pids a,b,c and only e.g b, c are running and a is not running.
  • By default, gProfiler will continue in its standard behavior of profiling everything. So if no PIDs are given, an empty set of filter PIDs will be considered ALL processes.
  • Please add adequate tests - 2 running Java apps, pass the PID of one of them and ensure that async-profiler was invoked only on one of them.
  • Deny passing --profile-spawned-processes with --pids, because it is meaningless, we target live processes.
@Jongy Jongy added the enhancement New feature or request label Nov 17, 2022
@Jongy Jongy added the defined-and-prioritized Tickets that have fully defined the desired outcome & are prioritized to be developed. label Jan 31, 2023
@Jongy
Copy link
Contributor Author

Jongy commented Mar 19, 2023

On top of this task we could have --containers= which accepts multiple container names, and will accept all PIDs originating from those containers. It can also accept the container name as a regex pattern. Not required for MVP but nice to have :)

@mpozniak95
Copy link
Contributor

On top of this task we could have --containers= which accepts multiple container names, and will accept all PIDs originating from those containers. It can also accept the container name as a regex pattern. Not required for MVP but nice to have :)

I opened issue for this: #762

@Jongy Jongy closed this as completed in df84102 Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defined-and-prioritized Tickets that have fully defined the desired outcome & are prioritized to be developed. enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants