refactor(file-count): stopping when number of files actually exceed thresholds, therefore dramatically faster for large directories

Author: markuslf

CHANGELOG.md

@@ -41,7 +41,7 @@ Monitoring Plugins:
 * by-ssh: add missing `--verbose` parameter
 * fs-ro: ignore `/run/credentials` (https://systemd.io/CREDENTIALS/)
 * keycloak-stats: fix incorrect symlink for lib
-* ntp-*: prevent `TypeError: ''=' not supported between instances of 'int' and 'str'`
+* ntp-\*: prevent `TypeError: ''=' not supported between instances of 'int' and 'str'`
 
 
 ### Changed ("refactor", "chore" etc.)
@@ -64,6 +64,7 @@ Grafana:
 Monitoring Plugins:
 
 * all plugins: ignore unknown arguments instead of generating an error (this helps with updating Icinga and Nagios service definitions considerably)
+* file-count: stopping when number of files actually exceed thresholds, therefore dramatically faster for large directories
 * nextcloud-version: modernize code
 * php-status: always assume http://localhost/monitoring.php and, if not found, be tolerant
 * redis-status, valkey-status: modernize code and unify both plugins again after [PR #954](https://github.com/Linuxfabrik/monitoring-plugins/pull/954)

check-plugins/file-count/README.md

@@ -4,7 +4,7 @@
 
 Checks the number of matching files or directories found. It can be also used to check the existence / absence of a single file.
 
-Depending on the file and user (e.g. running as *icinga*), sudo (sudoers) is needed. It supports globs in accordance with [Python 3](https://docs.python.org/3/library/pathlib.html#pathlib.Path.glob) or [Python 2](https://docs.python.org/2.7/library/glob.html). Beware that using recursive globs might cause high memory usage. Also note that there are small differences in recursive file matching between Python 2 and Python 3. Optionally, the check can be restricted to only consider files that were modified in a given timerange.
+Depending on the file and user (e.g. running as *icinga*), sudo (sudoers) is needed. It supports globs in accordance with [Python 3](https://docs.python.org/3/library/pathlib.html#pathlib.Path.glob). Beware that using recursive globs might cause high memory usage. Optionally, the check can be restricted to only consider files that were modified in a given timerange.
 
 
 ## Fact Sheet
@@ -38,8 +38,8 @@ options:
                         accordance with https://docs.python.org/3/library/path
                         lib.html#pathlib.Path.glob. Beware of using recursive
                         globs. This is mutually exclusive with -u / --url.
-  --only-dirs           Only consider directories.
-  --only-files          Only consider files.
+  --only-dirs           Only count directories.
+  --only-files          Only count files.
   --password PASSWORD   SMB Password.
   --pattern PATTERN     The search string to match against the names of SMB
                         directories or files. This pattern can use '*' as a

check-plugins/file-count/file-count

@@ -11,25 +11,25 @@
 """See the check's README for more details.
 """
 
-import argparse  # pylint: disable=C0413
-import sys  # pylint: disable=C0413
-from pathlib import Path  # pylint: disable=C0413
+import argparse
+import stat
+import sys
+from pathlib import Path
 
-import lib.base  # pylint: disable=C0413
+import lib.base
 
 try:
-    import lib.smb  # pylint: disable=C0413
+    import lib.smb
     HAVE_SMB = True
 except ModuleNotFoundError as e:
     HAVE_SMB = False
     missing_lib = e.name
-import lib.time  # pylint: disable=C0413
-import lib.txt  # pylint: disable=C0413
-from lib.globals import (STATE_CRIT, STATE_OK,  # pylint: disable=C0413
-                          STATE_UNKNOWN, STATE_WARN)
+import lib.time
+import lib.txt
+from lib.globals import (STATE_CRIT, STATE_OK, STATE_UNKNOWN, STATE_WARN)
 
 __author__ = 'Linuxfabrik GmbH, Zurich/Switzerland'
-__version__ = '2025100601'
+__version__ = '2025122301'
 
 DESCRIPTION = 'Checks the number of matching files.'
 
@@ -45,7 +45,7 @@ def parse_args():
     parser.add_argument(
         '-V', '--version',
         action='version',
-        version='%(prog)s: v{} by {}'.format(__version__, __author__)
+        version=f'%(prog)s: v{__version__} by {__author__}'
     )
 
     parser.add_argument(
@@ -64,21 +64,24 @@ def parse_args():
 
     parser.add_argument(
         '--filename',
-        help='File (or directory) name to check. Supports glob in accordance with https://docs.python.org/3/library/pathlib.html#pathlib.Path.glob. Beware of using recursive globs. This is mutually exclusive with -u / --url.',
+        help='File (or directory) name to check. Supports glob in accordance with '
+             'https://docs.python.org/3/library/pathlib.html#pathlib.Path.glob. '
+             'Beware of using recursive globs. '
+             'This is mutually exclusive with -u / --url.',
         dest='FILENAME',
     )
 
     parser.add_argument(
         '--only-dirs',
-        help='Only consider directories.',
+        help='Only count directories.',
         dest='ONLY_DIRS',
         action='store_true',
         default=False,
     )
 
     parser.add_argument(
         '--only-files',
-        help='Only consider files.',
+        help='Only count files.',
         dest='ONLY_FILES',
         action='store_true',
         default=False,
@@ -92,28 +95,35 @@ def parse_args():
 
     parser.add_argument(
         '--pattern',
-        help="The search string to match against the names of SMB directories or files. This pattern can use '*' as a wildcard for multiple chars and '?' as a wildcard for a single char. Does not support regex patterns. Default: %(default)s.",
+        help="The search string to match against the names of SMB directories or files. "
+             "This pattern can use '*' as a wildcard for multiple chars and '?' as a wildcard for "
+             "a single char. "
+             "Does not support regex patterns. "
+             "Default: %(default)s.",
         dest='PATTERN',
         default=DEFAULT_PATTERN,
     )
 
     parser.add_argument(
         '--timeout',
-        help='Network timeout in seconds. Default: %(default)s (seconds)',
+        help='Network timeout in seconds. '
+             'Default: %(default)s (seconds)',
         dest='TIMEOUT',
         type=int,
         default=DEFAULT_TIMEOUT,
     )
 
     parser.add_argument(
         '--timerange',
-        help='Set the timerange (seconds) in which the files should be considered. Supports ranges.',
+        help='Set the timerange (seconds) in which the files should be considered. '
+             'Supports ranges.',
         dest='TIMERANGE',
     )
 
     parser.add_argument(
         '-u', '--url',
-        help='Set the url of the file (or directory) to check, starting with "smb://". This is mutually exclusive with --filename.',
+        help='Set the url of the file (or directory) to check, starting with "smb://". '
+             'This is mutually exclusive with --filename.',
         dest='URL',
         type=str,
     )
@@ -126,14 +136,47 @@ def parse_args():
 
     parser.add_argument(
         '-w', '--warning',
-        help='Set the warning number of files. Supports ranges.',
+        help='Set the warning number of files. '
+             'Supports ranges.',
         dest='WARN',
     )
 
     args, _ = parser.parse_known_args()
     return args
 
 
+def get_early_break_threshold(warn, crit):
+    """Extract a numeric threshold for early loop termination.
+    Returns None if thresholds are too complex to parse.
+    """
+    thresholds = []
+    for threshold in [warn, crit]:
+        if not threshold:
+            continue
+        # Try to extract numeric value from simple threshold formats
+        # Handles: "100", "10:100", ":100", "10:", "@10:100"
+        threshold = threshold.lstrip('@')
+        if ':' in threshold:
+            parts = threshold.split(':')
+            # Get the upper bound if it exists
+            if parts[-1]:
+                try:
+                    thresholds.append(int(parts[-1]))
+                except ValueError:
+                    return None
+        else:
+            # Simple numeric threshold
+            try:
+                thresholds.append(int(threshold))
+            except ValueError:
+                return None
+
+    if thresholds:
+        # Use the maximum threshold + 1 to confirm we're "more than"
+        return max(thresholds) + 1
+    return None
+
+
 def main():
     """The main function. Hier spielt die Musik.
     """
@@ -146,48 +189,88 @@ def main():
 
     # fetch data
     if args.FILENAME and args.URL:
-        lib.base.cu('The --filename and -u / --url parameter are mutually exclusive. Please only use one.')
+        lib.base.cu(
+            'The --filename and -u / --url parameter are mutually exclusive. Please only use one.'
+        )
 
     now = lib.time.now()
     msg = ''
     file_count = 0
+    exceeded_threshold = False
+
+    # Determine if we can break early for performance
+    early_break_at = get_early_break_threshold(args.WARN, args.CRIT)
 
     if args.FILENAME:
         path = Path(args.FILENAME)
-        for item in sorted(Path(path.anchor).glob(str(path.relative_to(path.anchor)))):
-            if item.is_file() and args.ONLY_DIRS:
+        for item in Path(path.anchor).glob(str(path.relative_to(path.anchor))):
+            # Get stat info once to avoid multiple syscalls
+            try:
+                stat_info = item.stat()
+            except (OSError, PermissionError):
+                # Skip files we can't stat
+                continue
+
+            is_dir = stat.S_ISDIR(stat_info.st_mode)
+            is_file = stat.S_ISREG(stat_info.st_mode)
+
+            if is_file and args.ONLY_DIRS:
                 continue
-            if item.is_dir() and args.ONLY_FILES:
+            if is_dir and args.ONLY_FILES:
                 continue
             if args.TIMERANGE:
-                age = (now - item.stat().st_mtime)
+                age = now - stat_info.st_mtime
                 if not lib.base.coe(lib.base.match_range(age, args.TIMERANGE)):
                     continue
 
             file_count += 1
 
+            # Break early if we've exceeded the threshold because it makes no sense to count any
+            # further, it just costs time and resources
+            if early_break_at and file_count >= early_break_at:
+                exceeded_threshold = True
+                break
+
     if args.URL:
         split_url = args.URL.split('://')
         if len(split_url) != 2:
-            lib.base.cu('Could not parse the protocol of the url "{}".'.format(args.URL))
+            lib.base.cu(f'Could not parse the protocol of the url "{args.URL}".')
         proto, url = split_url
 
         if proto == 'smb':
             if not HAVE_SMB:
-                lib.base.cu('Python module "{}" is not installed.'.format(missing_lib))
-            for item in lib.base.coe(lib.smb.glob(url, args.USERNAME, args.PASSWORD, args.TIMEOUT, pattern=args.PATTERN)):
-                if item.is_file() and args.ONLY_DIRS:
+                lib.base.cu(f'Python module "{missing_lib}" is not installed.')  # pylint: disable=E0601
+            for item in lib.base.coe(
+                lib.smb.glob(url, args.USERNAME, args.PASSWORD, args.TIMEOUT, pattern=args.PATTERN)
+            ):
+                # Get stat info once to avoid multiple calls
+                try:
+                    stat_info = item.stat()
+                except (OSError, PermissionError):
+                    # Skip files we can't stat
+                    continue
+
+                is_dir = stat.S_ISDIR(stat_info.st_mode)
+                is_file = stat.S_ISREG(stat_info.st_mode)
+
+                if is_file and args.ONLY_DIRS:
                     continue
-                if item.is_dir() and args.ONLY_FILES:
+                if is_dir and args.ONLY_FILES:
                     continue
                 if args.TIMERANGE:
-                    age = (now - item.stat().st_mtime)
+                    age = now - stat_info.st_mtime
                     if not lib.base.coe(lib.base.match_range(age, args.TIMERANGE)):
                         continue
 
                 file_count += 1
+
+                # Break early if we've exceeded the threshold because it makes no sense to count any
+                # further, it just costs time and resources
+                if early_break_at and file_count >= early_break_at:
+                    exceeded_threshold = True
+                    break
         else:
-            lib.base.cu('The protocol "{}" is not supported.'.format(proto))
+            lib.base.cu(f'The protocol "{proto}" is not supported.')
 
     if not lib.base.coe(lib.base.match_range(file_count, args.CRIT)):
         state = STATE_CRIT
@@ -196,12 +279,15 @@ def main():
     else:
         state = STATE_OK
 
-    msg = 'Found {} matching {} (thresholds {}/{})'.format(
-        file_count,
-        lib.txt.pluralize('file', file_count),
-        args.WARN,
-        args.CRIT,
-    )
+    if exceeded_threshold:
+        # Report "more than" the threshold value we broke at
+        display_count = file_count - 1
+        msg = f'Found more than {display_count} matching ' \
+              f'{lib.txt.pluralize("file", display_count)} (thresholds {args.WARN}/{args.CRIT})'
+    else:
+        msg = f'Found {file_count} matching ' \
+              f'{lib.txt.pluralize("file", file_count)} (thresholds {args.WARN}/{args.CRIT})'
+
     perfdata = lib.base.get_perfdata('file_count', file_count, None, args.WARN, args.CRIT, 0, None)
 
     # over and out

check-plugins/file-count/icingaweb2-module-director/file-count.json

@@ -443,7 +443,7 @@
         "4": {
             "varname": "file_count_only_dirs",
             "caption": "File Count: Only Dirs?",
-            "description": "Only consider directories.",
+            "description": "Only count directories.",
             "datatype": "Icinga\\Module\\Director\\DataType\\DataTypeBoolean",
             "format": null,
             "settings": {},
@@ -452,7 +452,7 @@
         "5": {
             "varname": "file_count_only_files",
             "caption": "File Count: Only Files?",
-            "description": "Only consider files.",
+            "description": "Only count files.",
             "datatype": "Icinga\\Module\\Director\\DataType\\DataTypeBoolean",
             "format": null,
             "settings": {},
@@ -569,7 +569,7 @@
         "16": {
             "varname": "file_count_windows_only_dirs",
             "caption": "File Count: Only Dirs?",
-            "description": "Only consider directories.",
+            "description": "Only count directories.",
             "datatype": "Icinga\\Module\\Director\\DataType\\DataTypeBoolean",
             "format": null,
             "settings": {},
@@ -578,7 +578,7 @@
         "17": {
             "varname": "file_count_windows_only_files",
             "caption": "File Count: Only Files?",
-            "description": "Only consider files.",
+            "description": "Only count files.",
             "datatype": "Icinga\\Module\\Director\\DataType\\DataTypeBoolean",
             "format": null,
             "settings": {},