Update the logging config actor configuration format

The logging configuration was changed to make it compatible with the
configuration generated by the UI, which doesn't quote sub-key properly.

To do this, the logger name needs to be specified explicitly now (the
configuration key was used as the name before).

Signed-off-by: Leandro Lucarella <[email protected]>

Author: llucax

RELEASE_NOTES.md

@@ -6,7 +6,36 @@
 
 ## Upgrading
 
-<!-- Here goes notes on how to upgrade from previous versions, including deprecations and what they should be replaced with -->
+* The logging configuration was changed, and now the logger name needs to be specified explicitly (the configuration key was used as the name before). This is to be compatible with configuration generated by the UI, which doesn't quote sub-key properly.
+
+    - Before:
+
+      ```toml
+      [logging.loggers."frequenz.sdk.config"]
+      level = "DEBUG"
+      ```
+
+   - Now:
+
+      ```toml
+      [logging.loggers.frequenz_config]
+      name = "frequenz.sdk.config"
+      level = "DEBUG"
+      ```
+
+* The logging configuration now uses a separate class for the root logger configuration: `RootLoggerConfig`.
+
+    - Before:
+
+      ```py
+      LoggingConfig(root_logger=LoggerConfig(level="ERROR"))
+      ```
+
+    - Now:
+
+      ```py
+      LoggingConfig(root_logger=RootLoggerConfig(level="ERROR"))
+      ```
 
 ## New Features
 

src/frequenz/sdk/config/__init__.py

