refactor(ENG-15367): share telemetry opt-outs and the exit wait budget

cloudsmith_cli/conftest.py

@@ -46,6 +46,7 @@ def telemetry_config_dir(monkeypatch, tmp_path_factory):
         monkeypatch.setattr(reader, "config_files", [name])
         monkeypatch.setattr(reader, "config_searchpath", [str(path)])
     monkeypatch.delenv(telemetry.PROFILE_ENV, raising=False)
+    monkeypatch.delenv("CLOUDSMITH_CONFIG_FILE", raising=False)
     return path
 
 
@@ -88,3 +89,11 @@ def restore_default_proxy():
     """Clear the process-wide proxy that ``initialise_session`` sets."""
     yield
     session.set_default_proxy(None)
+
+
+@pytest.fixture(autouse=True)
+def reset_wait_budget():
+    """Give every test a full shared exit-wait budget."""
+    from cloudsmith_cli.core import telemetry_common
+
+    telemetry_common.WaitBudget.shared().reset(1.0)

cloudsmith_cli/core/telemetry.py

@@ -30,6 +30,16 @@
 import click
 
 from .certificate_errors import find_certificate_error
+from .telemetry_common import CLOUDSMITH_HOST_SUFFIXES as _ALLOWED_HOST_SUFFIXES
+from .telemetry_common import (  # noqa: F401  (re-exported for callers and tests)
+    DO_NOT_TRACK_ENV,
+    NO_TELEMETRY_ENV,
+    OPT_OUT_ENVS,
+    PROFILE_ENV,
+    config_disabled,
+    get_environment,
+    telemetry_disabled,
+)
 
 #: Production ingest DSN for the dedicated ``cloudsmith-cli`` Sentry project.
 #: Not a secret: a DSN is a public, write-only ingest key, and every client
@@ -41,18 +51,14 @@
 #: Set it empty to disable sending.
 DSN_ENV = "CLOUDSMITH_TELEMETRY_DSN"
 
-NO_TELEMETRY_ENV = "CLOUDSMITH_NO_TELEMETRY"
-DO_NOT_TRACK_ENV = "DO_NOT_TRACK"
-OPT_OUT_ENVS = (DO_NOT_TRACK_ENV, NO_TELEMETRY_ENV)
-_TRUTHY_ENV_VALUES = ("1", "true", "yes")
-
 #: Upper bound on how long a failing command waits for the report to send.
 FLUSH_TIMEOUT_SECONDS = 2.0
 MAX_MESSAGE_LENGTH = 1024
 
-#: Hosts that may appear in reports; any other host (custom/self-hosted API,
-#: proxies, internal registries) is replaced.
-_ALLOWED_HOST_SUFFIXES = ("cloudsmith.io", "cloudsmith.com")
+# Hosts that may appear in reports are ``_ALLOWED_HOST_SUFFIXES`` (imported
+# above); any other host (custom/self-hosted API, proxies, internal
+# registries) is replaced.
+
 #: Env vars whose values are treated as secrets wherever they appear.
 _SECRET_ENV_NAME = re.compile(r"KEY|TOKEN|SECRET|PASSW|CREDENTIAL|AUTH", re.IGNORECASE)
 _MIN_SECRET_LENGTH = 8
@@ -87,48 +93,6 @@ def format_exception_summary(exc: BaseException) -> str:
     return f"{type(exc).__name__}: {exc}"
 
 
