config: Remove DB metrics configuration

Scraping DB metrics requires expensive queries to the Praefect DB. By
default these metrics are split out to its own independent endpoint for
scraping. This reduces resouce consumption by enabling separate
configuration of the metrics scraping intervals. Currently the
`prometheus_exclude_database_from_default_metrics` configuration exists
to toggle this behavior. It is now desired for the default behavior to
be the only option. This change removes the configuration option and
makes it so DB metrics cannot be combined with other metrics on the same
endpoint.

5 files changed, 14 insertions, 122 deletions

diff --git a/go.mod b/go.mod
index fe1331e11..656afccef 100644
--- a/go.mod
+++ b/go.mod
@@ -33,7 +33,6 @@ require (
 	github.com/opentracing/opentracing-go v1.2.0
 	github.com/pelletier/go-toml/v2 v2.0.7
 	github.com/prometheus/client_golang v1.15.0
-	github.com/prometheus/client_model v0.3.0
 	github.com/rubenv/sql-migrate v1.4.0
 	github.com/sirupsen/logrus v1.9.0
 	github.com/stretchr/testify v1.8.2
@@ -167,6 +166,7 @@ require (
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c // indirect
+	github.com/prometheus/client_model v0.3.0 // indirect
 	github.com/prometheus/common v0.42.0 // indirect
 	github.com/prometheus/procfs v0.9.0 // indirect
 	github.com/prometheus/prometheus v0.42.0 // indirect
diff --git a/internal/cli/praefect/main_test.go b/internal/cli/praefect/main_test.go
index 3bb308d7b..b9f8a9745 100644
--- a/internal/cli/praefect/main_test.go
+++ b/internal/cli/praefect/main_test.go
@@ -3,24 +3,17 @@ package praefect
 import (
 	"errors"
 	"flag"
-	"fmt"
 	"os"
 	"path/filepath"
 	"testing"
 
 	"github.com/pelletier/go-toml/v2"
-	"github.com/prometheus/client_golang/prometheus"
-	dto "github.com/prometheus/client_model/go"
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/urfave/cli/v2"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/bootstrap"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/bootstrap/starter"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/perm"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/praefect/config"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/praefect/datastore"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testdb"
 )
 
 func TestMain(m *testing.M) {
@@ -177,89 +170,6 @@ func TestGetStarterConfigs(t *testing.T) {
 	}
 }
 
-func newMockRegisterer() *mockRegisterer {
-	return &mockRegisterer{}
-}
-
-type mockRegisterer struct {
-	repositoryCollectorRegistered bool
-}
-
-func (m *mockRegisterer) Register(c prometheus.Collector) error {
-	if _, ok := c.(*datastore.RepositoryStoreCollector); ok {
-		m.repositoryCollectorRegistered = true
-	}
-
-	return nil
-}
-
-func (m *mockRegisterer) MustRegister(collectors ...prometheus.Collector) {
-	for _, c := range collectors {
-		if _, ok := c.(*datastore.RepositoryStoreCollector); ok {
-			m.repositoryCollectorRegistered = true
-		}
-	}
-}
-
-func (m *mockRegisterer) Unregister(c prometheus.Collector) bool {
-	return false
-}
-
-func (m *mockRegisterer) Gather() ([]*dto.MetricFamily, error) {
-	return nil, nil
-}
-
-func TestExcludeDatabaseMetricsFromDefaultMetrics(t *testing.T) {
-	t.Parallel()
-
-	db := testdb.New(t)
-	dbConf := testdb.GetConfig(t, db.Name)
-
-	conf := config.Config{
-		SocketPath: testhelper.GetTemporaryGitalySocketFileName(t),
-		VirtualStorages: []*config.VirtualStorage{
-			{
-				Name: "praefect",
-				Nodes: []*config.Node{
-					{Storage: "storage-0", Address: "tcp://someaddress"},
-				},
-			},
-		},
-		DB: dbConf,
-		Failover: config.Failover{
-			Enabled:          true,
-			ElectionStrategy: config.ElectionStrategyPerRepository,
-		},
-		MemoryQueueEnabled: true,
-	}
-
-	starterConfigs, err := getStarterConfigs(conf)
-	require.NoError(t, err)
-
-	excludeDatabaseMetricsSettings := []bool{true, false}
-	for _, excludeDatabaseMetrics := range excludeDatabaseMetricsSettings {
-		t.Run(fmt.Sprintf("exclude database metrics %v", excludeDatabaseMetrics), func(t *testing.T) {
-			conf.PrometheusExcludeDatabaseFromDefaultMetrics = excludeDatabaseMetrics
-
-			stopped := make(chan struct{})
-			bootstrapper := bootstrap.NewNoop(prometheus.NewCounterVec(prometheus.CounterOpts{Name: "stub"}, []string{"type"}))
-
-			metricRegisterer, dbMetricsRegisterer := newMockRegisterer(), newMockRegisterer()
-			go func() {
-				defer close(stopped)
-				logger := testhelper.NewDiscardingLogEntry(t)
-				assert.NoError(t, server(starterConfigs, conf, logger, bootstrapper, metricRegisterer, dbMetricsRegisterer))
-			}()
-
-			bootstrapper.Terminate()
-			<-stopped
-
-			assert.True(t, dbMetricsRegisterer.repositoryCollectorRegistered)
-			assert.Equal(t, excludeDatabaseMetrics, !metricRegisterer.repositoryCollectorRegistered)
-		})
-	}
-}
-
 func writeConfigToFile(tb testing.TB, conf config.Config) string {
 	tb.Helper()
 	confData, err := toml.Marshal(conf)
diff --git a/internal/cli/praefect/serve.go b/internal/cli/praefect/serve.go
index 9fe09e7c8..b9b637f45 100644
--- a/internal/cli/praefect/serve.go
+++ b/internal/cli/praefect/serve.go
@@ -427,16 +427,9 @@ func server(
 			))
 		}
 
-		// Eventually, database-related metrics will always be exported via a separate
-		// endpoint such that it's possible to set a different scraping interval and thus to
-		// reduce database load. For now though, we register the metrics twice, once for the
-		// standard and once for the database-specific endpoint. This is done to ensure a
-		// transitory period where deployments can be moved to the new endpoint without
-		// causing breakage if they still use the old endpoint.
+		// Database-related metrics are exported via a separate endpoint such that it's possible
+		// to set a different scraping interval and thus to reduce database load.
 		dbPromRegistry.MustRegister(dbMetricCollectors...)
-		if !conf.PrometheusExcludeDatabaseFromDefaultMetrics {
-			promreg.MustRegister(dbMetricCollectors...)
-		}
 	}
 	promreg.MustRegister(metricsCollectors...)
 
