From f9ecb0468e991b6a9cfa19db4c5a2a33418b1eaa Mon Sep 17 00:00:00 2001 From: hari-mani Date: Fri, 24 Oct 2025 00:55:15 +0530 Subject: [PATCH 1/9] feat: initial commit for supporting Postgres 17 in PostgreSQLDatabaseMetrics Signed-off-by: hari-mani --- .../binder/db/PostgreSQLDatabaseMetrics.java | 117 ++++++++++++- ...reSQL17DatabaseMetricsIntegrationTest.java | 162 ++++++++++++++++++ 2 files changed, 276 insertions(+), 3 deletions(-) create mode 100644 micrometer-core/src/test/java/io/micrometer/core/instrument/binder/db/PostgreSQL17DatabaseMetricsIntegrationTest.java diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java index 7bf40176c0..b1c8552fea 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java @@ -13,8 +13,10 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package io.micrometer.core.instrument.binder.db; +import com.google.common.base.Splitter; import io.micrometer.core.instrument.*; import io.micrometer.core.instrument.binder.BaseUnits; import io.micrometer.core.instrument.binder.MeterBinder; @@ -25,9 +27,13 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; +import java.util.Iterator; import java.util.Map; +import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.function.DoubleSupplier; +import java.util.regex.Matcher; +import java.util.regex.Pattern; /** * {@link MeterBinder} for a PostgreSQL database. @@ -36,6 +42,7 @@ * @author Jon Schneider * @author Johnny Lim * @author Markus Dobel + * @apiNote Hari Mani * @since 1.1.0 */ @NullMarked @@ -53,8 +60,16 @@ public class PostgreSQLDatabaseMetrics implements MeterBinder { private static final String QUERY_BUFFERS_BACKEND = getBgWriterQuery("buffers_backend"); + private static final Stat BACKEND_BUFFER_WRITES = new Stat("pg_stat_io", "SUM(writes)"); + private static final String QUERY_BUFFERS_CHECKPOINT = getBgWriterQuery("buffers_checkpoint"); + private static final Stat CHECKPOINTER_BUFFERS_WRITTEN = new Stat("pg_stat_checkpointer", "buffers_written"); + + private static final Stat TIMED_CHECKPOINTS_COUNT = new Stat("pg_stat_checkpointer", "num_timed"); + + private static final Stat REQUESTED_CHECKPOINTS_COUNT = new Stat("pg_stat_checkpointer", "num_requested"); + private final String database; private final DataSource postgresDataSource; @@ -83,6 +98,8 @@ public class PostgreSQLDatabaseMetrics implements MeterBinder { private final String queryTransactionCount; + private final Version serverVersion; + public PostgreSQLDatabaseMetrics(DataSource postgresDataSource, String database) { this(postgresDataSource, database, Tags.empty()); } @@ -103,6 +120,7 @@ public PostgreSQLDatabaseMetrics(DataSource postgresDataSource, String database, this.queryBlockHits = getDBStatQuery(database, "blks_hit"); this.queryBlockReads = getDBStatQuery(database, "blks_read"); this.queryTransactionCount = getDBStatQuery(database, "xact_commit + xact_rollback"); + this.serverVersion = getServerVersion(); } private static Tag createDbTag(String database) { @@ -273,10 +291,16 @@ private Long getDeadTupleCount() { } private Long getTimedCheckpointsCount() { + if (this.serverVersion.isAbove(Version.V17)) { + return runQuery(TIMED_CHECKPOINTS_COUNT.getQuery()); + } return runQuery(QUERY_TIMED_CHECKPOINTS_COUNT); } private Long getRequestedCheckpointsCount() { + if (this.serverVersion.isAbove(Version.V17)) { + return runQuery(REQUESTED_CHECKPOINTS_COUNT.getQuery()); + } return runQuery(QUERY_REQUESTED_CHECKPOINTS_COUNT); } @@ -285,10 +309,16 @@ private Long getBuffersClean() { } private Long getBuffersBackend() { + if (this.serverVersion.isAbove(Version.V17)) { + return runQuery(BACKEND_BUFFER_WRITES.getQuery()); + } return runQuery(QUERY_BUFFERS_BACKEND); } private Long getBuffersCheckpoint() { + if (this.serverVersion.isAbove(Version.V17)) { + return runQuery(CHECKPOINTER_BUFFERS_WRITTEN.getQuery()); + } return runQuery(QUERY_BUFFERS_CHECKPOINT); } @@ -309,17 +339,26 @@ Double resettableFunctionalCounter(String functionalCounterKey, DoubleSupplier f return correctedValue; } + private Version getServerVersion() { + return runQuery("SHOW server_version", resultSet -> resultSet.getString(1)).map(Version::parse).orElse(Version.EMPTY); + } + private Long runQuery(String query) { + return runQuery(query, resultSet -> resultSet.getLong(1)).orElse(0L); + } + + private Optional runQuery(final String query, final ResultSetGetter resultSetGetter) { try (Connection connection = postgresDataSource.getConnection(); Statement statement = connection.createStatement(); ResultSet resultSet = statement.executeQuery(query)) { if (resultSet.next()) { - return resultSet.getLong(1); + return Optional.of(resultSetGetter.get(resultSet)); } } - catch (SQLException ignored) { + catch (SQLException err) { + System.err.println(err.getMessage()); } - return 0L; + return Optional.empty(); } private static String getDBStatQuery(String database, String statName) { @@ -366,4 +405,76 @@ private Names() { } + @FunctionalInterface + interface ResultSetGetter { + T get(ResultSet resultSet) throws SQLException; + } + + static class Stat { + + private final String statView; + + private final String statName; + + public Stat(String statView, String statName) { + this.statView = statView; + this.statName = statName; + } + + public String getQuery() { + return String.format("SELECT %s FROM %s;", this.statName, this.statView); + } + + } + + static final class Version { + + private static final Pattern VERSION_PATTERN = Pattern.compile("^(\\d+\\.\\d+).*"); + + private static final Splitter SPLITTER = Splitter.on(".").trimResults().omitEmptyStrings(); + + static final Version EMPTY = new Version(0, 0); + + static final Version V17 = new Version(17, 0); + + final int majorVersion; + + final int minorVersion; + + static Version parse(String versionString) { + try { + final Matcher matcher = VERSION_PATTERN.matcher(versionString); + if (!matcher.matches()) { + return EMPTY; + } + final Iterator version = SPLITTER.split(matcher.group(1)).iterator(); + return new Version(Integer.parseInt(version.next()), Integer.parseInt(version.next())); + } + catch (Exception exception) { + return EMPTY; + } + } + + Version(int majorVersion, int minorVersion) { + this.majorVersion = majorVersion; + this.minorVersion = minorVersion; + } + + public boolean isAbove(final Version other) { + if (this.majorVersion > other.majorVersion) { + return true; + } + if (this.majorVersion < other.majorVersion) { + return false; + } + return this.minorVersion >= other.minorVersion; + } + + @Override + public String toString() { + return majorVersion + "." + minorVersion; + } + + } + } diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/db/PostgreSQL17DatabaseMetricsIntegrationTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/db/PostgreSQL17DatabaseMetricsIntegrationTest.java new file mode 100644 index 0000000000..ec78f2980f --- /dev/null +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/db/PostgreSQL17DatabaseMetricsIntegrationTest.java @@ -0,0 +1,162 @@ +/* + * Copyright 2022 VMware, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.micrometer.core.instrument.binder.db; + +import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.Tags; +import io.micrometer.core.instrument.search.RequiredSearch; +import io.micrometer.core.instrument.simple.SimpleMeterRegistry; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.postgresql.ds.PGSimpleDataSource; +import org.testcontainers.containers.PostgreSQLContainer; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; +import org.testcontainers.utility.DockerImageName; + +import javax.sql.DataSource; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.SQLException; +import java.util.Arrays; +import java.util.List; + +import static io.micrometer.core.instrument.binder.db.PostgreSQLDatabaseMetrics.Names.*; +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Hari Mani + */ +@Testcontainers +@Tag("docker") +class PostgreSQL17DatabaseMetricsIntegrationTest { + + @Container + private final PostgreSQLContainer postgres = new PostgreSQLContainer<>(getDockerImageName()); + + private final MeterRegistry registry = new SimpleMeterRegistry(); + + private DataSource dataSource; + + private Tags tags; + + // statistics are updated only every PGSTAT_STAT_INTERVAL, which is 500ms. Add a bit + // for stable tests. + private static final long PGSTAT_STAT_INTERVAL = 500L + 50L; + + @BeforeEach + void setup() { + dataSource = createDataSource(); + tags = Tags.of("database", postgres.getDatabaseName()); + + // tag::setup[] + new PostgreSQLDatabaseMetrics(dataSource, postgres.getDatabaseName()).bindTo(registry); + // end::setup[] + } + + @Test + void gaugesAreNotZero() throws Exception { + /* create noise to increment gauges */ + // tag::result[] + executeSql("CREATE TABLE gauge_test_table (val varchar(255))", + "INSERT INTO gauge_test_table (val) VALUES ('foo')", "UPDATE gauge_test_table SET val = 'bar'", + "SELECT * FROM gauge_test_table", "DELETE FROM gauge_test_table"); + Thread.sleep(PGSTAT_STAT_INTERVAL); + + final List GAUGES = Arrays.asList(SIZE, CONNECTIONS, ROWS_DEAD, LOCKS); + + for (String name : GAUGES) { + assertThat(get(name).gauge().value()).withFailMessage("Gauge " + name + " is zero.").isGreaterThan(0); + } + // end::result[] + } + + @Test + void countersAreNotZero() throws Exception { + /* create noise to increment counters */ + // @formatter:off + executeSql( + "CREATE TABLE counter_test_table (val varchar(255))", + "INSERT INTO counter_test_table (val) VALUES ('foo')", + "UPDATE counter_test_table SET val = 'bar'", + "SELECT * FROM counter_test_table", + "DELETE FROM counter_test_table" + ); + // @formatter:on + Thread.sleep(PGSTAT_STAT_INTERVAL); + + final List COUNTERS = Arrays.asList(BLOCKS_HITS, BLOCKS_READS, TRANSACTIONS, ROWS_FETCHED, + ROWS_INSERTED, ROWS_UPDATED, ROWS_DELETED, BUFFERS_CHECKPOINT); + + /* + * the following counters are zero on a clean database and hard to increase + * reliably + */ + final List ZERO_COUNTERS = Arrays.asList(TEMP_WRITES, CHECKPOINTS_TIMED, CHECKPOINTS_REQUESTED, + BUFFERS_CLEAN, BUFFERS_BACKEND); + + for (String name : COUNTERS) { + assertThat(get(name).functionCounter().count()).withFailMessage("Counter " + name + " is zero.") + .isGreaterThan(0); + } + } + + @Test + void deadTuplesGaugeIncreases() throws Exception { + final double deadRowsBefore = get(ROWS_DEAD).gauge().value(); + + executeSql("CREATE TABLE dead_tuples_test_table (val varchar(255))", + "INSERT INTO dead_tuples_test_table (val) VALUES ('foo')", + "UPDATE dead_tuples_test_table SET val = 'bar'"); + + // wait for stats to be updated + Thread.sleep(PGSTAT_STAT_INTERVAL); + assertThat(get(ROWS_DEAD).gauge().value()).isGreaterThan(deadRowsBefore); + } + + private DataSource createDataSource() { + final PGSimpleDataSource dataSource = new PGSimpleDataSource(); + dataSource.setURL(postgres.getJdbcUrl()); + dataSource.setUser(postgres.getUsername()); + dataSource.setPassword(postgres.getPassword()); + dataSource.setDatabaseName(postgres.getDatabaseName()); + return dataSource; + } + + private void executeSql(String... statements) throws SQLException { + try (final Connection connection = dataSource.getConnection()) { + executeSql(connection, statements); + } + } + + private void executeSql(Connection connection, String... statements) throws SQLException { + for (String sql : statements) { + try (final PreparedStatement stmt = connection.prepareStatement(sql)) { + stmt.execute(); + } + } + } + + private RequiredSearch get(final String name) { + return registry.get(name).tags(tags); + } + + private static DockerImageName getDockerImageName() { + return DockerImageName.parse("public.ecr.aws/docker/library/postgres:17.4").asCompatibleSubstituteFor("postgres"); + } + +} From 761aa73b4c476ed1e688bb099acd74e6764d319a Mon Sep 17 00:00:00 2001 From: hari-mani Date: Mon, 27 Oct 2025 15:43:01 +0530 Subject: [PATCH 2/9] chore: use dockerhub image in integration test and address checkstyle issues Signed-off-by: hari-mani --- .../binder/db/PostgreSQLDatabaseMetrics.java | 13 ++++++------- .../PostgreSQL17DatabaseMetricsIntegrationTest.java | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java index b1c8552fea..84fe51256a 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java @@ -16,7 +16,6 @@ package io.micrometer.core.instrument.binder.db; -import com.google.common.base.Splitter; import io.micrometer.core.instrument.*; import io.micrometer.core.instrument.binder.BaseUnits; import io.micrometer.core.instrument.binder.MeterBinder; @@ -27,7 +26,6 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; -import java.util.Iterator; import java.util.Map; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; @@ -340,7 +338,8 @@ Double resettableFunctionalCounter(String functionalCounterKey, DoubleSupplier f } private Version getServerVersion() { - return runQuery("SHOW server_version", resultSet -> resultSet.getString(1)).map(Version::parse).orElse(Version.EMPTY); + return runQuery("SHOW server_version", resultSet -> resultSet.getString(1)).map(Version::parse) + .orElse(Version.EMPTY); } private Long runQuery(String query) { @@ -407,7 +406,9 @@ private Names() { @FunctionalInterface interface ResultSetGetter { + T get(ResultSet resultSet) throws SQLException; + } static class Stat { @@ -431,8 +432,6 @@ static final class Version { private static final Pattern VERSION_PATTERN = Pattern.compile("^(\\d+\\.\\d+).*"); - private static final Splitter SPLITTER = Splitter.on(".").trimResults().omitEmptyStrings(); - static final Version EMPTY = new Version(0, 0); static final Version V17 = new Version(17, 0); @@ -447,8 +446,8 @@ static Version parse(String versionString) { if (!matcher.matches()) { return EMPTY; } - final Iterator version = SPLITTER.split(matcher.group(1)).iterator(); - return new Version(Integer.parseInt(version.next()), Integer.parseInt(version.next())); + final String[] versionArr = matcher.group(1).split("\\.", 2); + return new Version(Integer.parseInt(versionArr[0]), Integer.parseInt(versionArr[1])); } catch (Exception exception) { return EMPTY; diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/db/PostgreSQL17DatabaseMetricsIntegrationTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/db/PostgreSQL17DatabaseMetricsIntegrationTest.java index ec78f2980f..cb528d2095 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/db/PostgreSQL17DatabaseMetricsIntegrationTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/db/PostgreSQL17DatabaseMetricsIntegrationTest.java @@ -156,7 +156,7 @@ private RequiredSearch get(final String name) { } private static DockerImageName getDockerImageName() { - return DockerImageName.parse("public.ecr.aws/docker/library/postgres:17.4").asCompatibleSubstituteFor("postgres"); + return DockerImageName.parse("postgres:17"); } } From cdca04d73294abaa28528520f990608239b3c935 Mon Sep 17 00:00:00 2001 From: hari-mani Date: Mon, 27 Oct 2025 16:45:29 +0530 Subject: [PATCH 3/9] fix: faiing PostgreSQLDatabaseMetricsTest Signed-off-by: hari-mani --- .../db/PostgreSQLDatabaseMetricsTest.java | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetricsTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetricsTest.java index 5c327e370b..c455f7a787 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetricsTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetricsTest.java @@ -20,16 +20,20 @@ import io.micrometer.core.instrument.search.RequiredSearch; import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import org.junit.jupiter.api.Test; +import org.mockito.BDDMockito; import javax.sql.DataSource; +import java.sql.SQLException; import static io.micrometer.core.instrument.binder.db.PostgreSQLDatabaseMetrics.Names.*; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; /** * @author Kristof Depypere * @author Markus Dobel + * @author Hari Mani */ class PostgreSQLDatabaseMetricsTest { @@ -37,14 +41,17 @@ class PostgreSQLDatabaseMetricsTest { private static final String FUNCTIONAL_COUNTER_KEY = "key"; - private final DataSource dataSource = mock(DataSource.class); + private final DataSource dataSource = mock(DataSource.class, BDDMockito.RETURNS_DEEP_STUBS); private final MeterRegistry registry = new SimpleMeterRegistry(); private final Tags tags = Tags.of("database", DATABASE_NAME); @Test - void shouldRegisterPostgreSqlMetrics() { + void shouldRegisterPostgreSqlMetrics() throws SQLException { + // noinspection resource + given(dataSource.getConnection().createStatement().executeQuery("SHOW server_version").getString(1)) + .willReturn("0.0.0"); PostgreSQLDatabaseMetrics metrics = new PostgreSQLDatabaseMetrics(dataSource, DATABASE_NAME); metrics.bindTo(registry); @@ -72,7 +79,10 @@ void shouldRegisterPostgreSqlMetrics() { } @Test - void shouldBridgePgStatReset() { + void shouldBridgePgStatReset() throws SQLException { + // noinspection resource + given(dataSource.getConnection().createStatement().executeQuery("SHOW server_version").getString(1)) + .willReturn("0.0.0"); PostgreSQLDatabaseMetrics metrics = new PostgreSQLDatabaseMetrics(dataSource, DATABASE_NAME); metrics.bindTo(registry); @@ -87,7 +97,10 @@ void shouldBridgePgStatReset() { } @Test - void shouldBridgeDoublePgStatReset() { + void shouldBridgeDoublePgStatReset() throws SQLException { + // noinspection resource + given(dataSource.getConnection().createStatement().executeQuery("SHOW server_version").getString(1)) + .willReturn("0.0.0"); PostgreSQLDatabaseMetrics metrics = new PostgreSQLDatabaseMetrics(dataSource, DATABASE_NAME); metrics.bindTo(registry); From 5857f2cfd1816d7e10c8b370e79966c649cda1a9 Mon Sep 17 00:00:00 2001 From: hari-mani Date: Wed, 29 Oct 2025 11:52:10 +0530 Subject: [PATCH 4/9] chore: remove sys err Signed-off-by: hari-mani --- .../core/instrument/binder/db/PostgreSQLDatabaseMetrics.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java index 84fe51256a..34fa5bcef1 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java @@ -354,8 +354,7 @@ private Optional runQuery(final String query, final ResultSetGetter re return Optional.of(resultSetGetter.get(resultSet)); } } - catch (SQLException err) { - System.err.println(err.getMessage()); + catch (SQLException ignored) { } return Optional.empty(); } From dc6403041a6069eedbc70eabc45415145f6037a4 Mon Sep 17 00:00:00 2001 From: hari-mani Date: Tue, 4 Nov 2025 11:48:17 +0530 Subject: [PATCH 5/9] feat: move version condition check up to ensure condition exection does not happen everytime the supplier is invoked Signed-off-by: hari-mani --- .../binder/db/PostgreSQLDatabaseMetrics.java | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java index 34fa5bcef1..0a8e43b844 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java @@ -207,21 +207,21 @@ private void registerRowCountMetrics(MeterRegistry registry) { private void registerCheckpointMetrics(MeterRegistry registry) { FunctionCounter .builder(Names.CHECKPOINTS_TIMED, postgresDataSource, - dataSource -> resettableFunctionalCounter(Names.CHECKPOINTS_TIMED, this::getTimedCheckpointsCount)) + dataSource -> resettableFunctionalCounter(Names.CHECKPOINTS_TIMED, getTimedCheckpointsCountSupplier())) .tags(tags) .description("Number of checkpoints timed") .register(registry); FunctionCounter .builder(Names.CHECKPOINTS_REQUESTED, postgresDataSource, dataSource -> resettableFunctionalCounter(Names.CHECKPOINTS_REQUESTED, - this::getRequestedCheckpointsCount)) + getRequestedCheckpointsCountSupplier())) .tags(tags) .description("Number of checkpoints requested") .register(registry); FunctionCounter .builder(Names.BUFFERS_CHECKPOINT, postgresDataSource, - dataSource -> resettableFunctionalCounter(Names.BUFFERS_CHECKPOINT, this::getBuffersCheckpoint)) + dataSource -> resettableFunctionalCounter(Names.BUFFERS_CHECKPOINT, getBuffersCheckpointSupplier())) .tags(tags) .description("Number of buffers written during checkpoints") .register(registry); @@ -233,7 +233,7 @@ private void registerCheckpointMetrics(MeterRegistry registry) { .register(registry); FunctionCounter .builder(Names.BUFFERS_BACKEND, postgresDataSource, - dataSource -> resettableFunctionalCounter(Names.BUFFERS_BACKEND, this::getBuffersBackend)) + dataSource -> resettableFunctionalCounter(Names.BUFFERS_BACKEND, getBuffersBackendSupplier())) .tags(tags) .description("Number of buffers written directly by a backend") .register(registry); @@ -288,36 +288,36 @@ private Long getDeadTupleCount() { return runQuery(QUERY_DEAD_TUPLE_COUNT); } - private Long getTimedCheckpointsCount() { + private DoubleSupplier getTimedCheckpointsCountSupplier() { if (this.serverVersion.isAbove(Version.V17)) { - return runQuery(TIMED_CHECKPOINTS_COUNT.getQuery()); + return () -> runQuery(TIMED_CHECKPOINTS_COUNT.getQuery()); } - return runQuery(QUERY_TIMED_CHECKPOINTS_COUNT); + return () -> runQuery(QUERY_TIMED_CHECKPOINTS_COUNT); } - private Long getRequestedCheckpointsCount() { + private DoubleSupplier getRequestedCheckpointsCountSupplier() { if (this.serverVersion.isAbove(Version.V17)) { - return runQuery(REQUESTED_CHECKPOINTS_COUNT.getQuery()); + return () -> runQuery(REQUESTED_CHECKPOINTS_COUNT.getQuery()); } - return runQuery(QUERY_REQUESTED_CHECKPOINTS_COUNT); + return () -> runQuery(QUERY_REQUESTED_CHECKPOINTS_COUNT); } private Long getBuffersClean() { return runQuery(QUERY_BUFFERS_CLEAN); } - private Long getBuffersBackend() { + private DoubleSupplier getBuffersBackendSupplier() { if (this.serverVersion.isAbove(Version.V17)) { - return runQuery(BACKEND_BUFFER_WRITES.getQuery()); + return () -> runQuery(BACKEND_BUFFER_WRITES.getQuery()); } - return runQuery(QUERY_BUFFERS_BACKEND); + return () -> runQuery(QUERY_BUFFERS_BACKEND); } - private Long getBuffersCheckpoint() { + private DoubleSupplier getBuffersCheckpointSupplier() { if (this.serverVersion.isAbove(Version.V17)) { - return runQuery(CHECKPOINTER_BUFFERS_WRITTEN.getQuery()); + return () -> runQuery(CHECKPOINTER_BUFFERS_WRITTEN.getQuery()); } - return runQuery(QUERY_BUFFERS_CHECKPOINT); + return () -> runQuery(QUERY_BUFFERS_CHECKPOINT); } /** From c6e973d3544b2d4a57a0dfe2a8f58812d87dda36 Mon Sep 17 00:00:00 2001 From: hari-mani Date: Tue, 4 Nov 2025 12:00:02 +0530 Subject: [PATCH 6/9] chore: remove Stat abstraction Signed-off-by: hari-mani --- .../binder/db/PostgreSQLDatabaseMetrics.java | 37 ++++++------------- 1 file changed, 12 insertions(+), 25 deletions(-) diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java index 0a8e43b844..31b509e44d 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java @@ -58,15 +58,15 @@ public class PostgreSQLDatabaseMetrics implements MeterBinder { private static final String QUERY_BUFFERS_BACKEND = getBgWriterQuery("buffers_backend"); - private static final Stat BACKEND_BUFFER_WRITES = new Stat("pg_stat_io", "SUM(writes)"); + private static final String BACKEND_BUFFER_WRITES = SELECT + "SUM(writes) FROM pg_stat_io"; private static final String QUERY_BUFFERS_CHECKPOINT = getBgWriterQuery("buffers_checkpoint"); - private static final Stat CHECKPOINTER_BUFFERS_WRITTEN = new Stat("pg_stat_checkpointer", "buffers_written"); + private static final String CHECKPOINTER_BUFFERS_WRITTEN = getStatCheckpointerQuery("buffers_written"); - private static final Stat TIMED_CHECKPOINTS_COUNT = new Stat("pg_stat_checkpointer", "num_timed"); + private static final String TIMED_CHECKPOINTS_COUNT = getStatCheckpointerQuery("num_timed"); - private static final Stat REQUESTED_CHECKPOINTS_COUNT = new Stat("pg_stat_checkpointer", "num_requested"); + private static final String REQUESTED_CHECKPOINTS_COUNT = getStatCheckpointerQuery("num_requested"); private final String database; @@ -290,14 +290,14 @@ private Long getDeadTupleCount() { private DoubleSupplier getTimedCheckpointsCountSupplier() { if (this.serverVersion.isAbove(Version.V17)) { - return () -> runQuery(TIMED_CHECKPOINTS_COUNT.getQuery()); + return () -> runQuery(TIMED_CHECKPOINTS_COUNT); } return () -> runQuery(QUERY_TIMED_CHECKPOINTS_COUNT); } private DoubleSupplier getRequestedCheckpointsCountSupplier() { if (this.serverVersion.isAbove(Version.V17)) { - return () -> runQuery(REQUESTED_CHECKPOINTS_COUNT.getQuery()); + return () -> runQuery(REQUESTED_CHECKPOINTS_COUNT); } return () -> runQuery(QUERY_REQUESTED_CHECKPOINTS_COUNT); } @@ -308,14 +308,14 @@ private Long getBuffersClean() { private DoubleSupplier getBuffersBackendSupplier() { if (this.serverVersion.isAbove(Version.V17)) { - return () -> runQuery(BACKEND_BUFFER_WRITES.getQuery()); + return () -> runQuery(BACKEND_BUFFER_WRITES); } return () -> runQuery(QUERY_BUFFERS_BACKEND); } private DoubleSupplier getBuffersCheckpointSupplier() { if (this.serverVersion.isAbove(Version.V17)) { - return () -> runQuery(CHECKPOINTER_BUFFERS_WRITTEN.getQuery()); + return () -> runQuery(CHECKPOINTER_BUFFERS_WRITTEN); } return () -> runQuery(QUERY_BUFFERS_CHECKPOINT); } @@ -371,6 +371,10 @@ private static String getBgWriterQuery(String statName) { return SELECT + statName + " FROM pg_stat_bgwriter"; } + private static String getStatCheckpointerQuery(String statName) { + return SELECT + statName + " FROM pg_stat_checkpointer"; + } + static final class Names { static final String SIZE = of("size"); @@ -410,23 +414,6 @@ interface ResultSetGetter { } - static class Stat { - - private final String statView; - - private final String statName; - - public Stat(String statView, String statName) { - this.statView = statView; - this.statName = statName; - } - - public String getQuery() { - return String.format("SELECT %s FROM %s;", this.statName, this.statView); - } - - } - static final class Version { private static final Pattern VERSION_PATTERN = Pattern.compile("^(\\d+\\.\\d+).*"); From a345533f27fb00144b352713bdf2f9dd04d27952 Mon Sep 17 00:00:00 2001 From: hari-mani Date: Tue, 4 Nov 2025 12:06:44 +0530 Subject: [PATCH 7/9] chore: correct javadoc annotation Signed-off-by: hari-mani --- .../core/instrument/binder/db/PostgreSQLDatabaseMetrics.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java index 31b509e44d..ea54030dd3 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java @@ -40,7 +40,7 @@ * @author Jon Schneider * @author Johnny Lim * @author Markus Dobel - * @apiNote Hari Mani + * @author Hari Mani * @since 1.1.0 */ @NullMarked @@ -207,7 +207,8 @@ private void registerRowCountMetrics(MeterRegistry registry) { private void registerCheckpointMetrics(MeterRegistry registry) { FunctionCounter .builder(Names.CHECKPOINTS_TIMED, postgresDataSource, - dataSource -> resettableFunctionalCounter(Names.CHECKPOINTS_TIMED, getTimedCheckpointsCountSupplier())) + dataSource -> resettableFunctionalCounter(Names.CHECKPOINTS_TIMED, + getTimedCheckpointsCountSupplier())) .tags(tags) .description("Number of checkpoints timed") .register(registry); From 23765309dfa9930724600495f9c06f166e30ecaf Mon Sep 17 00:00:00 2001 From: hari-mani Date: Thu, 6 Nov 2025 11:32:20 +0530 Subject: [PATCH 8/9] review: address @shakuzen's comments - add a warning log when version parsing fails - rename Version#isAbove to Version#isAtLeast to reflect the condition being checked - add tests for Version Signed-off-by: hari-mani --- .../binder/db/PostgreSQLDatabaseMetrics.java | 40 +++++++++++++++---- .../db/PostgreSQLDatabaseMetricsTest.java | 37 +++++++++++++++++ 2 files changed, 70 insertions(+), 7 deletions(-) diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java index ea54030dd3..2e735da819 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java @@ -16,6 +16,8 @@ package io.micrometer.core.instrument.binder.db; +import io.micrometer.common.util.internal.logging.InternalLogger; +import io.micrometer.common.util.internal.logging.InternalLoggerFactory; import io.micrometer.core.instrument.*; import io.micrometer.core.instrument.binder.BaseUnits; import io.micrometer.core.instrument.binder.MeterBinder; @@ -27,6 +29,7 @@ import java.sql.SQLException; import java.sql.Statement; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.function.DoubleSupplier; @@ -46,6 +49,8 @@ @NullMarked public class PostgreSQLDatabaseMetrics implements MeterBinder { + private static final InternalLogger log = InternalLoggerFactory.getInstance(PostgreSQLDatabaseMetrics.class); + private static final String SELECT = "SELECT "; private static final String QUERY_DEAD_TUPLE_COUNT = getUserTableQuery("SUM(n_dead_tup)"); @@ -290,14 +295,14 @@ private Long getDeadTupleCount() { } private DoubleSupplier getTimedCheckpointsCountSupplier() { - if (this.serverVersion.isAbove(Version.V17)) { + if (this.serverVersion.isAtLeast(Version.V17)) { return () -> runQuery(TIMED_CHECKPOINTS_COUNT); } return () -> runQuery(QUERY_TIMED_CHECKPOINTS_COUNT); } private DoubleSupplier getRequestedCheckpointsCountSupplier() { - if (this.serverVersion.isAbove(Version.V17)) { + if (this.serverVersion.isAtLeast(Version.V17)) { return () -> runQuery(REQUESTED_CHECKPOINTS_COUNT); } return () -> runQuery(QUERY_REQUESTED_CHECKPOINTS_COUNT); @@ -308,14 +313,14 @@ private Long getBuffersClean() { } private DoubleSupplier getBuffersBackendSupplier() { - if (this.serverVersion.isAbove(Version.V17)) { + if (this.serverVersion.isAtLeast(Version.V17)) { return () -> runQuery(BACKEND_BUFFER_WRITES); } return () -> runQuery(QUERY_BUFFERS_BACKEND); } private DoubleSupplier getBuffersCheckpointSupplier() { - if (this.serverVersion.isAbove(Version.V17)) { + if (this.serverVersion.isAtLeast(Version.V17)) { return () -> runQuery(CHECKPOINTER_BUFFERS_WRITTEN); } return () -> runQuery(QUERY_BUFFERS_CHECKPOINT); @@ -417,7 +422,7 @@ interface ResultSetGetter { static final class Version { - private static final Pattern VERSION_PATTERN = Pattern.compile("^(\\d+\\.\\d+).*"); + private static final Pattern VERSION_PATTERN = Pattern.compile("^((?:\\d+\\.?)+).*"); static final Version EMPTY = new Version(0, 0); @@ -433,20 +438,28 @@ static Version parse(String versionString) { if (!matcher.matches()) { return EMPTY; } - final String[] versionArr = matcher.group(1).split("\\.", 2); + final String[] versionArr = matcher.group(1).split("\\.", 3); + if (versionArr.length == 1) { + return new Version(Integer.parseInt(versionArr[0])); + } return new Version(Integer.parseInt(versionArr[0]), Integer.parseInt(versionArr[1])); } catch (Exception exception) { + log.warn("Unable to parse Postgres version", exception); return EMPTY; } } + Version(int majorVersion) { + this(majorVersion, 0); + } + Version(int majorVersion, int minorVersion) { this.majorVersion = majorVersion; this.minorVersion = minorVersion; } - public boolean isAbove(final Version other) { + public boolean isAtLeast(final Version other) { if (this.majorVersion > other.majorVersion) { return true; } @@ -461,6 +474,19 @@ public String toString() { return majorVersion + "." + minorVersion; } + @Override + public boolean equals(Object o) { + if (!(o instanceof Version)) + return false; + Version version = (Version) o; + return majorVersion == version.majorVersion && minorVersion == version.minorVersion; + } + + @Override + public int hashCode() { + return Objects.hash(majorVersion, minorVersion); + } + } } diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetricsTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetricsTest.java index c455f7a787..b5214c4c80 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetricsTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetricsTest.java @@ -19,13 +19,17 @@ import io.micrometer.core.instrument.Tags; import io.micrometer.core.instrument.search.RequiredSearch; import io.micrometer.core.instrument.simple.SimpleMeterRegistry; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.mockito.BDDMockito; import javax.sql.DataSource; import java.sql.SQLException; import static io.micrometer.core.instrument.binder.db.PostgreSQLDatabaseMetrics.Names.*; +import static io.micrometer.core.instrument.binder.db.PostgreSQLDatabaseMetrics.Version; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; @@ -120,4 +124,37 @@ private RequiredSearch get(final String name) { return registry.get(name).tags(tags); } + @Nested + class VersionTest { + + @ParameterizedTest + @CsvSource({ "17.6 (Debian 17.6-2.pgdg13+1), 17, 6", "9.6.24, 9, 6", "17 (Debian 17.pgdg13+1), 17, 0" }) + void parse_shouldParseGivenVersionString(final String versionString, final int expectedMajorVersion, + final int expectedMinorVersion) { + final Version expectedVersion = new Version(expectedMajorVersion, expectedMinorVersion); + assertThat(Version.parse(versionString)).isEqualTo(expectedVersion); + } + + @Test + void parse_whenParsingFailsShouldReturnEmptyVersion() { + final Version version = Version.parse("does not match the pattern"); + assertThat(version).isEqualTo(Version.EMPTY); + } + + @ParameterizedTest + @CsvSource({ "17, 0", "17, 1", "18, 0" }) + void isAtLeast_shouldReturnTrueWhenVersionIsGreaterThanOrEqual(final int majorVersion, final int minorVersion) { + final Version version = new Version(majorVersion, minorVersion); + assertThat(version.isAtLeast(Version.V17)).isTrue(); + } + + @ParameterizedTest + @CsvSource({ "16, 9", "9, 2" }) + void isAtLeast_shouldReturnFalseWhenVersionIsLesser(final int majorVersion, final int minorVersion) { + final Version version = new Version(majorVersion, minorVersion); + assertThat(version.isAtLeast(Version.V17)).isFalse(); + } + + } + } From 6ced60bfba7f29ea58228bd495f45172e01ef871 Mon Sep 17 00:00:00 2001 From: hari-mani Date: Fri, 7 Nov 2025 11:51:40 +0530 Subject: [PATCH 9/9] chore: add warning log when version does not match expected pattern Signed-off-by: hari-mani --- .../core/instrument/binder/db/PostgreSQLDatabaseMetrics.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java index 2e735da819..64bf8c5fb9 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/db/PostgreSQLDatabaseMetrics.java @@ -436,6 +436,8 @@ static Version parse(String versionString) { try { final Matcher matcher = VERSION_PATTERN.matcher(versionString); if (!matcher.matches()) { + log.warn("Received Postgres version {} does not match the expected pattern {}", versionString, + VERSION_PATTERN.pattern()); return EMPTY; } final String[] versionArr = matcher.group(1).split("\\.", 3);