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