Re: syntax change proposal

[Martin Pala]

Jan-Henrik Haukeland wrote:
> Martin Pala <address@hidden> writes:
>
>
> > Here is the solution. If you agree, i can sync it. This patch solves
> > the only drawback in new event engine which i knew about.
>
>
> These changes seems reasonable, the only consideration I have is the
> checksum statement. It may be interesting to have a IF CHANGED
> CHECKSUM statement but in most cases a checksum change is an error so
> this statement will be used in most cases:
>
>  IF FAILED [hashtype] CHECKSUM EXPECT STRING THEN action

Yes - that's the reason why 'IF FAILED ...' and 'ID CHANGED ...' are 
here for. FAILED version keeps the original value and is able to produce 
recovery action in the case that it changed back. CHANGED version is 
able to do trigger action each time the value changed - it is not 
expected that it will recover.

> But as far as I can see from the patch (I may be wrong, I just had a
> quick browse) the above statement will, if failed, continue to raise
> alerts on every poll cycle. The best thing to do is what we did
> before, raise one alert and set the old value to the new, silently
> ignoring the excpect string. In other words treat it the same way as
> you do with IF CHANGED CHECKSUM. The important thing is to avoid alert
> flooding on errors (for all statements).

Alerts are sent only once in both cases:

--
For constant version (IF FAILED) is the alert sent on state change only, 
i.e. whenever the event changes from failed->passed or passed->failed. 
For example:
CHECK FILE myfile WITH PATH /my/file
IF FAILED CHECKSUM THEN ALERT

In the case that the checksum failed, you will receive one alert. You 
will receive no additional alert, provided the the checksum remains 
false. As soon as the chekcsum will change back to the original value 
(state change), you will receive recovery alert. This is important for 
security reasons i think - you will know whether some anomaly happened 
and whether it remains.
--
For variable version (IF CHANGED) is the alert sent each time the change 
will occure. You will receive alert each time the condition becomes 
true, for example you will receive alert each time when will monit 
involve apache reload:

CHECK FILE httpd.conf WITH PATH /usr/local/apache/conf/httpd.conf
IF CHANGED CHECKSUM THEN EXEC "/usr/local/apache/bin/apachectl graceful"

This is correct in the context i think, because you need to be notified 
about each reload/file change in this case.
--

The trick is done in event.c Event_post (handle_action remains unchanged 
and sends alerts on state_change only) => it works as expected:

 if(e->id == EVENT_CHANGED || e->state != state)
 {
   /* The event is either 'variable' or 'constant with changed state'.
    * Variable event such as EVENT_CHANGED has no inversion, thus
    * we handle each its occurence as state change. */
   e->state = state;
   e->state_changed = TRUE;
 }

> Other than this minor change to the patch, it looks good. You have
> really been busy lately fixing lots of stuff :-) and as far as I can
> see these are all very good changes and improvements to monit.
>
> Here's my +1 pending a fix on the consideration raised

I will sync it :)

Martin