From 393dab81418c617aca4b6671e9afdaf74b5a4bfd Mon Sep 17 00:00:00 2001
From: Felix Yuan <felix.yuan@reddit.com>
Date: Wed, 28 Jun 2023 11:04:34 -0700
Subject: [PATCH 1/3] User Indexes collector and test

Signed-off-by: Felix Yuan <felix.yuan@reddit.com>
---
 collector/pg_stat_user_indexes.go      | 136 +++++++++++++++++++++++++
 collector/pg_stat_user_indexes_test.go | 113 ++++++++++++++++++++
 2 files changed, 249 insertions(+)
 create mode 100644 collector/pg_stat_user_indexes.go
 create mode 100644 collector/pg_stat_user_indexes_test.go

diff --git a/collector/pg_stat_user_indexes.go b/collector/pg_stat_user_indexes.go
new file mode 100644
index 00000000..d7b289cc
--- /dev/null
+++ b/collector/pg_stat_user_indexes.go
@@ -0,0 +1,136 @@
+// Copyright 2023 The Prometheus Authors
+// 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
+//
+// http://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 collector
+
+import (
+	"context"
+	"database/sql"
+
+	"github.com/go-kit/log"
+	"github.com/prometheus/client_golang/prometheus"
+)
+
+func init() {
+	registerCollector(statUserIndexesSubsystem, defaultDisabled, NewPGStatUserIndexesCollector)
+}
+
+type PGStatUserIndexesCollector struct {
+	log log.Logger
+}
+
+const statUserIndexesSubsystem = "stat_user_indexes"
+
+func NewPGStatUserIndexesCollector(config collectorConfig) (Collector, error) {
+	return &PGStatUserIndexesCollector{log: config.logger}, nil
+}
+
+var (
+	statUserIndexesIdxScan = prometheus.NewDesc(
+		prometheus.BuildFQName(namespace, statUserIndexesSubsystem, "idx_scans_total"),
+		"Number of index scans initiated on this index",
+		[]string{"schemaname", "relname", "indexrelname"},
+		prometheus.Labels{},
+	)
+	statUserIndexesIdxTupRead = prometheus.NewDesc(
+		prometheus.BuildFQName(namespace, statUserIndexesSubsystem, "idx_tup_reads_total"),
+		"Number of index entries returned by scans on this index",
+		[]string{"schemaname", "relname", "indexrelname"},
+		prometheus.Labels{},
+	)
+	statUserIndexesIdxTupFetch = prometheus.NewDesc(
+		prometheus.BuildFQName(namespace, statUserIndexesSubsystem, "idx_tup_fetches_total"),
+		"Number of live table rows fetched by simple index scans using this index",
+		[]string{"schemaname", "relname", "indexrelname"},
+		prometheus.Labels{},
+	)
+
+	statUserIndexesQuery = `
+	SELECT
+		schemaname,
+		relname,
+		indexrelname,
+		idx_scan,
+		idx_tup_read,
+		idx_tup_fetch
+	FROM pg_stat_user_indexes
+	`
+)
+
+func (c *PGStatUserIndexesCollector) Update(ctx context.Context, instance *instance, ch chan<- prometheus.Metric) error {
+	db := instance.getDB()
+	rows, err := db.QueryContext(ctx,
+		statUserIndexesQuery)
+
+	if err != nil {
+		return err
+	}
+	defer rows.Close()
+	for rows.Next() {
+		var schemaname, relname, indexrelname sql.NullString
+		var idxScan, idxTupRead, idxTupFetch sql.NullFloat64
+
+		if err := rows.Scan(&schemaname, &relname, &indexrelname, &idxScan, &idxTupRead, &idxTupFetch); err != nil {
+			return err
+		}
+		schemanameLabel := "unknown"
+		if schemaname.Valid {
+			schemanameLabel = schemaname.String
+		}
+		relnameLabel := "unknown"
+		if relname.Valid {
+			relnameLabel = relname.String
+		}
+		indexrelnameLabel := "unknown"
+		if indexrelname.Valid {
+			indexrelnameLabel = indexrelname.String
+		}
+		labels := []string{schemanameLabel, relnameLabel, indexrelnameLabel}
+
+		idxScanMetric := 0.0
+		if idxScan.Valid {
+			idxScanMetric = idxScan.Float64
+		}
+		ch <- prometheus.MustNewConstMetric(
+			statUserIndexesIdxScan,
+			prometheus.CounterValue,
+			idxScanMetric,
+			labels...,
+		)
+
+		idxTupReadMetric := 0.0
+		if idxTupRead.Valid {
+			idxTupReadMetric = idxTupRead.Float64
+		}
+		ch <- prometheus.MustNewConstMetric(
+			statUserIndexesIdxTupRead,
+			prometheus.CounterValue,
+			idxTupReadMetric,
+			labels...,
+		)
+
+		idxTupFetchMetric := 0.0
+		if idxTupFetch.Valid {
+			idxTupFetchMetric = idxTupFetch.Float64
+		}
+		ch <- prometheus.MustNewConstMetric(
+			statUserIndexesIdxTupFetch,
+			prometheus.CounterValue,
+			idxTupFetchMetric,
+			labels...,
+		)
+	}
+	if err := rows.Err(); err != nil {
+		return err
+	}
+	return nil
+}
diff --git a/collector/pg_stat_user_indexes_test.go b/collector/pg_stat_user_indexes_test.go
new file mode 100644
index 00000000..b0c7fde1
--- /dev/null
+++ b/collector/pg_stat_user_indexes_test.go
@@ -0,0 +1,113 @@
+// Copyright 2023 The Prometheus Authors
+// 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
+//
+// http://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 collector
+
+import (
+	"context"
+	"testing"
+
+	"github.com/DATA-DOG/go-sqlmock"
+	"github.com/prometheus/client_golang/prometheus"
+	dto "github.com/prometheus/client_model/go"
+	"github.com/smartystreets/goconvey/convey"
+)
+
+func TestPgStatUserIndexesCollector(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{
+		"schemaname",
+		"relname",
+		"indexrelname",
+		"idx_scan",
+		"idx_tup_read",
+		"idx_tup_fetch",
+	}
+	rows := sqlmock.NewRows(columns).
+		AddRow("public", "pgbench_accounts", "pgbench_accounts_pkey", 5, 6, 7)
+
+	mock.ExpectQuery(sanitizeQuery(statUserIndexesQuery)).WillReturnRows(rows)
+
+	ch := make(chan prometheus.Metric)
+	go func() {
+		defer close(ch)
+		c := PGStatUserIndexesCollector{}
+
+		if err := c.Update(context.Background(), inst, ch); err != nil {
+			t.Errorf("Error calling PGStatUserIndexesCollector.Update: %s", err)
+		}
+	}()
+	expected := []MetricResult{
+		{labels: labelMap{"schemaname": "public", "relname": "pgbench_accounts", "indexrelname": "pgbench_accounts_pkey"}, value: 5, metricType: dto.MetricType_COUNTER},
+		{labels: labelMap{"schemaname": "public", "relname": "pgbench_accounts", "indexrelname": "pgbench_accounts_pkey"}, value: 6, metricType: dto.MetricType_COUNTER},
+		{labels: labelMap{"schemaname": "public", "relname": "pgbench_accounts", "indexrelname": "pgbench_accounts_pkey"}, value: 7, metricType: dto.MetricType_COUNTER},
+	}
+	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)
+	}
+}
+
+func TestPgStatUserIndexesCollectorNull(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{
+		"schemaname",
+		"relname",
+		"indexrelname",
+		"idx_scan",
+		"idx_tup_read",
+		"idx_tup_fetch",
+	}
+	rows := sqlmock.NewRows(columns).
+		AddRow(nil, nil, nil, nil, nil, nil)
+
+	mock.ExpectQuery(sanitizeQuery(statUserIndexesQuery)).WillReturnRows(rows)
+
+	ch := make(chan prometheus.Metric)
+	go func() {
+		defer close(ch)
+		c := PGStatUserIndexesCollector{}
+
+		if err := c.Update(context.Background(), inst, ch); err != nil {
+			t.Errorf("Error calling PGStatUserIndexesCollector.Update: %s", err)
+		}
+	}()
+	expected := []MetricResult{
+		{labels: labelMap{"schemaname": "unknown", "relname": "unknown", "indexrelname": "unknown"}, value: 0, metricType: dto.MetricType_COUNTER},
+		{labels: labelMap{"schemaname": "unknown", "relname": "unknown", "indexrelname": "unknown"}, value: 0, metricType: dto.MetricType_COUNTER},
+		{labels: labelMap{"schemaname": "unknown", "relname": "unknown", "indexrelname": "unknown"}, value: 0, metricType: dto.MetricType_COUNTER},
+	}
+	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 66a22531ffa519107b330d724826a95eb743c904 Mon Sep 17 00:00:00 2001
From: Felix Yuan <felix.yuan@reddit.com>
Date: Thu, 29 Jun 2023 14:54:16 -0700
Subject: [PATCH 2/3] Continue if labels are bad

Signed-off-by: Felix Yuan <felix.yuan@reddit.com>
---
 collector/pg_stat_user_indexes.go      | 17 +++++++----------
 collector/pg_stat_user_indexes_test.go |  8 ++++----
 2 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/collector/pg_stat_user_indexes.go b/collector/pg_stat_user_indexes.go
index d7b289cc..865d641e 100644
--- a/collector/pg_stat_user_indexes.go
+++ b/collector/pg_stat_user_indexes.go
@@ -82,19 +82,16 @@ func (c *PGStatUserIndexesCollector) Update(ctx context.Context, instance *insta
 		if err := rows.Scan(&schemaname, &relname, &indexrelname, &idxScan, &idxTupRead, &idxTupFetch); err != nil {
 			return err
 		}
-		schemanameLabel := "unknown"
-		if schemaname.Valid {
-			schemanameLabel = schemaname.String
+		if !schemaname.Valid {
+			continue
 		}
-		relnameLabel := "unknown"
-		if relname.Valid {
-			relnameLabel = relname.String
+		if !relname.Valid {
+			continue
 		}
-		indexrelnameLabel := "unknown"
-		if indexrelname.Valid {
-			indexrelnameLabel = indexrelname.String
+		if !indexrelname.Valid {
+			continue
 		}
-		labels := []string{schemanameLabel, relnameLabel, indexrelnameLabel}
+		labels := []string{schemaname.String, relname.String, indexrelname.String}
 
 		idxScanMetric := 0.0
 		if idxScan.Valid {
diff --git a/collector/pg_stat_user_indexes_test.go b/collector/pg_stat_user_indexes_test.go
index b0c7fde1..3d25d2f3 100644
--- a/collector/pg_stat_user_indexes_test.go
+++ b/collector/pg_stat_user_indexes_test.go
@@ -83,7 +83,7 @@ func TestPgStatUserIndexesCollectorNull(t *testing.T) {
 		"idx_tup_fetch",
 	}
 	rows := sqlmock.NewRows(columns).
-		AddRow(nil, nil, nil, nil, nil, nil)
+		AddRow("foo", "bar", "blah", nil, nil, nil)
 
 	mock.ExpectQuery(sanitizeQuery(statUserIndexesQuery)).WillReturnRows(rows)
 
@@ -97,9 +97,9 @@ func TestPgStatUserIndexesCollectorNull(t *testing.T) {
 		}
 	}()
 	expected := []MetricResult{
-		{labels: labelMap{"schemaname": "unknown", "relname": "unknown", "indexrelname": "unknown"}, value: 0, metricType: dto.MetricType_COUNTER},
-		{labels: labelMap{"schemaname": "unknown", "relname": "unknown", "indexrelname": "unknown"}, value: 0, metricType: dto.MetricType_COUNTER},
-		{labels: labelMap{"schemaname": "unknown", "relname": "unknown", "indexrelname": "unknown"}, value: 0, metricType: dto.MetricType_COUNTER},
+		{labels: labelMap{"schemaname": "foo", "relname": "bar", "indexrelname": "blah"}, value: 0, metricType: dto.MetricType_COUNTER},
+		{labels: labelMap{"schemaname": "foo", "relname": "bar", "indexrelname": "blah"}, value: 0, metricType: dto.MetricType_COUNTER},
+		{labels: labelMap{"schemaname": "foo", "relname": "bar", "indexrelname": "blah"}, value: 0, metricType: dto.MetricType_COUNTER},
 	}
 	convey.Convey("Metrics comparison", t, func() {
 		for _, expect := range expected {

From 1fbfad3dc98cf838f476aabf6877946443760286 Mon Sep 17 00:00:00 2001
From: Felix Yuan <felix.yuan@reddit.com>
Date: Thu, 29 Jun 2023 15:48:25 -0700
Subject: [PATCH 3/3] Skip null metrics and add debug logs

Signed-off-by: Felix Yuan <felix.yuan@reddit.com>
---
 collector/pg_stat_user_indexes.go      | 30 ++++++++++-------
 collector/pg_stat_user_indexes_test.go | 45 --------------------------
 2 files changed, 19 insertions(+), 56 deletions(-)

diff --git a/collector/pg_stat_user_indexes.go b/collector/pg_stat_user_indexes.go
index 865d641e..e16a35e1 100644
--- a/collector/pg_stat_user_indexes.go
+++ b/collector/pg_stat_user_indexes.go
@@ -17,6 +17,7 @@ import (
 	"database/sql"
 
 	"github.com/go-kit/log"
+	"github.com/go-kit/log/level"
 	"github.com/prometheus/client_golang/prometheus"
 )
 
@@ -83,20 +84,33 @@ func (c *PGStatUserIndexesCollector) Update(ctx context.Context, instance *insta
 			return err
 		}
 		if !schemaname.Valid {
+			level.Debug(c.log).Log("msg", "Skipping stats on index because schemaname is not valid")
 			continue
 		}
 		if !relname.Valid {
+			level.Debug(c.log).Log("msg", "Skipping stats on index because relname is not valid")
 			continue
 		}
 		if !indexrelname.Valid {
+			level.Debug(c.log).Log("msg", "Skipping stats on index because indexrelname is not valid")
 			continue
 		}
 		labels := []string{schemaname.String, relname.String, indexrelname.String}
 
-		idxScanMetric := 0.0
-		if idxScan.Valid {
-			idxScanMetric = idxScan.Float64
+		if !idxScan.Valid {
+			level.Debug(c.log).Log("msg", "Skipping stats on index because idx_scan is not valid")
+			continue
 		}
+		if !idxTupRead.Valid {
+			level.Debug(c.log).Log("msg", "Skipping stats on index because idx_tup_read is not valid")
+			continue
+		}
+		if !idxTupFetch.Valid {
+			level.Debug(c.log).Log("msg", "Skipping stats on index because idx_tup_fetch is not valid")
+			continue
+		}
+
+		idxScanMetric := idxScan.Float64
 		ch <- prometheus.MustNewConstMetric(
 			statUserIndexesIdxScan,
 			prometheus.CounterValue,
@@ -104,10 +118,7 @@ func (c *PGStatUserIndexesCollector) Update(ctx context.Context, instance *insta
 			labels...,
 		)
 
-		idxTupReadMetric := 0.0
-		if idxTupRead.Valid {
-			idxTupReadMetric = idxTupRead.Float64
-		}
+		idxTupReadMetric := idxTupRead.Float64
 		ch <- prometheus.MustNewConstMetric(
 			statUserIndexesIdxTupRead,
 			prometheus.CounterValue,
@@ -115,10 +126,7 @@ func (c *PGStatUserIndexesCollector) Update(ctx context.Context, instance *insta
 			labels...,
 		)
 
-		idxTupFetchMetric := 0.0
-		if idxTupFetch.Valid {
-			idxTupFetchMetric = idxTupFetch.Float64
-		}
+		idxTupFetchMetric := idxTupFetch.Float64
 		ch <- prometheus.MustNewConstMetric(
 			statUserIndexesIdxTupFetch,
 			prometheus.CounterValue,
diff --git a/collector/pg_stat_user_indexes_test.go b/collector/pg_stat_user_indexes_test.go
index 3d25d2f3..ce59355b 100644
--- a/collector/pg_stat_user_indexes_test.go
+++ b/collector/pg_stat_user_indexes_test.go
@@ -66,48 +66,3 @@ func TestPgStatUserIndexesCollector(t *testing.T) {
 		t.Errorf("there were unfulfilled exceptions: %s", err)
 	}
 }
-
-func TestPgStatUserIndexesCollectorNull(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{
-		"schemaname",
-		"relname",
-		"indexrelname",
-		"idx_scan",
-		"idx_tup_read",
-		"idx_tup_fetch",
-	}
-	rows := sqlmock.NewRows(columns).
-		AddRow("foo", "bar", "blah", nil, nil, nil)
-
-	mock.ExpectQuery(sanitizeQuery(statUserIndexesQuery)).WillReturnRows(rows)
-
-	ch := make(chan prometheus.Metric)
-	go func() {
-		defer close(ch)
-		c := PGStatUserIndexesCollector{}
-
-		if err := c.Update(context.Background(), inst, ch); err != nil {
-			t.Errorf("Error calling PGStatUserIndexesCollector.Update: %s", err)
-		}
-	}()
-	expected := []MetricResult{
-		{labels: labelMap{"schemaname": "foo", "relname": "bar", "indexrelname": "blah"}, value: 0, metricType: dto.MetricType_COUNTER},
-		{labels: labelMap{"schemaname": "foo", "relname": "bar", "indexrelname": "blah"}, value: 0, metricType: dto.MetricType_COUNTER},
-		{labels: labelMap{"schemaname": "foo", "relname": "bar", "indexrelname": "blah"}, value: 0, metricType: dto.MetricType_COUNTER},
-	}
-	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)
-	}
-}