From 9e5a7047c11bbaed831e1f16f6e87a1fd49aaa1e Mon Sep 17 00:00:00 2001 From: Felix Yuan Date: Thu, 27 Jul 2023 11:15:45 -0700 Subject: [PATCH 1/5] Stats_reset as null seems to actually be legitimate for new databases, so don't fail for it Signed-off-by: Felix Yuan --- collector/pg_stat_database.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/collector/pg_stat_database.go b/collector/pg_stat_database.go index 382ff7825..995cb5030 100644 --- a/collector/pg_stat_database.go +++ b/collector/pg_stat_database.go @@ -344,9 +344,11 @@ 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 } + + statsResetMetric := 0.0 if !statsReset.Valid { level.Debug(c.log).Log("msg", "Skipping collecting metric because it has no stats_reset") - continue + statsResetMetric = float64(statsReset.Time.Unix()), } labels := []string{datid.String, datname.String} @@ -466,7 +468,7 @@ func (c *PGStatDatabaseCollector) Update(ctx context.Context, instance *instance ch <- prometheus.MustNewConstMetric( statDatabaseStatsReset, prometheus.CounterValue, - float64(statsReset.Time.Unix()), + statsResetMetric, labels..., ) } From 16212bf147fb6a78096149b85cfed4f8bb88b38b Mon Sep 17 00:00:00 2001 From: Felix Yuan Date: Thu, 27 Jul 2023 11:21:30 -0700 Subject: [PATCH 2/5] Fix up stat database test Signed-off-by: Felix Yuan --- collector/pg_stat_database.go | 6 +- collector/pg_stat_database_test.go | 98 ++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 2 deletions(-) diff --git a/collector/pg_stat_database.go b/collector/pg_stat_database.go index 995cb5030..4c720e3a8 100644 --- a/collector/pg_stat_database.go +++ b/collector/pg_stat_database.go @@ -345,10 +345,12 @@ func (c *PGStatDatabaseCollector) Update(ctx context.Context, instance *instance continue } - statsResetMetric := 0.0 + var statsResetMetric float64 if !statsReset.Valid { level.Debug(c.log).Log("msg", "Skipping collecting metric because it has no stats_reset") - statsResetMetric = float64(statsReset.Time.Unix()), + statsResetMetric = 0.0 + } else { + statsResetMetric = float64(statsReset.Time.Unix()) } labels := []string{datid.String, datname.String} diff --git a/collector/pg_stat_database_test.go b/collector/pg_stat_database_test.go index 70c73eb5b..fe1b17066 100644 --- a/collector/pg_stat_database_test.go +++ b/collector/pg_stat_database_test.go @@ -406,3 +406,101 @@ func TestPGStatDatabaseCollectorRowLeakTest(t *testing.T) { t.Errorf("there were unfulfilled exceptions: %s", err) } } + +func TestPGStatDatabaseCollectorTestNilStatReset(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} + + 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", + } + + rows := sqlmock.NewRows(columns). + AddRow( + "pid", + "postgres", + 354, + 4945, + 289097744, + 1242257, + int64(3275602074), + 89320867, + 450139, + 2034563757, + 0, + int64(2725688749), + 23, + 52, + 74, + 925, + 16, + 823, + nil) + + mock.ExpectQuery(sanitizeQuery(statDatabaseQuery)).WillReturnRows(rows) + + ch := make(chan prometheus.Metric) + go func() { + defer close(ch) + c := PGStatDatabaseCollector{ + log: log.With(log.NewNopLogger(), "collector", "pg_stat_database"), + } + + if err := c.Update(context.Background(), inst, ch); err != nil { + t.Errorf("Error calling PGStatDatabaseCollector.Update: %s", err) + } + }() + + expected := []MetricResult{ + {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_GAUGE, value: 354}, + {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 4945}, + {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 289097744}, + {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 1242257}, + {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 3275602074}, + {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 89320867}, + {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 450139}, + {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 2034563757}, + {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 0}, + {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 2725688749}, + {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 23}, + {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 52}, + {labels: labelMap{"datid": "pid", "datname": "postgres"}, metricType: dto.MetricType_COUNTER, value: 74}, + {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}, + } + + convey.Convey("Metrics comparison", t, func() { + for _, expect := range expected { + m := readMetric(<-ch) + convey.So(expect, convey.ShouldResemble, m) + } + }) + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("there were unfulfilled exceptions: %s", err) + } +} From d0b0c74a93b3b69e6e913a71b439974838e83994 Mon Sep 17 00:00:00 2001 From: Felix Yuan Date: Thu, 27 Jul 2023 15:47:23 -0700 Subject: [PATCH 3/5] Update collector/pg_stat_database.go Co-authored-by: Ben Kochie Signed-off-by: Felix Yuan --- collector/pg_stat_database.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/collector/pg_stat_database.go b/collector/pg_stat_database.go index 4c720e3a8..ca8dfd58e 100644 --- a/collector/pg_stat_database.go +++ b/collector/pg_stat_database.go @@ -348,8 +348,7 @@ func (c *PGStatDatabaseCollector) Update(ctx context.Context, instance *instance var statsResetMetric float64 if !statsReset.Valid { level.Debug(c.log).Log("msg", "Skipping collecting metric because it has no stats_reset") - statsResetMetric = 0.0 - } else { + if statsReset.Valid { statsResetMetric = float64(statsReset.Time.Unix()) } From 1f2560028217b6139a8819e4c69647ff5fd37f34 Mon Sep 17 00:00:00 2001 From: Felix Yuan Date: Thu, 27 Jul 2023 15:47:46 -0700 Subject: [PATCH 4/5] Update collector/pg_stat_database.go Co-authored-by: Ben Kochie Signed-off-by: Felix Yuan --- collector/pg_stat_database.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/collector/pg_stat_database.go b/collector/pg_stat_database.go index ca8dfd58e..d7a68dc0e 100644 --- a/collector/pg_stat_database.go +++ b/collector/pg_stat_database.go @@ -345,7 +345,7 @@ func (c *PGStatDatabaseCollector) Update(ctx context.Context, instance *instance continue } - var statsResetMetric float64 + statsResetMetric := 0.0 if !statsReset.Valid { level.Debug(c.log).Log("msg", "Skipping collecting metric because it has no stats_reset") if statsReset.Valid { From d398d70f0d253fae72e1fe1a9cc1622114f3a3ba Mon Sep 17 00:00:00 2001 From: Felix Yuan Date: Fri, 28 Jul 2023 15:53:14 -0700 Subject: [PATCH 5/5] Send a better message to debug Signed-off-by: Felix Yuan --- collector/pg_stat_database.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/collector/pg_stat_database.go b/collector/pg_stat_database.go index d7a68dc0e..328afee2c 100644 --- a/collector/pg_stat_database.go +++ b/collector/pg_stat_database.go @@ -347,7 +347,8 @@ func (c *PGStatDatabaseCollector) Update(ctx context.Context, instance *instance statsResetMetric := 0.0 if !statsReset.Valid { - level.Debug(c.log).Log("msg", "Skipping collecting metric because it has no stats_reset") + level.Debug(c.log).Log("msg", "No metric for stats_reset, will collect 0 instead") + } if statsReset.Valid { statsResetMetric = float64(statsReset.Time.Unix()) }