feat: Track git provider API usage metrics #2005
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,12 @@ var runningPRCount = stats.Float64("pipelines_as_code_running_pipelineruns_count | |
| "number of running pipelineruns by pipelines as code", | ||
| stats.UnitDimensionless) | ||
|
|
||
| var gitProviderAPIRequestCount = stats.Int64( | ||
| "pipelines_as_code_git_provider_api_request_count", | ||
| "number of API requests from pipelines as code to git providers", | ||
| stats.UnitDimensionless, | ||
| ) | ||
|
|
||
| // Recorder holds keys for metrics. | ||
| type Recorder struct { | ||
| initialized bool | ||
|
|
@@ -61,36 +67,42 @@ func NewRecorder() (*Recorder, error) { | |
|
|
||
| provider, errRegistering := tag.NewKey("provider") | ||
| if errRegistering != nil { | ||
| ErrRegistering = errRegistering | ||
| return | ||
| } | ||
| R.provider = provider | ||
|
|
||
| eventType, errRegistering := tag.NewKey("event-type") | ||
| if errRegistering != nil { | ||
| ErrRegistering = errRegistering | ||
| return | ||
| } | ||
| R.eventType = eventType | ||
|
|
||
| namespace, errRegistering := tag.NewKey("namespace") | ||
| if errRegistering != nil { | ||
| ErrRegistering = errRegistering | ||
| return | ||
| } | ||
| R.namespace = namespace | ||
|
|
||
| repository, errRegistering := tag.NewKey("repository") | ||
| if errRegistering != nil { | ||
| ErrRegistering = errRegistering | ||
| return | ||
| } | ||
| R.repository = repository | ||
|
|
||
| status, errRegistering := tag.NewKey("status") | ||
| if errRegistering != nil { | ||
| ErrRegistering = errRegistering | ||
| return | ||
| } | ||
| R.status = status | ||
|
|
||
| reason, errRegistering := tag.NewKey("reason") | ||
| if errRegistering != nil { | ||
| ErrRegistering = errRegistering | ||
| return | ||
| } | ||
| R.reason = reason | ||
|
|
@@ -116,11 +128,18 @@ func NewRecorder() (*Recorder, error) { | |
| Aggregation: view.LastValue(), | ||
| TagKeys: []tag.Key{R.namespace, R.repository}, | ||
| } | ||
| gitProviderAPIRequestView = &view.View{ | ||
| Description: gitProviderAPIRequestCount.Description(), | ||
| Measure: gitProviderAPIRequestCount, | ||
| Aggregation: view.Count(), | ||
| TagKeys: []tag.Key{R.provider, R.eventType, R.namespace, R.repository}, | ||
| } | ||
| ) | ||
|
|
||
| view.Unregister(prCountView, prDurationView, runningPRView, gitProviderAPIRequestView) | ||
| errRegistering = view.Register(prCountView, prDurationView, runningPRView, gitProviderAPIRequestView) | ||
| if errRegistering != nil { | ||
| ErrRegistering = errRegistering | ||
| R.initialized = false | ||
| return | ||
| } | ||
|
|
@@ -129,12 +148,19 @@ func NewRecorder() (*Recorder, error) { | |
| return R, ErrRegistering | ||
| } | ||
|
|
||
| func (r Recorder) assertInitialized() error { | ||
| if !r.initialized { | ||
| return fmt.Errorf( | ||
| "ignoring the metrics recording for pipelineruns, failed to initialize the metrics recorder") | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // Count logs number of times a pipelinerun is ran for a provider. | ||
| func (r *Recorder) Count(provider, event, namespace, repository string) error { | ||
| if err := r.assertInitialized(); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| ctx, err := tag.New( | ||
| context.Background(), | ||
|
|
@@ -153,9 +179,8 @@ func (r *Recorder) Count(provider, event, namespace, repository string) error { | |
|
|
||
| // CountPRDuration collects duration taken by a pipelinerun in seconds accumulate them in prDurationCount. | ||
| func (r *Recorder) CountPRDuration(namespace, repository, status, reason string, duration time.Duration) error { | ||
| if err := r.assertInitialized(); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| ctx, err := tag.New( | ||
|
|
@@ -175,9 +200,8 @@ func (r *Recorder) CountPRDuration(namespace, repository, status, reason string, | |
|
|
||
| // RunningPipelineRuns emits the number of running PipelineRuns for a repository and namespace. | ||
| func (r *Recorder) RunningPipelineRuns(namespace, repository string, runningPRs float64) error { | ||
| if err := r.assertInitialized(); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| ctx, err := tag.New( | ||
|
|
@@ -266,6 +290,26 @@ func (r *Recorder) ReportRunningPipelineRuns(ctx context.Context, lister listers | |
| } | ||
| } | ||
|
|
||
| func (r *Recorder) ReportGitProviderAPIUsage(provider, event, namespace, repository string) error { | ||
| if err := r.assertInitialized(); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| ctx, err := tag.New( | ||
|
There was a problem hiding this comment. @aThorp96 at the moment metrics have event tag but user may want to filter out metrics based on event SHA, so it would be better to add it. There was a problem hiding this comment. Event sha could be useful for debugging. I worry about the cardinality of tags though if we introduce the SHA as a tag. WDYT? There was a problem hiding this comment. yeah, cardinality could be issue but there is no other way to differentiate that how many API calls were there on an event. for example it has eventType tag but there could be many push events so API request count in this metric will be cumulative in this case. There was a problem hiding this comment. That's fair. I think for that use-case may be better suited for tracing, replaying an event, using a unit or e2e test. The way I see this being used is to first identify particularly heavy event types and repositories. I think that is useful for an SRE alone, but if an SRE wanted to know how many API calls were made for a given event, I think there are more appropriate tools/techniques than metrics There was a problem hiding this comment.
I don't think that user may want to run e2e tests in production 🙂 |
||
| context.Background(), | ||
| tag.Insert(r.provider, provider), | ||
| tag.Insert(r.eventType, event), | ||
| tag.Insert(r.namespace, namespace), | ||
| tag.Insert(r.repository, repository), | ||
| ) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| metrics.Record(ctx, gitProviderAPIRequestCount.M(1)) | ||
| return nil | ||
| } | ||
|
|
||
| func ResetRecorder() { | ||
| Once = sync.Once{} | ||
| R = nil | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ import ( | |
| "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" | ||
| "github.com/openshift-pipelines/pipelines-as-code/pkg/changedfiles" | ||
| "github.com/openshift-pipelines/pipelines-as-code/pkg/events" | ||
| "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/info" | ||
| "github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype" | ||
|
|
@@ -22,13 +23,43 @@ import ( | |
| var _ provider.Interface = (*Provider)(nil) | ||
|
|
||
| type Provider struct { | ||
| bbClient *bitbucket.Client | ||
| Logger *zap.SugaredLogger | ||
| metrics *metrics.Recorder | ||
| run *params.Run | ||
| pacInfo *info.PacOpts | ||
| Token, APIURL *string | ||
| Username *string | ||
| provenance string | ||
| repo *v1alpha1.Repository | ||
| triggerEvent string | ||
| } | ||
|
|
||
| func (v Provider) Client() *bitbucket.Client { | ||
| v.recordAPIUsageMetrics() | ||
| return v.bbClient | ||
| } | ||
|
|
||
| func (v *Provider) recordAPIUsageMetrics() { | ||
|
There was a problem hiding this comment. @aThorp96 this is implemented for every provider and seems repeated code, can you find a way for a common logic, wdyt? |
||
| if v.metrics == nil { | ||
| m, err := metrics.NewRecorder() | ||
| if err != nil { | ||
| v.Logger.Errorf("Error initializing bitbucketcloud metrics recorder: %v", err) | ||
| return | ||
| } | ||
| v.metrics = m | ||
| } | ||
|
Comment on lines
+44
to
+51
There was a problem hiding this comment. @aThorp96 like doing this in a new method on provider interface e.g. |
||
|
|
||
| name := "" | ||
| namespace := "" | ||
| if v.repo != nil { | ||
| name = v.repo.Name | ||
| namespace = v.repo.Namespace | ||
| } | ||
|
|
||
| if err := v.metrics.ReportGitProviderAPIUsage("bitbucketcloud", v.triggerEvent, namespace, name); err != nil { | ||
|
|
||
| v.Logger.Errorf("Error reporting git API usage metrics: %v", err) | ||
| } | ||
| } | ||
|
|
||
| // CheckPolicyAllowing TODO: Implement ME. | ||
|
|
@@ -104,11 +135,11 @@ func (v *Provider) CreateStatus(_ context.Context, event *info.Event, statusopts | |
| Revision: event.SHA, | ||
| } | ||
|
|
||
| if v.bbClient == nil { | ||
| return fmt.Errorf("no token has been set, cannot set status") | ||
| } | ||
|
|
||
| _, err := v.Client().Repositories.Commits.CreateCommitStatus(cmo, cso) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
@@ -121,7 +152,7 @@ func (v *Provider) CreateStatus(_ context.Context, event *info.Event, statusopts | |
| if statusopts.OriginalPipelineRunName != "" { | ||
| onPr = "/" + statusopts.OriginalPipelineRunName | ||
| } | ||
| _, err = v.Client().Repositories.PullRequests.AddComment( | ||
| &bitbucket.PullRequestCommentOptions{ | ||
| Owner: event.Organization, | ||
| RepoSlug: event.Repository, | ||
|
|
@@ -161,7 +192,7 @@ func (v *Provider) getDir(event *info.Event, path string) ([]bitbucket.Repositor | |
| Path: path, | ||
| } | ||
|
|
||
| repositoryFiles, err := v.Client().Repositories.Repository.ListFiles(repoFileOpts) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
@@ -176,17 +207,19 @@ func (v *Provider) GetFileInsideRepo(_ context.Context, event *info.Event, path, | |
| return v.getBlob(event, revision, path) | ||
| } | ||
|
|
||
| func (v *Provider) SetClient(_ context.Context, run *params.Run, event *info.Event, repo *v1alpha1.Repository, _ *events.EventEmitter) error { | ||
| if event.Provider.Token == "" { | ||
| return fmt.Errorf("no git_provider.secret has been set in the repo crd") | ||
| } | ||
| if event.Provider.User == "" { | ||
| return fmt.Errorf("no git_provider.user has been in repo crd") | ||
| } | ||
| v.bbClient = bitbucket.NewBasicAuth(event.Provider.User, event.Provider.Token) | ||
| v.Token = &event.Provider.Token | ||
| v.Username = &event.Provider.User | ||
| v.run = run | ||
| v.repo = repo | ||
| v.triggerEvent = event.EventType | ||
| return nil | ||
| } | ||
|
|
||
|
|
@@ -195,7 +228,7 @@ func (v *Provider) GetCommitInfo(_ context.Context, event *info.Event) error { | |
| if branchortag == "" { | ||
| branchortag = event.HeadBranch | ||
| } | ||
| response, err := v.Client().Repositories.Commits.GetCommits(&bitbucket.CommitsOptions{ | ||
| Owner: event.Organization, | ||
| RepoSlug: event.Repository, | ||
| Branchortag: branchortag, | ||
|
|
@@ -226,7 +259,7 @@ func (v *Provider) GetCommitInfo(_ context.Context, event *info.Event) error { | |
| event.SHA = commitinfo.Hash | ||
|
|
||
| // now to get the default branch from repository.Get | ||
| repo, err := v.Client().Repositories.Repository.Get(&bitbucket.RepositoryOptions{ | ||
| Owner: event.Organization, | ||
| RepoSlug: event.Repository, | ||
| }) | ||
|
|
@@ -278,7 +311,7 @@ func (v *Provider) concatAllYamlFiles(objects []bitbucket.RepositoryFile, event | |
| } | ||
|
|
||
| func (v *Provider) getBlob(runevent *info.Event, ref, path string) (string, error) { | ||
| blob, err := v.Client().Repositories.Repository.GetFileBlob(&bitbucket.RepositoryBlobOptions{ | ||
| Owner: runevent.Organization, | ||
| RepoSlug: runevent.Repository, | ||
| Ref: ref, | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would that give the ability to track each provider separately ? i mean is the admin can get insight how many calls has been done for gitlab and for github app when both are used on the same cluster
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chmouel Yes, this metric can be filtered using
providertag.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly. Additionally the provider tag for Gitlab and Gitea will use the API URL so you can see insight per provider-instance. Similarly Github will use either
githuborgithub-enterpriserespectively