From 8f26b0a00cabb03fa1ace15ec38ece3cd2b3a914 Mon Sep 17 00:00:00 2001 From: iTrooz Date: Thu, 9 Oct 2025 00:09:19 +0200 Subject: [PATCH 01/17] feat: make log level colored in `velero backup logs` Signed-off-by: iTrooz --- pkg/cmd/cli/backup/logs.go | 54 +++++++++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/cli/backup/logs.go b/pkg/cmd/cli/backup/logs.go index a0149acf17..dc274bfeb4 100644 --- a/pkg/cmd/cli/backup/logs.go +++ b/pkg/cmd/cli/backup/logs.go @@ -17,11 +17,15 @@ limitations under the License. package backup import ( + "bufio" "context" "fmt" + "io" "os" + "strings" "time" + "github.com/fatih/color" "github.com/spf13/cobra" "github.com/spf13/pflag" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -61,6 +65,51 @@ func (l *LogsOptions) BindFlags(flags *pflag.FlagSet) { flags.StringVar(&l.CaCertFile, "cacert", l.CaCertFile, "Path to a certificate bundle to use when verifying TLS connections.") } +func coloredLevel(level string) string { + switch level { + case "info": + return color.GreenString("info") + case "warning": + return color.YellowString("warning") + case "error": + return color.RedString("error") + case "debug": + return color.BlueString("debug") + default: + return level + } +} + + +// Process logs (by adding color) before printing them +func processAndPrintLogs(r io.Reader, w io.Writer) { + scanner := bufio.NewScanner(r) + for scanner.Scan() { + // Read line + line := scanner.Text() + + // Get level position + levelPrefixSize := len("level=") + levelStart := strings.Index(line, "level=") + levelSize := strings.Index(line[levelStart+levelPrefixSize:], " ") + if levelStart == -1 || levelSize == -1 { + // No space after level found, print as-is + fmt.Fprintln(w, line) + continue + } + + // Get and color level + level := line[levelStart+levelPrefixSize : levelStart+levelPrefixSize+levelSize] + level = coloredLevel(level) + + // Print line with colored level + fmt.Fprint(w, line[:levelStart+levelPrefixSize]) + fmt.Fprint(w, level) + fmt.Fprint(w, line[levelStart+levelPrefixSize+levelSize:]) + fmt.Fprintln(w) + } +} + func (l *LogsOptions) Run(c *cobra.Command, f client.Factory) error { backup := new(velerov1api.Backup) err := l.Client.Get(context.Background(), kbclient.ObjectKey{Namespace: f.Namespace(), Name: l.BackupName}, backup) @@ -86,7 +135,10 @@ func (l *LogsOptions) Run(c *cobra.Command, f client.Factory) error { bslCACert = "" } - err = downloadrequest.StreamWithBSLCACert(context.Background(), l.Client, f.Namespace(), l.BackupName, velerov1api.DownloadTargetKindBackupLog, os.Stdout, l.Timeout, l.InsecureSkipTLSVerify, l.CaCertFile, bslCACert) + pr, pw := io.Pipe() + go processAndPrintLogs(pr, os.Stdout) + + err = downloadrequest.StreamWithBSLCACert(context.Background(), l.Client, f.Namespace(), l.BackupName, velerov1api.DownloadTargetKindBackupLog, pw, l.Timeout, l.InsecureSkipTLSVerify, l.CaCertFile, bslCACert) return err } From bb975b16f4d37479f8ac044206f9967cc833eefd Mon Sep 17 00:00:00 2001 From: iTrooz Date: Thu, 9 Oct 2025 12:58:35 +0200 Subject: [PATCH 02/17] use logfmt for parsing logs Signed-off-by: iTrooz --- go.mod | 2 ++ go.sum | 2 ++ pkg/cmd/cli/backup/logs.go | 55 +++++++++++++++++--------------------- 3 files changed, 29 insertions(+), 30 deletions(-) diff --git a/go.mod b/go.mod index d32340b2ba..c42c8df5d6 100644 --- a/go.mod +++ b/go.mod @@ -106,6 +106,7 @@ require ( github.com/fxamacker/cbor/v2 v2.7.0 // indirect github.com/go-ini/ini v1.67.0 // indirect github.com/go-jose/go-jose/v4 v4.0.5 // indirect + github.com/go-logfmt/logfmt v0.4.0 // indirect github.com/go-logr/logr v1.4.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/go-ole/go-ole v1.3.0 // indirect @@ -135,6 +136,7 @@ require ( github.com/klauspost/cpuid/v2 v2.2.10 // indirect github.com/klauspost/pgzip v1.2.6 // indirect github.com/klauspost/reedsolomon v1.12.4 // indirect + github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515 // indirect github.com/kylelemons/godebug v1.1.0 // indirect github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de // indirect github.com/mailru/easyjson v0.7.7 // indirect diff --git a/go.sum b/go.sum index 84a94ed323..66f3370008 100644 --- a/go.sum +++ b/go.sum @@ -270,6 +270,7 @@ github.com/go-jose/go-jose/v4 v4.0.5 h1:M6T8+mKZl/+fNNuFHvGIzDz7BTLQPIounk/b9dw3 github.com/go-jose/go-jose/v4 v4.0.5/go.mod h1:s3P1lRrkT8igV8D9OjyL4WRyHvjB6a4JSllnOrmmBOA= github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9GBnD5lWE= +github.com/go-logfmt/logfmt v0.4.0 h1:MP4Eh7ZCb31lleYCFuwm0oe4/YGak+5l1vA2NOE80nA= github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk= github.com/go-logr/logr v0.1.0/go.mod h1:ixOQHD9gLJUVQQ2ZOR7zLEifBX6tGkNJF4QyIY7sIas= github.com/go-logr/logr v0.4.0/go.mod h1:z6/tIYblkpsD+a4lm/fGIIU9mZ+XfAiaFtq7xTgseGU= @@ -499,6 +500,7 @@ github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxv github.com/kopia/htmluibuild v0.0.1-0.20250607181534-77e0f3f9f557 h1:je1C/xnmKxnaJsIgj45me5qA51TgtK9uMwTxgDw+9H0= github.com/kopia/htmluibuild v0.0.1-0.20250607181534-77e0f3f9f557/go.mod h1:h53A5JM3t2qiwxqxusBe+PFgGcgZdS+DWCQvG5PTlto= github.com/kr/fs v0.1.0/go.mod h1:FFnZGqtBN9Gxj7eW1uZ42v5BccTP0vu6NEaFoC2HwRg= +github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515 h1:T+h1c/A9Gawja4Y9mFVWj2vyii2bbUNDw3kt9VxK2EY= github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pretty v0.2.0/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= diff --git a/pkg/cmd/cli/backup/logs.go b/pkg/cmd/cli/backup/logs.go index dc274bfeb4..18adb4af69 100644 --- a/pkg/cmd/cli/backup/logs.go +++ b/pkg/cmd/cli/backup/logs.go @@ -17,12 +17,10 @@ limitations under the License. package backup import ( - "bufio" "context" "fmt" "io" "os" - "strings" "time" "github.com/fatih/color" @@ -31,6 +29,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" kbclient "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/go-logfmt/logfmt" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/client" "github.com/vmware-tanzu/velero/pkg/cmd" @@ -65,47 +64,43 @@ func (l *LogsOptions) BindFlags(flags *pflag.FlagSet) { flags.StringVar(&l.CaCertFile, "cacert", l.CaCertFile, "Path to a certificate bundle to use when verifying TLS connections.") } -func coloredLevel(level string) string { +func getLevelColor(level string) *color.Color { switch level { case "info": - return color.GreenString("info") + return color.New(color.FgGreen) case "warning": - return color.YellowString("warning") + return color.New(color.FgYellow) case "error": - return color.RedString("error") + return color.New(color.FgRed) case "debug": - return color.BlueString("debug") + return color.New(color.FgBlue) default: - return level + return color.New() } } - // Process logs (by adding color) before printing them func processAndPrintLogs(r io.Reader, w io.Writer) { - scanner := bufio.NewScanner(r) - for scanner.Scan() { - // Read line - line := scanner.Text() - - // Get level position - levelPrefixSize := len("level=") - levelStart := strings.Index(line, "level=") - levelSize := strings.Index(line[levelStart+levelPrefixSize:], " ") - if levelStart == -1 || levelSize == -1 { - // No space after level found, print as-is - fmt.Fprintln(w, line) - continue + d := logfmt.NewDecoder(r) + for d.ScanRecord() { // get record (line) + // Scan fields and get color + var fields [][2][]byte + var lineColor *color.Color + for d.ScanKeyval() { + fields = append(fields, [2][]byte{d.Key(), d.Value()}) + if string(d.Key()) == "level" { + lineColor = getLevelColor(string(d.Value())) + } } - // Get and color level - level := line[levelStart+levelPrefixSize : levelStart+levelPrefixSize+levelSize] - level = coloredLevel(level) - - // Print line with colored level - fmt.Fprint(w, line[:levelStart+levelPrefixSize]) - fmt.Fprint(w, level) - fmt.Fprint(w, line[levelStart+levelPrefixSize+levelSize:]) + // Re-encode with color. We do not use logfmt Encoder because it does not support color + for _, field := range fields { + if lineColor == nil { // handle case where no color (log level) was found + fmt.Fprintf(w, "%s=%s ", field[0], field[1]) + } else { + fmt.Fprintf(w, "%s=%s ", lineColor.Sprintf("%s", field[0]), field[1]) + } + } fmt.Fprintln(w) } } From 6e56558c5616eb8961317e154d6ee87d0dba7eca Mon Sep 17 00:00:00 2001 From: iTrooz Date: Thu, 9 Oct 2025 13:05:09 +0200 Subject: [PATCH 03/17] colorize level value as well + refactor Signed-off-by: iTrooz --- pkg/cmd/cli/backup/logs.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/cli/backup/logs.go b/pkg/cmd/cli/backup/logs.go index 18adb4af69..0f7f896dcd 100644 --- a/pkg/cmd/cli/backup/logs.go +++ b/pkg/cmd/cli/backup/logs.go @@ -95,11 +95,20 @@ func processAndPrintLogs(r io.Reader, w io.Writer) { // Re-encode with color. We do not use logfmt Encoder because it does not support color for _, field := range fields { + var key, value string if lineColor == nil { // handle case where no color (log level) was found - fmt.Fprintf(w, "%s=%s ", field[0], field[1]) + key = string(field[0]) + value = string(field[1]) } else { - fmt.Fprintf(w, "%s=%s ", lineColor.Sprintf("%s", field[0]), field[1]) + key = lineColor.Sprintf("%s", field[0]) + if string(field[0]) == "level" { + colorCopy := *lineColor + value = colorCopy.Add(color.Bold).Sprintf("%s", field[1]) + } else { + value = string(field[1]) + } } + fmt.Fprintf(w, "%s=%s ", key, value) } fmt.Fprintln(w) } From f15192c9789d5f0580758780d83b363977e70eb9 Mon Sep 17 00:00:00 2001 From: iTrooz Date: Thu, 9 Oct 2025 13:06:29 +0200 Subject: [PATCH 04/17] fix typo Signed-off-by: iTrooz --- pkg/cmd/velero/velero.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/velero/velero.go b/pkg/cmd/velero/velero.go index a74c68fbcb..0e5ffae126 100644 --- a/pkg/cmd/velero/velero.go +++ b/pkg/cmd/velero/velero.go @@ -66,7 +66,7 @@ func NewCommand(name string) *cobra.Command { // Declare cmdFeatures and cmdColorzied here so we can access them in the PreRun hooks // without doing a chain of calls into the command's FlagSet var cmdFeatures veleroflag.StringArray - var cmdColorzied veleroflag.OptionalBool + var cmdColorized veleroflag.OptionalBool c := &cobra.Command{ Use: name, @@ -86,8 +86,8 @@ operations can also be performed as 'velero backup get' and 'velero schedule cre features.Enable(cmdFeatures...) switch { - case cmdColorzied.Value != nil: - color.NoColor = !*cmdColorzied.Value + case cmdColorized.Value != nil: + color.NoColor = !*cmdColorized.Value default: color.NoColor = !config.Colorized() } @@ -101,7 +101,7 @@ operations can also be performed as 'velero backup get' and 'velero schedule cre c.PersistentFlags().Var(&cmdFeatures, "features", "Comma-separated list of features to enable for this Velero process. Combines with values from $HOME/.config/velero/config.json if present") // Color will be enabled or disabled for all subcommands - c.PersistentFlags().Var(&cmdColorzied, "colorized", "Show colored output in TTY. Overrides 'colorized' value from $HOME/.config/velero/config.json if present. Enabled by default") + c.PersistentFlags().Var(&cmdColorized, "colorized", "Show colored output in TTY. Overrides 'colorized' value from $HOME/.config/velero/config.json if present. Enabled by default") c.AddCommand( backup.NewCommand(f), From 54c96254b680cffd84635f2a076f0329fa64836a Mon Sep 17 00:00:00 2001 From: iTrooz Date: Thu, 9 Oct 2025 13:07:40 +0200 Subject: [PATCH 05/17] If NoColor, do not parse logs Signed-off-by: iTrooz --- pkg/cmd/cli/backup/logs.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/cli/backup/logs.go b/pkg/cmd/cli/backup/logs.go index 0f7f896dcd..78ea3b5661 100644 --- a/pkg/cmd/cli/backup/logs.go +++ b/pkg/cmd/cli/backup/logs.go @@ -139,10 +139,17 @@ func (l *LogsOptions) Run(c *cobra.Command, f client.Factory) error { bslCACert = "" } - pr, pw := io.Pipe() - go processAndPrintLogs(pr, os.Stdout) + var w io.Writer + // If NoColor, do not parse logs + if color.NoColor { + w = os.Stdout + } else { + pr, pw := io.Pipe() + w = pw + go processAndPrintLogs(pr, os.Stdout) + } - err = downloadrequest.StreamWithBSLCACert(context.Background(), l.Client, f.Namespace(), l.BackupName, velerov1api.DownloadTargetKindBackupLog, pw, l.Timeout, l.InsecureSkipTLSVerify, l.CaCertFile, bslCACert) + err = downloadrequest.StreamWithBSLCACert(context.Background(), l.Client, f.Namespace(), l.BackupName, velerov1api.DownloadTargetKindBackupLog, w, l.Timeout, l.InsecureSkipTLSVerify, l.CaCertFile, bslCACert) return err } From 0150f06a695be90c680f825bfd2d0bf9353afd90 Mon Sep 17 00:00:00 2001 From: iTrooz Date: Thu, 9 Oct 2025 13:31:05 +0200 Subject: [PATCH 06/17] wait for all logs to be processed before exiting Signed-off-by: iTrooz --- pkg/cmd/cli/backup/logs.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/cli/backup/logs.go b/pkg/cmd/cli/backup/logs.go index 78ea3b5661..6119d416d1 100644 --- a/pkg/cmd/cli/backup/logs.go +++ b/pkg/cmd/cli/backup/logs.go @@ -21,6 +21,7 @@ import ( "fmt" "io" "os" + "sync" "time" "github.com/fatih/color" @@ -139,17 +140,30 @@ func (l *LogsOptions) Run(c *cobra.Command, f client.Factory) error { bslCACert = "" } + // Potentially add color to logs var w io.Writer - // If NoColor, do not parse logs + var wg sync.WaitGroup + wg.Add(1) + // If NoColor, do not parse logs and directly fall back to stdout if color.NoColor { w = os.Stdout + wg.Done() } else { pr, pw := io.Pipe() w = pw - go processAndPrintLogs(pr, os.Stdout) + go func(pr *io.PipeReader) { + defer wg.Done() + processAndPrintLogs(pr, os.Stdout) + }(pr) } err = downloadrequest.StreamWithBSLCACert(context.Background(), l.Client, f.Namespace(), l.BackupName, velerov1api.DownloadTargetKindBackupLog, w, l.Timeout, l.InsecureSkipTLSVerify, l.CaCertFile, bslCACert) + + // if pipe writer was used, close it to signal we're done writing + if pw, ok := w.(*io.PipeWriter); ok { + pw.Close() + } + wg.Wait() // wait for all logs to be processed and printed return err } From c9ebc4213f0b90386a5eff3c152c9dd3ef9a20b3 Mon Sep 17 00:00:00 2001 From: iTrooz Date: Thu, 9 Oct 2025 13:31:21 +0200 Subject: [PATCH 07/17] handle logs processing error Signed-off-by: iTrooz --- pkg/cmd/cli/backup/logs.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/cli/backup/logs.go b/pkg/cmd/cli/backup/logs.go index 6119d416d1..101860a270 100644 --- a/pkg/cmd/cli/backup/logs.go +++ b/pkg/cmd/cli/backup/logs.go @@ -81,7 +81,7 @@ func getLevelColor(level string) *color.Color { } // Process logs (by adding color) before printing them -func processAndPrintLogs(r io.Reader, w io.Writer) { +func processAndPrintLogs(r io.Reader, w io.Writer) error { d := logfmt.NewDecoder(r) for d.ScanRecord() { // get record (line) // Scan fields and get color @@ -113,6 +113,10 @@ func processAndPrintLogs(r io.Reader, w io.Writer) { } fmt.Fprintln(w) } + if err := d.Err(); err != nil { + return fmt.Errorf("error processing logs: %v", err) + } + return nil } func (l *LogsOptions) Run(c *cobra.Command, f client.Factory) error { @@ -153,7 +157,10 @@ func (l *LogsOptions) Run(c *cobra.Command, f client.Factory) error { w = pw go func(pr *io.PipeReader) { defer wg.Done() - processAndPrintLogs(pr, os.Stdout) + err := processAndPrintLogs(pr, os.Stdout) + if err != nil { + fmt.Fprintf(os.Stderr, "error processing logs: %v\n", err) + } }(pr) } From 3114ee69d38b1966e393ddbfcc49ac0a60f0a65a Mon Sep 17 00:00:00 2001 From: iTrooz Date: Thu, 9 Oct 2025 13:53:46 +0200 Subject: [PATCH 08/17] refactor into a different file + add helper function to avoid cluttering logic Signed-off-by: iTrooz --- pkg/cmd/cli/backup/logs.go | 86 +-------------------------- pkg/cmd/util/output/logs_color.go | 96 +++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 83 deletions(-) create mode 100644 pkg/cmd/util/output/logs_color.go diff --git a/pkg/cmd/cli/backup/logs.go b/pkg/cmd/cli/backup/logs.go index 101860a270..4318f6e9c5 100644 --- a/pkg/cmd/cli/backup/logs.go +++ b/pkg/cmd/cli/backup/logs.go @@ -19,23 +19,20 @@ package backup import ( "context" "fmt" - "io" "os" - "sync" "time" - "github.com/fatih/color" "github.com/spf13/cobra" "github.com/spf13/pflag" apierrors "k8s.io/apimachinery/pkg/api/errors" kbclient "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/go-logfmt/logfmt" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/client" "github.com/vmware-tanzu/velero/pkg/cmd" "github.com/vmware-tanzu/velero/pkg/cmd/util/cacert" "github.com/vmware-tanzu/velero/pkg/cmd/util/downloadrequest" + "github.com/vmware-tanzu/velero/pkg/cmd/util/output" ) type LogsOptions struct { @@ -65,60 +62,6 @@ func (l *LogsOptions) BindFlags(flags *pflag.FlagSet) { flags.StringVar(&l.CaCertFile, "cacert", l.CaCertFile, "Path to a certificate bundle to use when verifying TLS connections.") } -func getLevelColor(level string) *color.Color { - switch level { - case "info": - return color.New(color.FgGreen) - case "warning": - return color.New(color.FgYellow) - case "error": - return color.New(color.FgRed) - case "debug": - return color.New(color.FgBlue) - default: - return color.New() - } -} - -// Process logs (by adding color) before printing them -func processAndPrintLogs(r io.Reader, w io.Writer) error { - d := logfmt.NewDecoder(r) - for d.ScanRecord() { // get record (line) - // Scan fields and get color - var fields [][2][]byte - var lineColor *color.Color - for d.ScanKeyval() { - fields = append(fields, [2][]byte{d.Key(), d.Value()}) - if string(d.Key()) == "level" { - lineColor = getLevelColor(string(d.Value())) - } - } - - // Re-encode with color. We do not use logfmt Encoder because it does not support color - for _, field := range fields { - var key, value string - if lineColor == nil { // handle case where no color (log level) was found - key = string(field[0]) - value = string(field[1]) - } else { - key = lineColor.Sprintf("%s", field[0]) - if string(field[0]) == "level" { - colorCopy := *lineColor - value = colorCopy.Add(color.Bold).Sprintf("%s", field[1]) - } else { - value = string(field[1]) - } - } - fmt.Fprintf(w, "%s=%s ", key, value) - } - fmt.Fprintln(w) - } - if err := d.Err(); err != nil { - return fmt.Errorf("error processing logs: %v", err) - } - return nil -} - func (l *LogsOptions) Run(c *cobra.Command, f client.Factory) error { backup := new(velerov1api.Backup) err := l.Client.Get(context.Background(), kbclient.ObjectKey{Namespace: f.Namespace(), Name: l.BackupName}, backup) @@ -144,32 +87,9 @@ func (l *LogsOptions) Run(c *cobra.Command, f client.Factory) error { bslCACert = "" } - // Potentially add color to logs - var w io.Writer - var wg sync.WaitGroup - wg.Add(1) - // If NoColor, do not parse logs and directly fall back to stdout - if color.NoColor { - w = os.Stdout - wg.Done() - } else { - pr, pw := io.Pipe() - w = pw - go func(pr *io.PipeReader) { - defer wg.Done() - err := processAndPrintLogs(pr, os.Stdout) - if err != nil { - fmt.Fprintf(os.Stderr, "error processing logs: %v\n", err) - } - }(pr) - } - + w, wg := output.PrintLogsWithColor() err = downloadrequest.StreamWithBSLCACert(context.Background(), l.Client, f.Namespace(), l.BackupName, velerov1api.DownloadTargetKindBackupLog, w, l.Timeout, l.InsecureSkipTLSVerify, l.CaCertFile, bslCACert) - - // if pipe writer was used, close it to signal we're done writing - if pw, ok := w.(*io.PipeWriter); ok { - pw.Close() - } + w.Close() // signal we're done writing wg.Wait() // wait for all logs to be processed and printed return err } diff --git a/pkg/cmd/util/output/logs_color.go b/pkg/cmd/util/output/logs_color.go new file mode 100644 index 0000000000..0780753ef4 --- /dev/null +++ b/pkg/cmd/util/output/logs_color.go @@ -0,0 +1,96 @@ +package output + +import ( + "fmt" + "io" + "os" + "sync" + + "github.com/fatih/color" + "github.com/go-logfmt/logfmt" +) + +func getLevelColor(level string) *color.Color { + switch level { + case "info": + return color.New(color.FgGreen) + case "warning": + return color.New(color.FgYellow) + case "error": + return color.New(color.FgRed) + case "debug": + return color.New(color.FgBlue) + default: + return color.New() + } +} + +// Process logs (by adding color) before printing them +func processAndPrintLogs(r io.Reader, w io.Writer) error { + d := logfmt.NewDecoder(r) + for d.ScanRecord() { // get record (line) + // Scan fields and get color + var fields [][2][]byte + var lineColor *color.Color + for d.ScanKeyval() { + fields = append(fields, [2][]byte{d.Key(), d.Value()}) + if string(d.Key()) == "level" { + lineColor = getLevelColor(string(d.Value())) + } + } + + // Re-encode with color. We do not use logfmt Encoder because it does not support color + for _, field := range fields { + var key, value string + if lineColor == nil { // handle case where no color (log level) was found + key = string(field[0]) + value = string(field[1]) + } else { + key = lineColor.Sprintf("%s", field[0]) + if string(field[0]) == "level" { + colorCopy := *lineColor + value = colorCopy.Add(color.Bold).Sprintf("%s", field[1]) + } else { + value = string(field[1]) + } + } + fmt.Fprintf(w, "%s=%s ", key, value) + } + fmt.Fprintln(w) + } + if err := d.Err(); err != nil { + return fmt.Errorf("error processing logs: %v", err) + } + return nil +} + +type nopCloser struct { + io.Writer +} + +func (nopCloser) Close() error { return nil } + +// Print logfmt-formatted logs to stdout with color based on log level +// if color.NoColor is set, logs will be directly piped to w without processing +// Returns the writer to write logs to, and a waitgroup to wait for processing to finish +// Writer must be closed once all logs have been written +func PrintLogsWithColor() (io.WriteCloser, *sync.WaitGroup) { + // If NoColor, do not parse logs and directly fall back to stdout + var wg sync.WaitGroup + if color.NoColor { + return nopCloser{os.Stdout}, &wg + } else { + wg.Add(1) + pr, pw := io.Pipe() + + // Create coroutine to process logs + go func(pr *io.PipeReader) { + defer wg.Done() + err := processAndPrintLogs(pr, os.Stdout) + if err != nil { + fmt.Fprintf(os.Stderr, "error processing logs: %v\n", err) + } + }(pr) + return pw, &wg + } +} From fa40bbb3a1efa7c8c0563b1d5acd890fcad764bc Mon Sep 17 00:00:00 2001 From: iTrooz Date: Thu, 9 Oct 2025 13:54:00 +0200 Subject: [PATCH 09/17] implement for restore logs too Signed-off-by: iTrooz --- pkg/cmd/cli/restore/logs.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/cli/restore/logs.go b/pkg/cmd/cli/restore/logs.go index f4315c917f..b100c5871f 100644 --- a/pkg/cmd/cli/restore/logs.go +++ b/pkg/cmd/cli/restore/logs.go @@ -31,6 +31,7 @@ import ( "github.com/vmware-tanzu/velero/pkg/cmd" "github.com/vmware-tanzu/velero/pkg/cmd/util/cacert" "github.com/vmware-tanzu/velero/pkg/cmd/util/downloadrequest" + "github.com/vmware-tanzu/velero/pkg/cmd/util/output" ) func NewLogsCommand(f client.Factory) *cobra.Command { @@ -77,7 +78,10 @@ func NewLogsCommand(f client.Factory) *cobra.Command { bslCACert = "" } - err = downloadrequest.StreamWithBSLCACert(context.Background(), kbClient, f.Namespace(), restoreName, velerov1api.DownloadTargetKindRestoreLog, os.Stdout, timeout, insecureSkipTLSVerify, caCertFile, bslCACert) + w, wg := output.PrintLogsWithColor() + err = downloadrequest.StreamWithBSLCACert(context.Background(), kbClient, f.Namespace(), restoreName, velerov1api.DownloadTargetKindRestoreLog, w, timeout, insecureSkipTLSVerify, caCertFile, bslCACert) + w.Close() // signal we're done writing + wg.Wait() // wait for all logs to be processed and printed cmd.CheckError(err) }, } From 4ca48276d28b7e245c3709caed693fd6b1c0f9e4 Mon Sep 17 00:00:00 2001 From: iTrooz Date: Thu, 9 Oct 2025 13:58:59 +0200 Subject: [PATCH 10/17] add changelog entry Signed-off-by: iTrooz --- changelogs/unreleased/9317-itrooz | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelogs/unreleased/9317-itrooz diff --git a/changelogs/unreleased/9317-itrooz b/changelogs/unreleased/9317-itrooz new file mode 100644 index 0000000000..87e7fd841f --- /dev/null +++ b/changelogs/unreleased/9317-itrooz @@ -0,0 +1 @@ +add color to velero logs From 6b1f384a2d1bc3cef6a8473f913cf51814f3ab43 Mon Sep 17 00:00:00 2001 From: iTrooz Date: Thu, 9 Oct 2025 14:01:41 +0200 Subject: [PATCH 11/17] go mod tidy Signed-off-by: iTrooz --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index c42c8df5d6..2f7d273c97 100644 --- a/go.mod +++ b/go.mod @@ -19,6 +19,7 @@ require ( github.com/bombsimon/logrusr/v3 v3.0.0 github.com/evanphx/json-patch/v5 v5.9.11 github.com/fatih/color v1.18.0 + github.com/go-logfmt/logfmt v0.4.0 github.com/gobwas/glob v0.2.3 github.com/google/go-cmp v0.7.0 github.com/google/uuid v1.6.0 @@ -106,7 +107,6 @@ require ( github.com/fxamacker/cbor/v2 v2.7.0 // indirect github.com/go-ini/ini v1.67.0 // indirect github.com/go-jose/go-jose/v4 v4.0.5 // indirect - github.com/go-logfmt/logfmt v0.4.0 // indirect github.com/go-logr/logr v1.4.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/go-ole/go-ole v1.3.0 // indirect From eeaec823244a435f819c0e2ac9772f6920b092d9 Mon Sep 17 00:00:00 2001 From: iTrooz Date: Thu, 9 Oct 2025 16:49:08 +0200 Subject: [PATCH 12/17] quote value if necessary Signed-off-by: iTrooz --- pkg/cmd/util/output/logs_color.go | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/util/output/logs_color.go b/pkg/cmd/util/output/logs_color.go index 0780753ef4..a042352647 100644 --- a/pkg/cmd/util/output/logs_color.go +++ b/pkg/cmd/util/output/logs_color.go @@ -4,7 +4,9 @@ import ( "fmt" "io" "os" + "strings" "sync" + "unicode/utf8" "github.com/fatih/color" "github.com/go-logfmt/logfmt" @@ -25,6 +27,11 @@ func getLevelColor(level string) *color.Color { } } +// https://github.com/go-logfmt/logfmt/blob/e5396c6ee35145aead27da56e7921a7656f69624/encode.go#L235 +func needsQuotedValueRune(r rune) bool { + return r <= ' ' || r == '=' || r == '"' || r == 0x7f || r == utf8.RuneError +} + // Process logs (by adding color) before printing them func processAndPrintLogs(r io.Reader, w io.Writer) error { d := logfmt.NewDecoder(r) @@ -41,18 +48,21 @@ func processAndPrintLogs(r io.Reader, w io.Writer) error { // Re-encode with color. We do not use logfmt Encoder because it does not support color for _, field := range fields { - var key, value string - if lineColor == nil { // handle case where no color (log level) was found - key = string(field[0]) - value = string(field[1]) - } else { - key = lineColor.Sprintf("%s", field[0]) - if string(field[0]) == "level" { + key := string(field[0]) + value := string(field[1]) + + // Quote if needed + if strings.IndexFunc(value, needsQuotedValueRune) != -1 { + value = fmt.Sprintf("%q", value) + } + + // Add color + if lineColor != nil { // handle case where no color (log level) was found + if key == "level" { colorCopy := *lineColor - value = colorCopy.Add(color.Bold).Sprintf("%s", field[1]) - } else { - value = string(field[1]) + value = colorCopy.Add(color.Bold).Sprintf("%s", value) } + key = lineColor.Sprintf("%s", field[0]) } fmt.Fprintf(w, "%s=%s ", key, value) } From 61a541671008a3a00094124a2759f845e502d5b5 Mon Sep 17 00:00:00 2001 From: iTrooz Date: Sat, 25 Oct 2025 20:22:01 +0200 Subject: [PATCH 13/17] update doc Signed-off-by: iTrooz --- pkg/cmd/util/output/logs_color.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/util/output/logs_color.go b/pkg/cmd/util/output/logs_color.go index a042352647..3733f6a1bb 100644 --- a/pkg/cmd/util/output/logs_color.go +++ b/pkg/cmd/util/output/logs_color.go @@ -81,7 +81,7 @@ type nopCloser struct { func (nopCloser) Close() error { return nil } // Print logfmt-formatted logs to stdout with color based on log level -// if color.NoColor is set, logs will be directly piped to w without processing +// if color.NoColor is set, logs will be directly piped to stdout without processing // Returns the writer to write logs to, and a waitgroup to wait for processing to finish // Writer must be closed once all logs have been written func PrintLogsWithColor() (io.WriteCloser, *sync.WaitGroup) { From 05f9f0f84307b0b528b2a1a3ae9eaa1cd59fb65f Mon Sep 17 00:00:00 2001 From: iTrooz Date: Sun, 26 Oct 2025 21:06:50 +0100 Subject: [PATCH 14/17] add copyright Signed-off-by: iTrooz --- pkg/cmd/util/output/logs_color.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/pkg/cmd/util/output/logs_color.go b/pkg/cmd/util/output/logs_color.go index 3733f6a1bb..5261fb6983 100644 --- a/pkg/cmd/util/output/logs_color.go +++ b/pkg/cmd/util/output/logs_color.go @@ -1,3 +1,19 @@ +/* +Copyright the Velero contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package output import ( From 5a42287f1c311e6798c8132294a4dc3ce2bd0277 Mon Sep 17 00:00:00 2001 From: iTrooz Date: Sun, 26 Oct 2025 21:07:04 +0100 Subject: [PATCH 15/17] do not print extra space after lines Signed-off-by: iTrooz --- pkg/cmd/util/output/logs_color.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/util/output/logs_color.go b/pkg/cmd/util/output/logs_color.go index 5261fb6983..4ea56f4acf 100644 --- a/pkg/cmd/util/output/logs_color.go +++ b/pkg/cmd/util/output/logs_color.go @@ -63,7 +63,7 @@ func processAndPrintLogs(r io.Reader, w io.Writer) error { } // Re-encode with color. We do not use logfmt Encoder because it does not support color - for _, field := range fields { + for i, field := range fields { key := string(field[0]) value := string(field[1]) @@ -80,7 +80,10 @@ func processAndPrintLogs(r io.Reader, w io.Writer) error { } key = lineColor.Sprintf("%s", field[0]) } - fmt.Fprintf(w, "%s=%s ", key, value) + if i != 0 { + fmt.Fprint(w, " ") + } + fmt.Fprintf(w, "%s=%s", key, value) } fmt.Fprintln(w) } From 972b28dd7f69fe2c2df2dc07f809a14252554ee5 Mon Sep 17 00:00:00 2001 From: iTrooz Date: Sun, 26 Oct 2025 21:07:46 +0100 Subject: [PATCH 16/17] add comments Signed-off-by: iTrooz --- pkg/cmd/util/output/logs_color.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/cmd/util/output/logs_color.go b/pkg/cmd/util/output/logs_color.go index 4ea56f4acf..c2fd41dbe4 100644 --- a/pkg/cmd/util/output/logs_color.go +++ b/pkg/cmd/util/output/logs_color.go @@ -103,12 +103,14 @@ func (nopCloser) Close() error { return nil } // if color.NoColor is set, logs will be directly piped to stdout without processing // Returns the writer to write logs to, and a waitgroup to wait for processing to finish // Writer must be closed once all logs have been written +// Note: this function is a wrapper around processAndPrintLogs to avoid always creating a goroutine func PrintLogsWithColor() (io.WriteCloser, *sync.WaitGroup) { // If NoColor, do not parse logs and directly fall back to stdout var wg sync.WaitGroup if color.NoColor { return nopCloser{os.Stdout}, &wg } else { + // Else, create a goroutine to process logs. wg.Add(1) pr, pw := io.Pipe() From 2fb573e350d4a20bbf283b4ecafff748bdb36c53 Mon Sep 17 00:00:00 2001 From: iTrooz Date: Sun, 26 Oct 2025 21:07:50 +0100 Subject: [PATCH 17/17] add tests Signed-off-by: iTrooz --- pkg/cmd/util/output/logs_color_test.go | 76 ++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 pkg/cmd/util/output/logs_color_test.go diff --git a/pkg/cmd/util/output/logs_color_test.go b/pkg/cmd/util/output/logs_color_test.go new file mode 100644 index 0000000000..9db93a1556 --- /dev/null +++ b/pkg/cmd/util/output/logs_color_test.go @@ -0,0 +1,76 @@ +/* +Copyright the Velero contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package output + +import ( + "bytes" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +var LOG_LINE = "level=info msg=\"This is a test log\" key1=value1 key2=\"value with spaces\"" +var LOG_LINE_2 = "level=INVALID msg=\"This is a second test log\" key1=value1 key2=\"value 2 with spaces\"" +var LOG_LINE_3 = "level=error msg=\"This is a thirdtest log\" key1=value1 key2=\"value 3 with spaces\"" + +// Note that all comparisons in this file work because color.NoColor is set to false by default, and thus no colors are added, even +// through the color adding code is run. + +func TestColoredLogHasLog(t *testing.T) { + inputBuf := &bytes.Buffer{} + inputBuf.WriteString(LOG_LINE) + + outputBuf := &bytes.Buffer{} + err := processAndPrintLogs(inputBuf, outputBuf) + if err != nil { + t.Fatalf("processAndPrintLogs returned error: %v", err) + } + + assert.Contains(t, outputBuf.String(), "This is a test log") +} + +// Test log line is unchanged since log is decomposed and re-composed +func TestColoredLogIsSameAsUncoloredLog(t *testing.T) { + inputBuf := &bytes.Buffer{} + inputBuf.WriteString(LOG_LINE) + + outputBuf := &bytes.Buffer{} + err := processAndPrintLogs(inputBuf, outputBuf) + if err != nil { + t.Fatalf("processAndPrintLogs returned error: %v", err) + } + + assert.Equal(t, LOG_LINE+"\n", outputBuf.String()) +} + +// Test all log lines are sent correctly (and unchanged) +func TestMultipleColoredLogs(t *testing.T) { + inputBuf := &bytes.Buffer{} + inputBuf.WriteString(LOG_LINE) + inputBuf.WriteString("\n") + inputBuf.WriteString(LOG_LINE_2) + inputBuf.WriteString("\n") + inputBuf.WriteString(LOG_LINE_3) + + outputBuf := &bytes.Buffer{} + err := processAndPrintLogs(inputBuf, outputBuf) + if err != nil { + t.Fatalf("processAndPrintLogs returned error: %v", err) + } + + assert.Equal(t, fmt.Sprintf("%v\n%v\n%v\n", LOG_LINE, LOG_LINE_2, LOG_LINE_3), outputBuf.String()) +}