PB-381: make the log level configurable (#243)

Summary:

Add a new `[logging]` section to the configuration file. For now, this
section only provides a `level` key which can be one of the log levels
supported by default by the Python standard library: notset, debug,
info, warning, error, critical.

In the future, this `[logging]` section could be used to provide more
knobs for logging customatization: enabling colors for the terminal
handler, specifying a path for logging into a file, etc.

References:

https://xainag.atlassian.net/browse/PB-381

configs/example-config.toml

@@ -34,3 +34,8 @@ bucket = "xain-fl-aggregated-weights"
 access_key_id = "minio"
 # AWS secret access to use to authenticate to the storage service
 secret_access_key = "minio123"
+
+[logging]
+# The log level can be one of "notset", "debug", "info", "warning",
+# "error" and "critical". It defaults to "info".
+level = "debug"

configs/xain-fl.toml

@@ -36,3 +36,8 @@ bucket = "xain-fl-aggregated-weights"
 access_key_id = "minio"
 # AWS secret access to use to authenticate to the storage service
 secret_access_key = "minio123"
+
+[logging]
+# The log level can be one of "notset", "debug", "info", "warning",
+# "error" and "critical". It defaults to "info".
+level = "info"

tests/test_config.py

@@ -53,17 +53,54 @@ def storage_sample():
 
 
 @pytest.fixture
-def config_sample(server_sample, ai_sample, storage_sample):
+def logging_sample():
+    """
+    Return a valid "ai" section
+    """
+    return {
+        "level": "debug",
+    }
+
+
+@pytest.fixture
+def config_sample(server_sample, ai_sample, storage_sample, logging_sample):
     """
     Return a valid config
     """
     return {
         "ai": ai_sample,
         "server": server_sample,
         "storage": storage_sample,
+        "logging": logging_sample,
     }
 
 
+def test_default_logging_config(config_sample):
+    """Check that the config loads if the [logging] section is not
+    specified
+
+    """
+    del config_sample["logging"]
+    config = Config.from_unchecked_dict(config_sample)
+    assert config.logging.level == "info"
+
+    config_sample["logging"] = {}
+    config = Config.from_unchecked_dict(config_sample)
+    assert config.logging.level == "info"
+
+
+def test_invalid_logging_config(config_sample):
+    """Various negative cases for the [logging] section"""
+    config_sample["logging"] = {"level": "invalid"}
+
+    with AssertInvalid() as err:
+        Config.from_unchecked_dict(config_sample)
+
+    err.check_other(
+        "`logging.level`: value must be one of: notset, debug, info, warning, error, critical"
+    )
+
+
 def test_load_valid_config(config_sample):
     """
     Check that a valid config is loaded correctly

xain_fl/__main__.py

@@ -6,7 +6,7 @@
 from xain_fl.config import Config, InvalidConfig, get_cmd_parameters
 from xain_fl.coordinator.coordinator import Coordinator
 from xain_fl.coordinator.store import S3Store
-from xain_fl.logger import StructLogger, get_logger, initialize_logging
+from xain_fl.logger import StructLogger, get_logger, initialize_logging, set_log_level
 from xain_fl.serve import serve
 
 logger: StructLogger = get_logger(__name__)
@@ -24,6 +24,8 @@ def main():
         logger.error("Invalid config", error=str(err))
         sys.exit(1)
 
+    set_log_level(config.logging.level.upper())
+
     coordinator = Coordinator(
         num_rounds=config.ai.rounds,
         epochs=config.ai.epochs,

xain_fl/config/__init__.py

@@ -3,12 +3,20 @@
 file."""
 
 from xain_fl.config.cli import get_cmd_parameters
-from xain_fl.config.schema import AiConfig, Config, InvalidConfig, ServerConfig, StorageConfig
+from xain_fl.config.schema import (
+    AiConfig,
+    Config,
+    InvalidConfig,
+    LoggingConfig,
+    ServerConfig,
+    StorageConfig,
+)
 
 __all__ = [
     "get_cmd_parameters",
     "Config",
     "AiConfig",
+    "LoggingConfig",
     "StorageConfig",
     "ServerConfig",
     "InvalidConfig",

xain_fl/config/schema.py

@@ -229,11 +229,28 @@ def validate_path(path):
     }
 )
 
