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

Allow non-root user to run metrics-per-process.py #43

Open
rthouvenin opened this issue May 31, 2017 · 2 comments
Open

Allow non-root user to run metrics-per-process.py #43

rthouvenin opened this issue May 31, 2017 · 2 comments
Assignees

Comments

@rthouvenin
Copy link
Contributor

In my set-up (Ubuntu server 16.04), some of the files in /proc/<PID>/* can be read only by root. This is annoying because:

  • I actually don't care about these metrics
  • This prevents the whole check to run
  • I don't want the sensu-client to run as root.

As a temporary solution, I deployed a copy of metrics-per-process.py where I commented out the calls that fail. Now I am wondering what would be a better solution. I am happy to contribute, but wanted to check with you what's the best approach. I can think of:

  • Have a switch to turn off the metrics that are typically restricted to root. This assumes there is such a typical set, which I am not sure of at all, and also has the drawback of not solving the problem for non-typical set-ups
  • Catch the error when a metric fails, and skip it instead of aborting the whole check.
  • Have a regex option to filter the list of metrics to return.

The last two could be combined.

What do you think?

@majormoses
Copy link
Member

I think having an option to whitelist or blacklist metrics (we can make it support a list of regex keys) for which metrics seems reasonable even when not considering privileged access and already follows other metric plugin patterns.

Regarding needing root ownership, that really depends if processes run as root will certainly be owned by root. Most services other than core OS functionality should be owned by the user that launched it (yes I realize people still run services such as apache as root when they do not need to be). For our custom services that we monitor and wanted sensu to be able to remediate (for example restarting a dead process) we made sure that we made the pid dirs group by the service user and then gave sensu limited sudo access to those files and specific commands to ensure that it could still be limited.

@rthouvenin
Copy link
Contributor Author

Totally agree. Even then, some of the files are readable only by the user, not the group so indeed sensu would have to impersonate these users like you did.

Anyway, I also think it's good idea to have this filter, so I'll try to work on that when I get some time.

rthouvenin added a commit to rthouvenin/sensu-plugins-process-checks that referenced this issue Aug 17, 2017
rthouvenin added a commit to rthouvenin/sensu-plugins-process-checks that referenced this issue Sep 18, 2017
majormoses pushed a commit that referenced this issue Oct 4, 2017
* Add filter for stats name (#43)

* Turn lamba collection into if collection

* Accept multiple regexes

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

No branches or pull requests

2 participants