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

Complete and standardize README #103

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hillaryfraley
Copy link

Pull Request Checklist

Is this in reference to an existing issue?
No

General

  • Update Changelog following the conventions laid out here

  • 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
Copy link
Author

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
Copy link
Contributor

nixwiz commented Dec 18, 2019

@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
Copy link
Author

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

@jspaleta
Copy link
Contributor

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

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

Successfully merging this pull request may close these issues.

None yet

3 participants