diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-03-18 16:27:33 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-03-18 16:27:33 +0300 |
commit | 67a783574950e069587184bcc897bbab1662fe1d (patch) | |
tree | aaa997f9f1ca208db87132971ac96ee405db5158 | |
parent | ff2f4ee8b2ef8f200775d45c550bfb178c55ba9a (diff) |
housekeeping: Fix Prometheus metrics' latency bucketspks-housekeeping-track-total-count-of-optimizations
The housekeeping manager is hosting a latency metric which tracks how
long specific tasks of the housekeeping manager take. But while latency
metrics should typically use the GRPC latency buckets as configured in
the Gitaly configuration, we accidentally didn't in the context of the
housekeeping manager.
Fix this bug by passing in the Prometheus configuration.
-rw-r--r-- | cmd/gitaly/main.go | 2 | ||||
-rw-r--r-- | internal/git/housekeeping/clean_stale_data_test.go | 18 | ||||
-rw-r--r-- | internal/git/housekeeping/manager.go | 8 | ||||
-rw-r--r-- | internal/git/housekeeping/optimize_repository_ext_test.go | 2 | ||||
-rw-r--r-- | internal/git/housekeeping/optimize_repository_test.go | 2 | ||||
-rw-r--r-- | internal/git/objectpool/testhelper_test.go | 2 | ||||
-rw-r--r-- | internal/gitaly/maintenance/optimize_test.go | 2 | ||||
-rw-r--r-- | internal/gitaly/service/objectpool/fetch_into_object_pool_test.go | 2 | ||||
-rw-r--r-- | internal/gitaly/service/objectpool/testhelper_test.go | 2 | ||||
-rw-r--r-- | internal/gitaly/service/repository/create_fork_test.go | 2 | ||||
-rw-r--r-- | internal/gitaly/service/repository/search_files_test.go | 4 | ||||
-rw-r--r-- | internal/gitaly/service/smarthttp/inforefs_test.go | 2 | ||||
-rw-r--r-- | internal/gitaly/service/ssh/receive_pack_test.go | 2 | ||||
-rw-r--r-- | internal/praefect/replicator_test.go | 2 | ||||
-rw-r--r-- | internal/testhelper/testserver/gitaly.go | 2 |
15 files changed, 28 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_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{ |