feat: Adding slog support and handlers#722
feat: Adding slog support and handlers#722NucleoFusion wants to merge 12 commits intogoharbor:mainfrom
Conversation
Signed-off-by: NucleoFusion <lakshit.singh.mail@gmail.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #722 +/- ##
=========================================
- Coverage 10.99% 7.99% -3.00%
=========================================
Files 173 272 +99
Lines 8671 13256 +4585
=========================================
+ Hits 953 1060 +107
- Misses 7612 12082 +4470
- Partials 106 114 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: NucleoFusion <lakshit.singh.mail@gmail.com>
Signed-off-by: NucleoFusion <lakshit.singh.mail@gmail.com>
Signed-off-by: NucleoFusion <lakshit.singh.mail@gmail.com>
Signed-off-by: NucleoFusion <lakshit.singh.mail@gmail.com>
ec45ccd to
42b0a72
Compare
|
@bupd Did the stuff we discussed in the meet |
Signed-off-by: NucleoFusion <lakshit.singh.mail@gmail.com>
There was a problem hiding this comment.
Pull request overview
Adds a log/slog-based logging setup for the Harbor CLI, including a custom “pretty” handler with lipgloss-styled level rendering and initialization from the root command’s PersistentPreRunE.
Changes:
- Introduces a new
pkg/loggerpackage withSetup()and a customPrettyHandlerfor slog output. - Wires logger initialization into
cmd/harbor/root/cmd.goand logs CLI flags at debug level. - Supports JSON logging when the selected format is
"json".
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| cmd/harbor/root/cmd.go | Initializes slog logging during CLI startup and emits a debug log of flags. |
| pkg/logger/logger.go | Adds a centralized slog setup function selecting JSON vs pretty handler. |
| pkg/logger/handler.go | Implements a custom pretty slog handler with colored level formatting and attribute printing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Nucleo Fusion <148791547+NucleoFusion@users.noreply.github.com>
|
@NucleoFusion please address and mark reviews as resolved. I have also added my thoughts too. hope it helps. thanks |
Signed-off-by: NucleoFusion <lakshit.singh.mail@gmail.com>
|
@bupd changes have been made, you can take a look if you have time |
bupd
left a comment
There was a problem hiding this comment.
@NucleoFusion looks like lint is failing.
There was a problem hiding this comment.
Pull request overview
This PR adds structured logging support to the Harbor CLI by migrating from logrus to Go's standard library log/slog. The changes introduce a custom pretty-print handler for text output, JSON handler support, and integrates logging setup into the root command initialization with a new --log-format flag to control output format.
Changes:
- Created new
pkg/loggerpackage withSetupfunction to initialize slog with custom handlers and aPrettyHandlerfor formatted text output with colored log levels via lipgloss - Replaced logrus-based logging in
cmd/harbor/root/cmd.gowith slog-based setup, added--log-formatflag with "text" and "json" options, and integrated flag logging inPersistentPreRunE - Removed dependencies on
logrus,io, andtimeimports where no longer needed
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/logger/logger.go | New file establishing slog Setup function with format-based handler selection |
| pkg/logger/handler.go | New file implementing PrettyHandler for colored, formatted text logging output |
| cmd/harbor/root/cmd.go | Updated to use new slog-based logging with dedicated --log-format flag and flag logging in PersistentPreRunE |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if format == "json" { | ||
| handler = slog.NewJSONHandler(outp, &slog.HandlerOptions{Level: slog.LevelDebug}) | ||
| } else { | ||
| // Custom Text Handler | ||
| handler = NewPrettyHandler(outp, slog.LevelDebug) |
There was a problem hiding this comment.
The --log-format flag does not validate that the format value is one of the supported options ("json" or "text"). Currently, any invalid value silently defaults to the text handler. Consider adding explicit validation in the Setup function to reject unsupported formats, or document this fallback behavior more clearly. According to the PR discussion, only "json" and "text" should be accepted values.
| if format == "json" { | |
| handler = slog.NewJSONHandler(outp, &slog.HandlerOptions{Level: slog.LevelDebug}) | |
| } else { | |
| // Custom Text Handler | |
| handler = NewPrettyHandler(outp, slog.LevelDebug) | |
| switch format { | |
| case "json": | |
| handler = slog.NewJSONHandler(outp, &slog.HandlerOptions{Level: slog.LevelDebug}) | |
| case "text": | |
| // Custom Text Handler | |
| handler = NewPrettyHandler(outp, slog.LevelDebug) | |
| default: | |
| panic(`unsupported log format: ` + format + `; supported formats are "json" and "text"`) |
Signed-off-by: NucleoFusion <lakshit.singh.mail@gmail.com>
5ed9e8e to
a929b09
Compare
|
@bupd fixed lint |
resolves #717
Adding support for
log/slogwith flag based checking on output format or verbosity.Changes
--verboseflag shows INFO and DEBUG level as well.cobraon handling errors, now logged viasloglipglossbased formatting for levelsEDIT
Changes
PersistentPreRunEAdded better formatting and color codes