You've already forked postgres_exporter
mirror of
https://github.com/prometheus-community/postgres_exporter.git
synced 2026-10-23 12:08:57 +03:00
fix: add missing rows.Err() check in pg_stat_database.go (#1399)
Ensure any row preparation errors are validated by returning rows.Err() after the rows.Next() loop finishes in collector/pg_stat_database. Added a unit test and refactored the way functions return rows.Err() within the codebase for consistency. Signed-off-by: chief librarian <323671055+chieflibrarian@users.noreply.github.com>
This commit is contained in:
@@ -135,5 +135,6 @@ func (c PGDatabaseCollector) Update(ctx context.Context, instance *instance, ch
|
||||
)
|
||||
|
||||
}
|
||||
|
||||
return rows.Err()
|
||||
}
|
||||
|
||||
@@ -105,8 +105,6 @@ func (c *PGDatabaseWraparoundCollector) Update(ctx context.Context, instance *in
|
||||
ageDatminmxidMetric, datname.String,
|
||||
)
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
|
||||
return rows.Err()
|
||||
}
|
||||
|
||||
@@ -120,8 +120,6 @@ func (c PGLocksCollector) Update(ctx context.Context, instance *instance, ch cha
|
||||
datname.String, mode.String,
|
||||
)
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
|
||||
return rows.Err()
|
||||
}
|
||||
|
||||
@@ -110,8 +110,6 @@ func (c PGLongRunningTransactionsCollector) Update(ctx context.Context, instance
|
||||
age,
|
||||
)
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
|
||||
return rows.Err()
|
||||
}
|
||||
|
||||
@@ -280,6 +280,7 @@ func updateReplicationSlotsSlotMetrics(ctx context.Context, instance *instance,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
return rows.Err()
|
||||
}
|
||||
|
||||
|
||||
@@ -81,8 +81,6 @@ func (c PGStatActivityAutovacuumCollector) Update(ctx context.Context, instance
|
||||
ageInSeconds.Float64, relname,
|
||||
)
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
|
||||
return rows.Err()
|
||||
}
|
||||
|
||||
@@ -251,9 +251,7 @@ func (c *PGStatDatabaseCollector) Update(ctx context.Context, instance *instance
|
||||
columns = append(columns, "active_time")
|
||||
}
|
||||
|
||||
rows, err := db.QueryContext(ctx,
|
||||
statDatabaseQuery(columns),
|
||||
)
|
||||
rows, err := db.QueryContext(ctx, statDatabaseQuery(columns))
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -511,5 +509,6 @@ func (c *PGStatDatabaseCollector) Update(ctx context.Context, instance *instance
|
||||
)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
|
||||
return rows.Err()
|
||||
}
|
||||
|
||||
@@ -14,6 +14,7 @@ package collector
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -25,16 +26,8 @@ import (
|
||||
"github.com/smartystreets/goconvey/convey"
|
||||
)
|
||||
|
||||
func TestPGStatDatabaseCollector(t *testing.T) {
|
||||
db, mock, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("Error opening a stub db connection: %s", err)
|
||||
}
|
||||
defer db.Close()
|
||||
|
||||
inst := &instance{db: db, version: semver.MustParse("14.0.0")}
|
||||
|
||||
columns := []string{
|
||||
var (
|
||||
pgStatCollectorTestColumns = []string{
|
||||
"datid",
|
||||
"datname",
|
||||
"numbackends",
|
||||
@@ -56,13 +49,23 @@ func TestPGStatDatabaseCollector(t *testing.T) {
|
||||
"stats_reset",
|
||||
"active_time",
|
||||
}
|
||||
)
|
||||
|
||||
func TestPGStatDatabaseCollector(t *testing.T) {
|
||||
db, mock, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("Error opening a stub db connection: %s", err)
|
||||
}
|
||||
defer db.Close()
|
||||
|
||||
inst := &instance{db: db, version: semver.MustParse("14.0.0")}
|
||||
|
||||
srT, err := time.Parse("2006-01-02 15:04:05.00000-07", "2023-05-25 17:10:42.81132-07")
|
||||
if err != nil {
|
||||
t.Fatalf("Error parsing time: %s", err)
|
||||
}
|
||||
|
||||
rows := sqlmock.NewRows(columns).
|
||||
rows := sqlmock.NewRows(pgStatCollectorTestColumns).
|
||||
AddRow(
|
||||
"pid",
|
||||
"postgres",
|
||||
@@ -86,7 +89,7 @@ func TestPGStatDatabaseCollector(t *testing.T) {
|
||||
33,
|
||||
)
|
||||
|
||||
mock.ExpectQuery(sanitizeQuery(statDatabaseQuery(columns))).WillReturnRows(rows)
|
||||
mock.ExpectQuery(sanitizeQuery(statDatabaseQuery(pgStatCollectorTestColumns))).WillReturnRows(rows)
|
||||
|
||||
ch := make(chan prometheus.Metric)
|
||||
go func() {
|
||||
@@ -145,30 +148,7 @@ func TestPGStatDatabaseCollectorNullValues(t *testing.T) {
|
||||
}
|
||||
inst := &instance{db: db, version: semver.MustParse("14.0.0")}
|
||||
|
||||
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",
|
||||
"active_time",
|
||||
}
|
||||
|
||||
rows := sqlmock.NewRows(columns).
|
||||
rows := sqlmock.NewRows(pgStatCollectorTestColumns).
|
||||
AddRow(
|
||||
nil,
|
||||
"postgres",
|
||||
@@ -213,7 +193,7 @@ func TestPGStatDatabaseCollectorNullValues(t *testing.T) {
|
||||
srT,
|
||||
32,
|
||||
)
|
||||
mock.ExpectQuery(sanitizeQuery(statDatabaseQuery(columns))).WillReturnRows(rows)
|
||||
mock.ExpectQuery(sanitizeQuery(statDatabaseQuery(pgStatCollectorTestColumns))).WillReturnRows(rows)
|
||||
|
||||
ch := make(chan prometheus.Metric)
|
||||
go func() {
|
||||
@@ -267,35 +247,12 @@ func TestPGStatDatabaseCollectorRowLeakTest(t *testing.T) {
|
||||
|
||||
inst := &instance{db: db, version: semver.MustParse("14.0.0")}
|
||||
|
||||
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",
|
||||
"active_time",
|
||||
}
|
||||
|
||||
srT, err := time.Parse("2006-01-02 15:04:05.00000-07", "2023-05-25 17:10:42.81132-07")
|
||||
if err != nil {
|
||||
t.Fatalf("Error parsing time: %s", err)
|
||||
}
|
||||
|
||||
rows := sqlmock.NewRows(columns).
|
||||
rows := sqlmock.NewRows(pgStatCollectorTestColumns).
|
||||
AddRow(
|
||||
"pid",
|
||||
"postgres",
|
||||
@@ -362,7 +319,7 @@ func TestPGStatDatabaseCollectorRowLeakTest(t *testing.T) {
|
||||
srT,
|
||||
15,
|
||||
)
|
||||
mock.ExpectQuery(sanitizeQuery(statDatabaseQuery(columns))).WillReturnRows(rows)
|
||||
mock.ExpectQuery(sanitizeQuery(statDatabaseQuery(pgStatCollectorTestColumns))).WillReturnRows(rows)
|
||||
|
||||
ch := make(chan prometheus.Metric)
|
||||
go func() {
|
||||
@@ -436,30 +393,7 @@ func TestPGStatDatabaseCollectorTestNilStatReset(t *testing.T) {
|
||||
|
||||
inst := &instance{db: db, version: semver.MustParse("14.0.0")}
|
||||
|
||||
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",
|
||||
"active_time",
|
||||
}
|
||||
|
||||
rows := sqlmock.NewRows(columns).
|
||||
rows := sqlmock.NewRows(pgStatCollectorTestColumns).
|
||||
AddRow(
|
||||
"pid",
|
||||
"postgres",
|
||||
@@ -483,7 +417,7 @@ func TestPGStatDatabaseCollectorTestNilStatReset(t *testing.T) {
|
||||
7,
|
||||
)
|
||||
|
||||
mock.ExpectQuery(sanitizeQuery(statDatabaseQuery(columns))).WillReturnRows(rows)
|
||||
mock.ExpectQuery(sanitizeQuery(statDatabaseQuery(pgStatCollectorTestColumns))).WillReturnRows(rows)
|
||||
|
||||
ch := make(chan prometheus.Metric)
|
||||
go func() {
|
||||
@@ -528,3 +462,60 @@ func TestPGStatDatabaseCollectorTestNilStatReset(t *testing.T) {
|
||||
t.Errorf("there were unfulfilled exceptions: %s", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPGStatDatabaseCollectorRowsError(t *testing.T) {
|
||||
db, mock, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("Error opening a stub db connection: %s", err)
|
||||
}
|
||||
defer db.Close()
|
||||
|
||||
inst := &instance{db: db, version: semver.MustParse("14.0.0")}
|
||||
|
||||
expectedRowErr := errors.New("test row error")
|
||||
rows := sqlmock.NewRows(pgStatCollectorTestColumns).
|
||||
AddRow(
|
||||
"pid",
|
||||
"postgres",
|
||||
354,
|
||||
4945,
|
||||
289097744,
|
||||
1242257,
|
||||
int64(3275602074),
|
||||
89320867,
|
||||
450139,
|
||||
2034563757,
|
||||
0,
|
||||
int64(2725688749),
|
||||
23,
|
||||
52,
|
||||
74,
|
||||
925,
|
||||
16,
|
||||
823,
|
||||
nil,
|
||||
7,
|
||||
).RowError(0, expectedRowErr)
|
||||
|
||||
mock.ExpectQuery(sanitizeQuery(statDatabaseQuery(pgStatCollectorTestColumns))).WillReturnRows(rows)
|
||||
|
||||
convey.Convey("With a buffered channel and valid collector", t, func() {
|
||||
ch := make(chan prometheus.Metric, 1)
|
||||
defer close(ch)
|
||||
|
||||
c := PGStatDatabaseCollector{
|
||||
log: promslog.NewNopLogger().With("collector", "pg_stat_database"),
|
||||
}
|
||||
|
||||
convey.Convey("When doing an Update", func() {
|
||||
err := c.Update(context.Background(), inst, ch)
|
||||
|
||||
convey.Convey("It should return expected row error, with the channel remaining empty", func() {
|
||||
convey.So(err, convey.ShouldBeError, expectedRowErr)
|
||||
convey.So(ch, convey.ShouldBeEmpty)
|
||||
|
||||
convey.So(mock.ExpectationsWereMet(), convey.ShouldBeNil)
|
||||
})
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
@@ -220,8 +220,5 @@ func (c *PGStatProgressVacuumCollector) Update(ctx context.Context, instance *in
|
||||
ch <- prometheus.MustNewConstMetric(statProgressVacuumNumDeadTuples, prometheus.GaugeValue, numDead, labels...)
|
||||
}
|
||||
|
||||
if err := rows.Err(); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
return rows.Err()
|
||||
}
|
||||
|
||||
@@ -326,10 +326,8 @@ func (c PGStatStatementsCollector) Update(ctx context.Context, instance *instanc
|
||||
}
|
||||
}
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
|
||||
return rows.Err()
|
||||
}
|
||||
|
||||
func (c PGStatStatementsCollector) buildExclusionClause(identifiers []string, clauseTemplate string) string {
|
||||
|
||||
@@ -419,8 +419,5 @@ func (c *PGStatUserTablesCollector) Update(ctx context.Context, instance *instan
|
||||
)
|
||||
}
|
||||
|
||||
if err := rows.Err(); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
return rows.Err()
|
||||
}
|
||||
|
||||
@@ -261,8 +261,6 @@ func (c *PGStatWalReceiverCollector) Update(ctx context.Context, instance *insta
|
||||
labels...)
|
||||
}
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
|
||||
return rows.Err()
|
||||
}
|
||||
|
||||
@@ -103,8 +103,6 @@ func (c *PGStatioUserIndexesCollector) Update(ctx context.Context, instance *ins
|
||||
labels...,
|
||||
)
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
|
||||
return rows.Err()
|
||||
}
|
||||
|
||||
@@ -192,5 +192,6 @@ func (PGStatIOUserTablesCollector) Update(ctx context.Context, instance *instanc
|
||||
datnameLabel, schemanameLabel, relnameLabel,
|
||||
)
|
||||
}
|
||||
|
||||
return rows.Err()
|
||||
}
|
||||
|
||||
@@ -81,8 +81,6 @@ func (c PGXlogLocationCollector) Update(ctx context.Context, instance *instance,
|
||||
bytes,
|
||||
)
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
|
||||
return rows.Err()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user