-def telemetry_disabled(env: "os._Environ[str] | dict[str, str] | None" = None) -> bool:
-    """Tell whether the user has opted out of error reporting.
-
-    Either opt-out variable set to ``1``/``true``/``yes`` (case-insensitive)
-    disables reporting. Any other value, or unset, leaves it on.
-    """
-    env = os.environ if env is None else env
-    return any(
-        env.get(name, "").strip().lower() in _TRUTHY_ENV_VALUES for name in OPT_OUT_ENVS
-    )
-
-
-PROFILE_ENV = "CLOUDSMITH_PROFILE"
-
-
-def config_disabled() -> bool:
-    """Tell whether ``telemetry = false`` is set in config.ini.
-
-    Checked two ways, and either one saying ``false`` wins:
-
-    * the options the command loaded, which honour ``--config-file`` and
-      ``--profile`` - but only exist if the command loads config at all;
-    * a fresh read of config.ini (``[default]`` plus ``$CLOUDSMITH_PROFILE``),
-      which covers commands that never load config or failed before doing so.
-
-    Fails closed: if the config cannot be read we cannot confirm the user has
-    not opted out, so we do not report.
-    """
-    try:
-        from ..cli import config
-
-        opts = getattr(config.OPTIONS, "value", None)
-        if opts is not None and opts.telemetry is False:
-            return True
-
-        probe = config.Options()
-        config.ConfigReader.load_config(probe, profile=os.environ.get(PROFILE_ENV))
-        return probe.telemetry is False
-    except Exception:  # pylint: disable=broad-exception-caught
-        return True
-
-
 def get_dsn(env: "os._Environ[str] | dict[str, str] | None" = None) -> str:
     """Return the DSN to report to; empty means do not report."""
     env = os.environ if env is None else env
@@ -235,21 +199,6 @@ def _send(exc: BaseException, dsn: str) -> bool:
     return event_id is not None
 
 
-def get_environment(package_file=None, frozen=None):
-    """Return ``development`` for a source checkout, else ``production``.
-
-    Frozen builds and wheels installed into site-/dist-packages are releases;
-    anything imported from elsewhere is an editable install or a checkout, so
-    developers' errors stay out of production triage.
-    """
-    frozen = getattr(sys, "frozen", False) if frozen is None else frozen
-    if frozen:
-        return "production"
-    path = (package_file or os.path.abspath(__file__)).replace("\\", "/")
-    installed = "/site-packages/" in path or "/dist-packages/" in path
-    return "production" if installed else "development"
-
-
 def _safe(func, default):
     try:
         return func()

cloudsmith_cli/core/telemetry_common.py

