diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-11-18 12:34:25 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-11-18 12:34:25 +0300 |
commit | 0dba5bd4b2123b80edfa27f33ff8833036e94814 (patch) | |
tree | b6decb198b50d593fe05a8dcf11647a3d27cf896 | |
parent | d69b465fdff3c04f8e3f395aed34aaa59f23fe76 (diff) | |
parent | 90cb7fb7b9f8703547fa62719650394478653c62 (diff) |
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
-rw-r--r-- | internal/praefect/datastore/collector.go | 38 | ||||
-rw-r--r-- | internal/praefect/datastore/collector_test.go | 34 |
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) +} |