Added ini 8000 tial 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 |
📝 WalkthroughWalkthroughThis change introduces anonymous telemetry to Atmos, capturing command usage, errors, environment details, and workspace IDs. It adds configuration, environment variable support, a PostHog-based telemetry implementation, cache updates for installation IDs, CI/Docker detection, user-facing warnings, and comprehensive tests. Documentation and CLI messages were updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Telemetry
participant PostHog
User->>CLI: Run command
CLI->>Telemetry: CaptureCmd(cmd, err)
Telemetry->>Telemetry: Load config/cache, check enabled
Telemetry->>Telemetry: Detect CI/Docker/WorkspaceID
Telemetry->>PostHog: Send event (if enabled)
PostHog-->>Telemetry: Ack/Err
Telemetry-->>CLI: Return
CLI-->>User: Output (with telemetry notice if needed)
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Suggested reviewers
✨ Finishing Touches
🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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(atmos 8000 Config.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.C 8000 aptureCmdFailureperforms 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
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/cli_test.go(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
🔇 Additional comments (2)
tests/cli_test.go (2)
22-22: Import additions look appropriate for the new functionality.The new imports support the cache management functionality -
configfor getting the cache file path andassertfor error checking in tests.Also applies to: 31-31
609-612: Cache removal before tests ensures proper isolation.Good practice to clear cache state before each test to prevent interference between test runs.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/telemetry/utils.go (1)
32-50: Consider simplifying the variadic error parameter pattern.The current approach with variadic error parameters works but could be more idiomatic. Consider using a more explicit approach or dedicated wrapper functions.
-// CaptureCmdString is the public API for capturing command string telemetry. -// It accepts an optional error parameter and handles the case where no error is provided. -func CaptureCmdString(cmdString string, err ...error) { - var inErr error - if len(err) > 0 { - inErr = err[0] - } - captureCmdString(cmdString, inErr) -} +// CaptureCmdString captures telemetry for a successful command execution. +func CaptureCmdString(cmdString string) { + captureCmdString(cmdString, nil) +} + +// CaptureCmdStringWithError captures telemetry for a failed command execution. +func CaptureCmdStringWithError(cmdString string, err error) { + captureCmdString(cmdString, err) +}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/telemetry/utils.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
🔇 Additional comments (4)
pkg/telemetry/utils.go (4)
64-93: Solid telemetry initialization with good dependency injection support.The function properly handles configuration loading, cache management, and provides clean dependency injection for testing. The use of the generic cache helper is well-implemented.
110-128: Comprehensive telemetry data collection looks good.The event properties capture all necessary information while maintaining anonymity. The combination of system info, environment detection, and command details provides valuable insights without compromising user privacy.
140-167: Well-refactored warning message logic.The function now uses early returns and the generic cache helper, addressing the previous complexity concerns. The logic is much cleaner and easier to follow.
169-204: Excellent generic cache helper implementation.This function demonstrates good use of Go generics and provides a clean abstraction for cache operations. The error handling is appropriate and the function is well-documented.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/cli_test.go(4 hunks)
🧰 Additional context used
🪛 golangci-lint (1.64.8)
tests/cli_test.go
[error] 723-723: error is not nil (line 721) but it returns nil
(nilerr)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Build (ubuntu-latest, linux)
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Summary
🔇 Additional comments (3)
tests/cli_test.go (3)
22-23: LGTM! Imports support telemetry integration.The new imports are properly aligned with the telemetry functionality being added to the test suite.
Also applies to: 32-32
595-598: Good practice for test isolation.Removing the cache file before each test ensures tests don't rely on stale cached state, improving test reliability and reproducibility.
600-603: Excellent test isolation improvement.Preserving and restoring CI environment variables ensures tests behave consistently across different execution environments (local vs CI).
what
why
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores