-
Notifications
You must be signed in to change notification settings - Fork 812
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
Process management #434
Process management #434
Conversation
class ProcessCheck(AgentCheck): | ||
|
||
|
||
def __init__(self, name, init_config, agentConfig): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can get rid of the whole constructor as it's doing nothing more than the AgentCheck one
Thanks @mastrolinux , it's something that will be very useful ! |
search_string = instance.get('search_string') | ||
pids_list = self.process_finder(exact_match, search_string) | ||
self.log.debug('ProcessCheck: process %s analysed' % name) | ||
self.gauge('system.processes', len(pids_list), tags=[name]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a suffix to this metric name that would be more explicit about it's meaning ?
|
||
return set(found_process_list) | ||
|
||
def processes_memory(self, pids_list): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a couple of suggestions on naming here:
- no need to include the type. just call this variable
pids
. - this should be called
get_process_memory_size
or something more informative
I still need to solve: "for every instance of a check, you call process_finder once and then iterate over every process on the system. this could be more efficient by calling it a single time per agent loop and then looking up each instance in those results." @clutchski can you please explain me how to do it? I have added real memory as "rss - shared". It is not stricly real but an accurate estimation. |
Thanks @mastrolinux , merging it. I might change a few small things in it though. 👍 |
Thank you too @remh it was just a quick and dirty setup. A developer can improve it a lot. I can simply add the regex support in another PR if you like. Bye. |
Please close #400 and comment on this one. There are refactoring and @alq666 @MisterRayCo This one contains most of the fixes you suggested.