From ea31deabf93da88eaf6f584622d4e18e6601e198 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 8 Oct 2026 17:28:19 -0400 Subject: [PATCH] fix(unifi): the Overview's history buckets on Postgres; devices name their state (#5207) #5207 "UniFi Overview tab: func.strftime is SQLite-only": the WAN and client sparklines bucketed with strftime('%s'), which Postgres lacks, so /plugins/unifi/rows failed. Traefik's date_bin bucket moves to core/time_range.time_bucket and both plugins use it. Devices list offline and failing first, and a state other than online is named (Upgrading, Adopting, Heartbeat missed...) instead of drawn as Down. Co-Authored-By: Claude Opus 5.5 --- plugins/traefik/routes.py | 15 +--- plugins/unifi/routes.py | 94 +++++++++++++++---------- plugins/unifi/templates/unifi/rows.html | 8 ++- steward/core/time_range.py | 12 ++++ tests/integration/test_unifi_history.py | 52 ++++++++++++++ tests/plugins/unifi/test_unifi_page.py | 13 ++++ 6 files changed, 145 insertions(+), 49 deletions(-) create mode 100644 tests/integration/test_unifi_history.py diff --git a/plugins/traefik/routes.py b/plugins/traefik/routes.py index dc374ca..96f8d60 100644 --- a/plugins/traefik/routes.py +++ b/plugins/traefik/routes.py @@ -10,7 +10,7 @@ import time from steward.auth.middleware import require_role from steward.models.users import UserRole -from steward.core.time_range import parse_range, DEFAULT_RANGE, bucket_seconds, subsample +from steward.core.time_range import parse_range, DEFAULT_RANGE, bucket_seconds, subsample, time_bucket from steward.core.formatting import cert_left, status_class from steward.models.metrics import PluginMetricLatest from .models import TraefikAccessSummary, TraefikCert, TraefikGlobalStat, TraefikMetric @@ -430,15 +430,6 @@ async def widget_access_log(): summary=summary, widget_id=widget_id) -def _time_bucket(column, seconds: int): - """The start of the `seconds`-wide window a timestamp falls in, as - host_agent/metrics_query.py buckets. Integer epoch / int rendered as - numeric division (no two scrapes shared a bucket) and overflowed int4 - after 2038 (#5256).""" - return func.date_bin(func.make_interval(0, 0, 0, 0, 0, 0, int(seconds)), column, - func.to_timestamp(0)).label("bucket") - - # A rows or widget request past this logs where its time went, phase by # phase, so a slow query names itself (#5355: 8 s on every poll, warm). SLOW_PAGE_SECONDS = 1.0 @@ -473,7 +464,7 @@ async def router_history(db, names: list[str], since, bucket_secs: int) -> dict[ history: dict[str, list] = {n: [] for n in names} if not history: return history - bucket_col = _time_bucket(TraefikMetric.scraped_at, bucket_secs) + bucket_col = time_bucket(TraefikMetric.scraped_at, bucket_secs) result = await db.execute( select( TraefikMetric.router_name, @@ -509,7 +500,7 @@ async def request_history(db, since, bucket_seconds: int) -> list[dict]: .group_by(TraefikMetric.scraped_at) .subquery() ) - bucket_col = _time_bucket(per_scrape.c.at, bucket_seconds) + bucket_col = time_bucket(per_scrape.c.at, bucket_seconds) rows = (await db.execute( select( func.avg(per_scrape.c.req).label("req_rate"), diff --git a/plugins/unifi/routes.py b/plugins/unifi/routes.py index 55b48ea..2176a3d 100644 --- a/plugins/unifi/routes.py +++ b/plugins/unifi/routes.py @@ -2,11 +2,11 @@ from __future__ import annotations import json from quart import Blueprint, current_app, render_template, request -from sqlalchemy import Integer, cast, func, select +from sqlalchemy import func, select from steward.auth.middleware import require_role from steward.models.users import UserRole -from steward.core.time_range import parse_range, DEFAULT_RANGE, bucket_seconds +from steward.core.time_range import parse_range, DEFAULT_RANGE, bucket_seconds, time_bucket from .models import ( UnifiAlarm, UnifiClientSnapshot, UnifiDevice, UnifiDpiSnapshot, UnifiEvent, UnifiFwRule, UnifiKnownClient, UnifiNetwork, @@ -49,6 +49,58 @@ async def index(): ) +# A UniFi device's `state` code, as (status_dot state, label). Only 1 is +# working; the rest are named rather than all drawn as "Down" (#5207). +DEVICE_STATES = { + 0: ("down", "Offline"), 1: ("up", "Online"), 2: ("pending", "Pending adoption"), + 3: ("degraded", "Firmware mismatch"), 4: ("pending", "Upgrading"), 5: ("pending", "Provisioning"), + 6: ("degraded", "Heartbeat missed"), 7: ("pending", "Adopting"), 8: ("pending", "Deleting"), + 9: ("down", "Inform error"), 10: ("down", "Adoption failed"), 11: ("degraded", "Isolated"), +} +_DEVICE_ORDER = {"down": 0, "degraded": 1, "pending": 2, "up": 3} + + +def device_state(code) -> tuple[str, str]: + return DEVICE_STATES.get(code, ("unknown", f"State {code}")) + + +def devices_in_order(devices) -> list: + """Offline and failing devices first, then ones in a transition, then the + working ones; by name within each.""" + return sorted(devices, key=lambda d: (_DEVICE_ORDER.get(device_state(d.state)[0], 1), + (d.name or d.mac or "").lower())) + + +async def history(db, since, b_secs: int): + """(WAN rows, client-count rows) averaged per `b_secs` window since + `since`, oldest first, for the overview's sparklines.""" + wan_bucket = time_bucket(UnifiWanStat.scraped_at, b_secs) + wan = (await db.execute( + select( + func.avg(UnifiWanStat.latency_ms).label("latency_ms"), + func.avg(UnifiWanStat.rx_bytes_rate).label("rx_bytes_rate"), + func.avg(UnifiWanStat.tx_bytes_rate).label("tx_bytes_rate"), + func.min(UnifiWanStat.scraped_at).label("scraped_at"), + wan_bucket, + ) + .where(UnifiWanStat.scraped_at >= since) + .group_by(wan_bucket) + .order_by(wan_bucket) + )).all() + cs_bucket = time_bucket(UnifiClientSnapshot.scraped_at, b_secs) + clients = (await db.execute( + select( + func.avg(UnifiClientSnapshot.total_clients).label("total_clients"), + func.min(UnifiClientSnapshot.scraped_at).label("scraped_at"), + cs_bucket, + ) + .where(UnifiClientSnapshot.scraped_at >= since) + .group_by(cs_bucket) + .order_by(cs_bucket) + )).all() + return wan, clients + + @unifi_bp.get("/rows") @require_role(UserRole.viewer) async def rows(): @@ -58,23 +110,7 @@ async def rows(): b_secs = bucket_seconds(since) async with current_app.db_sessionmaker() as db: - # WAN history — bucketed for accurate full-range sparklines - wan_bucket = ( - cast(func.strftime('%s', UnifiWanStat.scraped_at), Integer) / b_secs - ).label("bucket") - result = await db.execute( - select( - func.avg(UnifiWanStat.latency_ms).label("latency_ms"), - func.avg(UnifiWanStat.rx_bytes_rate).label("rx_bytes_rate"), - func.avg(UnifiWanStat.tx_bytes_rate).label("tx_bytes_rate"), - func.min(UnifiWanStat.scraped_at).label("scraped_at"), - wan_bucket, - ) - .where(UnifiWanStat.scraped_at >= since) - .group_by(wan_bucket) - .order_by(wan_bucket) - ) - wan_history = result.all() + wan_history, client_history = await history(db, since, b_secs) # Latest WAN — always most recent raw row for current displayed values result = await db.execute( @@ -92,25 +128,10 @@ async def rows(): # All devices result = await db.execute( - select(UnifiDevice).order_by(UnifiDevice.state.desc(), UnifiDevice.name) + select(UnifiDevice) ) - devices = result.scalars().all() + devices = devices_in_order(result.scalars().all()) - # Client count history — bucketed - cs_bucket = ( - cast(func.strftime('%s', UnifiClientSnapshot.scraped_at), Integer) / b_secs - ).label("bucket") - result = await db.execute( - select( - func.avg(UnifiClientSnapshot.total_clients).label("total_clients"), - func.min(UnifiClientSnapshot.scraped_at).label("scraped_at"), - cs_bucket, - ) - .where(UnifiClientSnapshot.scraped_at >= since) - .group_by(cs_bucket) - .order_by(cs_bucket) - ) - client_history = result.all() # Latest WLAN stats (always most recent per SSID) result = await db.execute( @@ -149,6 +170,7 @@ async def rows(): latest_wan=latest_wan, latest_clients=latest_clients, devices=devices, + device_state=device_state, top_clients=top_clients, wlan_latest=wlan_latest, speedtest=speedtest, diff --git a/plugins/unifi/templates/unifi/rows.html b/plugins/unifi/templates/unifi/rows.html index aa1e7fb..b26adb9 100644 --- a/plugins/unifi/templates/unifi/rows.html +++ b/plugins/unifi/templates/unifi/rows.html @@ -50,7 +50,7 @@ {% for d in devices %} - {{ status_dot('up' if d.state == 1 else 'down') }} + {% set dot, label = device_state(d.state) %}{{ status_dot(dot, label) }} {{ d.name or d.mac }} {{ d.model or d.device_type or "" }} @@ -62,6 +62,12 @@ up {{ d.uptime_seconds|duration }} + {% elif d.state != 1 %} + + + {{ label }} + + {% endif %} {% endfor %} diff --git a/steward/core/time_range.py b/steward/core/time_range.py index ac7423c..33a6c0a 100644 --- a/steward/core/time_range.py +++ b/steward/core/time_range.py @@ -4,6 +4,8 @@ from __future__ import annotations from datetime import datetime, timedelta, timezone from typing import TypeVar +from sqlalchemy import func + RANGES: dict[str, timedelta] = { "1h": timedelta(hours=1), "6h": timedelta(hours=6), @@ -33,6 +35,16 @@ def bucket_seconds(since: datetime, target_points: int = 80) -> int: return max(60, int(range_secs / target_points)) +def time_bucket(column, seconds: int): + """The start of the `seconds`-wide window a timestamp falls in, aligned to + the epoch, as a Postgres timestamp labelled "bucket". Group and order by + it. (An integer epoch divided by an int rendered as numeric division in + Postgres, so no two scrapes shared a bucket, and overflowed int4 after + 2038 (#5256); strftime('%s') is SQLite-only (#5207).)""" + return func.date_bin(func.make_interval(0, 0, 0, 0, 0, 0, int(seconds)), column, + func.to_timestamp(0)).label("bucket") + + T = TypeVar("T") diff --git a/tests/integration/test_unifi_history.py b/tests/integration/test_unifi_history.py new file mode 100644 index 0000000..ca11624 --- /dev/null +++ b/tests/integration/test_unifi_history.py @@ -0,0 +1,52 @@ +"""Integration: the UniFi overview's sparkline history buckets on Postgres +(#5207: it used SQLite's strftime, so the overview rows failed). Requires +STEWARD_DATABASE_URL.""" +from __future__ import annotations + +import asyncio +import os +from datetime import datetime, timedelta, timezone + +import pytest + +pytestmark = pytest.mark.integration + + +@pytest.fixture +def app(): + if not os.environ.get("STEWARD_DATABASE_URL"): + pytest.skip("needs Postgres") + from steward.app import create_app + return create_app(testing=False) + + +def test_wan_and_client_history_average_per_window(app): + from sqlalchemy import delete + from plugins.unifi.models import UnifiClientSnapshot, UnifiWanStat + from plugins.unifi.routes import history + + # Two scrapes in one 10-minute window, one in the next. + start = datetime(2031, 1, 1, 12, 0, tzinfo=timezone.utc) + at = [start + timedelta(minutes=1), start + timedelta(minutes=4), start + timedelta(minutes=12)] + + async def go(): + async with app.db_sessionmaker() as s: + async with s.begin(): + await s.execute(delete(UnifiWanStat).where(UnifiWanStat.scraped_at >= start)) + await s.execute(delete(UnifiClientSnapshot).where(UnifiClientSnapshot.scraped_at >= start)) + for when, ms, n in zip(at, (10.0, 30.0, 50.0), (4, 6, 9)): + s.add(UnifiWanStat(scraped_at=when, is_up=True, latency_ms=ms, + rx_bytes_rate=1.0, tx_bytes_rate=2.0)) + s.add(UnifiClientSnapshot(scraped_at=when, total_clients=n)) + try: + async with s.begin(): + return await history(s, start, 600) + finally: + async with s.begin(): + await s.execute(delete(UnifiWanStat).where(UnifiWanStat.scraped_at >= start)) + await s.execute(delete(UnifiClientSnapshot).where(UnifiClientSnapshot.scraped_at >= start)) + + wan, clients = asyncio.run(go()) + assert [(r.latency_ms, r.scraped_at, r.bucket) for r in wan] == [ + (20.0, at[0], start), (50.0, at[2], start + timedelta(minutes=10))] + assert [float(r.total_clients) for r in clients] == [5.0, 9.0] diff --git a/tests/plugins/unifi/test_unifi_page.py b/tests/plugins/unifi/test_unifi_page.py index 58baba7..58118d4 100644 --- a/tests/plugins/unifi/test_unifi_page.py +++ b/tests/plugins/unifi/test_unifi_page.py @@ -3,6 +3,7 @@ by Settings → Thresholds, devices with status dots, categories as badges (#512 from datetime import datetime, timezone from types import SimpleNamespace as NS +from plugins.unifi.routes import device_state, devices_in_order from tests._templates import render_plugin_template from tests.test_style_tokens import hex_colours @@ -13,14 +14,19 @@ async def test_wan_health_and_devices_use_the_shared_shapes(app): devices = [NS(name="switch", mac="aa", model="USW", device_type="usw", ip="10.0.0.2", state=1, uptime_seconds=3600), NS(name="ap-attic", mac="bb", model="U6", device_type="uap", ip="10.0.0.3", state=0, + uptime_seconds=None), + NS(name="ap-porch", mac="cc", model="U6", device_type="uap", ip="10.0.0.4", state=4, uptime_seconds=None)] async with app.test_request_context("/plugins/unifi/rows"): page = await render_plugin_template(app, "unifi", "unifi/rows.html", latest_wan=wan, devices=devices, + device_state=device_state, sparkline_latency="", sparkline_rx="", sparkline_tx="") assert 'WAN' in page and "stat-crit" in page assert '250 ms' in page assert 'class="dot dot-ok"' in page and 'class="dot dot-crit"' in page + # #5207: a device mid-upgrade is named, not drawn as down. + assert 'class="dot dot-warn" aria-hidden="true" title="Upgrading"' in page and "Offline" in page assert "font-weight:600" not in page and "font-weight:700" not in page assert not hex_colours(page), hex_colours(page) @@ -32,3 +38,10 @@ async def test_event_categories_are_badges(app): page = await render_plugin_template(app, "unifi", "unifi/events.html", events=events) assert 'ids' in page assert "rgba(" not in page + + +def test_offline_devices_come_first_then_transitions_then_working(): + devices = [NS(name=n, mac=n, state=st) for n, st in + [("b-ok", 1), ("a-ok", 1), ("upgrading", 4), ("gone", 0), ("odd", 99)]] + assert [d.name for d in devices_in_order(devices)] == ["gone", "odd", "upgrading", "a-ok", "b-ok"] + assert device_state(99) == ("unknown", "State 99")