From 731e7cd6e68f7622276971e40af0dc4da342c19f Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 9 Oct 2026 09:58:29 -0400 Subject: [PATCH] fix(alerts): a metric picked on one part doesn't follow you to another (#5475) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #5475 "Rule form: pick the host, then its part…". CI run 8909 caught it: moving from the whole host to a mount kept "CPU" in the mount's Metric list, because a chosen metric always stayed on offer. Only a rule's own page keeps its resource and metric after they go quiet (keep=True from edit_rule); the pickers' swaps drop a metric the new part doesn't report. Co-Authored-By: Claude Opus 5.5 --- steward/alerts/routes.py | 10 ++++++---- steward/alerts/service.py | 9 +++++---- tests/core/test_alert_rule_target.py | 9 ++++++++- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/steward/alerts/routes.py b/steward/alerts/routes.py index 7e7945b..d0107aa 100644 --- a/steward/alerts/routes.py +++ b/steward/alerts/routes.py @@ -295,10 +295,12 @@ async def cover_groups_api(): async def _target(db, source: str, *, host: str = "", resource: str = "", metric: str = "", - covers: str = "resource") -> dict: - """The rule form's target pickers and the chosen metric's preview (#5475).""" + covers: str = "resource", keep: bool = False) -> dict: + """The rule form's target pickers and the chosen metric's preview (#5475); + `keep` on a rule's own page, so its resource and metric stay on offer.""" from steward.core.status import sparkline_svg - target = await alert_svc.rule_target(db, source, host=host, resource=resource, metric=metric) + target = await alert_svc.rule_target(db, source, host=host, resource=resource, metric=metric, + keep=keep) preview = None if target["resource"] and target["metric"]: preview = await alert_svc.metric_preview(db, source, target["resource"], target["metric"]) @@ -360,7 +362,7 @@ async def edit_rule(rule_id: str): return redirect(f"/alerts/rules/{rule.parent_id}/edit") ctx = {**await _cover_choices(db, rule.source_module, rule), **await _target(db, rule.source_module, resource=rule.resource_name, - metric=rule.metric_name, covers=rule.covers)} + metric=rule.metric_name, covers=rule.covers, keep=True)} hosts = sorted((c.resource_name for c in (await db.execute( select(AlertRule).where(AlertRule.parent_id == rule.id))).scalars()), key=str.lower) history_count = (await db.execute( diff --git a/steward/alerts/service.py b/steward/alerts/service.py index eda3c90..433b804 100644 --- a/steward/alerts/service.py +++ b/steward/alerts/service.py @@ -391,13 +391,14 @@ def alertable(source_module: str, reported: set[str]) -> list[str]: async def rule_target(db, source_module: str, *, host: str = "", resource: str = "", - metric: str = "") -> dict: + metric: str = "", keep: bool = False) -> dict: """What the rule form's target pickers offer (#5475), from the latest-value table: the resources that report something a rule can watch, and for the chosen one the metrics it reports. For a host source the resources split into a Host picker and a Part picker ("the whole host", then the host's - mounts, cores and so on). A resource or metric already chosen stays on - offer even when it has gone quiet, so editing a rule never drops them. + mounts, cores and so on). With `keep` (a rule's own page), its resource + and metric stay on offer even after they go quiet, so opening a rule + never drops them; a metric carried over from another part is dropped. Returns host_source, hosts, host, parts [(heading, [(value, label)])], resources, resource, metrics, metric.""" @@ -434,7 +435,7 @@ async def rule_target(db, source_module: str, *, host: str = "", resource: str = else: out["resources"] = sorted(pickable, key=str.lower) metrics = list(pickable.get(out["resource"]) or catalog) - if metric and metric not in metrics and (metric in catalog or not catalog): + if keep and metric and metric not in metrics and (metric in catalog or not catalog): metrics.append(metric) out.update(metrics=metrics, metric=metric if metric in metrics else "") return out diff --git a/tests/core/test_alert_rule_target.py b/tests/core/test_alert_rule_target.py index 1ac0d51..29fcd47 100644 --- a/tests/core/test_alert_rule_target.py +++ b/tests/core/test_alert_rule_target.py @@ -59,7 +59,14 @@ async def test_picking_another_host_starts_at_the_whole_host(): async def test_a_rule_s_own_resource_stays_on_offer_after_it_goes_quiet(): got = await alert_svc.rule_target(_db(), "host_agent", resource="h3:/data", - metric="disk_used_pct") + metric="disk_used_pct", keep=True) assert "h3" in got["hosts"] and got["host"] == "h3" assert got["parts"] == [("Mounts", [("h3:/data", "/data")])] assert got["metric"] == "disk_used_pct" + # A reporting part whose metric went quiet keeps it on the rule's own page. + got = await alert_svc.rule_target(_db(), "host_agent", resource="h1:/var", metric="disk_free_pct", + keep=True) + assert got["metric"] == "" # not in the catalog: not offered + got = await alert_svc.rule_target(_db(), "host_agent", resource="h1:/var", metric="uptime_secs", + keep=True) + assert got["metrics"] == ["disk_used_pct", "uptime_secs"] and got["metric"] == "uptime_secs"