logging: simplify cli logging setup

lib/patch_status_json.py

@@ -18,8 +18,7 @@
 import json
 import logging
 
-from uaclient import system, util
-from uaclient.cli import setup_logging
+from uaclient import config, defaults, log, system, util
 
 LOG = logging.getLogger("ubuntupro.lib.patch_status_json")
 
@@ -64,7 +63,9 @@ def patch_status_json_schema_0_1(status_file: str):
 
 
 if __name__ == "__main__":
-    setup_logging(logging.DEBUG)
+    log.setup_cli_logging(logging.DEBUG, defaults.CONFIG_DEFAULTS["log_level"])
+    cfg = config.UAConfig()
+    log.setup_cli_logging(cfg.log_level, cfg.log_file)
     patch_status_json_schema_0_1(
         status_file="/var/lib/ubuntu-advantage/status.json"
     )

lib/upgrade_lts_contract.py

@@ -7,13 +7,12 @@
 
 import logging
 
-from uaclient import http, upgrade_lts_contract
-from uaclient.cli import setup_logging
-from uaclient.config import UAConfig
+from uaclient import config, defaults, http, log, upgrade_lts_contract
 
 if __name__ == "__main__":
-    setup_logging(logging.DEBUG)
-    cfg = UAConfig()
+    log.setup_cli_logging(logging.DEBUG, defaults.CONFIG_DEFAULTS["log_level"])
+    cfg = config.UAConfig()
+    log.setup_cli_logging(cfg.log_level, cfg.log_file)
     http.configure_web_proxy(cfg.http_proxy, cfg.https_proxy)
     upgrade_lts_contract.process_contract_delta_after_apt_lock(cfg)
     upgrade_lts_contract.remove_private_esm_apt_cache()

uaclient/cli/__init__.py

@@ -3,7 +3,6 @@
 import argparse
 import json
 import logging
-import pathlib
 import sys
 import tarfile
 import tempfile
@@ -25,9 +24,10 @@
     exceptions,
     http,
     lock,
+    log,
+    messages,
+    security_status,
 )
-from uaclient import log as pro_log
-from uaclient import messages, security_status
 from uaclient import status as ua_status
 from uaclient import timer, util, version
 from uaclient.api.api import call_api
@@ -66,7 +66,6 @@
 )
 from uaclient.files import notices, state_files
 from uaclient.files.notices import Notice
-from uaclient.log import JsonArrayFormatter
 from uaclient.timer.update_messaging import refresh_motd, update_motd_messages
 from uaclient.yaml import safe_dump, safe_load
 
@@ -1650,43 +1649,6 @@ def _warn_about_output_redirection(cmd_args) -> None:
         )
 
 
-def setup_logging(log_level, log_file=None, logger=None):
-    """Setup console logging and debug logging to log_file
-
-    It configures the pro client logger.
-    If run as non_root and cfg.log_file is provided, it is replaced
-    with another non-root log file.
-    """
-    if log_file is None:
-        cfg = config.UAConfig()
-        log_file = cfg.log_file
-    # if we are running as non-root, change log file
-    if not util.we_are_currently_root():
-        log_file = pro_log.get_user_log_file()
-
-    if isinstance(log_level, str):
-        log_level = log_level.upper()
-
-    if not logger:
-        logger = logging.getLogger("ubuntupro")
-    logger.setLevel(log_level)
-    logger.addFilter(pro_log.RedactionFilter())
-
-    # Clear all handlers, so they are replaced for this logger
-    logger.handlers = []
-
-    # Setup file logging
-    log_file_path = pathlib.Path(log_file)
-    if not log_file_path.exists():
-        log_file_path.parent.mkdir(parents=True, exist_ok=True)
-        log_file_path.touch(mode=0o640)
-    file_handler = logging.FileHandler(log_file)
-    file_handler.setFormatter(JsonArrayFormatter())
-    file_handler.setLevel(log_level)
-    file_handler.set_name("upro-file")
-    logger.addHandler(file_handler)
-
-
 def set_event_mode(cmd_args):
     """Set the right event mode based on the args provided"""
     if cmd_args.command in ("attach", "detach", "enable", "disable", "status"):
@@ -1780,12 +1742,12 @@ def wrapper(*args, **kwargs):
 
 @main_error_handler
 def main(sys_argv=None):
