Skip to content

Commit 8d087f2

Browse files
SticksmanSuperQ
Sticksman
and
SuperQ
authored
Bug fix: Make collector not fail on null values (#823)
* Make all values nullable --------- Signed-off-by: Felix Yuan <[email protected]> Co-authored-by: Ben Kochie <[email protected]>
1 parent 6290786 commit 8d087f2

17 files changed

+1326
-244
lines changed

‎collector/pg_database.go‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ package collector
1515

1616
import (
1717
"context"
18+
"database/sql"
1819

1920
"github.com/go-kit/log"
2021
"github.com/prometheus/client_golang/prometheus"
@@ -79,32 +80,39 @@ func (c PGDatabaseCollector) Update(ctx context.Context, instance *instance, ch
7980
var databases []string
8081

8182
for rows.Next() {
82-
var datname string
83+
var datname sql.NullString
8384
if err := rows.Scan(&datname); err != nil {
8485
return err
8586
}
8687

88+
if !datname.Valid {
89+
continue
90+
}
8791
// Ignore excluded databases
8892
// Filtering is done here instead of in the query to avoid
8993
// a complicated NOT IN query with a variable number of parameters
90-
if sliceContains(c.excludedDatabases, datname) {
94+
if sliceContains(c.excludedDatabases, datname.String) {
9195
continue
9296
}
9397

94-
databases = append(databases, datname)
98+
databases = append(databases, datname.String)
9599
}
96100

97101
// Query the size of the databases
98102
for _, datname := range databases {
99-
var size int64
103+
var size sql.NullFloat64
100104
err = db.QueryRowContext(ctx, pgDatabaseSizeQuery, datname).Scan(&size)
101105
if err != nil {
102106
return err
103107
}
104108

109+
sizeMetric := 0.0
110+
if size.Valid {
111+
sizeMetric = size.Float64
112+
}
105113
ch <- prometheus.MustNewConstMetric(
106114
pgDatabaseSizeDesc,
107-
prometheus.GaugeValue, float64(size), datname,
115+
prometheus.GaugeValue, sizeMetric, datname,
108116
)
109117
}
110118
if err := rows.Err(); err != nil {

‎collector/pg_database_test.go‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,3 +59,43 @@ func TestPGDatabaseCollector(t *testing.T) {
5959
t.Errorf("there were unfulfilled exceptions: %s", err)
6060
}
6161
}
62+
63+
// TODO add a null db test
64+
65+
func TestPGDatabaseCollectorNullMetric(t *testing.T) {
66+
db, mock, err := sqlmock.New()
67+
if err != nil {
68+
t.Fatalf("Error opening a stub db connection: %s", err)
69+
}
70+
defer db.Close()
71+
72+
inst := &instance{db: db}
73+
74+
mock.ExpectQuery(sanitizeQuery(pgDatabaseQuery)).WillReturnRows(sqlmock.NewRows([]string{"datname"}).
75+
AddRow("postgres"))
76+
77+
mock.ExpectQuery(sanitizeQuery(pgDatabaseSizeQuery)).WithArgs("postgres").WillReturnRows(sqlmock.NewRows([]string{"pg_database_size"}).
78+
AddRow(nil))
79+
80+
ch := make(chan prometheus.Metric)
81+
go func() {
82+
defer close(ch)
83+
c := PGDatabaseCollector{}
84+
if err := c.Update(context.Background(), inst, ch); err != nil {
85+
t.Errorf("Error calling PGDatabaseCollector.Update: %s", err)
86+
}
87+
}()
88+
89+
expected := []MetricResult{
90+
{labels: labelMap{"datname": "postgres"}, value: 0, metricType: dto.MetricType_GAUGE},
91+
}
92+
convey.Convey("Metrics comparison", t, func() {
93+
for _, expect := range expected {
94+
m := readMetric(<-ch)
95+
convey.So(expect, convey.ShouldResemble, m)
96+
}
97+
})
98+
if err := mock.ExpectationsWereMet(); err != nil {
99+
t.Errorf("there were unfulfilled exceptions: %s", err)
100+
}
101+
}

‎collector/pg_postmaster.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ package collector
1515

1616
import (
1717
"context"
18+
"database/sql"
1819

1920
"github.com/prometheus/client_golang/prometheus"
2021
)
@@ -51,14 +52,18 @@ func (c *PGPostmasterCollector) Update(ctx context.Context, instance *instance,
5152
row := db.QueryRowContext(ctx,
5253
pgPostmasterQuery)
5354

54-
var startTimeSeconds float64
55+
var startTimeSeconds sql.NullFloat64
5556
err := row.Scan(&startTimeSeconds)
5657
if err != nil {
5758
return err
5859
}
60+
startTimeSecondsMetric := 0.0
61+
if startTimeSeconds.Valid {
62+
startTimeSecondsMetric = startTimeSeconds.Float64
63+
}
5964
ch <- prometheus.MustNewConstMetric(
6065
pgPostMasterStartTimeSeconds,
61-
prometheus.GaugeValue, startTimeSeconds,
66+
prometheus.GaugeValue, startTimeSecondsMetric,
6267
)
6368
return nil
6469
}

‎collector/pg_postmaster_test.go‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,39 @@ func TestPgPostmasterCollector(t *testing.T) {
5757
t.Errorf("there were unfulfilled exceptions: %s", err)
5858
}
5959
}
60+
61+
func TestPgPostmasterCollectorNullTime(t *testing.T) {
62+
db, mock, err := sqlmock.New()
63+
if err != nil {
64+
t.Fatalf("Error opening a stub db connection: %s", err)
65+
}
66+
defer db.Close()
67+
68+
inst := &instance{db: db}
69+
70+
mock.ExpectQuery(sanitizeQuery(pgPostmasterQuery)).WillReturnRows(sqlmock.NewRows([]string{"pg_postmaster_start_time"}).
71+
AddRow(nil))
72+
73+
ch := make(chan prometheus.Metric)
74+
go func() {
75+
defer close(ch)
76+
c := PGPostmasterCollector{}
77+
78+
if err := c.Update(context.Background(), inst, ch); err != nil {
79+
t.Errorf("Error calling PGPostmasterCollector.Update: %s", err)
80+
}
81+
}()
82+
83+
expected := []MetricResult{
84+
{labels: labelMap{}, value: 0, metricType: dto.MetricType_GAUGE},
85+
}
86+
convey.Convey("Metrics comparison", t, func() {
87+
for _, expect := range expected {
88+
m := readMetric(<-ch)
89+
convey.So(expect, convey.ShouldResemble, m)
90+
}
91+
})
92+
if err := mock.ExpectationsWereMet(); err != nil {
93+
t.Errorf("there were unfulfilled exceptions: %s", err)
94+
}
95+
}

‎collector/pg_process_idle.go‎

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,23 @@ package collector
1515

1616
import (
1717
"context"
18+
"database/sql"
1819

1920
"github.com/go-kit/log"
2021
"github.com/prometheus/client_golang/prometheus"
2122
)
2223

23-
const processIdleSubsystem = "process_idle"
24-
2524
func init() {
26-
registerCollector(processIdleSubsystem, defaultEnabled, NewPGProcessIdleCollector)
25+
// Making this default disabled because we have no tests for it
26+
registerCollector(processIdleSubsystem, defaultDisabled, NewPGProcessIdleCollector)
2727
}
2828

2929
type PGProcessIdleCollector struct {
3030
log log.Logger
3131
}
3232

33+
const processIdleSubsystem = "process_idle"
34+
3335
func NewPGProcessIdleCollector(config collectorConfig) (Collector, error) {
3436
return &PGProcessIdleCollector{log: config.logger}, nil
3537
}
@@ -41,8 +43,8 @@ var pgProcessIdleSeconds = prometheus.NewDesc(
4143
prometheus.Labels{},
4244
)
4345

44-
func (PGProcessIdleCollector) Update(ctx context.Context, instance *instance, ch chan<- prometheus.Metric) error {
45-
db := instance.getDB()
46+
func (PGProcessIdleCollector) Update(ctx context.Context, inst *instance, ch chan<- prometheus.Metric) error {
47+
db := inst.getDB()
4648
row := db.QueryRowContext(ctx,
4749
`WITH
4850
metrics AS (
@@ -79,9 +81,9 @@ func (PGProcessIdleCollector) Update(ctx context.Context, instance *instance, ch
7981
FROM metrics JOIN buckets USING (application_name)
8082
GROUP BY 1, 2, 3;`)
8183

82-
var applicationName string
83-
var secondsSum int64
84-
var secondsCount uint64
84+
var applicationName sql.NullString
85+
var secondsSum sql.NullInt64
86+
var secondsCount sql.NullInt64
8587
var seconds []int64
8688
var secondsBucket []uint64
8789

@@ -97,10 +99,24 @@ func (PGProcessIdleCollector) Update(ctx context.Context, instance *instance, ch
9799
if err != nil {
98100
return err
99101
}
102+
103+
applicationNameLabel := "unknown"
104+
if applicationName.Valid {
105+
applicationNameLabel = applicationName.String
106+
}
107+
108+
var secondsCountMetric uint64
109+
if secondsCount.Valid {
110+
secondsCountMetric = uint64(secondsCount.Int64)
111+
}
112+
secondsSumMetric := 0.0
113+
if secondsSum.Valid {
114+
secondsSumMetric = float64(secondsSum.Int64)
115+
}
100116
ch <- prometheus.MustNewConstHistogram(
101117
pgProcessIdleSeconds,
102-
secondsCount, float64(secondsSum), buckets,
103-
applicationName,
118+
secondsCountMetric, secondsSumMetric, buckets,
119+
applicationNameLabel,
104120
)
105121
return nil
106122
}

‎collector/pg_replication_slot.go‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ package collector
1515

1616
import (
1717
"context"
18+
"database/sql"
1819

1920
"github.com/go-kit/log"
2021
"github.com/prometheus/client_golang/prometheus"
@@ -82,32 +83,44 @@ func (PGReplicationSlotCollector) Update(ctx context.Context, instance *instance
8283
defer rows.Close()
8384

8485
for rows.Next() {
85-
var slotName string
86-
var walLSN int64
87-
var flushLSN int64
88-
var isActive bool
86+
var slotName sql.NullString
87+
var walLSN sql.NullFloat64
88+
var flushLSN sql.NullFloat64
89+
var isActive sql.NullBool
8990
if err := rows.Scan(&slotName, &walLSN, &flushLSN, &isActive); err != nil {
9091
return err
9192
}
9293

93-
isActiveValue := 0
94-
if isActive {
95-
isActiveValue = 1
94+
isActiveValue := 0.0
95+
if isActive.Valid && isActive.Bool {
96+
isActiveValue = 1.0
97+
}
98+
slotNameLabel := "unknown"
99+
if slotName.Valid {
100+
slotNameLabel = slotName.String
96101
}
97102

103+
var walLSNMetric float64
104+
if walLSN.Valid {
105+
walLSNMetric = walLSN.Float64
106+
}
98107
ch <- prometheus.MustNewConstMetric(
99108
pgReplicationSlotCurrentWalDesc,
100-
prometheus.GaugeValue, float64(walLSN), slotName,
109+
prometheus.GaugeValue, walLSNMetric, slotNameLabel,
101110
)
102-
if isActive {
111+
if isActive.Valid && isActive.Bool {
112+
var flushLSNMetric float64
113+
if flushLSN.Valid {
114+
flushLSNMetric = flushLSN.Float64
115+
}
103116
ch <- prometheus.MustNewConstMetric(
104117
pgReplicationSlotCurrentFlushDesc,
105-
prometheus.GaugeValue, float64(flushLSN), slotName,
118+
prometheus.GaugeValue, flushLSNMetric, slotNameLabel,
106119
)
107120
}
108121
ch <- prometheus.MustNewConstMetric(
109122
pgReplicationSlotIsActiveDesc,
110-
prometheus.GaugeValue, float64(isActiveValue), slotName,
123+
prometheus.GaugeValue, isActiveValue, slotNameLabel,
111124
)
112125
}
113126
if err := rows.Err(); err != nil {

‎collector/pg_replication_slot_test.go‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,3 +103,84 @@ func TestPgReplicationSlotCollectorInActive(t *testing.T) {
103103
}
104104

105105
}
106+
107+
func TestPgReplicationSlotCollectorActiveNil(t *testing.T) {
108+
db, mock, err := sqlmock.New()
109+
if err != nil {
110+
t.Fatalf("Error opening a stub db connection: %s", err)
111+
}
112+
defer db.Close()
113+
114+
inst := &instance{db: db}
115+
116+
columns := []string{"slot_name", "current_wal_lsn", "confirmed_flush_lsn", "active"}
117+
rows := sqlmock.NewRows(columns).
118+
AddRow("test_slot", 6, 12, nil)
119+
mock.ExpectQuery(sanitizeQuery(pgReplicationSlotQuery)).WillReturnRows(rows)
120+
121+
ch := make(chan prometheus.Metric)
122+
go func() {
123+
defer close(ch)
124+
c := PGReplicationSlotCollector{}
125+
126+
if err := c.Update(context.Background(), inst, ch); err != nil {
127+
t.Errorf("Error calling PGReplicationSlotCollector.Update: %s", err)
128+
}
129+
}()
130+
131+
expected := []MetricResult{
132+
{labels: labelMap{"slot_name": "test_slot"}, value: 6, metricType: dto.MetricType_GAUGE},
133+
{labels: labelMap{"slot_name": "test_slot"}, value: 0, metricType: dto.MetricType_GAUGE},
134+
}
135+
136+
convey.Convey("Metrics comparison", t, func() {
137+
for _, expect := range expected {
138+
m := readMetric(<-ch)
139+
convey.So(expect, convey.ShouldResemble, m)
140+
}
141+
})
142+
if err := mock.ExpectationsWereMet(); err != nil {
143+
t.Errorf("there were unfulfilled exceptions: %s", err)
144+
}
145+
}
146+
147+
func TestPgReplicationSlotCollectorTestNilValues(t *testing.T) {
148+
db, mock, err := sqlmock.New()
149+
if err != nil {
150+
t.Fatalf("Error opening a stub db connection: %s", err)
151+
}
152+
defer db.Close()
153+
154+
inst := &instance{db: db}
155+
156+
columns := []string{"slot_name", "current_wal_lsn", "confirmed_flush_lsn", "active"}
157+
rows := sqlmock.NewRows(columns).
158+
AddRow(nil, nil, nil, true)
159+
mock.ExpectQuery(sanitizeQuery(pgReplicationSlotQuery)).WillReturnRows(rows)
160+
161+
ch := make(chan prometheus.Metric)
162+
go func() {
163+
defer close(ch)
164+
c := PGReplicationSlotCollector{}
165+
166+
if err := c.Update(context.Background(), inst, ch); err != nil {
167+
t.Errorf("Error calling PGReplicationSlotCollector.Update: %s", err)
168+
}
169+
}()
170+
171+
expected := []MetricResult{
172+
{labels: labelMap{"slot_name": "unknown"}, value: 0, metricType: dto.MetricType_GAUGE},
173+
{labels: labelMap{"slot_name": "unknown"}, value: 0, metricType: dto.MetricType_GAUGE},
174+
{labels: labelMap{"slot_name": "unknown"}, value: 1, metricType: dto.MetricType_GAUGE},
175+
}
176+
177+
convey.Convey("Metrics comparison", t, func() {
178+
for _, expect := range expected {
179+
m := readMetric(<-ch)
180+
convey.So(expect, convey.ShouldResemble, m)
181+
}
182+
})
183+
if err := mock.ExpectationsWereMet(); err != nil {
184+
t.Errorf("there were unfulfilled exceptions: %s", err)
185+
}
186+
}

0 commit comments

Comments
 (0)