Merge pull request #5 from desultory/dev

improve/standardize the cached exporter, add tests

Author: desultory

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "prometheus_exporter"
-version = "1.1.0"
+version = "1.2.0"
 authors = [
   { name="Desultory", email="[email protected]" },
 ]

src/prometheus_exporter/cached_exporter.py

@@ -1,5 +1,16 @@
+from time import time
+
 from .exporter import Exporter
 
+def is_positive_number(func):
+    def wrapper(self, value):
+        if not isinstance(value, int) and not isinstance(value, float):
+            raise TypeError("%s must be an integer or float", func.__name__)
+        if value < 0:
+            raise ValueError("%s must be a positive number", func.__name__)
+        return func(self, value)
+    return wrapper
+
 
 def cached_exporter(cls):
     if not isinstance(cls, Exporter) and not issubclass(cls, Exporter):
@@ -13,54 +24,59 @@ class CachedExporter(cls):
         """
 
         def __init__(self, *args, **kwargs):
+            """Call the super which reads the config.
+            Prefer cache life setting from kwargs, then config, then default to 60 seconds.
+            """
             super().__init__(*args, **kwargs)
-            if cache_life := kwargs.pop("cache_life", None):
-                self.cache_life = cache_life
-            elif not hasattr(self, "cache_life"):
-                self.cache_life = 60
+            self.cache_life = kwargs.pop("cache_life", self.config.get("cache_life", 60))
+            self.logger.info("Cache life set to: %d seconds", self.cache_life)
+
+        @property
+        def cache_life(self) -> int:
+            return getattr(self, "_cache_life", 60)
+
+        @cache_life.setter
+        @is_positive_number
+        def cache_life(self, value) -> None:
+            self.logger.info("Setting cache_life to: %ds", value)
+            self._cache_life = value
+
+        @property
+        def cache_time(self) -> int:
+            return getattr(self, "_cache_time", 0)
 
-        def __setattr__(self, name, value):
-            """Override setattr for cache_life"""
-            if name == "cache_life":
-                if not isinstance(value, int):
-                    raise TypeError("cache_life must be an integer")
-                if value < 0:
-                    raise ValueError("cache_life must be a positive integer")
-                self.logger.info("Setting cache_life to: %ds", value)
-            super().__setattr__(name, value)
+        @cache_time.setter
+        @is_positive_number
+        def cache_time(self, value) -> None:
+            self.logger.info("Setting cache_time to: %d", value)
+            self._cache_time = value
 
-        def read_config(self):
-            """Override read_config to add cache_life"""
-            super().read_config()
-            if hasattr(self, "cache_life"):
-                self.logger.debug("Cache life already set to: %ds", self.cache_life)
-                return
-            self.cache_life = self.config.get("cache_life", 60)
-            self.logger.info("Set cache_life to: %d seconds", self.cache_life)
+        @property
+        def cache_age(self) -> int:
+            """ Returns the age of the cache """
+            cache_age = time() - getattr(self, "_cache_time", 0)
+            self.logger.debug("[%s] Cache age: %d" % (self.name, cache_age))
+            return time() - getattr(self, "_cache_time", 0)
 
-        async def get_metrics(self, label_filter={}):
-            """Get metrics from the exporter, caching the result."""
+        async def get_metrics(self, label_filter={}) -> list:
+            """Get metrics from the exporter, respecting label filters and caching the result."""
             for key, value in label_filter.items():
                 if key not in self.labels and self.labels[key] != value:
                     self.logger.debug("Label filter check failed: %s != %s", self.labels, label_filter)
-                    return
-            from time import time
+                    return []
 
-            cache_time = time() - getattr(self, "_cache_time", 0)
-            name = getattr(self, "name", self.__class__.__name__)
-            self.logger.debug("[%s] Cache time: %d" % (name, cache_time))
-            if not hasattr(self, "_cached_metrics") or cache_time >= self.cache_life:
-                self.metrics = []
+            if not hasattr(self, "_cached_metrics") or self.cache_age >= self.cache_life:
                 if new_metrics := await super().get_metrics(label_filter=label_filter):
                     self.metrics = new_metrics
                     self._cached_metrics = new_metrics
-                    self._cache_time = time()
+                    self.cache_time = time()
                 elif hasattr(self, "_cached_metrics"):
-                    self.logger.warning("[%s] Exporter returned no metrics, returning cached metrics" % name)
+                    self.logger.warning("[%s] Exporter returned no metrics, returning cached metrics" % self.name)
                     self.metrics = self._cached_metrics
             else:
-                self.logger.log(5, "[%s] Returning cached metrics: %s" % (name, self._cached_metrics))
+                self.logger.log(5, "[%s] Returning cached metrics: %s" % (self.name, self._cached_metrics))
                 self.metrics = self._cached_metrics
+            return self.metrics.copy()
 
     CachedExporter.__name__ = f"Cached{cls.__name__}"
     CachedExporter.__module__ = cls.__module__

src/prometheus_exporter/exporter.py

@@ -8,7 +8,9 @@
 from asyncio import all_tasks, ensure_future
 from pathlib import Path
 from signal import SIGHUP, SIGINT, signal
+from tomllib import load
 
+from aiohttp import web
 from aiohttp.web import Application, Response, get
 from zenlib.logging import ClassLogger
 
@@ -28,8 +30,10 @@ class Exporter(ClassLogger):
     Labels can be supplied as a dict as an argument, and in the config file.
     """
 
