Merge branch 'jc-dont-collect-db-metrics-on-startup' into 'master'

praefect: do not collect repository store metrics on startup

Closes #3920

See merge request gitlab-org/gitaly!4092

2 files changed, 56 insertions, 16 deletions

diff --git a/internal/praefect/datastore/collector.go b/internal/praefect/datastore/collector.go
index be3b6f646..a99f6514d 100644
--- a/internal/praefect/datastore/collector.go
+++ b/internal/praefect/datastore/collector.go
@@ -11,21 +11,25 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/praefect/datastore/glsql"
 )
 
-// This is kept for backwards compatibility as some alerting rules depend on this.
-// The unavailable repositories is a more accurate description for the metric and
-// is exported below so we can migrate to it.
-var descReadOnlyRepositories = prometheus.NewDesc(
-	"gitaly_praefect_read_only_repositories",
-	"Number of repositories in read-only mode within a virtual storage.",
-	[]string{"virtual_storage"},
-	nil,
-)
+var (
+	// This is kept for backwards compatibility as some alerting rules depend on this.
+	// The unavailable repositories is a more accurate description for the metric and
+	// is exported below so we can migrate to it.
+	descReadOnlyRepositories = prometheus.NewDesc(
+		"gitaly_praefect_read_only_repositories",
+		"Number of repositories in read-only mode within a virtual storage.",
+		[]string{"virtual_storage"},
+		nil,
+	)
+
+	descUnavailableRepositories = prometheus.NewDesc(
+		"gitaly_praefect_unavailable_repositories",
+		"Number of repositories that have no healthy, up to date replicas.",
+		[]string{"virtual_storage"},
+		nil,
+	)
 
-var descUnavailableRepositories = prometheus.NewDesc(
-	"gitaly_praefect_unavailable_repositories",
-	"Number of repositories that have no healthy, up to date replicas.",
-	[]string{"virtual_storage"},
-	nil,
+	descriptions = []*prometheus.Desc{descReadOnlyRepositories, descUnavailableRepositories}
 )
 
 // RepositoryStoreCollector collects metrics from the RepositoryStore.
@@ -48,7 +52,9 @@ func NewRepositoryStoreCollector(log logrus.FieldLogger, virtualStorages []strin
 
 //nolint: revive,stylecheck // This is unintentionally missing documentation.
 func (c *RepositoryStoreCollector) Describe(ch chan<- *prometheus.Desc) {
-	prometheus.DescribeByCollect(c, ch)
+	for _, desc := range descriptions {
+		ch <- desc
+	}
 }
 
 //nolint: revive,stylecheck // This is unintentionally missing documentation.
@@ -63,7 +69,7 @@ func (c *RepositoryStoreCollector) Collect(ch chan<- prometheus.Metric) {
 	}
 
 	for _, vs := range c.virtualStorages {
-		for _, desc := range []*prometheus.Desc{descReadOnlyRepositories, descUnavailableRepositories} {
+		for _, desc := range descriptions {
 			ch <- prometheus.MustNewConstMetric(desc, prometheus.GaugeValue, float64(unavailableCounts[vs]), vs)
 		}
 	}
diff --git a/internal/praefect/datastore/collector_test.go b/internal/praefect/datastore/collector_test.go
index 3fd1cfe41..861fe3998 100644
--- a/internal/praefect/datastore/collector_test.go
+++ b/internal/praefect/datastore/collector_test.go
@@ -2,14 +2,18 @@ package datastore
 
 import (
 	"context"
+	"database/sql"
+	"errors"
 	"fmt"
 	"strings"
 	"testing"
 	"time"
 
+	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/testutil"
 	"github.com/sirupsen/logrus"
 	"github.com/sirupsen/logrus/hooks/test"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/praefect/datastore/glsql"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
@@ -220,3 +224,33 @@ gitaly_praefect_unavailable_repositories{virtual_storage="virtual-storage-2"} 0
 		})
 	}
 }
+
+type checkIfQueriedDB struct {
+	queried bool
+}
+
+func (c *checkIfQueriedDB) QueryContext(ctx context.Context, query string, args ...interface{}) (*sql.Rows, error) {
+	c.queried = true
+	return nil, errors.New("QueryContext should not be called")
+}
+
+func (c *checkIfQueriedDB) QueryRowContext(ctx context.Context, query string, args ...interface{}) *sql.Row {
+	c.queried = true
+	return &sql.Row{}
+}
+
+func (c *checkIfQueriedDB) ExecContext(ctx context.Context, query string, args ...interface{}) (sql.Result, error) {
+	c.queried = true
+	return nil, errors.New("ExecContext should not be called")
+}
+
+func TestRepositoryStoreCollector_CollectNotCalledOnRegister(t *testing.T) {
+	logger, _ := test.NewNullLogger()
+
+	var db checkIfQueriedDB
+	c := NewRepositoryStoreCollector(logger, []string{"virtual-storage-1", "virtual-storage-2"}, &db, 2*time.Second)
+	registry := prometheus.NewRegistry()
+	registry.MustRegister(c)
+
+	assert.False(t, db.queried)
+}