configuration: add logging section & ad log_level to configuration_opts (#50)

olaf-mandel · cmeissner · web-flow · commit aa9483f2294c · 2024-10-06T21:14:22.000+02:00
* configuration: add logging section

Allow to configure both the logging-levels and targets of various
loggers by adding a new section `logging` to the configuration file.
Each entry in that section configures one logger / channel. An example
config is:

```yaml
logging:
  - name root
    level: INFO
  - name: foomodule.barcollector:
    level: WARNING
    target: /path/to/my/collector/logfile.log
```

* configuration: add log_level to configuration_opts

Add `CollectorBase.setLoggers(logger_names)` as a means for collectors
derived from `CollectorBase` to easily configure logging-levels. The new
method uses the `collector_opts.logging_level` value (if set in the
configuration) to set the logging-level of any loggers given to it.

---------

Co-authored-by: Christian Meißner &lt;cme+github@meissner.sh&gt;
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,8 @@ This project adheres to [Semantic Versioning](http://semver.org/) and [Keep a Ch
 
 ### New
 * Fix \#47 - add environment variables for fundamental parameters
+* configuration: add logging section
+* configuration: add log_level to configuration_opts
 
 ### Changes
 
diff --git a/README.md b/README.md
@@ -109,12 +109,29 @@ collectors:
   - netdev
 collector_opts:
   netdev:
+    log_level: DEBUG
     whitelist:
     blacklist:
       - docker0
       - lo
+logging:
+  - name: root
+    level: INFO
+  - name: foomodule.barcollector
+    level: WARNING
+    target: /path/to/my/collector/logfile.log
 ```
 
+The `collector_opts` can optionally contain a `log_level` entry which
+will configure the logging-level for that specific collector. Note that
+support for this must be implemented by each individual collector.
+
+Logging can optionally be configured for any logger. The entries must
+specify the name of the logger and can optionally specify a
+logging-level (default: stay at whatever the default logging-level for
+that logger is) and/or can specify a file to write the log to (default:
+log to stderr).
+
 ### Start-up Configuration
 
 You can define two fundamental Parameters on program start-up. The following table summarized you options:
diff --git a/p3.yml b/p3.yml
@@ -9,3 +9,6 @@ collector_opts:
     blacklist:
       - docker0
       - lo
+logging:
+  - name: root
+    level: INFO
diff --git a/p3exporter/__init__.py b/p3exporter/__init__.py
@@ -13,6 +13,35 @@
 from p3exporter.web import create_app
 
 
+def setup_logging(cfg: dict):
+    """Set up logging as configured.
+
+    The configuration may optionally contain an entry `logging`,
+    if it does not or if that entry is not an array then does nothing.
+    Each array element must be a dict that contains at least a key
+    `name` that refers to the logger to configure. It may also contain
+    the optional keys `level` and `target` that configure the
+    logging-level and a file-target, respectively if present.
+
+    :param cfg: Configuration as read from config-file.
+    :type cfg: dict
+    """
+    if not isinstance(cfg.get('logging'), list):
+        return
+    for c in cfg['logging']:
+        if not isinstance(c, dict):
+            return
+        if not isinstance(c.get('name'), str):
+            return
+        logger = logging.getLogger(c["name"])
+        level = c.get('level')
+        if level is not None:
+            logger.setLevel(level)
+        target = c.get('target')
+        if target is not None:
+            logger.addHandler(logging.FileHandler(target))
+
+
 def shutdown():
     """Shutdown the app in a clean way."""
     logging.info('Shutting down, see you next time!')
@@ -42,6 +71,7 @@ def main():
     with open(args.config, 'r') as config_file:
         cfg = yaml.load(config_file, Loader=yaml.SafeLoader)
     collector_config = CollectorConfig(**cfg)
+    setup_logging(cfg)
 
     Collector(collector_config)
 
diff --git a/p3exporter/collector/__init__.py b/p3exporter/collector/__init__.py
@@ -58,6 +58,25 @@ def collector_name_from_class(self):
 
         return '_'.join(class_name_parts)
 
+    def setLoggers(self, logger_names: list | str):
+        """Configure the provided logger(s) according to the CollectorConfig.
+
+        It is recommended to call this method from the constructor of any
+        deriving class. Either bump the required p3exporter version or check
+        dynamically if the method is supported.
+
+        :param logger_names: Name or names of loggers to configure.
+        :type logger_names: list or str
+        """
+        if not isinstance(logger_names, list):
+            logger_names = [logger_names]
+        if "log_level" not in self.opts:
+            return
+        level = self.opts["log_level"]
+        for name in logger_names:
+            logger = logging.getLogger(name)
+            logger.setLevel(level)
+
 
 class Collector(object):
     """Base class to load collectors.
diff --git a/tests/test_cases/logging_config.py b/tests/test_cases/logging_config.py
@@ -0,0 +1,157 @@
+from p3exporter import setup_logging
+from p3exporter.collector import CollectorBase, CollectorConfig
+import logging
+import os.path
+import pytest
+
+
+loggers = ["", "foo", "bar"]
+files = ["file1.log", "file2.log"]
+
+
+def setup_function(fn):
+    """Start with a clean slate of default logging-levels and no handlers."""
+    for name in loggers:
+        logger = logging.getLogger(name)
+        level = logging.WARNING if name == "" else logging.NOTSET
+        logger.setLevel(level)
+        for handler in logger.handlers:
+            logger.removeHandler(handler)
+
+
+def teardown_function(fn):
+    """Remove any files we may have created."""
+    for file in files:
+        if os.path.exists(file):
+            os.remove(file)
+
+
+data_logging_levels = [
+    pytest.param(None,
+                 [logging.WARNING, logging.NOTSET, logging.NOTSET],
+                 [None, None, None],
+                 id="no logging-section at all"),
+    pytest.param("Not an array",
+                 [logging.WARNING, logging.NOTSET, logging.NOTSET],
+                 [None, None, None],
+                 id="logging-section has wrong type"),
+    pytest.param([{"level": "INFO"},
+                  {"target": "file1.log"},
+                  {"level": "DEBUG", "target": "file2.log"}],
+                 [logging.WARNING, logging.NOTSET, logging.NOTSET],
+                 [None, None, None],
+                 id="no names in otherwise valid entries"),
+    pytest.param([{"name": "", "level": "INFO"},
+                  {"name": "foo", "level": "DEBUG"}],
+                 [logging.INFO, logging.DEBUG, logging.NOTSET],
+                 [None, None, None],
+                 id="levels only, using empty-string for root"),
+    pytest.param([{"name": "root", "level": "ERROR"},
+                  {"name": "bar", "level": "CRITICAL"}],
+                 [logging.ERROR, logging.NOTSET, logging.CRITICAL],
+                 [None, None, None],
+                 id="levels only, using name of root"),
+    pytest.param([{"name": "foo", "level": 10},
+                  {"name": "bar", "level": 20}],
+                 [logging.WARNING, logging.DEBUG, logging.INFO],
+                 [None, None, None],
+                 id="levels only, using integers for levels"),
+    pytest.param([{"name": "root", "target": "file1.log"},
+                  {"name": "foo", "target": "file2.log"}],
+                 [logging.WARNING, logging.NOTSET, logging.NOTSET],
+                 ["file1.log", "file2.log", None],
+                 id="targets only"),
+    pytest.param([{"name": "foo", "level": "INFO", "target": "file1.log"}],
+                 [logging.WARNING, logging.INFO, logging.NOTSET],
+                 [None, "file1.log", None],
+                 id="both level and target"),
+    ]
+
+
+@pytest.mark.parametrize("cfg_logging,levels,targets", data_logging_levels)
+def test_logging_levels(cfg_logging, levels, targets):
+    # pytest adds lots of extra handlers, so remember the starting state
+    orig_handlers = []
+    for name in loggers:
+        logger = logging.getLogger(name)
+        orig_handlers.append(logger.handlers.copy())
+
+    # GIVEN an input config-dictionary
+    cfg = {
+        "exporter_name": "Test only",
+        "collectors": [],
+        "collector_opts": {},
+    }
+    if cfg_logging is not None:
+        cfg["logging"] = cfg_logging
+
+    # WHEN calling setup_logging()
+    setup_logging(cfg)
+
+    # THEN the logging-levels should get changed to the expected
+    for i, name in enumerate(loggers):
+        logger = logging.getLogger(name)
+        assert logger.level == levels[i]
+
+    # AND the expected file-handlers should get added
+    for i, name in enumerate(loggers):
+        logger = logging.getLogger(name)
+        added_handlers = [h for h in logger.handlers
+                          if h not in orig_handlers[i]]
+        if targets[i] is None:
+            assert len(added_handlers) == 0
+        else:
+            assert len(added_handlers) == 1
+            handler = added_handlers[0]
+            assert isinstance(handler, logging.FileHandler)
+            assert handler.baseFilename == os.path.abspath(targets[i])
+
+
+class FooCollector(CollectorBase):
+    pass
+
+
+data_collectorbase_setloggers = [
+    pytest.param(None,
+                 ["foo", "bar"],
+                 [logging.WARNING, logging.NOTSET, logging.NOTSET],
+                 id="no log_level setting"),
+    pytest.param("CRITICAL",
+                 "foo",
+                 [logging.WARNING, logging.CRITICAL, logging.NOTSET],
+                 id="single logger-name"),
+    pytest.param("ERROR",
+                 ["foo", "bar"],
+                 [logging.WARNING, logging.ERROR, logging.ERROR],
+                 id="list of loggers"),
+    pytest.param(20,
+                 ["", "foo"],
+                 [logging.INFO, logging.INFO, logging.NOTSET],
+                 id="numeric log_level"),
+    ]
+
+
+@pytest.mark.parametrize("cfg_log_level,logger_names,expected",
+                         data_collectorbase_setloggers)
+def test_collectorbase_setloggers(cfg_log_level, logger_names, expected):
+    # GIVEN an input config-dictionary
+    cfg = {
+        "exporter_name": "Test only",
+        "collectors": ["foo"],
+        "collector_opts": {
+            "foo": {}
+        },
+    }
+    if cfg_log_level is not None:
+        cfg["collector_opts"]["foo"]["log_level"] = cfg_log_level
+
+    # AND a collector-base using this config
+    collector = FooCollector(CollectorConfig(**cfg))
+
+    # WHEN the setLoggers() method is called
+    collector.setLoggers(logger_names)
+
+    # THEN the logging-levels should get changed to the expected
+    for i, name in enumerate(loggers):
+        logger = logging.getLogger(name)
+        assert logger.level == expected[i]
