Repository: esanchezm/prometheus-qbittorrent-exporter

Issue:
Exporter generates metrics in an invalid format
This exporter currently generates a HELP and TYPE line for each metric + status and category label like this:
```
# HELP qbittorrent_torrents_count Number of torrents in status error under category test_category
# TYPE qbittorrent_torrents_count gauge
qbittorrent_torrents_count{category="test_category",server="127.0.0.1:8080",status="error"} 0.0
# HELP qbittorrent_torrents_count Number of torrents in status missingFiles under category test_category
# TYPE qbittorrent_torrents_count gauge
qbittorrent_torrents_count{category="test_category",server="127.0.0.1:8080",status="missingFiles"} 0.0
# HELP qbittorrent_torrents_count Number of torrents in status uploading under category test_category
# TYPE qbittorrent_torrents_count gauge
qbittorrent_torrents_count{category="test_category",server="127.0.0.1:8080",status="uploading"} 0.0
```

According to https://prometheus.io/docs/instrumenting/exposition_formats/#comments-help-text-and-type-information and https://prometheus.io/docs/instrumenting/exposition_formats/#grouping-and-sorting this should be grouped by the metric name with only one HELP and TYPE comment above the whole group:
```
# HELP qbittorrent_torrents_count Number of torrents
# TYPE qbittorrent_torrents_count gauge
qbittorrent_torrents_count{category="test_category",server="127.0.0.1:8080",status="error"} 0.0
qbittorrent_torrents_count{category="test_category",server="127.0.0.1:8080",status="missingFiles"} 0.0
qbittorrent_torrents_count{category="test_category",server="127.0.0.1:8080",status="uploading"} 0.0
```

The current format breaks other metrics collectors like telegraf with its prometheus input plugin. It generates the following error:
```
E! [inputs.prometheus] Error in plugin: error reading metrics for "http://127.0.0.1:8123/metrics": decoding response failed: text format parsing error in line 67: second HELP line for metric name "qbittorrent_torrents_count"
```

Hints from issue comments:

Solution patch:
diff --git a/qbittorrent_exporter/exporter.py b/qbittorrent_exporter/exporter.py
index 4f3bb5a..9479281 100644
--- a/qbittorrent_exporter/exporter.py
+++ b/qbittorrent_exporter/exporter.py
@@ -62,9 +62,7 @@ class QbittorrentMetricsCollector:
         """
         Yields Prometheus gauges and counters from metrics collected from qbittorrent.
         """
-        metrics: list[Metric] = self.get_qbittorrent_metrics()
-
-        for metric in metrics:
+        for metric in self._get_qbittorrent_status_metrics():
             if metric.metric_type == MetricType.COUNTER:
                 prom_metric = CounterMetricFamily(
                     metric.name, metric.help_text, labels=list(metric.labels.keys())
@@ -78,52 +76,38 @@ class QbittorrentMetricsCollector:
             )
             yield prom_metric
 
-    def get_qbittorrent_metrics(self) -> list[Metric]:
-        """
-        Calls and combines qbittorrent state metrics with torrent metrics.
-        """
-        metrics: list[Metric] = []
-        metrics.extend(self._get_qbittorrent_status_metrics())
-        metrics.extend(self._get_qbittorrent_torrent_tags_metrics())
-        metrics.extend(self._get_qbittorrent_by_torrent_metrics())
+        for gauge in self._get_qbittorrent_by_torrent_metric_gauges():
+            yield gauge
 
-        return metrics
+        yield self._get_qbittorrent_torrent_tags_metrics_gauge()
 
-    def _get_qbittorrent_by_torrent_metrics(self) -> list[Metric]:
+    def _get_qbittorrent_by_torrent_metric_gauges(self) -> list[GaugeMetricFamily]:
         if not self.config.get("export_metrics_by_torrent", False):
             return []
 
-        torrents = self._fetch_torrents()
+        torrent_size_gauge = GaugeMetricFamily(
+            f"{self.config['metrics_prefix']}_torrent_size",
+            "Size of the torrent",
+            labels=["name", "category", "server"],
+        )
 
-        metrics: list[Metric] = []
+        torrent_downloaded_gauge = GaugeMetricFamily(
+            f"{self.config['metrics_prefix']}_torrent_downloaded",
+            "Downloaded data for the torrent",
+            labels=["name", "category", "server"],
+        )
 
