Assuming people care about 404 pages and PHP errors and whatnot, I though nagios-check watchdog would be a reasonable request.

So I wrote a patch against 6.x which I'm on.

diff -u attached

It's a single new function and a one-line change to enable it, so even if you don't like diff -u you can just copy paste it.

There are probably some php-isms that should be done the Drupal Way, but it works, and it can always get re-factored by Drupal experts. :-)

And you can probably copy/paste the same thing into the 7.x branch.

CommentFileSizeAuthor
#21 nagios-implement-watchdog_1441572_21.patch6.43 KBjarrodirwin
#20 nagios-implement-watchdog_1441572_20.patch3.47 KBjarrodirwin
#15 nagios-implement-watchdog-7.x-1.2.patch3.47 KBStephenBrown
#14 nagios-fixes-1441572-7-9-10-11.patch2.12 KBStephenBrown
#14 nagios-some-variables-are-not-an-array-and-no-percent-sign-with-key-1441572-7-and-11.patch681 bytesStephenBrown
#13 nagios-ignore-older-watchdog-entries-1441572-9.patch1.14 KBStephenBrown
#13 nagios-fix-no-nagios_status_error-1441572-10.patch826 bytesStephenBrown
#13 nagios-some-variables-are-not-an-array-and-no-percent-sign-with-key-1441572-7-and-11.patch680 bytesStephenBrown
#13 nagios-fixes-1441572-7-9-10-11.patch2.12 KBStephenBrown
#3 nagios.module.patch5.6 KBRichardLynch
nagios_watchdog.patch3.06 KBRichardLynch

Comments

greg.harvey’s picture

greg.harvey
Primary language English
Location Occitanie, France
commented
Status: Active » Needs review

Will take a look, thanks.

greg.harvey’s picture

greg.harvey
Primary language English
Location Occitanie, France
commented
Status: Needs review » Needs work

So, this looks pretty good - main thing that needs to be fixed before I can apply it is the tabs need to be double-space indentations. If you can quickly make a new patch with double-spaced indentation instead of tabbed then I can apply this.

RichardLynch’s picture

RichardLynch commented
StatusFileSize
new nagios.module.patch5.6 KB

Patch without leading tabs.

I think I managed to replace some that were already there, or diff got a bit confused...

greg.harvey’s picture

greg.harvey
Primary language English
Location Occitanie, France
commented
Status: Needs work » Needs review

Thanks - looks like there's no change in that second block of code, just whitespace differences. Looks like you tidied up some other tabs that slipped through in the past. Thanks! =D

Will triple check this one last time, probably on Tuesday, then commit.

instanceofjamie’s picture

instanceofjamie commented
Status: Needs review » Reviewed & tested by the community

Reviewed, looks good for me

greg.harvey’s picture

greg.harvey
Primary language English
Location Occitanie, France
commented
Status: Reviewed & tested by the community » Patch (to be ported)

Added to dev snapshot for Drupal 6, need a Drupal 7 port of this.

RichardLynch’s picture

RichardLynch commented

A patch for my own patch...

Turns out the 'variables' field is no always a serialized array...

