From 05db1be8cd3992e7583335b44805e0b65dceef9c Mon Sep 17 00:00:00 2001 From: John Cai Date: Thu, 4 Aug 2022 15:48:49 -0400 Subject: hook: Remove PackObjectsMetric feature flag This feature flag has been running in production without any issues. We can now remove it now. --- internal/gitaly/service/hook/pack_objects.go | 3 +- internal/gitaly/service/hook/pack_objects_test.go | 41 +++++++++------------- .../featureflag/ff_pack_objects_metrics.go | 9 ----- 3 files changed, 18 insertions(+), 35 deletions(-) delete mode 100644 internal/metadata/featureflag/ff_pack_objects_metrics.go diff --git a/internal/gitaly/service/hook/pack_objects.go b/internal/gitaly/service/hook/pack_objects.go index 4a6d36e92..3028a46bc 100644 --- a/internal/gitaly/service/hook/pack_objects.go +++ b/internal/gitaly/service/hook/pack_objects.go @@ -22,7 +22,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/pktline" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/hook" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" - "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/v15/internal/stream" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "google.golang.org/protobuf/encoding/protojson" @@ -133,7 +132,7 @@ func (s *server) runPackObjects( repo := req.GetRepository() - if featureflag.PackObjectsMetrics.IsEnabled(ctx) && s.concurrencyTracker != nil { + if s.concurrencyTracker != nil { finishRepoLog := s.concurrencyTracker.LogConcurrency(ctx, "repository", repo.GetRelativePath()) defer finishRepoLog() diff --git a/internal/gitaly/service/hook/pack_objects_test.go b/internal/gitaly/service/hook/pack_objects_test.go index 164ba95ad..0bf4262da 100644 --- a/internal/gitaly/service/hook/pack_objects_test.go +++ b/internal/gitaly/service/hook/pack_objects_test.go @@ -20,7 +20,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/pktline" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" hookPkg "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/hook" - "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/v15/internal/streamcache" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg" @@ -80,10 +79,10 @@ func TestParsePackObjectsArgs(t *testing.T) { func TestServer_PackObjectsHook_separateContext(t *testing.T) { t.Parallel() - testhelper.NewFeatureSets(featureflag.PackObjectsMetrics).Run( + runTestsWithRuntimeDir( t, - runTestsWithRuntimeDir(t, testServerPackObjectsHookSeparateContextWithRuntimeDir), - ) + testServerPackObjectsHookSeparateContextWithRuntimeDir, + )(t, testhelper.Context(t)) } func testServerPackObjectsHookSeparateContextWithRuntimeDir(t *testing.T, ctx context.Context, runtimeDir string) { @@ -198,10 +197,10 @@ func testServerPackObjectsHookSeparateContextWithRuntimeDir(t *testing.T, ctx co func TestServer_PackObjectsHook_usesCache(t *testing.T) { t.Parallel() - testhelper.NewFeatureSets(featureflag.PackObjectsMetrics).Run( + runTestsWithRuntimeDir( t, - runTestsWithRuntimeDir(t, testServerPackObjectsHookUsesCache), - ) + testServerPackObjectsHookUsesCache, + )(t, testhelper.Context(t)) } func testServerPackObjectsHookUsesCache(t *testing.T, ctx context.Context, runtimeDir string) { @@ -283,10 +282,10 @@ func testServerPackObjectsHookUsesCache(t *testing.T, ctx context.Context, runti func TestServer_PackObjectsHookWithSidechannel(t *testing.T) { t.Parallel() - testhelper.NewFeatureSets(featureflag.PackObjectsMetrics).Run( + runTestsWithRuntimeDir( t, - runTestsWithRuntimeDir(t, testServerPackObjectsHookWithSidechannelWithRuntimeDir), - ) + testServerPackObjectsHookWithSidechannelWithRuntimeDir, + )(t, testhelper.Context(t)) } func testServerPackObjectsHookWithSidechannelWithRuntimeDir(t *testing.T, ctx context.Context, runtimeDir string) { @@ -410,8 +409,7 @@ func testServerPackObjectsHookWithSidechannelWithRuntimeDir(t *testing.T, ctx co require.False(t, strings.Contains(total, "\n")) }) - if featureflag.PackObjectsMetrics.IsEnabled(ctx) { - expectedMetrics := `# HELP gitaly_pack_objects_concurrent_processes Number of concurrent processes + expectedMetrics := `# HELP gitaly_pack_objects_concurrent_processes Number of concurrent processes # TYPE gitaly_pack_objects_concurrent_processes histogram gitaly_pack_objects_concurrent_processes_bucket{segment="repository",le="0"} 0 gitaly_pack_objects_concurrent_processes_bucket{segment="repository",le="5"} 1 @@ -468,15 +466,10 @@ gitaly_pack_objects_process_active_callers{segment="user_id"} 0 gitaly_pack_objects_process_active_callers_total{segment="repository"} 1 gitaly_pack_objects_process_active_callers_total{segment="user_id"} 1 ` - require.NoError(t, testutil.CollectAndCompare( - concurrencyTracker, - bytes.NewBufferString(expectedMetrics), - )) - } else { - require.Equal(t, 0, testutil.CollectAndCount( - concurrencyTracker, - )) - } + require.NoError(t, testutil.CollectAndCompare( + concurrencyTracker, + bytes.NewBufferString(expectedMetrics), + )) }) } } @@ -524,10 +517,10 @@ func TestServer_PackObjectsHookWithSidechannel_invalidArgument(t *testing.T) { func TestServer_PackObjectsHookWithSidechannel_Canceled(t *testing.T) { t.Parallel() - testhelper.NewFeatureSets(featureflag.PackObjectsMetrics).Run( + runTestsWithRuntimeDir( t, - runTestsWithRuntimeDir(t, testServerPackObjectsHookWithSidechannelCanceledWithRuntimeDir), - ) + testServerPackObjectsHookWithSidechannelCanceledWithRuntimeDir, + )(t, testhelper.Context(t)) } func testServerPackObjectsHookWithSidechannelCanceledWithRuntimeDir(t *testing.T, ctx context.Context, runtimeDir string) { diff --git a/internal/metadata/featureflag/ff_pack_objects_metrics.go b/internal/metadata/featureflag/ff_pack_objects_metrics.go deleted file mode 100644 index 7b8ee665a..000000000 --- a/internal/metadata/featureflag/ff_pack_objects_metrics.go +++ /dev/null @@ -1,9 +0,0 @@ -package featureflag - -// PackObjectsMetrics turns on metrics for pack objects -var PackObjectsMetrics = NewFeatureFlag( - "pack_objects_metrics", - "v15.2.0", - "https://gitlab.com/gitlab-org/gitaly/-/issues/4336", - false, -) -- cgit v1.2.3