refactor(diagnostics): emit system.ntp_synchronized + top-level pi_throttle (#102)

* refactor(diagnostics): emit system.ntp_synchronized + top-level pi_throttle

Splits the misnamed pi_health wire block. NTP-sync is a universal host signal and moves into system.ntp_synchronized; the eight vcgencmd throttle bits become a flat top-level pi_throttle block, absent on non-Pi feeders. collect_pi_ntp_sync renames to collect_ntp_sync.

* test(diagnostics): pin pi_throttle key-set and cover ntp_synchronized=false

Stub vcgencmd alongside timedatectl in the no-probes test so a Pi-based dev box can't leak vcgencmd output through the stub gap. Add an exact-key-set assertion on pi_throttle (the server sanitizer rejects partial blocks). Add a 'no' NTP case so 'false' is pinned through the prune pass. Tone down the collect_ntp_sync comment to match runtime behaviour in containers / stripped images.

Author: d4rken

scripts/airplanes-diagnostics.sh

@@ -288,8 +288,8 @@ collect_network() {
 #   bit 1  freq_capped_now         bit 17 freq_capped_ever
 #   bit 2  throttled_now           bit 18 throttled_ever
 #   bit 3  soft_temp_limit_now     bit 19 soft_temp_limit_ever
-# Sets pi_health_* globals. Returns 0 on success, 1 if vcgencmd absent or
-# its output unparseable.
+# Sets the eight pi_*_now / pi_*_ever globals. Returns 0 on success, 1
+# if vcgencmd is absent or its output is unparseable.
 collect_pi_throttle() {
     command -v vcgencmd >/dev/null 2>&1 || return 1
     local raw value
@@ -308,8 +308,13 @@ collect_pi_throttle() {
     pi_soft_temp_limit_ever=$(( (n >> 19) & 1 ))
 }
 
-# timedatectl show -p NTPSynchronized --value -> "yes" / "no" / ""
-collect_pi_ntp_sync() {
+# timedatectl show -p NTPSynchronized --value -> "yes" / "no" / "".
+# Universal host signal — any systemd host with a running timedated can
+# answer, not just Pis. Sits at `system.ntp_synchronized` on the wire,
+# alongside uptime and CPU. Containers, chroots, and stripped images may
+# ship `timedatectl` but fail at runtime; the helper returns 1 and the
+# field is omitted from the payload by the prune pass.
+collect_ntp_sync() {
     command -v timedatectl >/dev/null 2>&1 || return 1
     local raw
     raw="$(timeout 3s timedatectl show -p NTPSynchronized --value 2>/dev/null)" || return 1
@@ -460,15 +465,15 @@ build_service_json() {
          | with_entries(select(.value != null and .value != ""))'
 }
 
-# Build the pi_health block as JSON, or print "null" if neither sub-probe
-# produced data. Sub-probes are independent — a missing/broken
-# `timedatectl` doesn't suppress vcgencmd throttle data and vice versa.
-build_pi_health_json() {
-    local throttle_json='null' ntp_json='null'
+# Build the flat pi_throttle block as JSON, or print "null" if vcgencmd
+# is absent or its output failed to parse. Pi-only — absent on every
+# non-Pi feeder. NTP-sync lives in `system.ntp_synchronized` now (it is
+# universal-host, not Pi-specific) and is plumbed separately.
+build_pi_throttle_json() {
     local pi_undervoltage_now=0 pi_freq_capped_now=0 pi_throttled_now=0 pi_soft_temp_limit_now=0
     local pi_undervoltage_ever=0 pi_freq_capped_ever=0 pi_throttled_ever=0 pi_soft_temp_limit_ever=0
     if command -v vcgencmd >/dev/null 2>&1 && collect_pi_throttle; then
-        throttle_json="$(jq -nc \
+        jq -nc \
             --argjson uv_now "$pi_undervoltage_now" \
             --argjson fc_now "$pi_freq_capped_now" \
             --argjson th_now "$pi_throttled_now" \
@@ -484,22 +489,10 @@ build_pi_health_json() {
               undervoltage_ever: ($uv_ever == 1),
               freq_capped_ever: ($fc_ever == 1),
               throttled_ever: ($th_ever == 1),
-              soft_temp_limit_ever: ($st_ever == 1)}')"
-    fi
-    if command -v timedatectl >/dev/null 2>&1; then
-        local ntp
-        if ntp="$(collect_pi_ntp_sync)"; then
-            ntp_json="$ntp"
-        fi
-    fi
-    if [[ "$throttle_json" == 'null' && "$ntp_json" == 'null' ]]; then
-        printf 'null'
+              soft_temp_limit_ever: ($st_ever == 1)}'
         return
     fi
-    jq -nc \
-        --argjson throttle "$throttle_json" \
-        --argjson ntp "$ntp_json" \
-        '{throttle: $throttle, ntp_synchronized: $ntp}'
+    printf 'null'
 }
 
 # nullable_num VALUE — echoes the value if non-empty, otherwise "null".
@@ -689,8 +682,13 @@ main() {
             svc_978="$(build_service_json airplanes-978 "$(root_path /run/airplanes-978/state)")"
         fi
 
-        local pi_health_json
-        pi_health_json="$(build_pi_health_json)"
+        local pi_throttle_json ntp_sync_json
+        pi_throttle_json="$(build_pi_throttle_json)"
+        # Universal host-clock signal — emits "true"/"false" or null on
+        # any non-systemd host (no `timedatectl`). Wire path:
+        # `system.ntp_synchronized`. Server stamps `system.clock_skew_seconds`
+        # separately at ingest.
+        ntp_sync_json="$(collect_ntp_sync 2>/dev/null || printf 'null')"
 
         local feed_scripts_version os_pretty_name os_id os_version_id kernel architecture image_release
         feed_scripts_version="$(get_feed_scripts_version || true)"
@@ -730,7 +728,8 @@ main() {
             --argjson svc_readsb "$svc_readsb" \
             --argjson svc_dump978 "$svc_dump978" \
             --argjson svc_978 "$svc_978" \
-            --argjson pi_health "$pi_health_json" \
+            --argjson pi_throttle "$pi_throttle_json" \
+            --argjson ntp_synchronized "$ntp_sync_json" \
             --arg feed_scripts "${feed_scripts_version:-}" \
             --arg os_pretty_name "${os_pretty_name:-}" \
             --arg os_id "${os_id:-}" \
@@ -758,7 +757,8 @@ main() {
                     disk: {
                         used_percent: $disk_used_pct,
                         total_bytes: $disk_total_bytes
-                    }
+                    },
+                    ntp_synchronized: $ntp_synchronized
                 },
                 network: {
                     connection_type: $net_connection_type,
@@ -774,7 +774,7 @@ main() {
                     architecture: $architecture,
                     image_release: $image_release
                 },
-                pi_health: $pi_health
+                pi_throttle: $pi_throttle
             }
             | def _prune:
                 if type == "object" then

test/test_airplanes_diagnostics.bats

@@ -645,23 +645,35 @@ SH
     [ "$output" = '-58' ]
 }
 
-@test "POST body omits pi_health when both vcgencmd and timedatectl are absent" {
-    # Block both probes by stubbing them to fail. The default setup
-    # doesn't ship vcgencmd or timedatectl stubs, but the dev host
-    # likely has timedatectl on PATH (it's installed by systemd) which
-    # would otherwise produce a pi_health.ntp_synchronized field.
+@test "POST body omits pi_throttle and system.ntp_synchronized when both vcgencmd and timedatectl are absent" {
+    # Stub BOTH probes to fail. The dev / CI host usually ships
+    # `timedatectl` (installed by systemd) and a Pi-based dev box would
+    # also have `vcgencmd`; either of those leaking into this test would
+    # produce a payload that contradicts the test's title.
+    cat > "$STUB_DIR/vcgencmd" <<'SH'
+#!/usr/bin/env bash
+exit 1
+SH
+    chmod +x "$STUB_DIR/vcgencmd"
     cat > "$STUB_DIR/timedatectl" <<'SH'
 #!/usr/bin/env bash
 exit 1
 SH
     chmod +x "$STUB_DIR/timedatectl"
     run_script
     [ "$status" -eq 0 ]
-    run jq '.pi_health // empty' "$BODY_LOG"
-    [ -z "$output" ]
+    # pi_throttle is absent on non-Pi (no vcgencmd).
+    run jq -e 'has("pi_throttle") | not' "$BODY_LOG"
+    [ "$status" -eq 0 ]
+    # NTP-sync absent in system block (broken timedatectl).
+    run jq -e '.system | has("ntp_synchronized") | not' "$BODY_LOG"
+    [ "$status" -eq 0 ]
+    # Old container is gone — sentinel against accidental regressions.
+    run jq -e 'has("pi_health") | not' "$BODY_LOG"
+    [ "$status" -eq 0 ]
 }
 
-@test "pi_health throttle survives a broken timedatectl (sub-probes independent)" {
+@test "pi_throttle survives a broken timedatectl (probes are independent)" {
     cat > "$STUB_DIR/vcgencmd" <<'SH'
 #!/usr/bin/env bash
 [[ "$1" == "get_throttled" ]] && printf 'throttled=0x0\n'
@@ -674,15 +686,16 @@ SH
     chmod +x "$STUB_DIR/timedatectl"
     run_script
     [ "$status" -eq 0 ]
-    # throttle block is present, all bits false
-    run jq -er '.pi_health.throttle.throttled_now' "$BODY_LOG"
+    # pi_throttle bits present, all false
+    run jq -er '.pi_throttle.throttled_now' "$BODY_LOG"
     [ "$output" = 'false' ]
-    # ntp_synchronized is dropped (broken probe)
-    run jq '.pi_health.ntp_synchronized // empty' "$BODY_LOG"
-    [ -z "$output" ]
+    # NTP-sync is absent (broken probe). With separate top-level fields
+    # the independence is now structural, but the sentinel stays.
+    run jq -e '.system | has("ntp_synchronized") | not' "$BODY_LOG"
+    [ "$status" -eq 0 ]
 }
 
-@test "POST body pi_health bit decode for vcgencmd=0x50005" {
+@test "POST body pi_throttle bit decode for vcgencmd=0x50005" {
     cat > "$STUB_DIR/vcgencmd" <<'SH'
 #!/usr/bin/env bash
 [[ "$1" == "get_throttled" ]] && printf 'throttled=0x50005\n'
@@ -697,20 +710,39 @@ SH
     [ "$status" -eq 0 ]
     # bits 0, 2, 16, 18 set: undervoltage_now, throttled_now,
     # undervoltage_ever, throttled_ever — all true.
-    run jq -er '.pi_health.throttle.undervoltage_now' "$BODY_LOG"
+    run jq -er '.pi_throttle.undervoltage_now' "$BODY_LOG"
     [ "$output" = 'true' ]
-    run jq -er '.pi_health.throttle.throttled_now' "$BODY_LOG"
+    run jq -er '.pi_throttle.throttled_now' "$BODY_LOG"
     [ "$output" = 'true' ]
-    run jq -er '.pi_health.throttle.undervoltage_ever' "$BODY_LOG"
+    run jq -er '.pi_throttle.undervoltage_ever' "$BODY_LOG"
     [ "$output" = 'true' ]
-    run jq -er '.pi_health.throttle.throttled_ever' "$BODY_LOG"
+    run jq -er '.pi_throttle.throttled_ever' "$BODY_LOG"
     [ "$output" = 'true' ]
-    run jq -er '.pi_health.throttle.freq_capped_now' "$BODY_LOG"
+    run jq -er '.pi_throttle.freq_capped_now' "$BODY_LOG"
     [ "$output" = 'false' ]
-    run jq -er '.pi_health.throttle.soft_temp_limit_ever' "$BODY_LOG"
+    run jq -er '.pi_throttle.soft_temp_limit_ever' "$BODY_LOG"
     [ "$output" = 'false' ]
-    run jq -er '.pi_health.ntp_synchronized' "$BODY_LOG"
+    run jq -er '.system.ntp_synchronized' "$BODY_LOG"
     [ "$output" = 'true' ]
+    # Exact key-set: the server sanitizer drops the whole pi_throttle
+    # block unless all 8 known keys are present. Pinning the wire shape
+    # here catches an accidental drop of any one bit.
+    run jq -er '.pi_throttle | keys | sort | join(",")' "$BODY_LOG"
+    [ "$output" = 'freq_capped_ever,freq_capped_now,soft_temp_limit_ever,soft_temp_limit_now,throttled_ever,throttled_now,undervoltage_ever,undervoltage_now' ]
+}
+
+@test "system.ntp_synchronized is false when timedatectl reports no" {
+    # Pin that `false` survives the prune pass — `_prune` strips empty
+    # strings and nulls but must keep boolean false.
+    cat > "$STUB_DIR/timedatectl" <<'SH'
+#!/usr/bin/env bash
+[[ "$1" == "show" ]] && printf 'no\n'
+SH
+    chmod +x "$STUB_DIR/timedatectl"
+    run_script
+    [ "$status" -eq 0 ]
+    run jq -er '.system.ntp_synchronized' "$BODY_LOG"
+    [ "$output" = 'false' ]
 }
 
 # ---- response handling ----