-    def __init__(self, config_file="config.toml", labels=Labels(), no_config_file=False, *args, **kwargs):
+    def __init__(self, config_file="config.toml", name=None, labels=Labels(), no_config_file=False, *args, **kwargs):
         super().__init__(*args, **kwargs)
+        if name is not None:
+            self.name = name
         self.labels = Labels(dict_items=labels, logger=self.logger)
         self.config_file = Path(config_file)
         if not no_config_file:
@@ -45,15 +49,24 @@ def __init__(self, config_file="config.toml", labels=Labels(), no_config_file=Fa
         self.app.add_routes([get("/metrics", self.handle_metrics)])
         self.app.on_shutdown.append(self.on_shutdown)
 
+    @property
+    def name(self):
+        return getattr(self, "_name", self.__class__.__name__)
+
+    @name.setter
+    def name(self, value):
+        if getattr(self, "_name", None) is not None:
+            return self.logger.warning("[%s] Name already set, ignoring new name: %s", self.name, value)
+        assert isinstance(value, str), "Name must be a string, not: %s" % type(value)
+        self._name = value
+
     def __setattr__(self, name, value):
         if name == "labels":
             assert isinstance(value, Labels), "Labels must be a 'Labels' object."
         super().__setattr__(name, value)
 
     def read_config(self):
         """Reads the config file defined in self.config_file"""
-        from tomllib import load
-
         with open(self.config_file, "rb") as config:
             self.config = load(config)
 
@@ -62,8 +75,6 @@ def read_config(self):
 
     def start(self):
         """Starts the exporter server."""
-        from aiohttp import web
-
         self.logger.info("Exporter server address: %s:%d" % (self.listen_ip, self.listen_port))
         web.run_app(self.app, host=self.listen_ip, port=self.listen_port)
 
@@ -77,13 +88,13 @@ async def on_shutdown(self, app):
             task.cancel()
 
     def get_labels(self):
-        """ Returns a copy of the labels dict.
+        """Returns a copy of the labels dict.
         This is designed to be extended, and the lables object may be modified by the caller.
         """
         return self.labels.copy()
 
     async def get_metrics(self, *args, **kwargs) -> list:
-        """ Returns a copy of the metrics list.
+        """Returns a copy of the metrics list.
         This is designed to be extended in subclasses to get metrics from other sources.
         Clears the metric list before getting metrics, as layers may add metrics to the list.
         """

tests/test_exporter.py

@@ -1,11 +1,17 @@
 from asyncio import run
-from unittest import TestCase, expectedFailure, main
+from unittest import TestCase, main
 from uuid import uuid4
 
 from aiohttp.test_utils import AioHTTPTestCase
-from prometheus_exporter import Exporter
+from prometheus_exporter import Exporter, cached_exporter
 from zenlib.logging import loggify
 
+@cached_exporter
+class TestCachedExporter(Exporter):
+    async def get_metrics(self, *args, **kwargs) -> dict:
+        metrics = await super().get_metrics(*args, **kwargs)
+        print("Getting metrics:", metrics)
+        return metrics
 
 def generate_random_metric_config(count: int) -> dict:
     """Generate a random metric configuration"""
@@ -17,9 +23,9 @@ def generate_random_metric_config(count: int) -> dict:
 
 
 class TestExporter(TestCase):
-    @expectedFailure
     def test_no_config(self):
-        Exporter(config_file=str(uuid4()))  # Pass a random string as config
+        with self.assertRaises(FileNotFoundError):
+            Exporter(config_file=str(uuid4()))  # Pass a random string as config
 
     def test_proper_no_config(self):
         e = Exporter(no_config_file=True)
@@ -36,6 +42,17 @@ def test_random_metrics(self):
         for metric in random_metrics:
             self.assertIn(f"{metric} 0", export1)
 
+    def test_cached_exporter(self):
+        e = TestCachedExporter(no_config_file=True)
+        e.config["metrics"] = generate_random_metric_config(100)
+        export1 = run(e.export())
+        e.config["metrics"] = generate_random_metric_config(100)
+        export2 = run(e.export())
+        self.assertEqual(export1, export2)
+        e.cache_time = 0
+        export3 = run(e.export())
+        self.assertNotEqual(export1, export3)
+
     def test_global_labels(self):
         """Ensures that lables which are defined globally are applied to all metrics"""
         e = Exporter(labels={"global_label": "global_value"}, no_config_file=True)