-    setup_logging(
+    log.setup_cli_logging(
         defaults.CONFIG_DEFAULTS["log_level"],
         defaults.CONFIG_DEFAULTS["log_file"],
     )
     cfg = config.UAConfig()
-    setup_logging(cfg.log_level, cfg.log_file)
+    log.setup_cli_logging(cfg.log_level, cfg.log_file)
 
     if not sys_argv:
         sys_argv = sys.argv

uaclient/cli/tests/test_cli.py

@@ -3,7 +3,6 @@
 import json
 import logging
 import socket
-import sys
 import textwrap
 
 import mock
@@ -18,7 +17,6 @@
     assert_root,
     get_parser,
     main,
-    setup_logging,
 )
 from uaclient.entitlements import get_valid_entitlement_names
 from uaclient.exceptions import (
@@ -105,7 +103,7 @@ def _get_help_output():
                     "uaclient.config.UAConfig",
                     return_value=FakeConfig(),
                 ):
-                    with mock.patch("uaclient.cli.setup_logging"):
+                    with mock.patch("uaclient.log.setup_cli_logging"):
                         main()
             out, _err = capsys.readouterr()
 
@@ -499,7 +497,7 @@ class TestMain:
     @mock.patch(M_PATH_UACONFIG + "delete_cache_key")
     @mock.patch("uaclient.cli.event.info")
     @mock.patch("uaclient.cli.LOG.exception")
