feat(alerts): an acknowledgement is on the record, with who made it (#5463)
#5463 "Acknowledgements on record: an event and who did it", step 1 of milestone 499 "Alerts you can trust". The live walk in #5461 found that acknowledging changed only the alert's current state: once it resolved, nothing said who had acknowledged it or when. - alert_events.actor_id (migration 0040): the person behind a transition. - Acknowledging writes a firing → acknowledged event by the acknowledger. - A rule edit, disable or enable that resets an alert carries its actor (web and MCP alike). - The history page notes "acknowledged by X" and "rule disabled by X"; MCP get_alert_history returns `by`. - One usernames() helper in alerts.service, used by the active list, the history page and the MCP tools. - docs/core/alerting.md: the state diagram no longer shows an acknowledged alert re-firing, and says what an event records. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -144,8 +144,9 @@ async def active_fragment():
|
||||
rules_off=sum(1 for r in rules if not r.enabled))
|
||||
|
||||
|
||||
def _history_row(event, rule) -> dict:
|
||||
"""One transition as the history page shows it."""
|
||||
def _history_row(event, rule, names: dict | None = None) -> dict:
|
||||
"""One transition as the history page shows it. `names` maps user ids to
|
||||
usernames: a transition a person made says who (#5463)."""
|
||||
from steward.core.alerts import MUTED, RELEASED
|
||||
from steward.core.formatting import metric_label, metric_value
|
||||
notes, muted_by, released = [], None, False
|
||||
@@ -156,6 +157,12 @@ def _history_row(event, rule) -> dict:
|
||||
released = True
|
||||
elif part:
|
||||
notes.append(part)
|
||||
actor = getattr(event, "actor_id", None)
|
||||
by = (names or {}).get(actor) or ("a deleted user" if actor else None)
|
||||
if event.to_state == "acknowledged":
|
||||
notes.insert(0, f"acknowledged by {by}" if by else "acknowledged")
|
||||
elif by and notes:
|
||||
notes[0] = f"{notes[0]} by {by}"
|
||||
metric = rule.metric_name
|
||||
if event.metric_value is None:
|
||||
value = f"No {metric_label(metric)} reading"
|
||||
@@ -193,14 +200,15 @@ async def alert_history():
|
||||
rule_id = chosen.id if chosen else None
|
||||
rows = await alert_svc.history(db, since, rule_id, PAGE_SIZE, before)
|
||||
show_notified, show_action = await alert_svc.history_columns(db, since, rule_id)
|
||||
names = await alert_svc.usernames(db, (e.actor_id for e, _ in rows))
|
||||
verdict = None
|
||||
if chosen and not before:
|
||||
every = await alert_svc.history(db, since, rule_id, HISTORY_SUMMARY_ROWS)
|
||||
verdict = summary(episodes([_history_row(e, r) for e, r in every], newest_page=True,
|
||||
now=now, since=since))
|
||||
rows, after = next_page(rows, lambda row: (row[0].transitioned_at, row[0].id))
|
||||
ctx = {"episodes": episodes([_history_row(e, r) for e, r in rows], newest_page=not before,
|
||||
now=now, since=since),
|
||||
ctx = {"episodes": episodes([_history_row(e, r, names) for e, r in rows],
|
||||
newest_page=not before, now=now, since=since),
|
||||
"after": after, "rules": rules, "chosen": chosen, "current_range": range_key,
|
||||
"show_notified": show_notified, "show_action": show_action, "verdict": verdict}
|
||||
if before:
|
||||
@@ -327,7 +335,7 @@ async def update_rule(rule_id: str):
|
||||
fields = await alert_svc.rule_fields(db, **_rule_form(form))
|
||||
except ValueError as exc:
|
||||
return str(exc), 400
|
||||
await alert_svc.apply_rule_change(db, rule, fields)
|
||||
await alert_svc.apply_rule_change(db, rule, fields, actor_id=session.get("user_id"))
|
||||
if apply_action:
|
||||
rule.ansible_action = ansible_action
|
||||
await log_audit(current_app, session.get("user_id"), session.get("username", ""),
|
||||
@@ -344,7 +352,8 @@ async def toggle_rule(rule_id: str):
|
||||
rule = result.scalar_one_or_none()
|
||||
if rule is None:
|
||||
return "Not found", 404
|
||||
await alert_svc.apply_rule_change(db, rule, {}, enabled=not rule.enabled)
|
||||
await alert_svc.apply_rule_change(db, rule, {}, enabled=not rule.enabled,
|
||||
actor_id=session.get("user_id"))
|
||||
action = "alert_rule.enabled" if rule.enabled else "alert_rule.disabled"
|
||||
await log_audit(current_app, session.get("user_id"), session.get("username", ""),
|
||||
action, entity_type="alert_rule", entity_id=rule.name)
|
||||
|
||||
+23
-11
@@ -203,18 +203,24 @@ async def breach_started(db, rule_ids) -> dict[str, datetime]:
|
||||
)).all())
|
||||
|
||||
|
||||
async def usernames(db, ids) -> dict[str, str]:
|
||||
"""{user id: username} for the given ids (None and repeats ignored)."""
|
||||
from steward.models.users import User
|
||||
ids = {i for i in ids if i}
|
||||
if not ids:
|
||||
return {}
|
||||
return dict((await db.execute(select(User.id, User.username).where(User.id.in_(ids)))).all())
|
||||
|
||||
|
||||
async def active_rows(db, now: datetime) -> list[dict]:
|
||||
"""The alerts page's active list, firing first, then no data, then
|
||||
acknowledged, then pending; within each the longest-running first. Each
|
||||
row: its state, rule, label, sentence, the link to what it watches, when
|
||||
the breach began, and who acknowledged it."""
|
||||
from steward.models.users import User
|
||||
alerts = await active_alerts(db)
|
||||
started = await breach_started(db, {rule.id for state, rule in alerts
|
||||
if state.state.value in _BREACHING})
|
||||
ackers = {s.acknowledged_by for s, _ in alerts if s.acknowledged_by}
|
||||
names = dict((await db.execute(
|
||||
select(User.id, User.username).where(User.id.in_(ackers)))).all()) if ackers else {}
|
||||
names = await usernames(db, (s.acknowledged_by for s, _ in alerts))
|
||||
rows = []
|
||||
for state, rule in alerts:
|
||||
label = alert_label(state)
|
||||
@@ -430,11 +436,12 @@ async def rule_fields(db, *, name: str, source_module: str, resource_name: str,
|
||||
WATCHED_FIELDS = ("source_module", "resource_name", "metric_name", "operator", "threshold")
|
||||
|
||||
|
||||
async def apply_rule_change(db, rule: AlertRule, fields: dict, *, enabled: bool | None = None) -> None:
|
||||
async def apply_rule_change(db, rule: AlertRule, fields: dict, *, enabled: bool | None = None,
|
||||
actor_id: str | None = None) -> None:
|
||||
"""Apply validated fields (and optionally enabled) to a rule, in the
|
||||
caller's transaction. A change to what it watches, or disabling or
|
||||
re-enabling it, puts its alert back to inactive and starts its silence
|
||||
watch over (core.alerts.reset_rule_state)."""
|
||||
watch over (core.alerts.reset_rule_state); the event says who did it."""
|
||||
from steward.core.alerts import reset_rule_state
|
||||
changed = [k for k in WATCHED_FIELDS if k in fields and getattr(rule, k) != fields[k]]
|
||||
for k, v in fields.items():
|
||||
@@ -444,11 +451,11 @@ async def apply_rule_change(db, rule: AlertRule, fields: dict, *, enabled: bool
|
||||
if enabled is not None:
|
||||
rule.enabled = bool(enabled)
|
||||
if disabling:
|
||||
await reset_rule_state(db, rule, "rule disabled")
|
||||
await reset_rule_state(db, rule, "rule disabled", actor_id)
|
||||
elif enabling:
|
||||
await reset_rule_state(db, rule, "rule enabled")
|
||||
await reset_rule_state(db, rule, "rule enabled", actor_id)
|
||||
elif changed:
|
||||
await reset_rule_state(db, rule, "rule changed: " + ", ".join(changed))
|
||||
await reset_rule_state(db, rule, "rule changed: " + ", ".join(changed), actor_id)
|
||||
|
||||
|
||||
async def add_rule(db, fields: dict, *, created_by: str, enabled: bool = True,
|
||||
@@ -462,11 +469,14 @@ async def add_rule(db, fields: dict, *, created_by: str, enabled: bool = True,
|
||||
return rule
|
||||
|
||||
|
||||
async def acknowledge(db, rule_id: str, user_id: str) -> AlertState:
|
||||
"""Acknowledge a firing alert, in the caller's transaction.
|
||||
async def acknowledge(db, rule_id: str, user_id: str | None) -> AlertState:
|
||||
"""Acknowledge a firing alert, in the caller's transaction, and record it
|
||||
in the alert's history as a firing → acknowledged event by `user_id`, so
|
||||
who saw it outlives the alert (#5463).
|
||||
|
||||
LookupError when the rule has no state; ValueError when it is not firing.
|
||||
"""
|
||||
from steward.core.alerts import _add_event
|
||||
state = (await db.execute(
|
||||
select(AlertState).where(AlertState.rule_id == rule_id))).scalar_one_or_none()
|
||||
if state is None:
|
||||
@@ -479,6 +489,8 @@ async def acknowledge(db, rule_id: str, user_id: str) -> AlertState:
|
||||
state.acknowledged_by = user_id
|
||||
state.acknowledged_at = now
|
||||
state.last_transition_at = now
|
||||
_add_event(db, rule_id, AlertStateEnum.firing.value, AlertStateEnum.acknowledged.value,
|
||||
None, now, actor_id=user_id)
|
||||
return state
|
||||
|
||||
|
||||
|
||||
@@ -432,19 +432,22 @@ async def _evaluate_rule(
|
||||
return notifications
|
||||
|
||||
|
||||
async def reset_rule_state(session: AsyncSession, rule: AlertRule, note: str) -> None:
|
||||
async def reset_rule_state(session: AsyncSession, rule: AlertRule, note: str,
|
||||
actor_id: str | None = None) -> None:
|
||||
"""Put a rule's alert back to inactive because the rule itself changed
|
||||
(what it watches, its threshold, or it was disabled or re-enabled), and
|
||||
start its silence watch over. An alert that was
|
||||
firing or acknowledged gets a resolved event carrying `note`; nobody is
|
||||
notified, since nothing about the watched resource changed."""
|
||||
firing or acknowledged gets a resolved event carrying `note` and the
|
||||
person who changed the rule (`actor_id`); nobody is notified, since
|
||||
nothing about the watched resource changed."""
|
||||
state = (await session.execute(
|
||||
select(AlertState).where(AlertState.rule_id == rule.id))).scalar_one_or_none()
|
||||
if state is None:
|
||||
return
|
||||
now = datetime.now(timezone.utc)
|
||||
if state.state in (AlertStateEnum.firing, AlertStateEnum.acknowledged):
|
||||
_add_event(session, rule.id, state.state.value, "resolved", None, now, note=note)
|
||||
_add_event(session, rule.id, state.state.value, "resolved", None, now, note=note,
|
||||
actor_id=actor_id)
|
||||
# The watch starts over now: silence is measured from here, against the
|
||||
# cadence of whatever the rule watches from now on.
|
||||
state.last_transition_at = now
|
||||
@@ -466,6 +469,7 @@ def _add_event(
|
||||
value: float | None,
|
||||
now: datetime,
|
||||
note: str | None = None,
|
||||
actor_id: str | None = None,
|
||||
) -> str:
|
||||
"""Add an AlertEvent row and return its ID."""
|
||||
event_id = str(uuid.uuid4())
|
||||
@@ -477,6 +481,7 @@ def _add_event(
|
||||
metric_value=value,
|
||||
transitioned_at=now,
|
||||
note=note,
|
||||
actor_id=actor_id,
|
||||
)
|
||||
session.add(event)
|
||||
return event_id
|
||||
|
||||
@@ -8,7 +8,7 @@ from sqlalchemy import select
|
||||
from steward.alerts import service as alert_svc
|
||||
from steward.mcp._context import current_caller
|
||||
from steward.models.alerts import AlertOperator, AlertRule, AlertState, MaintenanceWindow
|
||||
from steward.models.users import User, UserRole
|
||||
from steward.models.users import UserRole
|
||||
|
||||
from ._audit import audit_mcp
|
||||
from ._shape import (
|
||||
@@ -33,13 +33,6 @@ def _action(rule: AlertRule) -> dict | None:
|
||||
"extra_vars", "check") if a.get(k) is not None}
|
||||
|
||||
|
||||
async def _usernames(db, ids) -> dict[str, str]:
|
||||
ids = {i for i in ids if i}
|
||||
if not ids:
|
||||
return {}
|
||||
return dict((await db.execute(select(User.id, User.username).where(User.id.in_(ids)))).all())
|
||||
|
||||
|
||||
async def list_alerts() -> dict:
|
||||
"""Alerts that need attention now: firing, pending, acknowledged but still
|
||||
breaching, or gone silent (no_data).
|
||||
@@ -56,7 +49,7 @@ async def list_alerts() -> dict:
|
||||
"""
|
||||
async with db_session() as db:
|
||||
active = await alert_svc.active_alerts(db)
|
||||
names = await _usernames(db, [s.acknowledged_by for s, _ in active])
|
||||
names = await alert_svc.usernames(db, (s.acknowledged_by for s, _ in active))
|
||||
order = {"firing": 0, "no_data": 1, "pending": 2, "acknowledged": 3}
|
||||
rows = [
|
||||
{
|
||||
@@ -89,7 +82,9 @@ async def get_alert_history(rule: str = "", range: str = "7d", limit: int = 50)
|
||||
|
||||
Each transition carries the metric value that caused it and whether the
|
||||
email/webhook notification went out (with the error if not), plus the id
|
||||
of any Ansible run the rule's action triggered.
|
||||
of any Ansible run the rule's action triggered. `by` names the person who
|
||||
made it happen (an acknowledgement, or a rule edit that reset the alert);
|
||||
null when a reading or Steward itself did.
|
||||
"""
|
||||
since, key = lookback(range)
|
||||
limit = clamp_limit(limit, 50, 500)
|
||||
@@ -97,6 +92,7 @@ async def get_alert_history(rule: str = "", range: str = "7d", limit: int = 50)
|
||||
rules = await alert_svc.list_rules(db)
|
||||
chosen = pick(rule, rules, kind="alert rule", list_tool="list_alert_rules") if rule else None
|
||||
rows = await alert_svc.history(db, since, chosen.id if chosen else None, limit)
|
||||
names = await alert_svc.usernames(db, (e.actor_id for e, _ in rows))
|
||||
events = [
|
||||
{
|
||||
"rule_id": r.id,
|
||||
@@ -105,6 +101,7 @@ async def get_alert_history(rule: str = "", range: str = "7d", limit: int = 50)
|
||||
"to_state": e.to_state,
|
||||
"metric_value": rnd(e.metric_value),
|
||||
"note": e.note,
|
||||
"by": names.get(e.actor_id, e.actor_id),
|
||||
"at": iso(e.transitioned_at),
|
||||
"age_s": age_s(e.transitioned_at),
|
||||
"email_sent": e.email_sent,
|
||||
@@ -296,7 +293,8 @@ async def update_alert_rule(rule: str, name: str | None = None, source_module: s
|
||||
consecutive_ok_required=(r.consecutive_ok_required
|
||||
if clear_after_checks is None else clear_after_checks),
|
||||
notify_no_data=r.notify_no_data if notify_no_data is None else notify_no_data)
|
||||
await alert_svc.apply_rule_change(db, r, fields, enabled=enabled)
|
||||
await alert_svc.apply_rule_change(db, r, fields, enabled=enabled,
|
||||
actor_id=current_caller().user_id)
|
||||
changed = sorted(k for k, v in {"name": name, "source_module": source_module,
|
||||
"resource": resource, "metric": metric, "operator": operator,
|
||||
"threshold": threshold, "consecutive_checks": consecutive_checks,
|
||||
|
||||
@@ -0,0 +1,24 @@
|
||||
"""Who made an alert transition happen (#5463): an acknowledgement, or a rule
|
||||
edit, disable or enable that reset the alert, records the person on its event.
|
||||
|
||||
Revision ID: 0040_alert_event_actor
|
||||
Revises: 0039_password_resets
|
||||
Create Date: 2026-10-09
|
||||
"""
|
||||
from typing import Sequence, Union
|
||||
from alembic import op
|
||||
import sqlalchemy as sa
|
||||
|
||||
revision: str = "0040_alert_event_actor"
|
||||
down_revision: Union[str, None] = "0039_password_resets"
|
||||
branch_labels: Union[str, Sequence[str], None] = None
|
||||
depends_on: Union[str, Sequence[str], None] = None
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
op.add_column("alert_events", sa.Column(
|
||||
"actor_id", sa.String(36), sa.ForeignKey("users.id", ondelete="SET NULL"), nullable=True))
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
op.drop_column("alert_events", "actor_id")
|
||||
@@ -124,6 +124,12 @@ class AlertEvent(Base):
|
||||
email_error: Mapped[str | None] = mapped_column(Text, nullable=True)
|
||||
webhook_sent: Mapped[bool | None] = mapped_column(Boolean, nullable=True)
|
||||
webhook_error: Mapped[str | None] = mapped_column(Text, nullable=True)
|
||||
# Who made this transition happen, when a person did: an acknowledgement,
|
||||
# or a rule edit, disable or enable that reset the alert (#5463). None for
|
||||
# every transition a sample or Steward itself caused.
|
||||
actor_id: Mapped[str | None] = mapped_column(
|
||||
String(36), ForeignKey("users.id", ondelete="SET NULL"), nullable=True
|
||||
)
|
||||
# Set when this firing event triggered an Ansible action run (audit link).
|
||||
ansible_run_id: Mapped[str | None] = mapped_column(
|
||||
String(36), ForeignKey("ansible_runs.id", ondelete="SET NULL"), nullable=True
|
||||
|
||||
Reference in New Issue
Block a user