diff --git a/headroom/telemetry/collector.py b/headroom/telemetry/collector.py index 1439c1a26..34682c38d 100644 --- a/headroom/telemetry/collector.py +++ b/headroom/telemetry/collector.py @@ -500,7 +500,7 @@ class TelemetryCollector: type_counts["string"] = type_counts.get("string", 0) + 1 elif isinstance(v, bool): type_counts["boolean"] = type_counts.get("boolean", 0) + 1 - elif isinstance(v, (int, float)): + elif isinstance(v, int | float): type_counts["numeric"] = type_counts.get("numeric", 0) + 1 elif isinstance(v, list): type_counts["array"] = type_counts.get("array", 0) + 1 @@ -532,7 +532,7 @@ class TelemetryCollector: dist.looks_like_id = dist.unique_ratio > 0.9 and dist.avg_length > 5 elif field_type == "numeric": - num_values = [v for v in values if isinstance(v, (int, float))] + num_values = [v for v in values if isinstance(v, int | float)] # Filter out infinity and NaN which can cause issues num_values = [ v @@ -744,8 +744,19 @@ def get_telemetry_collector( if _telemetry_collector is None: with _collector_lock: if _telemetry_collector is None: - # Check environment for opt-out - if os.environ.get("HEADROOM_TELEMETRY_DISABLED", "").lower() in ("1", "true"): + # Honour HEADROOM_TELEMETRY (the documented opt-out var, + # also used by the Supabase beacon at telemetry/beacon.py). + # Pre-#390 this only checked HEADROOM_TELEMETRY_DISABLED, + # so users who set HEADROOM_TELEMETRY=off (the value in + # the docs) still saw /v1/telemetry report enabled=true. + # HEADROOM_TELEMETRY_DISABLED stays accepted for back-compat. + from headroom.telemetry.beacon import is_telemetry_enabled + + disabled_legacy = os.environ.get("HEADROOM_TELEMETRY_DISABLED", "").lower() in ( + "1", + "true", + ) + if disabled_legacy or not is_telemetry_enabled(): config = config or TelemetryConfig() config.enabled = False diff --git a/tests/test_telemetry.py b/tests/test_telemetry.py index 4ad51cf3c..b00c52d3f 100644 --- a/tests/test_telemetry.py +++ b/tests/test_telemetry.py @@ -576,6 +576,40 @@ class TestGlobalTelemetryCollector: assert collector._config.enabled is False + @pytest.mark.parametrize("off_value", ["off", "false", "0", "no", "disable", "disabled"]) + def test_headroom_telemetry_off_disables_collector(self, monkeypatch, off_value): + """HEADROOM_TELEMETRY=off (and other documented opt-out values) disables + the collector — closes #390. + + Pre-#390 the collector only honoured HEADROOM_TELEMETRY_DISABLED, which + is undocumented. Users following the docs set HEADROOM_TELEMETRY=off and + watched /v1/telemetry continue to report enabled=true. The collector now + consults `is_telemetry_enabled()` (the same predicate the Supabase beacon + uses), so both env vars take effect. + """ + reset_telemetry_collector() + monkeypatch.delenv("HEADROOM_TELEMETRY_DISABLED", raising=False) + monkeypatch.setenv("HEADROOM_TELEMETRY", off_value) + + collector = get_telemetry_collector() + + assert collector._config.enabled is False, ( + f"HEADROOM_TELEMETRY={off_value!r} must disable the collector — " + "this is the documented opt-out path. If this assertion fails the " + "collector is silently ignoring the user's opt-out and /v1/telemetry " + "will report enabled=true even when telemetry is supposed to be off." + ) + + def test_headroom_telemetry_on_keeps_collector_enabled(self, monkeypatch): + """Sanity check: the explicit on/unset path leaves the collector enabled.""" + reset_telemetry_collector() + monkeypatch.delenv("HEADROOM_TELEMETRY_DISABLED", raising=False) + monkeypatch.setenv("HEADROOM_TELEMETRY", "on") + + collector = get_telemetry_collector() + + assert collector._config.enabled is True + class TestRetrievalStatsModel: """Test RetrievalStats data model."""