-    @mock.patch("uaclient.cli.setup_logging")
+    @mock.patch("uaclient.log.setup_cli_logging")
     @mock.patch("uaclient.cli.get_parser")
     def test_errors_handled_gracefully(
         self,
@@ -545,7 +543,7 @@ def test_errors_handled_gracefully(
     )
     @mock.patch(M_PATH_UACONFIG + "delete_cache_key")
     @mock.patch("uaclient.cli.LOG.error")
-    @mock.patch("uaclient.cli.setup_logging")
+    @mock.patch("uaclient.log.setup_cli_logging")
     @mock.patch("uaclient.cli.get_parser")
     def test_interrupt_errors_handled_gracefully(
         self,
@@ -591,7 +589,7 @@ def test_interrupt_errors_handled_gracefully(
     )
     @mock.patch("uaclient.cli.event.info")
     @mock.patch("uaclient.cli.LOG.error")
-    @mock.patch("uaclient.cli.setup_logging")
+    @mock.patch("uaclient.log.setup_cli_logging")
     @mock.patch("uaclient.cli.get_parser")
     def test_user_facing_error_handled_gracefully(
         self,
@@ -633,7 +631,7 @@ def test_user_facing_error_handled_gracefully(
     )
     @mock.patch("uaclient.cli.event.info")
     @mock.patch("uaclient.cli.LOG.exception")
-    @mock.patch("uaclient.cli.setup_logging")
+    @mock.patch("uaclient.log.setup_cli_logging")
     @mock.patch("uaclient.cli.get_parser")
     def test_url_error_handled_gracefully(
         self,
@@ -663,7 +661,7 @@ def test_url_error_handled_gracefully(
         assert [expected_log_call] == m_log_exception.call_args_list
 
     @pytest.mark.parametrize("caplog_text", [logging.DEBUG], indirect=True)
-    @mock.patch("uaclient.cli.setup_logging")
+    @mock.patch("uaclient.log.setup_cli_logging")
     @mock.patch("uaclient.cli.get_parser")
     def test_command_line_is_logged(
         self, _m_get_parser, _m_setup_logging, caplog_text
@@ -675,7 +673,7 @@ def test_command_line_is_logged(
         assert "['some', 'args']" in log
 
     @pytest.mark.parametrize("caplog_text", [logging.DEBUG], indirect=True)
-    @mock.patch("uaclient.cli.setup_logging")
+    @mock.patch("uaclient.log.setup_cli_logging")
     @mock.patch("uaclient.cli.get_parser")
     @mock.patch(
         "uaclient.cli.util.get_pro_environment",
@@ -695,7 +693,7 @@ def test_environment_is_logged(
         assert "UA_ENV=YES" in log
         assert "UA_FEATURES_WOW=XYZ" in log
 
-    @mock.patch("uaclient.cli.setup_logging")
+    @mock.patch("uaclient.log.setup_cli_logging")
     @mock.patch("uaclient.cli.get_parser")
     @mock.patch("uaclient.cli.config.UAConfig")
     @pytest.mark.parametrize("config_error", [True, False])
@@ -771,7 +769,7 @@ def test_argparse_errors_well_formatted(
     @mock.patch("uaclient.cli.event.info")
     @mock.patch("uaclient.cli.action_status")
     @mock.patch("uaclient.cli.action_security_status")
-    @mock.patch("uaclient.cli.setup_logging")
+    @mock.patch("uaclient.log.setup_cli_logging")
     @mock.patch("sys.stdout.isatty")
     def test_status_human_readable_warning(
         self,
@@ -806,39 +804,6 @@ def test_status_human_readable_warning(
             assert [] == m_event_info.call_args_list
 
 
-class TestSetupLogging:
-    def test_correct_handlers_added_to_logger(
-        self,
-        FakeConfig,
-    ):
-        log_level = logging.DEBUG
-        logger = logging.getLogger("logger_a")
-
-        handler = logging.StreamHandler(sys.stderr)
-        handler.setLevel(logging.ERROR)
-        handler.set_name("ua-test-console")
-        logger.addHandler(handler)
-
-        with mock.patch(
-            "uaclient.cli.config.UAConfig", return_value=FakeConfig()
-        ):
-            setup_logging(log_level, logger=logger)
-        assert len(logger.handlers) == 1
-        assert logger.handlers[0].name == "upro-file"
-        assert logger.handlers[0].level == log_level
-
-    @mock.patch("pathlib.Path.touch")
-    def test_log_file_created_if_not_present(self, m_path_touch, tmpdir):
-        logger = logging.getLogger("logger_b")
-        log_file = tmpdir.join("log_file").strpath
-        setup_logging(
-            logging.INFO,
-            log_file=log_file,
-            logger=logger,
-        )
-        assert m_path_touch.call_args_list == [mock.call(mode=0o640)]
-
-
 class TestGetValidEntitlementNames:
     @mock.patch(
         "uaclient.cli.entitlements.valid_services",

uaclient/cli/tests/test_cli_api.py

@@ -27,7 +27,7 @@
 
 class TestActionAPI:
     @mock.patch("uaclient.cli.entitlements.valid_services", return_value=[])
-    @mock.patch("uaclient.cli.setup_logging")
+    @mock.patch("uaclient.log.setup_cli_logging")
     def test_api_help(self, _m_setup_logging, valid_services, capsys):
         with pytest.raises(SystemExit):
             with mock.patch("sys.argv", ["/usr/bin/ua", "api", "--help"]):

uaclient/cli/tests/test_cli_attach.py

@@ -669,7 +669,7 @@ def test_magic_attach_fails_if_format_json_param_used(self, FakeConfig):
 
 @mock.patch(M_PATH + "contract.get_available_resources")
 class TestParser:
-    @mock.patch("uaclient.cli.setup_logging")
+    @mock.patch("uaclient.log.setup_cli_logging")
     def test_attach_help(
         self, _m_resources, _m_setup_logging, capsys, FakeConfig
     ):

uaclient/cli/tests/test_cli_auto_attach.py

@@ -43,7 +43,7 @@ def test_non_root_users_are_rejected(we_are_currently_root, FakeConfig):
 
 
 class TestActionAutoAttach:
-    @mock.patch("uaclient.cli.setup_logging")
+    @mock.patch("uaclient.log.setup_cli_logging")
     @mock.patch(M_PATH + "contract.get_available_resources")
     def test_auto_attach_help(
         self, _m_resources, _m_setup_logging, capsys, FakeConfig

uaclient/cli/tests/test_cli_collect_logs.py

@@ -29,7 +29,7 @@
 
 
 class TestActionCollectLogs:
-    @mock.patch("uaclient.cli.setup_logging")
+    @mock.patch("uaclient.log.setup_cli_logging")
     @mock.patch(M_PATH + "contract.get_available_resources")
     def test_collect_logs_help(
         self, _m_resources, _m_setup_logging, capsys, FakeConfig

uaclient/cli/tests/test_cli_config.py

@@ -22,7 +22,7 @@
 
 
 @mock.patch("uaclient.cli.LOG.error")
-@mock.patch("uaclient.cli.setup_logging")
+@mock.patch("uaclient.log.setup_cli_logging")
 @mock.patch(M_PATH + "contract.get_available_resources")
 class TestMainConfig:
     @pytest.mark.parametrize("additional_params", ([], ["--help"]))

uaclient/cli/tests/test_cli_config_set.py

@@ -25,7 +25,7 @@
 M_LIVEPATCH = "uaclient.entitlements.livepatch."
 
 
-@mock.patch("uaclient.cli.setup_logging")
+@mock.patch("uaclient.log.setup_cli_logging")
 class TestMainConfigSet:
     @pytest.mark.parametrize(
         "kv_pair,err_msg",

uaclient/cli/tests/test_cli_config_show.py

@@ -17,7 +17,7 @@
 
 
 @mock.patch("uaclient.cli.logging.error")
-@mock.patch("uaclient.cli.setup_logging")
+@mock.patch("uaclient.log.setup_cli_logging")
 @mock.patch(M_PATH + "contract.get_available_resources")
 class TestMainConfigShow:
     def test_config_show_help(

uaclient/cli/tests/test_cli_config_unset.py

@@ -24,7 +24,7 @@
 M_LIVEPATCH = "uaclient.entitlements.livepatch."
 
 
-@mock.patch("uaclient.cli.setup_logging")
+@mock.patch("uaclient.log.setup_cli_logging")
 @mock.patch("uaclient.cli.contract.get_available_resources")
 class TestMainConfigUnSet:
     @pytest.mark.parametrize(

uaclient/cli/tests/test_cli_disable.py

@@ -55,7 +55,7 @@ def all_service_msg(FakeConfig):
 
 
 class TestDisable:
-    @mock.patch("uaclient.cli.setup_logging")
+    @mock.patch("uaclient.log.setup_cli_logging")
     @mock.patch("uaclient.cli.contract.get_available_resources")
     def test_disable_help(
         self, _m_resources, _m_setup_logging, capsys, FakeConfig

uaclient/cli/tests/test_cli_enable.py

@@ -39,7 +39,7 @@
 
 @mock.patch("uaclient.contract.refresh")
 class TestActionEnable:
-    @mock.patch("uaclient.cli.setup_logging")
+    @mock.patch("uaclient.log.setup_cli_logging")
     @mock.patch("uaclient.cli.contract.get_available_resources")
     def test_enable_help(
         self,
@@ -59,7 +59,7 @@ def test_enable_help(
         out, _err = capsys.readouterr()
         assert HELP_OUTPUT == out
 
-    @mock.patch("uaclient.cli.setup_logging")
+    @mock.patch("uaclient.log.setup_cli_logging")
     @mock.patch("uaclient.util.we_are_currently_root", return_value=False)
     @mock.patch("uaclient.cli.contract.get_available_resources")
     def test_non_root_users_are_rejected(

uaclient/cli/tests/test_cli_fix.py

@@ -33,7 +33,7 @@
 
 
 class TestActionFix:
-    @mock.patch("uaclient.cli.setup_logging")
+    @mock.patch("uaclient.log.setup_cli_logging")
     @mock.patch("uaclient.cli.contract.get_available_resources")
     def test_fix_help(
         self, _m_resources, _m_setup_logging, capsys, FakeConfig

uaclient/cli/tests/test_cli_reboot_required.py

@@ -22,7 +22,7 @@
 
 
 class TestActionRebootRequired:
-    @mock.patch("uaclient.cli.setup_logging")
+    @mock.patch("uaclient.log.setup_cli_logging")
     def test_enable_help(self, _m_setup_logging, capsys, FakeConfig):
         with pytest.raises(SystemExit):
             with mock.patch(

uaclient/cli/tests/test_cli_refresh.py

@@ -28,7 +28,7 @@
 
 
 class TestActionRefresh:
-    @mock.patch("uaclient.cli.setup_logging")
+    @mock.patch("uaclient.log.setup_cli_logging")
     @mock.patch("uaclient.cli.contract.get_available_resources")
     def test_refresh_help(
         self, _m_resources, _m_setup_logging, capsys, FakeConfig