Repository: ansible/ansible-navigator (language: python)

Issue:
playbook-artifact file name with OS timezone-based time
<!-- Issues are for **concrete, actionable bugs and feature requests** only - if you're just asking for debugging help or technical support, please use:

TODO: Add ansible-navigator specific channels

We have to limit this because of limited volunteer time to respond to issues! -->

##### ISSUE TYPE

- Feature Idea

##### SUMMARY

<!-- Briefly describe the problem or desired enhancement. -->

I also want the time based on the OS time zone as a key to be included in the file name of playbook-artifact. Not just ts_utc.








Discussion / hints:

Fix patch:
diff --git a/docs/changelog-fragments.d/1086.feature.md b/docs/changelog-fragments.d/1086.feature.md
new file mode 100644
index 000000000..0376353c5
--- /dev/null
+++ b/docs/changelog-fragments.d/1086.feature.md
@@ -0,0 +1,4 @@
+Enabled the use of the `time_zone` settings entry for time stamps
+within the log file.
+
+-- by {user}`cidrblock`
diff --git a/src/ansible_navigator/cli.py b/src/ansible_navigator/cli.py
index 41f54a76c..a006de988 100644
--- a/src/ansible_navigator/cli.py
+++ b/src/ansible_navigator/cli.py
@@ -1,16 +1,17 @@
 # cspell:ignore getpid, gmtime, msecs
 """start here
 """
+import datetime
 import logging
 import os
 import signal
 import sys
-import time
 
 from copy import deepcopy
 from curses import wrapper
 from pathlib import Path
 from typing import List
+from typing import Optional
 from typing import Union
 
 from pkg_resources import working_set
@@ -27,6 +28,7 @@
 from .image_manager import ImagePuller
 from .initialization import error_and_exit_early
 from .initialization import parse_and_update
+from .utils.compatibility import zoneinfo
 from .utils.functions import ExitMessage
 from .utils.functions import ExitPrefix
 from .utils.functions import LogMessage
@@ -75,6 +77,37 @@ def pull_image(args):
         error_and_exit_early(image_puller.assessment.exit_messages)
 
 
