feat: add trace log level support for enhanced debugging#1525
Conversation
📝 WalkthroughWalkthroughReplaces the legacy logger with a new internal Atmos logger (pkg/logger), swaps charmbracelet/log imports across the repo, reworks cmd/root.go to support persistent log files, Trace handling, and Cleanup(), adds signal-driven cleanup in main, updates linter rules, tests, and logging docs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Main
participant Root as cmd/root.setupLogger
participant Logger as pkg/logger (global)
participant FS as Filesystem
Main->>Root: setupLogger(cfg)
Root->>Logger: ParseLogLevel(cfg.Logs.Level) -> SetLevel
alt cfg.Logs.File is /dev/stdout|/dev/stderr|/dev/null|""
Root->>Logger: SetOutput(special writer)
else cfg.Logs.File is real path
Root->>FS: Open file (persistent handle)
Root->>Logger: SetOutput(file handle)
end
Root-->>Main: logger configured
Note over Main,Root: exit or signal
Main->>Root: defer/trigger Cleanup()
Root->>FS: Close file handle if open
sequenceDiagram
autonumber
participant Caller as Application code
participant Facade as pkg/logger (log.go)
participant Default as logger.Default()
participant Charm as charm.Logger
Caller->>Facade: Info("msg", keyvals...)
Facade->>Default: Forward Info(...)
Default->>Charm: Info(...) -> underlying write
Charm-->>Facade: output written
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
7d10685 to
8e54645
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1525 +/- ##
==========================================
+ Coverage 57.51% 57.81% +0.30%
==========================================
Files 279 282 +3
Lines 30436 30576 +140
==========================================
+ Hits 17505 17678 +173
+ Misses 11078 11047 -31
+ Partials 1853 1851 -2
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:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
cmd/root_test.go (1)
128-166: Level mapping test is clear; avoid OS-specific paths in config.Assertion logic is good. For portability, prefer leaving Logs.File empty in tests that only check levels to avoid hard-coding /dev/stderr (Windows).
Proposed tweak:
- File: "/dev/stderr", // Default log file + File: "", // Use pre-set output or default; avoids OS-specific paths
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
cmd/root.go(3 hunks)cmd/root_test.go(2 hunks)pkg/logger/levels.go(1 hunks)pkg/logger/levels_test.go(1 hunks)tests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Files:
pkg/logger/levels.gopkg/logger/levels_test.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All Go comments must end with periods (enforced by golangci-lint's godot).
Wrap all errors with static errors defined in errors/errors.go; never return dynamic errors directly; use fmt.Errorf with %w and add details after the static error.
Always bind environment variables using viper.BindEnv and provide an ATMOS_ alternative for each external env var.
Use structured logging for system/debug events; logging must not affect execution and should use appropriate levels per docs/logging.md.
Prefer SDKs over shelling out to binaries for cross-platform compatibility; use filepath/os/runtime for portable paths and OS-specific logic.
Files:
pkg/logger/levels.gopkg/logger/levels_test.gocmd/root_test.gocmd/root.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
pkg/logger/levels.gocmd/root.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios
**/*_test.go: Use table-driven tests for unit tests and focus on pure functions where possible.
Always use t.Skipf with a clear reason when skipping tests; never use t.Skip.
For CLI tests requiring rebuilt binaries, set a package-level skipReason in TestMain and call os.Exit(m.Run()); individual tests must check and t.Skipf if set; never use log.Fatal for missing/stale binaries.
Mirror test file names to their implementation files (e.g., aws_ssm_store_test.go for aws_ssm_store.go) and co-locate tests with implementation.
Files:
pkg/logger/levels_test.gocmd/root_test.go
pkg/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Place unit tests under pkg/** matching the package they test.
Files:
pkg/logger/levels_test.go
cmd/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands
Implement each CLI command in a separate file under cmd/
Use Viper for managing configuration, environment variables, and flags in the CLI
Keep separation of concerns between CLI interface (cmd) and business logic
Use kebab-case for command-line flags
Provide comprehensive help text for all commands and flags
Include examples in Cobra command help
Use Viper for configuration management; support files, env vars, and flags with precedence flags > env > config > defaults
Follow single responsibility; separate command interface from business logic
Provide meaningful user feedback and include progress indicators for long-running operations
Provide clear error messages to users and troubleshooting hints where appropriate
cmd/**/*.go: Use utils.PrintfMarkdown to render embedded markdown content for CLI help/examples.
One Cobra command per file in cmd/ to keep files focused and maintainable.
Send UI prompts/status/progress to stderr and data/results to stdout; never use logging for UI.
Prefer utils.PrintfMessageToTUI for UI messages; only write directly to os.Stderr as a last resort.
For non-standard execution paths, capture telemetry with telemetry.CaptureCmd or telemetry.CaptureCmdString.
Files:
cmd/root_test.gocmd/root.go
cmd/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Place command tests under cmd/** with *_test.go naming.
Files:
cmd/root_test.go
cmd/root.go
📄 CodeRabbit inference engine (CLAUDE.md)
Commands added to RootCmd automatically capture telemetry via RootCmd.ExecuteC(); ensure new commands are wired into RootCmd.
Files:
cmd/root.go
🧠 Learnings (18)
📓 Common learnings
Learnt from: Listener430
PR: cloudposse/atmos#984
File: internal/exec/copy_glob.go:0-0
Timestamp: 2025-02-06T13:38:07.216Z
Learning: The `u.LogTrace` function in the `cloudposse/atmos` repository accepts `atmosConfig` as its first parameter, followed by the message string.
📚 Learning: 2025-01-30T16:56:43.004Z
Learnt from: mcalhoun
PR: cloudposse/atmos#980
File: pkg/utils/log_utils.go:62-63
Timestamp: 2025-01-30T16:56:43.004Z
Learning: In pkg/utils/log_utils.go, LogTrace is intentionally redirected to LogDebug since charmbracelet logger doesn't support Trace level, maintaining backward compatibility with the original LogTrace functionality.
Applied to files:
pkg/logger/levels.gopkg/logger/levels_test.gocmd/root_test.gocmd/root.go
📚 Learning: 2025-09-08T16:26:41.607Z
Learnt from: Benbentwo
PR: cloudposse/atmos#1452
File: internal/auth/manager.go:0-0
Timestamp: 2025-09-08T16:26:41.607Z
Learning: charmbracelet/log library supports printf-style logging methods including Debugf, Infof, Warnf, Errorf, and Fatalf in addition to structured logging methods.
Applied to files:
pkg/logger/levels.go
📚 Learning: 2025-02-06T13:38:07.216Z
Learnt from: Listener430
PR: cloudposse/atmos#984
File: internal/exec/copy_glob.go:0-0
Timestamp: 2025-02-06T13:38:07.216Z
Learning: The `u.LogTrace` function in the `cloudposse/atmos` repository accepts `atmosConfig` as its first parameter, followed by the message string.
Applied to files:
tests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.goldencmd/root.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests
Applied to files:
pkg/logger/levels_test.gocmd/root_test.go
📚 Learning: 2025-09-25T03:30:16.210Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-25T03:30:16.210Z
Learning: Applies to **/*.go : Use structured logging for system/debug events; logging must not affect execution and should use appropriate levels per docs/logging.md.
Applied to files:
pkg/logger/levels_test.gocmd/root.go
📚 Learning: 2025-09-25T03:30:16.210Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-25T03:30:16.210Z
Learning: Applies to pkg/**/*_test.go : Place unit tests under pkg/** matching the package they test.
Applied to files:
pkg/logger/levels_test.go
📚 Learning: 2025-09-25T03:30:16.210Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-25T03:30:16.210Z
Learning: Applies to tests/**/*_test.go : Place integration tests under tests/** with *_test.go naming and use fixtures in tests/test-cases/.
Applied to files:
pkg/logger/levels_test.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Test both happy paths and error conditions
Applied to files:
pkg/logger/levels_test.gocmd/root_test.go
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
PR: cloudposse/atmos#1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: The atmos codebase has a custom extension to *testing.T that provides a Chdir method, allowing test functions to call t.Chdir() to change working directories during tests. This is used consistently across test files in the codebase.
Applied to files:
cmd/root_test.go
📚 Learning: 2025-09-25T03:30:16.210Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-25T03:30:16.210Z
Learning: Applies to cmd/**/*_test.go : Place command tests under cmd/** with *_test.go naming.
Applied to files:
cmd/root_test.go
📚 Learning: 2025-09-25T03:30:16.209Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-25T03:30:16.209Z
Learning: Applies to pkg/config/**/*.go : Use Viper for configuration management with config name "atmos", add config path ".", and AutomaticEnv with ATMOS prefix.
Applied to files:
cmd/root.go
📚 Learning: 2024-12-11T18:40:12.808Z
Learnt from: Listener430
PR: cloudposse/atmos#844
File: cmd/helmfile.go:37-37
Timestamp: 2024-12-11T18:40:12.808Z
Learning: In the atmos project, `cliConfig` is initialized within the `cmd` package in `root.go` and can be used in other command files.
Applied to files:
cmd/root.go
📚 Learning: 2024-10-20T13:12:46.499Z
Learnt from: haitham911
PR: cloudposse/atmos#736
File: pkg/config/const.go:6-6
Timestamp: 2024-10-20T13:12:46.499Z
Learning: In `cmd/cmd_utils.go`, it's acceptable to have hardcoded references to `atmos.yaml` in logs, and it's not necessary to update them to use the `CliConfigFileName` constant.
Applied to files:
cmd/root.go
📚 Learning: 2025-08-16T23:32:40.412Z
Learnt from: aknysh
PR: cloudposse/atmos#1405
File: internal/exec/describe_dependents_test.go:455-456
Timestamp: 2025-08-16T23:32:40.412Z
Learning: In the cloudposse/atmos Go codebase, `InitCliConfig` returns a `schema.AtmosConfiguration` value (not a pointer), while `ExecuteDescribeDependents` expects a `*schema.AtmosConfiguration` pointer parameter. Therefore, when passing the result of `InitCliConfig` to `ExecuteDescribeDependents`, use `&atmosConfig` to pass the address of the value.
Applied to files:
cmd/root.go
📚 Learning: 2025-08-16T23:33:07.477Z
Learnt from: aknysh
PR: cloudposse/atmos#1405
File: internal/exec/describe_dependents_test.go:651-652
Timestamp: 2025-08-16T23:33:07.477Z
Learning: In the cloudposse/atmos Go codebase, ExecuteDescribeDependents expects a pointer to AtmosConfiguration (*schema.AtmosConfiguration), so when calling it with a value returned by cfg.InitCliConfig (which returns schema.AtmosConfiguration), the address-of operator (&) is necessary: ExecuteDescribeDependents(&atmosConfig, ...).
Applied to files:
cmd/root.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
PR: cloudposse/atmos#740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
cmd/root.go
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
PR: cloudposse/atmos#1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
cmd/root.go
🧬 Code graph analysis (3)
pkg/logger/levels_test.go (1)
pkg/logger/levels.go (4)
TraceLevel(11-11)Trace(14-16)Tracef(19-21)GetLevelString(25-35)
cmd/root_test.go (2)
pkg/logger/levels.go (2)
TraceLevel(11-11)Trace(14-16)pkg/schema/schema.go (5)
AtmosConfiguration(26-61)Logs(372-375)Settings(677-681)AtmosSettings(247-267)Terminal(204-212)
cmd/root.go (4)
pkg/logger/levels.go (3)
TraceLevel(11-11)Trace(14-16)GetLevelString(25-35)pkg/schema/schema.go (3)
Settings(677-681)Terminal(204-212)Logs(372-375)errors/error_funcs.go (1)
CheckErrorPrintAndExit(55-72)pkg/logger/logger.go (1)
ParseLogLevel(52-65)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Run pre-commit hooks
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Summary
🔇 Additional comments (10)
pkg/logger/levels_test.go (4)
14-21: Good guard: Trace is strictly more verbose than Debug.Validates relative ordering against upstream constants, protecting against future upstream changes.
44-86: Solid visibility tests; careful global state reset.Tests correctly isolate logger state by saving/restoring level and output.
120-193: Clear level-filtering matrix.Table-driven checks across Trace/Debug/Info/Warn are comprehensive and easy to extend.
263-289: Covers string mapping (trace/off) thoroughly.Validates GetLevelString for custom Trace and sentinel Off.
pkg/logger/levels.go (2)
11-16: API surface for Trace is minimal and idiomatic.Using log.Log/Logf at a custom level is the right approach; avoids forking the logger.
11-21: No legacyLogTracecalls detected
Search forLogTrace(across all.gofiles only matched the wrapper definitions inlog_utils.go; no migration needed.cmd/root_test.go (3)
168-251: Visibility tests are robust and aligned with Trace semantics.Buffered output + contains checks reliably validate filtering for trace/debug/info.
253-285: ENV-driven level selection test is on point.Confirms ATMOS_LOGS_LEVEL=Trace is honored by setupLogger.
286-315: No-color path sanity check is helpful.Catches regressions in styling when NO_COLOR-like conditions are in effect.
tests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden (1)
1-1: Abbreviation consistency confirmed – only TRAC appears in code, tests, and docs
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
|
Important Cloud Posse Engineering Team Review RequiredThis pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes. To expedite this process, reach out to us on Slack in the |
|
Warning This PR exceeds the recommended limit of 1,000 lines.Large PRs are difficult to review and may be rejected due to their size. Please verify that this PR does not address multiple issues. |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cmd/root.go (1)
93-166: Normalize the configured log level before setting it.Even after the parser fix above,
setupLoggerstill switches on the raw string, soATMOS_LOGS_LEVEL=debugdrops intodefault, bumps us to Warn, and then the validation step exits. We need to trim/parse once and drive the level from that canonical value. Bonus: checking the active level (instead of the raw string) keeps the “Set logs-level …” trace message correct.- switch atmosConfig.Logs.Level { - case "Trace": - log.SetLevel(log.TraceLevel) - case "Debug": - log.SetLevel(log.DebugLevel) - case "Info": - log.SetLevel(log.InfoLevel) - case "Warning": - log.SetLevel(log.WarnLevel) - case "Off": - log.SetLevel(math.MaxInt32) - default: - log.SetLevel(log.WarnLevel) - } + levelStr := strings.TrimSpace(atmosConfig.Logs.Level) + if levelStr == "" { + log.SetLevel(log.WarnLevel) + } else { + parsedLevel, err := log.ParseLogLevel(levelStr) + if err != nil { + errUtils.CheckErrorPrintAndExit(err, "", "") + } + + switch parsedLevel { + case log.LogLevelTrace: + log.SetLevel(log.TraceLevel) + case log.LogLevelDebug: + log.SetLevel(log.DebugLevel) + case log.LogLevelInfo: + log.SetLevel(log.InfoLevel) + case log.LogLevelWarning: + log.SetLevel(log.WarnLevel) + case log.LogLevelOff: + log.SetLevel(log.Level(math.MaxInt32)) + } + } @@ - if _, err := log.ParseLogLevel(atmosConfig.Logs.Level); err != nil { - errUtils.CheckErrorPrintAndExit(err, "", "") - } - // Use trace level for this message when trace is enabled, otherwise debug - if atmosConfig.Logs.Level == "Trace" { + // Use trace level for this message when trace is enabled, otherwise debug. + if log.GetLevel() == log.TraceLevel { log.Trace("Set", "logs-level", log.GetLevelString(), "logs-file", atmosConfig.Logs.File) } else { log.Debug("Set", "logs-level", log.GetLevelString(), "logs-file", atmosConfig.Logs.File) }internal/exec/terraform_test.go (1)
748-755: Restore the original default logger after the subtest.We grab
originalLogger := log.Default()before replacing the default, but after the subtest finishes we never switch the global default back—log.SetDefault(log.Default())is a no-op. Without restoring, later tests inherit the temporary logger (and its output writer), which can skew their observations or leave file descriptors open. Please replace that call withlog.SetDefault(originalLogger)(or reuse the existing defer) so we reset state before the function returns.
🧹 Nitpick comments (8)
internal/exec/pro.go (3)
156-159: Prefer structured logging fields for lock success.We just added the global logger; this is a good moment to emit the lock details as structured fields instead of a flurry of newline-terminated strings. One call keeps the record tidy and avoids double newlines from the formatter.
- log.Info("Stack successfully locked.\n") - log.Info(fmt.Sprintf("Key: %s", lock.Data.Key)) - log.Info(fmt.Sprintf("LockID: %s", lock.Data.ID)) - log.Info(fmt.Sprintf("Expires %s", lock.Data.ExpiresAt)) + log.Info( + "Stack successfully locked.", + "key", lock.Data.Key, + "lock_id", lock.Data.ID, + "expires_at", lock.Data.ExpiresAt, + )As per coding guidelines.
198-198: Log unlock outcome with fields instead of formatted string.Keeping output consistent with the structured style makes the trace level more useful downstream.
- log.Info(fmt.Sprintf("Key '%s' successfully unlocked.\n", dto.Key)) + log.Info("Key successfully unlocked.", "key", dto.Key)As per coding guidelines.
220-220: Promote the error into structured metadata.Handing the error to the logger via key/value lets the formatter render it cleanly and we can drop the manual formatting.
- log.Warn(fmt.Sprintf("Failed to get current git SHA: %v", err)) + log.Warn("Failed to get current git SHA.", "error", err)As per coding guidelines.
pkg/config/utils.go (3)
419-423: Wrap invalid logs.level errors with config context.Good validation. Add contextual wrapping using a static error from errors/errors.go (per guidelines), e.g., “invalid logs.level '' from config”.
As per coding guidelines
585-592: Wrap CLI logs.level errors with context.Same suggestion for flag-derived level: wrap with a static error and add context, e.g., “invalid --logs-level ''”.
As per coding guidelines
11-11: Consider demoting “found ENV variable” logs to Trace.These messages are very chatty; now that Trace exists, using log.Trace reduces Debug noise without losing detail.
internal/exec/vendor_utils.go (1)
23-27: Stderr logger init is fine; consider Trace for per-file decisions.Creating a dedicated stderr logger is reasonable. For high-volume file filtering logs (e.g., “Including path …”), consider Trace to avoid spamming Debug in tight loops.
cmd/list_metadata.go (1)
6-6: Prefer TUI for user messages, keep logs for system events.For “No metadata found”, use utils.PrintfMessageToTUI instead of log.Info to follow CLI UX guidance.
As per coding guidelines
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
Add a new trace log level (TRCE) that provides more detailed logging than debug level. The trace level is defined relative to debug level for maintainability. - Add TraceLevel constant and Trace/Tracef helper functions in pkg/logger/levels.go - Update setupLogger in cmd/root.go to recognize "Trace" log level - Add comprehensive test coverage (73.7%) for trace level functionality - Support trace level in CLI flags, environment variables, and config files 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
The Charm Bracelet log package doesn't know how to convert custom TraceLevel to a string representation, causing it to display as empty string. Using the original string value from the config instead of log.GetLevel() fixes this issue. Also update snapshot that had mismatched expected value.
The issue was that log.GetLevel() from Charmbracelet doesn't know how to handle our custom trace level and returns an empty string. We now track the actual log level string separately to display it correctly in debug messages.
- Use lipgloss SetString('TRAC') to properly label trace level messages
- Trace messages now show as 'TRAC' instead of 'DEBU' or being blank
- The 'Set logs-level=trace' message now uses trace level when trace is enabled
- Debug messages still show as 'DEBU' when trace is enabled (they are debug level)
- Updated golden snapshot to reflect the new TRAC prefix
Changed from TRAC to TRCE to maintain consistency with other 4-character level prefixes (DEBU, INFO, WARN, ERRO). This provides better visual alignment in log output.
Replace direct Charm Bracelet logger usage with an Atmos logger wrapper that provides: - Complete API compatibility with existing code (minimal changes required) - Centralized control over all logging operations - Integrated trace level support (no more mixed logger.Trace vs log.Debug patterns) - Consistent import pattern across entire codebase - Foundation for future logging enhancements Changes: - Created AtmosLogger struct that wraps Charm Bracelet logger - Implemented all standard log methods (Trace, Debug, Info, Warn, Error, Fatal) - Added global logger instance management - Provided package-level functions as drop-in replacement - Updated ~116 files to use new import path - Updated linting configuration for new logger - Removed old logger implementation files - Fixed static error in terraform_clean.go for linting compliance 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…an.go - Add errUtils import for centralized error handling - Replace local ErrSymbolicLink with errUtils.ErrRefuseDeleteSymbolicLink - Add path normalization for consistent cross-platform output - Add ErrRefuseDeleteSymbolicLink to centralized errors package
Updated test snapshots to match the new error message format that includes a colon after "invalid log level" for better readability. - Updated TestCLICommands_Invalid_Log_Level_in_Config_File snapshot - Updated TestCLICommands_Invalid_Log_Level_in_Environment_Variable snapshot 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add comprehensive tests for AtmosLogger interface methods - Add tests for all global logger functions (Trace, Debug, Info, Warn, Error) - Add tests for formatted logging functions (Tracef, Debugf, Infof, etc.) - Add tests for level filtering and output configuration - Add tests for WithPrefix and With context methods - Add tests for ParseLogLevel and ConvertLogLevel functions - Add LogLevelError constant for error level support - Update ParseLogLevel to support case-insensitive parsing - Add support for "warn" as alias for "warning" - Add benchmark tests to ensure performance - Enhance error_funcs tests with logger integration - Fix test compilation issues with correct termenv types Test coverage improved from 26.4% to 94.6%, exceeding the target of 80-90%. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Change AtmosLogger public methods to use logger.Level instead of charm.Level - Updated Log(), Logf(), SetLevel(), GetLevel() method signatures - Package now exposes only its own Level type alias in the public API - Internal charm.Level references converted to use package Level type - Add explicit yaml tags to CacheConfig struct fields - Ensures snake_case keys in YAML output (last_checked, installation_id, telemetry_disclosure_shown) - Maintains consistency with existing mapstructure tags - Makes YAML format explicit and prevents potential PascalCase output
- Use dedicated .lock file for Unix file locking to prevent lock loss during atomic rename - Apply requested file permissions on Windows (best-effort with os.Chmod) - Handle viper.ConfigFileNotFoundError gracefully in Unix read lock (return empty config) - Ensure consistent behavior between Unix and Windows for missing cache files These changes prevent race conditions during concurrent cache access and ensure proper file permissions are applied when creating cache files. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/cli_test.go (1)
182-205: Keep the init logger configuration consistent.Right now we call
log.New()twice. The second call overwrites the instance that we just configured, so the explicitSetLevel(log.InfoLevel)never survives and we end up running with the package default. Let’s initialize once and apply output, level, and styles on that single instance.- logger = log.New() - logger.SetOutput(os.Stdout) - logger.SetLevel(log.InfoLevel) + logger = log.New() + logger.SetOutput(os.Stderr) + logger.SetLevel(log.InfoLevel) … - logger = log.New() - logger.SetOutput(os.Stderr) + // keep using the same instance configured above
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
pkg/config/cache.go(1 hunks)pkg/config/cache_atomic_windows.go(1 hunks)pkg/config/cache_lock_unix.go(3 hunks)pkg/logger/atmos_logger.go(1 hunks)tests/cli_test.go(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- pkg/config/cache_lock_unix.go
- pkg/logger/atmos_logger.go
- pkg/config/cache.go
🧰 Additional context used
📓 Path-based instructions (7)
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Target >80% coverage for packages, focusing on pkg/ and internal/exec/.
Files:
pkg/config/cache_atomic_windows.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All comments must end with periods.
Wrap all returned errors with static errors from errors/errors.go; never return dynamic errors directly.
Use fmt.Errorf with %w to wrap the static error first, then add context details.
Always bind environment variables via viper.BindEnv, providing an ATMOS_ alternative for each external var.
All new configurations must support Go templating using the shared FuncMap(); test template rendering with various contexts.
Prefer SDKs over shelling out to binaries; use standard library for cross-platform behavior (filepath.Join, os.PathSeparator, runtime.GOOS).
For non-standard execution paths, capture telemetry via telemetry.CaptureCmd or telemetry.CaptureCmdString without user data.
Files:
pkg/config/cache_atomic_windows.gotests/cli_test.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
pkg/config/cache_atomic_windows.go
pkg/config/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Use Viper for configuration management (SetConfigName, AddConfigPath, AutomaticEnv, SetEnvPrefix).
Files:
pkg/config/cache_atomic_windows.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios
**/*_test.go: Use table-driven tests for unit tests where applicable.
Always use t.Skipf() with a reason; do not use t.Skip() or t.Skipf() without explanation.
TestMain must call os.Exit(m.Run()) to propagate test exit code for CLI tests.
Files:
tests/cli_test.go
tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place integration tests and shared test utilities under tests/ with fixtures in tests/test-cases/.
Files:
tests/cli_test.go
tests/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Use precondition-based skipping helpers from tests/test_preconditions.go for external dependencies (AWS, GitHub, network).
Files:
tests/cli_test.go
🧠 Learnings (5)
📓 Common learnings
Learnt from: mcalhoun
PR: cloudposse/atmos#980
File: internal/exec/template_funcs_gomplate_datasource.go:7-8
Timestamp: 2025-01-28T21:30:30.063Z
Learning: The logging wrappers in the codebase will be removed in a future PR to directly use charmbracelet logger, as part of the ongoing migration to standardize logging format.
Learnt from: mcalhoun
PR: cloudposse/atmos#980
File: pkg/utils/log_utils.go:62-63
Timestamp: 2025-01-30T16:56:43.004Z
Learning: In pkg/utils/log_utils.go, LogTrace is intentionally redirected to LogDebug since charmbracelet logger doesn't support Trace level, maintaining backward compatibility with the original LogTrace functionality.
Learnt from: samtholiya
PR: cloudposse/atmos#1077
File: pkg/downloader/custom_github_detector_test.go:0-0
Timestamp: 2025-02-21T20:56:20.761Z
Learning: The `github.com/charmbracelet/log` package should be imported with the alias `log`, not `clog`.
Learnt from: samtholiya
PR: cloudposse/atmos#1077
File: pkg/downloader/custom_github_detector.go:0-0
Timestamp: 2025-02-21T20:56:05.539Z
Learning: The `github.com/charmbracelet/log` package should be imported with the alias `log` according to the project's import alias configuration.
📚 Learning: 2024-10-28T01:51:30.811Z
Learnt from: osterman
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:329-332
Timestamp: 2024-10-28T01:51:30.811Z
Learning: In the Atmos Go code, when deleting directories or handling file paths (e.g., in `terraform_clean.go`), always resolve the absolute path using `filepath.Abs` and use the logger `u.LogWarning` for logging messages instead of using `fmt.Printf`.
Applied to files:
tests/cli_test.go
📚 Learning: 2025-02-21T20:56:05.539Z
Learnt from: samtholiya
PR: cloudposse/atmos#1077
File: pkg/downloader/custom_github_detector.go:0-0
Timestamp: 2025-02-21T20:56:05.539Z
Learning: The `github.com/charmbracelet/log` package should be imported with the alias `log` according to the project's import alias configuration.
Applied to files:
tests/cli_test.go
📚 Learning: 2025-02-21T20:56:20.761Z
Learnt from: samtholiya
PR: cloudposse/atmos#1077
File: pkg/downloader/custom_github_detector_test.go:0-0
Timestamp: 2025-02-21T20:56:20.761Z
Learning: The `github.com/charmbracelet/log` package should be imported with the alias `log`, not `clog`.
Applied to files:
tests/cli_test.go
📚 Learning: 2025-07-05T20:59:02.914Z
Learnt from: aknysh
PR: cloudposse/atmos#1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.
Applied to files:
tests/cli_test.go
🧬 Code graph analysis (1)
tests/cli_test.go (2)
pkg/logger/atmos_logger.go (1)
AtmosLogger(12-14)pkg/logger/log.go (3)
SetOutput(94-96)SetLevel(84-86)InfoLevel(183-183)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Build (macos-latest, macos)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Run pre-commit hooks
- GitHub Check: Summary
…t practices - Add comprehensive trace-level logging examples throughout both documents - Include detailed performance impact analysis and guidance - Add structured logging examples with trace-level field enrichment - Improve troubleshooting section with trace-level debugging scenarios - Add best practices for using trace logging in development vs production - Include examples of trace output for various Atmos operations - Add guidance on when to use each log level with clear decision criteria
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
docs/logging.md(1 hunks)docs/structured-logging.md(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: Listener430
PR: cloudposse/atmos#984
File: internal/exec/copy_glob.go:0-0
Timestamp: 2025-02-06T13:38:07.216Z
Learning: The `u.LogTrace` function in the `cloudposse/atmos` repository accepts `atmosConfig` as its first parameter, followed by the message string.
Learnt from: mcalhoun
PR: cloudposse/atmos#980
File: internal/exec/template_funcs_gomplate_datasource.go:7-8
Timestamp: 2025-01-28T21:30:30.063Z
Learning: The logging wrappers in the codebase will be removed in a future PR to directly use charmbracelet logger, as part of the ongoing migration to standardize logging format.
Learnt from: osterman
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:329-332
Timestamp: 2024-10-28T01:51:30.811Z
Learning: In the Atmos Go code, when deleting directories or handling file paths (e.g., in `terraform_clean.go`), always resolve the absolute path using `filepath.Abs` and use the logger `u.LogWarning` for logging messages instead of using `fmt.Printf`.
Learnt from: samtholiya
PR: cloudposse/atmos#1077
File: pkg/downloader/custom_github_detector.go:0-0
Timestamp: 2025-02-21T20:56:05.539Z
Learning: The `github.com/charmbracelet/log` package should be imported with the alias `log` according to the project's import alias configuration.
Learnt from: mcalhoun
PR: cloudposse/atmos#980
File: pkg/utils/log_utils.go:62-63
Timestamp: 2025-01-30T16:56:43.004Z
Learning: In pkg/utils/log_utils.go, LogTrace is intentionally redirected to LogDebug since charmbracelet logger doesn't support Trace level, maintaining backward compatibility with the original LogTrace functionality.
📚 Learning: 2025-09-26T05:52:03.918Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T05:52:03.918Z
Learning: Applies to cmd/**/*.go : Distinguish UI output (stderr) from structured logging; never use logging for UI elements.
Applied to files:
docs/logging.md
📚 Learning: 2025-09-26T05:52:03.918Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T05:52:03.918Z
Learning: Use consistent logging hierarchy (Fatal > Error > Warn > Debug > Trace) and structured logging without interpolation; production defaults: Error/Warn enabled, Debug/Trace disabled.
Applied to files:
docs/logging.md
📚 Learning: 2025-04-04T02:03:23.676Z
Learnt from: aknysh
PR: cloudposse/atmos#1185
File: internal/exec/yaml_func_store.go:26-26
Timestamp: 2025-04-04T02:03:23.676Z
Learning: The Atmos codebase currently uses `log.Fatal` for error handling in multiple places. The maintainers are aware this isn't an ideal pattern (should only be used in main() or init() functions) and plan to address it comprehensively in a separate PR. CodeRabbit should not flag these issues or push for immediate changes until that refactoring is complete.
Applied to files:
docs/structured-logging.md
🪛 markdownlint-cli2 (0.18.1)
docs/structured-logging.md
140-140: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Build (macos-latest, macos)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Run pre-commit hooks
- GitHub Check: Summary
🔇 Additional comments (1)
docs/logging.md (1)
3-369: Solid rewrite aligns with new logging architecture.The guidance is consistent with the Trace-capable logger and the TextUI separation we've been aiming for. Nice job reinforcing the hierarchy and performance caveats.
- Change code block language from 'text' to 'console' for terminal output - Make chmod failures fatal in Windows cache atomic write - Clean up temporary files on chmod errors - Return properly wrapped errors using project conventions 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
docs/structured-logging.md(1 hunks)pkg/config/cache_atomic_windows.go(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Target >80% coverage for packages, focusing on pkg/ and internal/exec/.
Files:
pkg/config/cache_atomic_windows.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All comments must end with periods.
Wrap all returned errors with static errors from errors/errors.go; never return dynamic errors directly.
Use fmt.Errorf with %w to wrap the static error first, then add context details.
Always bind environment variables via viper.BindEnv, providing an ATMOS_ alternative for each external var.
All new configurations must support Go templating using the shared FuncMap(); test template rendering with various contexts.
Prefer SDKs over shelling out to binaries; use standard library for cross-platform behavior (filepath.Join, os.PathSeparator, runtime.GOOS).
For non-standard execution paths, capture telemetry via telemetry.CaptureCmd or telemetry.CaptureCmdString without user data.
Files:
pkg/config/cache_atomic_windows.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
pkg/config/cache_atomic_windows.go
pkg/config/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Use Viper for configuration management (SetConfigName, AddConfigPath, AutomaticEnv, SetEnvPrefix).
Files:
pkg/config/cache_atomic_windows.go
🧠 Learnings (6)
📓 Common learnings
Learnt from: osterman
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:329-332
Timestamp: 2024-10-28T01:51:30.811Z
Learning: In the Atmos Go code, when deleting directories or handling file paths (e.g., in `terraform_clean.go`), always resolve the absolute path using `filepath.Abs` and use the logger `u.LogWarning` for logging messages instead of using `fmt.Printf`.
Learnt from: mcalhoun
PR: cloudposse/atmos#980
File: internal/exec/template_funcs_gomplate_datasource.go:7-8
Timestamp: 2025-01-28T21:30:30.063Z
Learning: The logging wrappers in the codebase will be removed in a future PR to directly use charmbracelet logger, as part of the ongoing migration to standardize logging format.
Learnt from: aknysh
PR: cloudposse/atmos#1185
File: internal/exec/yaml_func_store.go:26-26
Timestamp: 2025-04-04T02:03:23.676Z
Learning: The Atmos codebase currently uses `log.Fatal` for error handling in multiple places. The maintainers are aware this isn't an ideal pattern (should only be used in main() or init() functions) and plan to address it comprehensively in a separate PR. CodeRabbit should not flag these issues or push for immediate changes until that refactoring is complete.
📚 Learning: 2024-12-13T16:51:37.868Z
Learnt from: Listener430
PR: cloudposse/atmos#844
File: pkg/config/cache.go:69-97
Timestamp: 2024-12-13T16:51:37.868Z
Learning: In `pkg/config/cache.go`, the function `SaveCache2` is currently unused because it does not work properly on Windows and will be addressed later.
Applied to files:
pkg/config/cache_atomic_windows.go
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
PR: cloudposse/atmos#1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
pkg/config/cache_atomic_windows.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
PR: cloudposse/atmos#740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
pkg/config/cache_atomic_windows.go
📚 Learning: 2024-10-28T01:51:30.811Z
Learnt from: osterman
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:329-332
Timestamp: 2024-10-28T01:51:30.811Z
Learning: In the Atmos Go code, when deleting directories or handling file paths (e.g., in `terraform_clean.go`), always resolve the absolute path using `filepath.Abs` and use the logger `u.LogWarning` for logging messages instead of using `fmt.Printf`.
Applied to files:
pkg/config/cache_atomic_windows.go
📚 Learning: 2024-12-11T18:40:12.808Z
Learnt from: Listener430
PR: cloudposse/atmos#844
File: cmd/helmfile.go:37-37
Timestamp: 2024-12-11T18:40:12.808Z
Learning: In the atmos project, `cliConfig` is initialized within the `cmd` package in `root.go` and can be used in other command files.
Applied to files:
pkg/config/cache_atomic_windows.go
🧬 Code graph analysis (1)
pkg/config/cache_atomic_windows.go (1)
errors/errors.go (1)
ErrCacheWrite(140-140)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Build (macos-latest, macos)
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Run pre-commit hooks
- GitHub Check: Summary
🔇 Additional comments (1)
pkg/config/cache_atomic_windows.go (1)
49-54: Strict chmod handling LGTM.The fatal path upgrades the cache write contract without leaving stale temps—exactly what we needed here.
- Update all code examples to use logger.Trace/Debug/Info/Warn/Error - Replace log.GetLevel() with logger.IsLevelEnabled() - Add import statements for pkg/logger in all examples - Remove superfluous comment from cmd/root.go - Ensure documentation mirrors current codebase API 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
cmd/root.go(3 hunks)docs/structured-logging.md(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/root.go
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: mcalhoun
PR: cloudposse/atmos#980
File: internal/exec/template_funcs_gomplate_datasource.go:7-8
Timestamp: 2025-01-28T21:30:30.063Z
Learning: The logging wrappers in the codebase will be removed in a future PR to directly use charmbracelet logger, as part of the ongoing migration to standardize logging format.
Learnt from: osterman
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:329-332
Timestamp: 2024-10-28T01:51:30.811Z
Learning: In the Atmos Go code, when deleting directories or handling file paths (e.g., in `terraform_clean.go`), always resolve the absolute path using `filepath.Abs` and use the logger `u.LogWarning` for logging messages instead of using `fmt.Printf`.
Learnt from: aknysh
PR: cloudposse/atmos#1185
File: internal/exec/yaml_func_store.go:26-26
Timestamp: 2025-04-04T02:03:23.676Z
Learning: The Atmos codebase currently uses `log.Fatal` for error handling in multiple places. The maintainers are aware this isn't an ideal pattern (should only be used in main() or init() functions) and plan to address it comprehensively in a separate PR. CodeRabbit should not flag these issues or push for immediate changes until that refactoring is complete.
Learnt from: mcalhoun
PR: cloudposse/atmos#980
File: pkg/utils/log_utils.go:62-63
Timestamp: 2025-01-30T16:56:43.004Z
Learning: In pkg/utils/log_utils.go, LogTrace is intentionally redirected to LogDebug since charmbracelet logger doesn't support Trace level, maintaining backward compatibility with the original LogTrace functionality.
📚 Learning: 2025-09-26T05:52:03.918Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T05:52:03.918Z
Learning: Applies to **/*.go : Use fmt.Errorf with %w to wrap the static error first, then add context details.
Applied to files:
docs/structured-logging.md
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
PR: cloudposse/atmos#1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
docs/structured-logging.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Run pre-commit hooks
- GitHub Check: Summary
|
These changes were released in v1.192.0. |
|
These changes were released in v1.193.0-test.1. |
what
log.DebugLevel - 1for maintainability (not hardcoded)Trace()andTracef()helper functions for natural API usagewhy
Implementation Details
pkg/logger/package with custom logger implementationsync/atomicfor global instance managementlevels.gowith TraceLevel constant and helper functionssetupLoggerincmd/root.goto use the new logger package--logs-level=TraceATMOS_LOGS_LEVEL=Tracelogs.level: TraceFuture Possibilities
With this custom logger implementation, we can now add:
Testing
pkg/logger/with 94.6% coveragecmd/root_test.goreferences
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests