Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

fix/repo-updater: add WARN level logs every time we sync a code host #64519

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion internal/repos/syncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ func (s *Syncer) SyncExternalService(
minSyncInterval time.Duration,
progressRecorder progressRecorderFunc,
) (err error) {
logger := s.ObsvCtx.Logger.With(log.Int64("externalServiceID", externalServiceID))
logger := s.ObsvCtx.Logger.Scoped("SyncExternalService").With(log.Int64("externalServiceID", externalServiceID))
logger.Info("syncing external service")

// Ensure the job field is recorded when monitoring external API calls
Expand Down Expand Up @@ -365,6 +365,29 @@ func (s *Syncer) SyncExternalService(
modified = modified || len(diff.Modified)+len(diff.Added) > 0
}

// This information isn't necessarily an error in and of itself, but it's possible
// for the code host to misbehave in a way that returns a legitimate-looking response
// (e.g. a non-fatal HTTP status code, with a well-formed response body) that is
// nonetheless incorrect (e.g. temporarily returning empty set of repositories
// instead of an error).
//
// Because of this, we have to be able to log every response from the code host
// so that we can audit them later so that we can rule in/out the above scenario.
// So, we choose the WARN level instead of INFO so that this info is logged by default.
logger.Warn("finished listing repositories from external service",
log.Object("syncProgress",
log.Int32("synced", syncProgress.Synced),
log.Int32("errors", syncProgress.Errors),
log.Int32("added", syncProgress.Added),
log.Int32("removed", syncProgress.Removed),
log.Int32("modified", syncProgress.Modified),
log.Int32("unmodified", syncProgress.Unmodified),
),
log.Int("seen", len(seen)),
log.Bool("modified", modified),
log.Error(errs),
)

// We don't delete any repos of site-level external services if there were any
// non-warning errors during a sync.
//
Expand All @@ -380,13 +403,15 @@ func (s *Syncer) SyncExternalService(
// repos (by removing ones if code-host permissions have changed).
abortDeletion := false
if errs != nil {
logger.Error("received errors during sync", log.Error(errs))
var ref errors.MultiError
if errors.As(errs, &ref) {
for _, e := range ref.Errors() {
if errors.IsWarning(e) {
baseError := errors.Unwrap(e)
if !errcode.IsForbidden(baseError) && !errcode.IsUnauthorized(baseError) {
abortDeletion = true
logger.Info("aborting deletion due to fatal error", log.Error(e))
break
}
continue
Expand All @@ -395,6 +420,7 @@ func (s *Syncer) SyncExternalService(
continue
}
abortDeletion = true
logger.Info("aborting deletion due to fatal error", log.Error(e))
break
}
}
Expand Down
Loading