@@ -0,0 +1,146 @@
+# Copyright 2026 Cloudsmith Ltd
+"""Helpers shared by error reporting, update checks and usage telemetry.
+
+Keep this module's imports light (no ``requests``, ``ssl`` or ``urllib3``):
+it is imported on every command.
+"""
+
+import os
+import sys
+from urllib.parse import urlsplit
+
+NO_TELEMETRY_ENV = "CLOUDSMITH_NO_TELEMETRY"
+DO_NOT_TRACK_ENV = "DO_NOT_TRACK"
+OPT_OUT_ENVS = (DO_NOT_TRACK_ENV, NO_TELEMETRY_ENV)
+_TRUTHY_ENV_VALUES = ("1", "true", "yes")
+
+PROFILE_ENV = "CLOUDSMITH_PROFILE"
+CONFIG_FILE_ENV = "CLOUDSMITH_CONFIG_FILE"
+
+#: Hosts whose API traffic may be measured; any other host (self-hosted,
+#: proxies, internal registries) never sends usage telemetry.
+CLOUDSMITH_HOST_SUFFIXES = ("cloudsmith.io", "cloudsmith.com")
+
+
+def telemetry_disabled(env: "os._Environ[str] | dict[str, str] | None" = None) -> bool:
+    """Tell whether the user has opted out of telemetry (error reporting and usage).
+
+    Either opt-out variable set to ``1``/``true``/``yes`` (case-insensitive)
+    disables both. Any other value, or unset, leaves them on.
+    """
+    env = os.environ if env is None else env
+    return any(
+        env.get(name, "").strip().lower() in _TRUTHY_ENV_VALUES for name in OPT_OUT_ENVS
+    )
+
+
+def read_config_file():
+    """Read config.ini afresh (``[default]`` plus ``$CLOUDSMITH_PROFILE``).
+
+    The file named by ``$CLOUDSMITH_CONFIG_FILE`` is read first, as the CLI
+    reads it. Returns new options holding what the files set. Covers commands
+    that never load config or failed before doing so. Raises if a file cannot
+    be read; callers fail closed.
+    """
+    from ..cli import config
+
+    # load_config inserts the path into the reader's class-level lists, so
+    # read through a subclass with copies to leave the CLI's reader as it was.
+    reader = type(
+        "ProbeConfigReader",
+        (config.ConfigReader,),
+        {
+            "config_files": list(config.ConfigReader.config_files),
+            "config_searchpath": list(config.ConfigReader.config_searchpath),
+        },
+    )
+    probe = config.Options()
+    reader.load_config(
+        probe,
+        path=os.environ.get(CONFIG_FILE_ENV) or None,
+        profile=os.environ.get(PROFILE_ENV),
+    )
+    return probe
+
+
+def config_disabled(read_config=read_config_file) -> bool:
+    """Tell whether ``telemetry = false`` is set in config.ini.
+
+    It turns off telemetry (error reporting and usage).
+
+    Checked two ways, and either one saying ``false`` wins:
+
+    * the options the command loaded, which honour ``--config-file`` and
+      ``--profile`` - but only exist if the command loads config at all;
+    * a fresh read of config.ini by ``read_config``, which a caller can pass
+      to share one read with its own config.ini checks.
+
+    Fails closed: if the config cannot be read we cannot confirm the user has
+    not opted out, so we send nothing.
+    """
+    try:
+        from ..cli import config
+
+        opts = getattr(config.OPTIONS, "value", None)
+        if opts is not None and opts.telemetry is False:
+            return True
+
+        return read_config().telemetry is False
+    except Exception:  # pylint: disable=broad-exception-caught
+        return True
+
+
+def get_environment(package_file=None, frozen=None):
+    """Return ``development`` for a source checkout, else ``production``.
+
+    Frozen builds and wheels installed into site-/dist-packages are releases;
+    anything imported from elsewhere is an editable install or a checkout, so
+    developers' errors stay out of production triage.
+    """
+    frozen = getattr(sys, "frozen", False) if frozen is None else frozen
+    if frozen:
+        return "production"
+    path = (package_file or os.path.abspath(__file__)).replace("\\", "/")
+    installed = "/site-packages/" in path or "/dist-packages/" in path
+    return "production" if installed else "development"
+
+
+def is_cloudsmith_host(url_or_host):
+    """Tell whether ``url_or_host`` is a Cloudsmith host (empty means default)."""
+    if not url_or_host:
+        return True
+    text = url_or_host.strip()
+    host = urlsplit(text if "://" in text else f"//{text}").hostname or ""
+    host = host.lower().rstrip(".")
+    return any(
+        host == suffix or host.endswith("." + suffix)
+        for suffix in CLOUDSMITH_HOST_SUFFIXES
+    )
+
+
+class WaitBudget:
+    """One wait budget per process for background work at command exit.
+
+    The update check and usage telemetry both wait for a background thread
+    when the command ends; together they must not exceed ``total`` seconds.
+    """
+
+    _shared = None
+
+    def __init__(self, total=1.0):
+        self._remaining = total
+
+    @classmethod
+    def shared(cls):
+        if cls._shared is None:
+            cls._shared = cls()
+        return cls._shared
+
+    def remaining(self):
+        return max(0.0, self._remaining)
+
+    def spend(self, seconds):
+        self._remaining = max(0.0, self._remaining - max(0.0, seconds))
+
+    def reset(self, total=1.0):
+        self._remaining = total

cloudsmith_cli/core/tests/test_telemetry_common.py

