Skip to content

Add telemetry enable/disable commands#1301

Open
schurchleycci wants to merge 5 commits into
nextfrom
fact-161/telemetry-commands
Open

Add telemetry enable/disable commands#1301
schurchleycci wants to merge 5 commits into
nextfrom
fact-161/telemetry-commands

Conversation

@schurchleycci
Copy link
Copy Markdown

@schurchleycci schurchleycci commented May 13, 2026

Adds circleci settings telemetry enable and circleci settings telemetry disable. Preference is persisted to the config file; env vars (CIRCLECI_NO_TELEMETRY, NO_ANALYTICS, DO_NOT_TRACK, CI) always win. telemetry_enabled is also surfaced in circleci settings list.

Design notes

  • Telemetry commands live under circleci settings telemetry rather than as a top-level command group — telemetry is a CLI setting, so it belongs alongside set and list
  • No --json on enable/disable — pure mutation commands; read path is covered by settings list --json
  • Confirmation message uses the resolved config path (respects --config), not the XDG default
Screenshot 2026-05-14 at 11 04 04 AM

Test plan

  • circleci settings telemetry enable / disable write the correct value and print the right path
  • circleci --config /tmp/custom.yml settings telemetry enable confirmation shows /tmp/custom.yml
  • CIRCLECI_NO_TELEMETRY=1 circleci settings telemetry enable saves the preference but prints the override notice
  • circleci settings list and --json both include telemetry_enabled
  • task test

🤖 Generated with Claude Code

@linear
Copy link
Copy Markdown

linear Bot commented May 13, 2026

FACT-161

- Fix confirmation message showing wrong path when --config flag is used
- Use strconv.FormatBool instead of fmt.Sprintf("%t") in settings list
- Add unit tests for IsTelemetryEnabled covering env var × stored preference
- Pin TestTelemetryNoArgs assertion to stdout instead of stdout||stderr

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@schurchleycci schurchleycci force-pushed the fact-161/telemetry-commands branch from d552538 to 792eb86 Compare May 13, 2026 13:41
schurchleycci and others added 2 commits May 13, 2026 09:47
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…etenv in handler

- Handle config.Path() error instead of silently discarding it in both
  enable and disable handlers; pass resolvedPath (not original configPath)
  to SetTelemetryEnabled so the written path and success message always agree
- Unexport NoTelemetryEnvVars → noTelemetryEnvVars; add ActiveTelemetryOverrides()
  to expose only the currently-set overrides without handing callers a mutable slice
- Replace os.Getenv loop in enable handler with config.ActiveTelemetryOverrides(),
  removing the direct os import from the command layer

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@schurchleycci schurchleycci changed the title feat(telemetry): add telemetry enable/disable commands Add telemetry enable/disable commands May 13, 2026
@pete-woods
Copy link
Copy Markdown
Contributor

pete-woods commented May 14, 2026

This looks good, but I believe we should manage the setting under the settings command, like we do other settings.

@schurchleycci
Copy link
Copy Markdown
Author

This looks good, but I believe we should manage the setting under the settings command, like we do other settings.

Good call - so pretty much take what I've got here and put it under e.g. circleci settings telemetry enable instead of a top level command?

Telemetry enable/disable now live at `circleci settings telemetry enable`
and `circleci settings telemetry disable` rather than as a top-level
command group. Settings imports the telemetry package and registers it
as a subcommand; root no longer registers telemetry directly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@schurchleycci
Copy link
Copy Markdown
Author

@pete-woods this is updated to be circleci settings telemetry now

@@ -0,0 +1,90 @@
// Copyright (c) 2026 Circle Internet Services, Inc.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These files still look like they need moving.

Stamping, assuming that will be done, though.

@pete-woods
Copy link
Copy Markdown
Contributor

Have stamped under the assumption of moving those files.

The internal/cmd/telemetry package is removed entirely. The three files
are recreated as package settings under internal/cmd/settings/, and
settings.go now calls newTelemetryCmd() directly with no cross-package
import.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@schurchleycci
Copy link
Copy Markdown
Author

Have stamped under the assumption of moving those files.

Ahhh sorry about that, I've clearly been juggling too many Claude sessions. They're fully moved over now rather than duplicated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants