spreaker / prometheus-pgbouncer-exporter

Prometheus exporter for PgBouncer
MIT License
104 stars 38 forks source link

pgBouncer 1.8 - pgbouncer_stats_queries_total #5

Closed uspen closed 5 years ago

uspen commented 6 years ago

With pgBouncer 1.8.1 metic pgbouncer_stats_queries_total not worked

pracucci commented 6 years ago

Sorry @uspen for the extremely late reply (I somewhat missed the notification and I've just realized it). We (my team) are still on pgbouncer 1.7.x so we're not in hurry to fix 1.8.x metrics compatibility, which - I confirm - have changed SHOW STATS.

Are you willing to try to submit a PR which fixes it, keeping backward compatibility? If not - which is totally OK - I will fix it as soon as we'll upgrade to pgbouncer 1.8.x or above (may realistically happen in a couple of months).

uspen commented 6 years ago

Hello! Thank for you work :-) I cannot give PR, sorry. We used pgbouncer 1.7 in production too. I wait you.Thanks!

bitglue commented 5 years ago

The stats semantics have significantly changed from pgbouncer 1.7.2 to 1.8.0. For example, I ran through pgbouncer:

begin;
select 1;
select 2;
select 3;
commit;

The resulting stats in pgbouncer 1.7.2 are:

pgbouncer=# show stats;
 database  | total_requests | total_received | total_sent | total_query_time | avg_req | avg_recv | avg_sent | avg_query 
-----------+----------------+----------------+------------+------------------+---------+----------+----------+-----------
 postgres  |              1 |             70 |        248 |          7969592 |       0 |        0 |        2 |   7969592

And in 1.9.0 (1.8.0 would be similar):

pgbouncer=# show stats;
 database  | total_xact_count | total_query_count | total_received | total_sent | total_xact_time | total_query_time | total_wait_time | avg_xact_count | avg_query_count | avg_recv | avg_sent | avg_xact_time | avg_query_time | avg_wait_time 
-----------+------------------+-------------------+----------------+------------+-----------------+------------------+-----------------+----------------+-----------------+----------+----------+---------------+----------------+---------------
 postgres  |                1 |                 5 |             70 |        248 |         6717570 |             7390 |            4808 |              0 |               0 |        0 |        3 |       6717570 |           1478 |            66

1.7.2 shows 1 "request". 1.9.0 shows 5 queries and 1 transaction.

In other words, prior to pgbouncer 1.8, a "request" was everything from when the client was attached to a server, to when it was done. pgbouncer stats don't reflect at all what happened: if it was a single query, or a transaction of many queries.

Unfortunately prometheus-pgbouncer-exporter renames "request" (as reported by pgbouncer stats) to "query", as in pgbouncer_stats_queries_total. This is a little misleading. Moreover it conflicts with the 1.8+ stats, which really do report queries.

Thoughts on how to resolve the conflict? My preference would be to take the thing currently named pgbouncer_stats_queries_total and call it pgbouncer_stats_requests_total, to more closely match the actual pgbouncer output. When run against pgbouncer 1.8 this metric simply won't be present. pgbouncer_stats_queries_total then refers to total_query_count present only in pgbouncer 1.8+.

This way in a mixed environment of pgbouncer 1.7 and 1.8 there's no ambiguity about what's being reported. I would not want to overload pgbouncer_stats_queries_total to mean "maybe total_requests, or maybe total_query_count, depending on which pgbouncer version is being run", since they are different numbers, and comparing them is at least misleading.

pracucci commented 5 years ago

Thanks @bitglue for your patience. I've released version 2.0.0 which includes your PR #8.