512,513c512,516
< foreach($variables as $key => $value){
< $message = str_replace("%$key", $value, $message);
---
> //Some variables are not an array...
> if (is_array($variables)){
> foreach($variables as $key => $value){
> $message = str_replace("%$key", $value, $message);
> }

greg.harvey’s picture

greg.harvey
Primary language English
Location Occitanie, France
commented
Status: Patch (to be ported) » Needs review

Thanks!

RichardLynch’s picture

RichardLynch commented

After using this for awhile, I have come to the realization that...

Watchdog will have "old" entries for an issue that has been resolved.

In the ideal world, it would accept the Nagios last_check time and only select watchdog entries after that.

As a temporary patch, human judgment could be used with the trivial patch below to ignore older watchdog entries:

478c478
< $query = "SELECT wid, uid, type, severity, message, variables, link, location, hostname, timestamp FROM {watchdog} ORDER BY timestamp DESC LIMIT %d OFFSET %d";
---
> $query = "SELECT wid, uid, type, severity, message, variables, link, location, hostname, timestamp, from_unixtime(timestamp) as humantime FROM {watchdog} ORDER BY timestamp DESC LIMIT %d OFFSET %d";
508c508
< $message = "type: $row[type] $row[message]";
---
> $message = "$row[humantime] type: $row[type] $row[message]";

RichardLynch’s picture

RichardLynch commented

Good Lord!

How did I not catch this blunder:

494,497c494,497
< WATCHDOG_ERROR => NAGIOS_STATUS_ERROR,
< WATCHDOG_CRITICAL => NAGIOS_STATUS_ERROR,
< WATCHDOG_ALERT => NAGIOS_STATUS_ERROR,
< WATCHDOG_EMERG => NAGIOS_STATUS_ERROR
---
> WATCHDOG_ERROR => NAGIOS_STATUS_CRITICAL,
> WATCHDOG_CRITICAL => NAGIOS_STATUS_CRITICAL,
> WATCHDOG_ALERT => NAGIOS_STATUS_CRITICAL,
> WATCHDOG_EMERG => NAGIOS_STATUS_CRITICAL

There is no NAGIOS_STATUS_ERROR...

RichardLynch’s picture

RichardLynch commented

I must have had a REALLY off day...

512,513c513,517
< foreach($variables as $key => $value){
< $message = str_replace("%$key", $value, $message);
---
> //Some variables are not an array...
> if (is_array($variables)){
> foreach($variables as $key => $value){
> $message = str_replace("$key", $value, $message);
> }

No percent sign. $key already has the % in it.

xcession’s picture

xcession commented

Can someone please explain what is going on in this ticket? 6.x-1.3 was last updated in August 2012, yet this ticket contains fixes (not even submitted as patch files) for some glaring mistakes in the current release which predate its release.

As it stands the "Recommended release" 1.3 isn't fit for purpose, or certainly the Watchdog part of it isn't.

StephenBrown’s picture

StephenBrown commented
Version: 6.x-1.x-dev » 6.x-1.3
StatusFileSize
new nagios-fixes-1441572-7-9-10-11.patch2.12 KB
new nagios-some-variables-are-not-an-array-and-no-percent-sign-with-key-1441572-7-and-11.patch680 bytes
new nagios-fix-no-nagios_status_error-1441572-10.patch826 bytes
new nagios-ignore-older-watchdog-entries-1441572-9.patch1.14 KB

Here are some proper patches for the comments above, as well as a comprehensive patch combining all of them, which I got by applying the 9, 10, and 7-and-11 patch files in order.

Anyone interested in porting this to 7.x?

StephenBrown’s picture

StephenBrown commented
StatusFileSize
new nagios-some-variables-are-not-an-array-and-no-percent-sign-with-key-1441572-7-and-11.patch681 bytes
new nagios-fixes-1441572-7-9-10-11.patch2.12 KB

Aw crap... just found a bug in my patches while testing. Missed a paren and didn't test before uploading.

Re-attaching fixed files.

StephenBrown’s picture

StephenBrown commented
StatusFileSize
new nagios-implement-watchdog-7.x-1.2.patch3.47 KB

And, because it's nice to have things in Drupal 7, here's the reworked patch to add the watchdog command to 7.x-1.2.
All I needed to do was change around how the database query worked, to work with the Drupal 7 DB API.

edward.kay’s picture

edward.kay commented

Hi Greg,

Any news on getting these patches into a versioned release of this module?

Thanks

ZeWaren’s picture

ZeWaren commented

I'm also shocked that these patches still haven't been released.
The issue here is IMHO pretty critical. In my case, the defective watchdog check was hiding a core security update error, which I then didn't see until a week later.

pjeutr’s picture

pjeutr commented

I encountered a problem with this code that the patches will not solve.
See, http://drupal.org/node/1897026

victoriachan’s picture

victoriachan commented

Hi,

Can we at least commit the nagios-fix-no-nagios_status_error-1441572-10.patch patch in #13 please? This is a straightforward patch to correct the obvious problem of wrong error code, which is pretty crucial for the module to work.

Thanks,
Victoria

jarrodirwin’s picture

jarrodirwin commented
Issue summary: View changes
StatusFileSize
new nagios-implement-watchdog_1441572_20.patch3.47 KB

The D7 patch (and maybe D6?) logs the watchdog errors under the 'ADMIN' key. This wipes out any data from the 'requirements' check. Not ideal if the requirements has a Critical status and watchdog only a Warning...

Attached is the updated D7 patch that records watchdog errors under a key of 'WATCHDOG'.

jarrodirwin’s picture

jarrodirwin commented
Version: 6.x-1.3 » 7.x-1.x-dev
StatusFileSize
new nagios-implement-watchdog_1441572_21.patch6.43 KB

Attached is a new patch for the D7 version of this module.

Watchdog was flooding nagios with constant errors from messages sitting in watch dog. Due to this I have added a configurable option in the admin form to only have watchdog report new logs since the last check. This means that once the issue is fixed and watchdog is no longer logging the error, nagios won't complain about the historical messages and we won't have to manually remove anything from watchdog.

I have also moved the number of watchdog logs to check (was hardcoded to 50 in code) into the form so this can be set manually and added some extra checks so that identical logs in watchdog are only displayed once in the nagios output.

This is patched against the latest D7 dev branch.

greg.harvey’s picture

greg.harvey
Primary language English
Location Occitanie, France
commented

I don't know how I missed this patch for *2 years*. Sorry all. We'll review it asap and get it into the dev snapshot and the next point release.

(PS - I don't mind if folk ping me a message with the contact form to draw my attention to an issue - I prefer that to frustration building because I'm not doing something I should be doing... :-)

greg.harvey’s picture

greg.harvey
Primary language English
Location Occitanie, France
commented

Right, sorry about this - been nudged by email, this is now on *our* website for testing. Had to clean the patch slightly, because there have been a couple of changes since it was last rolled, but I've now had a read through and it seems fine. Assuming it works fine too, I'll commit it to dev shortly.

greg.harvey’s picture

greg.harvey
Primary language English
Location Occitanie, France
commented
Status: Needs review » Reviewed & tested by the community

Works great on our D7 site, committing.

greg.harvey’s picture

greg.harvey
Primary language English
Location Occitanie, France
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.