feat: print logging level on startup and centralize DEBUG constant#611
feat: print logging level on startup and centralize DEBUG constant#611
Conversation
lpcox
commented
Feb 3, 2026
- Print logging verbosity level to stdout on startup
- Create constants.go with EnvDebug and EnvDebugColors constants
- Refactor all DEBUG env var references to use logger.EnvDebug
- Update tests to use the centralized constant
- Print logging verbosity level to stdout on startup - Create constants.go with EnvDebug and EnvDebugColors constants - Refactor all DEBUG env var references to use logger.EnvDebug - Update tests to use the centralized constant
There was a problem hiding this comment.
Pull request overview
This PR centralizes logging-related environment variable names and surfaces the effective logging verbosity during startup, while wiring this behavior into tests and CI workflows.
Changes:
- Introduces
internal/logger/constants.gowithEnvDebugandEnvDebugColorsand refactors logger code and tests to use these constants for readingDEBUGandDEBUG_COLORS. - Updates
preRunininternal/cmd/root.goto derive logging verbosity from theverbosityflag, set theDEBUGenv var accordingly when unset, and print a human-readable logging level (or the existingDEBUGvalue) via the standard logger. - Adjusts tests (
internal/cmd/root_test.go,internal/logger/slog_adapter_test.go) and the smoke Copilot workflow to align with the centralizedDEBUGhandling and to propagateDEBUGcleanly into the MCP gateway container.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
internal/logger/constants.go |
Adds EnvDebug and EnvDebugColors constants with documentation to centralize logging-related env var names. |
internal/logger/logger.go |
Switches color control and DEBUG pattern evaluation to use EnvDebugColors/EnvDebug, keeping logger behavior but centralizing configuration keys. |
internal/logger/slog_adapter_test.go |
Updates slog adapter tests to use EnvDebug instead of the raw "DEBUG" string when deciding whether to run or skip. |
internal/cmd/root.go |
Enhances preRun to read DEBUG via logger.EnvDebug, conditionally set it based on verbosity, and log the resolved logging level or inherited DEBUG setting. |
internal/cmd/root_test.go |
Aligns pre-run validation tests with logger.EnvDebug, ensuring new verbosity-to-DEBUG behavior is covered and respects pre-existing env values. |
.github/workflows/smoke-copilot.lock.yml |
Exports DEBUG="*" explicitly and passes it through to the MCP gateway container via -e DEBUG, simplifying the docker env wiring. |
Comments suppressed due to low confidence (1)
internal/cmd/root.go:117
- Similar to the debug level case above, this branch still hardcodes the "DEBUG" environment variable name instead of using the new
logger.EnvDebugconstant, even though the PR description calls out centralizing DEBUG env var references. For consistency and easier future refactoring, it would be better to uselogger.EnvDebughere as well.
os.Setenv("DEBUG", debugAllPackages)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| log.Printf("Logging level: info (-v)") | ||
| case 2: | ||
| // Debug level - enable debug logs for main packages | ||
| os.Setenv("DEBUG", debugMainPackages) |
There was a problem hiding this comment.
This switch still hardcodes the "DEBUG" environment variable name when setting verbosity-derived patterns, even though this PR introduces the logger.EnvDebug constant and the description says all DEBUG env var references should be centralized. To keep the environment key definition in one place and avoid future drift, consider using logger.EnvDebug in this os.Setenv call.
| log.Printf("Logging level: info (-v)") | ||
| case 2: | ||
| // Debug level - enable debug logs for main packages | ||
| os.Setenv("DEBUG", debugMainPackages) | ||
| debugLog.Printf("Verbosity level: debug (DEBUG=%s)", debugMainPackages) | ||
| log.Printf("Logging level: debug (-vv), DEBUG=%s", debugMainPackages) |
There was a problem hiding this comment.
The new log.Printf("Logging level: ...") calls use Go's default logger, which writes to stderr by default, but the PR description says the verbosity level should be printed to stdout on startup. If the goal is truly stdout, consider either configuring the standard logger's output to os.Stdout or using a dedicated logger that targets stdout, or otherwise update the description/comments to match the actual behavior.