From a690d8a3cfae84ec1102d39567cee71626c0a3a0 Mon Sep 17 00:00:00 2001 From: Maarten van den Berg Date: Thu, 5 Sep 2024 15:21:52 +0200 Subject: [PATCH 1/5] Fix SyntaxWarning in `_instanceMetric` --- prometheus_pgbouncer_exporter/collector.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/prometheus_pgbouncer_exporter/collector.py b/prometheus_pgbouncer_exporter/collector.py index 1e04b45..9b0789d 100644 --- a/prometheus_pgbouncer_exporter/collector.py +++ b/prometheus_pgbouncer_exporter/collector.py @@ -30,9 +30,9 @@ def collect(self): return metrics.values() def _instanceMetric(self, data): - if data["type"] is "counter": + if data["type"] == "counter": return CounterMetricFamily(data["name"], data["help"], labels=data["labels"].keys()) - elif data["type"] is "gauge": + elif data["type"] == "gauge": return GaugeMetricFamily(data["name"], data["help"], labels=data["labels"].keys()) else: raise Exception("Unsupported metric type: {type}".format(type=data['type'])) From 5711d056a0916e05fc2002642d286ef7784ddb3b Mon Sep 17 00:00:00 2001 From: Maarten van den Berg Date: Thu, 5 Sep 2024 15:22:42 +0200 Subject: [PATCH 2/5] Make _filterMetricsBy{In,Ex}cludeDatabases return generators --- prometheus_pgbouncer_exporter/collector.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/prometheus_pgbouncer_exporter/collector.py b/prometheus_pgbouncer_exporter/collector.py index 9b0789d..e4b48d1 100644 --- a/prometheus_pgbouncer_exporter/collector.py +++ b/prometheus_pgbouncer_exporter/collector.py @@ -185,14 +185,14 @@ def _filterMetricsByIncludeDatabases(self, results, databases): if not databases: return results - return list(filter(lambda item: item["database"] in databases, results)) + return filter(lambda item: item["database"] in databases, results) def _filterMetricsByExcludeDatabases(self, results, databases): # No filtering if empty if not databases: return results - return list(filter(lambda item: item["database"] not in databases, results)) + return filter(lambda item: item["database"] not in databases, results) def _fetchMetrics(self, conn, query): cursor = False From 09d45444a6a45fab3f486aadac0206c876f0beaa Mon Sep 17 00:00:00 2001 From: Maarten van den Berg Date: Thu, 5 Sep 2024 15:27:10 +0200 Subject: [PATCH 3/5] SHOW POOLS: filter out pools with no client or server connections --- prometheus_pgbouncer_exporter/collector.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/prometheus_pgbouncer_exporter/collector.py b/prometheus_pgbouncer_exporter/collector.py index e4b48d1..bfac64c 100644 --- a/prometheus_pgbouncer_exporter/collector.py +++ b/prometheus_pgbouncer_exporter/collector.py @@ -80,6 +80,7 @@ def collect(self): if results: results = self._filterMetricsByIncludeDatabases(results, self.config.getIncludeDatabases()) results = self._filterMetricsByExcludeDatabases(results, self.config.getExcludeDatabases()) + results = self._filterEmptyPools(results) metrics += self._exportMetrics(results, "pgbouncer_pools_", [ {"type": "gauge", "column": "cl_active", "metric": "client_active_connections", "help": "Client connections that are linked to server connection and can process queries"}, {"type": "gauge", "column": "cl_waiting", "metric": "client_waiting_connections", "help": "Client connections have sent queries but have not yet got a server connection"}, @@ -194,6 +195,25 @@ def _filterMetricsByExcludeDatabases(self, results, databases): return filter(lambda item: item["database"] not in databases, results) + def _filterEmptyPools(self, results): + # Filter out pools that have no client or server connections. + return filter( + lambda item: sum( + item[x] + for x in [ + "cl_active", + "cl_waiting", + "sv_active", + "sv_idle", + "sv_used", + "sv_tested", + "sv_login", + ] + ) + > 0, + results, + ) + def _fetchMetrics(self, conn, query): cursor = False From 9858eccc47c6b54e56227b30994c79454fd342c3 Mon Sep 17 00:00:00 2001 From: Maarten van den Berg Date: Fri, 6 Sep 2024 11:40:35 +0200 Subject: [PATCH 4/5] Make empty pool filtering configurable --- README.md | 6 ++++++ prometheus_pgbouncer_exporter/collector.py | 3 ++- prometheus_pgbouncer_exporter/config.py | 3 +++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 385fc58..741c0f3 100644 --- a/README.md +++ b/README.md @@ -112,6 +112,12 @@ pgbouncers: extra_labels: pool_id: 1 + # Omit metrics for pools for which PgBouncer has no client or server + # connections open. Recommended if you have a lot of different database + # users, because PgBouncer never forgets about users it has seen. + # If omitted, no filtering is done. + filter_empty_pools: true + - dsn: postgresql://pgbouncer:$(PGBOUNCER_PASS)@localhost:6432/pgbouncer exclude_databases: - pgbouncer diff --git a/prometheus_pgbouncer_exporter/collector.py b/prometheus_pgbouncer_exporter/collector.py index bfac64c..ef6dfaa 100644 --- a/prometheus_pgbouncer_exporter/collector.py +++ b/prometheus_pgbouncer_exporter/collector.py @@ -80,7 +80,8 @@ def collect(self): if results: results = self._filterMetricsByIncludeDatabases(results, self.config.getIncludeDatabases()) results = self._filterMetricsByExcludeDatabases(results, self.config.getExcludeDatabases()) - results = self._filterEmptyPools(results) + if self.config.getFilterEmptyPools(): + results = self._filterEmptyPools(results) metrics += self._exportMetrics(results, "pgbouncer_pools_", [ {"type": "gauge", "column": "cl_active", "metric": "client_active_connections", "help": "Client connections that are linked to server connection and can process queries"}, {"type": "gauge", "column": "cl_waiting", "metric": "client_waiting_connections", "help": "Client connections have sent queries but have not yet got a server connection"}, diff --git a/prometheus_pgbouncer_exporter/config.py b/prometheus_pgbouncer_exporter/config.py index 89e6092..019e405 100644 --- a/prometheus_pgbouncer_exporter/config.py +++ b/prometheus_pgbouncer_exporter/config.py @@ -105,6 +105,9 @@ def getIncludeDatabases(self): def getExcludeDatabases(self): return self.config["exclude_databases"] if "exclude_databases" in self.config else [] + def getFilterEmptyPools(self): + return self.config.get("filter_empty_pools", False) + def getExtraLabels(self): # Lazy instance extra labels if self.labels is False: From 3e9541667848b8a29be060b4f9ae44767753f415 Mon Sep 17 00:00:00 2001 From: Maarten van den Berg Date: Fri, 6 Sep 2024 13:13:00 +0200 Subject: [PATCH 5/5] Add unit test to verify empty pool filtering behaviour --- tests/test_collector.py | 42 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/test_collector.py b/tests/test_collector.py index 0a9bda4..a4703b3 100644 --- a/tests/test_collector.py +++ b/tests/test_collector.py @@ -76,6 +76,17 @@ def fetchMetricsPartialFailureFromPgBouncer17Mock(conn, query): def fetchMetricsFailureMock(conn, query): raise Exception("Error while fetching metrics") +def fetchMetricsWithEmptyPool(conn, query): + if query == "SHOW POOLS": + return [ + {"database": "test", "user": "marco", "cl_active": 1, "cl_waiting": 2, "sv_active": 3, "sv_idle": 4, "sv_used": 5, "sv_tested": 6, "sv_login": 7, "maxwait": 8, "maxwait_us": 100000 }, + {"database": "prod", "user": "marco", "cl_active": 8, "cl_waiting": 7, "sv_active": 6, "sv_idle": 5, "sv_used": 4, "sv_tested": 3, "sv_login": 2, "maxwait": 1, "maxwait_us": 200000 }, + {"database": "test", "user": "empty", "cl_active": 0, "cl_waiting": 0, "sv_active": 0, "sv_idle": 0, "sv_used": 0, "sv_tested": 0, "sv_login": 0, "maxwait": 0, "maxwait_us": 0 }, + {"database": "prod", "user": "empty", "cl_active": 0, "cl_waiting": 0, "sv_active": 0, "sv_idle": 0, "sv_used": 0, "sv_tested": 0, "sv_login": 0, "maxwait": 0, "maxwait_us": 0 }, + ] + + return [] + # # Tests # @@ -429,6 +440,37 @@ def testShouldReturnScrapedMetricsOnPartialFailure(self): metrics = getMetricsByName(collector.collect(), "pgbouncer_pools_client_active_connections") self.assertEqual(len(metrics), 0) + # + # Empty pool filtering + # + + def testEmptyPoolFiltering(self): + # By default no filtering should be done + config_default = PgbouncerConfig({}) + collector = PgbouncerMetricsCollector(config_default) + collector._createConnection = MagicMock(return_value=False) + collector._fetchMetrics = MagicMock(side_effect=fetchMetricsWithEmptyPool) + + metrics = getMetricsByName(collector.collect(), "pgbouncer_pools_client_active_connections") + self.assertEqual(len(metrics), 4) + self.assertEqual(list(x["labels"]["user"] for x in metrics), ["marco", "marco", "empty", "empty"]) + + # Same when explicitly disabled: + config_no_filter = PgbouncerConfig({"filter_empty_pools": False}) + collector.config = config_no_filter + + metrics = getMetricsByName(collector.collect(), "pgbouncer_pools_client_active_connections") + self.assertEqual(len(metrics), 4) + self.assertEqual(list(x["labels"]["user"] for x in metrics), ["marco", "marco", "empty", "empty"]) + + # When filtering is on only the non-empty pools should remain: + config_yes_filter = PgbouncerConfig({"filter_empty_pools": True}) + collector.config = config_yes_filter + + metrics = getMetricsByName(collector.collect(), "pgbouncer_pools_client_active_connections") + self.assertEqual(len(metrics), 2) + self.assertEqual(list(x["labels"]["user"] for x in metrics), ["marco", "marco"]) + if __name__ == '__main__': unittest.main()