+
+def log_level(key: str) -> Schema:
+    """Return a validator for logging levels
+
+    Args:
+
+        key: key of the configuration item
+    """
+    log_levels = ["notset", "debug", "info", "warning", "error", "critical"]
+    error_message = "one of: " + ", ".join(log_levels)
+    log_level_validator = lambda value: value in log_levels
+    return And(str, log_level_validator, error=error(key, error_message))
+
+
+LOGGING_SCHEMA = Schema({Optional("level", default="info"): log_level("logging.level"),})
+
 CONFIG_SCHEMA = Schema(
     {
         Optional("server", default=SERVER_SCHEMA.validate({})): SERVER_SCHEMA,
         "ai": AI_SCHEMA,
         "storage": STORAGE_SCHEMA,
+        Optional("logging", default=LOGGING_SCHEMA.validate({})): LOGGING_SCHEMA,
     }
 )
 
@@ -268,6 +285,9 @@ def create_class_from_schema(class_name: str, schema: Schema) -> Any:
 StorageConfig = create_class_from_schema("StorageConfig", STORAGE_SCHEMA)
 StorageConfig.__doc__ = "Storage related configuration: storage endpoints and credentials, etc."
 
+LoggingConfig = create_class_from_schema("LoggingConfig", LOGGING_SCHEMA)
+LoggingConfig.__doc__ = "Logging related configuration: log level, colors, etc."
+
 T = TypeVar("T", bound="Config")
 
 
@@ -362,10 +382,13 @@ class Config:
        assert config.storage.access_key_id == "my-key"
     """
 
-    def __init__(self, ai: NamedTuple, storage: NamedTuple, server: NamedTuple):
+    def __init__(
+        self, ai: NamedTuple, storage: NamedTuple, server: NamedTuple, logging: NamedTuple
+    ):
         self.ai = ai
         self.storage = storage
         self.server = server
+        self.logging = logging
 
     @classmethod
     def from_unchecked_dict(cls: Type[T], dictionary: Mapping[str, Any]) -> T:
@@ -396,6 +419,7 @@ def from_valid_dict(cls: Type[T], dictionary: Mapping[str, Any]) -> T:
             AiConfig(**dictionary["ai"]),
             StorageConfig(**dictionary["storage"]),
             ServerConfig(**dictionary["server"]),
+            LoggingConfig(**dictionary["logging"]),
         )
 
     @classmethod

xain_fl/logger.py

@@ -1,7 +1,7 @@
 """XAIN FL structured logger"""
 
 import logging
-from typing import Any
+from typing import Any, Optional, Union
 
 import structlog
 
@@ -41,6 +41,20 @@ def configure_structlog():
     )
 
 
+def set_log_level(level: Union[str, int]):
+    """Set the log level on the root logger. Since by default, the root
+    logger log level is inherited by all the loggers, this is like
+    setting a default log level.
+
+    Args:
+
+        level: the log level, as documented in the `Python standard
+            library <https://docs.python.org/3/library/logging.html#levels>`_
+    """
+    root_logger = logging.getLogger()
+    root_logger.setLevel(level)
+
+
 def initialize_logging():
     """Set up logging
 
@@ -50,12 +64,14 @@ def initialize_logging():
 
 
 def get_logger(
-    name: str, level: int = logging.INFO
+    name: str, level: Optional[int] = None
 ) -> structlog._config.BoundLoggerLazyProxy:  # pylint: disable=protected-access
     """Wrap python logger with default configuration of structlog.
+
     Args:
-        name (str): Identification name. For module name pass name=__name__.
-        level (int): Threshold for this logger. Defaults to logging.INFO.
+        name: Identification name. For module name pass ``name=__name__``.
+        level: Threshold for this logger.
+
     Returns:
         Wrapped python logger with default configuration of structlog.
     """
@@ -66,6 +82,8 @@ def get_logger(
     handler.setFormatter(formatter)
     logger = logging.getLogger(name)
     logger.addHandler(handler)
-    logger.setLevel(level)
+
+    if level is not None:
+        logger.setLevel(level)
 
     return structlog.wrap_logger(logger)