+class Formatter(logging.Formatter):
+    """Format a logging timestamp using a time zone."""
+
+    def __init__(self, *args, **kwargs):
+        """Initialize the logging formatter.
+
+        :param args: The arguments
+        :param kwargs: The keyword arguments
+        """
+        self._time_zone = kwargs.pop("time_zone")
+        super().__init__(*args, **kwargs)
+
+    def formatTime(self, record: logging.LogRecord, _datefmt: Optional[str] = None) -> str:
+        """Format the log timestamp.
+
+        :param record: The log record
+        :param _datefmt: The optiona date format
+        :returns: The timestamp
+        """
+        if self._time_zone == "local":
+            return (
+                datetime.datetime.fromtimestamp(record.created, datetime.timezone.utc)
+                .astimezone()
+                .isoformat()
+            )
+        return datetime.datetime.fromtimestamp(
+            record.created,
+            tz=zoneinfo.ZoneInfo(self._time_zone),
+        ).isoformat()
+
+
 def setup_logger(args: ApplicationConfiguration) -> None:
     """set up the logger
 
@@ -83,11 +116,16 @@ def setup_logger(args: ApplicationConfiguration) -> None:
     if os.path.exists(args.log_file) and args.log_append is False:
         os.remove(args.log_file)
     handler = logging.FileHandler(args.log_file)
-    formatter = logging.Formatter(
-        fmt="%(asctime)s.%(msecs)03d %(levelname)s '%(name)s.%(funcName)s' %(message)s",
-        datefmt="%y%m%d%H%M%S",
+
+    time_zone = args.entry("time_zone").value.current
+    # When the configuration is rolled back, the time_zone will be C.NOT_SET
+    if isinstance(args.time_zone, Constants):
+        time_zone = args.entry("time_zone").value.default
+
+    formatter = Formatter(
+        fmt="%(asctime)s %(levelname)s '%(name)s.%(funcName)s' %(message)s",
+        time_zone=time_zone,
     )
-    setattr(formatter, "converter", time.gmtime)
     handler.setFormatter(formatter)
     logger.addHandler(handler)
     log_level = getattr(logging, args.log_level.upper())


Test patch:
diff --git a/tests/unit/logging/__init__.py b/tests/unit/logging/__init__.py
new file mode 100644
index 000000000..2085d8111
--- /dev/null
+++ b/tests/unit/logging/__init__.py
@@ -0,0 +1,1 @@
+"""Tests related to log generation and formatting."""
diff --git a/tests/unit/logging/test_append.py b/tests/unit/logging/test_append.py
new file mode 100644
index 000000000..e84ee630c
--- /dev/null
+++ b/tests/unit/logging/test_append.py
@@ -0,0 +1,80 @@
+"""Tests for appending to the log."""
+
+from dataclasses import dataclass
+from pathlib import Path
+from typing import List
+
+import pytest
+
+from ansible_navigator import cli
+
+
+@dataclass
+class TestData:
+    """Data for the log append tests."""
+
+    log_append: bool
+    repeat: int = 5
+    session_count: int = 1
+
+    def __str__(self):
+        """Provide the test id.
+
+        :returns: The test id
+        """
+        return f"{self.log_append}"
+
+    def args(self, log_file: Path) -> List[str]:
+        """Provide an argument list for the CLI.
+
+        :param log_file: The path to the lgo file
+        :returns: The list of CLI arguments
+        """
+        arg_list = [
+            "ansible-navigator",
+            "--la",
+            str(self.log_append),
+            "--lf",
+            str(log_file),
+            "--ll",
+            "info",
+        ]
+        return arg_list
+
+
+test_data = (
+    TestData(log_append=True, session_count=5),
+    TestData(log_append=False, session_count=1),
+)
+
+
+@pytest.mark.parametrize("data", test_data, ids=str)
+def test(data: TestData, monkeypatch: pytest.MonkeyPatch, tmp_path: Path):
+    """Start with the CLI, create log messages and count.
+
+    :param data: The test data
+    :param monkeypatch: The monkeypatch fixture
+    :param tmp_path: A temporary file path
+    """
+
+    def return_none(*_args, **_kwargs) -> None:
+        """Take no action, return none.
+
+        :param _args: Arguments
+        :param _kwargs: Keyword arguments
+        :returns: Nothing
+        """
+        return None
+
+    new_session_msg = "New ansible-navigator instance"
+    log_file = tmp_path / "ansible-navigator.log"
+
+    args = data.args(log_file=log_file)
+    monkeypatch.setattr("sys.argv", args)
+    monkeypatch.setattr("ansible_navigator.cli.wrapper", return_none)
+
+    for _ in range(data.repeat):
+        cli.main()
+        # prevent multiple handlers
+        cli.logger.handlers.clear()
+    assert log_file.read_text().count(new_session_msg) == data.session_count
diff --git a/tests/unit/logging/test_time_zone.py b/tests/unit/logging/test_time_zone.py
new file mode 100644
index 000000000..59a3ef8f3
--- /dev/null
+++ b/tests/unit/logging/test_time_zone.py
@@ -0,0 +1,89 @@
+"""Tests for time zone support in the log."""
+
+import re
+
+from dataclasses import dataclass
+from pathlib import Path
+from typing import List
+from typing import Optional
+from typing import Pattern
+
+import pytest
+
+from ansible_navigator import cli
+
+
+@dataclass
+class TestData:
+    """Data for time zone support in the logs."""
+
+    re_match: Pattern
+    time_zone: Optional[str] = None
+    will_exit: bool = False
+
+    def __str__(self):
+        """Provide the test id.
+
+        :returns: The test id
+        """
+        return f"{self.time_zone}"
+
+    def args(self, log_file: Path) -> List[str]:
+        """Provide an argument list for the CLI.
+
+        :param log_file: The path to the lgo file
+        :returns: The list of CLI arguments
+        """
+        arg_list = ["ansible-navigator", "--la", "false", "--lf", str(log_file), "--ll", "debug"]
+        if isinstance(self.time_zone, str):
+            arg_list.extend(["--tz", self.time_zone])
+        return arg_list
+
+
+test_data = (
+    TestData(re_match=re.compile(r"^.*\+00:00$")),
+    TestData(re_match=re.compile(r"^.*-0[78]:00$"), time_zone="America/Los_Angeles"),
+    TestData(re_match=re.compile(r"^.*\+09:00$"), time_zone="Japan"),
+    TestData(re_match=re.compile(r"^.*[+-][01][0-9]:[0-5][0-9]$"), time_zone="local"),
+    TestData(re_match=re.compile(r"^.*\+00:00$"), time_zone="does_not_exist", will_exit=True),
+)
+
+
+@pytest.mark.parametrize("data", test_data, ids=str)
+def test(
+    data: TestData,
+    caplog: pytest.LogCaptureFixture,
+    monkeypatch: pytest.MonkeyPatch,
+    tmp_path: Path,
+):
+    """Start with the CLI, create log messages and match the time zone.
+
+    :param caplog: The log capture fixture
+    :param data: The test data
+    :param monkeypatch: The monkeypatch fixture
+    :param tmp_path: A temporary file path
+    """
+
+    def return_none(*_args, **_kwargs) -> None:
+        """Take no action, return none.
+
+        :param _args: Arguments
+        :param _kwargs: Keyword arguments
+        :returns: Nothing
+        """
+        return None
+
+    log_file = tmp_path / "ansible-navigator.log"
+    args = data.args(log_file=log_file)
+    monkeypatch.setattr("sys.argv", args)
+    monkeypatch.setattr("ansible_navigator.cli.wrapper", return_none)
+
+    if data.will_exit:
+        with pytest.raises(SystemExit):
+            cli.main()
+    else:
+        cli.main()
+    # This is a conservative number based on debug logging, it should be closer to 200
+    # but this assertion is here to ensure many records were retrieved.
+    assert len(caplog.records) > 100
+    assert all(data.re_match.match(record.asctime) for record in caplog.records)
diff --git a/tests/unit/test_log_append.py b/tests/unit/test_log_append.py
deleted file mode 100644
index 3625029c5..000000000
--- a/tests/unit/test_log_append.py
+++ /dev/null
@@ -1,63 +0,0 @@
-"""Test appending to the log.
-"""
-
-# pylint: disable=preferred-module  # FIXME: remove once migrated per GH-872
-from unittest import mock
-
-from ansible_navigator import cli
-
-
-def test_log_append_true(tmp_path):
-    """run 5 times, append to log each time"""
-
-    def side_effect(*_args, **_kwargs):
-        return None
-
-    repeat = 5
-    new_session_msg = "New ansible-navigator instance"
-    log_file = tmp_path / "ansible-navigator.log"
-    cli_args = [
-        "ansible-navigator",
-        "--la",
-        "true",
-        "--lf",
-        str(log_file),
-        "--ll",
-        "info",
-    ]
-    with mock.patch("sys.argv", cli_args):
-        with mock.patch("ansible_navigator.cli.wrapper") as mocked_wrapper:
-            mocked_wrapper.side_effect = side_effect
-            for _ in range(repeat):
-                cli.main()
-                # prevent multiple handlers
-                cli.logger.handlers.clear()
-    assert log_file.read_text().count(new_session_msg) == repeat
-
-
-def test_log_append_false(tmp_path):
-    """run 5 times, never append to log each time"""
-
-    def side_effect(*_args, **_kwargs):
-        return None
-
-    repeat = 5
-    new_session_msg = "New ansible-navigator instance"
-    log_file = tmp_path / "ansible-navigator.log"
-    cli_args = [
-        "ansible-navigator",
-        "--la",
-        "false",
-        "--lf",
-        str(log_file),
-        "--ll",
-        "info",
-    ]
-    with mock.patch("sys.argv", cli_args):
-        with mock.patch("ansible_navigator.cli.wrapper") as mocked_wrapper:
-            mocked_wrapper.side_effect = side_effect
-            for _ in range(repeat):
-                cli.main()
-                # prevent multiple handlers
-                cli.logger.handlers.clear()
-                assert log_file.read_text().count(new_session_msg) == 1