diff --git a/internal/praefect/config/config.go b/internal/praefect/config/config.go
index 57549dfd0..dabc88ca4 100644
--- a/internal/praefect/config/config.go
+++ b/internal/praefect/config/config.go
@@ -211,20 +211,14 @@ type Config struct {
 	Sentry                 sentry.Config          `toml:"sentry,omitempty"`
 	PrometheusListenAddr   string                 `toml:"prometheus_listen_addr,omitempty"`
 	Prometheus             prometheus.Config      `toml:"prometheus,omitempty"`
-	// PrometheusExcludeDatabaseFromDefaultMetrics excludes database-related metrics from the
-	// default metrics. If set to `false`, then database metrics will be available both via
-	// `/metrics` and `/db_metrics`. Otherwise, they will only be accessible via `/db_metrics`.
-	// Defaults to `true`. This is used as a transitory configuration key: eventually, database
-	// metrics will always be removed from the standard metrics endpoint.
-	PrometheusExcludeDatabaseFromDefaultMetrics bool        `toml:"prometheus_exclude_database_from_default_metrics,omitempty"`
-	Auth                                        auth.Config `toml:"auth,omitempty"`
-	TLS                                         config.TLS  `toml:"tls,omitempty"`
-	DB                                          `toml:"database,omitempty"`
-	Failover                                    Failover            `toml:"failover,omitempty"`
-	MemoryQueueEnabled                          bool                `toml:"memory_queue_enabled,omitempty"`
-	GracefulStopTimeout                         duration.Duration   `toml:"graceful_stop_timeout,omitempty"`
-	RepositoriesCleanup                         RepositoriesCleanup `toml:"repositories_cleanup,omitempty"`
-	Yamux                                       Yamux               `toml:"yamux,omitempty"`
+	Auth                   auth.Config            `toml:"auth,omitempty"`
+	TLS                    config.TLS             `toml:"tls,omitempty"`
+	DB                     `toml:"database,omitempty"`
+	Failover               Failover            `toml:"failover,omitempty"`
+	MemoryQueueEnabled     bool                `toml:"memory_queue_enabled,omitempty"`
+	GracefulStopTimeout    duration.Duration   `toml:"graceful_stop_timeout,omitempty"`
+	RepositoriesCleanup    RepositoriesCleanup `toml:"repositories_cleanup,omitempty"`
+	Yamux                  Yamux               `toml:"yamux,omitempty"`
 }
 
 // Yamux contains Yamux related configuration values.
