Skip to content

Commit

Permalink
Enhance logging in EventWatcher (#5443)
Browse files Browse the repository at this point in the history 
* Add logger.Error() before returning for detailed logs

Signed-off-by: t-kikuc <tkikuchi07f@gmail.com>

* Enhance error logging in event watcher with additional context for push and commit failures

Signed-off-by: t-kikuc <tkikuchi07f@gmail.com>

---------

Signed-off-by: t-kikuc <tkikuchi07f@gmail.com>
Signed-off-by: pipecd-bot <pipecd.dev@gmail.com>
  • Loading branch information
@t-kikuc @pipecd-bot
t-kikuc authored and pipecd-bot committed Jan 21, 2025
1 parent c26955d commit 6c0bae0
Showing 1 changed file with 48 additions and 20 deletions.
68 changes: 48 additions & 20 deletions pkg/app/piped/eventwatcher/eventwatcher.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,14 +126,16 @@ func (w *watcher) Run(ctx context.Context) error {

workingDir, err := os.MkdirTemp("", "event-watcher")
if err != nil {
return fmt.Errorf("failed to create the working directory: %w", err)
w.logger.Error("failed to create the working directory", zap.Error(err))
return err
}
defer os.RemoveAll(workingDir)
w.workingDir = workingDir

for _, r := range w.config.Repositories {
repo, err := w.cloneRepo(ctx, r)
if err != nil {
w.logger.Error("failed to clone repository", zap.String("repo-id", r.RepoID), zap.Error(err))
return err
}
defer repo.Clean()
Expand Down Expand Up @@ -303,11 +305,13 @@ func (w *watcher) run(ctx context.Context, repo git.Repo, repoCfg config.PipedRe
func (w *watcher) cloneRepo(ctx context.Context, repoCfg config.PipedRepository) (git.Repo, error) {
dst, err := os.MkdirTemp(w.workingDir, repoCfg.RepoID)
if err != nil {
return nil, fmt.Errorf("failed to create a new temporary directory: %w", err)
w.logger.Error("failed to create a new temporary directory", zap.Error(err))
return nil, err
}
repo, err := w.gitClient.Clone(ctx, repoCfg.RepoID, repoCfg.Remote, repoCfg.Branch, dst)
if err != nil {
return nil, fmt.Errorf("failed to clone repository %s: %w", repoCfg.RepoID, err)
w.logger.Error("failed to clone repository", zap.String("repo-id", repoCfg.RepoID), zap.Error(err))
return nil, err
}
return repo, nil
}
Expand All @@ -317,11 +321,13 @@ func (w *watcher) execute(ctx context.Context, repo git.Repo, repoID string, eve
// Copy the repo to another directory to modify local file to avoid reverting previous changes.
tmpDir, err := os.MkdirTemp(w.workingDir, "repo")
if err != nil {
return fmt.Errorf("failed to create a new temporary directory: %w", err)
w.logger.Error("failed to create a new temporary directory", zap.Error(err))
return err
}
tmpRepo, err := repo.CopyToModify(filepath.Join(tmpDir, "tmp-repo"))
if err != nil {
return fmt.Errorf("failed to copy the repository to the temporary directory: %w", err)
w.logger.Error("failed to copy the repository to the temporary directory", zap.Error(err))
return err
}
// nolint: errcheck
defer tmpRepo.Clean()
Expand Down Expand Up @@ -368,7 +374,8 @@ func (w *watcher) execute(ctx context.Context, repo git.Repo, repoID string, eve
Labels: matcher.Labels,
})
if err != nil {
return fmt.Errorf("failed to get the latest event: %w", err)
w.logger.Error("failed to get the latest event", zap.Error(err))
return err
}
// The case where the latest event has already been handled.
if resp.Event.CreatedAt > latestEvent.CreatedAt {
Expand Down Expand Up @@ -429,7 +436,8 @@ func (w *watcher) execute(ctx context.Context, repo git.Repo, repoID string, eve
}
if len(outDatedEvents) > 0 {
if _, err := w.apiClient.ReportEventStatuses(ctx, &pipedservice.ReportEventStatusesRequest{Events: outDatedEvents}); err != nil {
return fmt.Errorf("failed to report event statuses: %w", err)
w.logger.Error("failed to report event statuses", zap.Error(err))
return err
}
w.logger.Info(fmt.Sprintf("successfully made %d events OUTDATED", len(outDatedEvents)))
}
Expand All @@ -449,8 +457,11 @@ func (w *watcher) execute(ctx context.Context, repo git.Repo, repoID string, eve
retry := backoff.NewRetry(retryPushNum, backoff.NewConstant(retryPushInterval))
for branch, events := range branchHandledEvents {
_, err = retry.Do(ctx, func() (interface{}, error) {
err := tmpRepo.Push(ctx, branch)
return nil, err
if err := tmpRepo.Push(ctx, branch); err != nil {
w.logger.Error("failed to push commits", zap.String("repo-id", repoID), zap.String("branch", branch), zap.Error(err))
return nil, err
}
return nil, nil
})

if err == nil {
Expand Down Expand Up @@ -493,11 +504,13 @@ func (w *watcher) updateValues(ctx context.Context, repo git.Repo, repoID string
// Copy the repo to another directory to modify local file to avoid reverting previous changes.
tmpDir, err := os.MkdirTemp(w.workingDir, "repo")
if err != nil {
return fmt.Errorf("failed to create a new temporary directory: %w", err)
w.logger.Error("failed to create a new temporary directory", zap.Error(err))
return err
}
tmpRepo, err := repo.CopyToModify(filepath.Join(tmpDir, "tmp-repo"))
if err != nil {
return fmt.Errorf("failed to copy the repository to the temporary directory: %w", err)
w.logger.Error("failed to copy the repository to the temporary directory", zap.Error(err))
return err
}
defer tmpRepo.Clean()

Expand Down Expand Up @@ -536,7 +549,8 @@ func (w *watcher) updateValues(ctx context.Context, repo git.Repo, repoID string
Labels: e.Labels,
})
if err != nil {
return fmt.Errorf("failed to get the latest event: %w", err)
w.logger.Error("failed to get the latest event", zap.Error(err))
return err
}
// The case where the latest event has already been handled.
if resp.Event.CreatedAt > latestEvent.CreatedAt {
Expand Down Expand Up @@ -577,7 +591,8 @@ func (w *watcher) updateValues(ctx context.Context, repo git.Repo, repoID string
}
if len(outDatedEvents) > 0 {
if _, err := w.apiClient.ReportEventStatuses(ctx, &pipedservice.ReportEventStatusesRequest{Events: outDatedEvents}); err != nil {
return fmt.Errorf("failed to report event statuses: %w", err)
w.logger.Error("failed to report event statuses", zap.Error(err))
return err
}
w.logger.Info(fmt.Sprintf("successfully made %d events OUTDATED", len(outDatedEvents)))
}
Expand All @@ -587,12 +602,16 @@ func (w *watcher) updateValues(ctx context.Context, repo git.Repo, repoID string

retry := backoff.NewRetry(retryPushNum, backoff.NewConstant(retryPushInterval))
_, err = retry.Do(ctx, func() (interface{}, error) {
err := tmpRepo.Push(ctx, tmpRepo.GetClonedBranch())
return nil, err
if err := tmpRepo.Push(ctx, tmpRepo.GetClonedBranch()); err != nil {
w.logger.Error("failed to push commits", zap.String("repo-id", repoID), zap.String("branch", tmpRepo.GetClonedBranch()), zap.Error(err))
return nil, err
}
return nil, nil
})
if err == nil {
if _, err := w.apiClient.ReportEventStatuses(ctx, &pipedservice.ReportEventStatusesRequest{Events: handledEvents}); err != nil {
return fmt.Errorf("failed to report event statuses: %w", err)
w.logger.Error("failed to report event statuses", zap.Error(err))
return err
}
w.milestoneMap.Store(repoID, maxTimestamp)
return nil
Expand All @@ -613,10 +632,12 @@ func (w *watcher) updateValues(ctx context.Context, repo git.Repo, repoID string
handledEvents[i].StatusDescription = fmt.Sprintf("Failed to push changed files: %v", err)
}
if _, err := w.apiClient.ReportEventStatuses(ctx, &pipedservice.ReportEventStatusesRequest{Events: handledEvents}); err != nil {
return fmt.Errorf("failed to report event statuses: %w", err)
w.logger.Error("failed to report event statuses: %w", zap.Error(err))
return err
}
w.milestoneMap.Store(repoID, maxTimestamp)
return fmt.Errorf("failed to push commits: %w", err)
w.logger.Error("failed to push commits", zap.Error(err))
return err
}

// commitFiles commits changes if the data in Git is different from the latest event.
Expand Down Expand Up @@ -647,14 +668,16 @@ func (w *watcher) commitFiles(ctx context.Context, latestEvent *model.Event, eve
newContent, upToDate, err = modifyText(path, r.Regex, latestEvent.Data)
}
if err != nil {
w.logger.Error("failed to modify file", zap.Error(err))
return "", err
}
if upToDate {
continue
}

if err := os.WriteFile(path, newContent, os.ModePerm); err != nil {
return "", fmt.Errorf("failed to write file: %w", err)
w.logger.Error("failed to write file", zap.Error(err))
return "", err
}
changes[filePath] = newContent
}
Expand All @@ -670,7 +693,12 @@ func (w *watcher) commitFiles(ctx context.Context, latestEvent *model.Event, eve
branch := makeBranchName(newBranch, eventName, repo.GetClonedBranch())
trailers := maps.Clone(latestEvent.Contexts)
if err := repo.CommitChanges(ctx, branch, commitMsg, newBranch, changes, trailers); err != nil {
return "", fmt.Errorf("failed to perform git commit: %w", err)
w.logger.Error("failed to perform git commit",
zap.String("branch", branch),
zap.Bool("make-new-branch", newBranch),
zap.Int("changed-files", len(changes)),
zap.Error(err))
return "", err
}
w.logger.Info(fmt.Sprintf("event watcher will update values of Event %q", eventName))
return branch, nil
Expand Down

0 comments on commit 6c0bae0

Please sign in to comment.