Skip to content
This repository has been archived by the owner on Oct 12, 2023. It is now read-only.

Add support for monitoring specific processes #29

Merged
merged 5 commits into from
Jan 31, 2019
Merged

Conversation

Porges
Copy link
Member

@Porges Porges commented Jan 20, 2019

Add a new commandline parameter which is a comma-separated list of process names to monitor. Stats for CPU and memory of these processes will be sent alongside the other metrics.

@Porges Porges force-pushed the master branch 4 times, most recently from 699dc60 to 7c525c3 Compare January 20, 2019 23:01
main.go Outdated Show resolved Hide resolved
continue
}

cpuPercent, err := p.CPUPercent()
Copy link
Member

Choose a reason for hiding this comment

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

Does this work fine on windows? The main CPU usage has been implemented wrongly on windows(Or at least was when I made it, might have been fixed) I had to call wmi directly

Copy link
Member Author

Choose a reason for hiding this comment

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

That's annoying...

Copy link
Member

Choose a reason for hiding this comment

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

I saw a PR on the repo that might have fixed it recently so it might just be working fine

main.go Show resolved Hide resolved
You can now set `AZ_BATCH_MONITOR_PROCESSES` to change the processes to
monitor.
@timotheeguerin
Copy link
Member

Can you also add docs to the readme on how to use this setting

@Porges
Copy link
Member Author

Porges commented Jan 31, 2019

Updated docs.

@timotheeguerin
Copy link
Member

timotheeguerin commented Jan 31, 2019

On windows you also need to update the run-windows.ps1 because of the way it starts background tasks environment variables are not passed through so they have to be converted to cli args

@Porges
Copy link
Member Author

Porges commented Jan 31, 2019

@timotheeguerin yes, that's why I wrote "this will require modifying the appropriate run script". Do you think I should be more explicit?

@timotheeguerin
Copy link
Member

Oh I see, any reason why you wouldn't change it here? Feel like this is an unnecessary step for users

@Porges
Copy link
Member Author

Porges commented Jan 31, 2019

Right, easy enough 🙂

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants