Complete and standardize README

hillaryfraley commented 4 years ago

Pull Request Checklist

Is this in reference to an existing issue? No

General

[ ] Update Changelog following the conventions laid out here
[x] Update README with any necessary configuration snippets
[ ] Binstubs are created if needed
[ ] RuboCop passes
[ ] Existing tests pass

Purpose

Updated to conform with standardization guidelines (sensu-plugins/community#134)

Known Compatibility Issues

None

hillaryfraley commented 4 years ago

Error when I try to print help for metrics-per-process.rb

[root@localhost bin]# ./ruby metrics-per-process.rb --help
Traceback (most recent call last):
  File "/opt/sensu/embedded/lib/ruby/gems/2.4.0/gems/sensu-plugins-process-checks-4.1.0/bin/metrics-per-process.py", line 68, in <module>
    import psutil
ImportError: No module named psutil

nixwiz commented 4 years ago

@hillaryfraley that error due to the fact that that particular "ruby" script just calls a python script with the same name which requires the psutil module (you'll notice that name change to .py in the help output below). That's not something we can enforce in this runtime, unfortunately. If your system does have that psutil module installed, the proper help output is:

$ metrics-per-process.rb --help
Usage: metrics-per-process.py [options]

Options:
  -h, --help            show this help message and exit
  -n PROCESS_NAME, --process-name=PROCESS_NAME
                        name of process to collect stats (imcompatible with -p
                        or -u)
  -p PROCESS_PID_FILE, --pid-file=PROCESS_PID_FILE
                        path to pid file for process to collect stats
                        (imcompatible with -n or -u)
  -u USERNAME, --user=USERNAME
                        username of user running the process to collect stats
                        (incompatible with -n or -p)
  -s GRAPHITE_SCHEME, --graphite_scheme=GRAPHITE_SCHEME
                        graphite scheme to prepend, default to <process_stats>
  -m METRICS_REGEXES, --metrics-regexes=METRICS_REGEXES
                        comma-separated list of regexes used to match the
                        metric names to collect, default to .*

I see that you had deleted metrics-per-process.py from the Files section, but the above shows it as a necessary part of the package. I think we need to call out the necessity of Python with the psutil module installed for this particular metric plugin command to work. Of course, this goes against the whole point of packaging this up as an asset. Alternatively we could drop metrics-per-process.(rb|py) completely. Thoughts @jspaleta ?

hillaryfraley commented 4 years ago

@nixwiz thank you for explaining. I will be happy to update the PR as appropriate once Jef weighs in.

jspaleta commented 4 years ago

Yeah we need to just note this is not available in the asset. I'm going to open an issue to refactor that and turn it into pure ruby or ruby with native extensions

sensu-plugins / sensu-plugins-process-checks