[app][cli] Finalize logging changes for #227

Author: Breakthrough

docs/changelog.md

@@ -352,5 +352,6 @@ This version of DVR-Scan includes a new, faster background subtraction algorithm
  * [feature] various UI enhancements:
      * input videos can now be sorted
      * add button to open log folder
- * [feature] Add `max-area`, `max-width`, and `max-height` options to help suppress improbable motion events, such as those caused by rain or fog  [#224](https://github.com/Breakthrough/DVR-Scan/issues/224) (thanks @elvis-epx)
- * [feature] Log files are now created with random names, and size limits have been removed. The number of logs to keep can be configured via `max-log-files`.
+ * [feature] Add `max-area`, `max-width`, and `max-height` options to help suppress improbable motion events, such as those caused by rain or fog [#224](https://github.com/Breakthrough/DVR-Scan/issues/224) (thanks @elvis-epx)
+ * [bugfix] Log files no longer append to the same file, and now have randomized suffixes to support multiple instances [#227](https://github.com/Breakthrough/DVR-Scan/issues/227)
+     * Config option changes: logs are no longer appended to the same file, so `max-log-size` is no longer required, and `max-log-files` has been raised from 4 to 15

dvr-scan.cfg

@@ -192,8 +192,6 @@
 # `--logfile` after running `dvr-scan --help` or `dvr-scan-app --help`.
 #save-log = yes
 
-# Max size of a debug log in bytes.
-
-# Max number of debug logs to keep. Old ones are deleted automatically.
-# Disk space usage will never exceed this times debug-log-max-len
-#max-log-files = 4
+# Max number of debug logs to keep. If more than this amount are found
+# on startup, the oldest ones are purged.
+#max-log-files = 15

dvr_scan/app/__main__.py

@@ -81,7 +81,7 @@ def get_cli_parser():
         type=str,
         help=(
             "Appends application output to file. If file does not exist it will be created. "
-            f"Debug log path: {LOGFILE_PATH}"
+            f"Log path: {LOGFILE_PATH.parent}"
         ),
     )
 
@@ -147,7 +147,7 @@ def main():
         if settings.get("save-log"):
             setup_logger(
                 logfile_path=LOGFILE_PATH,
-                max_files=settings.get("max-log-files"),
+                max_log_files=settings.get("max-log-files"),
                 name_prefix="dvr-scan-app",
             )
         failed_to_load_config = False

dvr_scan/cli.py

@@ -459,7 +459,7 @@ def get_cli_parser(user_config: ConfigRegistry):
         type=str,
         help=(
             "Appends application output to file. If file does not exist it will be created. "
-            f"Debug log path: {LOGFILE_PATH}"
+            f"Log path: {LOGFILE_PATH.parent}"
         ),
     )
 

dvr_scan/config.py

@@ -369,11 +369,11 @@ def from_config(config_value: str, default: "RGBValue") -> "RGBValue":
     # Logging
     "verbosity": "info",
     "save-log": True,
-    # TODO: max-log-size is not implemented. This is kept for backwards compatibility with older
-    # config files. Re-implementing size-based rotation is likely overly complicated since log
-    # files are not expected to be very large.
+    # max-log-size is not implemented, but is kept for backwards compatibility with older
+    # config files. Previously, logs would be appended to the same file until they reached this size
+    # after which a new file would be created.
     "max-log-size": 20000,
-    "max-log-files": 4,
+    "max-log-files": 15,
     # Development
     "debug": False,
 }

dvr_scan/controller.py

@@ -101,7 +101,7 @@ def parse_settings() -> ty.Optional[ScanSettings]:
         if config.get("save-log"):
             setup_logger(
                 logfile_path=LOGFILE_PATH,
-                max_files=config.get("max-log-files"),
+                max_log_files=config.get("max-log-files"),
                 name_prefix="dvr-scan",
             )
         failed_to_load_config = False

dvr_scan/shared/__init__.py

@@ -19,7 +19,6 @@
 import string
 import typing as ty
 from contextlib import contextmanager
-from datetime import datetime
 from logging import FileHandler
 from pathlib import Path
 
@@ -74,40 +73,44 @@ def logfile_path(name_prefix: str) -> Path:
     folder.mkdir(parents=True, exist_ok=True)
     # Generate a random suffix so multiple instances of dvr-scan don't try to write to the same
     # log file.
-    random_suffix = "".join(random.choices(string.ascii_lowercase + string.digits, k=8))
-    return folder / Path(f"{name_prefix}-{datetime.now():%Y%m%d-%H%M%S}-{random_suffix}.log")
+    random_suffix = "".join(random.choices(string.ascii_lowercase + string.digits, k=12))
+    return folder / Path(f"{name_prefix}.{random_suffix}.log")
 
 
-def prune_log_files(log_folder: Path, max_files: int, name_prefix: str):
+def prune_log_files(log_folder: Path, max_log_files: int, name_prefix: str):
     """Prune log files, keeping the latest `max_files` number of logs."""
     # Prune oldest log files if we have too many.
-    if max_files > 0:
+    if max_log_files > 0:
         # We find all DVR-Scan log files by globbing, then remove the oldest ones.
-        log_file_pattern = str(log_folder / f"{name_prefix}-*.log")
+        log_file_pattern = str(log_folder / f"{name_prefix}.*.log")
         log_files = list(glob.glob(log_file_pattern))
-        if len(log_files) > max_files:
+        if len(log_files) > max_log_files:
+            logger.debug(
+                "pruning oldest logs:"
+                f" max-log-files = {max_log_files}, len(log_files) = {len(log_files)}"
+            )
             log_files.sort(key=os.path.getmtime)
-            for i in range(len(log_files) - max_files):
-                logger.debug("Removing old log file: %s", log_files[i])
+            for i in range(len(log_files) - max_log_files):
                 try:
                     os.remove(log_files[i])
+                    logger.debug("removed log: %s", log_files[i])
                 except PermissionError:
                     logger.warning(
-                        "Failed to remove old log file: %s. It might be in use by another "
-                        "DVR-Scan process.",
+                        "Failed to remove log file: %s. It might be in use by another process. "
+                        "Try raising `max-log-files` if this is the case.",
                         log_files[i],
                     )
 
 
-def setup_logger(logfile_path: Path, max_files: int, name_prefix: str):
+def setup_logger(logfile_path: Path, max_log_files: int, name_prefix: str):
     """Initialize rolling debug logger."""
-    prune_log_files(logfile_path.parent, max_files, name_prefix)
+    prune_log_files(logfile_path.parent, max_log_files, name_prefix)
+    logger.debug(f"writing logs to {logfile_path}")
     handler = FileHandler(str(logfile_path))
     handler.setLevel(logging.DEBUG)
     handler.setFormatter(logging.Formatter(fmt=LOG_FORMAT_ROLLING_LOGS))
     # *WARNING*: This log message must come before we attach the handler otherwise it will get
     # written to the log file each time.
-    logger.debug(f"writing logs to {logfile_path} (max_files: {max_files})")
     attach_log_handler(handler)