From f390b7a3a47f43eb7a287c39940bfab87ae9070c Mon Sep 17 00:00:00 2001 From: MountainOwl Date: Tue, 20 Oct 2026 16:50:04 +0000 Subject: [PATCH 1/2] fix(analytics): send session_start once per process per day The service calls poll() in one process every few minutes, and each poll sent session_start, so an idle install produced about 96 liveness events a day. It is now sent once per process per UTC day, which still shows a long-running idle service as alive. A day is not spent while analytics consent is off. --- src/bubo/analytics.py | 23 +++++++++++- tests/conftest.py | 1 + tests/test_session_start.py | 72 +++++++++++++++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 tests/test_session_start.py diff --git a/src/bubo/analytics.py b/src/bubo/analytics.py index c6b2d8a..dfefc43 100644 --- a/src/bubo/analytics.py +++ b/src/bubo/analytics.py @@ -39,6 +39,7 @@ import threading import uuid from collections.abc import Iterable +from datetime import UTC, datetime from pathlib import Path from typing import Any from urllib.request import Request, urlopen @@ -194,6 +195,9 @@ def __init__(self, config, event, properties): _install_node: int | None = None _identity_lock = threading.Lock() _surface = "unknown" +# UTC day this process last sent session_start; the service polls in-process. +_session_day: str | None = None +_session_lock = threading.Lock() # Only these labels may leave the machine; paths are used locally and discarded. _LANGUAGES = { @@ -447,8 +451,25 @@ def track(cfg: AnalyticsConfig, event: str, **props: Any) -> None: _emit(cfg, event, props) +def _utc_day() -> str: + return datetime.now(UTC).date().isoformat() + + def record_session_start(cfg: AnalyticsConfig, *, scm_provider: str, projects_count: int) -> None: - """One event per poll cycle start — liveness + install context.""" + """Liveness + install context: once per process per UTC day. + + The service calls ``poll()`` every few minutes in one process; sending + this per poll made it the bulk of all events. Once a day still shows a + long-running idle service as alive. A day isn't spent while consent is off. + """ + global _session_day + if not analytics_enabled(cfg): + return + day = _utc_day() + with _session_lock: + if _session_day == day: + return + _session_day = day _emit( cfg, "session_start", diff --git a/tests/conftest.py b/tests/conftest.py index d2e775b..ca61991 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -64,6 +64,7 @@ def offline(): posthog.configure_otel(TelemetryConfig(analytics=AnalyticsConfig(enabled=False))) with analytics._pending_lock: analytics._pending_events.clear() + analytics._session_day = None assert not attempted, "A test attempted external telemetry without a fake transport" diff --git a/tests/test_session_start.py b/tests/test_session_start.py new file mode 100644 index 0000000..88064d9 --- /dev/null +++ b/tests/test_session_start.py @@ -0,0 +1,72 @@ +"""``session_start`` is a liveness signal, not a per-poll event. + +The service calls ``poll()`` in one process every few minutes. Drives the +real ``poll()`` with an idle config and counts the queued PostHog events. +""" + +from __future__ import annotations + +import tempfile +from collections.abc import Iterator +from contextlib import contextmanager +from pathlib import Path +from unittest.mock import MagicMock, patch + +from bubo import analytics, db, paths, poller +from bubo.analytics_config import AnalyticsConfig +from bubo.review_config import ReviewConfig + +CONSENT = AnalyticsConfig(enabled=True, endpoint="https://posthog.invalid/batch", api_key="k") + + +@contextmanager +def _state() -> Iterator[None]: + old = paths.DB + try: + with tempfile.TemporaryDirectory() as tmp: + paths.DB = Path(tmp) / "state" / "reviewer.sqlite" + paths.DB.parent.mkdir() + db.init_db() + yield + finally: + paths.DB = old + + +def _session_starts(cfg: ReviewConfig, polls: int, days: list[str] | None = None) -> int: + provider = MagicMock() + provider.name = "github" + provider.token.return_value = "t" + with ( + _state(), + patch.object(poller, "read_config", return_value=cfg), + patch.object(poller, "get_provider", return_value=provider), + patch.object(analytics, "flush"), + patch.object(analytics, "_utc_day", side_effect=days or ["2026-10-20"] * polls), + ): + for _ in range(polls): + poller.poll(worker_mode="inline") + with analytics._pending_lock: + return sum( + 1 for *_, queued in analytics._pending_events if queued["event"] == "session_start" + ) + + +def _idle() -> ReviewConfig: + return ReviewConfig(provider="github", projects=[], analytics_config=CONSENT) + + +def test_repeated_polls_in_one_process_send_one_session_start() -> None: + assert _session_starts(_idle(), polls=3) == 1 + + +def test_a_long_running_service_still_checks_in_once_a_day() -> None: + days = ["2026-10-20", "2026-10-20", "2026-10-21", "2026-10-21"] + assert _session_starts(_idle(), polls=4, days=days) == 2 + + +def test_session_start_is_not_spent_while_analytics_is_off() -> None: + off = ReviewConfig( + provider="github", projects=[], analytics_config=AnalyticsConfig(enabled=False) + ) + assert _session_starts(off, polls=1) == 0 + assert _session_starts(_idle(), polls=1) == 1 From 6cd1cb72dc84a448df2367d423ab3655bfb04fb9 Mon Sep 17 00:00:00 2001 From: MountainOwl Date: Tue, 20 Oct 2026 17:08:27 +0000 Subject: [PATCH 2/2] refactor(analytics): track session_start days in a set CodeQL reported the module-level day as an unused global because it was only reassigned through a global statement. --- src/bubo/analytics.py | 9 ++++----- tests/conftest.py | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/bubo/analytics.py b/src/bubo/analytics.py index dfefc43..755a0bb 100644 --- a/src/bubo/analytics.py +++ b/src/bubo/analytics.py @@ -195,8 +195,8 @@ def __init__(self, config, event, properties): _install_node: int | None = None _identity_lock = threading.Lock() _surface = "unknown" -# UTC day this process last sent session_start; the service polls in-process. -_session_day: str | None = None +# UTC days this process sent session_start on; the service polls in-process. +_session_days: set[str] = set() _session_lock = threading.Lock() # Only these labels may leave the machine; paths are used locally and discarded. @@ -462,14 +462,13 @@ def record_session_start(cfg: AnalyticsConfig, *, scm_provider: str, projects_co this per poll made it the bulk of all events. Once a day still shows a long-running idle service as alive. A day isn't spent while consent is off. """ - global _session_day if not analytics_enabled(cfg): return day = _utc_day() with _session_lock: - if _session_day == day: + if day in _session_days: return - _session_day = day + _session_days.add(day) _emit( cfg, "session_start", diff --git a/tests/conftest.py b/tests/conftest.py index ca61991..940251a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -64,7 +64,7 @@ def offline(): posthog.configure_otel(TelemetryConfig(analytics=AnalyticsConfig(enabled=False))) with analytics._pending_lock: analytics._pending_events.clear() - analytics._session_day = None + analytics._session_days.clear() assert not attempted, "A test attempted external telemetry without a fake transport"