From 678eaf84c44bd149424aa6b718106cc6ee438385 Mon Sep 17 00:00:00 2001 From: MarcWort <113890636+MarcWort@users.noreply.github.com> Date: Fri, 14 Jun 2024 12:40:27 +0200 Subject: [PATCH] fix: Only query active_time on pg>=14 --- collector/pg_stat_database.go | 91 ++++++++++++++++++------------ collector/pg_stat_database_test.go | 53 +++++++++-------- 2 files changed, 84 insertions(+), 60 deletions(-) diff --git a/collector/pg_stat_database.go b/collector/pg_stat_database.go index ea7075303..b3cb59eb2 100644 --- a/collector/pg_stat_database.go +++ b/collector/pg_stat_database.go @@ -16,7 +16,10 @@ package collector import ( "context" "database/sql" + "fmt" + "strings" + "github.com/blang/semver/v4" "github.com/go-kit/log" "github.com/go-kit/log/level" "github.com/prometheus/client_golang/prometheus" @@ -215,37 +218,44 @@ var ( []string{"datid", "datname"}, prometheus.Labels{}, ) - - statDatabaseQuery = ` - SELECT - datid - ,datname - ,numbackends - ,xact_commit - ,xact_rollback - ,blks_read - ,blks_hit - ,tup_returned - ,tup_fetched - ,tup_inserted - ,tup_updated - ,tup_deleted - ,conflicts - ,temp_files - ,temp_bytes - ,deadlocks - ,blk_read_time - ,blk_write_time - ,active_time - ,stats_reset - FROM pg_stat_database; - ` ) +func statDatabaseQuery(columns []string) string { + return fmt.Sprintf("SELECT %s FROM pg_stat_database;", strings.Join(columns, ",")) +} + func (c *PGStatDatabaseCollector) Update(ctx context.Context, instance *instance, ch chan<- prometheus.Metric) error { db := instance.getDB() + + columns := []string{ + "datid", + "datname", + "numbackends", + "xact_commit", + "xact_rollback", + "blks_read", + "blks_hit", + "tup_returned", + "tup_fetched", + "tup_inserted", + "tup_updated", + "tup_deleted", + "conflicts", + "temp_files", + "temp_bytes", + "deadlocks", + "blk_read_time", + "blk_write_time", + "stats_reset", + } + + activeTimeAvail := instance.version.GTE(semver.MustParse("14.0.0")) + if activeTimeAvail { + columns = append(columns, "active_time") + } + rows, err := db.QueryContext(ctx, - statDatabaseQuery, + statDatabaseQuery(columns), ) if err != nil { return err @@ -257,7 +267,7 @@ func (c *PGStatDatabaseCollector) Update(ctx context.Context, instance *instance var numBackends, xactCommit, xactRollback, blksRead, blksHit, tupReturned, tupFetched, tupInserted, tupUpdated, tupDeleted, conflicts, tempFiles, tempBytes, deadlocks, blkReadTime, blkWriteTime, activeTime sql.NullFloat64 var statsReset sql.NullTime - err := rows.Scan( + r := []any{ &datid, &datname, &numBackends, @@ -276,9 +286,14 @@ func (c *PGStatDatabaseCollector) Update(ctx context.Context, instance *instance &deadlocks, &blkReadTime, &blkWriteTime, - &activeTime, &statsReset, - ) + } + + if activeTimeAvail { + r = append(r, &activeTime) + } + + err := rows.Scan(r...) if err != nil { return err } @@ -355,7 +370,7 @@ func (c *PGStatDatabaseCollector) Update(ctx context.Context, instance *instance level.Debug(c.log).Log("msg", "Skipping collecting metric because it has no blk_write_time") continue } - if !activeTime.Valid { + if activeTimeAvail && !activeTime.Valid { level.Debug(c.log).Log("msg", "Skipping collecting metric because it has no active_time") continue } @@ -482,19 +497,21 @@ func (c *PGStatDatabaseCollector) Update(ctx context.Context, instance *instance labels..., ) - ch <- prometheus.MustNewConstMetric( - statDatabaseActiveTime, - prometheus.CounterValue, - activeTime.Float64/1000.0, - labels..., - ) - ch <- prometheus.MustNewConstMetric( statDatabaseStatsReset, prometheus.CounterValue, statsResetMetric, labels..., ) + + if activeTimeAvail { + ch <- prometheus.MustNewConstMetric( + statDatabaseActiveTime, + prometheus.CounterValue, + activeTime.Float64/1000.0, + labels..., + ) + } } return nil } diff --git a/collector/pg_stat_database_test.go b/collector/pg_stat_database_test.go index 2d4c25cf4..924612082 100644 --- a/collector/pg_stat_database_test.go +++ b/collector/pg_stat_database_test.go @@ -18,6 +18,7 @@ import ( "time" "github.com/DATA-DOG/go-sqlmock" + "github.com/blang/semver/v4" "github.com/go-kit/log" "github.com/prometheus/client_golang/prometheus" dto "github.com/prometheus/client_model/go" @@ -31,7 +32,7 @@ func TestPGStatDatabaseCollector(t *testing.T) { } defer db.Close() - inst := &instance{db: db} + inst := &instance{db: db, version: semver.MustParse("14.0.0")} columns := []string{ "datid", @@ -52,8 +53,8 @@ func TestPGStatDatabaseCollector(t *testing.T) { "deadlocks", "blk_read_time", "blk_write_time", - "active_time", "stats_reset", + "active_time", } srT, err := time.Parse("2006-01-02 15:04:05.00000-07", "2023-05-25 17:10:42.81132-07") @@ -81,10 +82,11 @@ func TestPGStatDatabaseCollector(t *testing.T) { 925, 16, 823, + srT, 33, - srT) + ) - mock.ExpectQuery(sanitizeQuery(statDatabaseQuery)).WillReturnRows(rows) + mock.ExpectQuery(sanitizeQuery(statDatabaseQuery(columns))).WillReturnRows(rows) ch := make(chan prometheus.Metric) go func() { @@ -115,8 +117,8 @@ func TestPGStatDatabaseCollector(t *testing.T) { {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 925}, {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 16}, {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 823}, - {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 0.033}, {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 1685059842}, + {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 0.033}, } convey.Convey("Metrics comparison", t, func() { @@ -141,7 +143,7 @@ func TestPGStatDatabaseCollectorNullValues(t *testing.T) { if err != nil { t.Fatalf("Error parsing time: %s", err) } - inst := &instance{db: db} + inst := &instance{db: db, version: semver.MustParse("14.0.0")} columns := []string{ "datid", @@ -162,8 +164,8 @@ func TestPGStatDatabaseCollectorNullValues(t *testing.T) { "deadlocks", "blk_read_time", "blk_write_time", - "active_time", "stats_reset", + "active_time", } rows := sqlmock.NewRows(columns). @@ -186,8 +188,9 @@ func TestPGStatDatabaseCollectorNullValues(t *testing.T) { 925, 16, 823, + srT, 32, - srT). + ). AddRow( "pid", "postgres", @@ -207,9 +210,10 @@ func TestPGStatDatabaseCollectorNullValues(t *testing.T) { 925, 16, 823, + srT, 32, - srT) - mock.ExpectQuery(sanitizeQuery(statDatabaseQuery)).WillReturnRows(rows) + ) + mock.ExpectQuery(sanitizeQuery(statDatabaseQuery(columns))).WillReturnRows(rows) ch := make(chan prometheus.Metric) go func() { @@ -240,8 +244,8 @@ func TestPGStatDatabaseCollectorNullValues(t *testing.T) { {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 925}, {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 16}, {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 823}, - {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 0.032}, {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 1685059842}, + {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 0.032}, } convey.Convey("Metrics comparison", t, func() { @@ -261,7 +265,7 @@ func TestPGStatDatabaseCollectorRowLeakTest(t *testing.T) { } defer db.Close() - inst := &instance{db: db} + inst := &instance{db: db, version: semver.MustParse("14.0.0")} columns := []string{ "datid", @@ -282,8 +286,8 @@ func TestPGStatDatabaseCollectorRowLeakTest(t *testing.T) { "deadlocks", "blk_read_time", "blk_write_time", - "active_time", "stats_reset", + "active_time", } srT, err := time.Parse("2006-01-02 15:04:05.00000-07", "2023-05-25 17:10:42.81132-07") @@ -311,8 +315,9 @@ func TestPGStatDatabaseCollectorRowLeakTest(t *testing.T) { 925, 16, 823, + srT, 14, - srT). + ). AddRow( nil, nil, @@ -354,9 +359,10 @@ func TestPGStatDatabaseCollectorRowLeakTest(t *testing.T) { 926, 17, 824, + srT, 15, - srT) - mock.ExpectQuery(sanitizeQuery(statDatabaseQuery)).WillReturnRows(rows) + ) + mock.ExpectQuery(sanitizeQuery(statDatabaseQuery(columns))).WillReturnRows(rows) ch := make(chan prometheus.Metric) go func() { @@ -387,8 +393,8 @@ func TestPGStatDatabaseCollectorRowLeakTest(t *testing.T) { {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 925}, {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 16}, {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 823}, - {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 0.014}, {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 1685059842}, + {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 0.014}, {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_GAUGE, value: 355}, {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 4946}, @@ -406,8 +412,8 @@ func TestPGStatDatabaseCollectorRowLeakTest(t *testing.T) { {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 926}, {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 17}, {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 824}, - {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 0.015}, {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 1685059842}, + {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 0.015}, } convey.Convey("Metrics comparison", t, func() { @@ -428,7 +434,7 @@ func TestPGStatDatabaseCollectorTestNilStatReset(t *testing.T) { } defer db.Close() - inst := &instance{db: db} + inst := &instance{db: db, version: semver.MustParse("14.0.0")} columns := []string{ "datid", @@ -449,8 +455,8 @@ func TestPGStatDatabaseCollectorTestNilStatReset(t *testing.T) { "deadlocks", "blk_read_time", "blk_write_time", - "active_time", "stats_reset", + "active_time", } rows := sqlmock.NewRows(columns). @@ -473,10 +479,11 @@ func TestPGStatDatabaseCollectorTestNilStatReset(t *testing.T) { 925, 16, 823, + nil, 7, - nil) + ) - mock.ExpectQuery(sanitizeQuery(statDatabaseQuery)).WillReturnRows(rows) + mock.ExpectQuery(sanitizeQuery(statDatabaseQuery(columns))).WillReturnRows(rows) ch := make(chan prometheus.Metric) go func() { @@ -507,8 +514,8 @@ func TestPGStatDatabaseCollectorTestNilStatReset(t *testing.T) { {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 925}, {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 16}, {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 823}, - {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 0.007}, {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 0}, + {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 0.007}, } convey.Convey("Metrics comparison", t, func() {