fix(cli): wire --debug flag to logger level #499
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #499 +/- ##
==========================================
+ Coverage 62.83% 63.13% +0.29%
==========================================
Files 62 62
Lines 4765 4771 +6
==========================================
+ Hits 2994 3012 +18
+ Misses 1527 1509 -18
- Partials 244 250 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
21b72c3 to
a7bcc12
Compare
|
Hey @y1yang0 just a heads up with the stderr tee, --debug surfaces a pretty verbose existing log in find.go:122 that dumps the full build plan as a single field. Makes the terminal output noisy. Should I drop the stderr tee and keep this to just the level fix, or is the tee worth keeping? |
|
I prefer keeping it as the fix only |
a7bcc12 to
82e02e7
Compare
82e02e7 to
52c8498
Compare
|
I have added Sources: cli.EnvVars(...) and simplified the os.Getenv check. Integration test failure is a pre-existing timeout on Windows (same 600s timeout reproduces on main without my changes, tested that locally). |
There was a problem hiding this comment.
Pull request overview
This PR wires the CLI --debug flag into logger level selection and adds an environment variable path so child toolexec invocations can inherit debug logging.
Changes:
- Adds
OTELC_DEBUGas a shared environment variable constant. - Sources the
--debugflag fromOTELC_DEBUG. - Sets
slog.LevelDebugand exportsOTELC_DEBUG=1when debug mode is enabled.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
tool/util/shared.go |
Adds the shared OTELC_DEBUG environment variable constant. |
tool/cmd/main.go |
Reads the debug flag/env source and uses it to configure logger level and child process propagation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Hey @y1yang0 all review feedback has been addressed, PTAL when you have the time thank you. |
|
Please fix test failure |
52c8498 to
c41ce1d
Compare
c41ce1d to
ba88777
Compare
|
Hey @y1yang0 so i added unit tests for initLogger covering default level, debug flag , env var propagation and cli.envVars sourcing. Remaining codecov gap is error return paths....that i cant trigger without mocking stdlib but atleast integration tests are passing now. do let me know if any changes are to be made.... |
Description
The
--debugflag is defined in the CLI butinitLoggernever reads it — the log level is hardcoded toslog.LevelInforegardless. This means allDebug()calls across setup and instrument phases are silently filtered out.This wires
cmd.Bool("debug")to setslog.LevelDebugwhen the flag is active, Debug output goes to debug.log in the work directory, consistent with existing logging behavior.Motivation
The flag has never actually worked, it was introduced in #50 as
"-debug"(triple dash), #281 fixed the typo but the wiring to the logger was never added. There are 13 existingDebug()calls across the codebase that are currently unreachable.Fixes #497
Checklist
make formatmake lintmake test