From 78f2b840915fc4c21ec80a8864c1fd18dbe856a6 Mon Sep 17 00:00:00 2001 From: Bartosz Blizniak Date: Mon, 26 Oct 2026 11:50:02 +0000 Subject: [PATCH 1/5] feat(ENG-15367): capture usage facts and send one event at the end of every run A hook on the root command records the command path, the flags and aliases used, where each setting came from, the login method and the API status, all from the Click context, so new commands and flags need no telemetry code. AliasGroup.main hands the run to the sender when it ends. Credential helper commands and login use the tracked command classes so their runs are recorded (or excluded) consistently. Grouped short options (-vS, -vPname) are split up as Click does, so a typed -S or -P always reaches the send rules. The reported env var is the first non-empty one, as Click uses, and a typed --help only counts as help when the run succeeded. Co-Authored-By: Claude Opus 5.5 --- cloudsmith_cli/cli/command.py | 90 +++- .../commands/credential_helper/__init__.py | 3 +- .../cli/commands/credential_helper/cargo.py | 3 +- .../cli/commands/credential_helper/docker.py | 3 +- .../cli/commands/credential_helper/generic.py | 3 +- .../cli/commands/credential_helper/manage.py | 7 +- .../cli/commands/credential_helper/pnpm.py | 3 +- .../commands/credential_helper/terraform.py | 3 +- cloudsmith_cli/cli/commands/login.py | 3 +- cloudsmith_cli/cli/config_sources.py | 69 +++ cloudsmith_cli/cli/decorators.py | 5 +- cloudsmith_cli/cli/exceptions.py | 3 + .../cli/tests/test_config_sources.py | 41 ++ .../cli/tests/test_startup_imports.py | 2 + .../cli/tests/test_usage_capture.py | 306 +++++++++++++ cloudsmith_cli/cli/tests/test_usage_main.py | 425 ++++++++++++++++++ cloudsmith_cli/cli/usage_capture.py | 347 ++++++++++++++ cloudsmith_cli/core/usage/event.py | 10 +- 18 files changed, 1312 insertions(+), 14 deletions(-) create mode 100644 cloudsmith_cli/cli/config_sources.py create mode 100644 cloudsmith_cli/cli/tests/test_config_sources.py create mode 100644 cloudsmith_cli/cli/tests/test_usage_capture.py create mode 100644 cloudsmith_cli/cli/tests/test_usage_main.py create mode 100644 cloudsmith_cli/cli/usage_capture.py diff --git a/cloudsmith_cli/cli/command.py b/cloudsmith_cli/cli/command.py index 87ddc813e..4e180be49 100644 --- a/cloudsmith_cli/cli/command.py +++ b/cloudsmith_cli/cli/command.py @@ -8,6 +8,7 @@ from click_didyoumean import DYMGroup from ..core.certificate_errors import describe_certificate_error +from .usage_capture import TrackedCommand, TrackingMixin, record_alias def _is_json_output_requested(exception): @@ -74,7 +75,7 @@ def _echo_certificate_hint(exception): click.echo(f"Hint: {click.style(hint, fg='yellow')}", err=True) -class AliasGroup(DYMGroup): +class AliasGroup(TrackingMixin, DYMGroup): """A command group with DYM, alias and lazy-subcommand support. ``lazy_commands`` maps a command name to the module that registers it. @@ -83,6 +84,8 @@ class AliasGroup(DYMGroup): ``lazy_aliases`` supplies the alias map for those commands up front. """ + command_class = TrackedCommand + def __init__(self, *args, **kwargs): lazy_commands = kwargs.pop("lazy_commands", None) lazy_aliases = kwargs.pop("lazy_aliases", None) @@ -96,6 +99,8 @@ def __init__(self, *args, **kwargs): self.inverse[alias] = name def resolve_command(self, ctx, args): + if args and args[0] in self.inverse: + record_alias(ctx, args[0]) try: return super().resolve_command(ctx, args) except click.exceptions.UsageError: @@ -106,6 +111,7 @@ def resolve_command(self, ctx, args): cmds = self.list_commands(ctx) matched = [cmd for cmd in cmds if cmd.startswith(cmd_name)] if len(matched) == 1 and len(cmd_name) > 1: + record_alias(ctx, "") args[0] = matched[0] return super().resolve_command(ctx, args) @@ -177,21 +183,60 @@ def format_commands(self, ctx, formatter): ctx.showing_help = True return super().format_commands(ctx, formatter) + def _main_shell_completion(self, ctx_args, prog_name, complete_var=None): + """Mark shell completion runs so usage telemetry ignores them. + + Click handles ``*_source`` before any context exists, so the + ``resilient_parsing`` check in ``_finish_usage`` never sees it. + """ + import os + + from . import usage_capture + + name = complete_var + if name is None: + # Same derivation as Click's own _main_shell_completion. + name = prog_name.replace("-", "_").replace(".", "_") + name = f"_{name}_COMPLETE".upper() + if os.environ.get(name): + usage_capture.mark_completion_run() + return super()._main_shell_completion(ctx_args, prog_name, complete_var) + def main(self, *args, **kwargs): """Override main to intercept exceptions and format as JSON if requested.""" import sys + import time + + from . import usage_capture original_standalone_mode = kwargs.get("standalone_mode", True) kwargs["standalone_mode"] = False + usage = {"outcome": "unexpected_error", "exit_code": 1} + depth = usage_capture.enter_run() + started = time.monotonic() try: - return super().main(*args, **kwargs) + rv = super().main(*args, **kwargs) + usage.update( + outcome="success" if not rv else "error_exit", + exit_code=(rv if isinstance(rv, int) else 1) if rv else 0, + ) + return rv except click.exceptions.Abort: + usage.update(outcome="aborted", exit_code=1) if not original_standalone_mode: raise click.echo("Aborted!", err=True) sys.exit(1) except click.exceptions.ClickException as e: + usage.update( + outcome=( + "usage_error" + if isinstance(e, click.exceptions.UsageError) + else "click_error" + ), + exit_code=e.exit_code, + ) if _is_json_output_requested(e): import json @@ -205,15 +250,24 @@ def main(self, *args, **kwargs): sys.exit(e.exit_code) except BrokenPipeError: # Click does not catch a failed write in its Ctrl+C handler. + usage.update(outcome="aborted", exit_code=1) if not original_standalone_mode: raise sys.stdout = _PacifyFlushWrapper(sys.stdout) sys.stderr = _PacifyFlushWrapper(sys.stderr) sys.exit(1) + except SystemExit as e: + code = e.code + usage.update( + exit_code=code if isinstance(code, int) else (0 if code is None else 1), + outcome="success" if code in (0, None) else "error_exit", + ) + raise except Exception as e: # pylint: disable=broad-exception-caught # Anything reaching here is unexpected: click's own errors and # handled API errors (which exit via ctx.exit) are dealt with # above or never get this far. + usage.update(outcome="unexpected_error", exit_code=1) if not original_standalone_mode: raise main_args = kwargs.get("args", args[0] if args else None) @@ -228,3 +282,35 @@ def main(self, *args, **kwargs): click.echo(f"Error: {telemetry.format_exception_summary(e)}", err=True) _echo_certificate_hint(e) sys.exit(1) + finally: + _finish_usage(usage, started, nested=depth > 1) + usage_capture.exit_run() + + +def _finish_usage(usage, started, *, nested): + """Hand the run to usage telemetry; never raises, never prints.""" + try: + import time + + from ..core.usage import run + from . import usage_capture + + meta = usage_capture.current_root_meta() or {} + contexts = meta.get(usage_capture.META_KEY, {}).get("contexts", []) + # Shell completion parses with resilient_parsing (or, for the + # *_source script, not at all); never report it. + completing = usage_capture.is_completion_run() or any( + getattr(c, "resilient_parsing", False) for c in contexts + ) + facts = usage_capture.collect_facts( + meta, + outcome=usage["outcome"], + exit_code=usage["exit_code"], + duration_ms=int((time.monotonic() - started) * 1000), + nested=nested or completing, + ) + if facts.api_status is not None and facts.outcome == "error_exit": + facts.outcome = "api_error" + run.finish(facts) + except BaseException: # pylint: disable=broad-exception-caught + return diff --git a/cloudsmith_cli/cli/commands/credential_helper/__init__.py b/cloudsmith_cli/cli/commands/credential_helper/__init__.py index bcf76528e..93f8038fe 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/__init__.py +++ b/cloudsmith_cli/cli/commands/credential_helper/__init__.py @@ -8,6 +8,7 @@ import click +from ...usage_capture import TrackedGroup from ..main import main from .cargo import cargo as cargo_cmd from .docker import docker as docker_cmd @@ -17,7 +18,7 @@ from .terraform import terraform as terraform_cmd -@click.group() +@click.group(cls=TrackedGroup) def credential_helper(): """ Credential helpers for package managers. diff --git a/cloudsmith_cli/cli/commands/credential_helper/cargo.py b/cloudsmith_cli/cli/commands/credential_helper/cargo.py index f52b8a8c1..74452eeea 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/cargo.py +++ b/cloudsmith_cli/cli/commands/credential_helper/cargo.py @@ -13,9 +13,10 @@ from ....credential_helpers.cargo import execute from ...decorators import common_api_auth_options, resolve_credentials +from ...usage_capture import TrackedCommand -@click.command(context_settings={"ignore_unknown_options": True}) +@click.command(cls=TrackedCommand, context_settings={"ignore_unknown_options": True}) @click.option( "--cargo-plugin", is_flag=True, diff --git a/cloudsmith_cli/cli/commands/credential_helper/docker.py b/cloudsmith_cli/cli/commands/credential_helper/docker.py index ad8345af3..60b21e619 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/docker.py +++ b/cloudsmith_cli/cli/commands/credential_helper/docker.py @@ -13,9 +13,10 @@ from ....credential_helpers.docker import execute from ...decorators import common_api_auth_options, resolve_credentials +from ...usage_capture import TrackedCommand -@click.command() +@click.command(cls=TrackedCommand) @click.argument("operation", required=False, default="get") @common_api_auth_options @resolve_credentials diff --git a/cloudsmith_cli/cli/commands/credential_helper/generic.py b/cloudsmith_cli/cli/commands/credential_helper/generic.py index db98b17da..c01722750 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/generic.py +++ b/cloudsmith_cli/cli/commands/credential_helper/generic.py @@ -15,9 +15,10 @@ common_cli_config_options, resolve_credentials, ) +from ...usage_capture import TrackedCommand -@click.command() +@click.command(cls=TrackedCommand) @common_cli_config_options @common_api_auth_options @resolve_credentials diff --git a/cloudsmith_cli/cli/commands/credential_helper/manage.py b/cloudsmith_cli/cli/commands/credential_helper/manage.py index 344757537..0ceb6388b 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/manage.py +++ b/cloudsmith_cli/cli/commands/credential_helper/manage.py @@ -37,6 +37,7 @@ common_cli_output_options, resolve_credentials, ) +from ...usage_capture import TrackedCommand # --------------------------------------------------------------------------- # Helper registry — extend here when new helpers are added @@ -248,7 +249,7 @@ def _terraform_workspace_next_steps(helper_args: tuple[str, ...]) -> list[str]: # --------------------------------------------------------------------------- -@click.command("install") +@click.command("install", cls=TrackedCommand) @click.argument("helper") @click.option( "--bin-dir", @@ -420,7 +421,7 @@ def install_cmd( # --------------------------------------------------------------------------- -@click.command("uninstall") +@click.command("uninstall", cls=TrackedCommand) @click.argument("helper") @click.option( "--bin-dir", @@ -487,7 +488,7 @@ def uninstall_cmd(ctx, opts, helper: str, bin_dir: str | None, dry_run: bool) -> # --------------------------------------------------------------------------- -@click.command("list") +@click.command("list", cls=TrackedCommand) @common_cli_config_options @common_cli_output_options @click.pass_context diff --git a/cloudsmith_cli/cli/commands/credential_helper/pnpm.py b/cloudsmith_cli/cli/commands/credential_helper/pnpm.py index 603f5e5c3..39e10963a 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/pnpm.py +++ b/cloudsmith_cli/cli/commands/credential_helper/pnpm.py @@ -13,9 +13,10 @@ from ....credential_helpers.pnpm import execute from ...config import Options from ...decorators import common_api_auth_options, resolve_credentials +from ...usage_capture import TrackedCommand -@click.command() +@click.command(cls=TrackedCommand) @click.argument("repo", required=False, default="npm.cloudsmith.io") @common_api_auth_options @resolve_credentials diff --git a/cloudsmith_cli/cli/commands/credential_helper/terraform.py b/cloudsmith_cli/cli/commands/credential_helper/terraform.py index c49008f69..c1f75dfaf 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/terraform.py +++ b/cloudsmith_cli/cli/commands/credential_helper/terraform.py @@ -19,9 +19,10 @@ common_cli_config_options, resolve_credentials, ) +from ...usage_capture import TrackedCommand -@click.command(context_settings={"ignore_unknown_options": True}) +@click.command(cls=TrackedCommand, context_settings={"ignore_unknown_options": True}) @click.option( "-r", "--repo", diff --git a/cloudsmith_cli/cli/commands/login.py b/cloudsmith_cli/cli/commands/login.py index d1654a4e4..eac4dedd2 100644 --- a/cloudsmith_cli/cli/commands/login.py +++ b/cloudsmith_cli/cli/commands/login.py @@ -8,6 +8,7 @@ from ...core.config import create_config_files, new_config_messaging from .. import decorators, deprecation, utils from ..exceptions import handle_api_exceptions +from ..usage_capture import TrackedCommand from ..utils import maybe_spinner from .main import main @@ -61,7 +62,7 @@ def summarise_removed_login(exc, detail, fields): return None -class DeprecatedLoginCommand(click.Command): +class DeprecatedLoginCommand(TrackedCommand): """Command that prints the deprecation warning before any prompt.""" def parse_args(self, ctx, args): diff --git a/cloudsmith_cli/cli/config_sources.py b/cloudsmith_cli/cli/config_sources.py new file mode 100644 index 000000000..2740e8447 --- /dev/null +++ b/cloudsmith_cli/cli/config_sources.py @@ -0,0 +1,69 @@ +# Copyright 2026 Cloudsmith Ltd +"""Which settings were set in config.ini / credentials.ini, by section. + +``ConfigReader.read_config()`` fills schema defaults, so it cannot tell an +explicit key from a default; this reads the raw files and reports key +presence only. Values are never read. +""" + +import configparser +import os + +from ..core.usage.event import SETTING_ALIASES +from . import config + + +def _files(reader, explicit): + paths = [explicit] if explicit else [] + paths.extend( + name if os.path.isabs(name) else os.path.join(searchpath, name) + for searchpath in reader.config_searchpath + for name in reader.config_files + ) + return [p for p in paths if p and os.path.isfile(p)] + + +def _declared(schema): + """Key names the CLI reads for a section schema (never user-authored keys).""" + return { + param.name + for param in vars(schema).values() + if isinstance(param, config.ConfigParam) + } + + +def _keys(paths, section, declared): + keys = set() + for path in paths: + parser = configparser.RawConfigParser() + try: + parser.read(path, encoding="utf-8") + except (configparser.Error, UnicodeDecodeError, OSError): + continue + if parser.has_section(section): + keys.update(parser.options(section)) + return keys & declared + + +def detect( + *, config_file: str | None, creds_file: str | None, profile: str | None +) -> dict[str, str]: + """Map each setting present in the ini files to where it was set.""" + found: dict[str, str] = {} + for reader, schema, explicit, base in ( + (config.ConfigReader, config.ConfigSchema.Default, config_file, "config"), + ( + config.CredentialsReader, + config.CredentialsSchema.Default, + creds_file, + "credentials", + ), + ): + paths = _files(reader, explicit) + declared = _declared(schema) + for key in _keys(paths, "default", declared): + found[SETTING_ALIASES.get(key, key)] = f"{base}_file" + if profile and profile != "default": + for key in _keys(paths, f"profile:{profile}", declared): + found[SETTING_ALIASES.get(key, key)] = f"{base}_profile" + return found diff --git a/cloudsmith_cli/cli/decorators.py b/cloudsmith_cli/cli/decorators.py index 8f7904583..78c5c1de6 100644 --- a/cloudsmith_cli/cli/decorators.py +++ b/cloudsmith_cli/cli/decorators.py @@ -19,7 +19,7 @@ disabled_detectors_from_env, registered_detectors, ) -from . import config, deprecation, utils +from . import config, deprecation, usage_capture, utils def _expired_saml_renewal_message(error): @@ -242,6 +242,7 @@ def wrapper(ctx, *args, **kwargs): opts.load_config_file(path=config_file, profile=profile) opts.load_creds_file(path=creds_file, profile=profile) + usage_capture.record_config(ctx) opts.api_key_from_file = opts.api_key _warn_deprecated_config_keys(ctx, opts) kwargs["opts"] = opts @@ -627,6 +628,7 @@ def wrapper(ctx, *args, **kwargs): deprecation.warn_envvar(legacy_envvar, "CLOUDSMITH_SSL_VERIFY=0") ctx.params["ssl_verify"] = ssl_verify opts.ssl_verify = ssl_verify + usage_capture.record_session(ctx) opts.ssl_verify_strict = kwargs.pop("ssl_verify_strict") opts.api_user_agent = kwargs.pop("api_user_agent") opts.api_headers = kwargs.pop("api_headers") @@ -812,6 +814,7 @@ def wrapper(ctx, *args, **kwargs): click.secho(message, fg="yellow", err=True) opts.credential = credential + usage_capture.record_credential(ctx, credential) kwargs["opts"] = opts return ctx.invoke(f, *args, **kwargs) diff --git a/cloudsmith_cli/cli/exceptions.py b/cloudsmith_cli/cli/exceptions.py index 5fcfea284..21b8b574c 100644 --- a/cloudsmith_cli/cli/exceptions.py +++ b/cloudsmith_cli/cli/exceptions.py @@ -116,6 +116,9 @@ def handle_api_exceptions( raise if exit_on_error: + from .usage_capture import record_api_status + + record_api_status(ctx, exc.status) ctx.exit(exc.status or 1) diff --git a/cloudsmith_cli/cli/tests/test_config_sources.py b/cloudsmith_cli/cli/tests/test_config_sources.py new file mode 100644 index 000000000..2ebe846f3 --- /dev/null +++ b/cloudsmith_cli/cli/tests/test_config_sources.py @@ -0,0 +1,41 @@ +from cloudsmith_cli.cli import config, config_sources + + +def _point_readers(monkeypatch, path): + for reader in (config.ConfigReader, config.CredentialsReader): + monkeypatch.setattr(reader, "config_searchpath", [str(path)]) + monkeypatch.setattr(config.ConfigReader, "config_files", ["config.ini"]) + monkeypatch.setattr(config.CredentialsReader, "config_files", ["credentials.ini"]) + + +def test_detects_default_and_profile_keys_not_defaults(monkeypatch, tmp_path): + (tmp_path / "config.ini").write_text( + "[default]\nworkspace = acme\n\n[profile:ci]\ncheck_for_update = false\n" + ) + (tmp_path / "credentials.ini").write_text("[profile:ci]\napi_key = not-real\n") + _point_readers(monkeypatch, tmp_path) + found = config_sources.detect(config_file=None, creds_file=None, profile="ci") + assert found == { + "workspace": "config_file", + "check_for_update": "config_profile", + "api_key": "credentials_profile", + } + assert "telemetry" not in found and "ssl_verify" not in found + + +def test_reports_only_schema_declared_keys_for_each_file(monkeypatch, tmp_path): + (tmp_path / "config.ini").write_text( + "[default]\nworkspace = acme\nmy_secret_token = x\n" + ) + (tmp_path / "credentials.ini").write_text( + "[default]\napi_key = not-real\nworkspace = other\n" + ) + _point_readers(monkeypatch, tmp_path) + found = config_sources.detect(config_file=None, creds_file=None, profile=None) + assert found == {"workspace": "config_file", "api_key": "credentials_file"} + + +def test_unreadable_file_is_ignored(monkeypatch, tmp_path): + (tmp_path / "config.ini").write_text("[default\nbroken") + _point_readers(monkeypatch, tmp_path) + assert not config_sources.detect(config_file=None, creds_file=None, profile=None) diff --git a/cloudsmith_cli/cli/tests/test_startup_imports.py b/cloudsmith_cli/cli/tests/test_startup_imports.py index f89cf2658..19d86fb71 100644 --- a/cloudsmith_cli/cli/tests/test_startup_imports.py +++ b/cloudsmith_cli/cli/tests/test_startup_imports.py @@ -20,6 +20,8 @@ "ssl", # Error reporting imports it on the error path only. "sentry_sdk", + # Usage telemetry loads only at the end of a run. + "cloudsmith_cli.core.usage", ) diff --git a/cloudsmith_cli/cli/tests/test_usage_capture.py b/cloudsmith_cli/cli/tests/test_usage_capture.py new file mode 100644 index 000000000..fbe5cc5dc --- /dev/null +++ b/cloudsmith_cli/cli/tests/test_usage_capture.py @@ -0,0 +1,306 @@ +from types import SimpleNamespace + +import click +import pytest +from click.testing import CliRunner + +from cloudsmith_cli.cli import usage_capture +from cloudsmith_cli.cli.command import AliasGroup + +CAPTURED = {} + + +@click.group(cls=AliasGroup) +def app(): + pass + + +@app.command(aliases=["ls"]) +@click.option("-w", "--workspace", "--org", "workspace", envvar="CLOUDSMITH_WORKSPACE") +@click.option("-F", "--output-format", "output") +@click.argument("name", required=False) +@click.pass_context +def listing(ctx, workspace, output, name): + # Collect inside the run: env vars and contexts are live here. + CAPTURED["facts"] = usage_capture.collect_facts( + ctx.meta, outcome="success", exit_code=0, duration_ms=5, nested=False + ) + + +@pytest.fixture(autouse=True) +def isolated_config(telemetry_config_dir): # pylint: disable=unused-argument + CAPTURED.clear() + + +def _run(args, env=None): + CliRunner().invoke(app, args, env=env or {}, catch_exceptions=False) + return CAPTURED["facts"] + + +def test_typed_spellings_not_values(): + tokens = ["-w", "acme", "-Fjson", "--org=x", "--", "-w"] + assert usage_capture.match_typed_flags(listing.params, tokens) == [ + "--org", + "-F", + "-w", + ] + + +@click.command() +@click.option("-v", "--verbose", is_flag=True) +@click.option("-c", "--count", count=True) +@click.option("-S", "--without-api-ssl-verify", is_flag=True) +@click.option("-P", "--profile") +def clustered(verbose, count, without_api_ssl_verify, profile): + pass + + +@pytest.mark.parametrize( + "token, expected", + [ + # Flags in a cluster are all seen, as Click applies them all. + ("-vS", ["-S", "-v"]), + ("-cvc", ["-c", "-v"]), + # An option that takes a value consumes the rest of the token. + ("-vPprivate", ["-P", "-v"]), + ("-PvS", ["-P"]), + # An undeclared character stops the expansion (Click rejects it). + ("-vxS", ["-v"]), + ], +) +def test_short_option_clusters_are_expanded(token, expected): + assert usage_capture.match_typed_flags(clustered.params, [token]) == expected + + +def test_clustered_profile_and_ssl_off_reach_the_gates(): + typed = usage_capture.match_typed_flags(clustered.params, ["-vPprivate", "-vS"]) + assert usage_capture.CONFIG_SPELLINGS & set(typed) + assert usage_capture.resolve_ssl_verify(None, typed) is False + + +def test_facts_from_a_run(): + facts = _run(["ls", "-w", "secret-ws", "-F", "json", "pkg-name"]) + assert facts.command == "listing" + assert facts.aliases_used == ["ls"] + assert set(facts.flags) == {"workspace", "output"} + assert facts.flags_typed == ["-F", "-w"] + assert "name" not in facts.setting_sources # arguments are not settings + assert "secret-ws" not in repr(facts) and "pkg-name" not in repr(facts) + + +def test_env_sources_report_names_only(): + facts = _run(["listing"], env={"CLOUDSMITH_WORKSPACE": "secret-ws"}) + assert facts.envvars == ["CLOUDSMITH_WORKSPACE"] + assert facts.setting_sources.get("workspace") == "env" + assert "secret-ws" not in repr(facts) + + +@app.command() +@click.option("--proxy", envvar=["CLOUDSMITH_PROXY", "CLOUDSMITH_API_PROXY"]) +@click.pass_context +def proxied(ctx, proxy): + CAPTURED["facts"] = usage_capture.collect_facts( + ctx.meta, outcome="success", exit_code=0, duration_ms=5, nested=False + ) + + +def test_an_empty_env_var_is_not_reported_as_the_source(): + # Click skips the empty CLOUDSMITH_PROXY and uses CLOUDSMITH_API_PROXY. + facts = _run( + ["proxied"], env={"CLOUDSMITH_PROXY": "", "CLOUDSMITH_API_PROXY": "secret"} + ) + assert facts.envvars == ["CLOUDSMITH_API_PROXY"] + assert "secret" not in repr(facts) + + +def _facts_with_typed(typed, outcome, exit_code): + meta = {usage_capture.META_KEY: {"contexts": [], "typed": typed, "aliases": []}} + return usage_capture.collect_facts( + meta, outcome=outcome, exit_code=exit_code, duration_ms=1, nested=False + ) + + +def test_typed_help_after_a_successful_exit_is_help(): + assert _facts_with_typed(["--help"], "success", 0).outcome == "help" + + +@pytest.mark.parametrize("outcome", ["usage_error", "click_error", "api_error"]) +def test_typed_help_never_hides_a_failure(outcome): + # e.g. `whoami --no-such-flag --help` fails parsing before help is shown. + assert _facts_with_typed(["--help"], outcome, 2).outcome == outcome + + +def test_help_spelling_is_seen(): + tokens = ["--help"] + params = listing.get_params(click.Context(listing)) + assert usage_capture.match_typed_flags(params, tokens) == ["--help"] + + +def test_every_command_in_the_cli_is_tracked(): + import importlib + import pkgutil + + from cloudsmith_cli.cli import commands + from cloudsmith_cli.cli.commands.main import main + + for info in pkgutil.walk_packages( + commands.__path__, prefix=commands.__name__ + "." + ): + importlib.import_module(info.name) + + def walk(cmd, path): + assert isinstance(cmd, usage_capture.TrackingMixin), " ".join(path) + for name, sub in getattr(cmd, "commands", {}).items(): + walk(sub, path + [name]) + + walk(main, []) + + +def test_collect_facts_does_not_read_config_files(monkeypatch): + from cloudsmith_cli.cli import config_sources + + def explode(**_): + raise AssertionError("config files read before the telemetry gates") + + monkeypatch.setattr(config_sources, "detect", explode) + facts = _run(["listing"], env={"CLOUDSMITH_WORKSPACE": "x"}) + assert facts.setting_sources == {"workspace": "env"} + + +def test_deferred_sources_return_the_config_file_sources(monkeypatch): + from cloudsmith_cli.cli import config_sources + + monkeypatch.setattr( + config_sources, "detect", lambda **_: {"workspace": "config_file"} + ) + facts = _run(["listing"]) + assert facts.deferred_sources() == {"workspace": "config_file"} + + +def test_a_parse_outside_a_run_pins_nothing(): + assert usage_capture._RUN["depth"] == 0 # pylint: disable=protected-access + CliRunner().invoke(app, ["listing"], catch_exceptions=False) + assert usage_capture.current_root_meta() is None + + +def test_session_env_names_match_the_option_declarations(): + from cloudsmith_cli.cli.commands import list_ + + envvars = {p.name: p.envvar for p in list_.repos.params} + assert envvars["api_host"] == usage_capture.API_HOST_ENV + assert envvars["ssl_verify"] == usage_capture.SSL_VERIFY_ENV + assert ( + envvars["deprecated_without_api_ssl_verify"] + == usage_capture.WITHOUT_SSL_VERIFY_ENV + ) + spellings = {s for p in list_.repos.params for s in p.opts + p.secondary_opts} + assert usage_capture.API_HOST_SPELLINGS <= spellings + assert usage_capture.SSL_OFF_SPELLINGS <= spellings + + +def test_session_resolved_only_for_the_leaf_or_its_descendants(): + root = click.Context(click.Group("root")) + group = click.Context(click.Group("list"), parent=root) + leaf = click.Context(click.Command("repos"), parent=group) + forwarded = click.Context(click.Command("get"), parent=leaf) + unrelated = click.Context(click.Command("other"), parent=root) + contexts = [root, group, leaf] + + def resolved(session): + store = {"contexts": contexts, "session": session} + return usage_capture._session_resolved(store) # pylint: disable=protected-access + + assert resolved(leaf) is True + assert resolved(forwarded) is True # ctx.forward / ctx.invoke sub-context + assert resolved(group) is False # set up before the leaf parsed its flags + assert resolved(root) is False + assert resolved(unrelated) is False + assert resolved(None) is False + assert usage_capture._session_resolved({"session": leaf}) is False # pylint: disable=protected-access + + +def test_config_resolved_only_for_the_leaf_or_its_descendants(): + root = click.Context(click.Group("root")) + group = click.Context(click.Group("list"), parent=root) + leaf = click.Context(click.Command("repos"), parent=group) + forwarded = click.Context(click.Command("get"), parent=leaf) + contexts = [root, group, leaf] + + def resolved(config_ctx): + store = {"contexts": contexts, "config": config_ctx} + return usage_capture._config_resolved(store) # pylint: disable=protected-access + + assert resolved(leaf) is True + assert resolved(forwarded) is True + assert resolved(group) is False # loaded before the leaf parsed its flags + assert resolved(root) is False + assert resolved(None) is False + + +def test_config_spellings_match_the_option_declarations(): + from cloudsmith_cli.cli.commands import list_ + + spellings = { + s + for p in list_.repos.params + if p.name in ("config_file", "profile") + for s in p.opts + p.secondary_opts + } + assert usage_capture.CONFIG_SPELLINGS == spellings + + +#: Options of the shared decorators that are not settings, so they have no +#: ``source_``. Every other option must alias onto ALLOWED_SETTINGS. +NOT_SETTINGS = frozenset( + { + "debug", # per-run output switches + "verbose", + "no_color_env", # env plumbing for colour; the names land in envvars + "force_color_env", + "term_env", + "allowed_api_host_suffixes", # hidden, internal trust lists + "allowed_proxy_suffixes", + "deprecated_allowed_api_proxy_suffixes", + "oidc_discovery_disabled", # no config.ini key + "without_rate_limit", # API client tuning, no config.ini key + "rate_limit_warning", + "error_retry_max", + "error_retry_backoff", + "error_retry_codes", + } +) + + +@pytest.mark.parametrize( + "name", + [ + "common_cli_config_options", + "common_cli_output_options", + "common_api_auth_options", + "initialise_session", + "resolve_credentials", # holds -w/--workspace (``org``) + "initialise_api", + ], +) +def test_every_shared_option_is_a_setting_or_explicitly_not(name): + from cloudsmith_cli.cli import decorators + from cloudsmith_cli.core.usage import event + + decorator = getattr(decorators, name) + command = click.command()(decorator(lambda **kwargs: None)) + for param in command.params: + setting = event.SETTING_ALIASES.get(param.name, param.name) + assert setting in event.ALLOWED_SETTINGS or param.name in NOT_SETTINGS, ( + param.name + ) + + +def test_record_credential_keeps_the_last_resolved_source(): + store_ctx = click.Context(click.Command("x")) + usage_capture.record_credential(store_ctx, SimpleNamespace(source_name="env_var")) + usage_capture.record_credential(store_ctx, None) + assert store_ctx.meta[usage_capture.META_KEY]["auth_source"] == "env_var" + usage_capture.record_credential( + store_ctx, SimpleNamespace(source_name="credentials_file") + ) + assert store_ctx.meta[usage_capture.META_KEY]["auth_source"] == "credentials_file" diff --git a/cloudsmith_cli/cli/tests/test_usage_main.py b/cloudsmith_cli/cli/tests/test_usage_main.py new file mode 100644 index 000000000..755b4bc4e --- /dev/null +++ b/cloudsmith_cli/cli/tests/test_usage_main.py @@ -0,0 +1,425 @@ +import json + +import pytest + +from cloudsmith_cli.cli.commands.main import main +from cloudsmith_cli.core.usage import notice, state + + +@pytest.fixture() +def notice_seen(): + state.write({"notice_version": notice.NOTICE_VERSION}) + + +def test_version_sends_success(usage_events, notice_seen): + assert main(args=["--version"], standalone_mode=False) in (0, None) + assert usage_events[-1]["properties"]["outcome"] == "success" + assert usage_events[-1]["properties"]["command"] == "" + + +def test_help_is_recorded_as_help(usage_events, notice_seen): + # Click returns 0 for --help when standalone_mode is False. + assert main(args=["repositories", "--help"], standalone_mode=False) == 0 + props = usage_events[-1]["properties"] + assert props["outcome"] == "help" and props["command"] == "repositories" + + +def test_usage_error_keeps_exit_code(usage_events, notice_seen): + with pytest.raises(SystemExit) as exit_info: + main(args=["repositories", "--no-such-flag"]) + assert exit_info.value.code == 2 + assert usage_events[-1]["properties"]["outcome"] == "usage_error" + + +def test_credential_helper_sends_nothing(usage_events, notice_seen): + with pytest.raises(SystemExit): + main(args=["credential-helper", "docker", "list"]) + assert usage_events == [] + + +def test_finish_failure_does_not_change_exit(usage_events, notice_seen, monkeypatch): + from cloudsmith_cli.core.usage import run + + monkeypatch.setattr(run, "_finish", lambda facts: 1 / 0) + assert main(args=["--version"], standalone_mode=False) in (0, None) + + +@pytest.mark.parametrize( + "env, args", + [ + ({"CLOUDSMITH_API_HOST": "https://artifacts.example.com"}, ["--version"]), + ({"CLOUDSMITH_SSL_VERIFY": "false"}, ["--version"]), + ({"CLOUDSMITH_SSL_VERIFY": "Off"}, ["--version"]), + ({"CLOUDSMITH_WITHOUT_API_SSL_VERIFY": "1"}, ["--version"]), + # Not `list repos`: test_deprecation runs that command object as a root + # command, and Click then caches its help option without `-h`. + ({}, ["repos", "get", "--api-host", "https://artifacts.example.com", "-h"]), + ( + {}, + ["list", "repos", "--api-host", "https://artifacts.example.com", "--help"], + ), + ({}, ["list", "repos", "--without-ssl-verify", "--help"]), + ({}, ["list", "repos", "-S", "--help"]), + ], +) +def test_host_and_ssl_gates_hold_before_session_setup( + usage_events, notice_seen, monkeypatch, env, args +): + # None of these runs reaches initialise_session, which is where the + # options normally land in opts. + for name, value in env.items(): + monkeypatch.setenv(name, value) + assert main(args=args, standalone_mode=False) in (0, None) + assert usage_events == [] + + +def test_cloudsmith_api_host_from_env_still_sends( + usage_events, notice_seen, monkeypatch +): + monkeypatch.setenv("CLOUDSMITH_API_HOST", "https://api.cloudsmith.io") + monkeypatch.setenv("CLOUDSMITH_SSL_VERIFY", "true") + assert main(args=["--version"], standalone_mode=False) in (0, None) + assert len(usage_events) == 1 + + +def test_empty_ssl_env_does_not_turn_ssl_off(usage_events, notice_seen, monkeypatch): + # Click treats an empty env var as unset. + monkeypatch.setenv("CLOUDSMITH_SSL_VERIFY", "") + monkeypatch.setenv("CLOUDSMITH_WITHOUT_API_SSL_VERIFY", "") + assert main(args=["--version"], standalone_mode=False) in (0, None) + assert len(usage_events) == 1 + + +@pytest.mark.parametrize( + "instruction, env", + [ + ("zsh_source", {}), + ("bash_complete", {"COMP_WORDS": "cloudsmith ", "COMP_CWORD": "1"}), + ], +) +def test_shell_completion_sends_nothing( + usage_events, notice_seen, monkeypatch, capsys, instruction, env +): + monkeypatch.setenv("_CLOUDSMITH_COMPLETE", instruction) + for name, value in env.items(): + monkeypatch.setenv(name, value) + with pytest.raises(SystemExit) as exit_info: + main(args=[], prog_name="cloudsmith") + assert exit_info.value.code == 0 + assert capsys.readouterr().out # the script or the completions + assert usage_events == [] + + +@pytest.fixture() +def reporting_enabled(sentry_events, monkeypatch): # pylint: disable=unused-argument + """Every opt-out cleared, as error reporting tests do (no usage_events).""" + for name in ( + "CLOUDSMITH_API_HOST", + "CLOUDSMITH_SSL_VERIFY", + "CLOUDSMITH_WITHOUT_API_SSL_VERIFY", + ): + monkeypatch.delenv(name, raising=False) + monkeypatch.setenv("CLOUDSMITH_NO_UPDATE_CHECK", "1") + + +def test_exported_key_never_reaches_tests( + reporting_enabled, notice_seen, blocked_usage_posts +): + # Discriminates when CLOUDSMITH_TELEMETRY_POSTHOG_KEY is exported: the + # autouse guard removes it, so the run stops before any POST. + assert main(args=["--version"], standalone_mode=False) in (0, None) + assert blocked_usage_posts == [] + + +def test_a_test_that_sets_the_key_still_posts_nothing( + reporting_enabled, notice_seen, monkeypatch, blocked_usage_posts +): + from cloudsmith_cli.core.usage import settings + + monkeypatch.setenv(settings.KEY_ENV, "phc_test") + assert main(args=["--version"], standalone_mode=False) in (0, None) + # The run reached the POST, and the guard took it instead of the network. + assert len(blocked_usage_posts) == 1 + blocked_usage_posts.clear() # expected: don't fail at teardown + + +def test_guarded_post_raises(blocked_usage_posts): + from cloudsmith_cli.core.usage import sender + + with pytest.raises(AssertionError, match="must not send usage telemetry"): + sender._post(None, "https://eu.i.posthog.com/i/v0/e/", "{}", 1.0) + assert len(blocked_usage_posts) == 1 + blocked_usage_posts.clear() + + +@pytest.fixture() +def no_repos_api(monkeypatch): + """Answer ``list repos`` with an empty page instead of calling the API.""" + from cloudsmith_cli.core.api import repos + from cloudsmith_cli.core.pagination import PageInfo + + calls = [] + + def list_repos(**kwargs): + calls.append(kwargs) + return [], PageInfo() + + monkeypatch.setattr(repos, "list_repos", list_repos) + return calls + + +INSTALL_ID = "5b0d9c1e-6a2f-4d0e-9b7a-3f1c2e8d4a10" + + +def test_install_id_never_reaches_sentry( + usage_events, sentry_events, notice_seen, monkeypatch +): + from cloudsmith_cli.core.api import repos + + state.write({"install_id": INSTALL_ID, "created_at": 1792076242}) + + def crash(**kwargs): + raise RuntimeError("boom") + + monkeypatch.setattr(repos, "list_repos", crash) + with pytest.raises(SystemExit) as exit_info: + main(args=["list", "repos", "acme/x"]) + assert exit_info.value.code == 1 + + # One run produced both: the usage event carries the ID, the error does not. + assert usage_events[-1]["distinct_id"] == INSTALL_ID + assert usage_events[-1]["properties"]["outcome"] == "unexpected_error" + (error,) = sentry_events + serialised = json.dumps(error) + assert INSTALL_ID not in serialised + assert "1792076242" not in serialised + assert "install_id" not in serialised + + +@pytest.mark.parametrize( + "host, sent", + [("https://api.cloudsmith.io", 1), ("https://artifacts.example.com", 0)], +) +def test_forwarding_command_resolves_a_typed_api_host( + usage_events, notice_seen, no_repos_api, host, sent +): + # `list repos` hands off to `repositories get` (ctx.forward), whose + # session setup runs in a sub-context of the leaf. + args = ["list", "repos", "acme/x", "-F", "json", "--api-host", host] + assert main(args=args, standalone_mode=False) in (0, None) + assert len(no_repos_api) == 1 # the run reached the session and the API + assert len(usage_events) == sent + + +def _run_root(args): + """Run ``main()`` on a root path that ends before config is loaded.""" + from cloudsmith_cli.cli import config + + if args == ["--help"]: + assert main(args=args, standalone_mode=False) == 0 + else: + with pytest.raises(SystemExit) as exit_info: + main(args=args) + assert exit_info.value.code == 2 + # Root --help exits at parse and root usage errors in resolve_command, + # both before the root callback loads config. + assert getattr(config.OPTIONS, "value", None) is None + + +@pytest.mark.parametrize("args", [["--help"], ["no-such-command"]]) +@pytest.mark.parametrize( + "setting", + [ + "api_host = https://artifacts.example.com", + "ssl_verify = false", + "api_ssl_verify = false", + ], +) +def test_config_file_host_and_ssl_gates_hold_before_config_loads( + usage_events, notice_seen, telemetry_config_dir, args, setting +): + (telemetry_config_dir / "config.ini").write_text(f"[default]\n{setting}\n") + _run_root(args) + assert usage_events == [] + + +def test_config_file_profile_host_gate_holds_before_config_loads( + usage_events, notice_seen, telemetry_config_dir, monkeypatch +): + (telemetry_config_dir / "config.ini").write_text( + "[default]\n[profile:work]\napi_host = https://artifacts.example.com\n" + ) + monkeypatch.setenv("CLOUDSMITH_PROFILE", "work") + _run_root(["--help"]) + assert usage_events == [] + + +@pytest.mark.parametrize("args", [["--help"], ["no-such-command"]]) +def test_config_file_cloudsmith_host_still_sends_before_config_loads( + usage_events, notice_seen, telemetry_config_dir, args +): + (telemetry_config_dir / "config.ini").write_text( + "[default]\napi_host = https://api.cloudsmith.io\n" + ) + _run_root(args) + assert len(usage_events) == 1 + + +def test_env_api_host_beats_the_config_file_before_config_loads( + usage_events, notice_seen, telemetry_config_dir, monkeypatch +): + # As in initialise_session, CLOUDSMITH_API_HOST wins over config.ini. + (telemetry_config_dir / "config.ini").write_text( + "[default]\napi_host = https://artifacts.example.com\n" + ) + monkeypatch.setenv("CLOUDSMITH_API_HOST", "https://api.cloudsmith.io") + _run_root(["--help"]) + assert len(usage_events) == 1 + + +def test_unreadable_config_file_sends_nothing_before_config_loads( + usage_events, notice_seen, telemetry_config_dir +): + (telemetry_config_dir / "config.ini").write_text("[default\napi_host = x\n") + _run_root(["--help"]) + assert usage_events == [] + + +def _run_any(args): + """Run ``main()`` like a user would, whatever the exit code.""" + try: + main(args=args) + except SystemExit: + pass + + +@pytest.mark.parametrize( + "args", + [ + ["ls", "pkgs", "ws/r", "--config-file", "{file}", "--help"], + ["ls", "pkgs", "--config-file", "{file}"], # missing argument + ["whoami", "--config-file", "{file}", "--help"], + ["whoami", "-C", "{file}", "--help"], + ["--config-file", "{file}", "--help"], + ], +) +@pytest.mark.parametrize( + "setting", ["telemetry = false", "api_host = https://artifacts.example.com"] +) +def test_typed_config_file_holds_before_the_command_loads_it( + usage_events, notice_seen, tmp_path, args, setting +): + path = tmp_path / "optout.ini" + path.write_text(f"[default]\n{setting}\n") + _run_any([arg.format(file=path) for arg in args]) + assert usage_events == [] + + +@pytest.mark.parametrize("args", [["--profile", "p", "--help"], ["-P", "p", "-h"]]) +def test_typed_profile_opt_out_holds_before_the_command_loads_it( + usage_events, notice_seen, telemetry_config_dir, args +): + (telemetry_config_dir / "config.ini").write_text( + "[default]\n[profile:p]\ntelemetry = false\n" + ) + _run_any(args) + assert usage_events == [] + + +@pytest.mark.parametrize( + "setting", ["telemetry = false", "api_host = https://artifacts.example.com"] +) +def test_config_file_env_holds_before_config_loads( + usage_events, notice_seen, tmp_path, monkeypatch, setting +): + path = tmp_path / "optout.ini" + path.write_text(f"[default]\n{setting}\n") + monkeypatch.setenv("CLOUDSMITH_CONFIG_FILE", str(path)) + _run_root(["--help"]) + assert usage_events == [] + + +def test_config_file_env_without_an_opt_out_still_sends( + usage_events, notice_seen, tmp_path, monkeypatch +): + path = tmp_path / "plain.ini" + path.write_text("[default]\ncheck_for_update = true\n") + monkeypatch.setenv("CLOUDSMITH_CONFIG_FILE", str(path)) + _run_root(["--help"]) + assert len(usage_events) == 1 + + +def test_typed_config_file_without_an_opt_out_sends_once_loaded( + usage_events, notice_seen, no_repos_api, tmp_path +): + # The leaf loads the file itself (and hands off through ctx.forward), so + # the opt-out check sees what the file sets. + path = tmp_path / "plain.ini" + path.write_text("[default]\ncheck_for_update = true\n") + args = ["list", "repos", "acme/x", "-F", "json", "--config-file", str(path)] + assert main(args=args, standalone_mode=False) in (0, None) + assert len(no_repos_api) == 1 + assert len(usage_events) == 1 + + +def test_typed_config_file_with_an_opt_out_sends_nothing_once_loaded( + usage_events, notice_seen, no_repos_api, tmp_path +): + path = tmp_path / "optout.ini" + path.write_text("[default]\ntelemetry = false\n") + args = ["list", "repos", "acme/x", "-F", "json", "--config-file", str(path)] + assert main(args=args, standalone_mode=False) in (0, None) + assert usage_events == [] + + +@pytest.mark.parametrize( + "env, extra, setting, source, flag", + [ + ({"CLOUDSMITH_WORKSPACE": "acme"}, [], "workspace", "env", None), + ({}, ["-w", "acme"], "workspace", "flag", "org"), + ({}, ["--config-file", "{file}"], "config_file_path", "flag", "config_file"), + ], +) +def test_setting_sources_use_the_setting_names( + usage_events, + notice_seen, + no_repos_api, + tmp_path, + monkeypatch, + env, + extra, + setting, + source, + flag, +): + path = tmp_path / "plain.ini" + path.write_text("[default]\ncheck_for_update = true\n") + for name, value in env.items(): + monkeypatch.setenv(name, value) + args = ["list", "repos", "acme/x", "-F", "json"] + args += [arg.format(file=path) for arg in extra] + assert main(args=args, standalone_mode=False) in (0, None) + props = usage_events[-1]["properties"] + assert props[f"source_{setting}"] == source + if flag: # flags keep the Click parameter names + assert flag in props["flags"] + + +def test_forwarding_command_keeps_the_login_method( + usage_events, notice_seen, no_repos_api, monkeypatch +): + # `list repos` resolves credentials again inside `repositories get` + # (ctx.forward), where the env key is not passed on; that later None + # must not hide the login method the run used. + monkeypatch.setenv("CLOUDSMITH_API_KEY", "not-a-real-key") + args = ["list", "repos", "acme/x", "-F", "json"] + assert main(args=args, standalone_mode=False) in (0, None) + assert usage_events[-1]["properties"]["auth_source"] == "env_var" + + +def test_command_that_never_logs_in_reports_no_login_method( + usage_events, notice_seen, monkeypatch +): + monkeypatch.setenv("CLOUDSMITH_API_KEY", "not-a-real-key") + main(args=["credential-helper", "list"], standalone_mode=False) + assert "auth_source" not in usage_events[-1]["properties"] diff --git a/cloudsmith_cli/cli/usage_capture.py b/cloudsmith_cli/cli/usage_capture.py new file mode 100644 index 000000000..71850a53c --- /dev/null +++ b/cloudsmith_cli/cli/usage_capture.py @@ -0,0 +1,347 @@ +# Copyright 2026 Cloudsmith Ltd +"""Record usage facts from the live Click context. + +Only names defined in code leave this module: command names, parameter +names, the option spellings declared on each option, env var names declared +on each option, and alias names. Argument and option values are never read. +""" + +import os +import sys +from typing import Any + +import click +from click.core import ParameterSource + +META_KEY = "cloudsmith_usage" +_HELP_SPELLINGS = frozenset({"-h", "--help"}) +_SOURCE_BY_CLICK = { + ParameterSource.COMMANDLINE: "flag", + ParameterSource.ENVIRONMENT: "env", +} +#: Click env vars that carry no information (set on almost every run). +_IGNORED_ENVVARS = frozenset({"TERM"}) + +# The env vars and spellings of the host and SSL options declared in +# ``decorators.initialise_session``. Their values reach ``opts`` only when +# that decorator runs, which ``--version``, ``--help`` and usage errors never +# reach, so the host and SSL gates also read them here. +API_HOST_ENV = "CLOUDSMITH_API_HOST" +SSL_VERIFY_ENV = "CLOUDSMITH_SSL_VERIFY" +WITHOUT_SSL_VERIFY_ENV = "CLOUDSMITH_WITHOUT_API_SSL_VERIFY" +API_HOST_SPELLINGS = frozenset({"--api-host"}) +SSL_OFF_SPELLINGS = frozenset( + {"--without-ssl-verify", "--without-api-ssl-verify", "-S"} +) +#: The config file and profile options of ``decorators.common_cli_config_options``. +#: A typed one names a file or profile only that decorator loads. +CONFIG_SPELLINGS = frozenset({"-C", "--config-file", "-P", "--profile"}) +#: Not a host: a typed ``--api-host`` that never reached opts fails the gate. +UNRESOLVED_HOST = "unresolved" +#: Click's boolean spellings (``BoolParamType``); an empty env var is unset. +_FALSE = frozenset({"0", "false", "no", "off", "n", "f"}) +_TRUE = frozenset({"1", "true", "yes", "on", "y", "t"}) + + +def _store(ctx): + return ctx.meta.setdefault(META_KEY, {"contexts": [], "typed": [], "aliases": []}) + + +def _takes_value(param): + return not ( + param.is_flag or param.count or getattr(param, "_flag_needs_value", False) + ) + + +def _short_cluster(token, spellings): + """Expand ``-vSPname`` the way Click does: ``-v``, ``-S``, ``-P``. + + Stops at the first option that takes a value (the rest of the token is + that value) or at an undeclared character. + """ + found = [] + for char in token[1:]: + spelling = f"-{char}" + if spelling not in spellings: + break + found.append(spelling) + if spellings[spelling]: + break + return found + + +def match_typed_flags(params, args): + """Return the declared option spellings present in ``args`` (no values).""" + # spelling -> whether the option takes a value + spellings = {} + for param in params: + if isinstance(param, click.Option): + for spelling in param.opts: + spellings[spelling] = _takes_value(param) + for spelling in param.secondary_opts: + spellings[spelling] = False + found = set() + for token in args: + if token == "--": + break + if not token.startswith("-") or token == "-": + continue + name = token.split("=", 1)[0] + if name in spellings: + found.add(name) + elif not name.startswith("--"): + found.update(_short_cluster(token, spellings)) + return sorted(found) + + +_RUN: dict[str, Any] = {"depth": 0, "root_meta": None, "completion": False} + + +def enter_run(): + """Mark the start of a top-level ``main()``; return the nesting depth.""" + _RUN["depth"] += 1 + if _RUN["depth"] == 1: + _RUN["root_meta"] = None + _RUN["completion"] = False + return _RUN["depth"] + + +def exit_run(): + """Mark the end of a ``main()`` run.""" + _RUN["depth"] = max(0, _RUN["depth"] - 1) + if _RUN["depth"] == 0: + _RUN["root_meta"] = None + _RUN["completion"] = False + + +def mark_completion_run(): + """Record that this run only serves shell completion.""" + _RUN["completion"] = True + + +def is_completion_run(): + """Tell whether this run only serves shell completion.""" + return _RUN["completion"] + + +def current_root_meta(): + """Return the ``ctx.meta`` of the root context of the current run.""" + return _RUN["root_meta"] + + +class TrackingMixin(click.Command): + """Record each context and its typed option spellings before parsing. + + Recording happens before ``super().parse_args`` so usage errors and + ``--help`` (which exit during parsing) still have a command path. + """ + + def parse_args(self, ctx, args): + if _RUN["depth"] >= 1 and ctx.parent is None and _RUN["root_meta"] is None: + _RUN["root_meta"] = ctx.meta + store = _store(ctx) + store["contexts"].append(ctx) + store["typed"].extend(match_typed_flags(self.get_params(ctx), args)) + return super().parse_args(ctx, args) + + +class TrackedCommand(TrackingMixin, click.Command): + """A command whose invocation is recorded for usage facts.""" + + +class TrackedGroup(TrackingMixin, click.Group): + """A group whose invocation is recorded; subcommands default to tracked.""" + + command_class = TrackedCommand + + +def record_alias(ctx, name): + """Record that a command was reached through an alias (or ````).""" + _store(ctx)["aliases"].append(name) + + +def record_api_status(ctx, status): + """Record the HTTP status of the API error that ended the run. + + Best effort: this runs on the error path, which usage capture must + never change, so a context that cannot hold the status is ignored. + """ + try: + _store(ctx)["api_status"] = status + except (AttributeError, TypeError): + pass + + +def _command_path(contexts): + leaf = contexts[-1] if contexts else None + names = [] + while leaf is not None and leaf.parent is not None: + if leaf.command.name: + names.append(leaf.command.name) + leaf = leaf.parent + return " ".join(reversed(names)) or "" + + +def _env_names(param): + if isinstance(param.envvar, str): + return [param.envvar] + return list(param.envvar or []) + + +def _param_facts(contexts): + from ..core.usage.event import SETTING_ALIASES + + flags, envvars, sources = set(), set(), {} + for ctx in contexts: + for param in ctx.command.params: + if not param.name or not isinstance(param, click.Option): + continue # arguments are neither flags nor settings + source = ctx.get_parameter_source(param.name) + if source == ParameterSource.COMMANDLINE: + flags.add(param.name) + if source == ParameterSource.ENVIRONMENT: + # Click uses the first non-empty variable, not the first set one. + used = next((n for n in _env_names(param) if os.environ.get(n)), None) + if used and used not in _IGNORED_ENVVARS: + envvars.add(used) + if source in _SOURCE_BY_CLICK: + setting = SETTING_ALIASES.get(param.name, param.name) + sources[setting] = _SOURCE_BY_CLICK[source] + return sorted(flags), sorted(envvars), sources + + +def _config_sources(meta): + from . import config_sources + + return config_sources.detect( + config_file=meta.get("config_file"), + creds_file=meta.get("creds_file"), + profile=meta.get("profile"), + ) + + +def _options(): + from . import config + + return getattr(config.OPTIONS, "value", None) + + +def record_session(ctx): + """Record that ``initialise_session`` put the host and SSL options in opts.""" + _store(ctx)["session"] = ctx + + +def record_credential(ctx, credential): + """Record how the run logged in, by provider name only. + + A command that hands off (``list repos`` to ``repositories get``) + resolves credentials again where the env key is not passed on, so a + later ``None`` must not replace the login the run used. + """ + source = getattr(credential, "source_name", None) + if source: + _store(ctx)["auth_source"] = source + + +def record_config(ctx): + """Record that ``common_cli_config_options`` loaded the config files.""" + _store(ctx)["config"] = ctx + + +def _set_up_by_leaf(store, key): + # A group (e.g. `list`) loads config and sets up the session before its + # subcommand parses, so only work done by the leaf counts: in the leaf's + # own context, or in a sub-context below it when the leaf hands off to + # another command (`list repos` runs `repositories get` via ctx.forward). + contexts = store.get("contexts", []) + ctx = store.get(key) + while contexts and ctx is not None: + if ctx is contexts[-1]: + return True + ctx = ctx.parent + return False + + +def _session_resolved(store): + return _set_up_by_leaf(store, "session") + + +def _config_resolved(store): + return _set_up_by_leaf(store, "config") + + +def resolve_api_host(opts, *, resolved, typed=()): + """Return the API host the run targets, for the host gate (never sent).""" + if resolved: + return getattr(opts, "api_host", None) + if API_HOST_SPELLINGS & set(typed): + return UNRESOLVED_HOST + # The env var beats config.ini, as it does in initialise_session. + return os.environ.get(API_HOST_ENV) or getattr(opts, "api_host", None) + + +def resolve_ssl_verify(opts, typed=()): + """Tell whether SSL verification is on; any sign of it being off wins.""" + if getattr(opts, "ssl_verify", True) is False or SSL_OFF_SPELLINGS & set(typed): + return False + verify = os.environ.get(SSL_VERIFY_ENV, "").strip().lower() + without = os.environ.get(WITHOUT_SSL_VERIFY_ENV, "").strip().lower() + return verify not in _FALSE and without not in _TRUE + + +def _ssl_verify_strict(opts): + """The resolved ``--without-ssl-verify-strict`` setting, or None if unset. + + Set by ``initialise_session`` even for commands that never load the SDK + (``update``), whose Configuration the sender would otherwise fall back to. + """ + value = getattr(opts, "ssl_verify_strict", None) + return value if isinstance(value, bool) else None + + +def typed_flags(meta): + """The option spellings typed in this run, from its root ``ctx.meta``.""" + return sorted(set((meta or {}).get(META_KEY, {}).get("typed", []))) + + +def collect_facts(meta, *, outcome, exit_code, duration_ms, nested): + """Build the ``RunFacts`` for a run from its recorded context metadata.""" + from ..core.usage.event import RunFacts + + store = meta.get(META_KEY, {}) + contexts = store.get("contexts", []) + flags, envvars, sources = _param_facts(contexts) + opts = _options() + typed = typed_flags(meta) + credential = getattr(opts, "credential", None) + return RunFacts( + command=_command_path(contexts), + flags=flags, + flags_typed=typed, + envvars=envvars, + aliases_used=list(store.get("aliases", [])), + output_format=getattr(opts, "output", None), + # A typed --help only means help was shown if the run then succeeded: + # `whoami --no-such-flag --help` is still a usage error. + outcome="help" + if _HELP_SPELLINGS & set(typed) and outcome == "success" + else outcome, + exit_code=exit_code, + api_status=store.get("api_status"), + duration_ms=duration_ms, + setting_sources=sources, + auth_source=store.get("auth_source") + or getattr(credential, "source_name", None), + api_host=resolve_api_host(opts, resolved=_session_resolved(store), typed=typed), + ssl_verify=resolve_ssl_verify(opts, typed), + ssl_verify_strict=_ssl_verify_strict(opts), + stdout_tty=bool(sys.stdout and sys.stdout.isatty()), + stdin_tty=bool(sys.stdin and sys.stdin.isatty()), + nested=nested, + # A typed config file or profile that the leaf never loaded may hold + # an opt-out or a host this run cannot see: never send. + config_unresolved=bool(CONFIG_SPELLINGS & set(typed)) + and not _config_resolved(store), + # Reads the ini files, so it runs only once a run is known to send. + deferred_sources=lambda: _config_sources(meta), + ) diff --git a/cloudsmith_cli/core/usage/event.py b/cloudsmith_cli/core/usage/event.py index 0d7de3bb8..a8b1cc893 100644 --- a/cloudsmith_cli/core/usage/event.py +++ b/cloudsmith_cli/core/usage/event.py @@ -49,7 +49,15 @@ "oidc_org": "workspace", "config_file": "config_file_path", "credentials_file": "credentials_file_path", - "without_api_ssl_verify": "ssl_verify", + # The hidden options of deprecated spellings (``deprecation.alias_options``). + "deprecated_api_proxy": "proxy", + "deprecated_without_api_ssl_verify": "ssl_verify", + "deprecated_S": "ssl_verify", + "deprecated_org": "workspace", + "deprecated_organization": "workspace", + "deprecated_oidc_org": "workspace", + "deprecated_owner": "workspace", + "deprecated_o": "workspace", } SOURCE_VALUES = frozenset( { From b676ca0859f5b1c4e23eb5245ee6d68035393644 Mon Sep 17 00:00:00 2001 From: Bartosz Blizniak Date: Mon, 26 Oct 2026 11:50:32 +0000 Subject: [PATCH 2/5] feat(ENG-15367): add cloudsmith telemetry status, id and reset - telemetry status: shows whether usage telemetry would send and why not. - telemetry id: shows the install ID. - telemetry reset: deletes the install ID. Our release, binary and image workflows set CLOUDSMITH_NO_TELEMETRY so smoke tests send nothing. AGENTS.md gets the rules for working on usage telemetry. telemetry status applies typed flags (-S) the same way the send rules do. The macOS clean-room smoke test passes CLOUDSMITH_NO_TELEMETRY explicitly, since env -i drops the workflow's value. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/binaries.yml | 9 +- .github/workflows/dev-release.yml | 4 + .github/workflows/image-build.yml | 6 +- .github/workflows/release.yml | 2 + AGENTS.md | 8 + CHANGELOG.md | 5 + cloudsmith_cli/cli/commands/registry.py | 1 + cloudsmith_cli/cli/commands/telemetry.py | 93 +++++++++ .../tests/commands/test_telemetry_command.py | 179 ++++++++++++++++++ 9 files changed, 305 insertions(+), 2 deletions(-) create mode 100644 cloudsmith_cli/cli/commands/telemetry.py create mode 100644 cloudsmith_cli/cli/tests/commands/test_telemetry_command.py diff --git a/.github/workflows/binaries.yml b/.github/workflows/binaries.yml index 029de3e71..5b475e21a 100644 --- a/.github/workflows/binaries.yml +++ b/.github/workflows/binaries.yml @@ -48,6 +48,10 @@ concurrency: group: binaries-${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true +env: + # Smoke tests are development noise: keep them out of CLI telemetry. + CLOUDSMITH_NO_TELEMETRY: "1" + jobs: inputs: runs-on: ubuntu-24.04 @@ -324,6 +328,7 @@ jobs: docker run --rm \ -e ASSERT_NO_PYTHON="${ASSERT_NO_PYTHON}" \ -e EXPECTED_VERSION \ + -e CLOUDSMITH_NO_TELEMETRY \ -v "${PWD}:/w" -w /w \ "${IMAGE}" sh -c ' if [ "${ASSERT_NO_PYTHON}" = 1 ]; then @@ -343,6 +348,7 @@ jobs: docker run --rm \ -e CLOUDSMITH_API_KEY \ -e CLOUDSMITH_NAMESPACE \ + -e CLOUDSMITH_NO_TELEMETRY \ -v "${PWD}:/w" -w /w \ "${IMAGE}" sh -c ' sh packaging/smoketest.sh cloudsmith/cloudsmith online @@ -357,7 +363,8 @@ jobs: export EXPECTED_VERSION EXPECTED_VERSION=$(cat cloudsmith_cli/data/VERSION) tar -xzf "dl/${ART}" -C . - env -i HOME="${HOME}" TMPDIR="${TMPDIR:-/tmp}" \ + # env -i drops the workflow's CLOUDSMITH_NO_TELEMETRY; pass it again. + env -i HOME="${HOME}" TMPDIR="${TMPDIR:-/tmp}" CLOUDSMITH_NO_TELEMETRY=1 \ "${PWD}/cloudsmith/cloudsmith" --version sh packaging/smoketest.sh cloudsmith/cloudsmith offline if [ -n "${ONLINE}" ] && [ -n "${CLOUDSMITH_API_KEY:-}" ]; then diff --git a/.github/workflows/dev-release.yml b/.github/workflows/dev-release.yml index 4cda232e9..bfc50e469 100644 --- a/.github/workflows/dev-release.yml +++ b/.github/workflows/dev-release.yml @@ -13,6 +13,10 @@ concurrency: group: dev-release-${{ github.ref }} cancel-in-progress: true +env: + # Dev builds are not user usage: keep them out of CLI telemetry. + CLOUDSMITH_NO_TELEMETRY: "1" + jobs: binaries: name: Binaries diff --git a/.github/workflows/image-build.yml b/.github/workflows/image-build.yml index a4d1e22b4..0db58d8c1 100644 --- a/.github/workflows/image-build.yml +++ b/.github/workflows/image-build.yml @@ -13,6 +13,10 @@ concurrency: group: image-build-${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true +env: + # The test step runs the CLI: keep it out of CLI telemetry. + CLOUDSMITH_NO_TELEMETRY: "1" + jobs: image-build: name: Build container image @@ -46,4 +50,4 @@ jobs: --build-arg VCS_REF="${GITHUB_SHA}" \ . - name: Test image - run: docker run -t cloudsmith-io/cli-dev --help + run: docker run -t -e CLOUDSMITH_NO_TELEMETRY cloudsmith-io/cli-dev --help diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 731e21587..a627ef3a2 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -15,6 +15,8 @@ concurrency: env: # Single source of truth for binary targets (also update the binaries.yml matrix). BINARY_TARGETS: "linux-x86_64-gnu linux-x86_64-musl linux-aarch64-gnu linux-aarch64-musl macos-arm64 macos-x86_64 windows-x86_64" + # Release and smoke-test runs are not user usage: keep them out of CLI telemetry. + CLOUDSMITH_NO_TELEMETRY: "1" jobs: validate: diff --git a/AGENTS.md b/AGENTS.md index eb7ac4b7f..54315f4d2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -68,3 +68,11 @@ Tests live alongside code: `cloudsmith_cli/cli/tests/` and `cloudsmith_cli/core/ - `pyupgrade --py310-plus` runs on every commit, so use modern syntax (`X | Y` unions, `dict[str, ...]` generics, etc.). - Adding a new subcommand: create a module under `cli/commands/`, register it in `cli/commands/__init__.py`, and attach it to `main` (or a subgroup) with the `AliasGroup` for alias support. - Adding a new API call: put SDK wrapping in `core/api/.py`, translate exceptions to `ApiException`, and call it from the command layer. Avoid making fresh `cloudsmith_api` request calls from `cli/commands/*`; importing `cloudsmith_api` for `Configuration` defaults or exception types is OK and matches what `login`/`logout`/`check` already do. + +## Usage telemetry + +- **What it is for:** one pseudonymous `cli command executed` event per run, sent to PostHog so the team can see which commands, flags and setups are used; every property states the question it answers. +- **Rules:** only send names defined in code (commands, parameters, declared option spellings, env var names), enums, booleans and numbers, never a value from the user. Never send for credential helper invocations, `mcp start`, the `telemetry` group, shell completion, non-Cloudsmith API hosts, SSL verification off, development builds without the key, or any opt-out (`DO_NOT_TRACK`, `CLOUDSMITH_NO_TELEMETRY`, `telemetry = false`). +- **Adding a property:** declare it in `cloudsmith_cli/core/usage/event.py` (`PROPERTIES`, with the question it answers), fill it in `_values`, and extend `cloudsmith_cli/core/tests/test_usage_event.py`; new commands and flags need no telemetry change because capture reads the Click context. +- **Performance:** `test_startup_imports.py` fails if importing the CLI loads heavy modules (HTTP clients, `cloudsmith_api`, `rich`) or any command module, so the usage code imports its HTTP session inside functions that run at the end of a run; the wait is capped by the shared 1 s budget and failures stay silent. +- **Testing locally:** `CLOUDSMITH_TELEMETRY=log` prints the event instead of sending it, and a test project needs `CLOUDSMITH_TELEMETRY_POSTHOG_KEY` (plus `CLOUDSMITH_TELEMETRY_POSTHOG_HOST` for a non-default host), because development builds (source checkouts and editable installs) send nothing without that key. Use the `usage_events` pytest fixture in tests. diff --git a/CHANGELOG.md b/CHANGELOG.md index 38ed7fb3e..0d111d43c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Added `skills` to `cloudsmith upstream`. Use `cloudsmith upstream skills list`, `create`, `update`, and `delete` to manage AI Skills upstreams. An AI Skills upstream caches skills and plugins from a GitHub repository. - Added `--changelog` to `cloudsmith push skills`. Use it to add release notes to an AI Skills package. The option is available only for the `skills` format. - Added `--compress` to `cloudsmith push skills`. With this option, you can pass a skill folder instead of a `.zip` archive. The CLI compresses the contents of the folder into a `.zip` archive, then uploads it. If you pass a folder without `--compress`, the command fails with a usage error. The option is available only for the `skills` format. +- Added the `cloudsmith telemetry` command group for usage telemetry. The CLI does not send usage telemetry yet. + - `cloudsmith telemetry status` shows whether usage telemetry is on, and the reason. The reason can be an opt-out, such as `DO_NOT_TRACK`, `CLOUDSMITH_NO_TELEMETRY`, or `telemetry = false` in `config.ini`. It can also be an API host that is not a Cloudsmith host, or certificate verification that is off. + - `cloudsmith telemetry id` shows the install ID that the CLI stores in `telemetry.json` in its config directory, or `none`. + - `cloudsmith telemetry reset` deletes the stored install ID. The CLI creates a new, unrelated ID on the next command. + - All three commands support `-F json` and `-F pretty_json`. ### Changed diff --git a/cloudsmith_cli/cli/commands/registry.py b/cloudsmith_cli/cli/commands/registry.py index 1162905c2..513e9a93c 100644 --- a/cloudsmith_cli/cli/commands/registry.py +++ b/cloudsmith_cli/cli/commands/registry.py @@ -36,6 +36,7 @@ "resync": f"{_PACKAGE}.resync", "status": f"{_PACKAGE}.status", "tags": f"{_PACKAGE}.tags", + "telemetry": f"{_PACKAGE}.telemetry", "tokens": f"{_PACKAGE}.tokens", "update": f"{_PACKAGE}.update", "upstream": f"{_PACKAGE}.upstream", diff --git a/cloudsmith_cli/cli/commands/telemetry.py b/cloudsmith_cli/cli/commands/telemetry.py new file mode 100644 index 000000000..4255b2de1 --- /dev/null +++ b/cloudsmith_cli/cli/commands/telemetry.py @@ -0,0 +1,93 @@ +# Copyright 2026 Cloudsmith Ltd +"""CLI - Telemetry command group: see and control usage telemetry.""" + +import os + +import click + +from ...core import telemetry_common +from ...core.usage import identity, settings, state +from .. import command, decorators, usage_capture, utils +from .main import main + + +def _reason(opts, typed=()): + # Same gates, same order, as core/usage/run.py::_finish, so status never + # says "on" for a run that would send nothing. + for name in (telemetry_common.DO_NOT_TRACK_ENV, telemetry_common.NO_TELEMETRY_ENV): + if telemetry_common.telemetry_disabled({name: os.environ.get(name, "")}): + return name + if telemetry_common.config_disabled(): + return "config" + # status itself runs initialise_session, so opts holds the resolved host. + host = usage_capture.resolve_api_host(opts, resolved=True) + if not telemetry_common.is_cloudsmith_host(host): + return "non-Cloudsmith API host" + # Typed spellings too, as collect_facts does: `-S` disables sending even + # when a later `--ssl-verify` wins for the command itself. + if not usage_capture.resolve_ssl_verify(opts, typed): + return "SSL verification off" + if ( + telemetry_common.get_environment() == "development" + and not settings.override_set() + ): + return "development build" + if settings.log_mode(): + # run.py logs the payload to stderr and never sends in log mode. + return "log only" + if not settings.project_key(): + return "no endpoint key" + return "enabled" + + +@main.group(cls=command.AliasGroup) +def telemetry(): + """See and control pseudonymous usage telemetry.""" + + +@telemetry.command() +@decorators.common_cli_config_options +@decorators.common_cli_output_options +@decorators.initialise_session +@click.pass_context +def status(ctx, opts): + """Show whether usage telemetry is on, and why.""" + reason = _reason(opts, usage_capture.typed_flags(ctx.meta)) + data = { + "enabled": reason == "enabled", + "reason": reason, + "log_mode": settings.log_mode(), + "install_id_enabled": settings.install_id_enabled(), + "endpoint": settings.host(), + } + if not utils.maybe_print_as_json(opts, data): + click.echo(f"Usage telemetry: {'on' if data['enabled'] else 'off'} ({reason})") + click.echo(f"Log mode: {'on' if data['log_mode'] else 'off'}") + click.echo(f"Install ID: {'on' if data['install_id_enabled'] else 'off'}") + + +@telemetry.command(name="id") +@decorators.common_cli_config_options +@decorators.common_cli_output_options +@click.pass_context +def show_id(ctx, opts): # pylint: disable=unused-argument + """Show the install ID, if one is stored.""" + install_id = state.read().get("install_id") + if not utils.maybe_print_as_json(opts, {"install_id": install_id}): + click.echo(install_id or "none") + + +@telemetry.command() +@decorators.common_cli_config_options +@decorators.common_cli_output_options +@click.pass_context +def reset(ctx, opts): # pylint: disable=unused-argument + """Delete the install ID; a new one is created on the next command.""" + try: + identity.reset() + except identity.IdentityResetError as exc: + # Raised, not ctx.exit(1): the command's main() returns an Exit code + # instead of raising SystemExit, which CliRunner would report as 0. + raise click.ClickException(str(exc)) from exc + if not utils.maybe_print_as_json(opts, {"deleted": True}): + click.echo("Install ID deleted. A new one is created on the next command.") diff --git a/cloudsmith_cli/cli/tests/commands/test_telemetry_command.py b/cloudsmith_cli/cli/tests/commands/test_telemetry_command.py new file mode 100644 index 000000000..cc23bb8d3 --- /dev/null +++ b/cloudsmith_cli/cli/tests/commands/test_telemetry_command.py @@ -0,0 +1,179 @@ +import json + +import pytest +from click.testing import CliRunner + +from cloudsmith_cli.cli.commands.main import main +from cloudsmith_cli.core import telemetry_common +from cloudsmith_cli.core.usage import settings + + +@pytest.fixture(autouse=True) +def no_update_check(monkeypatch): + """``status`` starts the update check like any leaf command; keep it off.""" + monkeypatch.setenv("CLOUDSMITH_NO_UPDATE_CHECK", "1") + + +def test_status_reports_opt_out(monkeypatch): + monkeypatch.setenv("DO_NOT_TRACK", "1") + monkeypatch.setenv(settings.KEY_ENV, "phc_testsecret") + result = CliRunner().invoke(main, ["telemetry", "status", "-F", "json"]) + assert result.exit_code == 0 + data = json.loads(result.output)["data"] + assert data["enabled"] is False and data["reason"] == "DO_NOT_TRACK" + assert "phc_" not in result.output + + +def test_status_reports_log_only(monkeypatch, telemetry_config_dir): # pylint: disable=unused-argument + monkeypatch.setattr(telemetry_common, "get_environment", lambda: "production") + monkeypatch.setenv(settings.MODE_ENV, "log") + monkeypatch.delenv(settings.KEY_ENV, raising=False) + monkeypatch.delenv(telemetry_common.DO_NOT_TRACK_ENV, raising=False) + monkeypatch.delenv(telemetry_common.NO_TELEMETRY_ENV, raising=False) + result = CliRunner().invoke(main, ["telemetry", "status", "-F", "json"]) + assert result.exit_code == 0 + data = json.loads(result.output)["data"] + assert data["enabled"] is False + assert data["reason"] == "log only" + + +def test_status_never_prints_the_key(monkeypatch, telemetry_config_dir): # pylint: disable=unused-argument + monkeypatch.setattr(telemetry_common, "get_environment", lambda: "production") + monkeypatch.setenv(settings.KEY_ENV, "phc_testsecret") + monkeypatch.delenv(settings.MODE_ENV, raising=False) + monkeypatch.delenv(telemetry_common.DO_NOT_TRACK_ENV, raising=False) + monkeypatch.delenv(telemetry_common.NO_TELEMETRY_ENV, raising=False) + runner = CliRunner() + as_json = runner.invoke(main, ["telemetry", "status", "-F", "json"]) + assert as_json.exit_code == 0 + data = json.loads(as_json.output)["data"] + assert data["enabled"] is True and data["reason"] == "enabled" + assert "phc_testsecret" not in as_json.output + as_text = runner.invoke(main, ["telemetry", "status"]) + assert as_text.exit_code == 0 + assert "Usage telemetry: on (enabled)" in as_text.output + assert "phc_testsecret" not in as_text.output + + +def test_status_reports_non_cloudsmith_host(monkeypatch, telemetry_config_dir): # pylint: disable=unused-argument + monkeypatch.delenv(telemetry_common.DO_NOT_TRACK_ENV, raising=False) + monkeypatch.delenv(telemetry_common.NO_TELEMETRY_ENV, raising=False) + result = CliRunner().invoke( + main, + [ + "telemetry", + "status", + "-F", + "json", + "--api-host", + "https://packages.example.com", + ], + ) + assert result.exit_code == 0 + data = json.loads(result.output)["data"] + assert data["enabled"] is False + assert data["reason"] == "non-Cloudsmith API host" + + +def test_status_reports_ssl_off_from_env_like_the_gate( + monkeypatch, telemetry_config_dir +): # pylint: disable=unused-argument + # The run gate treats a falsy CLOUDSMITH_SSL_VERIFY as off even when + # --ssl-verify is typed, so status must say off too. + monkeypatch.delenv(telemetry_common.DO_NOT_TRACK_ENV, raising=False) + monkeypatch.delenv(telemetry_common.NO_TELEMETRY_ENV, raising=False) + monkeypatch.delenv("CLOUDSMITH_API_HOST", raising=False) + monkeypatch.setenv("CLOUDSMITH_SSL_VERIFY", "false") + result = CliRunner().invoke( + main, ["telemetry", "status", "-F", "json", "--ssl-verify"] + ) + assert result.exit_code == 0 + data = json.loads(result.output)["data"] + assert data["enabled"] is False + assert data["reason"] == "SSL verification off" + + +@pytest.mark.parametrize("ssl_off", ["-S", "--without-api-ssl-verify"]) +def test_status_reports_a_typed_ssl_off_flag_like_the_gate( + monkeypatch, telemetry_config_dir, ssl_off +): # pylint: disable=unused-argument + # `--ssl-verify -S`: the command resolves verification on, but the run + # gate sees the typed -S and sends nothing, so status must say off. + monkeypatch.setattr(telemetry_common, "get_environment", lambda: "production") + monkeypatch.setenv(settings.KEY_ENV, "phc_test") + monkeypatch.delenv(settings.MODE_ENV, raising=False) + monkeypatch.delenv(telemetry_common.DO_NOT_TRACK_ENV, raising=False) + monkeypatch.delenv(telemetry_common.NO_TELEMETRY_ENV, raising=False) + monkeypatch.delenv("CLOUDSMITH_API_HOST", raising=False) + monkeypatch.delenv("CLOUDSMITH_SSL_VERIFY", raising=False) + monkeypatch.delenv("CLOUDSMITH_WITHOUT_API_SSL_VERIFY", raising=False) + result = CliRunner().invoke( + main, ["telemetry", "status", "-F", "json", "--ssl-verify", ssl_off] + ) + assert result.exit_code == 0, result.output + # The deprecated spelling also prints a warning, on stderr. + data = json.loads(result.stdout)["data"] + assert data["enabled"] is False + assert data["reason"] == "SSL verification off" + + +def test_status_reports_development_build(monkeypatch, telemetry_config_dir): # pylint: disable=unused-argument + monkeypatch.setattr(telemetry_common, "get_environment", lambda: "development") + monkeypatch.delenv(telemetry_common.DO_NOT_TRACK_ENV, raising=False) + monkeypatch.delenv(telemetry_common.NO_TELEMETRY_ENV, raising=False) + monkeypatch.delenv(settings.KEY_ENV, raising=False) + monkeypatch.delenv(settings.HOST_ENV, raising=False) + result = CliRunner().invoke(main, ["telemetry", "status", "-F", "json"]) + assert result.exit_code == 0 + data = json.loads(result.output)["data"] + assert data["enabled"] is False + assert data["reason"] == "development build" + + +def test_id_and_reset(monkeypatch, usage_state_dir): + from cloudsmith_cli.core.usage import identity + + created = identity.resolve(ephemeral=False, install_id_enabled=True).distinct_id + runner = CliRunner() + shown = json.loads(runner.invoke(main, ["telemetry", "id", "-F", "json"]).output)[ + "data" + ] + assert shown["install_id"] == created + reset = json.loads( + runner.invoke(main, ["telemetry", "reset", "-F", "json"]).output + )["data"] + assert reset == {"deleted": True} + shown = json.loads(runner.invoke(main, ["telemetry", "id", "-F", "json"]).output)[ + "data" + ] + assert shown["install_id"] is None + + +def test_reset_failure_exits_non_zero(monkeypatch, usage_state_dir): + from cloudsmith_cli.core.usage import identity + + def fail(): + raise identity.IdentityResetError("could not remove the stored install ID") + + monkeypatch.setattr(identity, "reset", fail) + result = CliRunner().invoke(main, ["telemetry", "reset"]) + assert result.exit_code == 1 + assert "could not remove the stored install ID" in result.output + + +@pytest.mark.parametrize("via_env", [False, True]) +def test_status_reports_a_config_file_opt_out( + monkeypatch, telemetry_config_dir, tmp_path, via_env +): # pylint: disable=unused-argument + monkeypatch.delenv(telemetry_common.DO_NOT_TRACK_ENV, raising=False) + monkeypatch.delenv(telemetry_common.NO_TELEMETRY_ENV, raising=False) + path = tmp_path / "optout.ini" + path.write_text("[default]\ntelemetry = false\n") + args = ["telemetry", "status", "-F", "json"] + if via_env: + monkeypatch.setenv(telemetry_common.CONFIG_FILE_ENV, str(path)) + else: + args += ["--config-file", str(path)] + result = CliRunner().invoke(main, args) + assert result.exit_code == 0 + assert json.loads(result.output)["data"]["reason"] == "config" From e96204dc7b0fa4f2e3f31bb502f538362e3d1ae3 Mon Sep 17 00:00:00 2001 From: Bartosz Blizniak Date: Mon, 26 Oct 2026 11:51:11 +0000 Subject: [PATCH 3/5] feat(ENG-15367): label runs made by AI coding agents A run gets persona "agent" when the environment shows an AI coding agent ran the command. Only markers that agents set for the commands they run are read (Claude Code, Codex CLI, Gemini CLI, Copilot, then AI_AGENT); markers that IDEs also set in a person's own terminal are ignored. A new "agent" property carries a name defined in code, never the variable's value, and unknown AI_AGENT values become "unidentified_agent". Agent wins the persona even in CI; ci and ci_provider stay as they are. Co-Authored-By: Claude Opus 5.5 --- cloudsmith_cli/cli/tests/test_usage_main.py | 17 +++++++ cloudsmith_cli/conftest.py | 28 +++++++----- .../core/tests/test_usage_environment.py | 45 +++++++++++++++++++ cloudsmith_cli/core/tests/test_usage_event.py | 18 +++++++- cloudsmith_cli/core/usage/environment.py | 43 +++++++++++++++++- cloudsmith_cli/core/usage/event.py | 9 +++- cloudsmith_cli/core/usage/run.py | 1 + 7 files changed, 147 insertions(+), 14 deletions(-) diff --git a/cloudsmith_cli/cli/tests/test_usage_main.py b/cloudsmith_cli/cli/tests/test_usage_main.py index 755b4bc4e..0a451f9f7 100644 --- a/cloudsmith_cli/cli/tests/test_usage_main.py +++ b/cloudsmith_cli/cli/tests/test_usage_main.py @@ -17,6 +17,23 @@ def test_version_sends_success(usage_events, notice_seen): assert usage_events[-1]["properties"]["command"] == "" +def test_run_without_agent_markers_is_human(usage_events, notice_seen): + main(args=["--version"], standalone_mode=False) + props = usage_events[-1]["properties"] + assert props["persona"] == "human" and "agent" not in props + + +def test_agent_run_sends_a_name_never_the_raw_value( + usage_events, notice_seen, monkeypatch +): + monkeypatch.setenv("AI_AGENT", "acme-internal-bot_9") + main(args=["--version"], standalone_mode=False) + props = usage_events[-1]["properties"] + assert props["persona"] == "agent" + assert props["agent"] == "unidentified_agent" + assert "acme" not in json.dumps(usage_events[-1]) + + def test_help_is_recorded_as_help(usage_events, notice_seen): # Click returns 0 for --help when standalone_mode is False. assert main(args=["repositories", "--help"], standalone_mode=False) == 0 diff --git a/cloudsmith_cli/conftest.py b/cloudsmith_cli/conftest.py index d99ca9b99..4d5bd2c48 100644 --- a/cloudsmith_cli/conftest.py +++ b/cloudsmith_cli/conftest.py @@ -155,16 +155,24 @@ def usage_events(monkeypatch, telemetry_config_dir, blocked_usage_posts): # pyl ci_names = [name for _, names in environment._CI_PROVIDERS for name in names] + [ "KUBERNETES_SERVICE_HOST" ] - for name in ci_names + [ - "CLOUDSMITH_TELEMETRY", - "CLOUDSMITH_TELEMETRY_CI_SAMPLE_RATE", - "CLOUDSMITH_TELEMETRY_INSTALL_ID", - "CLOUDSMITH_TELEMETRY_POSTHOG_HOST", - "CLOUDSMITH_TELEMETRY_INTERNAL", - "CLOUDSMITH_API_HOST", - "CLOUDSMITH_SSL_VERIFY", - "CLOUDSMITH_WITHOUT_API_SSL_VERIFY", - ]: + # Tests may themselves run under an AI agent, which sets these. + agent_names = [ + name for _, names in environment._AGENT_MARKERS for name in names + ] + [environment.AI_AGENT_ENV] + for name in ( + ci_names + + agent_names + + [ + "CLOUDSMITH_TELEMETRY", + "CLOUDSMITH_TELEMETRY_CI_SAMPLE_RATE", + "CLOUDSMITH_TELEMETRY_INSTALL_ID", + "CLOUDSMITH_TELEMETRY_POSTHOG_HOST", + "CLOUDSMITH_TELEMETRY_INTERNAL", + "CLOUDSMITH_API_HOST", + "CLOUDSMITH_SSL_VERIFY", + "CLOUDSMITH_WITHOUT_API_SSL_VERIFY", + ] + ): monkeypatch.delenv(name, raising=False) monkeypatch.setenv("CLOUDSMITH_TELEMETRY_POSTHOG_KEY", "phc_test") # Later tests that call the real main() must never run the update check's diff --git a/cloudsmith_cli/core/tests/test_usage_environment.py b/cloudsmith_cli/core/tests/test_usage_environment.py index b99ed89b3..71044eb1c 100644 --- a/cloudsmith_cli/core/tests/test_usage_environment.py +++ b/cloudsmith_cli/core/tests/test_usage_environment.py @@ -36,6 +36,51 @@ def test_non_tty_alone_is_not_ci(): assert environment.persona(ci=True) == "ci" +@pytest.mark.parametrize( + "env,expected", + [ + ({}, None), + ({"CLAUDE_CODE_CHILD_SESSION": "1"}, "claude_code"), + ({"CODEX_THREAD_ID": "t-1"}, "codex"), + ({"GEMINI_CLI": "1"}, "gemini_cli"), + ({"COPILOT_AGENT": "1"}, "copilot"), + ({"COPILOT_CLI": "1"}, "copilot"), + ({"COPILOT_AGENT_SESSION_ID": "s-1"}, "copilot"), + ({"AI_AGENT": "claude-code_2-1-308_agent"}, "claude_code"), + ({"AI_AGENT": "github_copilot_vscode_agent"}, "copilot"), + ({"AI_AGENT": "opencode"}, "opencode"), + ({"AI_AGENT": "acme-internal-bot_9"}, "unidentified_agent"), + ({"AI_AGENT": ""}, None), + ({"CLAUDE_CODE_CHILD_SESSION": ""}, None), + ], +) +def test_agent_detection(env, expected): + assert environment.agent(env) == expected + + +def test_markers_that_ides_set_for_people_are_ignored(): + # These are also set in a person's own IDE terminal. + env = { + "CLAUDECODE": "1", + "CURSOR_TRACE_ID": "x", + "REPL_ID": "x", + "TERM_PROGRAM": "vscode", + "COPILOT_MODEL": "x", + } + assert environment.agent(env) is None + + +def test_vendor_marker_beats_ai_agent(): + env = {"CODEX_THREAD_ID": "t-1", "AI_AGENT": "claude-code_1_agent"} + assert environment.agent(env) == "codex" + + +def test_agent_wins_persona_even_in_ci(): + assert environment.persona(ci=True, agent="copilot") == "agent" + assert environment.persona(ci=False, agent="claude_code") == "agent" + assert environment.persona(ci=True, agent=None) == "ci" + + def test_runtime_properties_truncate_versions(monkeypatch): monkeypatch.setattr(environment.platform, "system", lambda: "Linux") monkeypatch.setattr(environment.platform, "release", lambda: "6.8.0-1070-azure") diff --git a/cloudsmith_cli/core/tests/test_usage_event.py b/cloudsmith_cli/core/tests/test_usage_event.py index 5bb2f6fc3..914638370 100644 --- a/cloudsmith_cli/core/tests/test_usage_event.py +++ b/cloudsmith_cli/core/tests/test_usage_event.py @@ -30,7 +30,7 @@ def _facts(**overrides): return event.RunFacts(**base) -def _build(facts, anonymous=True): +def _build(facts, anonymous=True, ci_provider=None, agent=None): ident = Identity( "id-1", anonymous=anonymous, first_run=False, persisted=not anonymous ) @@ -46,13 +46,27 @@ def _build(facts, anonymous=True): channel="pip", environment="production", cli_version="1.31.0", - ci_provider=None, + ci_provider=ci_provider, + agent=agent, sample_rate=100, internal=False, timestamp="2026-10-23T12:00:00+00:00", ) +def test_agent_run_in_ci_keeps_both(): + props = _build(_facts(), ci_provider="github_actions", agent="copilot")[ + "properties" + ] + assert props["persona"] == "agent" + assert props["agent"] == "copilot" + assert props["ci"] is True and props["ci_provider"] == "github_actions" + + +def test_no_agent_property_without_an_agent(): + assert "agent" not in _build(_facts())["properties"] + + def test_only_declared_properties_are_sent(): declared = {p.name for p in event.PROPERTIES} props = _build(_facts())["properties"] diff --git a/cloudsmith_cli/core/usage/environment.py b/cloudsmith_cli/core/usage/environment.py index ca7230df9..bd0689a29 100644 --- a/cloudsmith_cli/core/usage/environment.py +++ b/cloudsmith_cli/core/usage/environment.py @@ -24,6 +24,30 @@ ) +#: (agent, variables any of which marks it), checked in order. Only markers +#: an agent sets for the commands it runs: variables that IDEs also set in a +#: person's own terminal (``CLAUDECODE``, ``CURSOR_TRACE_ID``, ``REPL_ID``, +#: ``TERM_PROGRAM``) would label people as agents. +_AGENT_MARKERS = ( + ("claude_code", ("CLAUDE_CODE_CHILD_SESSION",)), + ("codex", ("CODEX_THREAD_ID",)), + ("gemini_cli", ("GEMINI_CLI",)), + ("copilot", ("COPILOT_AGENT", "COPILOT_CLI", "COPILOT_AGENT_SESSION_ID")), +) +#: The cross-vendor convention, read after the vendor markers. +AI_AGENT_ENV = "AI_AGENT" +#: Known ``AI_AGENT`` value prefixes; any other value is counted as +#: ``UNIDENTIFIED_AGENT`` and never sent. +_AI_AGENT_PREFIXES = ( + ("claude-code", "claude_code"), + ("github_copilot", "copilot"), + ("codex", "codex"), + ("gemini", "gemini_cli"), + ("opencode", "opencode"), +) +UNIDENTIFIED_AGENT = "unidentified_agent" + + def ci_provider(env=None): env = os.environ if env is None else env for provider, names in _CI_PROVIDERS: @@ -47,7 +71,24 @@ def is_ephemeral(*, env=None, channel, state_writable): return is_container() -def persona(*, ci): +def agent(env=None): + """Name the AI agent that ran this command, from code-defined names only.""" + env = os.environ if env is None else env + for name, markers in _AGENT_MARKERS: + if any(env.get(marker) for marker in markers): + return name + declared = env.get(AI_AGENT_ENV, "").strip().lower() + if not declared: + return None + for prefix, name in _AI_AGENT_PREFIXES: + if declared.startswith(prefix): + return name + return UNIDENTIFIED_AGENT + + +def persona(*, ci, agent=None): # pylint: disable=redefined-outer-name + if agent: + return "agent" return "ci" if ci else "human" diff --git a/cloudsmith_cli/core/usage/event.py b/cloudsmith_cli/core/usage/event.py index a8b1cc893..b017d352b 100644 --- a/cloudsmith_cli/core/usage/event.py +++ b/cloudsmith_cli/core/usage/event.py @@ -9,6 +9,8 @@ from collections.abc import Callable from dataclasses import dataclass, field +from .environment import persona + EVENT_NAME = "cli command executed" SCHEMA_VERSION = 1 @@ -129,6 +131,7 @@ class Property: Property("python_version", str, "support policy"), Property("arch", str, "dimension"), Property("persona", str, "persona"), + Property("agent", str, "persona"), Property("ci", bool, "persona"), Property("ci_provider", str, "persona"), Property("stdout_tty", bool, "persona"), @@ -163,6 +166,7 @@ def _values( environment, cli_version, ci_provider, + agent, sample_rate, internal, ): @@ -181,7 +185,8 @@ def _values( "cli_version": cli_version, "channel": channel, "environment": environment, - "persona": "ci" if ci_provider else "human", + "persona": persona(ci=ci_provider is not None, agent=agent), + "agent": agent, "ci": ci_provider is not None, "ci_provider": ci_provider, "stdout_tty": facts.stdout_tty, @@ -206,6 +211,7 @@ def build_event( environment, cli_version, ci_provider, + agent, sample_rate, internal, timestamp, @@ -218,6 +224,7 @@ def build_event( environment, cli_version, ci_provider, + agent, sample_rate, internal, ) diff --git a/cloudsmith_cli/core/usage/run.py b/cloudsmith_cli/core/usage/run.py index 2ba616b41..a3dc113d5 100644 --- a/cloudsmith_cli/core/usage/run.py +++ b/cloudsmith_cli/core/usage/run.py @@ -157,6 +157,7 @@ def _finish(facts): # pylint: disable=too-many-return-statements,too-many-local environment=env_name, cli_version=version.get_version(), ci_provider=provider, + agent=environment.agent(), sample_rate=sample_rate, internal=settings.internal(), timestamp=datetime.now(UTC).isoformat(), From 3986f10aee799324b0250b45cacc4c229ca34ec5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tighearn=C3=A1n=20Carroll?= <13870403+tigh-latte@users.noreply.github.com> Date: Tue, 3 Nov 2026 10:46:48 +0000 Subject: [PATCH 4/5] feat(ENG-15367): let commands choose what telemetry they send A command declares what it may send with @telemetry.send(...) from cloudsmith_cli/cli/telemetry.py: NONE, ERRORS (Sentry reports only), USAGE (usage events only) or ALL, the default. A policy on a group applies to every command under it unless a command sets its own, and it only narrows what is sent: the opt-outs always win. This replaces the list of excluded command names in core/usage/run.py. Credential helpers, mcp start and the telemetry group use ERRORS, so they still report crashes and send no usage events, as before. A test lists every command that sends less than ALL, so a change of policy is always visible in review. CLOUDSMITH_TELEMETRY=log now covers error reports too: the scrubbed Sentry event is printed to stderr instead of being sent, even without a DSN. Before, log mode only showed usage events and error reports were still sent. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 5 +- CHANGELOG.md | 1 + cloudsmith_cli/cli/command.py | 19 +++- .../cli/commands/credential_helper/cargo.py | 3 + .../cli/commands/credential_helper/docker.py | 3 + .../cli/commands/credential_helper/generic.py | 3 + .../cli/commands/credential_helper/pnpm.py | 3 + .../commands/credential_helper/terraform.py | 3 + cloudsmith_cli/cli/commands/mcp.py | 4 +- cloudsmith_cli/cli/commands/telemetry.py | 5 ++ cloudsmith_cli/cli/telemetry.py | 23 +++++ .../cli/tests/test_unexpected_errors.py | 75 ++++++++++++++++ .../cli/tests/test_usage_capture.py | 88 +++++++++++++++++++ cloudsmith_cli/cli/usage_capture.py | 15 ++++ cloudsmith_cli/core/telemetry.py | 34 ++++++- cloudsmith_cli/core/telemetry_common.py | 66 ++++++++++++++ .../core/tests/test_telemetry_common.py | 73 +++++++++++++++ cloudsmith_cli/core/tests/test_usage_run.py | 24 ++--- cloudsmith_cli/core/usage/event.py | 3 + cloudsmith_cli/core/usage/run.py | 28 ++---- cloudsmith_cli/core/usage/settings.py | 7 +- 21 files changed, 437 insertions(+), 48 deletions(-) create mode 100644 cloudsmith_cli/cli/telemetry.py diff --git a/AGENTS.md b/AGENTS.md index 54315f4d2..ff6e35ed7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -72,7 +72,8 @@ Tests live alongside code: `cloudsmith_cli/cli/tests/` and `cloudsmith_cli/core/ ## Usage telemetry - **What it is for:** one pseudonymous `cli command executed` event per run, sent to PostHog so the team can see which commands, flags and setups are used; every property states the question it answers. -- **Rules:** only send names defined in code (commands, parameters, declared option spellings, env var names), enums, booleans and numbers, never a value from the user. Never send for credential helper invocations, `mcp start`, the `telemetry` group, shell completion, non-Cloudsmith API hosts, SSL verification off, development builds without the key, or any opt-out (`DO_NOT_TRACK`, `CLOUDSMITH_NO_TELEMETRY`, `telemetry = false`). +- **Rules:** only send names defined in code (commands, parameters, declared option spellings, env var names), enums, booleans and numbers, never a value from the user. Never send for shell completion, non-Cloudsmith API hosts, SSL verification off, development builds without the key, or any opt-out (`DO_NOT_TRACK`, `CLOUDSMITH_NO_TELEMETRY`, `telemetry = false`). +- **Per-command policy:** a command (or a group and everything under it) that should send less than everything declares it with `@telemetry.send(telemetry.NONE | ERRORS | USAGE)` from `cloudsmith_cli/cli/telemetry.py`, put directly under its `@.command()` line; `ALL` (Sentry error reports and usage events) is the default and the nearest policy wins. Credential helpers, `mcp start` and the `telemetry` group use `ERRORS`. A policy only narrows what is sent: the opt-outs always win. `test_every_command_that_sends_less_than_all_is_listed_here` in `cli/tests/test_usage_capture.py` lists every such command, so update it in the same change. - **Adding a property:** declare it in `cloudsmith_cli/core/usage/event.py` (`PROPERTIES`, with the question it answers), fill it in `_values`, and extend `cloudsmith_cli/core/tests/test_usage_event.py`; new commands and flags need no telemetry change because capture reads the Click context. - **Performance:** `test_startup_imports.py` fails if importing the CLI loads heavy modules (HTTP clients, `cloudsmith_api`, `rich`) or any command module, so the usage code imports its HTTP session inside functions that run at the end of a run; the wait is capped by the shared 1 s budget and failures stay silent. -- **Testing locally:** `CLOUDSMITH_TELEMETRY=log` prints the event instead of sending it, and a test project needs `CLOUDSMITH_TELEMETRY_POSTHOG_KEY` (plus `CLOUDSMITH_TELEMETRY_POSTHOG_HOST` for a non-default host), because development builds (source checkouts and editable installs) send nothing without that key. Use the `usage_events` pytest fixture in tests. +- **Testing locally:** `CLOUDSMITH_TELEMETRY=log` prints the usage event and any Sentry error report (scrubbed) instead of sending them, and a test project needs `CLOUDSMITH_TELEMETRY_POSTHOG_KEY` (plus `CLOUDSMITH_TELEMETRY_POSTHOG_HOST` for a non-default host), because development builds (source checkouts and editable installs) send nothing without that key. Use the `usage_events` pytest fixture in tests. diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d111d43c..2b1a42de0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - `cloudsmith telemetry id` shows the install ID that the CLI stores in `telemetry.json` in its config directory, or `none`. - `cloudsmith telemetry reset` deletes the stored install ID. The CLI creates a new, unrelated ID on the next command. - All three commands support `-F json` and `-F pretty_json`. +- Added `CLOUDSMITH_TELEMETRY=log` to see what the CLI would send. When a command fails with an unexpected error, the CLI writes the error report to stderr after it removes the private data, and it does not send the report. The CLI also writes the usage event that it would send. The opt-outs still apply: with `DO_NOT_TRACK`, `CLOUDSMITH_NO_TELEMETRY`, or `telemetry = false`, the CLI writes nothing. ### Changed diff --git a/cloudsmith_cli/cli/command.py b/cloudsmith_cli/cli/command.py index 4e180be49..5c8d96f4c 100644 --- a/cloudsmith_cli/cli/command.py +++ b/cloudsmith_cli/cli/command.py @@ -278,7 +278,8 @@ def main(self, *args, **kwargs): from ..core import telemetry # Reporting is silent: the user sees the same line either way. - telemetry.report_exception(e) + if _errors_allowed(): + telemetry.report_exception(e) click.echo(f"Error: {telemetry.format_exception_summary(e)}", err=True) _echo_certificate_hint(e) sys.exit(1) @@ -287,6 +288,22 @@ def main(self, *args, **kwargs): usage_capture.exit_run() +def _errors_allowed(): + """Whether the command's ``@telemetry.send`` policy allows error reports. + + The contexts are still recorded here (they are cleared in ``finally``). + """ + try: + from ..core.telemetry_common import Send + from . import usage_capture + + return Send.ERRORS in usage_capture.current_policy() + except Exception: # pylint: disable=broad-exception-caught + # A policy narrows reporting; if it cannot be read, fall back to the + # default (the user's opt-outs are still checked by the reporter). + return True + + def _finish_usage(usage, started, *, nested): """Hand the run to usage telemetry; never raises, never prints.""" try: diff --git a/cloudsmith_cli/cli/commands/credential_helper/cargo.py b/cloudsmith_cli/cli/commands/credential_helper/cargo.py index 74452eeea..5def42020 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/cargo.py +++ b/cloudsmith_cli/cli/commands/credential_helper/cargo.py @@ -12,11 +12,14 @@ import click from ....credential_helpers.cargo import execute +from ... import telemetry from ...decorators import common_api_auth_options, resolve_credentials from ...usage_capture import TrackedCommand +# Run by package managers on every request: report crashes, not usage. @click.command(cls=TrackedCommand, context_settings={"ignore_unknown_options": True}) +@telemetry.send(telemetry.ERRORS) @click.option( "--cargo-plugin", is_flag=True, diff --git a/cloudsmith_cli/cli/commands/credential_helper/docker.py b/cloudsmith_cli/cli/commands/credential_helper/docker.py index 60b21e619..558f21c32 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/docker.py +++ b/cloudsmith_cli/cli/commands/credential_helper/docker.py @@ -12,11 +12,14 @@ import click from ....credential_helpers.docker import execute +from ... import telemetry from ...decorators import common_api_auth_options, resolve_credentials from ...usage_capture import TrackedCommand +# Run by package managers on every request: report crashes, not usage. @click.command(cls=TrackedCommand) +@telemetry.send(telemetry.ERRORS) @click.argument("operation", required=False, default="get") @common_api_auth_options @resolve_credentials diff --git a/cloudsmith_cli/cli/commands/credential_helper/generic.py b/cloudsmith_cli/cli/commands/credential_helper/generic.py index c01722750..a97d860bb 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/generic.py +++ b/cloudsmith_cli/cli/commands/credential_helper/generic.py @@ -10,6 +10,7 @@ import click from ....credential_helpers.generic import execute +from ... import telemetry from ...decorators import ( common_api_auth_options, common_cli_config_options, @@ -18,7 +19,9 @@ from ...usage_capture import TrackedCommand +# Run by package managers on every request: report crashes, not usage. @click.command(cls=TrackedCommand) +@telemetry.send(telemetry.ERRORS) @common_cli_config_options @common_api_auth_options @resolve_credentials diff --git a/cloudsmith_cli/cli/commands/credential_helper/pnpm.py b/cloudsmith_cli/cli/commands/credential_helper/pnpm.py index 39e10963a..5c18d45a0 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/pnpm.py +++ b/cloudsmith_cli/cli/commands/credential_helper/pnpm.py @@ -11,12 +11,15 @@ import click from ....credential_helpers.pnpm import execute +from ... import telemetry from ...config import Options from ...decorators import common_api_auth_options, resolve_credentials from ...usage_capture import TrackedCommand +# Run by package managers on every request: report crashes, not usage. @click.command(cls=TrackedCommand) +@telemetry.send(telemetry.ERRORS) @click.argument("repo", required=False, default="npm.cloudsmith.io") @common_api_auth_options @resolve_credentials diff --git a/cloudsmith_cli/cli/commands/credential_helper/terraform.py b/cloudsmith_cli/cli/commands/credential_helper/terraform.py index c1f75dfaf..fefa3ba76 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/terraform.py +++ b/cloudsmith_cli/cli/commands/credential_helper/terraform.py @@ -14,6 +14,7 @@ import click from ....credential_helpers.terraform import execute +from ... import telemetry from ...decorators import ( common_api_auth_options, common_cli_config_options, @@ -22,7 +23,9 @@ from ...usage_capture import TrackedCommand +# Run by package managers on every request: report crashes, not usage. @click.command(cls=TrackedCommand, context_settings={"ignore_unknown_options": True}) +@telemetry.send(telemetry.ERRORS) @click.option( "-r", "--repo", diff --git a/cloudsmith_cli/cli/commands/mcp.py b/cloudsmith_cli/cli/commands/mcp.py index 8b16375a8..fb3782714 100644 --- a/cloudsmith_cli/cli/commands/mcp.py +++ b/cloudsmith_cli/cli/commands/mcp.py @@ -18,7 +18,7 @@ from ...core.mcp.data import OpenAPITool from ..config import Options -from .. import command, decorators, utils +from .. import command, decorators, telemetry, utils from ..exceptions import handle_api_exceptions from .main import main @@ -48,6 +48,8 @@ def mcp_(ctx, opts): # pylint: disable=unused-argument @mcp_.command(name="start") +# Long-running: report crashes, not usage. +@telemetry.send(telemetry.ERRORS) @decorators.initialise_api @decorators.initialise_mcp @click.pass_context diff --git a/cloudsmith_cli/cli/commands/telemetry.py b/cloudsmith_cli/cli/commands/telemetry.py index 4255b2de1..9856600ce 100644 --- a/cloudsmith_cli/cli/commands/telemetry.py +++ b/cloudsmith_cli/cli/commands/telemetry.py @@ -8,6 +8,9 @@ from ...core import telemetry_common from ...core.usage import identity, settings, state from .. import command, decorators, usage_capture, utils + +# Not `from .. import telemetry`: this module defines the `telemetry` group. +from ..telemetry import ERRORS, send from .main import main @@ -41,6 +44,8 @@ def _reason(opts, typed=()): @main.group(cls=command.AliasGroup) +# Manages telemetry itself: report crashes, never usage. +@send(ERRORS) def telemetry(): """See and control pseudonymous usage telemetry.""" diff --git a/cloudsmith_cli/cli/telemetry.py b/cloudsmith_cli/cli/telemetry.py new file mode 100644 index 000000000..ede9c1c6c --- /dev/null +++ b/cloudsmith_cli/cli/telemetry.py @@ -0,0 +1,23 @@ +# Copyright 2026 Cloudsmith Ltd +"""What a command may send: ``@telemetry.send(telemetry.ERRORS)``. + +``ALL`` (error reports and usage events) is the default, so only commands +that send less need the decorator: + +* ``NONE``: nothing. +* ``ERRORS``: Sentry reports of unexpected exceptions only. +* ``USAGE``: the usage event only. + +A policy on a group applies to every command under it unless a command sets +its own. The user's opt-outs always win. Imported by command modules, so it +must stay light: only the stdlib-only ``core.telemetry_common``. +""" + +from ..core.telemetry_common import Send, send + +NONE = Send.NONE +ERRORS = Send.ERRORS +USAGE = Send.USAGE +ALL = Send.ALL + +__all__ = ["ALL", "ERRORS", "NONE", "USAGE", "Send", "send"] diff --git a/cloudsmith_cli/cli/tests/test_unexpected_errors.py b/cloudsmith_cli/cli/tests/test_unexpected_errors.py index 4794f700e..6f4f64df3 100644 --- a/cloudsmith_cli/cli/tests/test_unexpected_errors.py +++ b/cloudsmith_cli/cli/tests/test_unexpected_errors.py @@ -493,3 +493,78 @@ def test_python_m_exits_1_on_unexpected_exception(tmp_path, monkeypatch, capsys) assert exc_info.value.code == 1 assert "Error: RuntimeError: boom" in capsys.readouterr().err + + +# --- Per-command policy (@telemetry.send) and log mode ---------------------- + + +def _crashing_app(): + from cloudsmith_cli.cli import telemetry as policy + from cloudsmith_cli.cli.command import AliasGroup + + @click.group(cls=AliasGroup) + def app(): + pass + + def crash(): + raise RuntimeError("boom") + + for name, level in [ + ("default", None), + ("errors", policy.ERRORS), + ("usage", policy.USAGE), + ("none", policy.NONE), + ]: + callback = crash if level is None else policy.send(level)(lambda: crash()) + app.command(name=name)(callback) + return app + + +@pytest.mark.parametrize( + "command, reported", [("default", 1), ("errors", 1), ("usage", 0), ("none", 0)] +) +def test_only_commands_whose_policy_has_errors_report( + runner, sentry_events, command, reported +): + result = runner.invoke(_crashing_app(), [command]) + # The user sees the same thing either way. + assert result.exit_code == 1 + assert result.stderr.strip().endswith("Error: RuntimeError: boom") + assert len(sentry_events) == reported + + +def test_log_mode_prints_the_scrubbed_report_and_sends_nothing( + runner, tmp_path, sentry_events, monkeypatch +): + monkeypatch.setenv("CLOUDSMITH_TELEMETRY", "log") + with whoami_raises(RuntimeError("boom")): + result = runner.invoke(main, whoami_args(tmp_path)) + + assert result.exit_code == 1 + assert sentry_events == [] + assert "would send error report (log mode, nothing sent)" in result.stderr + printed = result.stderr.split("nothing sent):\n", 1)[1] + logged, _ = json.JSONDecoder().raw_decode(printed) + assert logged["tags"]["command"] == "whoami" + assert logged["exception"]["values"][-1]["type"] == "RuntimeError" + # Scrubbed exactly as a sent report would be. + assert API_KEY not in result.stderr + + +def test_log_mode_prints_without_a_dsn(runner, tmp_path, sentry_events, monkeypatch): + monkeypatch.setenv("CLOUDSMITH_TELEMETRY", "log") + monkeypatch.setenv(telemetry.DSN_ENV, "") + with whoami_raises(RuntimeError("boom")): + result = runner.invoke(main, whoami_args(tmp_path)) + assert "would send error report" in result.stderr + + +def test_log_mode_still_honours_the_opt_out( + runner, tmp_path, sentry_events, monkeypatch +): + monkeypatch.setenv("CLOUDSMITH_TELEMETRY", "log") + monkeypatch.setenv("DO_NOT_TRACK", "1") + with whoami_raises(RuntimeError("boom")): + result = runner.invoke(main, whoami_args(tmp_path)) + assert "would send error report" not in result.stderr + assert sentry_events == [] diff --git a/cloudsmith_cli/cli/tests/test_usage_capture.py b/cloudsmith_cli/cli/tests/test_usage_capture.py index fbe5cc5dc..c2d67787c 100644 --- a/cloudsmith_cli/cli/tests/test_usage_capture.py +++ b/cloudsmith_cli/cli/tests/test_usage_capture.py @@ -4,6 +4,7 @@ import pytest from click.testing import CliRunner +from cloudsmith_cli.cli import telemetry as cli_telemetry from cloudsmith_cli.cli import usage_capture from cloudsmith_cli.cli.command import AliasGroup @@ -304,3 +305,90 @@ def test_record_credential_keeps_the_last_resolved_source(): store_ctx, SimpleNamespace(source_name="credentials_file") ) assert store_ctx.meta[usage_capture.META_KEY]["auth_source"] == "credentials_file" + + +def _every_command(): + """Yield (path, command, parent path) for the whole real CLI.""" + import importlib + import pkgutil + + from cloudsmith_cli.cli import commands + from cloudsmith_cli.cli.commands.main import main + + for info in pkgutil.walk_packages( + commands.__path__, prefix=commands.__name__ + "." + ): + importlib.import_module(info.name) + + def walk(cmd, path): + yield path, cmd + for name, sub in getattr(cmd, "commands", {}).items(): + yield from walk(sub, (*path, name)) + + yield from walk(main, ()) + + +def test_every_command_that_sends_less_than_all_is_listed_here(): + from cloudsmith_cli.cli.telemetry import ALL, ERRORS + from cloudsmith_cli.core.telemetry_common import declared_policy + + # Change this only on purpose: it is the reviewed list of what does not + # send everything. + expected = { + "credential-helper cargo": ERRORS, + "credential-helper docker": ERRORS, + "credential-helper generic": ERRORS, + "credential-helper pnpm": ERRORS, + "credential-helper terraform": ERRORS, + "mcp start": ERRORS, + "telemetry": ERRORS, + "telemetry id": ERRORS, + "telemetry reset": ERRORS, + "telemetry status": ERRORS, + } + effective = {} + for path, cmd in _every_command(): + inherited = effective.get(" ".join(path[:-1]), ALL) if path else ALL + effective[" ".join(path)] = declared_policy(cmd) or inherited + assert {p: v for p, v in effective.items() if v is not ALL} == expected + + +@click.group(cls=AliasGroup) +def policy_app(): + pass + + +@policy_app.group(cls=AliasGroup) +@cli_telemetry.send(cli_telemetry.ERRORS) +def quiet(): + pass + + +@quiet.command() +@click.pass_context +def inherits(ctx): + CAPTURED["facts"] = usage_capture.collect_facts( + ctx.meta, outcome="success", exit_code=0, duration_ms=1, nested=False + ) + + +@quiet.command() +@cli_telemetry.send(cli_telemetry.ALL) +@click.pass_context +def overrides(ctx): + CAPTURED["facts"] = usage_capture.collect_facts( + ctx.meta, outcome="success", exit_code=0, duration_ms=1, nested=False + ) + + +@pytest.mark.parametrize( + "args, allowed", + [(["quiet", "inherits"], False), (["quiet", "overrides"], True)], +) +def test_the_command_policy_reaches_the_run_facts(args, allowed): + CliRunner().invoke(policy_app, args, catch_exceptions=False) + assert CAPTURED["facts"].usage_allowed is allowed + + +def test_a_command_without_a_policy_sends_usage(): + assert _run(["listing"]).usage_allowed is True diff --git a/cloudsmith_cli/cli/usage_capture.py b/cloudsmith_cli/cli/usage_capture.py index 71850a53c..4e84b6c17 100644 --- a/cloudsmith_cli/cli/usage_capture.py +++ b/cloudsmith_cli/cli/usage_capture.py @@ -299,6 +299,19 @@ def _ssl_verify_strict(opts): return value if isinstance(value, bool) else None +def policy(meta): + """What this run may send, from ``@telemetry.send`` nearest its command.""" + from ..core.telemetry_common import resolve_policy + + contexts = (meta or {}).get(META_KEY, {}).get("contexts", []) + return resolve_policy(contexts[-1] if contexts else None) + + +def current_policy(): + """What the current run may send; ``ALL`` before any command is parsed.""" + return policy(current_root_meta()) + + def typed_flags(meta): """The option spellings typed in this run, from its root ``ctx.meta``.""" return sorted(set((meta or {}).get(META_KEY, {}).get("typed", []))) @@ -306,6 +319,7 @@ def typed_flags(meta): def collect_facts(meta, *, outcome, exit_code, duration_ms, nested): """Build the ``RunFacts`` for a run from its recorded context metadata.""" + from ..core.telemetry_common import Send from ..core.usage.event import RunFacts store = meta.get(META_KEY, {}) @@ -338,6 +352,7 @@ def collect_facts(meta, *, outcome, exit_code, duration_ms, nested): stdout_tty=bool(sys.stdout and sys.stdout.isatty()), stdin_tty=bool(sys.stdin and sys.stdin.isatty()), nested=nested, + usage_allowed=Send.USAGE in policy(meta), # A typed config file or profile that the leaf never loaded may hold # an opt-out or a host this run cannot see: never send. config_unresolved=bool(CONFIG_SPELLINGS & set(typed)) diff --git a/cloudsmith_cli/core/telemetry.py b/cloudsmith_cli/core/telemetry.py index b43447b28..8e9d8a22c 100644 --- a/cloudsmith_cli/core/telemetry.py +++ b/cloudsmith_cli/core/telemetry.py @@ -10,6 +10,9 @@ * ``telemetry = false`` in config.ini - so the choice survives a new shell. Any one opt-out wins; ``telemetry = true`` does not override the env vars. +A command whose ``@telemetry.send`` policy leaves out ERRORS never reports +(checked by the caller, ``AliasGroup.main``). ``CLOUDSMITH_TELEMETRY=log`` +prints the scrubbed report instead of sending it. Anonymisation is enforced by :func:`scrub_event`, which *rebuilds* the event from an allowlist rather than deleting known-bad fields, so anything new the @@ -38,6 +41,7 @@ PROFILE_ENV, config_disabled, get_environment, + log_mode, telemetry_disabled, ) @@ -123,6 +127,9 @@ def is_reportable(exc: BaseException) -> bool: def report_exception(exc: BaseException) -> bool: """Report ``exc`` unless opted out; return whether an event was sent. + In log mode (``CLOUDSMITH_TELEMETRY=log``) the scrubbed event is printed + to stderr instead and nothing is sent, even without a DSN. + Never raises - not even ``KeyboardInterrupt`` during the flush wait - and never waits longer than :data:`FLUSH_TIMEOUT_SECONDS`: a failure while reporting must not change the outcome of the command that is failing. @@ -132,6 +139,10 @@ def report_exception(exc: BaseException) -> bool: return False if telemetry_disabled() or config_disabled(): return False + if log_mode(): + # The DSN is only parsed here: the log transport never sends. + _send(exc, get_dsn() or DSN, log=True) + return False dsn = get_dsn() if not dsn: return False @@ -140,7 +151,26 @@ def report_exception(exc: BaseException) -> bool: return False -def _send(exc: BaseException, dsn: str) -> bool: +def _log_transport(): + """A transport that prints each event to stderr and sends nothing.""" + import json + + from sentry_sdk.transport import Transport + + class LogTransport(Transport): + def capture_envelope(self, envelope): + for item in envelope.items: + if item.type == "event": + click.echo( + "[telemetry] would send error report (log mode, nothing sent):\n" + + json.dumps(item.payload.json, indent=2, sort_keys=True), + err=True, + ) + + return LogTransport() + + +def _send(exc: BaseException, dsn: str, log: bool = False) -> bool: """Build a locked-down, single-use Sentry client and send one event.""" import sentry_sdk @@ -151,7 +181,7 @@ def _send(exc: BaseException, dsn: str) -> bool: proxy = get_default_proxy() client = sentry_sdk.Client( dsn=dsn, - transport=_transport, + transport=_log_transport() if log else _transport, # Both: the SDK prefers https_proxy (and $HTTPS_PROXY) for HTTPS DSNs. http_proxy=proxy, https_proxy=proxy, diff --git a/cloudsmith_cli/core/telemetry_common.py b/cloudsmith_cli/core/telemetry_common.py index 98f279130..40d93ba19 100644 --- a/cloudsmith_cli/core/telemetry_common.py +++ b/cloudsmith_cli/core/telemetry_common.py @@ -5,6 +5,7 @@ it is imported on every command. """ +import enum import os import sys from urllib.parse import urlsplit @@ -14,6 +15,71 @@ OPT_OUT_ENVS = (DO_NOT_TRACK_ENV, NO_TELEMETRY_ENV) _TRUTHY_ENV_VALUES = ("1", "true", "yes") +#: ``CLOUDSMITH_TELEMETRY=log`` prints what would be sent (error reports and +#: usage events) to stderr instead of sending it. +MODE_ENV = "CLOUDSMITH_TELEMETRY" +MODE_LOG = "log" + + +def log_mode(env: "os._Environ[str] | dict[str, str] | None" = None) -> bool: + env = os.environ if env is None else env + return env.get(MODE_ENV, "").strip().lower() == MODE_LOG + + +class Send(enum.Flag): + """What a command may send. ``ALL`` is the default for every command. + + A command's policy only narrows what is sent: the user's opt-outs and + every other rule still apply on top of it. + """ + + NONE = 0 + #: Sentry reports of unexpected exceptions. + ERRORS = enum.auto() + #: The end-of-run usage event. + USAGE = enum.auto() + ALL = ERRORS | USAGE + + +_POLICY_ATTR = "__cloudsmith_telemetry__" + + +def send(policy: Send): + """Set what a command (or a group and every command under it) may send. + + Works above or below ``@.command()``; below it, put it directly + under that line. The policy nearest the command that runs wins. + """ + if not isinstance(policy, Send): + raise TypeError(f"send() takes a telemetry policy, not {policy!r}") + + def mark(f): + target = getattr(f, "callback", None) or f + setattr(target, _POLICY_ATTR, policy) + return f + + return mark + + +def declared_policy(command) -> "Send | None": + """The policy set on a Click command or its callback, if any.""" + for obj in (command, getattr(command, "callback", None)): + policy = getattr(obj, _POLICY_ATTR, None) + if isinstance(policy, Send): + return policy + return None + + +def resolve_policy(ctx) -> Send: + """The policy nearest ``ctx``'s command, walking up to the root.""" + while ctx is not None: + policy = declared_policy(getattr(ctx, "command", None)) + if policy is not None: + return policy + ctx = getattr(ctx, "parent", None) + return Send.ALL + + PROFILE_ENV = "CLOUDSMITH_PROFILE" CONFIG_FILE_ENV = "CLOUDSMITH_CONFIG_FILE" diff --git a/cloudsmith_cli/core/tests/test_telemetry_common.py b/cloudsmith_cli/core/tests/test_telemetry_common.py index da0dce56c..682a084eb 100644 --- a/cloudsmith_cli/core/tests/test_telemetry_common.py +++ b/cloudsmith_cli/core/tests/test_telemetry_common.py @@ -66,3 +66,76 @@ def test_config_file_env_matches_the_option_declaration(): envvars = {p.name: p.envvar for p in main.main.params} assert envvars["config_file"] == telemetry_common.CONFIG_FILE_ENV + + +# --- Per-command policy (@telemetry.send) ------------------------------------ + +Send = telemetry_common.Send + + +def test_policy_levels_combine_as_flags(): + assert Send.ALL == Send.ERRORS | Send.USAGE + assert Send.ERRORS in Send.ALL and Send.USAGE in Send.ALL + assert Send.USAGE not in Send.ERRORS and Send.ERRORS not in Send.USAGE + assert Send.ERRORS not in Send.NONE and Send.USAGE not in Send.NONE + + +def test_cli_module_exposes_the_four_levels(): + from cloudsmith_cli.cli import telemetry as cli_telemetry + + assert (cli_telemetry.NONE, cli_telemetry.ERRORS) == (Send.NONE, Send.ERRORS) + assert (cli_telemetry.USAGE, cli_telemetry.ALL) == (Send.USAGE, Send.ALL) + assert cli_telemetry.send is telemetry_common.send + + +def test_send_rejects_anything_but_a_policy(): + with pytest.raises(TypeError): + telemetry_common.send("errors") # ty: ignore[invalid-argument-type] + + +def _ctx(command, parent=None): + import types + + return types.SimpleNamespace(command=command, parent=parent) + + +def test_send_works_above_or_below_the_click_decorator(): + import click + + @click.command() + @telemetry_common.send(Send.ERRORS) + def below(): + pass + + @telemetry_common.send(Send.USAGE) + @click.command() + def above(): + pass + + assert telemetry_common.declared_policy(below) is Send.ERRORS + assert telemetry_common.declared_policy(above) is Send.USAGE + + +def test_the_policy_nearest_the_command_wins_and_all_is_the_default(): + import click + + @click.group() + @telemetry_common.send(Send.ERRORS) + def group(): + pass + + @group.command() + def inherits(): + pass + + @group.command() + @telemetry_common.send(Send.ALL) + def overrides(): + pass + + root = _ctx(click.Group("root")) + group_ctx = _ctx(group, root) + assert telemetry_common.resolve_policy(_ctx(inherits, group_ctx)) is Send.ERRORS + assert telemetry_common.resolve_policy(_ctx(overrides, group_ctx)) is Send.ALL + assert telemetry_common.resolve_policy(root) is Send.ALL + assert telemetry_common.resolve_policy(None) is Send.ALL diff --git a/cloudsmith_cli/core/tests/test_usage_run.py b/cloudsmith_cli/core/tests/test_usage_run.py index 5268dbede..98cf2fb56 100644 --- a/cloudsmith_cli/core/tests/test_usage_run.py +++ b/cloudsmith_cli/core/tests/test_usage_run.py @@ -21,17 +21,10 @@ def test_sends_one_event(usage_events, interactive): assert usage_events[0]["properties"]["command"] == "whoami" -@pytest.mark.parametrize( - "command", - [ - "credential-helper docker", - "credential-helper terraform", - "mcp start", - "telemetry reset", - ], -) -def test_excluded_commands_send_nothing_and_touch_no_state(usage_events, command): - run.finish(facts(command=command)) +def test_a_command_without_usage_in_its_policy_sends_nothing_and_touches_no_state( + usage_events, +): + run.finish(facts(usage_allowed=False)) assert usage_events == [] and state.read() == {} @@ -200,11 +193,6 @@ def test_config_file_opt_out_sends_nothing( assert usage_events == [] -def test_credential_helper_install_still_sends(usage_events, interactive): - run.finish(facts(command="credential-helper install")) - assert len(usage_events) == 1 - - class _Loader: """A deferred-sources loader that records how often it ran.""" @@ -222,7 +210,7 @@ def __call__(self): def test_excluded_command_never_loads_deferred_sources(usage_events): loader = _Loader({"workspace": "config_file"}) - run.finish(facts(command="credential-helper docker", deferred_sources=loader)) + run.finish(facts(usage_allowed=False, deferred_sources=loader)) assert loader.calls == 0 and usage_events == [] @@ -426,7 +414,7 @@ def test_opting_back_in_creates_a_new_install_id(stored_id, usage_events, monkey def test_excluded_command_keeps_the_install_id_when_opted_out(stored_id, monkeypatch): monkeypatch.setenv("DO_NOT_TRACK", "1") - run.finish(facts(command="credential-helper docker")) + run.finish(facts(usage_allowed=False)) assert state.read()["install_id"] == stored_id diff --git a/cloudsmith_cli/core/usage/event.py b/cloudsmith_cli/core/usage/event.py index b017d352b..2c0c8d5fe 100644 --- a/cloudsmith_cli/core/usage/event.py +++ b/cloudsmith_cli/core/usage/event.py @@ -95,6 +95,9 @@ class RunFacts: stdout_tty: bool = False stdin_tty: bool = False nested: bool = False + #: False when the command's ``@telemetry.send`` policy leaves out USAGE; + #: the run sends nothing. Not an event property. + usage_allowed: bool = True #: A typed ``--config-file``/``--profile`` the command never loaded; the #: run sends nothing. Not an event property. config_unresolved: bool = False diff --git a/cloudsmith_cli/core/usage/run.py b/cloudsmith_cli/core/usage/run.py index a3dc113d5..42a3b133e 100644 --- a/cloudsmith_cli/core/usage/run.py +++ b/cloudsmith_cli/core/usage/run.py @@ -1,8 +1,9 @@ # Copyright 2026 Cloudsmith Ltd """Decide whether to send a usage event for this run, and send it. -Checks run cheapest-first and stop at the first "no", so excluded commands -(credential helpers on every pull) pay almost nothing. Never raises. +Checks run cheapest-first and stop at the first "no", so commands that send +no usage event (``@telemetry.send`` without USAGE, e.g. credential helpers +on every pull) pay almost nothing. Never raises. """ import functools @@ -10,27 +11,9 @@ import time from datetime import UTC, datetime -EXCLUDED_COMMANDS = frozenset({"mcp start"}) -#: Machine-invoked or telemetry-managing commands. ``install``, -#: ``uninstall`` and ``list`` under credential-helper are run by people and -#: do send. -EXCLUDED_PREFIXES = ( - "credential-helper cargo", - "credential-helper docker", - "credential-helper generic", - "credential-helper pnpm", - "credential-helper terraform", - "telemetry", -) BACKOFF_SECONDS = 24 * 60 * 60 -def _excluded(command): - if command in EXCLUDED_COMMANDS: - return True - return any(command == p or command.startswith(p + " ") for p in EXCLUDED_PREFIXES) - - def _load_deferred_sources(facts): """Merge config-file setting sources in; Click flag/env sources win.""" if facts.deferred_sources is None: @@ -89,10 +72,11 @@ def finish(facts): def _finish(facts): # pylint: disable=too-many-return-statements,too-many-locals - if facts.nested or facts.config_unresolved or _excluded(facts.command): + if facts.nested or facts.config_unresolved or not facts.usage_allowed: return - # Imported late so an excluded command pays for none of these modules. + # Imported late so a command that sends no usage event pays for none of + # these modules. from .. import installation, telemetry_common, version from . import environment, event, identity, notice, sender, settings, state diff --git a/cloudsmith_cli/core/usage/settings.py b/cloudsmith_cli/core/usage/settings.py index 76486a9d1..16e4b4445 100644 --- a/cloudsmith_cli/core/usage/settings.py +++ b/cloudsmith_cli/core/usage/settings.py @@ -7,12 +7,15 @@ import os +# Log mode is shared with error reporting. +from ..telemetry_common import MODE_ENV # noqa: F401 +from ..telemetry_common import log_mode as _shared_log_mode + POSTHOG_PROJECT_KEY = "" POSTHOG_HOST = "https://eu.i.posthog.com" KEY_ENV = "CLOUDSMITH_TELEMETRY_POSTHOG_KEY" HOST_ENV = "CLOUDSMITH_TELEMETRY_POSTHOG_HOST" -MODE_ENV = "CLOUDSMITH_TELEMETRY" INTERNAL_ENV = "CLOUDSMITH_TELEMETRY_INTERNAL" INSTALL_ID_ENV = "CLOUDSMITH_TELEMETRY_INSTALL_ID" SAMPLE_RATE_ENV = "CLOUDSMITH_TELEMETRY_CI_SAMPLE_RATE" @@ -42,7 +45,7 @@ def host(env=None): def log_mode(env=None): - return _env(env).get(MODE_ENV, "").strip().lower() == "log" + return _shared_log_mode(env) def internal(env=None): From be8c6e3e64d2dd2ad3a830e54c3bc08a73c4cee4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tighearn=C3=A1n=20Carroll?= <13870403+tigh-latte@users.noreply.github.com> Date: Thu, 5 Nov 2026 10:12:24 +0000 Subject: [PATCH 5/5] feat(ENG-15367): send the deprecated names a run used Add a `deprecated` list property: the deprecated commands, option spellings, environment variables and config.ini keys that the run used, as `:` (for example `option:--org`, `env:CLOUDSMITH_API_PROXY`, `config:api_proxy`, `command:login`). It answers who still depends on a name before it is removed in 2.0.0. `deprecation.record` notes each name in the root ctx.meta. The warn_* helpers and the deprecated login command call it before they decide whether to print, so a suppressed warning (credential helpers, shell completion) is still recorded; those runs send no usage event anyway. Every name comes from the code: option spellings and env var names that the options declare, keys of CONFIG_KEY_REPLACEMENTS, and command names. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 1 + cloudsmith_cli/cli/commands/login.py | 1 + cloudsmith_cli/cli/deprecation.py | 28 ++++++++++--- cloudsmith_cli/cli/tests/test_deprecation.py | 37 +++++++++++++++++ cloudsmith_cli/cli/tests/test_usage_main.py | 41 +++++++++++++++++++ cloudsmith_cli/cli/usage_capture.py | 2 + cloudsmith_cli/core/tests/test_usage_event.py | 11 ++++- cloudsmith_cli/core/usage/event.py | 3 ++ 8 files changed, 118 insertions(+), 6 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ff6e35ed7..c5e291df9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -75,5 +75,6 @@ Tests live alongside code: `cloudsmith_cli/cli/tests/` and `cloudsmith_cli/core/ - **Rules:** only send names defined in code (commands, parameters, declared option spellings, env var names), enums, booleans and numbers, never a value from the user. Never send for shell completion, non-Cloudsmith API hosts, SSL verification off, development builds without the key, or any opt-out (`DO_NOT_TRACK`, `CLOUDSMITH_NO_TELEMETRY`, `telemetry = false`). - **Per-command policy:** a command (or a group and everything under it) that should send less than everything declares it with `@telemetry.send(telemetry.NONE | ERRORS | USAGE)` from `cloudsmith_cli/cli/telemetry.py`, put directly under its `@.command()` line; `ALL` (Sentry error reports and usage events) is the default and the nearest policy wins. Credential helpers, `mcp start` and the `telemetry` group use `ERRORS`. A policy only narrows what is sent: the opt-outs always win. `test_every_command_that_sends_less_than_all_is_listed_here` in `cli/tests/test_usage_capture.py` lists every such command, so update it in the same change. - **Adding a property:** declare it in `cloudsmith_cli/core/usage/event.py` (`PROPERTIES`, with the question it answers), fill it in `_values`, and extend `cloudsmith_cli/core/tests/test_usage_event.py`; new commands and flags need no telemetry change because capture reads the Click context. +- **Deprecations:** warn through `cli/deprecation.py` (`warn_option`, `warn_envvar`, `warn_config_key`, or `record` for anything else); it records `:` for the `deprecated` property, so dashboards show who still uses a name before it is removed. - **Performance:** `test_startup_imports.py` fails if importing the CLI loads heavy modules (HTTP clients, `cloudsmith_api`, `rich`) or any command module, so the usage code imports its HTTP session inside functions that run at the end of a run; the wait is capped by the shared 1 s budget and failures stay silent. - **Testing locally:** `CLOUDSMITH_TELEMETRY=log` prints the usage event and any Sentry error report (scrubbed) instead of sending them, and a test project needs `CLOUDSMITH_TELEMETRY_POSTHOG_KEY` (plus `CLOUDSMITH_TELEMETRY_POSTHOG_HOST` for a non-default host), because development builds (source checkouts and editable installs) send nothing without that key. Use the `usage_events` pytest fixture in tests. diff --git a/cloudsmith_cli/cli/commands/login.py b/cloudsmith_cli/cli/commands/login.py index eac4dedd2..1e94cff4e 100644 --- a/cloudsmith_cli/cli/commands/login.py +++ b/cloudsmith_cli/cli/commands/login.py @@ -69,6 +69,7 @@ def parse_args(self, ctx, args): # Click prompts for --login and --password while parsing, so the # warning must be printed here rather than in the command body. # Shell completion also parses (resiliently); the warning skips it. + deprecation.record("command", ctx.info_name, ctx=ctx) deprecation.warn( f"`cloudsmith {ctx.info_name}`", removal=DEPRECATION_REMOVAL, diff --git a/cloudsmith_cli/cli/deprecation.py b/cloudsmith_cli/cli/deprecation.py index 5ee43cbae..a0e8571d4 100644 --- a/cloudsmith_cli/cli/deprecation.py +++ b/cloudsmith_cli/cli/deprecation.py @@ -41,6 +41,7 @@ PAGE_ALL_ALIASES = {"--show-all": "--page-all"} _WARNED_META_KEY = "cloudsmith_cli.deprecation_warnings" +_USED_META_KEY = "cloudsmith_cli.deprecated_used" def format_warning(subject, replacement=None, *, removal=None, detail=None): @@ -63,6 +64,23 @@ def _is_credential_helper_runtime(ctx): ) +def record(kind, name, ctx=None): + """Note that this run used the deprecated `name`, for usage telemetry. + + `kind` is "command", "option", "env" or "config", and `name` is a name the + code declares, never a value from the user. Recorded even when the warning + is not printed. + """ + ctx = ctx or click.get_current_context(silent=True) + if ctx is not None: + ctx.find_root().meta.setdefault(_USED_META_KEY, set()).add(f"{kind}:{name}") + + +def used(meta): + """The deprecated names this run used, from its root ``ctx.meta``.""" + return sorted((meta or {}).get(_USED_META_KEY, ())) + + def warn(subject, replacement=None, *, removal=None, detail=None, ctx=None): """Print the deprecation warning for `subject` to stderr, once per run.""" message = format_warning(subject, replacement, removal=removal, detail=detail) @@ -79,20 +97,20 @@ def warn(subject, replacement=None, *, removal=None, detail=None, ctx=None): def warn_option(spelling, replacement, ctx=None): """Warn that the option `spelling` is deprecated.""" + record("option", spelling, ctx=ctx) warn(f"`{spelling}`", f"`{replacement}`", ctx=ctx) def warn_config_key(key, ctx=None): """Warn that the config.ini key `key` is deprecated.""" - warn( - f"The `{key}` key in config.ini", - f"`{CONFIG_KEY_REPLACEMENTS[key]}`", - ctx=ctx, - ) + replacement = CONFIG_KEY_REPLACEMENTS[key] + record("config", key, ctx=ctx) + warn(f"The `{key}` key in config.ini", f"`{replacement}`", ctx=ctx) def warn_envvar(name, replacement, ctx=None): """Warn that the environment variable `name` is deprecated.""" + record("env", name, ctx=ctx) warn(f"`{name}`", f"`{replacement}`", ctx=ctx) diff --git a/cloudsmith_cli/cli/tests/test_deprecation.py b/cloudsmith_cli/cli/tests/test_deprecation.py index 711ffc2d6..a12b57df0 100644 --- a/cloudsmith_cli/cli/tests/test_deprecation.py +++ b/cloudsmith_cli/cli/tests/test_deprecation.py @@ -426,3 +426,40 @@ def test_help_hides_the_deprecated_spellings(): ): assert spelling not in result.stdout assert "--workspace" in result.stdout + + +def test_record_keeps_each_deprecated_name_once_in_the_root_context(): + root = click.Context(click.Group("root")) + leaf = click.Context(click.Command("leaf"), parent=root) + deprecation.record("option", "--org", ctx=leaf) + deprecation.record("option", "--org", ctx=leaf) + deprecation.record("config", "api_proxy", ctx=root) + assert deprecation.used(root.meta) == ["config:api_proxy", "option:--org"] + assert deprecation.used(None) == [] + + +def test_record_without_a_context_does_nothing(): + deprecation.record("option", "--org") + + +def test_warnings_record_even_when_not_printed(): + # Credential helper runtimes print no warning, but the name is recorded. + @click.group(name="credential-helper") + def credential_helper(): + pass + + seen = {} + + @credential_helper.command(name="docker") + @click.pass_context + def docker(ctx): + deprecation.warn_option("--org", "--workspace", ctx=ctx) + seen["used"] = deprecation.used(ctx.find_root().meta) + + result = click.testing.CliRunner().invoke( + click.Group(commands={"credential-helper": credential_helper}), + ["credential-helper", "docker"], + catch_exceptions=False, + ) + assert "is deprecated" not in result.output + assert seen["used"] == ["option:--org"] diff --git a/cloudsmith_cli/cli/tests/test_usage_main.py b/cloudsmith_cli/cli/tests/test_usage_main.py index 0a451f9f7..7c289cb70 100644 --- a/cloudsmith_cli/cli/tests/test_usage_main.py +++ b/cloudsmith_cli/cli/tests/test_usage_main.py @@ -440,3 +440,44 @@ def test_command_that_never_logs_in_reports_no_login_method( monkeypatch.setenv("CLOUDSMITH_API_KEY", "not-a-real-key") main(args=["credential-helper", "list"], standalone_mode=False) assert "auth_source" not in usage_events[-1]["properties"] + + +PROXY = "http://proxy.example:3128" + + +@pytest.mark.parametrize( + "extra, env, config, deprecated", + [ + ([], {}, "", []), + (["--show-all"], {}, "", ["option:--show-all"]), + (["--org", "acme"], {}, "", ["option:--org"]), + ([], {"CLOUDSMITH_API_PROXY": PROXY}, "", ["env:CLOUDSMITH_API_PROXY"]), + ([], {}, f"api_proxy = {PROXY}", ["config:api_proxy"]), + ], +) +def test_deprecated_names_used_are_sent( + usage_events, + notice_seen, + no_repos_api, + telemetry_config_dir, + monkeypatch, + extra, + env, + config, + deprecated, +): + (telemetry_config_dir / "config.ini").write_text(f"[default]\n{config}\n") + for name, value in env.items(): + monkeypatch.setenv(name, value) + args = ["list", "repos", "acme", "-F", "json", *extra] + assert main(args=args, standalone_mode=False) in (0, None) + (event,) = usage_events + assert event["properties"]["deprecated"] == deprecated + + +@pytest.mark.parametrize("name", ["login", "token"]) +def test_deprecated_command_is_sent(usage_events, notice_seen, name): + assert main(args=[name, "--help"], standalone_mode=False) in (0, None) + (event,) = usage_events + assert event["properties"]["deprecated"] == [f"command:{name}"] + assert event["properties"]["outcome"] == "help" diff --git a/cloudsmith_cli/cli/usage_capture.py b/cloudsmith_cli/cli/usage_capture.py index 4e84b6c17..6f80cfee8 100644 --- a/cloudsmith_cli/cli/usage_capture.py +++ b/cloudsmith_cli/cli/usage_capture.py @@ -321,6 +321,7 @@ def collect_facts(meta, *, outcome, exit_code, duration_ms, nested): """Build the ``RunFacts`` for a run from its recorded context metadata.""" from ..core.telemetry_common import Send from ..core.usage.event import RunFacts + from . import deprecation store = meta.get(META_KEY, {}) contexts = store.get("contexts", []) @@ -334,6 +335,7 @@ def collect_facts(meta, *, outcome, exit_code, duration_ms, nested): flags_typed=typed, envvars=envvars, aliases_used=list(store.get("aliases", [])), + deprecated=deprecation.used(meta), output_format=getattr(opts, "output", None), # A typed --help only means help was shown if the run then succeeded: # `whoami --no-such-flag --help` is still a usage error. diff --git a/cloudsmith_cli/core/tests/test_usage_event.py b/cloudsmith_cli/core/tests/test_usage_event.py index 914638370..abc06e2ed 100644 --- a/cloudsmith_cli/core/tests/test_usage_event.py +++ b/cloudsmith_cli/core/tests/test_usage_event.py @@ -109,7 +109,9 @@ def test_wrong_type_is_dropped(): assert "duration_ms" not in props -@pytest.mark.parametrize("name", ["flags", "flags_typed", "envvars", "aliases_used"]) +@pytest.mark.parametrize( + "name", ["flags", "flags_typed", "envvars", "aliases_used", "deprecated"] +) @pytest.mark.parametrize("bad", ["--page-size", None, ["ok", 3], {"a": 1}, 7]) def test_malformed_name_lists_are_dropped_not_split(name, bad): # A bare string must not be sent one character at a time, and None or @@ -127,3 +129,10 @@ def test_name_lists_are_deduplicated_and_sorted_but_aliases_keep_order(): )["properties"] assert props["flags"] == ["output_format", "page_size"] assert props["aliases_used"] == ["ls", "repos"] + + +def test_deprecated_names_are_deduplicated_and_sorted(): + props = _build( + _facts(deprecated=["option:--org", "config:api_proxy", "option:--org"]) + )["properties"] + assert props["deprecated"] == ["config:api_proxy", "option:--org"] diff --git a/cloudsmith_cli/core/usage/event.py b/cloudsmith_cli/core/usage/event.py index 2c0c8d5fe..23aa51512 100644 --- a/cloudsmith_cli/core/usage/event.py +++ b/cloudsmith_cli/core/usage/event.py @@ -80,6 +80,7 @@ class RunFacts: flags_typed: list[str] = field(default_factory=list) envvars: list[str] = field(default_factory=list) aliases_used: list[str] = field(default_factory=list) + deprecated: list[str] = field(default_factory=list) output_format: str | None = None outcome: str = "success" exit_code: int = 0 @@ -120,6 +121,7 @@ class Property: Property("flags_typed", list, "command and flag use"), Property("envvars", list, "configuration habits; login methods"), Property("aliases_used", list, "command and flag use"), + Property("deprecated", list, "deprecation impact: who still uses removed names"), Property("output_format", str, "command and flag use; persona"), Property("outcome", str, "reliability and speed"), Property("exit_code", int, "reliability and speed"), @@ -179,6 +181,7 @@ def _values( "flags_typed": _names(facts.flags_typed), "envvars": _names(facts.envvars), "aliases_used": _names(facts.aliases_used, ordered=True), + "deprecated": _names(facts.deprecated), "output_format": facts.output_format, "outcome": facts.outcome, "exit_code": facts.exit_code,