diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index cf471e1dd..3cc8651fe 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -8,6 +8,37 @@ * Includes a major update of the numpy dependency to v2.x. +* 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 diff --git a/mkdocs.yml b/mkdocs.yml index bb59da720..ad7dba795 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -101,8 +101,8 @@ plugins: default_handler: python handlers: python: + paths: ["src"] options: - paths: ["src"] docstring_section_style: spacy inherited_members: true merge_init_into_class: false diff --git a/src/frequenz/sdk/config/__init__.py b/src/frequenz/sdk/config/__init__.py index 461bc7e8a..199cbedc1 100644 --- a/src/frequenz/sdk/config/__init__.py +++ b/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", ] diff --git a/src/frequenz/sdk/config/_logging_actor.py b/src/frequenz/sdk/config/_logging_actor.py index 739e0be07..c45a3f380 100644 --- a/src/frequenz/sdk/config/_logging_actor.py +++ b/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.") diff --git a/tests/config/test_logging_actor.py b/tests/config/test_logging_actor.py index c06aa07dc..1ee2d92ee 100644 --- a/tests/config/test_logging_actor.py +++ b/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) diff --git a/tests/config/test_manager.py b/tests/config/test_manager.py index b36154758..131e973fa 100644 --- a/tests/config/test_manager.py +++ b/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" """ )