Merge branch 'pks-housekeeping-track-total-count-of-optimizations' into 'master'

housekeeping: Track total number of optimization calls

See merge request gitlab-org/gitaly!4418

16 files changed, 30 insertions, 26 deletions

diff --git a/cmd/gitaly/main.go b/cmd/gitaly/main.go
index f0b525da2..7d58bdd88 100644
--- a/cmd/gitaly/main.go
+++ b/cmd/gitaly/main.go
@@ -166,7 +166,7 @@ func run(cfg config.Cfg) error {
 	transactionManager := transaction.NewManager(cfg, registry)
 	prometheus.MustRegister(transactionManager)
 
-	housekeepingManager := housekeeping.NewManager(transactionManager)
+	housekeepingManager := housekeeping.NewManager(cfg.Prometheus, transactionManager)
 	prometheus.MustRegister(housekeepingManager)
 
 	hookManager := hook.Manager(hook.DisabledManager{})
diff --git a/internal/git/housekeeping/clean_stale_data_test.go b/internal/git/housekeeping/clean_stale_data_test.go
index 28e52621c..0811e5805 100644
--- a/internal/git/housekeeping/clean_stale_data_test.go
+++ b/internal/git/housekeeping/clean_stale_data_test.go
@@ -265,7 +265,7 @@ func TestPerform(t *testing.T) {
 				e.create(t, repoPath)
 			}
 
-			mgr := NewManager(nil)
+			mgr := NewManager(cfg.Prometheus, nil)
 
 			require.NoError(t, mgr.CleanStaleData(ctx, repo))
 
@@ -361,7 +361,7 @@ func TestPerform_references(t *testing.T) {
 			}
 			ctx := testhelper.Context(t)
 
-			mgr := NewManager(nil)
+			mgr := NewManager(cfg.Prometheus, nil)
 
 			require.NoError(t, mgr.CleanStaleData(ctx, repo))
 
@@ -479,7 +479,7 @@ func TestPerform_emptyRefDirs(t *testing.T) {
 				e.create(t, repoPath)
 			}
 
-			mgr := NewManager(nil)
+			mgr := NewManager(cfg.Prometheus, nil)
 
 			require.NoError(t, mgr.CleanStaleData(ctx, repo))
 
@@ -543,7 +543,7 @@ func TestPerform_withSpecificFile(t *testing.T) {
 
 			cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
 			repo := localrepo.NewTestRepo(t, cfg, repoProto)
-			mgr := NewManager(nil)
+			mgr := NewManager(cfg.Prometheus, nil)
 
 			require.NoError(t, mgr.CleanStaleData(ctx, repo))
 			for _, subcase := range []struct {
@@ -686,7 +686,7 @@ func TestPerform_referenceLocks(t *testing.T) {
 			require.NoError(t, err)
 			require.ElementsMatch(t, expectedReferenceLocks, staleLockfiles)
 
-			mgr := NewManager(nil)
+			mgr := NewManager(cfg.Prometheus, nil)
 
 			require.NoError(t, mgr.CleanStaleData(ctx, repo))
 
@@ -780,7 +780,7 @@ func TestPerformRepoDoesNotExist(t *testing.T) {
 
 	require.NoError(t, os.RemoveAll(repoPath))
 
-	require.NoError(t, NewManager(nil).CleanStaleData(ctx, repo))
+	require.NoError(t, NewManager(cfg.Prometheus, nil).CleanStaleData(ctx, repo))
 }
 
 func TestPerform_UnsetConfiguration(t *testing.T) {
@@ -816,7 +816,7 @@ func TestPerform_UnsetConfiguration(t *testing.T) {
 	unrelated = untouched
 `), 0o644))
 
-	require.NoError(t, NewManager(nil).CleanStaleData(ctx, repo))
+	require.NoError(t, NewManager(cfg.Prometheus, nil).CleanStaleData(ctx, repo))
 	require.Equal(t,
 		`[core]
 	repositoryformatversion = 0
@@ -849,7 +849,7 @@ func TestPerform_UnsetConfiguration_transactional(t *testing.T) {
 		AuthInfo: backchannel.WithID(nil, 1234),
 	})
 
-	require.NoError(t, NewManager(txManager).CleanStaleData(ctx, repo))
+	require.NoError(t, NewManager(cfg.Prometheus, txManager).CleanStaleData(ctx, repo))
 	require.Equal(t, 2, len(txManager.Votes()))
 
 	configKeys := gittest.Exec(t, cfg, "-C", repoPath, "config", "--list", "--local", "--name-only")
@@ -893,7 +893,7 @@ func TestPerform_cleanupConfig(t *testing.T) {
 [remote "tmp-8c948ca94832c2725733e48cb2902287"]
 `), 0o644))
 
-	require.NoError(t, NewManager(nil).CleanStaleData(ctx, repo))
+	require.NoError(t, NewManager(cfg.Prometheus, nil).CleanStaleData(ctx, repo))
 	require.Equal(t, `[core]
 	repositoryformatversion = 0
 	filemode = true
diff --git a/internal/git/housekeeping/manager.go b/internal/git/housekeeping/manager.go
index 859d7091e..a40e1c102 100644
--- a/internal/git/housekeeping/manager.go
+++ b/internal/git/housekeeping/manager.go
@@ -5,6 +5,7 @@ import (
 
 	"github.com/prometheus/client_golang/prometheus"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/localrepo"
+	gitalycfgprom "gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config/prometheus"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/transaction"
 )
 
@@ -28,7 +29,7 @@ type RepositoryManager struct {
 }
 
 // NewManager creates a new RepositoryManager.
-func NewManager(txManager transaction.Manager) *RepositoryManager {
+func NewManager(promCfg gitalycfgprom.Config, txManager transaction.Manager) *RepositoryManager {
 	return &RepositoryManager{
 		txManager: txManager,
 
@@ -41,8 +42,9 @@ func NewManager(txManager transaction.Manager) *RepositoryManager {
 		),
 		tasksLatency: prometheus.NewHistogramVec(
 			prometheus.HistogramOpts{
-				Name: "gitaly_housekeeping_tasks_latency",
-				Help: "Latency of the housekeeping tasks performed",
+				Name:    "gitaly_housekeeping_tasks_latency",
+				Help:    "Latency of the housekeeping tasks performed",
+				Buckets: promCfg.GRPCLatencyBuckets,
 			},
 			[]string{"housekeeping_task"},
 		),
diff --git a/internal/git/housekeeping/optimize_repository.go b/internal/git/housekeeping/optimize_repository.go
index 20c0032eb..fc38753f4 100644
--- a/internal/git/housekeeping/optimize_repository.go
+++ b/internal/git/housekeeping/optimize_repository.go
@@ -46,6 +46,8 @@ func (m *RepositoryManager) OptimizeRepository(ctx context.Context, repo *localr
 				m.tasksTotal.WithLabelValues(task).Add(1)
 			}
 		}
+
+		m.tasksTotal.WithLabelValues("total").Add(1)
 	}()
 
 	timer := prometheus.NewTimer(m.tasksLatency.WithLabelValues("clean-stale-data"))
diff --git a/internal/git/housekeeping/optimize_repository_ext_test.go b/internal/git/housekeeping/optimize_repository_ext_test.go
index e12a172d7..b494edba4 100644
--- a/internal/git/housekeeping/optimize_repository_ext_test.go
+++ b/internal/git/housekeeping/optimize_repository_ext_test.go
@@ -118,7 +118,7 @@ func TestPruneIfNeeded(t *testing.T) {
 			logger, hook := test.NewNullLogger()
 			ctx := ctxlogrus.ToContext(ctx, logrus.NewEntry(logger))
 
-			require.NoError(t, housekeeping.NewManager(nil).OptimizeRepository(ctx, repo))
+			require.NoError(t, housekeeping.NewManager(cfg.Prometheus, nil).OptimizeRepository(ctx, repo))
 			require.Equal(t,
 				struct {
 					PackedObjectsIncremental bool `json:"packed_objects_incremental"`
diff --git a/internal/git/housekeeping/optimize_repository_test.go b/internal/git/housekeeping/optimize_repository_test.go
index 2533b7b99..2f5a2d8fd 100644
--- a/internal/git/housekeeping/optimize_repository_test.go
+++ b/internal/git/housekeeping/optimize_repository_test.go
@@ -562,7 +562,7 @@ func TestOptimizeRepository(t *testing.T) {
 			repoProto := tc.setup(t)
 			repo := localrepo.NewTestRepo(t, cfg, repoProto)
 
-			manager := NewManager(txManager)
+			manager := NewManager(cfg.Prometheus, txManager)
 
 			err := manager.OptimizeRepository(ctx, repo)
 			require.Equal(t, tc.expectedErr, err)
diff --git a/internal/git/objectpool/testhelper_test.go b/internal/git/objectpool/testhelper_test.go
index 71995e6e0..7e5d84433 100644
--- a/internal/git/objectpool/testhelper_test.go
+++ b/internal/git/objectpool/testhelper_test.go
@@ -36,7 +36,7 @@ func setupObjectPool(t *testing.T, ctx context.Context) (config.Cfg, *ObjectPool
 		gitCommandFactory,
 		catfileCache,
 		txManager,
-		housekeeping.NewManager(txManager),
+		housekeeping.NewManager(cfg.Prometheus, txManager),
 		repo.GetStorageName(),
 		gittest.NewObjectPoolName(t),
 	)
diff --git a/internal/gitaly/maintenance/optimize_test.go b/internal/gitaly/maintenance/optimize_test.go
index b2416131f..418c1eb18 100644
--- a/internal/gitaly/maintenance/optimize_test.go
+++ b/internal/gitaly/maintenance/optimize_test.go
@@ -39,7 +39,7 @@ func (mo *mockOptimizer) OptimizeRepository(ctx context.Context, req *gitalypb.O
 	mo.t.Cleanup(catfileCache.Stop)
 	git2goExecutor := git2go.NewExecutor(mo.cfg, gitCmdFactory, l)
 	txManager := transaction.NewManager(mo.cfg, backchannel.NewRegistry())
-	housekeepingManager := housekeeping.NewManager(txManager)
+	housekeepingManager := housekeeping.NewManager(mo.cfg.Prometheus, txManager)
 
 	connsPool := client.NewPool()
 	mo.t.Cleanup(func() { testhelper.MustClose(mo.t, connsPool) })
diff --git a/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go b/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go
index 12e1376c3..879c1cf1a 100644
--- a/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go
+++ b/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go
@@ -200,7 +200,7 @@ func TestFetchIntoObjectPool_Failure(t *testing.T) {
 		gitCmdFactory,
 		catfileCache,
 		txManager,
-		housekeeping.NewManager(txManager),
+		housekeeping.NewManager(cfg.Prometheus, txManager),
 	)
 	ctx := testhelper.Context(t)
 
diff --git a/internal/gitaly/service/objectpool/testhelper_test.go b/internal/gitaly/service/objectpool/testhelper_test.go
index 36b349132..f0ba73dd8 100644
--- a/internal/gitaly/service/objectpool/testhelper_test.go
+++ b/internal/gitaly/service/objectpool/testhelper_test.go
@@ -90,7 +90,7 @@ func newObjectPool(t testing.TB, cfg config.Cfg, storage, relativePath string) *
 		gittest.NewCommandFactory(t, cfg),
 		catfileCache,
 		txManager,
-		housekeeping.NewManager(txManager),
+		housekeeping.NewManager(cfg.Prometheus, txManager),
 		storage,
 		relativePath,
 	)
diff --git a/internal/gitaly/service/repository/create_fork_test.go b/internal/gitaly/service/repository/create_fork_test.go
index 800a9c7ac..b8ea85654 100644
--- a/internal/gitaly/service/repository/create_fork_test.go
+++ b/internal/gitaly/service/repository/create_fork_test.go
@@ -272,7 +272,7 @@ func runSecureServer(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) s
 	catfileCache := catfile.NewCache(cfg)
 	t.Cleanup(catfileCache.Stop)
 
-	housekeepingManager := housekeeping.NewManager(txManager)
+	housekeepingManager := housekeeping.NewManager(cfg.Prometheus, txManager)
 
 	connsPool := client.NewPool()
 	t.Cleanup(func() { testhelper.MustClose(t, connsPool) })
diff --git a/internal/gitaly/service/repository/search_files_test.go b/internal/gitaly/service/repository/search_files_test.go
index e81a491fa..bd6443ba3 100644
--- a/internal/gitaly/service/repository/search_files_test.go
+++ b/internal/gitaly/service/repository/search_files_test.go
@@ -219,7 +219,7 @@ func TestSearchFilesByContentFailure(t *testing.T) {
 
 	git2goExecutor := git2go.NewExecutor(cfg, gitCommandFactory, locator)
 	txManager := transaction.NewManager(cfg, backchannel.NewRegistry())
-	housekeepingManager := housekeeping.NewManager(txManager)
+	housekeepingManager := housekeeping.NewManager(cfg.Prometheus, txManager)
 
 	server := NewServer(
 		cfg,
@@ -355,7 +355,7 @@ func TestSearchFilesByNameFailure(t *testing.T) {
 
 	git2goExecutor := git2go.NewExecutor(cfg, gitCommandFactory, locator)
 	txManager := transaction.NewManager(cfg, backchannel.NewRegistry())
-	housekeepingManager := housekeeping.NewManager(txManager)
+	housekeepingManager := housekeeping.NewManager(cfg.Prometheus, txManager)
 
 	server := NewServer(
 		cfg,
diff --git a/internal/gitaly/service/smarthttp/inforefs_test.go b/internal/gitaly/service/smarthttp/inforefs_test.go
index 3ab23b3b2..4c95547a5 100644
--- a/internal/gitaly/service/smarthttp/inforefs_test.go
+++ b/internal/gitaly/service/smarthttp/inforefs_test.go
@@ -228,7 +228,7 @@ func TestObjectPoolRefAdvertisementHiding(t *testing.T) {
 		gittest.NewCommandFactory(t, cfg),
 		nil,
 		txManager,
-		housekeeping.NewManager(txManager),
+		housekeeping.NewManager(cfg.Prometheus, txManager),
 		repo.GetStorageName(),
 		gittest.NewObjectPoolName(t),
 	)
diff --git a/internal/gitaly/service/ssh/receive_pack_test.go b/internal/gitaly/service/ssh/receive_pack_test.go
index 0057bd144..cda2fa4aa 100644
--- a/internal/gitaly/service/ssh/receive_pack_test.go
+++ b/internal/gitaly/service/ssh/receive_pack_test.go
@@ -265,7 +265,7 @@ func TestObjectPoolRefAdvertisementHidingSSH(t *testing.T) {
 		gittest.NewCommandFactory(t, cfg),
 		nil,
 		txManager,
-		housekeeping.NewManager(txManager),
+		housekeeping.NewManager(cfg.Prometheus, txManager),
 		repo.GetStorageName(),
 		gittest.NewObjectPoolName(t),
 	)
diff --git a/internal/praefect/replicator_test.go b/internal/praefect/replicator_test.go
index 7a9915a4a..79e4135d7 100644
--- a/internal/praefect/replicator_test.go
+++ b/internal/praefect/replicator_test.go
@@ -94,7 +94,7 @@ func testReplMgr_ProcessBacklog(t *testing.T, ctx context.Context) {
 		gittest.NewCommandFactory(t, primaryCfg),
 		nil,
 		txManager,
-		housekeeping.NewManager(txManager),
+		housekeeping.NewManager(primaryCfg.Prometheus, txManager),
 		testRepoProto.GetStorageName(),
 		objectPoolPath,
 	)
diff --git a/internal/testhelper/testserver/gitaly.go b/internal/testhelper/testserver/gitaly.go
index 145208e42..9f60a4afa 100644
--- a/internal/testhelper/testserver/gitaly.go
+++ b/internal/testhelper/testserver/gitaly.go
@@ -314,7 +314,7 @@ func (gsd *gitalyServerDeps) createDependencies(t testing.TB, cfg config.Cfg, ru
 	}
 
 	if gsd.housekeepingManager == nil {
-		gsd.housekeepingManager = housekeeping.NewManager(gsd.txMgr)
+		gsd.housekeepingManager = housekeeping.NewManager(cfg.Prometheus, gsd.txMgr)
 	}
 
 	return &service.Dependencies{