From b4ff802ddf7d9d99a5ac0c2b2039b5c1f63c9279 Mon Sep 17 00:00:00 2001 From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com> Date: Sun, 1 Nov 2026 12:30:35 -0600 Subject: [PATCH] report notice counts cumulatively from a database baseline Moving watermarks after each accepted send kept the install ID and the notice baseline out of step: pre-consent notices went out, notices raised before the first send were dropped, and a send that finished after an off/on toggle overwrote the new identity's baseline. health.notices now carries cumulative counts since the install ID was created. Creating an ID copies the lifetime counts into the existing reported_* columns before the state file names it, and nothing is written after a send, so the backend diffs consecutive reports per install ID. --- docs/static/frigate-analytics-schema.json | 8 +- frigate/analytics/collectors/health.py | 12 +- frigate/analytics/report.py | 4 +- frigate/analytics/reporter.py | 40 +++--- frigate/analytics/schema.py | 9 +- frigate/models.py | 2 +- frigate/notices/registry.py | 19 +-- .../test/test_analytics_collectors_health.py | 2 +- frigate/test/test_analytics_report.py | 36 +++++ frigate/test/test_analytics_reporter.py | 131 ++++++++++++------ frigate/test/test_notice_registry.py | 32 +++-- 11 files changed, 191 insertions(+), 104 deletions(-) diff --git a/docs/static/frigate-analytics-schema.json b/docs/static/frigate-analytics-schema.json index 10c12fb697..d861a44c08 100644 --- a/docs/static/frigate-analytics-schema.json +++ b/docs/static/frigate-analytics-schema.json @@ -906,7 +906,7 @@ "additionalProperties": { "$ref": "#/$defs/NoticeCounts" }, - "description": "Notice counts by kind since the last accepted report", + "description": "Cumulative notice counts by kind since the install ID was created", "propertyNames": { "$ref": "#/$defs/NoticeKindKey" }, @@ -1103,21 +1103,21 @@ "additionalProperties": false, "properties": { "acknowledgements": { - "description": "Acknowledgements since the last accepted report", + "description": "Acknowledgements since the install ID was created", "minimum": 0, "title": "Acknowledgements", "type": "integer", "x-public": true }, "mutes": { - "description": "Mutes since the last accepted report", + "description": "Mutes since the install ID was created", "minimum": 0, "title": "Mutes", "type": "integer", "x-public": true }, "occurrences": { - "description": "Occurrences since the last accepted report", + "description": "Occurrences since the install ID was created", "minimum": 0, "title": "Occurrences", "type": "integer", diff --git a/frigate/analytics/collectors/health.py b/frigate/analytics/collectors/health.py index 3b8129fd7b..22d7501a81 100644 --- a/frigate/analytics/collectors/health.py +++ b/frigate/analytics/collectors/health.py @@ -23,9 +23,9 @@ TIMING_STATS = { REPORTABLE_KINDS = frozenset(key.value for key in NoticeKindKey) -def notice_deltas(notice_stats: list[dict[str, Any]]) -> dict[Any, NoticeCounts]: - """What changed since the last accepted report, per reportable kind.""" - deltas: dict[Any, NoticeCounts] = {} +def notice_counts(notice_stats: list[dict[str, Any]]) -> dict[Any, NoticeCounts]: + """Counts since the install ID was created, per reportable kind.""" + counts: dict[Any, NoticeCounts] = {} for row in notice_stats: if row["kind"] not in REPORTABLE_KINDS: @@ -38,11 +38,11 @@ def notice_deltas(notice_stats: list[dict[str, Any]]) -> dict[Any, NoticeCounts] mutes = max(row["mutes"] - row["reported_mutes"], 0) if occurrences or acknowledgements or mutes: - deltas[NoticeKindKey(row["kind"])] = NoticeCounts( + counts[NoticeKindKey(row["kind"])] = NoticeCounts( occurrences=occurrences, acknowledgements=acknowledgements, mutes=mutes ) - return deltas + return counts def collect(ctx: ReportContext) -> HealthSection: @@ -59,5 +59,5 @@ def collect(ctx: ReportContext) -> HealthSection: cpu_percent=min(round(rate(cpu)), 100), enrichment_ms={timing: value for timing, value in timings.items() if value > 0}, retention_unmet=bool(service.get("retention_unmet", False)), - notices=notice_deltas(ctx.notice_stats), + notices=notice_counts(ctx.notice_stats), ) diff --git a/frigate/analytics/report.py b/frigate/analytics/report.py index d7d38835e8..c173feae90 100644 --- a/frigate/analytics/report.py +++ b/frigate/analytics/report.py @@ -85,5 +85,7 @@ def preview_report( ) -> AnalyticsReport: """The report the next send would carry, without sending it.""" state = load_state() - ctx = gather_context(config, stats_emitter, notice_registry) + + # an opt-in resets the baseline, so before one there are no notices to show + ctx = gather_context(config, stats_emitter, notice_registry if state else None) return build_report(ctx, state.install_id if state else PREVIEW_INSTALL_ID) diff --git a/frigate/analytics/reporter.py b/frigate/analytics/reporter.py index 4539115c7b..bc4554d9bc 100644 --- a/frigate/analytics/reporter.py +++ b/frigate/analytics/reporter.py @@ -12,8 +12,7 @@ from pydantic import ValidationError from ruamel.yaml import YAML from ruamel.yaml.error import YAMLError -from frigate.analytics.context import ReportContext -from frigate.analytics.report import build_report +from frigate.analytics.report import build_report, gather_context from frigate.analytics.state import ( STATE_PATH, AnalyticsState, @@ -128,16 +127,20 @@ class AnalyticsReporter(threading.Thread): if opted_in: resolve_notice(PROMPT_KIND) - # a new install ID counts notices from the opt-in, so nothing - # raised before consent is sent and nothing raised after is lost - if load_state(self.state_path) is None and save_state( - new_state(), self.state_path - ): - self.notice_registry.mark_reported(self.notice_registry.stats()) + # the opt-in sets the notice baseline, so only notices raised with + # consent are counted; tick() retries a failed write + if load_state(self.state_path) is None: + save_state(self._new_identity(), self.state_path) else: raise_notice(PROMPT_KIND) delete_state(self.state_path) + def _new_identity(self) -> AnalyticsState: + # called with the lock held; the baseline resets before the state file + # names the new ID, so a failed write only means another reset + self.notice_registry.reset_analytics_baseline() + return new_state() + def tick(self) -> None: """Apply consent and send a report when one is due.""" with self._lock: @@ -158,9 +161,9 @@ class AnalyticsReporter(threading.Thread): if now < self.first_due: return - state = load_state(self.state_path) or new_state() + state = load_state(self.state_path) - if not self._due(state.last_attempt_at, now): + if state is not None and not self._due(state.last_attempt_at, now): return # a YAML editor save or a hand edit reaches the live config only on @@ -169,10 +172,14 @@ class AnalyticsReporter(threading.Thread): delete_state(self.state_path) return + state = AnalyticsState( + (state or self._new_identity()).install_id, last_attempt_at=now + ) + # saved before sending, so a failing endpoint or a crash mid-send # still waits a full interval; without saved state every boot would # send under a new install ID - if not save_state(AnalyticsState(state.install_id, now), self.state_path): + if not save_state(state, self.state_path): if not self.warned_unwritable: logger.warning( "Analytics is on, but %s isn't writable, so no report is sent", @@ -194,11 +201,8 @@ class AnalyticsReporter(threading.Thread): ) def _send(self, install_id: str, now: float) -> None: - notice_stats = self.notice_registry.stats() - ctx = ReportContext( - config=self.config_holder.config, - stats=self.stats_emitter.get_latest_stats(), - notice_stats=notice_stats, + ctx = gather_context( + self.config_holder.config, self.stats_emitter, self.notice_registry ) report = build_report(ctx, install_id, sent_at=int(now)) body = report.model_dump_json() @@ -210,9 +214,5 @@ class AnalyticsReporter(threading.Thread): if self.send(self.url, body) is not SendOutcome.accepted: return - # a failed health section sent no notice counts, so they stay pending - if report.health is not None: - self.notice_registry.mark_reported(notice_stats) - logger.info("Sent the daily analytics report") logger.debug("Analytics report: %s", body) diff --git a/frigate/analytics/schema.py b/frigate/analytics/schema.py index cf1da36fdc..6ad59c2dbe 100644 --- a/frigate/analytics/schema.py +++ b/frigate/analytics/schema.py @@ -378,9 +378,9 @@ class FeaturesSection(AnalyticsModel): class NoticeCounts(AnalyticsModel): - occurrences: int = count("Occurrences since the last accepted report") - acknowledgements: int = count("Acknowledgements since the last accepted report") - mutes: int = count("Mutes since the last accepted report") + occurrences: int = count("Occurrences since the install ID was created") + acknowledgements: int = count("Acknowledgements since the install ID was created") + mutes: int = count("Mutes since the install ID was created") class HealthSection(AnalyticsModel): @@ -393,7 +393,8 @@ class HealthSection(AnalyticsModel): "Whether storage can't keep the configured retention" ) notices: dict[NoticeKindKey, NoticeCounts] = metric( - "Notice counts by kind since the last accepted report", public=False + "Cumulative notice counts by kind since the install ID was created", + public=False, ) diff --git a/frigate/models.py b/frigate/models.py index a5214074fa..b816bde550 100644 --- a/frigate/models.py +++ b/frigate/models.py @@ -209,7 +209,7 @@ class NoticeStats(Model): mutes = IntegerField(default=0) first_seen = DateTimeField() last_seen = DateTimeField() - # counts the last accepted analytics report covered + # lifetime counts when the current analytics install ID was created reported_occurrences = IntegerField(default=0) reported_acknowledgements = IntegerField(default=0) reported_mutes = IntegerField(default=0) diff --git a/frigate/notices/registry.py b/frigate/notices/registry.py index 95049ef2ee..7e47689fa8 100644 --- a/frigate/notices/registry.py +++ b/frigate/notices/registry.py @@ -385,19 +385,14 @@ class NoticeRegistry: if row.kind in NOTICE_KINDS ] - def mark_reported(self, snapshot: list[dict[str, Any]]) -> None: - """Move the analytics watermarks to the counts a sent report was built from. - - Using the snapshot rather than the current counts sends anything raised - while the report was in flight with the next one. - """ + def reset_analytics_baseline(self) -> None: + """Count analytics notices from now on, for a new analytics install ID.""" with self._lock: - for row in snapshot: - NoticeStats.update( - reported_occurrences=row["occurrences"], - reported_acknowledgements=row["acknowledgements"], - reported_mutes=row["mutes"], - ).where(NoticeStats.kind == row["kind"]).execute() + NoticeStats.update( + reported_occurrences=NoticeStats.occurrences, + reported_acknowledgements=NoticeStats.acknowledgements, + reported_mutes=NoticeStats.mutes, + ).execute() def _write_repeats( self, diff --git a/frigate/test/test_analytics_collectors_health.py b/frigate/test/test_analytics_collectors_health.py index 00118d8b6f..b965594811 100644 --- a/frigate/test/test_analytics_collectors_health.py +++ b/frigate/test/test_analytics_collectors_health.py @@ -7,7 +7,7 @@ from frigate.test.analytics_helpers import make_context class TestHealthCollector(unittest.TestCase): - def test_collects_uptime_cpu_timings_and_notice_deltas(self): + def test_collects_uptime_cpu_timings_and_notices_since_the_baseline(self): stats = { "service": {"uptime": 7300, "retention_unmet": True}, "cpu_usages": {"frigate.full_system": {"cpu": "23.6", "mem": "40"}}, diff --git a/frigate/test/test_analytics_report.py b/frigate/test/test_analytics_report.py index 4669d04ea8..33a359dbef 100644 --- a/frigate/test/test_analytics_report.py +++ b/frigate/test/test_analytics_report.py @@ -13,6 +13,7 @@ from playhouse.sqliteq import SqliteQueueDatabase from frigate.analytics import report from frigate.analytics.collectors import hardware from frigate.analytics.schema import AnalyticsReport, InstallSection +from frigate.analytics.state import AnalyticsState from frigate.models import User from frigate.test.analytics_helpers import make_context from frigate.test.const import TEST_DB, TEST_DB_CLEANUPS @@ -105,3 +106,38 @@ class TestRealCollectors(unittest.TestCase): built = report.preview_report(make_context().config, stats_emitter, None) self.assertEqual(built.install_id, report.PREVIEW_INSTALL_ID) + + def test_preview_counts_notices_only_once_opted_in(self): + stats_emitter = Mock() + stats_emitter.get_latest_stats.return_value = {} + registry = Mock() + registry.stats.return_value = [ + { + "kind": "detector_stuck", + "occurrences": 3, + "acknowledgements": 0, + "mutes": 0, + "reported_occurrences": 1, + "reported_acknowledgements": 0, + "reported_mutes": 0, + } + ] + + for state, expected in ( + (None, {}), + (AnalyticsState("a" * 32, 0.0), {"detector_stuck": 2}), + ): + with ( + self.subTest(opted_in=state is not None), + patch.object(report, "load_state", return_value=state), + patch.object(hardware.hardware_prober, "probe", return_value=[]), + patch.object(hardware, "hwaccel_options", return_value=("", [])), + ): + built = report.preview_report( + make_context().config, stats_emitter, registry + ) + + self.assertEqual( + {kind: c.occurrences for kind, c in built.health.notices.items()}, + expected, + ) diff --git a/frigate/test/test_analytics_reporter.py b/frigate/test/test_analytics_reporter.py index b03fc7ddf7..2bae0660ce 100644 --- a/frigate/test/test_analytics_reporter.py +++ b/frigate/test/test_analytics_reporter.py @@ -4,7 +4,7 @@ import os import tempfile import threading import unittest -from unittest.mock import Mock, call, patch +from unittest.mock import Mock, patch from frigate.analytics import reporter as reporter_module from frigate.analytics.reporter import ( @@ -20,18 +20,6 @@ from frigate.analytics.transport import SendOutcome from frigate.config.holder import ConfigHolder from frigate.test.analytics_helpers import make_config -SNAPSHOT = [ - { - "kind": "detector_stuck", - "occurrences": 3, - "acknowledgements": 0, - "mutes": 0, - "reported_occurrences": 1, - "reported_acknowledgements": 0, - "reported_mutes": 0, - } -] - class TestAnalyticsReporter(unittest.TestCase): def setUp(self): @@ -43,7 +31,6 @@ class TestAnalyticsReporter(unittest.TestCase): self.now = 1_790_000_000.0 self.send = Mock(return_value=SendOutcome.accepted) self.registry = Mock() - self.registry.stats.return_value = SNAPSHOT self.stats = Mock() self.stats.get_latest_stats.return_value = {} self.holder = ConfigHolder(make_config({"telemetry": {"analytics": True}})) @@ -63,9 +50,8 @@ class TestAnalyticsReporter(unittest.TestCase): with open(self.config_path, "w") as f: f.write(text) - def seed_state(self) -> None: - # an install that has reported before, so no fresh-ID reset runs - save_state(AnalyticsState("a" * 32, 0.0), self.path) + def seed_state(self, last_attempt_at: float = 0.0) -> None: + save_state(AnalyticsState("a" * 32, last_attempt_at), self.path) def reporter(self, path: str | None = None) -> AnalyticsReporter: # uniform(a, b) returns a: the shortest startup delay and interval @@ -101,7 +87,6 @@ class TestAnalyticsReporter(unittest.TestCase): self.send.assert_called_once_with( "https://example.test/report", '{"report": 1}' ) - self.registry.mark_reported.assert_called_once_with(SNAPSHOT) self.assertEqual(load_state(self.path).last_attempt_at, self.now) def test_sends_once_per_interval(self): @@ -118,7 +103,7 @@ class TestAnalyticsReporter(unittest.TestCase): self.assertEqual(self.send.call_count, 2) def test_a_restart_inside_the_interval_does_not_send(self): - save_state(AnalyticsState("a" * 32, self.now), self.path) + self.seed_state(self.now) reporter = self.reporter() self.advance(FIRST_DELAY_S[0]) @@ -126,7 +111,7 @@ class TestAnalyticsReporter(unittest.TestCase): self.send.assert_not_called() - def test_a_failed_send_keeps_watermarks_and_waits_a_full_interval(self): + def test_a_failed_send_waits_a_full_interval(self): self.send.return_value = SendOutcome.failed self.seed_state() reporter = self.reporter() @@ -137,10 +122,31 @@ class TestAnalyticsReporter(unittest.TestCase): reporter.tick() self.assertEqual(self.send.call_count, 1) - self.registry.mark_reported.assert_not_called() - def test_an_accepted_report_without_health_keeps_the_watermarks(self): - self.built.health = None + def assert_reset_before_the_state_file(self) -> None: + # the baseline must be in place before the file names the new ID + self.registry.reset_analytics_baseline.side_effect = lambda: self.assertIsNone( + load_state(self.path) + ) + + def test_the_opt_in_resets_the_baseline_and_writes_the_identity(self): + self.assert_reset_before_the_state_file() + reporter = self.reporter() + + # consent is seen on the first wake, well before the first send + reporter.tick() + self.registry.reset_analytics_baseline.assert_called_once() + self.assertIsNotNone(load_state(self.path)) + self.send.assert_not_called() + + # the first report counts from that reset, so nothing resets again + self.advance(FIRST_DELAY_S[0]) + reporter.tick() + + self.send.assert_called_once() + self.registry.reset_analytics_baseline.assert_called_once() + + def test_a_restart_with_saved_state_keeps_the_baseline(self): self.seed_state() reporter = self.reporter() self.advance(FIRST_DELAY_S[0]) @@ -148,28 +154,55 @@ class TestAnalyticsReporter(unittest.TestCase): reporter.tick() self.send.assert_called_once() - self.registry.mark_reported.assert_not_called() + self.registry.reset_analytics_baseline.assert_not_called() - def test_a_new_install_id_counts_notices_from_the_opt_in(self): - before = [{**SNAPSHOT[0], "occurrences": 2}] - self.registry.stats.return_value = before + def test_a_settings_opt_in_resets_the_baseline_when_saved(self): + self.holder.set(make_config()) reporter = self.reporter() - - # consent is seen on the first wake, well before the first send reporter.tick() - self.registry.mark_reported.assert_called_once_with(before) + + self.holder.set(make_config({"telemetry": {"analytics": True}})) + + self.registry.reset_analytics_baseline.assert_called_once() self.assertIsNotNone(load_state(self.path)) - self.send.assert_not_called() - # a notice raised during the startup delay goes out with the first report - self.registry.stats.return_value = SNAPSHOT + def test_a_re_opt_in_during_the_request_keeps_its_new_identity(self): + self.seed_state() + reporter = self.reporter() self.advance(FIRST_DELAY_S[0]) + created = [] + + def flip_consent(*args): + self.holder.set(make_config()) + self.holder.set(make_config({"telemetry": {"analytics": True}})) + created.append(load_state(self.path)) + return SendOutcome.accepted + + self.send.side_effect = flip_consent reporter.tick() - self.assertIs(self.build_report.call_args.args[0].notice_stats, SNAPSHOT) - self.assertEqual( - self.registry.mark_reported.call_args_list, [call(before), call(SNAPSHOT)] - ) + # nothing after the send touches the new identity or its baseline + self.assertNotEqual(created[0].install_id, "a" * 32) + self.assertEqual(load_state(self.path), created[0]) + self.registry.reset_analytics_baseline.assert_called_once() + + def test_an_opt_in_without_a_writable_state_file_resets_again_later(self): + state_dir = os.path.join(self.dir.name, "missing") + reporter = self.reporter(os.path.join(state_dir, ".analytics.json")) + + with self.assertLogs("frigate.analytics.reporter", "WARNING"): + reporter.tick() + self.advance(FIRST_DELAY_S[0]) + reporter.tick() + + self.send.assert_not_called() + os.mkdir(state_dir) + self.registry.reset_analytics_baseline.reset_mock() + self.advance(WAKE_S) + reporter.tick() + + self.send.assert_called_once() + self.registry.reset_analytics_baseline.assert_called_once() def test_an_opt_out_saved_only_to_the_file_stops_the_send(self): self.seed_state() @@ -183,6 +216,22 @@ class TestAnalyticsReporter(unittest.TestCase): self.assertFalse(os.path.exists(self.path)) self.raise_notice.assert_not_called() + def test_reversing_a_file_only_opt_out_resets_the_baseline(self): + self.seed_state() + self.save_config("telemetry:\n analytics: false\n") + reporter = self.reporter() + self.advance(FIRST_DELAY_S[0]) + reporter.tick() + + self.save_config("telemetry:\n analytics: true\n") + self.assert_reset_before_the_state_file() + self.advance(WAKE_S) + reporter.tick() + + self.assertNotEqual(load_state(self.path).install_id, "a" * 32) + self.registry.reset_analytics_baseline.assert_called_once() + self.send.assert_called_once() + def test_yaml_truthy_spellings_in_the_file_count_as_consent(self): for value in ("yes", "on", "1", "'true'"): with self.subTest(value=value): @@ -222,7 +271,7 @@ class TestAnalyticsReporter(unittest.TestCase): self.assertFalse(os.path.exists(self.path)) def test_turning_sharing_off_and_on_between_wakes_starts_a_fresh_identity(self): - save_state(AnalyticsState("a" * 32, 0.0), self.path) + self.seed_state() reporter = self.reporter() reporter.tick() @@ -236,7 +285,7 @@ class TestAnalyticsReporter(unittest.TestCase): def test_opting_out_raises_the_prompt_once_and_deletes_the_state(self): self.holder.set(make_config()) - save_state(AnalyticsState("a" * 32, 0.0), self.path) + self.seed_state() reporter = self.reporter() reporter.tick() @@ -258,7 +307,7 @@ class TestAnalyticsReporter(unittest.TestCase): def test_safe_mode_touches_nothing(self): self.holder.set(make_config({"safe_mode": True})) - save_state(AnalyticsState("a" * 32, 0.0), self.path) + self.seed_state() reporter = self.reporter() self.advance(FIRST_DELAY_S[0]) @@ -281,7 +330,7 @@ class TestAnalyticsReporter(unittest.TestCase): self.assertEqual(len(logs.output), 1) def test_a_last_attempt_in_the_future_is_ignored(self): - save_state(AnalyticsState("a" * 32, self.now + 10 * INTERVAL_S), self.path) + self.seed_state(self.now + 10 * INTERVAL_S) reporter = self.reporter() self.advance(FIRST_DELAY_S[0]) diff --git a/frigate/test/test_notice_registry.py b/frigate/test/test_notice_registry.py index 02ab10d08e..d950bea709 100644 --- a/frigate/test/test_notice_registry.py +++ b/frigate/test/test_notice_registry.py @@ -357,23 +357,27 @@ class TestNoticeRegistry(RegistryTestCase): self.assertEqual(stats["skipped_detections"]["mutes"], 1) -class TestMarkReported(RegistryTestCase): - def test_moves_watermarks_to_the_snapshot_not_the_current_count(self): +class TestAnalyticsBaseline(RegistryTestCase): + def test_reset_counts_from_now_and_new_kinds_start_at_zero(self): + stuck = {"scope": "ov", "params": {"detector": "ov"}} + self.registry.raise_notice("detector_stuck", **stuck) + self.registry.raise_notice("detector_stuck", **stuck) + + self.registry.reset_analytics_baseline() + self.registry.raise_notice("detector_stuck", **stuck) self.registry.raise_notice( - "detector_stuck", scope="ov", params={"detector": "ov"} - ) - snapshot = self.registry.stats() - self.registry.raise_notice( - "detector_stuck", scope="ov", params={"detector": "ov"} + "skipped_detections", scope="front_door", params={"pct": 12.5} ) - self.registry.mark_reported(snapshot) - - row = self.registry.stats()[0] - self.assertEqual(row["occurrences"], 2) - self.assertEqual(row["reported_occurrences"], 1) - self.assertEqual(row["reported_acknowledgements"], 0) - self.assertEqual(row["reported_mutes"], 0) + rows = {row["kind"]: row for row in self.registry.stats()} + self.assertEqual( + ( + rows["detector_stuck"]["occurrences"], + rows["detector_stuck"]["reported_occurrences"], + ), + (3, 2), + ) + self.assertEqual(rows["skipped_detections"]["reported_occurrences"], 0) class TestApply(RegistryTestCase):