@@ -424,7 +424,12 @@ async def run(self) -> None:
 """
 
 from ._base_schema import BaseConfigSchema
-from ._logging_actor import LoggerConfig, LoggingConfig, LoggingConfigUpdatingActor
+from ._logging_actor import (
+    LoggerConfig,
+    LoggingConfig,
+    LoggingConfigUpdatingActor,
+    RootLoggerConfig,
+)
 from ._manager import ConfigManager, InvalidValueForKeyError, wait_for_first
 from ._managing_actor import ConfigManagingActor
 from ._util import load_config
@@ -437,6 +442,7 @@ async def run(self) -> None:
     "LoggerConfig",
     "LoggingConfig",
     "LoggingConfigUpdatingActor",
+    "RootLoggerConfig",
     "load_config",
     "wait_for_first",
 ]

src/frequenz/sdk/config/_logging_actor.py

@@ -25,8 +25,8 @@
 
 
 @dataclass(frozen=True, kw_only=True)
-class LoggerConfig:
-    """A configuration for a logger."""
+class RootLoggerConfig:
+    """A configuration for the root logger."""
 
     level: LogLevel = field(
         default="NOTSET",
@@ -36,15 +36,29 @@ class LoggerConfig:
             },
         },
     )
-    """The log level for the logger."""
+    """The log level for the root logger."""
+
+
+@dataclass(frozen=True, kw_only=True)
+class LoggerConfig(RootLoggerConfig):
+    """A configuration for a logger."""
+
+    name: str = field(
+        metadata={
+            "metadata": {
+                "description": "The name of the logger that will be affected by this "
+                "configuration."
+            },
+        },
+    )
 
 
 @dataclass(frozen=True, kw_only=True)
 class LoggingConfig:
     """A configuration for the logging system."""
 
-    root_logger: LoggerConfig = field(
-        default_factory=lambda: LoggerConfig(level="INFO"),
+    root_logger: RootLoggerConfig = field(
+        default_factory=lambda: RootLoggerConfig(level="INFO"),
         metadata={
             "metadata": {
                 "description": "Default default configuration for all loggers.",
@@ -73,10 +87,12 @@ class LoggingConfigUpdatingActor(Actor):
         [logging.root_logger]
         level = "INFO"
 
-        [logging.loggers."frequenz.sdk.actor.power_distributing"]
+        [logging.loggers.power_dist]
+        name = "frequenz.sdk.actor.power_distributing"
         level = "DEBUG"
 
-        [logging.loggers."frequenz.channels"]
+        [logging.loggers.chan]
+        name = "frequenz.channels"
         level = "DEBUG"
         ```
 
@@ -184,10 +200,12 @@ def _reconfigure(self, config_update: LoggingConfig | Exception | None) -> None:
     def _update_logging(self, config: LoggingConfig) -> None:
         """Configure the logging level."""
         # If the logger is not in the new config, set it to NOTSET
-        loggers_to_unset = self._current_config.loggers.keys() - config.loggers.keys()
-        for logger_id in loggers_to_unset:
-            _logger.debug("Unsetting log level for logger '%s'", logger_id)
-            logging.getLogger(logger_id).setLevel(logging.NOTSET)
+        old_names = {old.name for old in self._current_config.loggers.values()}
+        new_names = {new.name for new in config.loggers.values()}
+        loggers_to_unset = old_names - new_names
+        for logger_name in loggers_to_unset:
+            _logger.debug("Unsetting log level for logger '%s'", logger_name)
+            logging.getLogger(logger_name).setLevel(logging.NOTSET)
 
         self._current_config = config
         _logger.info(
@@ -196,12 +214,12 @@ def _update_logging(self, config: LoggingConfig) -> None:
         logging.getLogger().setLevel(self._current_config.root_logger.level)
 
         # For each logger in the new config, set the log level
-        for logger_id, logger_config in self._current_config.loggers.items():
+        for logger_config in self._current_config.loggers.values():
             _logger.info(
                 "Setting log level for logger '%s' to '%s'",
-                logger_id,
+                logger_config.name,
                 logger_config.level,
             )
-            logging.getLogger(logger_id).setLevel(logger_config.level)
+            logging.getLogger(logger_config.name).setLevel(logger_config.level)
 
         _logger.info("Logging config update completed.")

tests/config/test_logging_actor.py

@@ -16,6 +16,7 @@
     LoggerConfig,
     LoggingConfig,
     LoggingConfigUpdatingActor,
+    RootLoggerConfig,
     load_config,
 )
 
@@ -25,15 +26,15 @@ def test_logging_config() -> None:
     config_raw = {
         "root_logger": {"level": "DEBUG"},
         "loggers": {
-            "frequenz.sdk.actor": {"level": "INFO"},
-            "frequenz.sdk.timeseries": {"level": "ERROR"},
+            "actor": {"name": "frequenz.sdk.actor", "level": "INFO"},
+            "timeseries": {"name": "frequenz.sdk.timeseries", "level": "ERROR"},
         },
     }
     config = LoggingConfig(
-        root_logger=LoggerConfig(level="DEBUG"),
+        root_logger=RootLoggerConfig(level="DEBUG"),
         loggers={
-            "frequenz.sdk.actor": LoggerConfig(level="INFO"),
-            "frequenz.sdk.timeseries": LoggerConfig(level="ERROR"),
+            "actor": LoggerConfig(name="frequenz.sdk.actor", level="INFO"),
+            "timeseries": LoggerConfig(name="frequenz.sdk.timeseries", level="ERROR"),
         },
     )
 
@@ -92,10 +93,12 @@ async def test_logging_config_updating_actor(
 
         # Send first config
         expected_config = LoggingConfig(
-            root_logger=LoggerConfig(level="ERROR"),
+            root_logger=RootLoggerConfig(level="ERROR"),
             loggers={
-                "frequenz.sdk.actor": LoggerConfig(level="DEBUG"),
-                "frequenz.sdk.timeseries": LoggerConfig(level="ERROR"),
+                "actor": LoggerConfig(name="frequenz.sdk.actor", level="DEBUG"),
+                "timeseries": LoggerConfig(
+                    name="frequenz.sdk.timeseries", level="ERROR"
+                ),
             },
         )
         await mock_config_manager.config_channel.new_sender().send(expected_config)
@@ -121,9 +124,9 @@ async def test_logging_config_updating_actor(
 
         # Update config
         expected_config = LoggingConfig(
-            root_logger=LoggerConfig(level="WARNING"),
+            root_logger=RootLoggerConfig(level="WARNING"),
             loggers={
-                "frequenz.sdk.actor": LoggerConfig(level="INFO"),
+                "actor": LoggerConfig(name="frequenz.sdk.actor", level="INFO"),
             },
         )
         await mock_config_manager.config_channel.new_sender().send(expected_config)

tests/config/test_manager.py

@@ -250,6 +250,7 @@ def config_file(self, tmp_path: pathlib.Path) -> pathlib.Path:
             value = 42
 
             [logging.loggers.test]
+            name = "test"
             level = "DEBUG"
             """
         )
@@ -276,6 +277,7 @@ async def test_full_config_flow(self, config_file: pathlib.Path) -> None:
                 value = 43
 
                 [logging.loggers.test]
+                name = "test"
                 level = "INFO"
                 """
             )
@@ -313,6 +315,7 @@ async def test_full_config_flow_without_logging(
                 value = 43
 
                 [logging.loggers.test]
+                name = "test"
                 level = "DEBUG"
                 """
             )