NETWAYS / check_logstash

Icinga check plugin to check the status of Logstash
GNU General Public License v3.0
22 stars 11 forks source link

Add CLI flag to override exit code if API is unreachable #99

Closed martialblog closed 8 months ago

martialblog commented 8 months ago

Fixes #98

TODO

martialblog commented 8 months ago

@RincewindsHat Not sure about the flag's name yet. Ideas?

martialblog commented 8 months ago

I'd like to have a generic CLI flag name so that we can re-use it in different plugins

RincewindsHat commented 8 months ago

critical-if-unreachable for a really verbose version unreachable-state $STATE for setting the state on the caller side

martialblog commented 8 months ago

Would be nice to have an pflag Enum Type, but that's currently not available. Our go-check module fortunately is clever enough to handle things other than 0,1,2,3

RincewindsHat commented 8 months ago

Would be really nice to be able to use "2", "Critical", "critical" or "CRITICAL" equally.

martialblog commented 8 months ago

But then we would have to have a StringVarP, and then validate if it's an "accepted" (warn, crit, unknown) string, then map the string to an int... I don't see any added value to that?

RincewindsHat commented 8 months ago

Usability? How many people do actually know how the statuses map to integers?

martialblog commented 8 months ago

I'd assume that people in the monitoring plugins ecosystem are at least familiar with exit codes and their meaning. I would prefer to keep the code simple and people can read up on stuff IF they need this feature at all

Also - and I realize that this point is far fetched - but what if another monitoring plugin system does not use the same "warning" "critical" "unkown" mapping that Icinga does.

RincewindsHat commented 8 months ago

ALL the nagios decendents use the same mapping to my knowledge. If you change that you can just stop using these monitoring plugins at all. They would be pretty much useless.

But this option can be expanded in the future without breaking the API to the user, so I would say we go for it now with numerical codes only.

I would request a enhancement of the help text for the option though to make it clearer what the 3 means and also maybe what the other options are.

martialblog commented 8 months ago

Updated the help text and README