Correct PostgreSQL metrics for dead tuples, with integration tests and refactoring (#2474)

`pg_stat_user_tables` has a row for each table in the database, so selecting the first row of it results in "dead rows for a single random table". Calculating the sum over `n_dead_tup` results in all dead rows instead.

`pg_stat_database` has only one row for each database, so the sum() over `numbackends` was harmless but unnecessary, so I removed that one.

At least for PostgreSQL, there's a difference between `getObject(n, Long.class)` and `getLong(n)`. The former can only convert INT4 and INT8 (but not NUMERIC and DECIMAL) to Long, the latter converts all numeric types. `sum(n_dead_tup)` is such a case where this matters, since its type is NUMERIC.

Co-authored-by: Tommy Ludwig <[email protected]>

Author: easimon

dependencies.gradle

@@ -72,10 +72,12 @@ def VERSIONS = [
         'org.mockito:mockito-core:3.+',
         'org.mockito:mockito-inline:3.+',
         'org.mongodb:mongodb-driver-sync:latest.release',
+        'org.postgresql:postgresql:latest.release',
         'org.slf4j:slf4j-api:1.7.+',
         'org.springframework:spring-context:latest.release',
         'org.testcontainers:junit-jupiter:latest.release',
         'org.testcontainers:kafka:latest.release',
+        'org.testcontainers:postgresql:latest.release',
         'org.testcontainers:testcontainers:latest.release',
         'ru.lanwen.wiremock:wiremock-junit5:latest.release',
         'software.amazon.awssdk:cloudwatch:2.16.+'

micrometer-core/build.gradle

@@ -98,6 +98,10 @@ dependencies {
     testImplementation 'org.testcontainers:testcontainers'
     testImplementation 'org.testcontainers:junit-jupiter'
     testImplementation 'org.testcontainers:kafka'
+
+    // Postgres Binder IT dependencies
+    testImplementation 'org.testcontainers:postgresql'
+    testImplementation 'org.postgresql:postgresql'
 }
 
 task shenandoahTest(type: Test) {

micrometer-core/gradle.lockfile

@@ -36,6 +36,7 @@
  * @author Kristof Depypere
  * @author Jon Schneider
  * @author Johnny Lim
+ * @author Markus Dobel
  * @since 1.1.0
  */
 @NonNullApi
@@ -44,7 +45,7 @@ public class PostgreSQLDatabaseMetrics implements MeterBinder {
 
     private static final String SELECT = "SELECT ";
 
-    private static final String QUERY_DEAD_TUPLE_COUNT = getUserTableQuery("n_dead_tup");
+    private static final String QUERY_DEAD_TUPLE_COUNT = getUserTableQuery("SUM(n_dead_tup)");
     private static final String QUERY_TIMED_CHECKPOINTS_COUNT = getBgWriterQuery("checkpoints_timed");
     private static final String QUERY_REQUESTED_CHECKPOINTS_COUNT = getBgWriterQuery("checkpoints_req");
     private static final String QUERY_BUFFERS_CLEAN = getBgWriterQuery("buffers_clean");
@@ -78,7 +79,7 @@ public PostgreSQLDatabaseMetrics(DataSource postgresDataSource, String database,
         this.beforeResetValuesCacheMap = new ConcurrentHashMap<>();
         this.previousValueCacheMap = new ConcurrentHashMap<>();
 
-        this.queryConnectionCount = getDBStatQuery(database, "SUM(numbackends)");
+        this.queryConnectionCount = getDBStatQuery(database, "numbackends");
         this.queryReadCount = getDBStatQuery(database, "tup_fetched");
         this.queryInsertCount = getDBStatQuery(database, "tup_inserted");
         this.queryTempBytes = getDBStatQuery(database, "temp_bytes");
@@ -95,38 +96,38 @@ private static Tag createDbTag(String database) {
 
     @Override
     public void bindTo(MeterRegistry registry) {
-        Gauge.builder("postgres.size", postgresDataSource, dataSource -> getDatabaseSize())
+        Gauge.builder(Names.SIZE, postgresDataSource, dataSource -> getDatabaseSize())
                 .tags(tags)
                 .description("The database size")
                 .register(registry);
-        Gauge.builder("postgres.connections", postgresDataSource, dataSource -> getConnectionCount())
+        Gauge.builder(Names.CONNECTIONS, postgresDataSource, dataSource -> getConnectionCount())
                 .tags(tags)
                 .description("Number of active connections to the given db")
                 .register(registry);
 
         // Hit ratio can be derived from dividing hits/reads
-        FunctionCounter.builder("postgres.blocks.hits", postgresDataSource,
-                dataSource -> resettableFunctionalCounter("postgres.blocks.hits", this::getBlockHits))
+        FunctionCounter.builder(Names.BLOCKS_HITS, postgresDataSource,
+                dataSource -> resettableFunctionalCounter(Names.BLOCKS_HITS, this::getBlockHits))
                 .tags(tags)
                 .description("Number of times disk blocks were found already in the buffer cache, so that a read was not necessary")
                 .register(registry);
-        FunctionCounter.builder("postgres.blocks.reads", postgresDataSource,
-                dataSource -> resettableFunctionalCounter("postgres.blocks.reads", this::getBlockReads))
+        FunctionCounter.builder(Names.BLOCKS_READS, postgresDataSource,
+                dataSource -> resettableFunctionalCounter(Names.BLOCKS_READS, this::getBlockReads))
                 .tags(tags)
                 .description("Number of disk blocks read in this database")
                 .register(registry);
 
-        FunctionCounter.builder("postgres.transactions", postgresDataSource,
-                dataSource -> resettableFunctionalCounter("postgres.transactions", this::getTransactionCount))
+        FunctionCounter.builder(Names.TRANSACTIONS, postgresDataSource,
+                dataSource -> resettableFunctionalCounter(Names.TRANSACTIONS, this::getTransactionCount))
                 .tags(tags)
                 .description("Total number of transactions executed (commits + rollbacks)")
                 .register(registry);
-        Gauge.builder("postgres.locks", postgresDataSource, dataSource -> getLockCount())
+        Gauge.builder(Names.LOCKS, postgresDataSource, dataSource -> getLockCount())
                 .tags(tags)
                 .description("Number of locks on the given db")
                 .register(registry);
-        FunctionCounter.builder("postgres.temp.writes", postgresDataSource,
-                dataSource -> resettableFunctionalCounter("postgres.temp.writes", this::getTempBytes))
+        FunctionCounter.builder(Names.TEMP_WRITES, postgresDataSource,
+                dataSource -> resettableFunctionalCounter(Names.TEMP_WRITES, this::getTempBytes))
                 .tags(tags)
                 .description("The total amount of temporary writes to disk to execute queries")
                 .baseUnit(BaseUnits.BYTES)
@@ -137,56 +138,56 @@ public void bindTo(MeterRegistry registry) {
     }
 
     private void registerRowCountMetrics(MeterRegistry registry) {
-        FunctionCounter.builder("postgres.rows.fetched", postgresDataSource,
-                dataSource -> resettableFunctionalCounter("postgres.rows.fetched", this::getReadCount))
+        FunctionCounter.builder(Names.ROWS_FETCHED, postgresDataSource,
+                dataSource -> resettableFunctionalCounter(Names.ROWS_FETCHED, this::getReadCount))
                 .tags(tags)
                 .description("Number of rows fetched from the db")
                 .register(registry);
-        FunctionCounter.builder("postgres.rows.inserted", postgresDataSource,
-                dataSource -> resettableFunctionalCounter("postgres.rows.inserted", this::getInsertCount))
+        FunctionCounter.builder(Names.ROWS_INSERTED, postgresDataSource,
+                dataSource -> resettableFunctionalCounter(Names.ROWS_INSERTED, this::getInsertCount))
                 .tags(tags)
                 .description("Number of rows inserted from the db")
                 .register(registry);
-        FunctionCounter.builder("postgres.rows.updated", postgresDataSource,
-                dataSource -> resettableFunctionalCounter("postgres.rows.updated", this::getUpdateCount))
+        FunctionCounter.builder(Names.ROWS_UPDATED, postgresDataSource,
+                dataSource -> resettableFunctionalCounter(Names.ROWS_UPDATED, this::getUpdateCount))
                 .tags(tags)
                 .description("Number of rows updated from the db")
                 .register(registry);
-        FunctionCounter.builder("postgres.rows.deleted", postgresDataSource,
-                dataSource -> resettableFunctionalCounter("postgres.rows.deleted", this::getDeleteCount))
+        FunctionCounter.builder(Names.ROWS_DELETED, postgresDataSource,
+                dataSource -> resettableFunctionalCounter(Names.ROWS_DELETED, this::getDeleteCount))
                 .tags(tags)
                 .description("Number of rows deleted from the db")
                 .register(registry);
-        Gauge.builder("postgres.rows.dead", postgresDataSource, dataSource -> getDeadTupleCount())
+        Gauge.builder(Names.ROWS_DEAD, postgresDataSource, dataSource -> getDeadTupleCount())
                 .tags(tags)
                 .description("Total number of dead rows in the current database")
                 .register(registry);
     }
 
     private void registerCheckpointMetrics(MeterRegistry registry) {
-        FunctionCounter.builder("postgres.checkpoints.timed", postgresDataSource,
-                dataSource -> resettableFunctionalCounter("postgres.checkpoints.timed", this::getTimedCheckpointsCount))
+        FunctionCounter.builder(Names.CHECKPOINTS_TIMED, postgresDataSource,
+                dataSource -> resettableFunctionalCounter(Names.CHECKPOINTS_TIMED, this::getTimedCheckpointsCount))
                 .tags(tags)
                 .description("Number of checkpoints timed")
                 .register(registry);
-        FunctionCounter.builder("postgres.checkpoints.requested", postgresDataSource,
-                dataSource -> resettableFunctionalCounter("postgres.checkpoints.requested", this::getRequestedCheckpointsCount))
+        FunctionCounter.builder(Names.CHECKPOINTS_REQUESTED, postgresDataSource,
+                dataSource -> resettableFunctionalCounter(Names.CHECKPOINTS_REQUESTED, this::getRequestedCheckpointsCount))
                 .tags(tags)
                 .description("Number of checkpoints requested")
                 .register(registry);
 
-        FunctionCounter.builder("postgres.buffers.checkpoint", postgresDataSource,
-                dataSource -> resettableFunctionalCounter("postgres.buffers.checkpoint", this::getBuffersCheckpoint))
+        FunctionCounter.builder(Names.BUFFERS_CHECKPOINT, postgresDataSource,
+                dataSource -> resettableFunctionalCounter(Names.BUFFERS_CHECKPOINT, this::getBuffersCheckpoint))
                 .tags(tags)
                 .description("Number of buffers written during checkpoints")
                 .register(registry);
-        FunctionCounter.builder("postgres.buffers.clean", postgresDataSource,
-                dataSource -> resettableFunctionalCounter("postgres.buffers.clean", this::getBuffersClean))
+        FunctionCounter.builder(Names.BUFFERS_CLEAN, postgresDataSource,
+                dataSource -> resettableFunctionalCounter(Names.BUFFERS_CLEAN, this::getBuffersClean))
                 .tags(tags)
                 .description("Number of buffers written by the background writer")
                 .register(registry);
-        FunctionCounter.builder("postgres.buffers.backend", postgresDataSource,
-                dataSource -> resettableFunctionalCounter("postgres.buffers.backend", this::getBuffersBackend))
+        FunctionCounter.builder(Names.BUFFERS_BACKEND, postgresDataSource,
+                dataSource -> resettableFunctionalCounter(Names.BUFFERS_BACKEND, this::getBuffersBackend))
                 .tags(tags)
                 .description("Number of buffers written directly by a backend")
                 .register(registry);
@@ -282,7 +283,7 @@ private Long runQuery(String query) {
              Statement statement = connection.createStatement();
              ResultSet resultSet = statement.executeQuery(query)) {
             if (resultSet.next()) {
-                return resultSet.getObject(1, Long.class);
+                return resultSet.getLong(1);
             }
         } catch (SQLException ignored) {
         }
@@ -300,4 +301,34 @@ private static String getUserTableQuery(String statName) {
     private static String getBgWriterQuery(String statName) {
         return SELECT + statName + " FROM pg_stat_bgwriter";
     }
+
+    static final class Names {
+        public static final String SIZE = of("size");
+        public static final String CONNECTIONS = of("connections");
+        public static final String BLOCKS_HITS = of("blocks.hits");
+        public static final String BLOCKS_READS = of("blocks.reads");
+        public static final String TRANSACTIONS = of("transactions");
+        public static final String LOCKS = of("locks");
+        public static final String TEMP_WRITES = of("temp.writes");
+
+        public static final String ROWS_FETCHED = of("rows.fetched");
+        public static final String ROWS_INSERTED = of("rows.inserted");
+        public static final String ROWS_UPDATED = of("rows.updated");
+        public static final String ROWS_DELETED = of("rows.deleted");
+        public static final String ROWS_DEAD = of("rows.dead");
+
+        public static final String CHECKPOINTS_TIMED = of("checkpoints.timed");
+        public static final String CHECKPOINTS_REQUESTED = of("checkpoints.requested");
+
+        public static final String BUFFERS_CHECKPOINT = of("buffers.checkpoint");
+        public static final String BUFFERS_CLEAN = of("buffers.clean");
+        public static final String BUFFERS_BACKEND = of("buffers.backend");
+
+        private static String of(String name) {
+            return "postgres." + name;
+        }
+
+        private Names() {
+        }
+    }
 }