Merge branch 'jc-configure-prometheus-histogram-buckets' into 'master'

Move prometheus config to its own package

See merge request gitlab-org/gitaly!1676

9 files changed, 65 insertions, 49 deletions

diff --git a/changelogs/unreleased/jc-configure-prometheus-histogram-buckets.yml b/changelogs/unreleased/jc-configure-prometheus-histogram-buckets.yml
new file mode 100644
index 000000000..037a77fd6
--- /dev/null
+++ b/changelogs/unreleased/jc-configure-prometheus-histogram-buckets.yml
@@ -0,0 +1,5 @@
+---
+title: Move prometheus config to its own package
+merge_request: 1676
+author:
+type: other
diff --git a/cmd/gitaly/main.go b/cmd/gitaly/main.go
index 82a60fc40..18cc8f62c 100644
--- a/cmd/gitaly/main.go
+++ b/cmd/gitaly/main.go
@@ -71,7 +71,7 @@ func main() {
 
 	config.ConfigureLogging()
 	sentry.ConfigureSentry(version.GetVersion(), sentry.Config(config.Config.Logging.Sentry))
-	config.ConfigurePrometheus()
+	config.Config.Prometheus.Configure()
 	config.ConfigureConcurrencyLimits()
 	tracing.Initialize(tracing.WithServiceName("gitaly"))
 
diff --git a/cmd/praefect/main.go b/cmd/praefect/main.go
index af7245a7e..b667273d7 100644
--- a/cmd/praefect/main.go
+++ b/cmd/praefect/main.go
@@ -75,6 +75,7 @@ func configure() (config.Config, error) {
 
 	if conf.PrometheusListenAddr != "" {
 		logger.WithField("address", conf.PrometheusListenAddr).Info("Starting prometheus listener")
+		conf.Prometheus.Configure()
 
 		go func() {
 			if err := monitoring.Serve(
diff --git a/internal/config/config.go b/internal/config/config.go
index 945e5b048..037c96027 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -17,6 +17,7 @@ import (
 	log "github.com/sirupsen/logrus"
 	"gitlab.com/gitlab-org/gitaly/internal/config/auth"
 	internallog "gitlab.com/gitlab-org/gitaly/internal/config/log"
+	"gitlab.com/gitlab-org/gitaly/internal/config/prometheus"
 	"gitlab.com/gitlab-org/gitaly/internal/config/sentry"
 	"gitlab.com/gitlab-org/gitaly/internal/helper/text"
 )
@@ -30,20 +31,20 @@ var (
 
 // Cfg is a container for all config derived from config.toml.
 type Cfg struct {
-	SocketPath                 string        `toml:"socket_path" split_words:"true"`
-	ListenAddr                 string        `toml:"listen_addr" split_words:"true"`
-	TLSListenAddr              string        `toml:"tls_listen_addr" split_words:"true"`
-	PrometheusListenAddr       string        `toml:"prometheus_listen_addr" split_words:"true"`
-	BinDir                     string        `toml:"bin_dir"`
-	Git                        Git           `toml:"git" envconfig:"git"`
-	Storages                   []Storage     `toml:"storage" envconfig:"storage"`
-	Logging                    Logging       `toml:"logging" envconfig:"logging"`
-	Prometheus                 Prometheus    `toml:"prometheus"`
-	Auth                       auth.Config   `toml:"auth"`
-	TLS                        TLS           `toml:"tls"`
-	Ruby                       Ruby          `toml:"gitaly-ruby"`
-	GitlabShell                GitlabShell   `toml:"gitlab-shell"`
-	Concurrency                []Concurrency `toml:"concurrency"`
+	SocketPath                 string            `toml:"socket_path" split_words:"true"`
+	ListenAddr                 string            `toml:"listen_addr" split_words:"true"`
+	TLSListenAddr              string            `toml:"tls_listen_addr" split_words:"true"`
+	PrometheusListenAddr       string            `toml:"prometheus_listen_addr" split_words:"true"`
+	BinDir                     string            `toml:"bin_dir"`
+	Git                        Git               `toml:"git" envconfig:"git"`
+	Storages                   []Storage         `toml:"storage" envconfig:"storage"`
+	Logging                    Logging           `toml:"logging" envconfig:"logging"`
+	Prometheus                 prometheus.Config `toml:"prometheus"`
+	Auth                       auth.Config       `toml:"auth"`
+	TLS                        TLS               `toml:"tls"`
+	Ruby                       Ruby              `toml:"gitaly-ruby"`
+	GitlabShell                GitlabShell       `toml:"gitlab-shell"`
+	Concurrency                []Concurrency     `toml:"concurrency"`
 	GracefulRestartTimeout     time.Duration
 	GracefulRestartTimeoutToml duration `toml:"graceful_restart_timeout"`
 	InternalSocketDir          string   `toml:"internal_socket_dir"`
@@ -96,11 +97,6 @@ type Logging struct {
 	RubySentryDSN string `toml:"ruby_sentry_dsn"`
 }
 
-// Prometheus contains additional configuration data for prometheus
-type Prometheus struct {
-	GRPCLatencyBuckets []float64 `toml:"grpc_latency_buckets"`
-}
-
 // Concurrency allows endpoints to be limited to a maximum concurrency per repo
 type Concurrency struct {
 	RPC        string `toml:"rpc"`
diff --git a/internal/config/prometheus.go b/internal/config/prometheus.go
deleted file mode 100644
index da6e2d203..000000000
--- a/internal/config/prometheus.go
+++ /dev/null
@@ -1,23 +0,0 @@
-package config
-
-import (
-	grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus"
-	"github.com/prometheus/client_golang/prometheus"
-	log "github.com/sirupsen/logrus"
-	"gitlab.com/gitlab-org/gitaly/internal/middleware/limithandler"
-)
-
-// ConfigurePrometheus uses the global configuration to configure prometheus
-func ConfigurePrometheus() {
-	if len(Config.Prometheus.GRPCLatencyBuckets) == 0 {
-		return
-	}
-
-	log.WithField("latencies", Config.Prometheus.GRPCLatencyBuckets).Debug("grpc prometheus histograms enabled")
-
-	grpc_prometheus.EnableHandlingTimeHistogram(func(histogramOpts *prometheus.HistogramOpts) {
-		histogramOpts.Buckets = Config.Prometheus.GRPCLatencyBuckets
-	})
-
-	limithandler.EnableAcquireTimeHistogram(Config.Prometheus.GRPCLatencyBuckets)
-}
diff --git a/internal/config/prometheus/config.go b/internal/config/prometheus/config.go
new file mode 100644
index 000000000..31619f1a8
--- /dev/null
+++ b/internal/config/prometheus/config.go
@@ -0,0 +1,28 @@
+package prometheus
+
+import (
+	grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus"
+	"github.com/prometheus/client_golang/prometheus"
+	log "github.com/sirupsen/logrus"
+	"gitlab.com/gitlab-org/gitaly/internal/middleware/limithandler"
+)
+
+// Config contains additional configuration data for prometheus
+type Config struct {
+	GRPCLatencyBuckets []float64 `toml:"grpc_latency_buckets"`
+}
+
+// Configure configures latency buckets for prometheus timing histograms
+func (c *Config) Configure() {
+	if len(c.GRPCLatencyBuckets) == 0 {
+		return
+	}
+
+	log.WithField("latencies", c.GRPCLatencyBuckets).Debug("grpc prometheus histograms enabled")
+
+	grpc_prometheus.EnableHandlingTimeHistogram(func(histogramOpts *prometheus.HistogramOpts) {
+		histogramOpts.Buckets = c.GRPCLatencyBuckets
+	})
+
+	limithandler.EnableAcquireTimeHistogram(c.GRPCLatencyBuckets)
+}
diff --git a/internal/praefect/config/config.go b/internal/praefect/config/config.go
index b074c1f01..7c2c3daf6 100644
--- a/internal/praefect/config/config.go
+++ b/internal/praefect/config/config.go
@@ -9,6 +9,7 @@ import (
 
 	"gitlab.com/gitlab-org/gitaly/internal/config/auth"
 	"gitlab.com/gitlab-org/gitaly/internal/config/log"
+	"gitlab.com/gitlab-org/gitaly/internal/config/prometheus"
 	"gitlab.com/gitlab-org/gitaly/internal/config/sentry"
 	"gitlab.com/gitlab-org/gitaly/internal/praefect/models"
 )
@@ -20,12 +21,13 @@ type Config struct {
 	VirtualStorages []*VirtualStorage `toml:"virtual_storage"`
 	//TODO: Remove VirtualStorageName and Nodes once omnibus and gdk are updated with support for
 	// VirtualStorages
-	VirtualStorageName   string         `toml:"virtual_storage_name"`
-	Nodes                []*models.Node `toml:"node"`
-	Logging              log.Config     `toml:"logging"`
-	Sentry               sentry.Config  `toml:"sentry"`
-	PrometheusListenAddr string         `toml:"prometheus_listen_addr"`
-	Auth                 auth.Config    `toml:"auth"`
+	VirtualStorageName   string            `toml:"virtual_storage_name"`
+	Nodes                []*models.Node    `toml:"node"`
+	Logging              log.Config        `toml:"logging"`
+	Sentry               sentry.Config     `toml:"sentry"`
+	PrometheusListenAddr string            `toml:"prometheus_listen_addr"`
+	Prometheus           prometheus.Config `toml:"prometheus"`
+	Auth                 auth.Config       `toml:"auth"`
 }
 
 // VirtualStorage represents a set of nodes for a storage
diff --git a/internal/praefect/config/config_test.go b/internal/praefect/config/config_test.go
index ee659c349..63d2ed06d 100644
--- a/internal/praefect/config/config_test.go
+++ b/internal/praefect/config/config_test.go
@@ -7,6 +7,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/internal/config/log"
+	gitaly_prometheus "gitlab.com/gitlab-org/gitaly/internal/config/prometheus"
 	"gitlab.com/gitlab-org/gitaly/internal/config/sentry"
 	"gitlab.com/gitlab-org/gitaly/internal/praefect/models"
 )
@@ -164,6 +165,9 @@ func TestConfigParsing(t *testing.T) {
 						},
 					},
 				},
+				Prometheus: gitaly_prometheus.Config{
+					GRPCLatencyBuckets: []float64{0.1, 0.2, 0.3},
+				},
 			},
 		},
 		//TODO: Remove this test, as well as the fixture in testdata/single-virtual-storage.config.toml
diff --git a/internal/praefect/config/testdata/config.toml b/internal/praefect/config/testdata/config.toml
index bd1958975..e57aa6314 100644
--- a/internal/praefect/config/testdata/config.toml
+++ b/internal/praefect/config/testdata/config.toml
@@ -25,3 +25,6 @@ name = "praefect"
   [[virtual_storage.node]]
     address = "tcp://gitaly-internal-3.example.com"
     storage = "praefect-internal-3"
+
+[prometheus]
+  grpc_latency_buckets = [0.1, 0.2, 0.3]