From 68a35f8a7375dc1c261ac270bf95637417e4cbcd Mon Sep 17 00:00:00 2001
From: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date: Fri, 27 Nov 2020 17:33:50 +0100
Subject: [PATCH] Support multiple log level specifications in the swh cli

This allows users to pass multiple --log-level values to the swh cli, to allow
finer grained configuration of the logging of some modules, without having to
resort to a full-fledged, very verbose log config file.

Close T2819.
---
 swh/core/cli/__init__.py   | 70 ++++++++++++++++++++++++++++++++------
 swh/core/tests/test_cli.py | 34 ++++++++++++++++--
 2 files changed, 92 insertions(+), 12 deletions(-)

diff --git a/swh/core/cli/__init__.py b/swh/core/cli/__init__.py
index 580d07f0..c2dee465 100644
--- a/swh/core/cli/__init__.py
+++ b/swh/core/cli/__init__.py
@@ -5,6 +5,7 @@
 
 import logging
 import logging.config
+from typing import Optional
 import warnings
 
 import click
@@ -48,12 +49,43 @@ def clean_exit_on_signal(signal, frame):
     raise SystemExit(0)
 
 
+def validate_loglevel_params(ctx, param, value):
+    """Validate the --log-level parameters, with multiple values"""
+    if value is None:
+        return None
+    return [validate_loglevel(ctx, param, v) for v in value]
+
+
+def validate_loglevel(ctx, param, value):
+    """Validate a single loglevel specification, of the form LOGLEVEL or
+    module:LOGLEVEL."""
+    if ":" in value:
+        try:
+            module, log_level = value.split(":")
+        except ValueError:
+            raise click.BadParameter(
+                "Invalid log level specification `%s`, "
+                "needs to be in format `module:LOGLEVEL`" % value
+            )
+    else:
+        module = None
+        log_level = value
+
+    if log_level not in LOG_LEVEL_NAMES:
+        raise click.BadParameter(
+            "Log level %s unknown (in `%s`) needs to be one of %s"
+            % (log_level, value, ", ".join(LOG_LEVEL_NAMES))
+        )
+
+    return (module, log_level)
+
+
 @click.group(
     context_settings=CONTEXT_SETTINGS,
     cls=AliasedGroup,
     option_notes="""\
-If both options are present, --log-level will override the root logger
-configuration set in --log-config.
+If both options are present, --log-level values will override the configuration
+in --log-config.
 
 The --log-config YAML must conform to the logging.config.dictConfig schema
 documented at https://docs.python.org/3/library/logging.config.html.
@@ -62,9 +94,16 @@ documented at https://docs.python.org/3/library/logging.config.html.
 @click.option(
     "--log-level",
     "-l",
+    "log_levels",
     default=None,
-    type=click.Choice(LOG_LEVEL_NAMES),
-    help="Log level (defaults to INFO).",
+    callback=validate_loglevel_params,
+    multiple=True,
+    help=(
+        "Log level (defaults to INFO). "
+        "Can override the log level for a specific module, by using the "
+        "`specific.module:LOGLEVEL` syntax (e.g. `--log-level swh.core:DEBUG` "
+        "will enable DEBUG logging for swh.core)."
+    ),
 )
 @click.option(
     "--log-config",
@@ -82,7 +121,7 @@ documented at https://docs.python.org/3/library/logging.config.html.
     help="Enable debugging of sentry",
 )
 @click.pass_context
-def swh(ctx, log_level, log_config, sentry_dsn, sentry_debug):
+def swh(ctx, log_levels, log_config, sentry_dsn, sentry_debug):
     """Command line interface for Software Heritage.
     """
     import signal
@@ -96,18 +135,29 @@ def swh(ctx, log_level, log_config, sentry_dsn, sentry_debug):
 
     init_sentry(sentry_dsn, debug=sentry_debug)
 
-    if log_level is None and log_config is None:
-        log_level = "INFO"
+    set_default_loglevel: Optional[str] = None
 
     if log_config:
         logging.config.dictConfig(yaml.safe_load(log_config.read()))
+        set_default_loglevel = logging.root.getEffectiveLevel()
+
+    if not log_levels:
+        log_levels = []
 
-    if log_level:
+    for module, log_level in log_levels:
+        logger = logging.getLogger(module)
         log_level = logging.getLevelName(log_level)
-        logging.root.setLevel(log_level)
+        logger.setLevel(log_level)
+
+        if module is None:
+            set_default_loglevel = log_level
+
+    if not set_default_loglevel:
+        logging.root.setLevel("INFO")
+        set_default_loglevel = "INFO"
 
     ctx.ensure_object(dict)
-    ctx.obj["log_level"] = log_level
+    ctx.obj["log_level"] = set_default_loglevel
 
 
 def main():
diff --git a/swh/core/tests/test_cli.py b/swh/core/tests/test_cli.py
index ca40fbff..0eb86e41 100644
--- a/swh/core/tests/test_cli.py
+++ b/swh/core/tests/test_cli.py
@@ -242,7 +242,7 @@ def log_config_path(tmp_path):
     yield str(tmp_path / "log_config.yml")
 
 
-def test_log_config(caplog, log_config_path, swhmain):
+def test_log_config(log_config_path, swhmain):
     @swhmain.command(name="test")
     @click.pass_context
     def swhtest(ctx):
@@ -264,7 +264,7 @@ def test_log_config(caplog, log_config_path, swhmain):
     )
 
 
-def test_log_config_log_level_interaction(caplog, log_config_path, swhmain):
+def test_log_config_log_level_interaction(log_config_path, swhmain):
     @swhmain.command(name="test")
     @click.pass_context
     def swhtest(ctx):
@@ -287,6 +287,36 @@ def test_log_config_log_level_interaction(caplog, log_config_path, swhmain):
     )
 
 
+def test_multiple_log_level_behavior(swhmain):
+    @swhmain.command(name="test")
+    @click.pass_context
+    def swhtest(ctx):
+        assert logging.getLevelName(logging.root.level) == "DEBUG"
+        assert logging.getLevelName(logging.getLogger("dontshowdebug").level) == "INFO"
+        return 0
+
+    runner = CliRunner()
+    result = runner.invoke(
+        swhmain, ["--log-level", "DEBUG", "--log-level", "dontshowdebug:INFO", "test",]
+    )
+
+    assert result.exit_code == 0, result.output
+
+
+def test_invalid_log_level(swhmain):
+    runner = CliRunner()
+    result = runner.invoke(swhmain, ["--log-level", "broken:broken:DEBUG"])
+
+    assert result.exit_code != 0
+    assert "Invalid log level specification" in result.output
+
+    runner = CliRunner()
+    result = runner.invoke(swhmain, ["--log-level", "UNKNOWN"])
+
+    assert result.exit_code != 0
+    assert "Log level UNKNOWN unknown" in result.output
+
+
 def test_aliased_command(swhmain):
     @swhmain.command(name="canonical-test")
     @click.pass_context
-- 
GitLab