diff --git a/.golangci.yml b/.golangci.yml index cd3d897e08..38f1e214c1 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -58,7 +58,7 @@ linters: - zerologlint settings: misspell: - ignore-words: + ignore-rules: - cancelled extra-words: - typo: "canceled" @@ -72,6 +72,8 @@ linters: - fmt.Fprintln - (io.Closer).Close - updateConfigMap + exhaustive: + default-signifies-exhaustive: true gocritic: disabled-checks: - unlambda diff --git a/pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go b/pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go index bcb49a2fcd..31f246b962 100644 --- a/pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go +++ b/pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go @@ -42,6 +42,7 @@ type Provider struct { projectKey string repo *v1alpha1.Repository triggerEvent string + cachedChangedFiles *changedfiles.ChangedFiles } func (v Provider) Client() *scm.Client { @@ -371,13 +372,28 @@ func (v *Provider) GetConfig() *info.ProviderConfig { } } +// GetFiles gets and caches the list of files changed by a given event. func (v *Provider) GetFiles(ctx context.Context, runevent *info.Event) (changedfiles.ChangedFiles, error) { - OrgAndRepo := fmt.Sprintf("%s/%s", runevent.Organization, runevent.Repository) - if runevent.TriggerTarget == triggertype.PullRequest { + if v.cachedChangedFiles == nil { + changes, err := v.fetchChangedFiles(ctx, runevent) + if err != nil { + return changedfiles.ChangedFiles{}, err + } + v.cachedChangedFiles = &changes + } + return *v.cachedChangedFiles, nil +} + +func (v *Provider) fetchChangedFiles(ctx context.Context, runevent *info.Event) (changedfiles.ChangedFiles, error) { + changedFiles := changedfiles.ChangedFiles{} + + orgAndRepo := fmt.Sprintf("%s/%s", runevent.Organization, runevent.Repository) + + switch runevent.TriggerTarget { + case triggertype.PullRequest: opts := &scm.ListOptions{Page: 1, Size: apiResponseLimit} - changedFiles := changedfiles.ChangedFiles{} for { - changes, _, err := v.Client().PullRequests.ListChanges(ctx, OrgAndRepo, runevent.PullRequestNumber, opts) + changes, _, err := v.Client().PullRequests.ListChanges(ctx, orgAndRepo, runevent.PullRequestNumber, opts) if err != nil { return changedfiles.ChangedFiles{}, fmt.Errorf("failed to list changes for pull request: %w", err) } @@ -408,14 +424,10 @@ func (v *Provider) GetFiles(ctx context.Context, runevent *info.Event) (changedf opts.Page++ } - return changedFiles, nil - } - - if runevent.TriggerTarget == triggertype.Push { + case triggertype.Push: opts := &scm.ListOptions{Page: 1, Size: apiResponseLimit} - changedFiles := changedfiles.ChangedFiles{} for { - changes, _, err := v.Client().Git.ListChanges(ctx, OrgAndRepo, runevent.SHA, opts) + changes, _, err := v.Client().Git.ListChanges(ctx, orgAndRepo, runevent.SHA, opts) if err != nil { return changedfiles.ChangedFiles{}, fmt.Errorf("failed to list changes for commit %s: %w", runevent.SHA, err) } @@ -442,9 +454,10 @@ func (v *Provider) GetFiles(ctx context.Context, runevent *info.Event) (changedf opts.Page++ } - return changedFiles, nil + default: + // No action necessary } - return changedfiles.ChangedFiles{}, nil + return changedFiles, nil } func (v *Provider) CreateToken(_ context.Context, _ []string, _ *info.Event) (string, error) { diff --git a/pkg/provider/bitbucketdatacenter/bitbucketdatacenter_test.go b/pkg/provider/bitbucketdatacenter/bitbucketdatacenter_test.go index b786939c8b..5829693815 100644 --- a/pkg/provider/bitbucketdatacenter/bitbucketdatacenter_test.go +++ b/pkg/provider/bitbucketdatacenter/bitbucketdatacenter_test.go @@ -22,11 +22,14 @@ import ( "github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" bbtest "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/bitbucketdatacenter/test" + "github.com/openshift-pipelines/pipelines-as-code/pkg/test/metrics" "github.com/jenkins-x/go-scm/scm" "go.uber.org/zap" zapobserver "go.uber.org/zap/zaptest/observer" "gotest.tools/v3/assert" + "knative.dev/pkg/metrics/metricstest" + _ "knative.dev/pkg/metrics/testing" rtesting "knative.dev/pkg/reconciler/testing" ) @@ -796,6 +799,7 @@ func TestGetFiles(t *testing.T) { ctx, _ := rtesting.SetupFakeContext(t) client, mux, tearDown, tURL := bbtest.SetupBBDataCenterClient() defer tearDown() + metrics.ResetMetrics() stats := &bbtest.DiffStats{ Values: tt.changeFiles, @@ -821,13 +825,17 @@ func TestGetFiles(t *testing.T) { } }) } - v := &Provider{client: client, baseURL: tURL} + + metricsTags := map[string]string{"provider": "bitbucket-datacenter", "event-type": string(tt.event.TriggerTarget)} + metricstest.CheckStatsNotReported(t, "pipelines_as_code_git_provider_api_request_count") + + v := &Provider{client: client, baseURL: tURL, triggerEvent: string(tt.event.TriggerTarget)} changedFiles, err := v.GetFiles(ctx, tt.event) if tt.wantError { assert.Equal(t, err.Error(), tt.errMsg) - return + } else { + assert.NilError(t, err, nil) } - assert.NilError(t, err, nil) assert.Equal(t, tt.wantAddedFilesCount, len(changedFiles.Added)) assert.Equal(t, tt.wantDeletedFilesCount, len(changedFiles.Deleted)) assert.Equal(t, tt.wantModifiedFilesCount, len(changedFiles.Modified)) @@ -844,6 +852,17 @@ func TestGetFiles(t *testing.T) { assert.Equal(t, tt.changeFiles[i].Path.ToString, changedFiles.All[i]) } } + + // Check caching + metricstest.CheckCountData(t, "pipelines_as_code_git_provider_api_request_count", metricsTags, 1) + _, _ = v.GetFiles(ctx, tt.event) + if tt.wantError { + // No caching on error + metricstest.CheckCountData(t, "pipelines_as_code_git_provider_api_request_count", metricsTags, 2) + } else { + // Cache API results on success + metricstest.CheckCountData(t, "pipelines_as_code_git_provider_api_request_count", metricsTags, 1) + } }) } } diff --git a/pkg/provider/github/github.go b/pkg/provider/github/github.go index c882cc0593..ad5bd7c7af 100644 --- a/pkg/provider/github/github.go +++ b/pkg/provider/github/github.go @@ -55,7 +55,8 @@ type Provider struct { PaginedNumber int userType string // The type of user i.e bot or not skippedRun - triggerEvent string + triggerEvent string + cachedChangedFiles *changedfiles.ChangedFiles } type skippedRun struct { @@ -518,11 +519,24 @@ func (v *Provider) getPullRequest(ctx context.Context, runevent *info.Event) (*i return runevent, nil } -// GetFiles get a files from pull request. +// GetFiles gets and caches the list of files changed by a given event. func (v *Provider) GetFiles(ctx context.Context, runevent *info.Event) (changedfiles.ChangedFiles, error) { - if runevent.TriggerTarget == triggertype.PullRequest { + if v.cachedChangedFiles == nil { + changes, err := v.fetchChangedFiles(ctx, runevent) + if err != nil { + return changedfiles.ChangedFiles{}, err + } + v.cachedChangedFiles = &changes + } + return *v.cachedChangedFiles, nil +} + +func (v *Provider) fetchChangedFiles(ctx context.Context, runevent *info.Event) (changedfiles.ChangedFiles, error) { + changedFiles := changedfiles.ChangedFiles{} + + switch runevent.TriggerTarget { + case triggertype.PullRequest: opt := &github.ListOptions{PerPage: v.PaginedNumber} - changedFiles := changedfiles.ChangedFiles{} for { repoCommit, resp, err := wrapAPI(v, "list_pull_request_files", func() ([]*github.CommitFile, *github.Response, error) { return v.Client().PullRequests.ListFiles(ctx, runevent.Organization, runevent.Repository, runevent.PullRequestNumber, opt) @@ -550,11 +564,7 @@ func (v *Provider) GetFiles(ctx context.Context, runevent *info.Event) (changedf } opt.Page = resp.NextPage } - return changedFiles, nil - } - - if runevent.TriggerTarget == "push" { - changedFiles := changedfiles.ChangedFiles{} + case triggertype.Push: rC, _, err := wrapAPI(v, "get_commit_files", func() (*github.RepositoryCommit, *github.Response, error) { return v.Client().Repositories.GetCommit(ctx, runevent.Organization, runevent.Repository, runevent.SHA, &github.ListOptions{}) }) @@ -576,9 +586,10 @@ func (v *Provider) GetFiles(ctx context.Context, runevent *info.Event) (changedf changedFiles.Renamed = append(changedFiles.Renamed, *rC.Files[i].Filename) } } - return changedFiles, nil + default: + // No action necessary } - return changedfiles.ChangedFiles{}, nil + return changedFiles, nil } // getObject Get an object from a repository. diff --git a/pkg/provider/github/github_test.go b/pkg/provider/github/github_test.go index 190d8467c4..91da8975cd 100644 --- a/pkg/provider/github/github_test.go +++ b/pkg/provider/github/github_test.go @@ -20,7 +20,6 @@ import ( "github.com/jonboulle/clockwork" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" - "github.com/openshift-pipelines/pipelines-as-code/pkg/metrics" "github.com/openshift-pipelines/pipelines-as-code/pkg/params" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/clients" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" @@ -29,6 +28,8 @@ import ( testclient "github.com/openshift-pipelines/pipelines-as-code/pkg/test/clients" ghtesthelper "github.com/openshift-pipelines/pipelines-as-code/pkg/test/github" "github.com/openshift-pipelines/pipelines-as-code/pkg/test/logger" + "github.com/openshift-pipelines/pipelines-as-code/pkg/test/metrics" + "go.uber.org/zap" zapobserver "go.uber.org/zap/zaptest/observer" "gotest.tools/v3/assert" @@ -347,7 +348,7 @@ func TestGetTektonDir(t *testing.T) { } for _, tt := range testGetTektonDir { t.Run(tt.name, func(t *testing.T) { - resetMetrics() + metrics.ResetMetrics() observer, exporter := zapobserver.New(zap.InfoLevel) fakelogger := zap.New(observer).Sugar() ctx, _ := rtesting.SetupFakeContext(t) @@ -949,6 +950,7 @@ func TestGetFiles(t *testing.T) { wantDeletedFilesCount int wantModifiedFilesCount int wantRenamedFilesCount int + wantAPIRequestCount int64 }{ { name: "pull-request", @@ -977,6 +979,7 @@ func TestGetFiles(t *testing.T) { wantDeletedFilesCount: 1, wantModifiedFilesCount: 1, wantRenamedFilesCount: 1, + wantAPIRequestCount: 2, }, { name: "push", @@ -1007,43 +1010,15 @@ func TestGetFiles(t *testing.T) { wantDeletedFilesCount: 1, wantModifiedFilesCount: 1, wantRenamedFilesCount: 1, + wantAPIRequestCount: 1, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { fakeclient, mux, _, teardown := ghtesthelper.SetupGH() defer teardown() - prCommitFiles := []*github.CommitFile{ - { - Filename: ptr.String("modified.yaml"), - Status: ptr.String("modified"), - }, { - Filename: ptr.String("added.doc"), - Status: ptr.String("added"), - }, { - Filename: ptr.String("removed.yaml"), - Status: ptr.String("removed"), - }, { - Filename: ptr.String("renamed.doc"), - Status: ptr.String("renamed"), - }, - } + metrics.ResetMetrics() - pushCommitFiles := []*github.CommitFile{ - { - Filename: ptr.String("modified.yaml"), - Status: ptr.String("modified"), - }, { - Filename: ptr.String("added.doc"), - Status: ptr.String("added"), - }, { - Filename: ptr.String("removed.yaml"), - Status: ptr.String("removed"), - }, { - Filename: ptr.String("renamed.doc"), - Status: ptr.String("renamed"), - }, - } if tt.event.TriggerTarget == "pull_request" { mux.HandleFunc(fmt.Sprintf("/repos/%s/%s/pulls/%d/files", tt.event.Organization, tt.event.Repository, tt.event.PullRequestNumber), func(rw http.ResponseWriter, r *http.Request) { @@ -1051,7 +1026,7 @@ func TestGetFiles(t *testing.T) { rw.Header().Add("Link", fmt.Sprintf("; rel=\"next\"", tt.event.Organization, tt.event.Repository, tt.event.PullRequestNumber)) fmt.Fprint(rw, "[]") } else { - b, _ := json.Marshal(prCommitFiles) + b, _ := json.Marshal(tt.commitFiles) fmt.Fprint(rw, string(b)) } }) @@ -1060,17 +1035,26 @@ func TestGetFiles(t *testing.T) { mux.HandleFunc(fmt.Sprintf("/repos/%s/%s/commits/%s", tt.event.Organization, tt.event.Repository, tt.event.SHA), func(rw http.ResponseWriter, _ *http.Request) { c := &github.RepositoryCommit{ - Files: pushCommitFiles, + Files: tt.commit.Files, } b, _ := json.Marshal(c) fmt.Fprint(rw, string(b)) }) } + metricsTags := map[string]string{"provider": "github", "event-type": string(tt.event.TriggerTarget)} + metricstest.CheckStatsNotReported(t, "pipelines_as_code_git_provider_api_request_count") + + log, _ := logger.GetLogger() ctx, _ := rtesting.SetupFakeContext(t) provider := &Provider{ ghClient: fakeclient, PaginedNumber: 1, + + // necessary for metrics + providerName: "github", + triggerEvent: string(tt.event.TriggerTarget), + Logger: log, } changedFiles, err := provider.GetFiles(ctx, tt.event) assert.NilError(t, err, nil) @@ -1089,6 +1073,11 @@ func TestGetFiles(t *testing.T) { assert.Equal(t, *tt.commit.Files[i].Filename, changedFiles.All[i]) } } + + // Check caching + metricstest.CheckCountData(t, "pipelines_as_code_git_provider_api_request_count", metricsTags, tt.wantAPIRequestCount) + _, _ = provider.GetFiles(ctx, tt.event) + metricstest.CheckCountData(t, "pipelines_as_code_git_provider_api_request_count", metricsTags, tt.wantAPIRequestCount) }) } } @@ -1439,18 +1428,6 @@ func TestIsHeadCommitOfBranch(t *testing.T) { } } -func resetMetrics() { - metricstest.Unregister( - "pipelines_as_code_pipelinerun_count", - "pipelines_as_code_pipelinerun_duration_seconds_sum", - "pipelines_as_code_running_pipelineruns_count", - "pipelines_as_code_git_provider_api_request_count", - ) - - // have to reset sync.Once to allow recreation of Recorder. - metrics.ResetRecorder() -} - func TestCreateComment(t *testing.T) { tests := []struct { name string diff --git a/pkg/provider/gitlab/gitlab.go b/pkg/provider/gitlab/gitlab.go index 145ac18626..bf9c0cfd9c 100644 --- a/pkg/provider/gitlab/gitlab.go +++ b/pkg/provider/gitlab/gitlab.go @@ -64,7 +64,8 @@ type Provider struct { triggerEvent string // memberCache caches membership/permission checks by user ID within the // current provider instance lifecycle to avoid repeated API calls. - memberCache map[int]bool + memberCache map[int]bool + cachedChangedFiles *changedfiles.ChangedFiles } func (v *Provider) Client() *gitlab.Client { @@ -497,12 +498,28 @@ func (v *Provider) GetCommitInfo(_ context.Context, runevent *info.Event) error return nil } -func (v *Provider) GetFiles(_ context.Context, runevent *info.Event) (changedfiles.ChangedFiles, error) { +// GetFiles gets and caches the list of files changed by a given event. +func (v *Provider) GetFiles(ctx context.Context, runevent *info.Event) (changedfiles.ChangedFiles, error) { + if v.cachedChangedFiles == nil { + changes, err := v.fetchChangedFiles(ctx, runevent) + if err != nil { + return changedfiles.ChangedFiles{}, err + } + v.cachedChangedFiles = &changes + } + return *v.cachedChangedFiles, nil +} + +func (v *Provider) fetchChangedFiles(_ context.Context, runevent *info.Event) (changedfiles.ChangedFiles, error) { if v.gitlabClient == nil { return changedfiles.ChangedFiles{}, fmt.Errorf("no gitlab client has been initialized, " + "exiting... (hint: did you forget setting a secret on your repo?)") } - if runevent.TriggerTarget == triggertype.PullRequest { + + changedFiles := changedfiles.ChangedFiles{} + + switch runevent.TriggerTarget { + case triggertype.PullRequest: opt := &gitlab.ListMergeRequestDiffsOptions{ ListOptions: gitlab.ListOptions{ OrderBy: "id", @@ -512,11 +529,11 @@ func (v *Provider) GetFiles(_ context.Context, runevent *info.Event) (changedfil }, } options := []gitlab.RequestOptionFunc{} - changedFiles := changedfiles.ChangedFiles{} for { mrchanges, resp, err := v.Client().MergeRequests.ListMergeRequestDiffs(v.targetProjectID, runevent.PullRequestNumber, opt, options...) if err != nil { + // TODO: Should this return the files found so far? return changedfiles.ChangedFiles{}, err } @@ -546,15 +563,11 @@ func (v *Provider) GetFiles(_ context.Context, runevent *info.Event) (changedfil gitlab.WithKeysetPaginationParameters(resp.NextLink), } } - return changedFiles, nil - } - - if runevent.TriggerTarget == "push" { + case triggertype.Push: pushChanges, _, err := v.Client().Commits.GetCommitDiff(v.sourceProjectID, runevent.SHA, &gitlab.GetCommitDiffOptions{}) if err != nil { return changedfiles.ChangedFiles{}, err } - changedFiles := changedfiles.ChangedFiles{} for _, change := range pushChanges { changedFiles.All = append(changedFiles.All, change.NewPath) if change.NewFile { @@ -570,9 +583,10 @@ func (v *Provider) GetFiles(_ context.Context, runevent *info.Event) (changedfil changedFiles.Renamed = append(changedFiles.Renamed, change.NewPath) } } - return changedFiles, nil + default: + // No action necessary } - return changedfiles.ChangedFiles{}, nil + return changedFiles, nil } func (v *Provider) CreateToken(_ context.Context, _ []string, _ *info.Event) (string, error) { diff --git a/pkg/provider/gitlab/gitlab_test.go b/pkg/provider/gitlab/gitlab_test.go index 34be3d2437..3afb5d6667 100644 --- a/pkg/provider/gitlab/gitlab_test.go +++ b/pkg/provider/gitlab/gitlab_test.go @@ -22,11 +22,14 @@ import ( thelp "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/gitlab/test" testclient "github.com/openshift-pipelines/pipelines-as-code/pkg/test/clients" "github.com/openshift-pipelines/pipelines-as-code/pkg/test/logger" + "github.com/openshift-pipelines/pipelines-as-code/pkg/test/metrics" gitlab "gitlab.com/gitlab-org/api/client-go" "go.uber.org/zap" zapobserver "go.uber.org/zap/zaptest/observer" "gotest.tools/v3/assert" + "knative.dev/pkg/metrics/metricstest" + _ "knative.dev/pkg/metrics/testing" rtesting "knative.dev/pkg/reconciler/testing" ) @@ -1245,60 +1248,30 @@ func TestGetFiles(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ctx, _ := rtesting.SetupFakeContext(t) + metrics.ResetMetrics() fakeclient, mux, teardown := thelp.Setup(t) defer teardown() - mergeFileChanges := []*gitlab.MergeRequestDiff{ - { - NewPath: "modified.yaml", - }, - { - NewPath: "added.doc", - NewFile: true, - }, - { - NewPath: "removed.yaml", - DeletedFile: true, - }, - { - NewPath: "renamed.doc", - RenamedFile: true, - }, - } if tt.event.TriggerTarget == "pull_request" { mux.HandleFunc(fmt.Sprintf("/projects/10/merge_requests/%d/diffs", tt.event.PullRequestNumber), func(rw http.ResponseWriter, _ *http.Request) { - jeez, err := json.Marshal(mergeFileChanges) + jeez, err := json.Marshal(tt.mrchanges) assert.NilError(t, err) _, _ = rw.Write(jeez) }) } - pushFileChanges := []*gitlab.Diff{ - { - NewPath: "modified.yaml", - }, - { - NewPath: "added.doc", - NewFile: true, - }, - { - NewPath: "removed.yaml", - DeletedFile: true, - }, - { - NewPath: "renamed.doc", - RenamedFile: true, - }, - } if tt.event.TriggerTarget == "push" { mux.HandleFunc(fmt.Sprintf("/projects/0/repository/commits/%s/diff", tt.event.SHA), func(rw http.ResponseWriter, _ *http.Request) { - jeez, err := json.Marshal(pushFileChanges) + jeez, err := json.Marshal(tt.pushChanges) assert.NilError(t, err) _, _ = rw.Write(jeez) }) } - providerInfo := &Provider{gitlabClient: fakeclient, sourceProjectID: tt.sourceProjectID, targetProjectID: tt.targetProjectID} + metricsTags := map[string]string{"provider": "api.gitlab.com", "event-type": string(tt.event.TriggerTarget)} + metricstest.CheckStatsNotReported(t, "pipelines_as_code_git_provider_api_request_count") + + providerInfo := &Provider{gitlabClient: fakeclient, sourceProjectID: tt.sourceProjectID, targetProjectID: tt.targetProjectID, triggerEvent: string(tt.event.TriggerTarget), apiURL: "api.gitlab.com"} changedFiles, err := providerInfo.GetFiles(ctx, tt.event) if tt.wantError != true { assert.NilError(t, err, nil) @@ -1318,6 +1291,17 @@ func TestGetFiles(t *testing.T) { assert.Equal(t, tt.pushChanges[i].NewPath, changedFiles.All[i]) } } + + // Check caching + metricstest.CheckCountData(t, "pipelines_as_code_git_provider_api_request_count", metricsTags, 1) + _, _ = providerInfo.GetFiles(ctx, tt.event) + if tt.wantError { + // No caching on error + metricstest.CheckCountData(t, "pipelines_as_code_git_provider_api_request_count", metricsTags, 2) + } else { + // Cache API results on success + metricstest.CheckCountData(t, "pipelines_as_code_git_provider_api_request_count", metricsTags, 1) + } }) } } diff --git a/pkg/provider/metrics/metrics_test.go b/pkg/provider/metrics/metrics_test.go index f9b2ff1770..d428b1b3e8 100644 --- a/pkg/provider/metrics/metrics_test.go +++ b/pkg/provider/metrics/metrics_test.go @@ -9,7 +9,7 @@ import ( "knative.dev/pkg/metrics/metricstest" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" - "github.com/openshift-pipelines/pipelines-as-code/pkg/metrics" + metricsutils "github.com/openshift-pipelines/pipelines-as-code/pkg/test/metrics" _ "knative.dev/pkg/metrics/testing" ) @@ -47,15 +47,7 @@ func TestRecordAPIUsage(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.desc, func(t *testing.T) { - defer func() { - metricstest.Unregister( - "pipelines_as_code_pipelinerun_count", - "pipelines_as_code_pipelinerun_duration_seconds_sum", - "pipelines_as_code_running_pipelineruns_count", - "pipelines_as_code_git_provider_api_request_count", - ) - metrics.ResetRecorder() - }() + defer metricsutils.ResetMetrics() observer, _ := zapobserver.New(zap.InfoLevel) fakelogger := zap.New(observer).Sugar() diff --git a/pkg/reconciler/emit_metrics_test.go b/pkg/reconciler/emit_metrics_test.go index d6c0aaa7fd..b27ad33491 100644 --- a/pkg/reconciler/emit_metrics_test.go +++ b/pkg/reconciler/emit_metrics_test.go @@ -6,6 +6,7 @@ import ( "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys" "github.com/openshift-pipelines/pipelines-as-code/pkg/metrics" + metricsutils "github.com/openshift-pipelines/pipelines-as-code/pkg/test/metrics" tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "gotest.tools/v3/assert" corev1 "k8s.io/api/core/v1" @@ -86,7 +87,7 @@ func TestCountPipelineRun(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - unregisterMetrics() + metricsutils.ResetMetrics() m, err := metrics.NewRecorder() assert.NilError(t, err) r := &Reconciler{ @@ -224,7 +225,7 @@ func TestCalculatePipelineRunDuration(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - unregisterMetrics() + metricsutils.ResetMetrics() m, err := metrics.NewRecorder() assert.NilError(t, err) r := &Reconciler{ @@ -291,7 +292,7 @@ func TestCountRunningPRs(t *testing.T) { prl = append(prl, pr) } - unregisterMetrics() + metricsutils.ResetMetrics() m, err := metrics.NewRecorder() assert.NilError(t, err) r := &Reconciler{ @@ -306,15 +307,3 @@ func TestCountRunningPRs(t *testing.T) { } metricstest.CheckLastValueData(t, "pipelines_as_code_running_pipelineruns_count", tags, float64(numberOfRunningPRs)) } - -func unregisterMetrics() { - metricstest.Unregister( - "pipelines_as_code_pipelinerun_count", - "pipelines_as_code_pipelinerun_duration_seconds_sum", - "pipelines_as_code_running_pipelineruns_count", - "pipelines_as_code_git_provider_api_request_count", - ) - - // have to reset sync.Once to allow recreation of Recorder. - metrics.ResetRecorder() -} diff --git a/pkg/test/metrics/metrics.go b/pkg/test/metrics/metrics.go new file mode 100644 index 0000000000..11e94c770f --- /dev/null +++ b/pkg/test/metrics/metrics.go @@ -0,0 +1,18 @@ +package metrics + +import ( + "github.com/openshift-pipelines/pipelines-as-code/pkg/metrics" + "knative.dev/pkg/metrics/metricstest" +) + +func ResetMetrics() { + metricstest.Unregister( + "pipelines_as_code_pipelinerun_count", + "pipelines_as_code_pipelinerun_duration_seconds_sum", + "pipelines_as_code_running_pipelineruns_count", + "pipelines_as_code_git_provider_api_request_count", + ) + + // have to reset sync.Once to allow recreation of Recorder. + metrics.ResetRecorder() +}