@@ -0,0 +1,68 @@
+"""Tests for the helpers shared by error reporting, update checks and usage."""
+
+import pytest
+
+from cloudsmith_cli.core import telemetry, telemetry_common
+
+
+@pytest.mark.parametrize(
+    "value,expected",
+    [
+        ("https://api.cloudsmith.io", True),
+        ("https://api.cloudsmith.com/v1/", True),
+        ("api.cloudsmith.io", True),
+        ("https://cloudsmith.io", True),
+        ("https://evilcloudsmith.io", False),
+        ("https://cloudsmith.io.evil.com", False),
+        ("https://artifacts.example.com", False),
+        ("", True),
+        (None, True),
+    ],
+)
+def test_is_cloudsmith_host(value, expected):
+    # Empty or missing means the default host, which is Cloudsmith's.
+    assert telemetry_common.is_cloudsmith_host(value) is expected
+
+
+def test_sentry_module_reexports_shared_names():
+    assert telemetry.telemetry_disabled is telemetry_common.telemetry_disabled
+    assert telemetry.config_disabled is telemetry_common.config_disabled
+    assert telemetry.get_environment is telemetry_common.get_environment
+    assert telemetry.OPT_OUT_ENVS == telemetry_common.OPT_OUT_ENVS
+
+
+def test_wait_budget_is_shared_and_never_negative():
+    budget = telemetry_common.WaitBudget.shared()
+    budget.reset(1.0)
+    budget.spend(0.4)
+    assert telemetry_common.WaitBudget.shared().remaining() == pytest.approx(0.6)
+    budget.spend(5.0)
+    assert budget.remaining() == 0.0
+
+
+def test_read_config_file_honours_the_config_file_env(
+    telemetry_config_dir, tmp_path, monkeypatch
+):
+    from cloudsmith_cli.cli import config
+
+    (telemetry_config_dir / "config.ini").write_text("[default]\ntelemetry = true\n")
+    path = tmp_path / "optout.ini"
+    path.write_text("[default]\ntelemetry = false\n")
+    files = list(config.ConfigReader.config_files)
+    searchpath = list(config.ConfigReader.config_searchpath)
+
+    monkeypatch.setenv(telemetry_common.CONFIG_FILE_ENV, str(path))
+    assert telemetry_common.read_config_file().telemetry is False
+    # The CLI's own reader is left as it was for the rest of the run.
+    assert config.ConfigReader.config_files == files
+    assert config.ConfigReader.config_searchpath == searchpath
+
+    monkeypatch.delenv(telemetry_common.CONFIG_FILE_ENV)
+    assert telemetry_common.read_config_file().telemetry is True
+
+
+def test_config_file_env_matches_the_option_declaration():
+    from cloudsmith_cli.cli.commands import main
+
+    envvars = {p.name: p.envvar for p in main.main.params}
+    assert envvars["config_file"] == telemetry_common.CONFIG_FILE_ENV

cloudsmith_cli/core/tests/test_update_check.py

@@ -705,3 +705,25 @@ def writer():
         thread.start()
         update_check._finish_and_notify(thread, self._Opts(), "list", now=NOW + 1)
         assert NEWER_VERSION in capsys.readouterr().err
+
+
+def test_finish_spends_shared_wait_budget(monkeypatch):
+    import threading
+    import time
+
+    from cloudsmith_cli.core import telemetry_common, update_check
+
+    budget = telemetry_common.WaitBudget.shared()
+    budget.reset(1.0)
+    gate = threading.Event()
+    thread = threading.Thread(target=gate.wait, args=(5,), daemon=True)
+    thread.start()
+    monkeypatch.setattr(update_check, "notice_suppressed", lambda *a: True)
+
+    started = time.monotonic()
+    update_check._finish_and_notify(thread, opts=object(), invoked=None)
+    elapsed = time.monotonic() - started
+    gate.set()
+
+    assert 0.9 <= elapsed < 1.5
+    assert budget.remaining() < 0.1

cloudsmith_cli/core/update_check.py

@@ -434,7 +434,12 @@ def _finish_and_notify(
     interactive run can still speak.
     """
     if thread is not None:
-        thread.join(BACKGROUND_JOIN_TIMEOUT_SECONDS)
+        from .telemetry_common import WaitBudget
+
+        budget = WaitBudget.shared()
+        started = time.monotonic()
+        thread.join(min(BACKGROUND_JOIN_TIMEOUT_SECONDS, budget.remaining()))
+        budget.spend(time.monotonic() - started)
     if notice_suppressed(getattr(opts, "output", None), invoked):
         return
     state = read_cached_state()