CVTJNII
commented
8 years ago I also agree in moving this onto the core. With the current behavior the handler is called on every event as a separate process, which is expensive. This could be a landmine for users, as with checks set to 'standard' OK events aren't handled, but if something major happens (like a core router dropping) suddenly the majority of an environment could go into alert, effectively fork-bombing the Sensu server with handlers. This may be a case many users have not tested for.
Sensu core supports filters now, but with the filter logic in the handlers I consider using both dangerous as it would be easy to create a filter setup that blocks all events, so no events are ever handled. It would be very nice, at the minimum, to have an option in the base handler class to switch the default filter method to a no-op. This would allow the default behavior to stay the same but let Sensu filter users shut off the handler filters as needed. Perhaps this flag should be a subtask of this larger story?
cwjohnston
zbintliff
fessyfoo
portertech
julian7
The problem
The
Sensu::Handlerclass currently implements a number of methods for filtering events, e.g.filter_disabled,filter_repeated,filter_silenced, andfilter_dependencies. Each of these filters is evaluated every time an event is handled by a Sensu::Handler plugin.This Sensu::Handler class has been widely adopted by the plugins in the community Sensu Plugins collection. The original intention was for organizations to maintain their own plugins and override the
filtermethod for their own purposes, but thats not what has manifest in our ecosystem.Because of how these filter methods are implemented inside
Sensu::Handler, and owing to the popularity of the community plugins in our ecosystem, it has become very difficult for operators to distinguish between bugs inSensu::Handler's filtering methods and strange behavior in Sensu Core itself. For example, even rather commonly used check attributes likerefresharen't implemented directly in Sensu Core, but are used only inSensu::Helperfilter methods.In particular, the
filter_repeatedmethod uses logic to compareoccurrences,refreshandstatusin order to determine if the event should be disregarded as a repeat. It's apparently prone to filtering events which should have been sent , often to indicate the outage was resolved. There may be some room for improvements to the existing filtering made possible by making use oflast_okandlast_state_changeevent attributes, but welack tests to describe the current behavior andcan't change much without breaking many plugins.The proposed solution
As stewards of Sensu, we'd like to see filters to become the primary way for operators to separate signal from noise, instead of relying on this library to cover all cases. Although we think operators can use Sensu filters in a similar capacity now, events that pass all filter evaluation may still be erroneously dropped by handlers that use the
filter_repeatedevent occurrence filtering behavior in this library, and we cannot change this without breaking a few eggs.To that end we're proposing the following steps:
Sensu::Handlerevent filtering optional at both the top level and the occurrence filtering (filter_repeated) leven via check configuration, defaulting to enabled (next minor revision release)Sensu::Handler(next minor revision release)Sensu::Handlerevent filtering from enabled to disabled (next major revision release)Sensu::Handleraltogether (next major revision + 1)Related issues
Edit: I've updated the description of this PR to indicate that the scope of my proposed change has narrowed to deprecating the event occurrence logic of thefilter_repeatedmethod.Edit: I've changed this description again to describe the more granular implementation now being proposed in #136.