Skip to content

Commit e00b429

Browse files
t-kikuct-kikuc
authored
Enhance logging in EventWatcher (#5443)
* Add logger.Error() before returning for detailed logs Signed-off-by: t-kikuc <[email protected]> * Enhance error logging in event watcher with additional context for push and commit failures Signed-off-by: t-kikuc <[email protected]> --------- Signed-off-by: t-kikuc <[email protected]>
1 parent bcdba1f commit e00b429

File tree

1 file changed

+48
-20
lines changed

1 file changed

+48
-20
lines changed

‎pkg/app/piped/eventwatcher/eventwatcher.go

+48-20
Original file line numberDiff line numberDiff line change
@@ -126,14 +126,16 @@ func (w *watcher) Run(ctx context.Context) error {
126126

127127
workingDir, err := os.MkdirTemp("", "event-watcher")
128128
if err != nil {
129-
return fmt.Errorf("failed to create the working directory: %w", err)
129+
w.logger.Error("failed to create the working directory", zap.Error(err))
130+
return err
130131
}
131132
defer os.RemoveAll(workingDir)
132133
w.workingDir = workingDir
133134

134135
for _, r := range w.config.Repositories {
135136
repo, err := w.cloneRepo(ctx, r)
136137
if err != nil {
138+
w.logger.Error("failed to clone repository", zap.String("repo-id", r.RepoID), zap.Error(err))
137139
return err
138140
}
139141
defer repo.Clean()
@@ -303,11 +305,13 @@ func (w *watcher) run(ctx context.Context, repo git.Repo, repoCfg config.PipedRe
303305
func (w *watcher) cloneRepo(ctx context.Context, repoCfg config.PipedRepository) (git.Repo, error) {
304306
dst, err := os.MkdirTemp(w.workingDir, repoCfg.RepoID)
305307
if err != nil {
306-
return nil, fmt.Errorf("failed to create a new temporary directory: %w", err)
308+
w.logger.Error("failed to create a new temporary directory", zap.Error(err))
309+
return nil, err
307310
}
308311
repo, err := w.gitClient.Clone(ctx, repoCfg.RepoID, repoCfg.Remote, repoCfg.Branch, dst)
309312
if err != nil {
310-
return nil, fmt.Errorf("failed to clone repository %s: %w", repoCfg.RepoID, err)
313+
w.logger.Error("failed to clone repository", zap.String("repo-id", repoCfg.RepoID), zap.Error(err))
314+
return nil, err
311315
}
312316
return repo, nil
313317
}
@@ -317,11 +321,13 @@ func (w *watcher) execute(ctx context.Context, repo git.Repo, repoID string, eve
317321
// Copy the repo to another directory to modify local file to avoid reverting previous changes.
318322
tmpDir, err := os.MkdirTemp(w.workingDir, "repo")
319323
if err != nil {
320-
return fmt.Errorf("failed to create a new temporary directory: %w", err)
324+
w.logger.Error("failed to create a new temporary directory", zap.Error(err))
325+
return err
321326
}
322327
tmpRepo, err := repo.CopyToModify(filepath.Join(tmpDir, "tmp-repo"))
323328
if err != nil {
324-
return fmt.Errorf("failed to copy the repository to the temporary directory: %w", err)
329+
w.logger.Error("failed to copy the repository to the temporary directory", zap.Error(err))
330+
return err
325331
}
326332
// nolint: errcheck
327333
defer tmpRepo.Clean()
@@ -368,7 +374,8 @@ func (w *watcher) execute(ctx context.Context, repo git.Repo, repoID string, eve
368374
Labels: matcher.Labels,
369375
})
370376
if err != nil {
371-
return fmt.Errorf("failed to get the latest event: %w", err)
377+
w.logger.Error("failed to get the latest event", zap.Error(err))
378+
return err
372379
}
373380
// The case where the latest event has already been handled.
374381
if resp.Event.CreatedAt > latestEvent.CreatedAt {
@@ -429,7 +436,8 @@ func (w *watcher) execute(ctx context.Context, repo git.Repo, repoID string, eve
429436
}
430437
if len(outDatedEvents) > 0 {
431438
if _, err := w.apiClient.ReportEventStatuses(ctx, &pipedservice.ReportEventStatusesRequest{Events: outDatedEvents}); err != nil {
432-
return fmt.Errorf("failed to report event statuses: %w", err)
439+
w.logger.Error("failed to report event statuses", zap.Error(err))
440+
return err
433441
}
434442
w.logger.Info(fmt.Sprintf("successfully made %d events OUTDATED", len(outDatedEvents)))
435443
}
@@ -449,8 +457,11 @@ func (w *watcher) execute(ctx context.Context, repo git.Repo, repoID string, eve
449457
retry := backoff.NewRetry(retryPushNum, backoff.NewConstant(retryPushInterval))
450458
for branch, events := range branchHandledEvents {
451459
_, err = retry.Do(ctx, func() (interface{}, error) {
452-
err := tmpRepo.Push(ctx, branch)
453-
return nil, err
460+
if err := tmpRepo.Push(ctx, branch); err != nil {
461+
w.logger.Error("failed to push commits", zap.String("repo-id", repoID), zap.String("branch", branch), zap.Error(err))
462+
return nil, err
463+
}
464+
return nil, nil
454465
})
455466

456467
if err == nil {
@@ -493,11 +504,13 @@ func (w *watcher) updateValues(ctx context.Context, repo git.Repo, repoID string
493504
// Copy the repo to another directory to modify local file to avoid reverting previous changes.
494505
tmpDir, err := os.MkdirTemp(w.workingDir, "repo")
495506
if err != nil {
496-
return fmt.Errorf("failed to create a new temporary directory: %w", err)
507+
w.logger.Error("failed to create a new temporary directory", zap.Error(err))
508+
return err
497509
}
498510
tmpRepo, err := repo.CopyToModify(filepath.Join(tmpDir, "tmp-repo"))
499511
if err != nil {
500-
return fmt.Errorf("failed to copy the repository to the temporary directory: %w", err)
512+
w.logger.Error("failed to copy the repository to the temporary directory", zap.Error(err))
513+
return err
501514
}
502515
defer tmpRepo.Clean()
503516

@@ -536,7 +549,8 @@ func (w *watcher) updateValues(ctx context.Context, repo git.Repo, repoID string
536549
Labels: e.Labels,
537550
})
538551
if err != nil {
539-
return fmt.Errorf("failed to get the latest event: %w", err)
552+
w.logger.Error("failed to get the latest event", zap.Error(err))
553+
return err
540554
}
541555
// The case where the latest event has already been handled.
542556
if resp.Event.CreatedAt > latestEvent.CreatedAt {
@@ -577,7 +591,8 @@ func (w *watcher) updateValues(ctx context.Context, repo git.Repo, repoID string
577591
}
578592
if len(outDatedEvents) > 0 {
579593
if _, err := w.apiClient.ReportEventStatuses(ctx, &pipedservice.ReportEventStatusesRequest{Events: outDatedEvents}); err != nil {
580-
return fmt.Errorf("failed to report event statuses: %w", err)
594+
w.logger.Error("failed to report event statuses", zap.Error(err))
595+
return err
581596
}
582597
w.logger.Info(fmt.Sprintf("successfully made %d events OUTDATED", len(outDatedEvents)))
583598
}
@@ -587,12 +602,16 @@ func (w *watcher) updateValues(ctx context.Context, repo git.Repo, repoID string
587602

588603
retry := backoff.NewRetry(retryPushNum, backoff.NewConstant(retryPushInterval))
589604
_, err = retry.Do(ctx, func() (interface{}, error) {
590-
err := tmpRepo.Push(ctx, tmpRepo.GetClonedBranch())
591-
return nil, err
605+
if err := tmpRepo.Push(ctx, tmpRepo.GetClonedBranch()); err != nil {
606+
w.logger.Error("failed to push commits", zap.String("repo-id", repoID), zap.String("branch", tmpRepo.GetClonedBranch()), zap.Error(err))
607+
return nil, err
608+
}
609+
return nil, nil
592610
})
593611
if err == nil {
594612
if _, err := w.apiClient.ReportEventStatuses(ctx, &pipedservice.ReportEventStatusesRequest{Events: handledEvents}); err != nil {
595-
return fmt.Errorf("failed to report event statuses: %w", err)
613+
w.logger.Error("failed to report event statuses", zap.Error(err))
614+
return err
596615
}
597616
w.milestoneMap.Store(repoID, maxTimestamp)
598617
return nil
@@ -613,10 +632,12 @@ func (w *watcher) updateValues(ctx context.Context, repo git.Repo, repoID string
613632
handledEvents[i].StatusDescription = fmt.Sprintf("Failed to push changed files: %v", err)
614633
}
615634
if _, err := w.apiClient.ReportEventStatuses(ctx, &pipedservice.ReportEventStatusesRequest{Events: handledEvents}); err != nil {
616-
return fmt.Errorf("failed to report event statuses: %w", err)
635+
w.logger.Error("failed to report event statuses: %w", zap.Error(err))
636+
return err
617637
}
618638
w.milestoneMap.Store(repoID, maxTimestamp)
619-
return fmt.Errorf("failed to push commits: %w", err)
639+
w.logger.Error("failed to push commits", zap.Error(err))
640+
return err
620641
}
621642

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

656678
if err := os.WriteFile(path, newContent, os.ModePerm); err != nil {
657-
return "", fmt.Errorf("failed to write file: %w", err)
679+
w.logger.Error("failed to write file", zap.Error(err))
680+
return "", err
658681
}
659682
changes[filePath] = newContent
660683
}
@@ -670,7 +693,12 @@ func (w *watcher) commitFiles(ctx context.Context, latestEvent *model.Event, eve
670693
branch := makeBranchName(newBranch, eventName, repo.GetClonedBranch())
671694
trailers := maps.Clone(latestEvent.Contexts)
672695
if err := repo.CommitChanges(ctx, branch, commitMsg, newBranch, changes, trailers); err != nil {
673-
return "", fmt.Errorf("failed to perform git commit: %w", err)
696+
w.logger.Error("failed to perform git commit",
697+
zap.String("branch", branch),
698+
zap.Bool("make-new-branch", newBranch),
699+
zap.Int("changed-files", len(changes)),
700+
zap.Error(err))
701+
return "", err
674702
}
675703
w.logger.Info(fmt.Sprintf("event watcher will update values of Event %q", eventName))
676704
return branch, nil

0 commit comments

Comments
 (0)