Skip to content

Commit

Permalink
autoupdater: use different logfield than operation for retryer
Browse files Browse the repository at this point in the history
Do not use the event logfield to assign a name for the operation that
the retryer runs. It can already be in reuse, resulting in 2 event
fields. Use a new logfield called "operation" instead.

Also fix an issue where logfields got appended to a passed logfields
slice in updatePRWithBase.
This can cause that the wrong logfields are displayed in some messages.
  • Loading branch information
fho committed Dec 2, 2024
1 parent 7ee39bb commit 9e66a8a
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 4 deletions.
2 changes: 1 addition & 1 deletion internal/autoupdate/ci.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func (c *CI) RunAll(ctx context.Context, retryer Retryer, pr *PullRequest) error

logfields := append(
[]zap.Field{
logfields.Event("triggering_ci_job"),
logfields.Operation("triggering_ci_job"),
logfields.CIJob(job.String()),
},
pr.LogFields...,
Expand Down
7 changes: 4 additions & 3 deletions internal/autoupdate/queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"maps"
"slices"

Check failure on line 8 in internal/autoupdate/queue.go

View workflow job for this annotation

GitHub Actions / lint

"slices" imported and not used) (typecheck)

Check failure on line 8 in internal/autoupdate/queue.go

View workflow job for this annotation

GitHub Actions / lint

"slices" imported and not used (typecheck)

Check failure on line 8 in internal/autoupdate/queue.go

View workflow job for this annotation

GitHub Actions / build

"slices" imported and not used

Check failure on line 8 in internal/autoupdate/queue.go

View workflow job for this annotation

GitHub Actions / test

"slices" imported and not used
"strings"
"sync"
"sync/atomic"
Expand Down Expand Up @@ -717,8 +718,6 @@ func (q *queue) updatePR(ctx context.Context, pr *PullRequest, task Task) {

// TODO: passing logger and loggingFields as parameters is redundant, only pass one of them
func (q *queue) updatePRWithBase(ctx context.Context, pr *PullRequest, logger *zap.Logger, loggingFields []zapcore.Field) (changed bool, headCommit string, updateBranchErr error) {
loggingFields = append(loggingFields, logfields.Event("update_branch"))

updateBranchErr = q.retryer.Run(ctx, func(ctx context.Context) error {
result, err := q.ghClient.UpdateBranch(
ctx,
Expand Down Expand Up @@ -747,7 +746,9 @@ func (q *queue) updatePRWithBase(ctx context.Context, pr *PullRequest, logger *z
headCommit = result.HeadCommitID

return nil
}, loggingFields)
},
append([]zapcore.Field{logfields.Operation("update_branch")}, loggingFields...),
)

if updateBranchErr != nil {
if isPRIsClosedErr(updateBranchErr) {
Expand Down
7 changes: 7 additions & 0 deletions internal/logfields/logfields.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package logfields

import "go.uber.org/zap"

func Operation(val string) zap.Field {
return zap.String("operation", val)
}

0 comments on commit 9e66a8a

Please sign in to comment.