Maoni0 / realmon

A monitoring tool that tells you when GCs happen in a process and some characteristics about these GCs

MIT License

281 stars 25 forks source link

For multiple processes, prompt the user to select one based on the pid #34

Closed MokoSan closed 2 years ago

MokoSan commented 2 years ago

For the case of multiple processes, rather than exiting the process, prompt the user to choose one based on the PID. This is to fix #32.

The behavior is now like the following: Administrator_ Windows PowerShell 2021-12-02 22-36-20

Maoni0 commented 2 years ago

seems like it would be helpful to the user to display the commandline args of those processes for them to decide which pid they want. what do you think?

MokoSan commented 2 years ago

Definitely think it's a great idea! Thanks! Command line args are only available to other processes if we are in sudo/admin mode. The way to discern the sudo / admin mode for Linux and MacOs / Windows are different additionally, the methodology to get to command line args amongst the different OSes is different; the last commit before this comment takes care of this.

A different prompt is presented to the user if they are in sudo/admin mode than if they are not.

For Windows:

Non-Admin

Admin

For Linux:

Non-Sudo

Sudo

Maoni0 commented 2 years ago

sorry @MokoSan, I didn't get to your PRs earlier. I'm also starting my vacation so my response will be slow. I've talked to @mangod9 to see if he could review some of the PRs here while I'm OOF.

right, you do need admin privilege to see other processes's cmdline args. I forgot about that. on linux EventPipe does not require you to have admin privilege so users could be both but on windows, since we are using ETW, it means you'd already need to have admin privilege. in your 1st pic (non admin on windows) are you able to actually monitor?

MokoSan commented 2 years ago

Thanks for taking a look!

in your 1st pic (non admin on windows) are you able to actually monitor?

Yes - I am able to monitor without admin privileges (always have been able to minus the call stack work).

Enjoy your vacation!

MokoSan commented 2 years ago

39 will make doing this much easier as we'll have Spectre.CLI support. Will include this with this PR once #39 is merged.

MokoSan commented 2 years ago

@Maoni0, this PR is also ready to be merged with the latest rebased changes + additions to the different IConsoleOut impls. Additionally, I removed the requirement of being a super user for Linux / MacOS to read the commandline args as it's a simple file read.

Plain Text Prompt

Prompt_PlainText

Spectre Prompt

Maoni0 commented 2 years ago

awesome! thanks @MokoSan