Refactor/slog - #1092
Open
qcserestipy wants to merge 4 commits into
Open
Conversation
Replace all logrus call sites (~290 across 90+ files) with log/slog: - Fatal/Fatalf become slog.Error followed by os.Exit(1), keeping exit semantics - Formatted calls ending in %v/%s with an error arg become structured logs with an error attribute; others keep their text via fmt.Sprintf - WithField/WithFields/WithError are flattened to key-value attributes - logs.go uses a dedicated slog TextHandler; audit entry time is now a time attribute since slog records cannot override the timestamp - Set up the logger before config init so early logs respect the configured handler - Fix PrettyHandler.WithAttrs/WithGroup dropping the writer and level - Adapt log-capture tests (elevate_test, doc_test) to slog - Drop logrus from go.mod/go.sum Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1092 +/- ##
=========================================
- Coverage 10.99% 9.56% -1.43%
=========================================
Files 173 326 +153
Lines 8671 16654 +7983
=========================================
+ Hits 953 1593 +640
- Misses 7612 14922 +7310
- Partials 106 139 +33 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Replace every slog.Error + os.Exit(1) pair introduced by the slog migration with a real error that propagates to the cobra command, following the project paradigm that commands handle errors via RunE: - Convert inline RunE exit sites to returned errors, keeping the original message text - Give 21 previously void view functions an error return (label/immutable/user/password/cve/quota/webhook/registry/robot/ replication/preheat/scanner create and update views) and propagate at every caller - Change prompt.GetRepoNameFromUser and the replication prompt functions to return (T, error), hoisting the API call out of the goroutine so failures can be returned - Return errors from utils.InitConfig, GetDataPaths, GetUserIdFromUser and the config file helpers; handle InitConfig in PersistentPreRunE - Return errors from the doc generators' helper functions; only their main() functions still exit, as there is no caller above them Pre-existing os.Exit(1) sites not tied to slog are left unchanged and will be addressed separately. Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Migrates Harbor CLI logging from Logrus to log/slog, centralizes structured logging, and propagates previously fatal errors through command handlers.
Changes:
- Replaces Logrus calls and removes the dependency.
- Converts interactive views and configuration helpers to return errors.
- Fixes derived
PrettyHandlerconfiguration and updates logging tests.
Reviewed changes
Copilot reviewed 124 out of 126 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
test/helper/helpers.go |
Handles configuration initialization errors. |
pkg/views/webhook/edit/view.go |
Returns form errors. |
pkg/views/user/create/view.go |
Returns form errors. |
pkg/views/scanner/update/view.go |
Returns scanner form errors. |
pkg/views/scanner/create/view.go |
Returns scanner form errors. |
pkg/views/scan-all/update/view.go |
Returns schedule form errors. |
pkg/views/robot/update/view.go |
Returns robot form errors. |
pkg/views/robot/create/view.go |
Returns robot and secret form errors. |
pkg/views/replication/policies/create/view.go |
Returns policy form errors. |
pkg/views/registry/update/view.go |
Returns registry form errors. |
pkg/views/registry/create/view.go |
Returns registry form errors. |
pkg/views/quota/update/view.go |
Returns quota form errors. |
pkg/views/project/create/view.go |
Migrates client errors to slog. |
pkg/views/project/config/update/view.go |
Returns metadata form errors. |
pkg/views/preheat/policy/create/view.go |
Propagates provider and form errors. |
pkg/views/password/reset/view.go |
Returns reset form errors. |
pkg/views/password/change/view.go |
Returns password form errors. |
pkg/views/login/create.go |
Returns login form errors. |
pkg/views/label/update/view.go |
Returns label form errors. |
pkg/views/label/create/view.go |
Returns label form errors. |
pkg/views/immutable/create/view.go |
Returns immutable-rule form errors. |
pkg/views/cveallowlist/update/view.go |
Returns CVE form errors. |
pkg/views/confirmation.go |
Propagates confirmation errors. |
pkg/views/artifact/tags/create/view.go |
Returns tag form errors. |
pkg/utils/utils.go |
Migrates logging and user-ID errors. |
pkg/utils/encryption.go |
Migrates keyring logging. |
pkg/utils/config.go |
Propagates configuration and file errors. |
pkg/utils/config_test.go |
Checks initialization errors. |
pkg/utils/client.go |
Migrates client logging. |
pkg/prompt/prompt.go |
Adds errors to repository and replication prompts. |
pkg/logger/handler.go |
Preserves writer and level in derived handlers. |
pkg/config/replication/policies.go |
Migrates replication-config logging. |
pkg/config/preheat/policies.go |
Migrates preheat-config logging. |
pkg/api/webhook_handler.go |
Migrates webhook logging. |
pkg/api/robot_handler.go |
Migrates robot logging. |
pkg/api/project_handler.go |
Migrates project logging. |
pkg/api/ping_handler.go |
Migrates ping logging. |
pkg/api/artifact_handler.go |
Migrates artifact logging. |
go.sum |
Removes Logrus checksums. |
go.mod |
Removes the Logrus dependency. |
empty.yaml |
Includes an empty YAML file. |
doc/man-docs/man_doc.go |
Propagates documentation errors through slog. |
doc/doc.go |
Migrates documentation logging and exits. |
doc/doc_test.go |
Captures slog output in tests. |
cmd/harbor/root/webhook/edit.go |
Handles webhook view errors. |
cmd/harbor/root/vulnerability/list.go |
Migrates vulnerability logging. |
cmd/harbor/root/user/password.go |
Handles password view errors. |
cmd/harbor/root/user/list.go |
Migrates user-list logging. |
cmd/harbor/root/user/elevate.go |
Migrates elevation logging. |
cmd/harbor/root/user/elevate_test.go |
Captures slog output in tests. |
cmd/harbor/root/user/delete.go |
Migrates deletion logging. |
cmd/harbor/root/user/create.go |
Handles user form errors. |
cmd/harbor/root/tag/immutable/create.go |
Handles immutable-rule form errors. |
cmd/harbor/root/scanner/update.go |
Handles scanner update form errors. |
cmd/harbor/root/scanner/create.go |
Handles scanner creation form errors. |
cmd/harbor/root/scan_all/view_schedule.go |
Migrates schedule logging. |
cmd/harbor/root/scan_all/update_schedule.go |
Migrates logging and form errors. |
cmd/harbor/root/scan_all/stop.go |
Migrates stop-operation logging. |
cmd/harbor/root/scan_all/run.go |
Migrates scan logging. |
cmd/harbor/root/scan_all/metrics.go |
Migrates metrics logging. |
cmd/harbor/root/robot/update.go |
Migrates logging and form errors. |
cmd/harbor/root/robot/refresh.go |
Propagates refresh and secret errors. |
cmd/harbor/root/robot/create.go |
Migrates robot creation logging. |
cmd/harbor/root/repository/view.go |
Handles repository prompt errors. |
cmd/harbor/root/repository/update.go |
Handles repository prompt errors. |
cmd/harbor/root/repository/list.go |
Migrates repository-list logging. |
cmd/harbor/root/repository/delete.go |
Handles repository prompt errors. |
cmd/harbor/root/replication/stop.go |
Handles replication prompt errors. |
cmd/harbor/root/replication/start.go |
Handles policy prompt errors. |
cmd/harbor/root/replication/policies/view.go |
Handles policy prompt errors. |
cmd/harbor/root/replication/policies/update.go |
Migrates logging and view errors. |
cmd/harbor/root/replication/policies/list.go |
Adds structured policy logging. |
cmd/harbor/root/replication/policies/delete.go |
Handles policy prompt errors. |
cmd/harbor/root/replication/policies/create.go |
Migrates logging and view errors. |
cmd/harbor/root/replication/logs.go |
Handles replication selection errors. |
cmd/harbor/root/replication/executions/view.go |
Handles execution selection errors. |
cmd/harbor/root/replication/executions/list.go |
Migrates execution logging. |
cmd/harbor/root/registry/update.go |
Handles registry form errors. |
cmd/harbor/root/registry/list.go |
Migrates registry-list logging. |
cmd/harbor/root/registry/create.go |
Handles registry form errors. |
cmd/harbor/root/quota/update.go |
Handles quota form errors. |
cmd/harbor/root/project/view.go |
Migrates project-view logging. |
cmd/harbor/root/project/search.go |
Migrates project-search logging. |
cmd/harbor/root/project/robot/update.go |
Handles robot form errors. |
cmd/harbor/root/project/robot/refresh.go |
Propagates secret errors. |
cmd/harbor/root/project/robot/list.go |
Migrates logging and prompt errors. |
cmd/harbor/root/project/robot/delete.go |
Propagates prompt errors. |
cmd/harbor/root/project/robot/create.go |
Migrates robot creation logging. |
cmd/harbor/root/project/preheat/policy/view.go |
Migrates policy-view logging. |
cmd/harbor/root/project/preheat/policy/update.go |
Migrates logging and form errors. |
cmd/harbor/root/project/preheat/policy/start.go |
Migrates policy-start logging. |
cmd/harbor/root/project/preheat/policy/list.go |
Migrates policy-list logging. |
cmd/harbor/root/project/preheat/policy/delete.go |
Migrates policy-delete logging. |
cmd/harbor/root/project/preheat/policy/create.go |
Migrates logging and form errors. |
cmd/harbor/root/project/preheat/execution/view.go |
Migrates execution-view logging. |
cmd/harbor/root/project/preheat/execution/stop.go |
Migrates execution-stop logging. |
cmd/harbor/root/project/preheat/execution/list.go |
Migrates execution-list logging. |
cmd/harbor/root/project/member/create.go |
Migrates member creation logging. |
cmd/harbor/root/project/logs.go |
Adds structured project-log diagnostics. |
cmd/harbor/root/project/list.go |
Adds structured pagination logging. |
cmd/harbor/root/project/delete.go |
Migrates concurrent deletion logging. |
cmd/harbor/root/project/create.go |
Migrates project creation logging. |
cmd/harbor/root/project/config/update.go |
Handles metadata form errors. |
cmd/harbor/root/password.go |
Handles password form errors. |
cmd/harbor/root/logs.go |
Uses a dedicated slog audit logger. |
cmd/harbor/root/login.go |
Migrates login logging and errors. |
cmd/harbor/root/labels/update.go |
Handles label form errors. |
cmd/harbor/root/labels/list.go |
Migrates label-list logging. |
cmd/harbor/root/labels/create.go |
Handles label form errors. |
cmd/harbor/root/instance/view.go |
Migrates instance-view logging. |
cmd/harbor/root/instance/ping.go |
Migrates instance-ping logging. |
cmd/harbor/root/info.go |
Migrates info-command logging. |
cmd/harbor/root/cve/add.go |
Handles CVE form errors. |
cmd/harbor/root/context/update.go |
Migrates encryption diagnostics. |
cmd/harbor/root/context/delete.go |
Migrates context deletion messages. |
cmd/harbor/root/cmd.go |
Initializes logging before configuration. |
cmd/harbor/root/artifact/view.go |
Handles repository prompt errors. |
cmd/harbor/root/artifact/tags/list.go |
Handles repository prompt errors. |
cmd/harbor/root/artifact/tags/delete.go |
Handles repository prompt errors. |
cmd/harbor/root/artifact/tags/create.go |
Handles prompt and tag-form errors. |
cmd/harbor/root/artifact/scan/scan.go |
Propagates repository prompt errors. |
cmd/harbor/root/artifact/list.go |
Handles repository prompt errors. |
cmd/harbor/root/artifact/label/list.go |
Handles repository prompt errors. |
cmd/harbor/root/artifact/label/delete.go |
Handles repository prompt errors. |
cmd/harbor/root/artifact/label/add.go |
Handles repository prompt errors. |
cmd/harbor/root/artifact/delete.go |
Migrates artifact deletion logging. |
Suppressed comments (1)
pkg/logger/handler.go:136
- Appending a group while reusing raw
preAttrschanges the group of attributes that were attached earlier. For example,logger.WithGroup("a").With("x", 1).WithGroup("b")should emita.x, butHandlequalifies every pre-attribute with the final group list and emitsa.b.x. Preserve each pre-attribute's group context whenWithAttrsis called, and apply new groups only to later attributes.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+73
to
74
| // Sets up logging before anything else logs | ||
| logger.Setup(verbose, logFormat) |
| var ConfigInitialization = &Once{} | ||
|
|
||
| func InitConfig(cfgFile string, userSpecifiedConfig bool) { | ||
| func InitConfig(cfgFile string, userSpecifiedConfig bool) error { |
Comment on lines
+117
to
+118
| out: h.out, | ||
| level: h.level, |
Comment on lines
32
to
33
| func CreateRegistryView(createView *api.CreateRegView) error { | ||
| registries, _ := api.GetRegistryProviders() |
| // 5. Confirm to the user (no error here) | ||
| canonicalPath := strings.Join(actualSegments, ".") | ||
| logrus.Infof("Successfully cleared %s", canonicalPath) | ||
| slog.Info(fmt.Sprintf("Successfully cleared %s", canonicalPath)) |
Comment on lines
74
to
76
| if err != nil { | ||
| log.Error(err) | ||
| slog.Error(err.Error()) | ||
| } |
Comment on lines
51
to
53
| if err != nil { | ||
| log.Error(err) | ||
| slog.Error(err.Error()) | ||
| } |
Comment on lines
77
to
79
| if err != nil { | ||
| log.Error(err) | ||
| slog.Error(err.Error()) | ||
| } |
Comment on lines
89
to
91
| if err != nil { | ||
| log.Error(err) | ||
| slog.Error(err.Error()) | ||
| } |
Comment on lines
79
to
81
| if err != nil { | ||
| log.Error(err) | ||
| slog.Error(err.Error()) | ||
| } |
- Route verbose logging to stderr so command payloads on stdout stay parsable; non-verbose output is still discarded - Share a mutex pointer across PrettyHandler instances derived via WithAttrs/WithGroup to prevent interleaved writes to the same writer - Clear the cached configInitError when the InitConfig once callback runs, so a Reset and successful retry no longer report a stale error - Return errors instead of logging and continuing: PrintFormat failures in info, labels/registry/repository/user list; project member creation; project selection in artifact delete - Return the GetRegistryProviders error in CreateRegistryView instead of proceeding with an empty provider list - Convert prompt.GetQuotaIDFromUser to return (int64, error), hoisting the API call out of the goroutine to avoid a nil dereference on failure; update the quota update caller - Print the context delete confirmation with fmt.Printf so it is visible without --verbose Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
What this PR does
Migrates all logging from github.com/sirupsen/logrus to the standard library's log/slog and removes the logrus dependency entirely (~290 call sites across 90+ files).
Why
The slog infrastructure already existed (pkg/logger with a custom pretty handler and JSON support, wired up in the root command), but only a single call site actually used it — everything else still logged through logrus. This completes the migration so the CLI has one logging system, drops a third-party dependency in favor of the stdlib, and enables structured logging output.
log.Info/log.Debug#881Type of Change
Changes