-        for torrent in torrents:
-            metrics.append(
-                Metric(
-                    name=f"{self.config['metrics_prefix']}_torrent_size",
-                    value=torrent["size"],
-                    labels={
-                        "name": torrent["name"],
-                        "category": torrent["category"],
-                        "server": self.server,
-                    },
-                    help_text="Size of the torrent",
-                )
+        for torrent in self._fetch_torrents():
+            torrent_size_gauge.add_metric(
+                value=torrent["size"],
+                labels=[torrent["name"], torrent["category"], self.server],
             )
-            metrics.append(
-                Metric(
-                    name=f"{self.config['metrics_prefix']}_torrent_downloaded",
-                    value=torrent["downloaded"],
-                    labels={
-                        "name": torrent["name"],
-                        "category": torrent["category"],
-                        "server": self.server,
-                    },
-                    help_text="Downloaded data for the torrent",
-                )
+            torrent_downloaded_gauge.add_metric(
+                value=torrent["downloaded"],
+                labels=[torrent["name"], torrent["category"], self.server],
             )
 
-        return metrics
+        return [torrent_size_gauge, torrent_downloaded_gauge]
 
     def _get_qbittorrent_status_metrics(self) -> list[Metric]:
         """
@@ -247,39 +231,31 @@ class QbittorrentMetricsCollector:
         """Filters torrents by the given state."""
         return [torrent for torrent in torrents if torrent["state"] == state.value]
 
-    def _construct_metric(self, state: str, category: str, count: int) -> Metric:
-        """Constructs and returns a metric object with a torrent count and appropriate
-        labels."""
-        return Metric(
-            name=f"{self.config['metrics_prefix']}_torrents_count",
-            value=count,
-            labels={
-                "status": state,
-                "category": category,
-                "server": self.server,
-            },
-            help_text=f"Number of torrents in status {state} under category {category}",
-        )
-
-    def _get_qbittorrent_torrent_tags_metrics(self) -> list[Metric]:
+    def _get_qbittorrent_torrent_tags_metrics_gauge(self) -> GaugeMetricFamily:
         categories = self._fetch_categories()
         torrents = self._fetch_torrents()
 
         metrics: list[Metric] = []
         categories["Uncategorized"] = {"name": "Uncategorized", "savePath": ""}
 
+        torrents_count_gauge = GaugeMetricFamily(
+            f"{self.config['metrics_prefix']}_torrents_count",
+            "Number of torrents",
+            labels=["status", "category", "server"],
+        )
+
         for category in categories:
             category_torrents = self._filter_torrents_by_category(category, torrents)
             for state in TorrentStates:
                 state_torrents = self._filter_torrents_by_state(
                     state, category_torrents
                 )
-                metric = self._construct_metric(
-                    state.value, category, len(state_torrents)
+                torrents_count_gauge.add_metric(
+                    value=len(state_torrents),
+                    labels=[state.value, category, self.server],
                 )
-                metrics.append(metric)
 
-        return metrics
+        return torrents_count_gauge
 
 
 class ShutdownSignalHandler:


Test patch:
diff --git a/tests/exporter_test.py b/tests/exporter_test.py
index c2339c6..602d7be 100644
--- a/tests/exporter_test.py
+++ b/tests/exporter_test.py
@@ -24,6 +24,7 @@ class TestQbittorrentMetricsCollector(unittest.TestCase):
             "password": "pass",
             "verify_webui_certificate": False,
             "metrics_prefix": "qbittorrent",
+            "export_metrics_by_torrent": True,
         }
         self.torrentsState = [
             {"name": "Torrent DOWNLOADING 1", "state": TorrentStates.DOWNLOADING},
@@ -50,48 +51,87 @@ class TestQbittorrentMetricsCollector(unittest.TestCase):
             VERIFY_WEBUI_CERTIFICATE=self.config["verify_webui_certificate"],
         )
 
-    def test_collect_gauge(self):
-        mock_metric = Metric(
-            name="test_gauge",
-            metric_type=MetricType.GAUGE,
-            help_text="Test Gauge",
-            labels={"label1": "value1", "server": "localhost:8080"},
-            value=10,
-        )
-        self.collector.get_qbittorrent_metrics = MagicMock(return_value=[mock_metric])
+    def test_collect_by_torrent_metric_gauges(self):
+        # Mock the return value of self.client.torrents.info()
+        self.collector.client.torrents.info.return_value = [
+            {
+                "name": "Torrent 1",
+                "size": 100,
+                "category": "category1",
+                "downloaded": 100,
+            },
+            {
+                "name": "Torrent 2",
+                "size": 200,
+                "category": "category2",
+                "downloaded": 200,
+            },
+            {
+                "name": "Torrent 3",
+                "size": 300,
+                "category": "category3",
+                "downloaded": 300,
+            },
+        ]
+        # Mock the client.torrent_categories.categories attribute
+        self.collector.client.torrent_categories.categories = {
+            "category1": {"name": "Category 1"},
+            "category2": {"name": "Category 2"},
+            "category3": {"name": "Category 3"},
+        }
 
-        result = next(self.collector.collect())
+        result = self.collector._get_qbittorrent_by_torrent_metric_gauges()
 
-        self.assertIsInstance(result, GaugeMetricFamily)
-        self.assertEqual(result.name, "test_gauge")
-        self.assertEqual(result.documentation, "Test Gauge")
+        torrent_size_metric = result[0]
+        self.assertIsInstance(torrent_size_metric, GaugeMetricFamily)
+        self.assertEqual(torrent_size_metric.name, "qbittorrent_torrent_size")
+        self.assertEqual(torrent_size_metric.documentation, "Size of the torrent")
+        self.assertEqual(
+            torrent_size_metric.samples[0].labels,
+            {
+                "name": "Torrent 1",
+                "category": "category1",
+                "server": "localhost:8080/qbt/",
+            },
+        )
+        self.assertEqual(torrent_size_metric.samples[0].value, 100)
+
+        torrent_downloaded_metric = result[1]
+        self.assertIsInstance(torrent_downloaded_metric, GaugeMetricFamily)
         self.assertEqual(
-            result.samples[0].labels, {"label1": "value1", "server": "localhost:8080"}
+            torrent_downloaded_metric.name, "qbittorrent_torrent_downloaded"
         )
-        self.assertEqual(result.samples[0].value, 10)
-
-    def test_collect_counter(self):
-        mock_metric = Metric(
-            name="test_counter",
-            metric_type=MetricType.COUNTER,
-            help_text="Test Counter",
-            labels={"label2": "value2", "server": "localhost:8080"},
-            value=230,
+        self.assertEqual(
+            torrent_downloaded_metric.documentation, "Downloaded data for the torrent"
+        )
+        self.assertEqual(
+            torrent_downloaded_metric.samples[0].labels,
+            {
+                "name": "Torrent 1",
+                "category": "category1",
+                "server": "localhost:8080/qbt/",
+            },
         )
-        self.collector.get_qbittorrent_metrics = MagicMock(return_value=[mock_metric])
+        self.assertEqual(torrent_downloaded_metric.samples[0].value, 100)
 
-        result = next(self.collector.collect())
+    def test_collect_torrent_tags_metric_gauge(self):
+        result = self.collector._get_qbittorrent_torrent_tags_metrics_gauge()
 
-        self.assertIsInstance(result, CounterMetricFamily)
-        self.assertEqual(result.name, "test_counter")
-        self.assertEqual(result.documentation, "Test Counter")
+        self.assertIsInstance(result, GaugeMetricFamily)
+        self.assertEqual(result.name, "qbittorrent_torrents_count")
+        self.assertEqual(result.documentation, "Number of torrents")
         self.assertEqual(
-            result.samples[0].labels, {"label2": "value2", "server": "localhost:8080"}
+            result.samples[0].labels,
+            {
+                "status": "error",
+                "category": "Uncategorized",
+                "server": "localhost:8080/qbt/",
+            },
         )
-        self.assertEqual(result.samples[0].value, 230)
+        self.assertEqual(result.samples[0].value, 0)
 
-    def test_get_qbittorrent_metrics(self):
-        metrics = self.collector.get_qbittorrent_metrics()
+    def test_collect(self):
+        metrics = list(self.collector.collect())
         self.assertNotEqual(len(metrics), 0)
 
     def test_fetch_categories(self):
@@ -192,37 +232,6 @@ class TestQbittorrentMetricsCollector(unittest.TestCase):
         )
         self.assertEqual(result, expected_result)
 
-    def test_construct_metric_with_valid_state_and_category(self):
-        state = "downloading"
-        category = "movies"
-        count = 10
-
-        metric = self.collector._construct_metric(state, category, count)
-
-        self.assertEqual(metric.name, "qbittorrent_torrents_count")
-        self.assertEqual(metric.value, count)
-        self.assertEqual(metric.labels["status"], state)
-        self.assertEqual(metric.labels["category"], category)
-        self.assertEqual(
-            metric.help_text,
-            f"Number of torrents in status {state} under category {category}",
-        )
-
-    def test_construct_metric_with_empty_state_and_category(self):
-        state = ""
-        category = ""
-        count = 5
-
-        metric = self.collector._construct_metric(state, category, count)
-
-        self.assertEqual(metric.name, "qbittorrent_torrents_count")
-        self.assertEqual(metric.value, count)
-        self.assertEqual(metric.labels["status"], state)
-        self.assertEqual(metric.labels["category"], category)
-        self.assertEqual(
-            metric.help_text, "Number of torrents in status  under category "
-        )
-
     def test_get_qbittorrent_status_metrics(self):
         self.collector.client.sync_maindata.return_value = {
             "server_state": {"connection_status": "connected"}