On Sun, Jan 31, 2016 at 6:45 AM, sebb <seb...@gmail.com> wrote:
> On 30 January 2016 at 00:17, Sam Ruby <ru...@apache.org> wrote:
>> Commit 97680383779cd485498d6bb8fa8288af49cc2145:
>>     warnings should trump info
>>     also implement parsing todo
>>
>>
>> Branch: refs/heads/master
>> Author: Sam Ruby <ru...@intertwingly.net>
>> Committer: Sam Ruby <ru...@intertwingly.net>
>> Pusher: rubys <ru...@apache.org>
>>
>> ------------------------------------------------------------
>> www/status/monitors/public_json.rb                           | ++++++ ---
>> ------------------------------------------------------------
>> 16 changes: 11 additions, 5 deletions.
>> ------------------------------------------------------------
>>
>>
>> diff --git a/www/status/monitors/public_json.rb 
>> b/www/status/monitors/public_json.rb
>> index 39eb31b..7ff71b1 100644
>> --- a/www/status/monitors/public_json.rb
>> +++ b/www/status/monitors/public_json.rb
>> @@ -21,16 +21,22 @@ def Monitor.public_json(previous_status)
>>        # Ignore Wunderbar logging for normal messages (may occur multiple 
>> times)
>>        contents.gsub! /^(_INFO|_DEBUG) .*\n+/, ''
>>
>> -      # Wunderbar warning (TODO - extract the text and return it?)
>> -      if contents.gsub! /^_WARN .*?\n+/, ''
>> -        status[name].merge! level: 'warning', title: 'warning'
>> -      end
>> -
>>        # diff -u output:
>>        if contents.sub! /^--- .*?\n(\n|\Z)/m, ''
>>          status[name].merge! level: 'info', title: 'updated'
>>        end
>>
>> +      # Wunderbar warning
>> +      warnings = contents.scan(/^_WARN (.*?)\n+/)
>> +      if warnings.length == 1
>> +        contents.sub! /^_WARN (.*?)\n+/, ''
>> +        status[name].merge! level: 'warning', data: $1
>> +      elsif warnings.length > 0
>> +        contents.gsub! /^_WARN (.*?)\n+/, ''
>> +        status[name].merge! level: 'warning', data: warnings.flatten,
>> +          title: "#{warnings.length} warnings"
>> +      end
>> +
>
> Is it intended that the title is omitted when there is only one _WARN line?
> If not, the else clause would be sufficient.

Normally titles aren't intended to set by monitors:

https://github.com/apache/whimsy/tree/master/www/status#titles

The intent of the above is to surface the individual warning should
there only be one.  Otherwise provide a count of the number of
warnings.

If you note, the warnings that we are getting via email are fairly
informative (e.g., they tell you what LDAP server is down), instead of
simply saying "1 warnings".

> Also I see that the Ping My Box service is set up to treat warning
> level as a failure.
> I assume we cannot change that just for Whimsy monitoring.

Actually, we can:

https://github.com/apache/whimsy/tree/master/www/status#alerts
https://github.com/apache/whimsy/blob/master/www/status/index.cgi#L24

> I don't think we want PMB 'Down' mails just for warnings, so perhaps
> we should convert the _WARN to informational level?
>
>>        unless contents.empty?
>>          status[name].merge! level: 'danger', data: contents.split("\n")
>>        end

Let's take an individual LDAP server being unavailable as an example.

The question is: given that we have LDAP failover, is one server being
down only informational or something that you actually want to warn
somebody about?  When a warning is issued, do you want the warning to
only show up in the status page should somebody happen to look for it,
or actually trigger an email?

I took an initial stab at it (assuming warnings would only be used
when you actually wanted a human to see the message).  But this is
open for discussion.  Thoughts?

- Sam Ruby

Reply via email to