Skip to content

Commit a2b6047

Browse files
iamayushmiamayushm
authored
fix: incorrect gitops metrics (#6444)
* wip: fix gitops metrics * fix pull metrics * github create repo function * wip: metrics changed from defer function * fix metrics publish * self review changes
1 parent 81e2abf commit a2b6047

File tree

14 files changed

+143
-84
lines changed

14 files changed

+143
-84
lines changed

‎client/argocdServer/ArgoClientWrapperService.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ func (impl *ArgoClientWrapperServiceImpl) RegisterGitOpsRepoInArgoWithRetry(ctx
348348
return nil
349349
}
350350

351-
callback := func() error {
351+
callback := func(int) error {
352352
return impl.createRepoInArgoCd(ctx, grpcConfig, gitOpsRepoUrl)
353353
}
354354
argoCdErr := retryFunc.Retry(callback,

‎cmd/external-app/wire_gen.go‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/deployment/gitOps/git/GitOperationService.go‎

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ type GitOperationService interface {
5151
PushChartToGitOpsRepoForHelmApp(ctx context.Context, pushChartToGitRequest *bean.PushChartToGitRequestDTO, requirementsConfig, valuesConfig *ChartConfig) (*commonBean.ChartGitAttribute, string, error)
5252

5353
CreateRepository(ctx context.Context, dto *apiBean.GitOpsConfigDto, userId int32) (string, bool, bool, error)
54-
GetRepoUrlByRepoName(repoName string) (string, error)
5554

5655
GetClonedDir(ctx context.Context, chartDir, repoUrl, targetRevision string) (string, error)
5756
ReloadGitOpsProvider() error
@@ -170,7 +169,7 @@ func (impl *GitOperationServiceImpl) PushChartToGitRepo(ctx context.Context, git
170169
commit, err := impl.gitFactory.GitOpsHelper.CommitAndPushAllChanges(newCtx, clonedDir, targetRevision, "first commit", userName, userEmailId)
171170
if err != nil {
172171
impl.logger.Errorw("error in pushing git", "err", err)
173-
callback := func() error {
172+
callback := func(int) error {
174173
commit, err = impl.updateRepoAndPushAllChanges(newCtx, clonedDir, repoUrl, targetRevision,
175174
tempReferenceTemplateDir, dir, userName, userEmailId, impl.gitFactory.GitOpsHelper)
176175
return err
@@ -264,8 +263,12 @@ func (impl *GitOperationServiceImpl) CommitValues(ctx context.Context, chartGitA
264263
return commitHash, commitTime, err
265264
}
266265
gitOpsConfig := &apiBean.GitOpsConfigDto{BitBucketWorkspaceId: bitbucketMetadata.BitBucketWorkspaceId}
267-
callback := func() error {
268-
commitHash, commitTime, err = impl.gitFactory.Client.CommitValues(newCtx, chartGitAttr, gitOpsConfig)
266+
callback := func(retriesLeft int) error {
267+
publishStatusConflictError := false
268+
if retriesLeft <= 0 {
269+
publishStatusConflictError = true
270+
}
271+
commitHash, commitTime, err = impl.gitFactory.Client.CommitValues(newCtx, chartGitAttr, gitOpsConfig, publishStatusConflictError)
269272
return err
270273
}
271274
err = retryFunc.Retry(callback, impl.isRetryableGitCommitError,
@@ -303,26 +306,6 @@ func (impl *GitOperationServiceImpl) CreateRepository(ctx context.Context, dto *
303306
return repoUrl, isNew, isEmpty, nil
304307
}
305308

306-
func (impl *GitOperationServiceImpl) GetRepoUrlByRepoName(repoName string) (string, error) {
307-
repoUrl := ""
308-
bitbucketMetadata, err := impl.gitOpsConfigReadService.GetBitbucketMetadata()
309-
if err != nil {
310-
impl.logger.Errorw("error in getting bitbucket metadata", "err", err)
311-
return repoUrl, err
312-
}
313-
dto := &apiBean.GitOpsConfigDto{
314-
GitRepoName: repoName,
315-
BitBucketWorkspaceId: bitbucketMetadata.BitBucketWorkspaceId,
316-
BitBucketProjectKey: bitbucketMetadata.BitBucketProjectKey,
317-
}
318-
repoUrl, _, err = impl.gitFactory.Client.GetRepoUrl(dto)
319-
if err != nil {
320-
//will allow to continue to persist status on next operation
321-
impl.logger.Errorw("error in getting repo url", "err", err, "repoName", repoName)
322-
}
323-
return repoUrl, nil
324-
}
325-
326309
// PushChartToGitOpsRepoForHelmApp pushes built chart to GitOps repo (Specific implementation for Helm Apps)
327310
// TODO refactoring: Make a common method for both PushChartToGitRepo and PushChartToGitOpsRepoForHelmApp
328311
func (impl *GitOperationServiceImpl) PushChartToGitOpsRepoForHelmApp(ctx context.Context, pushChartToGitRequest *bean.PushChartToGitRequestDTO, requirementsConfig, valuesConfig *ChartConfig) (*commonBean.ChartGitAttribute, string, error) {

‎pkg/deployment/gitOps/git/GitOpsClient.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import (
3030

3131
type GitOpsClient interface {
3232
CreateRepository(ctx context.Context, config *gitOps.GitOpsConfigDto) (url string, isNew bool, isEmpty bool, detailedErrorGitOpsConfigActions DetailedErrorGitOpsConfigActions)
33-
CommitValues(ctx context.Context, config *ChartConfig, gitOpsConfig *gitOps.GitOpsConfigDto) (commitHash string, commitTime time.Time, err error)
33+
CommitValues(ctx context.Context, config *ChartConfig, gitOpsConfig *gitOps.GitOpsConfigDto, publishStatusConflictError bool) (commitHash string, commitTime time.Time, err error)
3434
GetRepoUrl(config *gitOps.GitOpsConfigDto) (repoUrl string, isRepoEmpty bool, err error)
3535
DeleteRepository(config *gitOps.GitOpsConfigDto) error
3636
CreateReadme(ctx context.Context, config *gitOps.GitOpsConfigDto) (string, error)

‎pkg/deployment/gitOps/git/GitOpsHelper.go‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,15 @@ func (impl *GitOpsHelper) Clone(url, targetDir, targetRevision string) (clonedDi
9797

9898
func (impl *GitOpsHelper) Pull(repoRoot, targetRevision string) (err error) {
9999
start := time.Now()
100-
defer func() {
101-
util.TriggerGitOpsMetrics("Pull", "GitService", start, err)
102-
}()
103100
ctx := git.BuildGitContext(context.Background()).WithCredentials(impl.Auth).
104101
WithTLSData(impl.tlsConfig.CaData, impl.tlsConfig.TLSKeyData, impl.tlsConfig.TLSCertData, impl.isTlsEnabled)
105-
return impl.gitCommandManager.Pull(ctx, targetRevision, repoRoot)
102+
err = impl.gitCommandManager.Pull(ctx, targetRevision, repoRoot)
103+
if err != nil {
104+
util.TriggerGitOpsMetrics("Pull", "GitService", start, err)
105+
return err
106+
}
107+
util.TriggerGitOpsMetrics("Pull", "GitService", start, nil)
108+
return nil
106109
}
107110

108111
const PushErrorMessage = "failed to push some refs"
@@ -129,11 +132,14 @@ func (impl *GitOpsHelper) pullFromBranch(ctx git.GitContext, rootDir, targetRevi
129132
impl.logger.Warnw("no branch found in git repo", "rootDir", rootDir)
130133
return "", "", err
131134
}
135+
start := time.Now()
132136
response, errMsg, err := impl.gitCommandManager.PullCli(ctx, rootDir, branch)
133137
if err != nil {
138+
util.TriggerGitOpsMetrics("Pull", "GitCli", start, err)
134139
impl.logger.Errorw("error on git pull", "branch", branch, "err", err)
135140
return response, errMsg, err
136141
}
142+
util.TriggerGitOpsMetrics("Pull", "GitCli", start, nil)
137143
return response, errMsg, err
138144
}
139145

‎pkg/deployment/gitOps/git/GitServiceAzure.go‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -107,19 +107,18 @@ func (impl GitAzureClient) CreateRepository(ctx context.Context, config *bean2.G
107107
repoExists bool
108108
)
109109
start := time.Now()
110-
defer func() {
111-
globalUtil.TriggerGitOpsMetrics("CreateRepository", "GitAzureClient", start, err)
112-
}()
113110

114111
detailedErrorGitOpsConfigActions.StageErrorMap = make(map[string]error)
115112
url, repoExists, isEmpty, err = impl.repoExists(config.GitRepoName, impl.project)
116113
if err != nil {
117114
impl.logger.Errorw("error in communication with azure", "err", err)
118115
detailedErrorGitOpsConfigActions.StageErrorMap[GetRepoUrlStage] = err
116+
globalUtil.TriggerGitOpsMetrics("CreateRepository", "GitAzureClient", start, err)
119117
return "", false, isEmpty, detailedErrorGitOpsConfigActions
120118
}
121119
if repoExists {
122120
detailedErrorGitOpsConfigActions.SuccessfulStages = append(detailedErrorGitOpsConfigActions.SuccessfulStages, GetRepoUrlStage)
121+
globalUtil.TriggerGitOpsMetrics("CreateRepository", "GitAzureClient", start, nil)
123122
return url, false, isEmpty, detailedErrorGitOpsConfigActions
124123
}
125124
gitRepositoryCreateOptions := git.GitRepositoryCreateOptions{
@@ -138,6 +137,7 @@ func (impl GitAzureClient) CreateRepository(ctx context.Context, config *bean2.G
138137
impl.logger.Errorw("error in communication with azure", "err", err)
139138
}
140139
if err != nil || !repoExists {
140+
globalUtil.TriggerGitOpsMetrics("CreateRepository", "GitAzureClient", start, err)
141141
return "", true, isEmpty, detailedErrorGitOpsConfigActions
142142
}
143143
}
@@ -146,11 +146,14 @@ func (impl GitAzureClient) CreateRepository(ctx context.Context, config *bean2.G
146146
validated, err := impl.ensureProjectAvailabilityOnHttp(config.GitRepoName)
147147
if err != nil {
148148
impl.logger.Errorw("error in ensuring project availability azure", "project", config.GitRepoName, "err", err)
149+
globalUtil.TriggerGitOpsMetrics("CreateRepository", "GitAzureClient", start, err)
149150
detailedErrorGitOpsConfigActions.StageErrorMap[CloneHttpStage] = err
150151
return *operationReference.WebUrl, true, isEmpty, detailedErrorGitOpsConfigActions
151152
}
152153
if !validated {
153-
detailedErrorGitOpsConfigActions.StageErrorMap[CloneHttpStage] = fmt.Errorf("unable to validate project:%s in given time", config.GitRepoName)
154+
err = fmt.Errorf("unable to validate project:%s in given time", config.GitRepoName)
155+
detailedErrorGitOpsConfigActions.StageErrorMap[CloneHttpStage] = err
156+
globalUtil.TriggerGitOpsMetrics("CreateRepository", "GitAzureClient", start, err)
154157
return "", true, isEmpty, detailedErrorGitOpsConfigActions
155158
}
156159
detailedErrorGitOpsConfigActions.SuccessfulStages = append(detailedErrorGitOpsConfigActions.SuccessfulStages, CloneHttpStage)
@@ -159,6 +162,7 @@ func (impl GitAzureClient) CreateRepository(ctx context.Context, config *bean2.G
159162
if err != nil {
160163
impl.logger.Errorw("error in creating readme azure", "project", config.GitRepoName, "err", err)
161164
detailedErrorGitOpsConfigActions.StageErrorMap[CreateReadmeStage] = err
165+
globalUtil.TriggerGitOpsMetrics("CreateRepository", "GitAzureClient", start, err)
162166
return *operationReference.WebUrl, true, isEmpty, detailedErrorGitOpsConfigActions
163167
}
164168
isEmpty = false //As we have created readme, repo is no longer empty
@@ -167,13 +171,17 @@ func (impl GitAzureClient) CreateRepository(ctx context.Context, config *bean2.G
167171
if err != nil {
168172
impl.logger.Errorw("error in ensuring project availability azure", "project", config.GitRepoName, "err", err)
169173
detailedErrorGitOpsConfigActions.StageErrorMap[CloneSshStage] = err
174+
globalUtil.TriggerGitOpsMetrics("CreateRepository", "GitAzureClient", start, err)
170175
return *operationReference.WebUrl, true, isEmpty, detailedErrorGitOpsConfigActions
171176
}
172177
if !validated {
173-
detailedErrorGitOpsConfigActions.StageErrorMap[CloneSshStage] = fmt.Errorf("unable to validate project:%s in given time", config.GitRepoName)
178+
err = fmt.Errorf("unable to validate project:%s in given time", config.GitRepoName)
179+
detailedErrorGitOpsConfigActions.StageErrorMap[CloneSshStage] = err
180+
globalUtil.TriggerGitOpsMetrics("CreateRepository", "GitAzureClient", start, err)
174181
return "", true, isEmpty, detailedErrorGitOpsConfigActions
175182
}
176183
detailedErrorGitOpsConfigActions.SuccessfulStages = append(detailedErrorGitOpsConfigActions.SuccessfulStages, CloneSshStage)
184+
globalUtil.TriggerGitOpsMetrics("CreateRepository", "GitAzureClient", start, nil)
177185
return *operationReference.WebUrl, true, isEmpty, detailedErrorGitOpsConfigActions
178186
}
179187

@@ -196,14 +204,15 @@ func (impl GitAzureClient) CreateReadme(ctx context.Context, config *bean2.GitOp
196204
UserName: config.Username,
197205
UserEmailId: config.UserEmailId,
198206
}
199-
hash, _, err := impl.CommitValues(ctx, cfg, config)
207+
hash, _, err := impl.CommitValues(ctx, cfg, config, true)
200208
if err != nil {
201209
impl.logger.Errorw("error in creating readme azure", "repo", config.GitRepoName, "err", err)
202210
}
203211
return hash, err
204212
}
205213

206-
func (impl GitAzureClient) CommitValues(ctx context.Context, config *ChartConfig, gitOpsConfig *bean2.GitOpsConfigDto) (commitHash string, commitTime time.Time, err error) {
214+
func (impl GitAzureClient) CommitValues(ctx context.Context, config *ChartConfig, gitOpsConfig *bean2.GitOpsConfigDto, publishStatusConflictError bool) (commitHash string, commitTime time.Time, err error) {
215+
start := time.Now()
207216
branch := config.TargetRevision
208217
if len(branch) == 0 {
209218
branch = globalUtil.GetDefaultTargetRevision()
@@ -229,13 +238,15 @@ func (impl GitAzureClient) CommitValues(ctx context.Context, config *ChartConfig
229238
if err != nil {
230239
if e, ok := err.(azuredevops.WrappedError); !ok || *e.StatusCode >= 500 {
231240
impl.logger.Errorw("error in fetching branch from azure devops", "err", err)
241+
globalUtil.TriggerGitOpsMetrics("CommitValues", "GitAzureClient", start, err)
232242
return "", time.Time{}, err
233243
}
234244
} else if branchStat != nil {
235245
oldObjId = *branchStat.Commit.CommitId
236246
}
237247
} else {
238248
impl.logger.Errorw("error in fetching file from azure devops", "err", err)
249+
globalUtil.TriggerGitOpsMetrics("CommitValues", "GitAzureClient", start, err)
239250
return "", time.Time{}, err
240251
}
241252
} else {
@@ -292,9 +303,13 @@ func (impl GitAzureClient) CommitValues(ctx context.Context, config *ChartConfig
292303
})
293304
if e := (azuredevops.WrappedError{}); errors.As(err, &e) && e.StatusCode != nil && *e.StatusCode == http2.StatusConflict {
294305
impl.logger.Warn("conflict found in commit azure", "err", err, "config", config)
306+
if publishStatusConflictError {
307+
globalUtil.TriggerGitOpsMetrics("CommitValues", "GitAzureClient", start, err)
308+
}
295309
return "", time.Time{}, retryFunc.NewRetryableError(err)
296310
} else if err != nil {
297311
impl.logger.Errorw("error in commit azure", "err", err)
312+
globalUtil.TriggerGitOpsMetrics("CommitValues", "GitAzureClient", start, err)
298313
return "", time.Time{}, err
299314
}
300315
//gitPush.Commits
@@ -305,6 +320,7 @@ func (impl GitAzureClient) CommitValues(ctx context.Context, config *ChartConfig
305320
commitAuthorTime = (*push.Commits)[0].Author.Date.Time
306321
}
307322
// push.Commits[0].CommitId
323+
globalUtil.TriggerGitOpsMetrics("CommitValues", "GitAzureClient", start, nil)
308324
return commitId, commitAuthorTime, nil
309325
}
310326

0 commit comments

Comments
 (0)