Added initial implementation of telemetry#1308
Conversation
|
💥 This pull request now has conflicts. Could you fix it @goruha? 🙏 |
|
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 |
There was a problem hiding this comment.
Actionable comments posted: 13
🔭 Outside diff range comments (5)
cmd/terraform.go (1)
92-99: No telemetry emitted for the plan-diff exit path
ErrPlanHasDiffis handled as a non-fatal flow-control error but neitherCaptureCmdnorCaptureCmdFailureis called here.
That leaves an important execution branch invisible in the metrics.if errors.Is(err, terrerrors.ErrPlanHasDiff) { // Print the error message but return the error to be handled by main.go u.PrintErrorMarkdown("", err, "") - return err + telemetry.CaptureCmdFailure(actualCmd) // or CaptureCmd, depending on semantics + return err }Tag the diff path explicitly so dashboards don’t quietly under-count runs with drift.
cmd/workflow.go (1)
24-34: Success event is lost in the zero-arg execution branch
Whenlen(args)==0andExecuteWorkflowCmdsucceeds, the functionreturns without callingtelemetry.CaptureCmd, so successful interactive-UI executions aren’t recorded.@@ - if len(args) == 0 { - err := e.ExecuteWorkflowCmd(cmd, args) - if err != nil { - telemetry.CaptureCmdFailure(cmd) - u.LogErrorAndExit(err) - } - return + if len(args) == 0 { + err := e.ExecuteWorkflowCmd(cmd, args) + if err != nil { + telemetry.CaptureCmdFailure(cmd) + u.LogErrorAndExit(err) + } + telemetry.CaptureCmd(cmd) + return }cmd/root.go (1)
72-84: Failure fromExecuteAtmosCmdis not capturedErrors bubbling up from the core execution path skip telemetry.
@@ err = e.ExecuteAtmosCmd() if err != nil { - u.LogErrorAndExit(err) + telemetry.CaptureCmdFailure(cmd) + u.LogErrorAndExit(err) }cmd/docs.go (2)
68-78: Missing failure telemetry before early exitWhen the component directory does not exist you exit without recording the failure event, breaking the success/failure ratio.
- if !componentPathExists { - u.PrintErrorMarkdownAndExit("", fmt.Errorf("Component `%s` not found in path: `%s`", info.Component, componentPath), "") + if !componentPathExists { + telemetry.CaptureCmdFailure(cmd) + u.PrintErrorMarkdownAndExit("", fmt.Errorf("Component `%s` not found in path: `%s`", info.Component, componentPath), "") }Replicate this pattern everywhere
PrintErrorMarkdownAndExitmight short-circuit.
107-110: Render error path skips telemetryIf
glamour.Renderfails we log and exit but never emit the failure metric.- if err != nil { - u.LogErrorAndExit(err) + if err != nil { + telemetry.CaptureCmdFailure(cmd) + u.LogErrorAndExit(err) }Small fix, big win for observability.
♻️ Duplicate comments (1)
cmd/helmfile.go (1)
36-41: Same duplication comment asvendor_diff.goThe success/failure capture block is duplicated here as well. Extracting a helper (see earlier suggestion) will keep helmfile commands in sync with the rest of the CLI while reducing churn.
🧹 Nitpick comments (29)
pkg/config/cache.go (1)
78-85:SaveCache2duplicates logic—consolidate
SaveCache2adds a lock but otherwise mirrorsSaveCache. Consider unifying into one implementation taking awithLockflag or always acquiring the lock to prevent drift.cmd/support.go (1)
24-27: Happy path only
telemetry.CaptureCmd(cmd)fires only when the markdown prints fine. IfPrintfMarkdownever errors (unlikely but possible), the failure path isn’t recorded. Minor, but mirroring the error-capture pattern in other commands keeps telemetry consistent.cmd/about.go (1)
21-24: Same minor telemetry gap assupportcommandConsider wrapping the markdown print + telemetry in a small helper with success/failure capture to avoid repetition across simple informational commands.
cmd/list_metadata.go (2)
38-40: Consider enriching failure telemetry with error metadataRight now only the command name is sent. Including at least the error type or a hash of the message would give you much more useful failure analytics without exposing sensitive data.
42-44: Capture success before user-facing output to avoid skewIf
utils.PrintMessagewrites a large payload or blocks (e.g. when piped), users may interrupt the process and the success event will never fire. Swapping the two calls makes the capture more resilient.- utils.PrintMessage(output) - telemetry.CaptureCmd(cmd) + telemetry.CaptureCmd(cmd) + utils.PrintMessage(output)cmd/pro_lock.go (1)
28-29: Mirror the print-before-capture comment from list_metadataSame ordering concern: record success first to minimise chances of the event being lost.
cmd/list_settings.go (2)
39-41: Include error classification in failure telemetryAs with
list_metadata, consider passing a lightweight, non-PII error identifier to aid debugging aggregated telemetry.
43-45: Capture success prior to printingSwap the two calls to reduce risk of missing the success event if the user aborts the output.
cmd/pro_unlock.go (1)
28-29: Success capture should precede any potentially blocking I/OFor consistency and robustness, call
telemetry.CaptureCmd(cmd)before any further stdout/stderr writes.cmd/vendor_diff.go (1)
24-30: Consider centralising the repeating “capture-then-exit” patternAlmost every command now follows the same structure:
if err != nil { telemetry.CaptureCmdFailure(cmd) u.PrintErrorMarkdownAndExit("", err, "") } telemetry.CaptureCmd(cmd)Duplicating this in ~40 command files is brittle – any future change to the logic (e.g. flushing/closing the telemetry client before exit) must be applied everywhere.
A small helper would remove that risk and shrink the call-sites:+func handleCmdResult(cmd *cobra.Command, err error) { + if err != nil { + telemetry.CaptureCmdFailure(cmd) + u.PrintErrorMarkdownAndExit("", err, "") + } + telemetry.CaptureCmd(cmd) +} ... -err := e.ExecuteVendorDiffCmd(cmd, args) -if err != nil { - telemetry.CaptureCmdFailure(cmd) - u.PrintErrorMarkdownAndExit("", err, "") -} -telemetry.CaptureCmd(cmd) +handleCmdResult(cmd, e.ExecuteVendorDiffCmd(cmd, args))That keeps behaviour identical while eliminating copy-paste overhead.
tests/test-cases/telemetry.yaml (1)
1-9: Test does not assert that telemetry is actually emittedThe case only checks
exit_code: 0. If the goal is to exercise the new telemetry path, consider:
- Setting
ATMOS_TELEMETRY_ENDPOINTto a local test server that records requests, or- Asserting that the CLI outputs the anonymised instance ID that telemetry injects into
atmos version(if applicable).Without such an assertion the test won’t fail even if telemetry is entirely disabled.
cmd/docs_generate.go (1)
20-25: Redundant manual arg-count check
cobra.ExactArgs(1)already guaranteeslen(args)==1; the extra check just adds dead code (and one more telemetry failure). You can safely drop it.- if len(args) != 1 { - telemetry.CaptureCmdFailure(cmd) - return ErrInvalidArguments - }cmd/list_workflows.go (1)
24-62: Verbose but repetitive error handling
Every flag retrieval wraps the same three lines (capture, print, return). Extracting a tiny helper (e.g.handleFlagErr(cmd, err, msg)) would reduce noise and future maintenance.No functional issues seen – telemetry hooks fire in the proper branches.
cmd/describe_dependents.go (2)
25-29: Same flushing concern & duplicated boilerplate
- The early-exit path mirrors the pattern above – add a flush before
os.Exit.- You’ve repeated
telemetry.CaptureCmdFailure(cmd)in four places in this file alone.
A small helper such ashandleErr(cmd, err, msg)could call telemetry, log, and print once, reducing duplication and future drift.Example:
func handleCmdErr(cmd *cobra.Command, userMsg string, err error) { telemetry.CaptureCmdFailure(cmd) telemetry.Flush() u.PrintErrorMarkdownAndExit(userMsg, err, "") }
41-42: Flush on init-time flag error, tooThe
MarkPersistentFlagRequiredfailure triggers an immediate exit.
Add the same telemetry flush to keep consistency across all failure paths.cmd/list_vendor.go (1)
30-83: High repetition – extract a reusable error-handling helperSix identical blocks:
telemetry.CaptureCmdFailure(cmd) log.Error("...", "error", err) cmd.PrintErrln(fmt.Errorf("...", err)) returnBesides the flushing issue, the copy-paste makes maintenance harder and risks inconsistency.
Suggested minimal helper:
func fail(cmd *cobra.Command, userMsg string, err error) { telemetry.CaptureCmdFailure(cmd) telemetry.Flush() log.Error(userMsg, "error", err) cmd.PrintErrln(fmt.Errorf("%s: %w", userMsg, err)) }Then each site becomes:
if err != nil { fail(cmd, "Error getting the 'format' flag", err) return }Cuts the block to one line, keeps telemetry consistent, and centralises the flush.
cmd/describe_workflows.go (1)
21-25: Add telemetry flush for reliabilitySame rationale: call a flush/close right after
CaptureCmdFailure/CaptureCmdto ensure PostHog events surviveos.Exit.-telemetry.CaptureCmdFailure(cmd) +telemetry.CaptureCmdFailure(cmd) +telemetry.Flush()and
-telemetry.CaptureCmd(cmd) +telemetry.CaptureCmd(cmd) +telemetry.Flush()tests/fixtures/scenarios/telemetry/atmos.yaml (1)
20-24: Config looks good – just ensure the endpoint is intentional
enabled: truewith the hard-coded PostHog SaaS endpoint means any test run will phone home.
If you ever need completely offline CI runs, consider overriding this via env‐var or a local endpoint.cmd/terraform.go (1)
58-67: Help/usage early-exit is also untrackedWhen the user asks for
--help, the function returnsnilwithout any telemetry.
Capturing help invocations can be valuable to understand discoverability issues.Up to you if that signal is worth collecting; just flagging the omission.
cmd/describe_affected.go (1)
38-41: Minor: error parsing CLI args is captured, but parse errors inside helpers are not
parseDescribeAffectedCliArgsperforms several validations that return errors; all bubbled up here are captured, but any panic insidecheckFlagErrorbypasses telemetry.
If those panics remain, consider converting them to errors so they’re observable.cmd/workflow.go (1)
40-44: Missing failure capture for invalid--fileusageWhen no
--fileflag is supplied, the command prints usage but does not emit a failure event. If you want telemetry to reflect user mistakes consistently, addCaptureCmdFailure(cmd)here as well.cmd/validate_editorconfig.go (1)
105-117: Variable shadowing obscures intent
format := outputformat.OutputFormat(...)redeclares the globalformatstring, then you shadow it again in theelse ifbranch. This is legal but confusing and easy to misuse.- format := outputformat.OutputFormat(atmosConfig.Validate.EditorConfig.Format) + selectedFormat := outputformat.OutputFormat(atmosConfig.Validate.EditorConfig.Format) @@ - format := outputformat.OutputFormat(format) + selectedFormat := outputformat.OutputFormat(format) @@ - cliConfig.Format = format + cliConfig.Format = selectedFormatRenaming avoids the double shadow and clarifies which value lands in
cliConfig.cmd/helmfile_generate_varfile.go (1)
18-28: Early help/usage exit not instrumented
handleHelpRequestmay callshowUsageAndExit, terminating without a failure metric. Consider wrapping that helper or emittingtelemetry.CaptureCmdFailure(cmd)just before it exits to keep stats accurate.pkg/telemetry/telemetry.go (2)
95-99: Avoid expensive stack processing just to read telemetry settings
InitCliConfig(..., true)processes stacks which can be heavy.
Telemetry only needs the settings – passprocessStacks=falsefor cheaper startup.- atmosConfig, err := cfg.InitCliConfig(schema.ConfigAndStacksInfo{}, true) + atmosConfig, err := cfg.InitCliConfig(schema.ConfigAndStacksInfo{}, false)
113-118: Endpoint and token fields are initialised but never used
Telemetry.endpointis never set andCaptureErrorusesposthog.New
(notNewWithConfig) which ignores the field.
Either populateendpointfrom config or drop the struct field to avoid dead code.cmd/cmd_utils.go (2)
188-196: High-frequency duplicate telemetry in loopsEvery alias execution spins a fresh PostHog client (three network handshakes: success/failure/success).
For batch workflows this can quickly flood PostHog and slow CLI startup.Consider:
- caching a singleton telemetry instance per process, or
- emitting a single event per high-level command rather than per internal alias step.
248-268: Two failure events for one user error
preCustomCommandemitsCaptureCmdFailurethenu.LogErrorAndExit; upstream defer/handlers may emit again, producing duplicates.
Ensure each failure path calls telemetry once.cmd/describe_component.go (1)
90-95:checkFlagNotPresentErrorpanics after recording failure – misleading stack tracesPanic prints a Go stack trace which end-users usually don’t need.
Preferu.LogErrorAndExitfor a clean exit after capturing telemetry.- telemetry.CaptureCmdFailure(cmd) - panic(err) + telemetry.CaptureCmdFailure(cmd) + u.LogErrorAndExit(err)cmd/validate_schema.go (1)
60-63: Minor: capture once and exit
CaptureCmdFailurealready logs the failure; the extralog.Errorduplicates output.
Drop it for cleaner stderr.- telemetry.CaptureCmdFailure(cmd) - log.Error("key not provided for the schema to be used") + telemetry.CaptureCmdFailure(cmd)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (50)
cmd/about.go(2 hunks)cmd/atlantis_generate_repo_config.go(2 hunks)cmd/aws_eks_update_kubeconfig.go(2 hunks)cmd/cmd_utils.go(13 hunks)cmd/completion.go(2 hunks)cmd/describe_affected.go(2 hunks)cmd/describe_component.go(4 hunks)cmd/describe_config.go(2 hunks)cmd/describe_dependents.go(3 hunks)cmd/describe_stacks.go(2 hunks)cmd/describe_workflows.go(2 hunks)cmd/docs.go(7 hunks)cmd/docs_generate.go(2 hunks)cmd/helmfile.go(2 hunks)cmd/helmfile_generate_varfile.go(3 hunks)cmd/list_components.go(2 hunks)cmd/list_metadata.go(2 hunks)cmd/list_settings.go(2 hunks)cmd/list_stacks.go(2 hunks)cmd/list_values.go(4 hunks)cmd/list_vendor.go(6 hunks)cmd/list_workflows.go(2 hunks)cmd/pro_lock.go(2 hunks)cmd/pro_unlock.go(2 hunks)cmd/root.go(3 hunks)cmd/support.go(2 hunks)cmd/terraform.go(2 hunks)cmd/terraform_commands.go(3 hunks)cmd/terraform_generate_backend.go(3 hunks)cmd/terraform_generate_backends.go(2 hunks)cmd/terraform_generate_planfile.go(3 hunks)cmd/terraform_generate_varfile.go(3 hunks)cmd/terraform_generate_varfiles.go(3 hunks)cmd/validate_component.go(3 hunks)cmd/validate_editorconfig.go(3 hunks)cmd/validate_schema.go(2 hunks)cmd/validate_stacks.go(2 hunks)cmd/vendor_diff.go(2 hunks)cmd/vendor_pull.go(2 hunks)cmd/version.go(2 hunks)cmd/workflow.go(4 hunks)go.mod(4 hunks)internal/exec/atmos.go(5 hunks)pkg/config/cache.go(1 hunks)pkg/schema/schema.go(1 hunks)pkg/telemetry/telemetry.go(1 hunks)pkg/telemetry/telemetry_test.go(1 hunks)tests/fixtures/scenarios/telemetry/atmos.yaml(1 hunks)tests/fixtures/scenarios/telemetry/stacks/deploy/nonprod.yaml(1 hunks)tests/test-cases/telemetry.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
cmd/version.go (1)
Learnt from: Listener430
PR: cloudposse/atmos#844
File: cmd/version.go:34-44
Timestamp: 2024-12-13T15:28:13.630Z
Learning: In `cmd/version.go`, when handling the `--check` flag in the `versionCmd`, avoid using `CheckForAtmosUpdateAndPrintMessage(cliConfig)` as it updates the cache timestamp, which may not be desired in this context.
cmd/describe_affected.go (1)
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.
cmd/list_vendor.go (1)
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.
cmd/describe_stacks.go (1)
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.
cmd/validate_schema.go (1)
Learnt from: samtholiya
PR: cloudposse/atmos#1147
File: internal/exec/validate_schema.go:42-57
Timestamp: 2025-04-11T22:06:46.999Z
Learning: The "ExecuteAtmosValidateSchemaCmd" function in internal/exec/validate_schema.go has been reviewed and confirmed to have acceptable cognitive complexity despite static analysis warnings. The function uses a clean structure with only three if statements for error handling and delegates complex operations to helper methods.
🧬 Code Graph Analysis (30)
cmd/about.go (1)
pkg/telemetry/telemetry.go (1)
CaptureCmd(132-134)
cmd/version.go (1)
pkg/telemetry/telemetry.go (1)
CaptureCmd(132-134)
cmd/support.go (1)
pkg/telemetry/telemetry.go (1)
CaptureCmd(132-134)
cmd/pro_lock.go (1)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)
cmd/completion.go (2)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)pkg/utils/log_utils.go (1)
LogErrorAndExit(48-58)
cmd/docs_generate.go (3)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)cmd/list_values.go (1)
ErrInvalidArguments(30-30)internal/exec/docs_generate.go (1)
ExecuteDocsGenerateCmd(45-84)
cmd/list_metadata.go (1)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)
cmd/list_stacks.go (4)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)pkg/utils/markdown_utils.go (1)
PrintErrorMarkdownAndExit(87-89)pkg/utils/log_utils.go (1)
PrintMessageInColor(31-33)pkg/ui/theme/colors.go (1)
Colors(72-84)
cmd/validate_stacks.go (4)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)pkg/utils/markdown_utils.go (1)
PrintErrorMarkdownAndExit(87-89)pkg/utils/log_utils.go (1)
PrintMessageInColor(31-33)pkg/ui/theme/colors.go (1)
Colors(72-84)
cmd/vendor_pull.go (2)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)pkg/utils/markdown_utils.go (1)
PrintErrorMarkdownAndExit(87-89)
cmd/describe_config.go (5)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)pkg/config/config.go (1)
InitCliConfig(25-62)pkg/schema/schema.go (2)
ConfigAndStacksInfo(439-510)Terminal(200-207)internal/exec/describe_config.go (1)
NewDescribeConfig(33-40)pkg/utils/markdown_utils.go (1)
PrintErrorMarkdown(28-50)
cmd/describe_workflows.go (2)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)pkg/utils/markdown_utils.go (1)
PrintErrorMarkdownAndExit(87-89)
cmd/list_settings.go (1)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)
cmd/terraform_generate_planfile.go (1)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)
cmd/terraform_generate_varfiles.go (1)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)
cmd/list_values.go (2)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)pkg/list/errors/types.go (2)
ComponentVarsNotFoundError(165-167)NoValuesFoundError(9-12)
cmd/terraform.go (1)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)
cmd/list_vendor.go (1)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)
pkg/telemetry/telemetry_test.go (1)
pkg/telemetry/telemetry.go (2)
NewTelemetry(28-34)DefaultTelemetryToken(17-17)
cmd/workflow.go (1)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)
cmd/cmd_utils.go (3)
internal/exec/shell_utils.go (1)
ExecuteShell(78-99)pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)internal/exec/template_utils.go (1)
ProcessTmpl(26-63)
cmd/terraform_generate_backend.go (1)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)
cmd/docs.go (2)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)pkg/utils/log_utils.go (1)
LogErrorAndExit(48-58)
cmd/validate_editorconfig.go (2)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)pkg/utils/log_utils.go (5)
LogErrorAndExit(48-58)LogDebug(80-82)LogTrace(74-76)LogInfo(86-88)PrintMessage(26-28)
cmd/terraform_commands.go (1)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)
cmd/validate_schema.go (3)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)pkg/utils/log_utils.go (2)
OsExit(23-23)LogErrorAndExit(48-58)internal/exec/validate_schema.go (2)
NewAtmosValidatorExecutor(32-40)ErrInvalidYAML(14-14)
cmd/describe_component.go (2)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)pkg/utils/markdown_utils.go (1)
PrintErrorMarkdownAndExit(87-89)
cmd/helmfile_generate_varfile.go (2)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(136-138)CaptureCmd(132-134)pkg/utils/log_utils.go (1)
LogErrorAndExit(48-58)
pkg/schema/schema.go (1)
pkg/telemetry/telemetry.go (1)
Telemetry(21-26)
pkg/telemetry/telemetry.go (4)
pkg/utils/log_utils.go (1)
LogWarning(92-94)pkg/config/config.go (1)
InitCliConfig(25-62)pkg/schema/schema.go (1)
ConfigAndStacksInfo(439-510)pkg/config/cache.go (2)
LoadCache(49-70)SaveCache(88-100)
🪛 golangci-lint (1.64.8)
pkg/telemetry/telemetry.go
[error] 60-60: Error return value of client.Enqueue is not checked
(errcheck)
[error] 85-85: Error return value of client.Enqueue is not checked
(errcheck)
[error] 94-94: File is not properly formatted
(gofumpt)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (26)
cmd/version.go (1)
18-23: Let’s inspect the body ofversionExec.Executeto see how it handles errors and the update check:#!/bin/bash set -e echo "→ Extracting versionExec.Execute implementation" awk '/func \(v versionExec\) Execute/,/^}/ {print}' internal/exec/version.gocmd/vendor_pull.go (1)
24-29: Looks solid – success & failure both capturedThe added telemetry hooks mirror the established pattern and are in the right order relative to the exit call. 👍
cmd/pro_lock.go (1)
25-27: Verify telemetry is flushed before forced exit
u.PrintErrorMarkdownAndExitlikely callsos.Exit, which terminates the program immediately.
Iftelemetry.CaptureCmdFailureperforms network I/O asynchronously, the process may exit before the batch is delivered.Please confirm the telemetry package blocks or flushes on process shutdown, or call an explicit
telemetry.Flush()right before exit.cmd/pro_unlock.go (1)
25-27: Ensure telemetry send completes before process exit
u.PrintErrorMarkdownAndExitprobably triggers an immediateos.Exit, which can kill any in-flight telemetry request.
Confirm that the telemetry client flushes synchronously or add an explicit flush.tests/fixtures/scenarios/telemetry/stacks/deploy/nonprod.yaml (1)
1-13: YAML looks good – schema reference and test data are clearNothing to add here.
cmd/terraform_generate_backends.go (1)
22-28: Potential double-counting of success/failure events
ExecuteTerraformGenerateBackendsCmdeventually delegates intointernal/exec/atmos.go, which (per the PR) already callstelemetry.CaptureCmd*.
Capturing again here could inflate metrics and distort usage statistics.Please verify whether the lower-level execution path already fires telemetry and, if so, decide which layer should own the event.
#!/bin/bash # Quick scan: list functions that call CaptureCmd* inside the exec package rg --no-heading -n 'CaptureCmd' internal/exec | headcmd/describe_config.go (1)
23-34: Telemetry hooks for flag errors look good
The failure events are captured immediately before returning the error – this is exactly the right spot.cmd/atlantis_generate_repo_config.go (1)
24-30: Telemetry placement is consistent
CaptureCmdFailureprecedes the early-exit path andCaptureCmdis reached only on success becausePrintErrorMarkdownAndExitterminates the process. Looks solid.cmd/docs_generate.go (1)
26-31: Telemetry wiring otherwise correct
Failure and success events are emitted in the right branches.cmd/list_components.go (1)
28-36: Good failure/success segregation
Failure is captured before the fatal exit and success only on the happy path – perfect.cmd/validate_stacks.go (1)
26-32: [running additional checks]To see how telemetry is wired up and whether any flush/close is exposed, let’s dump the key sections of
telemetry.go.#!/bin/bash set -e echo "=== Package header & imports ===" sed -n '1,50p' pkg/telemetry/telemetry.go echo -e "\n=== Client initialization & config ===" sed -n '50,110p' pkg/telemetry/telemetry.go echo -e "\n=== CaptureCmd & CaptureCmdFailure implementations ===" sed -n '110,180p' pkg/telemetry/telemetry.go echo -e "\n=== PostHog references ===" rg -n "posthog" pkg/telemetry/telemetry.go || echo "— none found —" echo -e "\n=== Any goroutine usage ===" rg -n "go " pkg/telemetry/telemetry.go || echo "— none found —"cmd/aws_eks_update_kubeconfig.go (1)
36-40: Telemetry hook looks goodFailure and success paths are instrumented without altering control flow.
No further action needed.cmd/terraform_generate_varfile.go (1)
24-29: Looks fine – telemetry wired correctlyThe command records failure before exiting and success on normal completion.
cmd/terraform_generate_planfile.go (2)
24-29: Good ‑ success/failure both capturedThe capture calls are correctly placed before the terminal
Exitand after a successful run.
39-43: Flag-initialisation failure captured – solidNice touch catching telemetry even in
initerrors. 👍cmd/terraform_generate_backend.go (2)
23-29: Mirrors the planfile pattern – looks consistentImplementation aligns with other generators; no concerns.
33-39: Same remark as above – well handledcmd/describe_affected.go (2)
41-44: Success telemetry correctly emittedAfter exec completes without error, success is captured – looks good.
47-52: Let’s double-check with a simpler search pattern:#!/bin/bash # Search for any function definitions named checkErrorAndExit in cmd/ rg -n '^func checkErrorAndExit' cmdPlease share the results so we can confirm there are no duplicates.
cmd/validate_component.go (1)
26-35: Telemetry hooks look correct 👍
CaptureCmdFailureis invoked on the error path andCaptureCmdon the success path. Placement before the exit helpers guarantees the event is flushed.
No further action needed.cmd/list_values.go (1)
57-74: Good placement of success/failure hooksAll other branches correctly emit success or failure before returning. Nice job.
cmd/terraform_generate_varfiles.go (1)
22-28: Telemetry integration looks solidFailure and success are captured in the right spots. No issues spotted.
cmd/root.go (1)
55-66: Nice: early-config validation failures are now trackedThe added hooks inside
PersistentPreRunwill give visibility into configuration issues. Well done.go.mod (1)
221-223: Let’s search the codebase for any imports of the v1 LRU module:#!/bin/bash rg -n "github.com/hashicorp/golang-lru" .cmd/describe_stacks.go (1)
63-64: Success metric placement looks goodTelemetry is emitted once all validation and execution steps succeed. Nice and clean.
internal/exec/atmos.go (1)
40-42: Potential nil-pointer fromInitializeTelemetry
telemetry.CaptureCmdFailureString("atmos")can panic if telemetry initialisation fails (see previous comment).
After fixingCapture…to guard againstnil, this call path will be safe.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
pkg/telemetry/telemetry.go (4)
45-47: Uselog.Warninstead of deprecatedu.LogWarning
u.LogWarningis marked deprecated (SA1019); switch tolog.Warnto avoid future breakage.Also applies to: 102-104, 109-111
94-97: Avoid returningnilfromInitializeTelemetryOn config or cache load failures this returns
nil, but callers assume a valid instance and panic. Either return a no-opTelemetrywhen disabled/error or enforcenilchecks in constructors.Also applies to: 100-104
85-89: Check error return fromclient.EnqueueinCaptureErrorSame gap in
CaptureError: enqueue errors are ignored. Add handling to log failures and satisfyerrcheck:- client.Enqueue(posthog.Capture{ + if err := client.Enqueue(posthog.Capture{ DistinctId: t.distinctId, Event: eventName, Properties: propertiesMap, - }) + }); err != nil { + log.Warn(fmt.Sprintf("failed to enqueue telemetry error %q: %v", eventName, err)) + }
60-64: Check error return fromclient.EnqueueIgnoring the error from
client.Enqueuesilently drops events and triggerserrcheck. Handle it explicitly:- client.Enqueue(posthog.Capture{ + if err := client.Enqueue(posthog.Capture{ DistinctId: t.distinctId, Event: eventName, Properties: propertiesMap, - }) + }); err != nil { + log.Warn(fmt.Sprintf("failed to enqueue telemetry event %q: %v", eventName, err)) + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
cmd/describe_affected.go(2 hunks)cmd/describe_stacks.go(2 hunks)cmd/describe_workflows.go(2 hunks)pkg/telemetry/telemetry.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- cmd/describe_affected.go
- cmd/describe_workflows.go
- cmd/describe_stacks.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
pkg/telemetry/telemetry.go (4)
pkg/utils/log_utils.go (1)
LogWarning(92-94)pkg/config/config.go (1)
InitCliConfig(25-62)pkg/schema/schema.go (3)
ConfigAndStacksInfo(439-510)Settings(786-790)Command(595-605)pkg/config/cache.go (2)
LoadCache(49-70)SaveCache(88-100)
🪛 Gitleaks (8.26.0)
pkg/telemetry/telemetry.go
17-17: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 GitHub Check: golangci-lint
pkg/telemetry/telemetry.go
[failure] 45-45:
SA1019: u.LogWarning is deprecated: Use log.Warn instead. This function will be removed in a future release. LogWarning logs the provided warning message
[failure] 60-60:
Error return value of client.Enqueue is not checked
[failure] 85-85:
Error return value of client.Enqueue is not checked
[failure] 102-102:
SA1019: u.LogWarning is deprecated: Use log.Warn instead. This function will be removed in a future release. LogWarning logs the provided warning message
[failure] 110-110:
SA1019: u.LogWarning is deprecated: Use log.Warn instead. This function will be removed in a future release. LogWarning logs the provided warning message
🪛 golangci-lint (1.64.8)
pkg/telemetry/telemetry.go
[error] 60-60: Error return value of client.Enqueue is not checked
(errcheck)
[error] 85-85: Error return value of client.Enqueue is not checked
(errcheck)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pkg/telemetry/telemetry.go (1)
16-20: Remove hard-coded PostHog tokenEmbedding
DefaultTelemetryTokenin source risks accidental key leaks and still triggers secret-scanning. Load the token exclusively from config/env instead.
🧹 Nitpick comments (3)
pkg/telemetry/telemetry.go (3)
7-7: Satisfy import-alias linter
golangci-lint importasexpects an explicit alias:-import "github.com/charmbracelet/log" +import log "github.com/charmbracelet/log"Keeps
log.Warncalls unchanged.
38-46: Avoid per-event client construction
posthog.NewWithConfigis relatively heavy and TLS-handshake cost is non-trivial. Build the client once (e.g., inInitializeTelemetry) and reuse it to cut latency & memory.
62-70: Escalate enqueue failures to warningDropping telemetry silently hampers debugging. Consider
log.Warninstead oflog.Debug(or expose a retry) so operators notice connectivity issues.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cmd/validate_editorconfig.go(3 hunks)cmd/validate_schema.go(2 hunks)internal/exec/atmos.go(5 hunks)pkg/schema/schema.go(1 hunks)pkg/telemetry/telemetry.go(1 hunks)pkg/telemetry/telemetry_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- cmd/validate_schema.go
- internal/exec/atmos.go
- cmd/validate_editorconfig.go
- pkg/telemetry/telemetry_test.go
- pkg/schema/schema.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
pkg/telemetry/telemetry.go (3)
pkg/config/config.go (1)
InitCliConfig(25-62)pkg/schema/schema.go (3)
ConfigAndStacksInfo(439-510)Settings(786-790)Command(595-605)pkg/config/cache.go (2)
LoadCache(49-70)SaveCache(88-100)
🪛 golangci-lint (1.64.8)
pkg/telemetry/telemetry.go
[error] 7-7: import "github.com/charmbracelet/log" imported without alias but must be with alias "log" according to config
(importas)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Build (ubuntu-latest, linux)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Summary
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
pkg/telemetry/telemetry.go (2)
106-139:InitializeTelemetryreloads config & cache on every event – cache the instance withsync.OnceEvery call does disk IO and YAML parsing, which will be executed for every
CaptureCmd*invocation. Wrap construction insync.Once, store the*Telemetryin a package-level var, and just return it after the first call.+var ( + telemetryOnce sync.Once + telemetryInst *Telemetry +) func InitializeTelemetry() *Telemetry { - atmosConfig, err := cfg.InitCliConfig(schema.ConfigAndStacksInfo{}, true) - ... - return NewTelemetry(atmosConfig.Settings.Telemetry.Enabled, token, endpoint, cacheCfg.AtmosInstanceId) + telemetryOnce.Do(func() { + atmosConfig, err := cfg.InitCliConfig(schema.ConfigAndStacksInfo{}, true) + if err != nil { + return + } + ... + telemetryInst = NewTelemetry(atmosConfig.Settings.Telemetry.Enabled, token, endpoint, cacheCfg.AtmosInstanceId) + }) + return telemetryInst }This keeps behaviour unchanged while avoiding repeated work.
16-20: Remove the hard-coded PostHog token – it trips Gitleaks and is a latent secret-exposure riskEmbedding even a “public” key in the binary will be flagged by scanners (see static-analysis hint) and makes future rotation impossible. Prefer loading the token from config / env only and leave the default empty.
-const ( - DefaultTelemetryToken = "phc_7s7MrHWxPR2if1DHHDrKBRgx7SvlaoSM59fIiQueexS" - DefaultTelemetryEndpoint = "https://us.i.posthog.com" - DefaultEventName = "command" -) +const ( + // Intentionally blank – the real token must come from config or $ATMOS_TELEMETRY_TOKEN. + DefaultTelemetryToken = "" + DefaultTelemetryEndpoint = "https://us.i.posthog.com" + DefaultEventName = "command" +)Add
os.LookupEnv("ATMOS_TELEMETRY_TOKEN")as another source insideInitializeTelemetry.
🧹 Nitpick comments (3)
pkg/telemetry/telemetry.go (3)
43-51: Creating a new PostHog client for every event is expensive
posthog-goholds network connections and internal buffers; constructing & closing it per event defeats batching. Either:
- Promote the client into the
Telemetrystruct and reuse it (remember to callClose()in aShutdown()func or fromsync.Once’sDone()), or- Use
posthog.WithBatchSize(1)if you really need fire-and-forget per event.Re-using a single client will cut TLS handshakes and GC pressure.
52-70:CaptureEventandCaptureErrorduplicate ~90 % of their codeFactor out the shared logic into an unexported helper:
func (t *Telemetry) capture(eventName string, isError bool, props map[string]interface{}) { ... propertiesMap := posthog.NewProperties(). Set("error", isError). ... }
CaptureEventandCaptureErrorthen callcapture(..., false, ...)andcapture(..., true, ...). Shorter, less risk of one path drifting.Also applies to: 86-103
68-69: Log enqueue failures atWarn, notDebugDropping telemetry silently makes troubleshooting harder. A warning level is more appropriate:
- log.Debug(fmt.Sprintf("Could not enqueue event: %s", err)) + log.Warn("Could not enqueue telemetry", "event", eventName, "error", err)Minor, yet improves visibility without being noisy.
Also applies to: 100-103
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/telemetry/telemetry.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
pkg/telemetry/telemetry.go (2)
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: 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`.
🪛 Gitleaks (8.26.0)
pkg/telemetry/telemetry.go
17-17: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Build (ubuntu-latest, linux)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Summary
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
cmd/validate_editorconfig.go (1)
108-119: Rename the localformatvariable to avoid shadowing the package-level flagUsing the same identifier twice in this block hides the global
formatflag and makes the logic harder to follow. A distinct name clarifies intent and eliminates accidental misuse.- format := outputformat.OutputFormat(atmosConfig.Validate.EditorConfig.Format) - if ok := format.IsValid(); !ok { + parsedFormat := outputformat.OutputFormat(atmosConfig.Validate.EditorConfig.Format) + if ok := parsedFormat.IsValid(); !ok { u.LogErrorAndExit(fmt.Errorf("%v is not a valid format choose from the following: %v", atmosConfig.Validate.EditorConfig.Format, outputformat.GetArgumentChoiceText())) } - cliConfig.Format = format + cliConfig.Format = parsedFormat ... - format := outputformat.OutputFormat(format) - if ok := format.IsValid(); !ok { + parsedFormat := outputformat.OutputFormat(format) + if ok := parsedFormat.IsValid(); !ok { u.LogErrorAndExit(fmt.Errorf("%v is not a valid format choose from the following: %v", atmosConfig.Validate.EditorConfig.Format, outputformat.GetArgumentChoiceText())) } - cliConfig.Format = format + cliConfig.Format = parsedFormat
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cmd/validate_editorconfig.go(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
cmd/validate_editorconfig.go (2)
Learnt from: aknysh
PR: cloudposse/atmos#820
File: cmd/list_stacks.go:55-56
Timestamp: 2024-12-05T22:33:54.807Z
Learning: In the atmos project, the `u.LogErrorAndExit` function logs the error and exits the command execution appropriately within flag completion functions.
Learnt from: aknysh
PR: cloudposse/atmos#820
File: cmd/list_components.go:53-54
Timestamp: 2024-12-05T22:33:40.955Z
Learning: In the Atmos CLI Go codebase, using `u.LogErrorAndExit` within completion functions is acceptable because it logs the error and exits the command execution.
🧬 Code Graph Analysis (1)
cmd/validate_editorconfig.go (2)
pkg/telemetry/telemetry.go (2)
CaptureCmdFailure(157-159)CaptureCmd(153-155)pkg/utils/log_utils.go (4)
LogDebug(80-82)LogTrace(74-76)LogInfo(86-88)PrintMessage(26-28)
🪛 GitHub Check: golangci-lint
cmd/validate_editorconfig.go
[failure] 51-51:
deep-exit: calls to log.Fatal only in main() or init() functions
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pkg/telemetry/utils.go (1)
47-47: Use consistent logging key.For consistency with the rest of the codebase, use
"err"instead of"error"as the key.Apply this diff:
- log.Warn("Could not load config", "error", err) + log.Warn("Could not load config", "err", err)- log.Warn("Could not load config", "error", err) + log.Warn("Could not load config", "err", err)- log.Warn("Could not save cache", "error", err) + log.Warn("Could not save cache", "err", err)Also applies to: 79-79, 145-145
🧹 Nitpick comments (1)
pkg/telemetry/utils.go (1)
129-129: Fix typo in comment.-// the updated cache. If cache operations fail, it logs a disclosure but continues execution. +// the updated cache. If cache operations fail, it logs a warning but continues execution.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pkg/telemetry/utils.go(1 hunks)pkg/telemetry/utils_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/telemetry/utils_test.go
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.go`: Use Viper for managing configuration, environment variables, and flag...
**/*.go: Use Viper for managing configuration, environment variables, and flags
Use interfaces for external dependencies to facilitate mocking
Follow Go's error handling idioms
Use meaningful error messages
Wrap errors with context usingfmt.Errorf("context: %w", err)
Consider using a custom error type for domain-specific errors
Follow standard Go coding style
Usegofmtandgoimportsto format code
Prefer short, descriptive variable names
Use snake_case for environment variables
Document all exported functions, types, and methods
Document complex logic with inline comments
Follow Go's documentation conventions
Use Viper for configuration management
Support configuration via files, environment variables, and flags
Follow the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users
Log detailed errors for debugging
📄 Source: CodeRabbit Inference Engine (.cursor/rules/atmos-rules.mdc)
List of files the instruction was applied to:
pkg/telemetry/utils.go
🧠 Learnings (2)
📓 Common learnings
Learnt from: Listener430
PR: cloudposse/atmos#934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Clearly describe the changes in the PR description
Learnt from: aknysh
PR: cloudposse/atmos#944
File: go.mod:206-206
Timestamp: 2025-01-17T00:18:57.769Z
Learning: For indirect dependencies with license compliance issues in the cloudposse/atmos repository, the team prefers to handle them in follow-up PRs rather than blocking the current changes, as these issues often require deeper investigation of the dependency tree.
pkg/telemetry/utils.go (34)
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.
Learnt from: samtholiya
PR: cloudposse/atmos#959
File: cmd/workflow.go:74-74
Timestamp: 2025-01-30T19:30:59.120Z
Learning: Error handling for `cmd.Usage()` is not required in the Atmos CLI codebase, as confirmed by the maintainer.
Learnt from: Listener430
PR: cloudposse/atmos#844
File: cmd/version.go:34-44
Timestamp: 2024-12-13T15:28:13.630Z
Learning: In `cmd/version.go`, when handling the `--check` flag in the `versionCmd`, avoid using `CheckForAtmosUpdateAndPrintMessage(cliConfig)` as it updates the cache timestamp, which may not be desired in this context.
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.
Learnt from: Listener430
PR: cloudposse/atmos#825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to cmd/*.go : Include progress indicators for long-running operations
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to cmd/*.go : Implement each Cobra command in a separate file under the `cmd/` directory
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.
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to cmd/*.go : Include troubleshooting hints when appropriate
Learnt from: aknysh
PR: cloudposse/atmos#820
File: cmd/list_components.go:53-54
Timestamp: 2024-12-05T22:33:40.955Z
Learning: In the Atmos CLI Go codebase, using `u.LogErrorAndExit` within completion functions is acceptable because it logs the error and exits the command execution.
Learnt from: samtholiya
PR: cloudposse/atmos#896
File: cmd/editor_config.go:37-40
Timestamp: 2025-01-07T20:38:09.618Z
Learning: Error handling suggestion for `cmd.Help()` in `cmd/editor_config.go` was deferred as the code is planned for future modifications.
Learnt from: osterman
PR: cloudposse/atmos#768
File: internal/exec/vendor_utils.go:0-0
Timestamp: 2024-11-10T19:28:17.365Z
Learning: When TTY is not supported, log the downgrade message at the Warn level using `u.LogWarning(cliConfig, ...)` instead of `fmt.Println`.
Learnt from: samtholiya
PR: cloudposse/atmos#959
File: cmd/describe_config.go:20-20
Timestamp: 2025-02-03T06:00:11.419Z
Learning: Commands should use `PrintErrorMarkdownAndExit` with empty title and suggestion (`"", err, ""`) for general error handling. Specific titles like "Invalid Usage" or "File Not Found" should only be used for validation or specific error scenarios.
Learnt from: samtholiya
PR: cloudposse/atmos#959
File: cmd/describe_config.go:20-20
Timestamp: 2025-02-03T06:00:11.419Z
Learning: The `describe config` command should use `PrintErrorMarkdownAndExit` with empty title and suggestion for consistency with other commands.
Learnt from: samtholiya
PR: cloudposse/atmos#1147
File: pkg/config/load.go:0-0
Timestamp: 2025-04-10T20:48:22.687Z
Learning: In the `bindEnv` function in `pkg/config/load.go`, panic is used deliberately instead of returning errors because errors from `BindEnv` would only occur due to developer mistakes. Using panic helps with early detection of these developer errors during initialization.
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*.go : Provide clear error messages to users
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*.go : Log detailed errors for debugging
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*.go : Use meaningful error messages
Learnt from: osterman
PR: cloudposse/atmos#808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
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.
Learnt from: Listener430
PR: cloudposse/atmos#844
File: pkg/config/cache.go:42-42
Timestamp: 2024-12-13T16:48:00.294Z
Learning: The function `withCacheFileLock` in `pkg/config/cache.go` is currently unused and left for future upgrade.
Learnt from: osterman
PR: cloudposse/atmos#768
File: internal/exec/vendor_utils.go:496-513
Timestamp: 2024-11-22T12:38:33.132Z
Learning: In the Atmos project, continue to flag path traversal issues in code reviews but acknowledge when they are expected and acceptable in specific cases.
Learnt from: samtholiya
PR: cloudposse/atmos#1068
File: tests/snapshots/TestCLICommands_atmos_terraform_help.stdout.golden:59-64
Timestamp: 2025-02-18T13:13:11.497Z
Learning: For Atmos CLI help text, angle brackets in command examples and flag descriptions should be escaped using HTML entities (e.g., `<component>`) rather than converted to backticks or other markdown formatting.
Learnt from: samtholiya
PR: cloudposse/atmos#955
File: tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden:0-0
Timestamp: 2025-01-19T15:49:15.593Z
Learning: In future commits, the help text for Atmos CLI commands should be limited to only show component and stack parameters for commands that actually use them. This applies to the example usage section in command help text.
Learnt from: Cerebrovinny
PR: cloudposse/atmos#729
File: internal/exec/terraform.go:258-264
Timestamp: 2024-10-19T17:42:29.152Z
Learning: Prefer not to declare variables within 'if' statements in Go code.
Learnt from: samtholiya
PR: cloudposse/atmos#1147
File: internal/exec/validate_schema.go:42-57
Timestamp: 2025-04-11T22:06:46.999Z
Learning: The "ExecuteAtmosValidateSchemaCmd" function in internal/exec/validate_schema.go has been reviewed and confirmed to have acceptable cognitive complexity despite static analysis warnings. The function uses a clean structure with only three if statements for error handling and delegates complex operations to helper methods.
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: All code must pass golangci-lint checks
Learnt from: goruha
PR: cloudposse/atmos#1308
File: pkg/telemetry/telemetry.go:0-0
Timestamp: 2025-06-19T00:58:36.557Z
Learning: For CLI telemetry implementations, avoid singleton patterns when each telemetry call needs connection management (create/close cycles). CLI tools with multiple exit points make connection cleanup coordination complex, and short-lived command executions don't benefit from connection caching. The per-call connection pattern with defer cleanup is more appropriate.
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Test both happy paths and error conditions
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: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to cmd/*.go : Provide meaningful feedback to users in commands
Learnt from: haitham911
PR: cloudposse/atmos#727
File: internal/exec/path_utils.go:145-146
Timestamp: 2024-10-23T22:11:41.077Z
Learning: In the `atmos` project, the preference is to print relative paths in log messages instead of full paths.
Learnt from: osterman
PR: cloudposse/atmos#729
File: internal/exec/help.go:48-51
Timestamp: 2024-10-27T16:59:26.187Z
Learning: In the Atmos CLI help messages, when providing examples that include the version number, use the actual version variable (e.g., `version.Version`) instead of placeholders like `<version>`.
Learnt from: Listener430
PR: cloudposse/atmos#1149
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:7-7
Timestamp: 2025-03-18T12:26:25.329Z
Learning: In the Atmos project, typos or inconsistencies in test snapshot files (such as "terrafrom" instead of "terraform") may be intentional as they capture the exact output of commands and should not be flagged as issues requiring correction.
🧬 Code Graph Analysis (1)
pkg/telemetry/utils.go (4)
pkg/telemetry/telemetry.go (4)
TelemetryClientProvider(14-14)Telemetry(27-33)PosthogClientProvider(20-22)NewTelemetry(38-46)pkg/config/config.go (1)
InitCliConfig(25-62)pkg/schema/schema.go (2)
ConfigAndStacksInfo(440-514)Settings(791-795)pkg/config/cache.go (3)
CacheConfig(17-20)LoadCache(48-69)SaveCache(88-101)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Build (ubuntu-latest, linux)
- GitHub Check: Build (macos-latest, macos)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Summary
🔇 Additional comments (1)
pkg/telemetry/utils.go (1)
115-150: Well-designed generic cache helper.The generic implementation with getter/setter functions is clean and reusable. Good error handling that logs warnings without interrupting execution.
* Consolidate cmd telemetry * [autofix.ci] apply automated fixes * Update root.go * Capture commands * Telemetry - `atmos describe` cmd (#1342) * Added telemetry to describe commands * Update TestCLICommands_atmos_describe_config_imports.stderr.golden * Update TestCLICommands_atmos_describe_configuration.stderr.golden * Fix tests * Update describe_component.go * Update describe_config.go * Update describe_stacks.go * Update describe_workflows.go * Fix tests * Fix tests --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
* Added telemetry site page * Update website/docs/cli/telemetry.mdx Co-authored-by: Erik Osterman (CEO @ Cloud Posse) <erik@cloudposse.com> * Update website/docs/cli/telemetry.mdx Co-authored-by: Erik Osterman (CEO @ Cloud Posse) <erik@cloudposse.com> * Update website/docs/cli/telemetry.mdx Co-authored-by: Erik Osterman (CEO @ Cloud Posse) <erik@cloudposse.com> * Update website/docs/cli/telemetry.mdx Co-authored-by: Erik Osterman (CEO @ Cloud Posse) <erik@cloudposse.com> * Apply suggestions from code review Co-authored-by: Erik Osterman (CEO @ Cloud Posse) <erik@cloudposse.com> --------- Co-authored-by: Erik Osterman (CEO @ Cloud Posse) <erik@cloudposse.com>
* Added disclosure messages * Added disclosure messages * Cherry pick disclosure message * Fix tests * Fix tests * Fix tests * Improve disclosure messages * Fix tests * Fix tests * [autofix.ci] apply automated fixes --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
tests/cli_test.go (1)
722-726: Fix error handling to avoid masking configuration issues.The function currently ignores errors from
config.GetCacheFilePath()but should propagate them since the caller usesassert.NoErrorexpecting proper error reporting.Apply this fix to properly handle configuration errors:
func removeCacheFile() error { cacheFilePath, err := config.GetCacheFilePath() if err != nil { - return nil + return err }pkg/telemetry/utils_test.go (2)
22-23: Fix comment formatting.The comment should end with a period according to the coding standards.
60-82: Address code duplication.The test logic here is very similar to
TestCaptureCmdErrorString(lines 109-148). Consider refactoring into a table-driven test to reduce duplication and improve maintainability.Example approach:
func TestCaptureCmdString(t *testing.T) { testCases := []struct { name string command string error error expected bool // expected error flag }{ {"success", "test-cmd", nil, false}, {"error", "test-cmd", errors.New("test-error"), true}, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { // common test setup... // expect error flag to be tc.expected // run captureCmdString(tc.command, tc.error, mockProvider) }) } }
🧹 Nitpick comments (5)
website/docs/cli/telemetry.mdx (3)
60-60: Minor grammatical improvement needed.Consider adding "the" before "environment variable" for better readability.
-or set environment variable `ATMOS_TELEMETRY_ENABLED` to `false` +or set the environment variable `ATMOS_TELEMETRY_ENABLED` to `false`
64-64: Minor typo correction needed.There's a small typo in "telemery" that should be "telemetry".
-You can switch telemery to your own [PostHog](https://posthog.com/) account: +You can switch telemetry to your own [PostHog](https://posthog.com/) account:
78-78: Consider simplifying redundant phrasing.The phrase "to your own values" could be simplified since ownership is already implied.
-or set environment variables `ATMOS_TELEMETRY_TOKEN` and `ATMOS_TELEMETRY_ENDPOINT` to your own values +or set environment variables `ATMOS_TELEMETRY_TOKEN` and `ATMOS_TELEMETRY_ENDPOINT` appropriatelypkg/telemetry/utils_test.go (2)
243-243: Fix typo in function name."Intergration" should be "Integration".
-func TestGetTelemetryFromConfigIntergration(t *testing.T) { +func TestGetTelemetryFromConfigIntegration(t *testing.T) {
479-479: Fix formatting issue in comment.The comment has irregular spacing/tab characters that should be cleaned up.
- // Load cache configuration and set telemetry war ning as already shown. + // Load cache configuration and set telemetry warning as already shown.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (46)
cmd/cmd_utils.go(2 hunks)cmd/root.go(4 hunks)internal/exec/version.go(2 hunks)pkg/config/cache.go(3 hunks)pkg/telemetry/utils.go(1 hunks)pkg/telemetry/utils_test.go(1 hunks)tests/cli_test.go(4 hunks)tests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_Valid_Log_Level_in_Environment_Variable.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_Valid_log_file_in_env_should_be_priortized_over_config.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_Valid_log_file_in_flag_should_be_priortized_over_env_and_config.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_Valid_log_level_in_env_should_be_priortized_over_config.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_Valid_log_level_in_flag_should_be_priortized_over_env_and_config.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_--help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_about_--help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_--help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_generate_--help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_generate_help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_--help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_describe_config_imports.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_doesnt_warn_if_in_git_repo_with_no_atmos_config.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_helmfile_--help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_helmfile_apply_--help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_helmfile_apply_help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_helmfile_help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_terraform_--help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_terraform_--help_alias_subcommand_check.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_terraform_apply_help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_terraform_help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_terraform_plan_non-existent_in_non_workspace.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_validate_editorconfig_help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_vendor_pull_component_using_SSH.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_vendor_pull_custom_detector_credentials_leakage.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_vendor_pull_using_SSH.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_vendor_pull_with_custom_detector_and_handling_credentials_leakage.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_check_atmos_in_empty-dir.stdout.golden(1 hunks)tests/test-cases/log-level-validation.yaml(8 hunks)website/docs/cli/telemetry.mdx(1 hunks)
✅ Files skipped from review due to trivial changes (39)
- tests/snapshots/TestCLICommands_atmos_vendor_pull_component_using_SSH.stderr.golden
- tests/snapshots/TestCLICommands_atmos_vendor_pull_custom_detector_credentials_leakage.stderr.golden
- tests/snapshots/TestCLICommands_Valid_log_level_in_env_should_be_priortized_over_config.stderr.golden
- tests/snapshots/TestCLICommands_check_atmos_in_empty-dir.stdout.golden
- cmd/cmd_utils.go
- tests/snapshots/TestCLICommands_atmos_helmfile_help.stdout.golden
- tests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden
- tests/snapshots/TestCLICommands_atmos_vendor_pull_using_SSH.stderr.golden
- tests/snapshots/TestCLICommands_atmos_atlantis_--help.stdout.golden
- internal/exec/version.go
- tests/snapshots/TestCLICommands_atmos.stdout.golden
- tests/snapshots/TestCLICommands_atmos_validate_editorconfig_help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_helmfile_apply_--help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_terraform_plan_non-existent_in_non_workspace.stdout.golden
- tests/snapshots/TestCLICommands_Valid_log_file_in_env_should_be_priortized_over_config.stdout.golden
- tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config.stdout.golden
- tests/snapshots/TestCLICommands_atmos_helmfile_apply_help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_help.stdout.golden
- tests/snapshots/TestCLICommands_Valid_log_level_in_flag_should_be_priortized_over_env_and_config.stderr.golden
- tests/snapshots/TestCLICommands_atmos_vendor_pull_with_custom_detector_and_handling_credentials_leakage.stderr.golden
- tests/snapshots/TestCLICommands_atmos_terraform_--help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_doesnt_warn_if_in_git_repo_with_no_atmos_config.stdout.golden
- tests/snapshots/TestCLICommands_atmos_about_--help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_atlantis_generate_--help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_terraform_help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_atlantis_help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stdout.golden
- tests/snapshots/TestCLICommands_atmos_atlantis_generate_help.stdout.golden
- tests/snapshots/TestCLICommands_Valid_log_file_in_flag_should_be_priortized_over_env_and_config.stdout.golden
- tests/snapshots/TestCLICommands_atmos_terraform_apply_help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_--help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_helmfile_--help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_--help.stdout.golden
- tests/test-cases/log-level-validation.yaml
- tests/snapshots/TestCLICommands_atmos_describe_config_imports.stderr.golden
- tests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.golden
- tests/snapshots/TestCLICommands_Valid_Log_Level_in_Environment_Variable.stderr.golden
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/config/cache.go
- pkg/telemetry/utils.go
🧰 Additional context used
📓 Path-based instructions (4)
`cmd/*.go`: Implement each Cobra command in a separate file under the `cmd/` dir...
cmd/*.go: Implement each Cobra command in a separate file under thecmd/directory
Use kebab-case for command-line flags
Provide comprehensive help text for all commands and flags
Include examples in command help
Provide meaningful feedback to users in commands
Include progress indicators for long-running operations
Include troubleshooting hints when appropriate
📄 Source: CodeRabbit Inference Engine (.cursor/rules/atmos-rules.mdc)
List of files the instruction was applied to:
cmd/root.go
`**/*.go`: Use Viper for managing configuration, environment variables, and flag...
**/*.go: Use Viper for managing configuration, environment variables, and flags
Use interfaces for external dependencies to facilitate mocking
Follow Go's error handling idioms
Use meaningful error messages
Wrap errors with context usingfmt.Errorf("context: %w", err)
Consider using a custom error type for domain-specific errors
Follow standard Go coding style
Usegofmtandgoimportsto format code
Prefer short, descriptive variable names
Use snake_case for environment variables
Document all exported functions, types, and methods
Document complex logic with inline comments
Follow Go's documentation conventions
Use Viper for configuration management
Support configuration via files, environment variables, and flags
Follow the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users
Log detailed errors for debugging
📄 Source: CodeRabbit Inference Engine (.cursor/rules/atmos-rules.mdc)
List of files the instruction was applied to:
cmd/root.gotests/cli_test.gopkg/telemetry/utils_test.go
`**/*_test.go`: Every new feature must include comprehensive unit tests Test bot...
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for testing multiple scenarios
Include integration tests for command flows
Use test fixtures for complex inputs
Consider using testify/mock for creating mock implementations
📄 Source: CodeRabbit Inference Engine (.cursor/rules/atmos-rules.mdc)
List of files the instruction was applied to:
tests/cli_test.gopkg/telemetry/utils_test.go
`website/**`: Update website documentation in the `website/` directory when addi...
website/**: Update website documentation in thewebsite/directory when adding new features
Follow the website's documentation structure and style
Keep website code in thewebsite/directory
Follow the existing website architecture and style
Document new features on the website
Include examples and use cases in website documentation
📄 Source: CodeRabbit Inference Engine (.cursor/rules/atmos-rules.mdc)
List of files the instruction was applied to:
website/docs/cli/telemetry.mdx
🧠 Learnings (6)
📓 Common learnings
Learnt from: Listener430
PR: cloudposse/atmos#934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Clearly describe the changes in the PR description
Learnt from: aknysh
PR: cloudposse/atmos#944
File: go.mod:206-206
Timestamp: 2025-01-17T00:18:57.769Z
Learning: For indirect dependencies with license compliance issues in the cloudposse/atmos repository, the team prefers to handle them in follow-up PRs rather than blocking the current changes, as these issues often require deeper investigation of the dependency tree.
tests/snapshots/TestCLICommands_atmos_terraform_--help_alias_subcommand_check.stdout.golden (17)
Learnt from: Listener430
PR: cloudposse/atmos#1149
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:7-7
Timestamp: 2025-03-18T12:26:25.329Z
Learning: In the Atmos project, typos or inconsistencies in test snapshot files (such as "terrafrom" instead of "terraform") may be intentional as they capture the exact output of commands and should not be flagged as issues requiring correction.
Learnt from: samtholiya
PR: cloudposse/atmos#955
File: tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden:0-0
Timestamp: 2025-01-19T15:49:15.593Z
Learning: In future commits, the help text for Atmos CLI commands should be limited to only show component and stack parameters for commands that actually use them. This applies to the example usage section in command help text.
Learnt from: Listener430
PR: cloudposse/atmos#1061
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:8-8
Timestamp: 2025-02-14T23:12:38.030Z
Learning: Test snapshots in the Atmos project, particularly for dry run scenarios, may be updated during the development process, and temporary inconsistencies in their content should not be flagged as issues.
Learnt from: Listener430
PR: cloudposse/atmos#825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Learnt from: aknysh
PR: cloudposse/atmos#810
File: examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml:28-32
Timestamp: 2024-12-01T00:33:20.298Z
Learning: In `examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml`, `!exec atmos terraform output` is used in examples to demonstrate its usage, even though `!terraform.output` is the recommended approach according to the documentation.
Learnt from: osterman
PR: cloudposse/atmos#729
File: internal/exec/help.go:48-51
Timestamp: 2024-10-27T16:59:26.187Z
Learning: In the Atmos CLI help messages, when providing examples that include the version number, use the actual version variable (e.g., `version.Version`) instead of placeholders like `<version>`.
Learnt from: samtholiya
PR: cloudposse/atmos#959
File: cmd/workflow.go:74-74
Timestamp: 2025-01-30T19:30:59.120Z
Learning: Error handling for `cmd.Usage()` is not required in the Atmos CLI codebase, as confirmed by the maintainer.
Learnt from: samtholiya
PR: cloudposse/atmos#1068
File: tests/snapshots/TestCLICommands_atmos_terraform_help.stdout.golden:59-64
Timestamp: 2025-02-18T13:13:11.497Z
Learning: For Atmos CLI help text, angle brackets in command examples and flag descriptions should be escaped using HTML entities (e.g., `<component>`) rather than converted to backticks or other markdown formatting.
Learnt from: pkbhowmick
PR: cloudposse/atmos#786
File: internal/exec/shell_utils.go:159-162
Timestamp: 2024-11-16T17:30:52.893Z
Learning: For the `atmos terraform shell` command in `internal/exec/shell_utils.go`, input validation for the custom shell prompt is not required, as users will use this as a CLI tool and any issues will impact themselves.
Learnt from: RoseSecurity
PR: cloudposse/atmos#797
File: pkg/list/atmos.yaml:213-214
Timestamp: 2024-11-25T17:17:15.703Z
Learning: The file `pkg/list/atmos.yaml` is primarily intended for testing purposes.
Learnt from: osterman
PR: cloudposse/atmos#727
File: internal/exec/terraform.go:114-118
Timestamp: 2024-10-21T17:51:53.976Z
Learning: When `atmos terraform clean --everything` is used without specifying a component and without the `--force` flag, prompt the user for confirmation before deleting all components. Use the `--force` flag to skip the confirmation prompt.
Learnt from: osterman
PR: cloudposse/atmos#727
File: internal/exec/terraform.go:114-118
Timestamp: 2024-10-21T17:51:07.087Z
Learning: Use `bubbletea` for confirmation prompts instead of `fmt.Scanln` in the `atmos terraform clean` command.
Learnt from: aknysh
PR: cloudposse/atmos#810
File: internal/exec/terraform_utils.go:145-146
Timestamp: 2024-12-03T05:29:07.718Z
Learning: In the Atmos project, a 5-minute timeout in the `execTerraformOutput` function is acceptable for retrieving `terraform output` for a component in a stack.
Learnt from: aknysh
PR: cloudposse/atmos#810
File: internal/exec/terraform_utils.go:40-213
Timestamp: 2024-12-03T05:18:49.169Z
Learning: In the context of the Atmos project, it's acceptable for functions like `execTerraformOutput` to remain as single functions if they perform a single purpose, such as retrieving Terraform outputs for a component in a stack, even if the function is lengthy.
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: samtholiya
PR: cloudposse/atmos#959
File: cmd/cmd_utils.go:121-148
Timestamp: 2025-02-03T05:57:18.407Z
Learning: The Atmos CLI should fail fast (exit) when encountering configuration errors, including command alias configuration issues, to prevent undefined behavior. Use LogErrorAndExit instead of returning errors in such cases.
Learnt from: osterman
PR: cloudposse/atmos#768
File: website/docs/cheatsheets/vendoring.mdx:70-70
Timestamp: 2024-11-12T13:06:56.194Z
Learning: In `atmos vendor pull --everything`, the `--everything` flag uses the TTY for TUI but is not interactive.
cmd/root.go (33)
Learnt from: Listener430
PR: cloudposse/atmos#825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to cmd/*.go : Provide comprehensive help text for all commands and flags
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to cmd/*.go : Include troubleshooting hints when appropriate
Learnt from: samtholiya
PR: cloudposse/atmos#914
File: cmd/root.go:91-92
Timestamp: 2025-01-18T15:16:54.196Z
Learning: The `showUsageAndExit` function in `cmd/root.go` is designed to display usage information and terminate the program, making error handling unnecessary as the function never returns.
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to cmd/*.go : Provide meaningful feedback to users in commands
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to cmd/*.go : Include examples in command help
Learnt from: Listener430
PR: cloudposse/atmos#844
File: cmd/version.go:34-44
Timestamp: 2024-12-13T15:28:13.630Z
Learning: In `cmd/version.go`, when handling the `--check` flag in the `versionCmd`, avoid using `CheckForAtmosUpdateAndPrintMessage(cliConfig)` as it updates the cache timestamp, which may not be desired in this context.
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to cmd/*.go : Include progress indicators for long-running operations
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.
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Include integration tests for command flows
Learnt from: osterman
PR: cloudposse/atmos#808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
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: Cerebrovinny
PR: cloudposse/atmos#737
File: internal/exec/vendor_utils.go:181-182
Timestamp: 2024-10-31T07:09:31.983Z
Learning: In `internal/exec/vendor_utils.go`, the variables `mergedSources` and `mergedImports` are declared and used later in the code. Do not suggest removing them as unused variables.
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: 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: Cerebrovinny
PR: cloudposse/atmos#764
File: internal/exec/describe_stacks.go:289-295
Timestamp: 2024-11-13T21:37:07.852Z
Learning: In the `internal/exec/describe_stacks.go` file of the `atmos` project written in Go, avoid extracting the stack name handling logic into a helper function within the `ExecuteDescribeStacks` method, even if the logic appears duplicated.
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: 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.
Learnt from: aknysh
PR: cloudposse/atmos#820
File: cmd/list_components.go:53-54
Timestamp: 2024-12-05T22:33:40.955Z
Learning: In the Atmos CLI Go codebase, using `u.LogErrorAndExit` within completion functions is acceptable because it logs the error and exits the command execution.
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: aknysh
PR: cloudposse/atmos#1274
File: go.mod:63-63
Timestamp: 2025-05-30T03:21:37.197Z
Learning: The redis dependency (github.com/redis/go-redis/v9) in the atmos project is only used in tests, not in production code.
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.
Learnt from: RoseSecurity
PR: cloudposse/atmos#797
File: pkg/list/atmos.yaml:213-214
Timestamp: 2024-11-25T17:17:15.703Z
Learning: The file `pkg/list/atmos.yaml` is primarily intended for testing purposes.
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to cmd/*.go : Implement each Cobra command in a separate file under the `cmd/` directory
Learnt from: Listener430
PR: cloudposse/atmos#844
File: cmd/terraform.go:39-39
Timestamp: 2024-12-11T18:46:02.483Z
Learning: `cliConfig` is initialized in `cmd/root.go` and can be used across the `cmd` package.
Learnt from: samtholiya
PR: cloudposse/atmos#1068
File: cmd/vendor_pull.go:31-31
Timestamp: 2025-02-18T13:18:53.146Z
Learning: Error checking is not required for cobra.Command.RegisterFlagCompletionFunc calls as these are static configurations done at init time.
Learnt from: samtholiya
PR: cloudposse/atmos#959
File: cmd/describe_config.go:20-20
Timestamp: 2025-02-03T06:00:11.419Z
Learning: Commands should use `PrintErrorMarkdownAndExit` with empty title and suggestion (`"", err, ""`) for general error handling. Specific titles like "Invalid Usage" or "File Not Found" should only be used for validation or specific error scenarios.
Learnt from: samtholiya
PR: cloudposse/atmos#914
File: cmd/terraform_commands.go:260-265
Timestamp: 2025-01-09T22:37:01.004Z
Learning: In the terraform commands implementation (cmd/terraform_commands.go), the direct use of `os.Args[2:]` for argument handling is intentionally preserved to avoid extensive refactoring. While it could be improved to use cobra's argument parsing, such changes should be handled in a dedicated PR to maintain focus and minimize risk.
Learnt from: samtholiya
PR: cloudposse/atmos#914
File: cmd/root.go:172-178
Timestamp: 2025-01-18T15:18:35.475Z
Learning: The `showUsageAndExit` function in `cmd/cmd_utils.go` provides user feedback by showing error messages, command suggestions, and valid subcommands before terminating the program with `os.Exit(1)`. It never returns to the caller, making error handling unnecessary for calls to this function.
Learnt from: samtholiya
PR: cloudposse/atmos#914
File: cmd/terraform.go:37-46
Timestamp: 2025-01-18T15:15:41.645Z
Learning: In the atmos CLI, error handling is intentionally structured to use LogErrorAndExit for consistent error display, avoiding Cobra's default error handling to prevent duplicate error messages.
Learnt from: samtholiya
PR: cloudposse/atmos#959
File: cmd/workflow.go:74-74
Timestamp: 2025-01-30T19:30:59.120Z
Learning: Error handling for `cmd.Usage()` is not required in the Atmos CLI codebase, as confirmed by the maintainer.
Learnt from: samtholiya
PR: cloudposse/atmos#914
File: cmd/root.go:172-178
Timestamp: 2025-01-18T15:18:35.475Z
Learning: The `showUsageAndExit` function in `cmd/cmd_utils.go` terminates the program with `os.Exit(1)` and never returns to the caller, making error handling unnecessary for calls to this function.
Learnt from: osterman
PR: cloudposse/atmos#727
File: internal/exec/terraform.go:114-118
Timestamp: 2024-10-21T17:51:07.087Z
Learning: Use `bubbletea` for confirmation prompts instead of `fmt.Scanln` in the `atmos terraform clean` command.
tests/cli_test.go (44)
Learnt from: Listener430
PR: cloudposse/atmos#844
File: cmd/version.go:34-44
Timestamp: 2024-12-13T15:28:13.630Z
Learning: In `cmd/version.go`, when handling the `--check` flag in the `versionCmd`, avoid using `CheckForAtmosUpdateAndPrintMessage(cliConfig)` as it updates the cache timestamp, which may not be desired in this context.
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.
Learnt from: Listener430
PR: cloudposse/atmos#844
File: pkg/config/cache.go:42-42
Timestamp: 2024-12-13T16:48:00.294Z
Learning: The function `withCacheFileLock` in `pkg/config/cache.go` is currently unused and left for future upgrade.
Learnt from: Listener430
PR: cloudposse/atmos#825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Include integration tests for command flows
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: CI should run unit tests, integration tests, golangci-lint, and coverage reporting
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.
Learnt from: haitham911
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:215-223
Timestamp: 2024-10-27T04:41:49.199Z
Learning: In `internal/exec/terraform_clean.go`, the function `determineCleanPath` is necessary and should not be removed.
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.
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.
Learnt from: Listener430
PR: cloudposse/atmos#844
File: pkg/config/cache.go:17-31
Timestamp: 2024-12-13T15:33:34.159Z
Learning: In `pkg/config/cache.go`, when `XDG_CACHE_HOME` is not set, falling back to `.` (current directory) is acceptable and aligns with the requirement to primarily use `XDG_CACHE_HOME` for the cache directory.
Learnt from: haitham911
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:314-319
Timestamp: 2024-10-27T04:54:32.397Z
Learning: When deleting empty folders in the `deleteFolders` function, handling errors from `os.Remove` are not required, as failures do not affect the process.
Learnt from: haitham911
PR: cloudposse/atmos#736
File: pkg/utils/file_utils.go:177-194
Timestamp: 2024-10-20T13:06:20.839Z
Learning: In `pkg/utils/file_utils.go`, the `SearchConfigFile` function should focus on returning the file path and an `exists` boolean without returning an error.
Learnt from: osterman
PR: cloudposse/atmos#808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
Learnt from: aknysh
PR: cloudposse/atmos#775
File: internal/exec/template_funcs_component.go:157-159
Timestamp: 2024-11-12T03:16:02.910Z
Learning: In the Go code for `componentFunc` in `internal/exec/template_funcs_component.go`, the function `cleanTerraformWorkspace` does not return errors, and it's acceptable if the file does not exist. Therefore, error handling for `cleanTerraformWorkspace` is not needed.
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: Cerebrovinny
PR: cloudposse/atmos#737
File: internal/exec/vendor_utils.go:131-141
Timestamp: 2024-10-22T23:00:20.627Z
Learning: In the `ReadAndProcessVendorConfigFile` function in `internal/exec/vendor_utils.go`, the existence of the vendor config file is already checked, so additional file existence checks may be unnecessary.
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: samtholiya
PR: cloudposse/atmos#959
File: cmd/workflow.go:74-74
Timestamp: 2025-01-30T19:30:59.120Z
Learning: Error handling for `cmd.Usage()` is not required in the Atmos CLI codebase, as confirmed by the maintainer.
Learnt from: samtholiya
PR: cloudposse/atmos#914
File: cmd/terraform.go:37-46
Timestamp: 2025-01-18T15:15:41.645Z
Learning: In the atmos CLI, error handling is intentionally structured to use LogErrorAndExit for consistent error display, avoiding Cobra's default error handling to prevent duplicate error messages.
Learnt from: aknysh
PR: cloudposse/atmos#820
File: cmd/list_components.go:53-54
Timestamp: 2024-12-05T22:33:40.955Z
Learning: In the Atmos CLI Go codebase, using `u.LogErrorAndExit` within completion functions is acceptable because it logs the error and exits the command execution.
Learnt from: samtholiya
PR: cloudposse/atmos#1147
File: internal/exec/validate_schema.go:42-57
Timestamp: 2025-04-11T22:06:46.999Z
Learning: The "ExecuteAtmosValidateSchemaCmd" function in internal/exec/validate_schema.go has been reviewed and confirmed to have acceptable cognitive complexity despite static analysis warnings. The function uses a clean structure with only three if statements for error handling and delegates complex operations to helper methods.
Learnt from: milldr
PR: cloudposse/atmos#1229
File: internal/exec/workflow_test.go:0-0
Timestamp: 2025-06-02T14:12:02.710Z
Learning: In the atmos codebase, workflow error handling was refactored to use `PrintErrorMarkdown` followed by returning the error instead of `PrintErrorMarkdownAndExit`. This pattern allows proper error testing without the function terminating the process with `os.Exit`, enabling unit tests to assert on error conditions.
Learnt from: samtholiya
PR: cloudposse/atmos#959
File: cmd/cmd_utils.go:121-148
Timestamp: 2025-02-03T05:57:18.407Z
Learning: The Atmos CLI should fail fast (exit) when encountering configuration errors, including command alias configuration issues, to prevent undefined behavior. Use LogErrorAndExit instead of returning errors in such cases.
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*.go : Follow Go's error handling idioms
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: 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.
Learnt from: aknysh
PR: cloudposse/atmos#1274
File: go.mod:63-63
Timestamp: 2025-05-30T03:21:37.197Z
Learning: The redis dependency (github.com/redis/go-redis/v9) in the atmos project is only used in tests, not in production code.
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: 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: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*.go : Support configuration via files, environment variables, and flags
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Consider using testify/mock for creating mock implementations
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Test both happy paths and error conditions
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Use test fixtures for complex inputs
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: All code must pass golangci-lint checks
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests
Learnt from: samtholiya
PR: cloudposse/atmos#1266
File: pkg/utils/highlight_utils.go:0-0
Timestamp: 2025-06-07T19:27:40.807Z
Learning: In pkg/utils/highlight_utils.go, the global variable `isTermPresent` that caches terminal detection at package initialization is an intentional design choice and should not be changed to per-call detection.
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*.go : Use interfaces for external dependencies to facilitate mocking
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to cmd/*.go : Include troubleshooting hints when appropriate
Learnt from: samtholiya
PR: cloudposse/atmos#1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: In the atmos codebase, t.Chdir() is a valid method call on *testing.T objects and works correctly for changing directories in tests. This is implemented through custom testing framework extensions and is used consistently throughout the test suite.
Learnt from: samtholiya
PR: cloudposse/atmos#1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: In the atmos codebase, t.Chdir() is a valid method that can be called on *testing.T objects. This functionality is implemented through custom testing framework extensions and is used consistently throughout the test suite for changing working directories during tests.
Learnt from: samtholiya
PR: cloudposse/atmos#1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: In the atmos codebase, t.Chdir() is a valid method call on *testing.T objects and works correctly for changing directories in tests.
Learnt from: samtholiya
PR: cloudposse/atmos#959
File: cmd/describe_config.go:20-20
Timestamp: 2025-02-03T06:00:11.419Z
Learning: Commands should use `PrintErrorMarkdownAndExit` with empty title and suggestion (`"", err, ""`) for general error handling. Specific titles like "Invalid Usage" or "File Not Found" should only be used for validation or specific error scenarios.
pkg/telemetry/utils_test.go (30)
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Consider using testify/mock for creating mock implementations
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Include integration tests for command flows
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Test both happy paths and error conditions
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Use test fixtures for complex inputs
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios
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.
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: CI should run unit tests, integration tests, golangci-lint, and coverage reporting
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*.go : Use interfaces for external dependencies to facilitate mocking
Learnt from: osterman
PR: cloudposse/atmos#731
File: pkg/utils/file_utils.go:198-202
Timestamp: 2024-10-23T20:13:23.054Z
Learning: In `pkg/utils/file_utils.go`, the current implementation of the `IsURL` function is considered sufficient; avoid suggesting more complex URL validation in future reviews.
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: osterman
PR: cloudposse/atmos#808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
Learnt from: aknysh
PR: cloudposse/atmos#1274
File: go.mod:63-63
Timestamp: 2025-05-30T03:21:37.197Z
Learning: The redis dependency (github.com/redis/go-redis/v9) in the atmos project is only used in tests, not in production code.
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.
Learnt from: osterman
PR: cloudposse/atmos#1202
File: pkg/utils/yaml_func_exec.go:104-104
Timestamp: 2025-04-23T15:02:50.246Z
Learning: In the Atmos codebase, direct calls to `os.Getenv` should be avoided. Instead, use `viper.BindEnv` for environment variable access. This provides a consistent approach to configuration management across the codebase.
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#944
File: go.mod:3-3
Timestamp: 2025-01-17T00:21:32.987Z
Learning: The project uses Go version 1.23.0 which has been confirmed by the maintainer to be working in production for months. Do not flag this as an invalid Go version.
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to go.mod : Manage dependencies with Go modules
Learnt from: aknysh
PR: cloudposse/atmos#944
File: go.mod:3-3
Timestamp: 2025-01-17T00:21:32.987Z
Learning: Go version 1.23.0 was deliberately introduced by the maintainer (aknysh) in January 2025. While this might be a pre-release or development version of Go, it has been approved for use in this project.
Learnt from: Cerebrovinny
PR: cloudposse/atmos#729
File: internal/exec/terraform.go:258-264
Timestamp: 2024-10-19T17:42:29.152Z
Learning: Prefer not to declare variables within 'if' statements in Go code.
Learnt from: Cerebrovinny
PR: cloudposse/atmos#764
File: internal/exec/describe_stacks.go:289-295
Timestamp: 2024-11-13T21:37:07.852Z
Learning: In the `internal/exec/describe_stacks.go` file of the `atmos` project written in Go, avoid extracting the stack name handling logic into a helper function within the `ExecuteDescribeStacks` method, even if the logic appears duplicated.
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: All code must pass golangci-lint checks
Learnt from: RoseSecurity
PR: cloudposse/atmos#797
File: pkg/list/atmos.yaml:213-214
Timestamp: 2024-11-25T17:17:15.703Z
Learning: The file `pkg/list/atmos.yaml` is primarily intended for testing purposes.
Learnt from: Listener430
PR: cloudposse/atmos#1149
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:7-7
Timestamp: 2025-03-18T12:26:25.329Z
Learning: In the Atmos project, typos or inconsistencies in test snapshot files (such as "terrafrom" instead of "terraform") may be intentional as they capture the exact output of commands and should not be flagged as issues requiring correction.
Learnt from: samtholiya
PR: cloudposse/atmos#1147
File: internal/exec/validate_schema.go:42-57
Timestamp: 2025-04-11T22:06:46.999Z
Learning: The "ExecuteAtmosValidateSchemaCmd" function in internal/exec/validate_schema.go has been reviewed and confirmed to have acceptable cognitive complexity despite static analysis warnings. The function uses a clean structure with only three if statements for error handling and delegates complex operations to helper methods.
Learnt from: Cerebrovinny
PR: cloudposse/atmos#737
File: internal/exec/vendor_utils.go:131-141
Timestamp: 2024-10-22T23:00:20.627Z
Learning: In the `ReadAndProcessVendorConfigFile` function in `internal/exec/vendor_utils.go`, the existence of the vendor config file is already checked, so additional file existence checks may be unnecessary.
Learnt from: osterman
PR: cloudposse/atmos#768
File: internal/exec/vendor_utils.go:496-513
Timestamp: 2024-11-22T12:38:33.132Z
Learning: In the Atmos project, continue to flag path traversal issues in code reviews but acknowledge when they are expected and acceptable in specific cases.
Learnt from: samtholiya
PR: cloudposse/atmos#1068
File: tests/snapshots/TestCLICommands_atmos_terraform_help.stdout.golden:59-64
Timestamp: 2025-02-18T13:13:11.497Z
Learning: For Atmos CLI help text, angle brackets in command examples and flag descriptions should be escaped using HTML entities (e.g., `<component>`) rather than converted to backticks or other markdown formatting.
Learnt from: Listener430
PR: cloudposse/atmos#844
File: cmd/version.go:34-44
Timestamp: 2024-12-13T15:28:13.630Z
Learning: In `cmd/version.go`, when handling the `--check` flag in the `versionCmd`, avoid using `CheckForAtmosUpdateAndPrintMessage(cliConfig)` as it updates the cache timestamp, which may not be desired in this context.
Learnt from: Listener430
PR: cloudposse/atmos#844
File: cmd/cmd_utils.go:454-464
Timestamp: 2024-12-15T10:20:08.436Z
Learning: Avoid adding timeout handling for GitHub API calls in `CheckForAtmosUpdateAndPrintMessage` function in `cmd/cmd_utils.go`, as it might be disabled by user settings.
website/docs/cli/telemetry.mdx (6)
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to website/** : Document new features on the website
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Keep CLI documentation and website documentation in sync
Learnt from: samtholiya
PR: cloudposse/atmos#955
File: tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden:0-0
Timestamp: 2025-01-19T15:49:15.593Z
Learning: In future commits, the help text for Atmos CLI commands should be limited to only show component and stack parameters for commands that actually use them. This applies to the example usage section in command help text.
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to website/** : Update website documentation in the `website/` directory when adding new features
Learnt from: RoseSecurity
PR: cloudposse/atmos#797
File: pkg/list/atmos.yaml:213-214
Timestamp: 2024-11-25T17:17:15.703Z
Learning: The file `pkg/list/atmos.yaml` is primarily intended for testing purposes.
Learnt from: Listener430
PR: cloudposse/atmos#934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
🧬 Code Graph Analysis (2)
cmd/root.go (1)
pkg/telemetry/utils.go (2)
CaptureCmd(44-50)PrintTelemetryDisclosure(55-59)
tests/cli_test.go (2)
pkg/telemetry/ci.go (2)
PreserveCIEnvVars(85-114)RestoreCIEnvVars(123-128)pkg/config/cache.go (1)
GetCacheFilePath(23-37)
🪛 golangci-lint (1.64.8)
tests/cli_test.go
[error] 725-725: error is not nil (line 723) but it returns nil
(nilerr)
🪛 LanguageTool
website/docs/cli/telemetry.mdx
[uncategorized] ~60-~60: You might be missing the article “the” here.
Context: ... enabled: false ``` or set environment variable ATMOS_TELEMETRY_ENABLED to `...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[style] ~78-~78: Since ownership is already implied, this phrasing may be redundant.
Context: ...OKENandATMOS_TELEMETRY_ENDPOINT` to your own values
(PRP_OWN)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
🔇 Additional comments (12)
tests/snapshots/TestCLICommands_atmos_terraform_--help_alias_subcommand_check.stdout.golden (1)
2-6: Telemetry notice properly integrated into CLI help output.The addition of the telemetry disclosure notice at the beginning of the help output is consistent with the PR objective and follows good practices for transparency.
cmd/root.go (4)
27-27: Good integration of telemetry package import.The telemetry package import is properly placed and aligns with the new functionality being added.
173-174: Proper command execution tracking implementation.The switch from
Execute()toExecuteC()correctly captures both the command and error for telemetry purposes. Thetelemetry.CaptureCmd(cmd, err)call appropriately tracks command execution.
264-264: Telemetry disclosure properly integrated into help flow.The placement of
telemetry.PrintTelemetryDisclosure()in the help function ensures users are informed about telemetry collection when viewing help.
278-278: Consistent telemetry disclosure in root help.The telemetry disclosure is consistently added to both help paths, ensuring transparency regardless of how help is accessed.
website/docs/cli/telemetry.mdx (1)
1-80: Comprehensive telemetry documentation.The documentation effectively covers all essential aspects of the telemetry feature including data collection, privacy considerations, and configuration options. Well structured and user-friendly.
tests/cli_test.go (3)
22-23: Good addition of required imports for telemetry testing.The imports for config, telemetry, and assert packages are properly added to support the new telemetry-related test functionality.
Also applies to: 32-32
597-600: Proper cache cleanup for test isolation.Removing the cache file before each test ensures a clean state and prevents interference between tests. Good practice for reliable testing.
602-605: Good CI environment isolation for tests.The preservation and restoration of CI environment variables ensures proper test isolation and prevents CI-specific behavior from affecting test results.
pkg/telemetry/utils_test.go (3)
24-58: LGTM! Well-structured telemetry configuration test.The test properly validates telemetry configuration consistency across multiple instances and covers all required fields.
444-470: Excellent disclosure message testing.The test properly verifies the one-time disclosure behavior and includes the full expected message text for validation.
492-513: Good CI environment testing.Properly tests that disclosure messages are suppressed in CI environments, which is the expected behavior for automated systems.
what
why
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests