Skip to content
Snippets Groups Projects
Commit 90cb7fb7 authored by John Cai's avatar John Cai Committed by Patrick Steinhardt
Browse files

praefect: Do not collect repository store metrics on startup

Our current code path will trigger the RepositoryStoreCollector to query
the database on startup, even if the prometheus listener is not
listening. This is because we call DescribeByCollect in the Describe
method. The Prometheus client will call Describe on Register, which ends
up triggering the Collect method and hence runs the queries. Instead, we
can just provide the decriptions separately from the Collect method.

Changelog: fixed
parent d69b465f
No related branches found
No related tags found
Loading
......@@ -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)
}
}
......
......@@ -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)
}
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment