Skip to content

Add --log-color flag to control ANSI color log output#1374

Open
zhengxiexie wants to merge 1 commit intovmware-tanzu:mainfrom
zhengxiexie:topic/zhengxie/main/log_color
Open

Add --log-color flag to control ANSI color log output#1374
zhengxiexie wants to merge 1 commit intovmware-tanzu:mainfrom
zhengxiexie:topic/zhengxie/main/log_color

Conversation

@zhengxiexie
Copy link
Contributor

@zhengxiexie zhengxiexie commented Feb 9, 2026

✨ What's Changed

Log Color Control

  • ANSI color output is now disabled by default in log output
  • A new --log-color flag is added to explicitly enable colored log output
  • Color is fully decoupled from log level — users opt in via --log-color

Implementation Details

  • Added LogColor config variable and --log-color CLI flag (default false) in pkg/config/config.go
  • Updated ZapCustomLogger signature to accept enableColor bool parameter
  • Set ConsoleWriter.NoColor based on the flag to control zerolog color output
  • Guarded FormatLevel and FormatCaller color wrapping with enableColor check
  • Widened log level switch from == 2 to >= 2 for TraceLevel mapping
  • Registered --log-color flag in all entry points: cmd/main.go, cmd_clean/main.go, test/e2e/main_test.go
  • Added unit tests TestGetLogLevel and TestAnsiColorControlledByFlag

🎯 Motivation

ANSI color escape codes were always emitted in log output regardless of log level. This causes garbled text in non-terminal environments such as kubectl logs, log aggregators, and CI pipelines where ANSI codes are not interpreted.

Issue: Log output contains raw ANSI escape sequences (e.g. \033[32m) when viewed in environments that don't support terminal colors, making logs harder to read and parse.

Solution: Make color output opt-in via a dedicated --log-color flag, keeping it independent from log verbosity levels. This is typically used only during local development/debugging where a real terminal is available.

✅ Testing

Unit Tests

  • All existing unit tests pass
  • Added new test coverage:
    • TestGetLogLevel - Validates log level calculation for all debug/logLevel combinations (default, debug_flag_only, log_level_1, log_level_3, debug_with_higher_level, debug_with_lower_level)
    • TestAnsiColorControlledByFlag - Captures log output to buffer and verifies:
      • color_disabled - No ANSI escape codes when --log-color is not set
      • color_enabled - ANSI escape codes present when --log-color is set
$ go test ./pkg/logger/ -v -run "TestGetLogLevel|TestAnsiColor"
=== RUN   TestGetLogLevel
=== RUN   TestGetLogLevel/default
=== RUN   TestGetLogLevel/debug_flag_only
=== RUN   TestGetLogLevel/log_level_1
=== RUN   TestGetLogLevel/log_level_3
=== RUN   TestGetLogLevel/debug_with_higher_level
=== RUN   TestGetLogLevel/debug_with_lower_level
--- PASS: TestGetLogLevel (0.00s)
=== RUN   TestAnsiColorControlledByFlag
=== RUN   TestAnsiColorControlledByFlag/color_disabled
=== RUN   TestAnsiColorControlledByFlag/color_enabled
--- PASS: TestAnsiColorControlledByFlag (0.00s)
PASS

🔄 Backward Compatibility

This change is fully backward compatible:

  • Default behavior now outputs plain text instead of colored text, which is strictly better for production environments
  • Users who want colored output can explicitly opt in with --log-color
  • No configuration file changes or API changes required

@zhengxiexie zhengxiexie force-pushed the topic/zhengxie/main/log_color branch from 58d11e4 to 0a1dd6b Compare February 9, 2026 02:58
@zhengxiexie
Copy link
Contributor Author

/e2e

@codecov-commenter
Copy link

codecov-commenter commented Feb 9, 2026

Codecov Report

❌ Patch coverage is 64.28571% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.58%. Comparing base (0334455) to head (9eab983).

Files with missing lines Patch % Lines
cmd_clean/main.go 0.00% 3 Missing ⚠️
cmd/main.go 0.00% 1 Missing ⚠️
pkg/config/config.go 0.00% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (64.28%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1374      +/-   ##
==========================================
- Coverage   76.65%   76.58%   -0.07%     
==========================================
  Files         151      151              
  Lines       21102    21109       +7     
==========================================
- Hits        16175    16166       -9     
- Misses       3773     3788      +15     
- Partials     1154     1155       +1     
Flag Coverage Δ
unit-tests 76.58% <64.28%> (-0.07%) ⬇️
Files with missing lines Coverage Δ
pkg/clean/clean.go 89.28% <100.00%> (ø)
pkg/logger/logger.go 73.91% <100.00%> (-12.30%) ⬇️
cmd/main.go 0.00% <0.00%> (ø)
pkg/config/config.go 70.89% <0.00%> (-0.34%) ⬇️
cmd_clean/main.go 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

// ZapCustomLogger creates a CustomLogger with both logr.Logger and zerolog.Logger using the same configuration as ZapLogger
func ZapCustomLogger(cfDebug bool, cfLogLevel int) CustomLogger {
logLevel := getLogLevel(cfDebug, cfLogLevel)
enableColor := logLevel >= 3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use a individual flag to control the color switch? I feel that we'd better not bind the color with a specific log level..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok,done

@zhengxiexie zhengxiexie force-pushed the topic/zhengxie/main/log_color branch from 0a1dd6b to 03063d5 Compare February 9, 2026 07:11
@zhengxiexie zhengxiexie changed the title Enable ANSI color log output only when log-level >= 3 Add --log-color flag to control ANSI color log output Feb 9, 2026
@zhengxiexie
Copy link
Contributor Author

/e2e

1 similar comment
@zhengxiexie
Copy link
Contributor Author

/e2e

log = logger.ZapCustomLogger(cf.DefaultConfig.Debug, config.LogLevel, config.LogColor)
logger.Log = log
logf.SetLogger(log.Logger)
err := clean.Clean(ctx, cf, &log.Logger, cf.DefaultConfig.Debug, config.LogLevel)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pass config.LogColor to clean.Clean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok,done

- Updated `ZapCustomLogger` to accept `enableColor` flag instead of relying on log level.
- Added `--log-color` flag to main programs and test frameworks for explicit control.
- Updated tests to validate color handling based on the flag setting.

Change-Id: I80a900072a535f0879f82ea99a90d68763da2aaf
@zhengxiexie zhengxiexie force-pushed the topic/zhengxie/main/log_color branch from 03063d5 to 9eab983 Compare February 10, 2026 06:43
Copy link
Contributor

@yanjunz97 yanjunz97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants