config: Remove DB metrics configurationjt-db-metrics-config

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, 10 insertions, 115 deletions

diff --git a/cmd/praefect/main.go b/cmd/praefect/main.go
index 76cb2071b..1cf76c8c4 100644
--- a/cmd/praefect/main.go
+++ b/cmd/praefect/main.go
@@ -456,16 +456,9 @@ func run(
 			))
 		}
 
-		// 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/cmd/praefect/main_test.go b/cmd/praefect/main_test.go
index 3879a5437..f066b5872 100644
--- a/cmd/praefect/main_test.go
+++ b/cmd/praefect/main_test.go
@@ -4,19 +4,13 @@ package main
 
 import (
 	"errors"
-	"fmt"
 	"testing"
 
-	"github.com/prometheus/client_golang/prometheus"
-	dto "github.com/prometheus/client_model/go"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/bootstrap"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/bootstrap/starter"
 	"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) {
@@ -156,85 +150,3 @@ 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)
-				assert.NoError(t, run(starterConfigs, conf, bootstrapper, metricRegisterer, dbMetricsRegisterer))
-			}()
-
-			bootstrapper.Terminate()
-			<-stopped
-
-			assert.True(t, dbMetricsRegisterer.repositoryCollectorRegistered)
-			assert.Equal(t, excludeDatabaseMetrics, !metricRegisterer.repositoryCollectorRegistered)
-		})
-	}
-}
diff --git a/go.mod b/go.mod
index 8ddd307a9..164496dac 100644
--- a/go.mod
+++ b/go.mod
@@ -31,7 +31,6 @@ require (
 	github.com/opentracing/opentracing-go v1.2.0
 	github.com/pelletier/go-toml/v2 v2.0.5
 	github.com/prometheus/client_golang v1.13.0
-	github.com/prometheus/client_model v0.2.0
 	github.com/rubenv/sql-migrate v1.1.2
 	github.com/sirupsen/logrus v1.9.0
 	github.com/stretchr/testify v1.8.0
@@ -148,6 +147,7 @@ require (
 	github.com/philhofer/fwd v1.1.1 // indirect
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
+	github.com/prometheus/client_model v0.2.0 // indirect
 	github.com/prometheus/common v0.37.0 // indirect
 	github.com/prometheus/procfs v0.8.0 // indirect
 	github.com/rubyist/tracerx v0.0.0-20170927163412-787959303086 // indirect
diff --git a/internal/praefect/config/config.go b/internal/praefect/config/config.go
index 5af6eccdb..c63ff7db3 100644
--- a/internal/praefect/config/config.go
+++ b/internal/praefect/config/config.go
@@ -140,16 +140,10 @@ 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"`
+	Auth                   auth.Config            `toml:"auth,omitempty"`
+	TLS                    config.TLS             `toml:"tls,omitempty"`
+	DB                     `toml:"database,omitempty"`
+	Failover               Failover `toml:"failover,omitempty"`
 	// Keep for legacy reasons: remove after Omnibus has switched
 	FailoverEnabled     bool                `toml:"failover_enabled,omitempty"`
 	MemoryQueueEnabled  bool                `toml:"memory_queue_enabled,omitempty"`
@@ -181,7 +175,6 @@ func FromFile(filePath string) (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 b0e1879f6..3f783b46b 100644
--- a/internal/praefect/config/config_test.go
+++ b/internal/praefect/config/config_test.go
@@ -294,7 +294,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,
@@ -353,8 +352,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,
@@ -376,9 +374,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,