refactor(health-report): put summary where the other writer puts it
check_history has two producers. sysmon-go writes `summary` at the top level beside `status`; this module wrote it under `metrics`. So a reader had to know which producer wrote a row before it could find out what the row said, and a query written the obvious way found one and silently missed the other. That is the T-36 failure repeating. There, per-domain queries returned rows from August and looked like a system that had stopped reporting, because the data was nested under a composite row nobody had mentioned. Nothing was missing; the query was asking the wrong shape. verify.sh had already grown a coalesce over both spellings, which is the tell: a compatibility shim that hides a schema disagreement rather than resolving it. D-33 made this table a contract between producers, and a contract needs one spelling. Summary is now a required parameter with no default. sysmon-go enforces the same thing through Domain.Run's signature, and the reason is identical: a row whose substance is missing looks exactly like a row whose check found nothing to say. Both call sites pass it; the failure path passes the exception rather than leaving the field to the metrics blob. Old rows keep the nested spelling and verify.sh keeps reading both, because rewriting history to match a new convention is a worse trade than a fallback with a reason attached. Also drops "Three consequences" from the module docstring, which by then listed five. A hardcoded count beside the thing it counts is the same defect as install.sh printing "wrote 8 keys" while writing ten — this morning's bug, in prose instead of code. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -7,9 +7,10 @@ nothing naming the task. The health record could not answer which job broke,
|
||||
which is most of what a health record is for.
|
||||
|
||||
Executors are called as `execute(config, settings)` and are never told which
|
||||
task they are, so the name travels in a ContextVar. These tests pin the three
|
||||
properties that makes safe: it reaches the reporter, it survives the worker
|
||||
thread T-74 introduced, and concurrent executions cannot read each other's.
|
||||
task they are, so the name travels in a ContextVar. These tests pin what makes
|
||||
that safe: it reaches the reporter, it survives the worker thread T-74
|
||||
introduced, and concurrent executions cannot read each other's. They also pin
|
||||
where the summary lives, since two producers write this table.
|
||||
"""
|
||||
import asyncio
|
||||
import json
|
||||
@@ -44,7 +45,8 @@ def captured_row(monkeypatch):
|
||||
def _report(settings, **kw):
|
||||
health_report.report(
|
||||
settings, domain="backup", status=health_report.OK,
|
||||
source="scheduler/config_backup_executor", metrics={}, **kw
|
||||
source="scheduler/config_backup_executor",
|
||||
summary="backed up 3 sources", metrics={}, **kw
|
||||
)
|
||||
|
||||
|
||||
@@ -89,7 +91,8 @@ class TestAttribution:
|
||||
with task_scope("backup_portainer_daily"):
|
||||
await health_report.report_async(
|
||||
test_settings, domain="backup", status=health_report.OK,
|
||||
source="scheduler/portainer_backup_executor", metrics={},
|
||||
source="scheduler/portainer_backup_executor",
|
||||
summary="backed up Portainer", metrics={},
|
||||
)
|
||||
assert captured_row['result']['task'] == "backup_portainer_daily"
|
||||
|
||||
@@ -152,3 +155,35 @@ class TestAttributionIsolation:
|
||||
)
|
||||
|
||||
assert seen == {"slow_one": "slow_one", "fast_one": "fast_one"}
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestSummaryPlacement:
|
||||
"""The two writers of check_history must agree where the substance lives.
|
||||
|
||||
sysmon-go writes `summary` at the top level, beside `status`. This module
|
||||
wrote it under `metrics` until 2026-08-11, so a reader had to know which
|
||||
producer wrote a row before it could find out what the row said — and a
|
||||
query written the obvious way silently found half the data. That is the T-36
|
||||
failure exactly, where per-domain queries returned nothing because the value
|
||||
was nested somewhere else.
|
||||
"""
|
||||
|
||||
def test_summary_is_top_level(self, test_settings: Settings, captured_row):
|
||||
_report(test_settings)
|
||||
r = captured_row['result']
|
||||
assert r['summary'] == "backed up 3 sources"
|
||||
assert 'summary' not in r['metrics'], "summary must not also live under metrics"
|
||||
|
||||
def test_summary_is_required(self, test_settings: Settings, captured_row):
|
||||
"""Omitting it is an error at the call, not a silently empty column.
|
||||
|
||||
sysmon-go enforces this through Domain.Run's signature; a parameter with
|
||||
no default is the equivalent here. A row whose substance is missing looks
|
||||
exactly like a row whose check found nothing to say.
|
||||
"""
|
||||
with pytest.raises(TypeError):
|
||||
health_report.report(
|
||||
test_settings, domain="backup", status=health_report.OK,
|
||||
source="scheduler/x", metrics={},
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user