change message and state when logfile is not found #37

kurneko · 2019-09-06T09:30:58Z

If logfile is not found. This plugin return "OK - No matches found.".
I worry about if I wrong name in --logfile option. i will not notice it.

In check_log3.pl, if can not read logfile, return error.
https://github.com/pmcaulay/nagios-plugins/blob/master/check_log3.pl#L921-L934

** Proposal

If logfile is not found. Return code UNKNOWN, And message 「UNKNOWN: Logfile is not found.」

kurneko · 2019-09-09T02:04:48Z

oh... travisCI errors.
This error is not relevant.

[python 2.6]

0.12s$ curl -sSf -o python-2.6.tar.bz2 ${archive_url}
curl: (22) The requested URL returned error: 404 Not Found
Unable to download 2.6 archive. The archive may not exist. Please consider a different version.

[python 2.7 ~ ]

======================================================================
FAIL: test_logfile (test_check_log_ng.LogCheckerTestCase)
--logfile option
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_check_log_ng.py", line 564, in test_logfile
    line1, self.logfile1, line2, self.logfile2))
AssertionError: u'WARNING: Found 2 lines (limit=1/0): Sep  6 09:32:41 hostname test: ERROR at /h [truncated]... != u'WARNING: Found 2 lines (limit=1/0): Sep  6 09:32:41 hostname test: ERROR at /h [truncated]...
Diff is 985 characters long. Set self.maxDiff to None to see it.
----------------------------------------------------------------------

ttkzw · 2019-09-09T07:28:27Z

I fixed the issue #38 which Travis CI failed.

kurneko · 2019-10-01T09:53:28Z

thanks. travis-ci check was ok.

netmarkjp · 2019-10-07T07:42:05Z

@kurneko Thanks for PR.

"If log file does not exist, return OK" is expected behavior.
This PR lead to unaccepatable breaking change to default behavior, I hope you to add arg/opt to switch behavior.
(ex: --ensure-file-exist , --error-no-exist )

kurneko · 2019-10-26T10:02:13Z

@netmarkjp

Thank you for checking

This PR lead to unaccepatable breaking change to default behavior, I hope you to add arg/opt to switch behavior.

OK. Add --error-no-exist option.
please review.

netmarkjp · 2019-10-28T03:48:13Z

check_log_ng.py

@@ -613,6 +613,10 @@ def _check_log(self, logfile, seekfile):
        _debug("logfile='{0}', seekfile='{1}'".format(logfile, seekfile))
        logfile = LogChecker.to_unicode(logfile)
        if not os.path.exists(logfile):
+            # if set error_no_exist, return UNKNOWN.
+            if self.config['error_no_exist']:
+                self.message = "UNKNOWN: Logfile is not found."


I think it's better to contain logfile path in output.

OK. I modified to display logfile path.

netmarkjp · 2019-10-28T03:48:24Z

check_log_ng.py

+
+        # if set error_no_exist, return UNKNOWN.
+        if self.config['error_no_exist'] and log_exist == False:
+            self.message = "UNKNOWN: Logfile is not found."


I think it's better to contain logfile path in output.

OK. I modified to display logfile path.

netmarkjp · 2019-10-28T04:01:25Z

check_log_ng.py

@@ -696,6 +702,11 @@ def _check_log_multi(self, logfile_pattern, remove_seekfile=False, tag=''):
                self._remove_old_seekfile_with_inode(logfile_pattern, tag)
            else:
                self._remove_old_seekfile(logfile_pattern, tag)
+
+        # if set error_no_exist, return UNKNOWN.
+        if self.config['error_no_exist'] and log_exist == False:


I think == False ( and == True ) is not good expression.
and not log_exist is better.

In this context, I think the log_exist variable is unnecessary, "and not logfile_list" is enough.
( Evaluation of "list" returns false if len(list) == 0 )

log_exist is removed. and use len(logfile_list) == 0

netmarkjp · 2019-10-28T04:16:32Z

check_log_ng.py

+        action="store_true",
+        dest="error_no_exist",
+        default=False,
+        help=("If file not exist, return UNKNOWN error.")


I think we should use one of "not found" or "not exist" in all messages.

OK. Use only "not exist".

ttkzw · 2019-10-28T06:09:05Z

The variable name log_exist is ambiguous.
I think logfile_exists or logfile_exist is better.

ttkzw · 2019-10-28T06:16:27Z

I think the option name --error-no-exist and the variable name error_no_exist are ambiguous.
What does not exist?

modify help message

kurneko · 2019-11-10T11:57:08Z

The variable name log_exist is ambiguous.
I think logfile_exists or logfile_exist is better.

The variable "log_exist" has been removed.

kurneko · 2019-11-10T12:04:23Z

I think the option name --error-no-exist and the variable name error_no_exist are ambiguous.
What does not exist?

You’re right. Change "--error-no-exist" to "--error-logfile-not-exist".

ttkzw · 2020-08-19T04:36:06Z

This looks good.

change message and state when logfile is not found

588b0de

retry travis-ci

fd7675e

add option --error-no-exist

a69d442

netmarkjp reviewed Oct 28, 2019

View reviewed changes

kurneko added 4 commits October 30, 2019 20:28

modify error message: to display log file name.

10f909b

remove log_exist flag

e8bef2c

modify option name error-no-exist to error-logfile-not-exist

a35d039

modify help message

Unify words

850ca60

kurneko added 2 commits August 19, 2020 12:47

rename error-no-exist -> error-logfile-not-exist

3785989

grammar correction

5a0d2e3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

change message and state when logfile is not found #37

change message and state when logfile is not found #37

kurneko commented Sep 6, 2019

kurneko commented Sep 9, 2019

ttkzw commented Sep 9, 2019

kurneko commented Oct 1, 2019

netmarkjp commented Oct 7, 2019

kurneko commented Oct 26, 2019

netmarkjp Oct 28, 2019

kurneko Nov 9, 2019

netmarkjp Oct 28, 2019

kurneko Nov 9, 2019

netmarkjp Oct 28, 2019

kurneko Nov 10, 2019

netmarkjp Oct 28, 2019

kurneko Nov 10, 2019

ttkzw commented Oct 28, 2019 •

edited

Loading

ttkzw commented Oct 28, 2019

kurneko commented Nov 10, 2019

kurneko commented Nov 10, 2019

ttkzw commented Aug 19, 2020

change message and state when logfile is not found #37

Are you sure you want to change the base?

change message and state when logfile is not found #37

Conversation

kurneko commented Sep 6, 2019

kurneko commented Sep 9, 2019

ttkzw commented Sep 9, 2019

kurneko commented Oct 1, 2019

netmarkjp commented Oct 7, 2019

kurneko commented Oct 26, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ttkzw commented Oct 28, 2019 • edited Loading

ttkzw commented Oct 28, 2019

kurneko commented Nov 10, 2019

kurneko commented Nov 10, 2019

ttkzw commented Aug 19, 2020

ttkzw commented Oct 28, 2019 •

edited

Loading