search

sensu-plugins / sensu-plugins-sensu

This plugin provides monitoring and metrics for Sensu.
http://sensu-plugins.io
MIT License
12 stars 35 forks source link

Should handler-sensu.rb use Sensu 0.26's client syntax? #31

Closed cj-dimaggio closed 7 years ago

cj-dimaggio commented 7 years ago

Sensu 0.26 added the feature that all clients be automatically given a unique subscription following the format "client: [client-name]" which gives a good system for executing events on specific clients (I believe it's used for silencing).

I just created a fork at https://github.com/Ssawa/sensu-plugins-sensu that updates this plugin to use this system for targeting the client during remediation (if "trigger_on" isn't set) and have been using a version of it in production for awhile. I was going to submit a PR but I'm aware that this effectively breaks backwards compatibility with anything pre-0.26 so figured it would be worth opening as a discussion first.

Personally I think it's worth it; the current system of using only the client's name as the subscription just doesn't work with the way Sensu operates and requires the user to dig into the internals to figure out why it doesn't work and then forces them to add the client's name to the client's subscriptions which, in a post-0.26 world, is unnecessary.

The alternative is that a user who has adjusted to that and are still using a pre 0.26 Sensu and upgrade their plugin to this version will need to manually add the "client:" prefix to their client subscriptions.

majormoses commented 7 years ago

@Ssawa that makes sense to me, I am fine with breaking changes assuming they are properly called out in changelog (on you) appropriately and versioned appropriately (on us). This is the reason we use semver and tell people to pin their gems so that these kinds of breaking changes do not affect them until they are ready to upgrade.

majormoses commented 7 years ago

@Ssawa I applied your patch and gave a bit of a more descriptive changelog entry to denote how this is a breaking change and how someone who needs to keep using sensu clients < 0.26 how they can proceed using trigger_on with newer versions of this gem. It will be versioned according to semver so it will be a major rev (3.0.0) on merge so anyone who is currently running it and pinning their versions will be unaffected.

eheydrick commented 7 years ago

Fixed in 2.2.2, now live on rubygems: https://rubygems.org/gems/sensu-plugins-sensu/versions/2.2.2. The change was made in a non-breaking way.