check-process.rb: return CRITICAL if pid file is specified and does not exist

sensu-plugins / sensu-plugins-process-checks

This plugin provides native process instrumentation for monitoring and metrics collection, including: process status, uptime, thread count, and others.

http://sensu-plugins.io

MIT License

20 stars 55 forks source link

check-process.rb: return CRITICAL if pid file is specified and does not exist #34

Closed swibowo closed 7 years ago

swibowo commented 7 years ago

Pull Request Checklist

Addressing #23 .

General

[x] Update Changelog following the conventions laid out on Keep A Changelog
[x] RuboCop passes
[x] Existing tests pass

Purpose

I encountered the same issue.

If pid file is specified, it is expected that the pid file must exist and contain pid to match. If pid file becomes non-existent, it very likely means that the monitored process cease to run (or the check should be treated as 'fail' because the pid file given is invalid) and check-process.rb check is expected to return CRITICAL rather than UNKNOWN.

Known Compatablity Issues

Technically, none.

majormoses commented 7 years ago

@swibowo mentioned just now on the issue I think that should be configurable option to fail on pid not found. I would say it should default to false to maintain backwards compatibility.

swibowo commented 7 years ago

You raised a good point there @majormoses. I've modified the pull request to reflect the said changes.

majormoses commented 7 years ago

@swibowo any reason to not make it an actual argument and then we can still use unknown for users that want to keep existing behavior?

swibowo commented 7 years ago

@majormoses , I've reviewed the logic and per your suggestion, added a new option -F to trigger a critical if pid file is non-existent. If -F is not specified, an unknown is triggered (backwards compatible).

majormoses commented 7 years ago

@swibowo now I'm a maintainer so I can merge this now...