@@ -310,7 +304,6 @@ func FromReader(reader io.Reader) (Config, error) {
 		Reconciliation:         DefaultReconciliationConfig(),
 		Replication:            DefaultReplicationConfig(),
 		Prometheus:             prometheus.DefaultConfig(),
-		PrometheusExcludeDatabaseFromDefaultMetrics: true,
 		// Sets the default Failover, to be overwritten when deserializing the TOML
 		Failover:            Failover{Enabled: true, ElectionStrategy: ElectionStrategyPerRepository},
 		RepositoriesCleanup: DefaultRepositoriesCleanup(),
diff --git a/internal/praefect/config/config_test.go b/internal/praefect/config/config_test.go
index 7b79803d7..3c434b5cc 100644
--- a/internal/praefect/config/config_test.go
+++ b/internal/praefect/config/config_test.go
@@ -302,7 +302,6 @@ func TestConfigParsing(t *testing.T) {
 					ScrapeTimeout:      duration.Duration(time.Second),
 					GRPCLatencyBuckets: []float64{0.1, 0.2, 0.3},
 				},
-				PrometheusExcludeDatabaseFromDefaultMetrics: true,
 				DB: DB{
 					Host:        "1.2.3.4",
 					Port:        5432,
@@ -365,8 +364,7 @@ func TestConfigParsing(t *testing.T) {
 					SchedulingInterval: 0,
 					HistogramBuckets:   []float64{1, 2, 3, 4, 5},
 				},
-				Prometheus: prometheus.DefaultConfig(),
-				PrometheusExcludeDatabaseFromDefaultMetrics: true,
+				Prometheus:  prometheus.DefaultConfig(),
 				Replication: Replication{BatchSize: 1, ParallelStorageProcessingWorkers: 2},
 				Failover: Failover{
 					Enabled:           false,
@@ -389,9 +387,8 @@ func TestConfigParsing(t *testing.T) {
 			expected: Config{
 				GracefulStopTimeout: duration.Duration(time.Minute),
 				Prometheus:          prometheus.DefaultConfig(),
-				PrometheusExcludeDatabaseFromDefaultMetrics: true,
-				Reconciliation: DefaultReconciliationConfig(),
-				Replication:    DefaultReplicationConfig(),
+				Reconciliation:      DefaultReconciliationConfig(),
+				Replication:         DefaultReplicationConfig(),
 				Failover: Failover{
 					Enabled:           true,
 					ElectionStrategy:  ElectionStrategyPerRepository,
@@ -948,7 +945,6 @@ func TestConfig_ValidateV2(t *testing.T) {
 				ScrapeTimeout:      duration.Duration(-1),
 				GRPCLatencyBuckets: []float64{1},
 			},
-			PrometheusExcludeDatabaseFromDefaultMetrics: false,
 			TLS: config.TLS{
 				CertPath: "/doesnt/exist",
 				KeyPath:  tmpFile,