From f48c806718387c63591dc9a119b3bb992f9e0e27 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Sun, 6 Sep 2026 12:45:40 +0100 Subject: [PATCH] Preserve observed Docker storage evidence Missing block I/O and container image sizes could become false evidence for diagnosis. Preserve per-direction counter presence and measured zero through collection, resource conversion and browser rendering. Separate new observed history from ambiguous retained disk series without deleting old rows or changing public metric names. Keep partial host rates distinct and persist a newly enabled Disk I/O column across the first preference reload. --- .../PATROL_ASSISTANT_CUSTOMER_JOURNEY.md | 86 +++++++++ docs/release-control/v6/internal/status.json | 4 +- .../v6/internal/subsystems/agent-lifecycle.md | 5 + .../subsystems/frontend-primitives.md | 8 + .../v6/internal/subsystems/monitoring.md | 14 ++ .../subsystems/performance-and-scalability.md | 14 ++ .../v6/internal/subsystems/registry.json | 9 +- .../internal/subsystems/storage-recovery.md | 8 + .../internal/subsystems/unified-resources.md | 10 ++ frontend-modern/browser-verification.json | 43 +++-- .../UnifiedResourceHostTableCard.tsx | 20 ++- .../__tests__/infrastructureSelectors.test.ts | 13 ++ .../Infrastructure/infrastructureSelectors.ts | 9 +- .../docker/DockerHostDrawerOverview.tsx | 10 +- .../standalone/AgentsMachinesTable.tsx | 20 ++- .../__tests__/agentMachineTableModel.test.ts | 10 ++ .../standalone/agentMachineTableModel.ts | 4 +- .../__tests__/useColumnVisibility.test.ts | 23 +++ .../__tests__/useUnifiedResources.test.ts | 19 ++ .../src/hooks/useColumnVisibility.ts | 30 ++-- .../src/hooks/useUnifiedResources.ts | 4 +- frontend-modern/src/types/resource.ts | 4 +- .../src/utils/__tests__/format.test.ts | 11 ++ frontend-modern/src/utils/format.ts | 8 + internal/api/router_state_test.go | 2 +- internal/dockeragent/blockio_presence_test.go | 48 +++++ internal/dockeragent/collect.go | 17 +- internal/models/converters.go | 7 +- internal/models/converters_test.go | 8 +- internal/models/models_frontend.go | 4 +- .../monitoring/canonical_guardrails_test.go | 8 +- .../monitoring/docker_metric_presence_test.go | 164 ++++++++++++++++++ .../monitoring/issue1613_contract_test.go | 4 +- internal/monitoring/monitor.go | 21 +-- internal/monitoring/monitor_agents.go | 20 +-- .../monitoring/monitor_unified_state_test.go | 2 +- internal/unifiedresources/metrics.go | 17 +- internal/unifiedresources/metrics_test.go | 26 ++- pkg/agents/docker/blockio_presence_test.go | 32 ++++ pkg/agents/docker/report.go | 23 ++- pkg/metrics/docker_observation_contract.go | 35 ++++ pkg/metrics/store.go | 15 +- .../store_docker_observation_contract_test.go | 148 ++++++++++++++++ .../canonical_completion_guard_test.py | 4 + .../release_control/subsystem_lookup_test.py | 2 + 45 files changed, 864 insertions(+), 129 deletions(-) create mode 100644 internal/dockeragent/blockio_presence_test.go create mode 100644 internal/monitoring/docker_metric_presence_test.go create mode 100644 pkg/agents/docker/blockio_presence_test.go create mode 100644 pkg/metrics/docker_observation_contract.go create mode 100644 pkg/metrics/store_docker_observation_contract_test.go diff --git a/docs/qualification/PATROL_ASSISTANT_CUSTOMER_JOURNEY.md b/docs/qualification/PATROL_ASSISTANT_CUSTOMER_JOURNEY.md index fdcf448c3..24ec1eb30 100644 --- a/docs/qualification/PATROL_ASSISTANT_CUSTOMER_JOURNEY.md +++ b/docs/qualification/PATROL_ASSISTANT_CUSTOMER_JOURNEY.md @@ -1408,3 +1408,89 @@ correction above is a separate scoped change and requires its own landing checks The redesign remains open for reliable interpretation, the config-read contract, storage/backup, approved/rejected action outcomes and supported autonomous Patrol qualification. Wider customer readiness still requires independent Pro environments. + +### Docker measurement correction plan, 2026-09-06 + +The next shared-source correction distinguishes absent block-I/O observations +from measured idle zero and removes the Docker layer-size ratio from filesystem +capacity. Counter presence uses the existing rate tracker contract. Missing +reports must not reset the baseline or fabricate samples. Container layer sizes +remain descriptive metadata. + +Persisted Docker-family disk series previously mixed invalid capacity ratios and +unobserved I/O zeros with real measurements. New disk observations use separate +physical series keys while public metric names remain unchanged. Retained reads +exclude ambiguous legacy disk series without deleting or relabelling them. The +shared app-container storage family also serves non-Docker providers, so new +valid capacity observations must remain supported. Non-Docker series retain +existing behavior. Explicit zero must survive every retained-read API and rollup. + +Browser verification is required after the final backend build. Interaction +matrix: `/docker` at 1440x1000, 900x1000 and 390x1000, container selection, +resource drawer open/close, current metrics, history expansion, measured idle, +unavailable readings, and reload. Inspect actual pixels, scrolling, focus and +Escape dismissal. `/patrol` evidence rendering must preserve absent versus zero +in current-resource and retained-history tool results. Controlled responses may +qualify rendering but cannot qualify diagnosis. Live read-only API observations +must bind to the rebuilt backend. No autonomous subscription retry or paid-model +request is authorized by this correction. + +Collection also carries optional presence for each I/O direction. Explicit zero +entries survive the report JSON. For older reports without presence, only +positive counters establish an observation, so ambiguous zeros remain unavailable +until the agent is updated or a positive baseline exists. This does not require +re-enrollment. Docker-host first-disk history and network-counter presence are +adjacent limits outside this container block-I/O correction. + +The final browser matrix also covers the shared host I/O table, Docker host +Overview and Machines table/tooltip at the same three widths. Partial read/write +observations must show a missing marker for the absent direction, retain measured +zero, and remain excluded from sums used for sorting and comparison. Exercise +column selection, hover/focus, tooltip dismissal and scrolling where present. + +### Docker correction qualification and scope + +The implementation carries per-direction presence from collection and report +JSON into the existing rate tracker, canonical resource metrics, persisted +history and resource-to-browser conversion. REST resource adaptation also +preserves optional rates. Shared rate formatting keeps missing values distinct +from zero in Machines and Docker host details. Incomplete rates do not become +complete throughput totals for sorting or comparison. A browser-discovered +first-user column migration bug is corrected in the shared preference hook, so +showing Disk I/O survives the first reload. + +Legacy workload conversion in `frontend-modern/src/hooks/useWorkloads.ts` still +uses numeric direction fields with grouped availability. Its direction-level +modernization remains a separate consumer follow-up. Docker-host first-disk +history and network presence are also outside this container measurement slice. +The correction must not be represented as complete coverage of all metrics or +all monitoring surfaces. No new model competence or autonomous action result is +claimed. + +Affected Go package checks and targeted race checks ran on pulse-dev with +Go1.26.8. The changed websocket assertion now expects observed read zero with +absent write omitted. Targeted frontend suites and type checking cover optional +rates, REST conversion, sorting, formatting and column persistence. Ten paired +read-benchmark rounds used the unchanged parent store via Go overlay. The +canonical >10%, p<0.05 regression gate passed, with +0.82% timing geomean in this +scoped comparison. This is not a fleet-load or full-product performance claim. + +The Pro backend was cross-built on pulse-dev from the changed source and +installed into the existing local dev runtime. Binary SHA256: +`59f05f954ff8080bd3e8f3054b2b059281c49172ee771b2e454c255241158a4a`. +No production agent was replaced. Older agents remain compatible and treat +ambiguous zero counters conservatively. + +Private receipts: `/Volumes/Development/pulse/tmp/patrol-docker-observed-metrics/`. +Worker logs: `/opt/pulse-release-worker/patrol-docker-observed-proof/`. +PR1934's preceding identity correction merged at +`6b0abc3bee9ffa81f6ab298b5b67ee11369688a0` with all checks passing. The current +measurement slice passed final-source Playwright inspection on `/docker`, +`/standalone/machines` and `/patrol` at 1440x1000, 900x1000 and 390x1000. +Live history, controlled absence/idle/loading/error, partial host rates, column +persistence, nested picker dismissal, tooltip focus, and expanded Assistant +evidence were exercised. The source-bound receipt is +`frontend-modern/browser-verification.json`. The unused shared host table card +has type and selector coverage, not an active-route browser claim. Controlled +responses qualify rendering only. Landing checks remain separate from the +unperformed diagnosis, approved/rejected action and recovery qualifications. diff --git a/docs/release-control/v6/internal/status.json b/docs/release-control/v6/internal/status.json index bf404a895..f8cbf9237 100644 --- a/docs/release-control/v6/internal/status.json +++ b/docs/release-control/v6/internal/status.json @@ -10201,7 +10201,7 @@ }, { "id": "patrol-assistant-customer-outcome-qualification", - "summary": "The explicit Patrol/Assistant redesign goal, contract, execution plan and source-bound evidence remain in docs/qualification/PATROL_ASSISTANT_CUSTOMER_JOURNEY.md. Model judgment owns diagnosis. Observations, hypotheses, proposals, executions and independently verified outcomes remain distinct. Assistant continues the same issue and governed action records. The recorded 2026-09-05 baseline has 127 paid installations, 71 with Patrol enabled and 23 with Assistant calls. Fourteen verified resolutions came from one installation. Schema 17 outcome/provider/cost fields had no adoption. Usage does not prove useful linked tasks or representative false-alarm, missed-problem or success rates. Shared risk, provenance, history, missing-access and diagnostic-continuity corrections have regression and named browser proof. Proposal promotion, duplicate causal inference, contextless evaluations and count-based diagnostic completion policy were removed. Independent Docker fault/oracle contracts qualify reproducible injection, negative controls and cleanup, not model competence or governed action outcomes. Integrated CI exposed retained-query performance, disk-probe ordering and route-label timing regressions. Their scoped corrections and qualification records landed through PR1928 and PR1929. PR1929 merged at cf98358c0eb46987a82def5776fa41db5f54210a with backend, frontend, benchmark, governance, CodeQL and all eight Core E2E shards passing. The current ordinary retained-history diagnosis still contradicts explicit temporal semantics and remains unqualified. Two additional read-only Assistant requests used claude-subscription:claude-opus-5 against run-owned containers on the monitored Tower host. The healthy request took 82.835s and seven tools, correctly recommending no action, but overstated absence of storage impact. The dependency request took 204.384s and sixteen tools with three failed reads. It identified the stopped dependency, preserved the missing command access and causal uncertainty, but overstated storage exclusion and recovery implications. Config reads incorrectly reported app-container not found after successful canonical gets. These single cases remain partial diagnosis evidence, not a qualification pass. The fixture deadline performed two-pass cleanup with unchanged original inventory. Post-answer fault readback and explicit recovery were not completed, so no action outcome is claimed. Captured responses exposed concurrent tool-ID merging, sibling approval removal and renderer mutation of shared evidence. The current scoped correction keeps supplied invocation IDs authoritative and stable message rows without deep transcript reconciliation. All 167 affected frontend tests pass. Final-source browser replay at /patrol, 1440/900/390x1000, preserves all seven and sixteen exact tool inputs/outputs. Controlled stream states verify concurrent progress, cancellation, failed completion and sibling approval retention without provider or infrastructure actions. Exact captures, failed reproductions, hashes and remaining limits are in the plan. This correction still requires its own scoped landing checks. Claude Max explicitly refused autonomous Patrol readiness. Cached refusal and API409 enforcement remain intact, with no bypass or repeated retry. Ordinary Assistant is not autonomous qualification. Approval for an alternate separately billed provider remains pending, and no paid request occurred. Reliable interpretation, the config-read contract, broader storage/backup and approved/rejected action outcomes remain required local work. Independent volunteered Pro environments remain a separate wider-readiness gate.", + "summary": "The explicit redesign plan and source-bound receipts remain in docs/qualification/PATROL_ASSISTANT_CUSTOMER_JOURNEY.md. The goal is open. Model judgment owns diagnosis, with observations, hypotheses, proposals, executions and independently verified outcomes kept distinct. The recorded baseline has 127 paid installations, 71 with Patrol enabled, 23 with Assistant calls and fourteen verified resolutions from one installation. Schema17 outcome/provider/cost fields had no adoption. These are not representative success, false-alarm or missed-problem rates. Shared provenance, history, risk, missing-access and continuity corrections landed through PR1928/1929. Proposal-as-proof and proxy-driven diagnostic completion policy were removed. PR1934 merged the tool-ID and transcript-identity correction at 6b0abc3bee9ffa81f6ab298b5b67ee11369688a0 with all checks passing. Its captured-response browser replay preserves seven healthy and sixteen dependency tool records. The two ordinary subscription Assistant cases took 82.835s and 204.384s. Their primary decisions had useful evidence, but storage exclusion, recovery implications and config-not-found errors remain unqualified. Independent Docker fixtures prove injection, negative controls and deadline cleanup, not completed recovery or model/action competence. The current scoped Docker measurement correction preserves per-direction observation presence, separates corrected history from ambiguous legacy rows, removes image-layer ratios from capacity, and carries optional rates through resource/browser conversion. Legacy workload direction availability, Docker-host first-disk history and network presence remain distinct consumer/source follow-ups. Exact regression, browser and delivery state is recorded in the plan. Claude Max explicitly refused autonomous Patrol readiness. Cached refusal and API409 enforcement remain intact. Ordinary Assistant is not autonomous qualification. Separate paid-provider approval remains pending and no paid request occurred. Reliable interpretation, config-read semantics, storage/backup and approved/rejected action outcomes remain required local work. Independent volunteered Pro environments remain a separate wider-readiness gate.", "owner": "project-owner", "status": "planned", "recorded_at": "2026-09-05", @@ -10215,6 +10215,7 @@ "ai-runtime", "api-contracts", "frontend-primitives", + "monitoring", "patrol-intelligence", "performance-and-scalability", "unified-resources" @@ -10386,6 +10387,7 @@ "ai-runtime", "api-contracts", "frontend-primitives", + "monitoring", "patrol-intelligence", "performance-and-scalability", "unified-resources" diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 301a0412f..29d4b8f97 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -15,6 +15,11 @@ ## Purpose +Docker block-I/O report presence fields are optional measurement metadata. +They preserve zero and omitted directions independently without changing report +admission, enrollment, identity, command permission or agent lifecycle state. +Older agents remain accepted, with ambiguous omitted zero counters unavailable. + The internal Patrol request bridge carries explicit execution limits and capability allowlists without a diagnostic report-count budget. Finding writes retain their server-owned scope and cannot enter or satisfy the infrastructure diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index 0b217c234..cf6628297 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -20,6 +20,14 @@ ## Purpose +Disk I/O presentation preserves each observed direction independently. Shared +formatting renders a missing rate as a dash and measured idle as numeric zero. +Partial observations cannot form a complete throughput total for sorting or +comparison. Machines column preferences must preserve an explicit user choice +across the first reload, including default-hidden migrations. Final-source +browser proof covers Docker host details, Machines column selection and tooltip +focus/dismissal at desktop, intermediate and narrow widths. + The shared delivery-health card wraps action groups according to available space, retaining readable explanation width when Review, Retry, Dismiss and Refresh appear together. Its heading uses the opaque semantic foreground, diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 9e1793589..e9c89ae80 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -17,6 +17,20 @@ ## Purpose +Docker collection records read and write counter presence independently, +including explicit zero, in optional report fields. Older reports without those +fields establish only positive counters. Container reports propagate this +presence to the shared rate tracker. An omitted block-I/O payload and the first counter sample +produce no rate history. Unchanged observed counters produce measured zero, +including after an omitted report. Container writable/root layer sizes never +produce capacity usage history. The ingestion regression is +`internal/monitoring/docker_metric_presence_test.go`. This changes measurement +projection only and grants no agent lifecycle authority. +The shared resource-to-browser conversion preserves each optional I/O rate +independently. Missing directions are omitted from JSON, while measured zero +remains numeric zero. No aggregate presence flag may fabricate its sibling +direction. `TestResourceDiskIOWirePreservesAbsentDirection` pins that wire path. + Physical disk inventory has an independent collector schedule. The PVE poller carries its default five-minute or configured interval with each disk record, while keeping the last successful observation timestamp on retained records. diff --git a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md index 6cdd0a1f5..0b215b09d 100644 --- a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md +++ b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md @@ -15,6 +15,20 @@ ## Purpose +The Docker/app-container history families `dockercontainer` and `docker` use +separate physical `.observed` series for new disk capacity and block-I/O +measurements. Older disk series lack the required presence/capacity semantics +and remain stored unchanged, but retained reads exclude them from current +evidence. All four shared read APIs expose corrected series under the existing +public metric names. Valid capacity from other providers sharing this storage +family remains writable. `NormalizedSeriesKey` describes physical storage for +coverage/backfill matching. Rollups aggregate each physical generation separately. +Projection happens once per returned series, not per observation, and adds no +query, schema migration, or per-row work for other resource families. +`pkg/metrics/store_docker_observation_contract_test.go` pins legacy coexistence, +zero retention, selected/fleet read parity, rollup separation and unaffected +resource families. + Retained reads use one shared query contract in `pkg/metrics/store.go` for `Query`, `QueryAll`, `QueryAllBatch` and `QueryMetricTypesBatch`. A non-empty preferred resolution no longer hides a newer raw tail, an older uncovered diff --git a/docs/release-control/v6/internal/subsystems/registry.json b/docs/release-control/v6/internal/subsystems/registry.json index dd2a7fa00..31e433330 100644 --- a/docs/release-control/v6/internal/subsystems/registry.json +++ b/docs/release-control/v6/internal/subsystems/registry.json @@ -1611,6 +1611,7 @@ "internal/config/host_continuity_test.go", "internal/models/metrics_types_test.go", "internal/monitoring/availability_probe_agent_test.go", + "internal/monitoring/docker_metric_presence_test.go", "internal/monitoring/monitor_host_agent_removal_lifecycle_test.go", "internal/monitoring/monitor_host_agents_test.go", "scripts/installtests/agent_state_dir_lifecycle_test.go", @@ -5945,6 +5946,7 @@ "test_prefixes": [], "exact_files": [ "internal/config/host_continuity_test.go", + "internal/monitoring/docker_metric_presence_test.go", "internal/monitoring/issue1485_unraid_lifecycle_test.go", "internal/monitoring/issue1595_collection_trust_test.go", "internal/monitoring/monitor_docker_test.go", @@ -6066,6 +6068,7 @@ "internal/dockeragent/agent_collect_test.go", "internal/dockeragent/agent_cpu_test.go", "internal/dockeragent/agent_internal_test.go", + "internal/dockeragent/blockio_presence_test.go", "internal/dockeragent/swarm_coverage_test.go" ] }, @@ -6125,13 +6128,15 @@ "internal/models/issue1639_pbs_collision_test.go", "internal/models/metrics_types_test.go", "internal/models/state_host_test.go", + "internal/monitoring/docker_metric_presence_test.go", "internal/monitoring/issue1595_collection_trust_test.go", "internal/monitoring/monitor_full_coverage_test.go", "internal/monitoring/monitor_host_agent_removal_lifecycle_test.go", "internal/monitoring/monitor_host_agents_test.go", "internal/monitoring/monitor_package_updates_test.go", "internal/unifiedresources/adapter_coverage_test.go", - "internal/unifiedresources/registry_test.go" + "internal/unifiedresources/registry_test.go", + "pkg/agents/docker/blockio_presence_test.go" ] }, { @@ -7005,6 +7010,7 @@ "exact_files": [ "pkg/metrics/store_additional_test.go", "pkg/metrics/store_bench_test.go", + "pkg/metrics/store_docker_observation_contract_test.go", "pkg/metrics/store_query_plan_test.go", "pkg/metrics/store_slo_test.go" ] @@ -7132,6 +7138,7 @@ "allow_same_subsystem_tests": false, "test_prefixes": [], "exact_files": [ + "frontend-modern/src/components/Infrastructure/__tests__/infrastructureSelectors.test.ts", "frontend-modern/src/components/Infrastructure/__tests__/UnifiedResourceTable.performance.contract.test.tsx", "frontend-modern/src/components/Infrastructure/__tests__/unifiedResourceTableStateModel.test.ts", "frontend-modern/src/components/Infrastructure/__tests__/useTableWindowing.test.ts", diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 562ae7917..dc956e1c8 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -21,6 +21,14 @@ ## Purpose +Container image-layer sizes do not establish filesystem capacity. Docker +resource metrics omit that invalid ratio, and retained queries exclude legacy +ambiguous disk observations while preserving new valid provider measurements. +The shared REST resource projection retains missing read/write directions +independently of measured zero. An idle I/O rate cannot establish available +capacity, backup coverage or recoverability. This correction adds no recovery +authority or verified recovery outcome. + Patrol consumes storage evidence in the original diagnostic conversation. A saved finding does not close storage-read authority before the explicit run limit. Missing backup or recovery evidence remains unknown, and neither finding diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index 6739cc88e..077ac9f72 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -15,6 +15,16 @@ ## Purpose +Docker container writable/root layer bytes describe image composition, not +used/total filesystem capacity, and cannot populate `ResourceMetrics.Disk`. +Optional valid block-I/O rate pointers preserve measured zero. Missing, negative +and non-finite rates remain unavailable. Raw layer metadata remains available. +`TestMetricsFromDockerContainerDistinguishesAbsentAndIdleIO` and the container +I/O projection proof in `internal/unifiedresources/metrics_test.go` pin this +boundary. +The frontend resource contract likewise makes read and write rates independently +optional, preserving this distinction through current-history labels. + Physical disk source freshness preserves the collector-authored `expectedUpdateIntervalSeconds` alongside the actual last observation. Registry ingest, merge, cloning and typed disk views preserve it. Staleness uses the diff --git a/frontend-modern/browser-verification.json b/frontend-modern/browser-verification.json index a942fbaa7..dd102f878 100644 --- a/frontend-modern/browser-verification.json +++ b/frontend-modern/browser-verification.json @@ -1,18 +1,34 @@ { "version": 1, - "base_sha": "9b4fa52d84e6196108e961ccba2b317df497268b", - "verified_at": "2026-09-06T10:15:18.709765Z", + "base_sha": "6b0abc3bee9ffa81f6ab298b5b67ee11369688a0", + "verified_at": "2026-09-06T11:20:14.502Z", "result": "passed", "changed_paths": [ - "frontend-modern/src/components/AI/Chat/ChatMessages.tsx", - "frontend-modern/src/components/AI/Chat/hooks/useChat.ts" + "frontend-modern/src/components/Infrastructure/UnifiedResourceHostTableCard.tsx", + "frontend-modern/src/components/Infrastructure/infrastructureSelectors.ts", + "frontend-modern/src/features/docker/DockerHostDrawerOverview.tsx", + "frontend-modern/src/features/standalone/AgentsMachinesTable.tsx", + "frontend-modern/src/features/standalone/agentMachineTableModel.ts", + "frontend-modern/src/hooks/useColumnVisibility.ts", + "frontend-modern/src/hooks/useUnifiedResources.ts", + "frontend-modern/src/types/resource.ts", + "frontend-modern/src/utils/format.ts" ], "content_sha256": { - "frontend-modern/src/components/AI/Chat/ChatMessages.tsx": "9672f7608d1e3a531c73cba20fd4a78752316783212afd0c292ddfd11d2bf371", - "frontend-modern/src/components/AI/Chat/hooks/useChat.ts": "0b56b7a56e35d51ca96f0e126dd493b3164aa9e0ad4d8ae24bcf3af7a574b97c" + "frontend-modern/src/components/Infrastructure/UnifiedResourceHostTableCard.tsx": "f183ef158f646d52b9b2ff6c3af028d7b9b993a27542690ad664ab84f93d0d73", + "frontend-modern/src/components/Infrastructure/infrastructureSelectors.ts": "a3ff0b74581e769db70b12dba94e30324aa52ccbb7242b7513cf82b8f8e2fd5e", + "frontend-modern/src/features/docker/DockerHostDrawerOverview.tsx": "8957154b965f4e4240e7c19888fada63dcaf518a9e0e3a0e384b184ebaf4e716", + "frontend-modern/src/features/standalone/AgentsMachinesTable.tsx": "4342ba59fe9dbd33ae85131a2da45ac7ebeb7efe61457b43ce32640109bd3214", + "frontend-modern/src/features/standalone/agentMachineTableModel.ts": "ae419b49a1d761910612bf95b82e9c6226374c648d48d252259ed70b30b955c0", + "frontend-modern/src/hooks/useColumnVisibility.ts": "0112d88ca35fc0f1f3866070aa0913c706370bd6c0fb954afb7a89a34bd25cf3", + "frontend-modern/src/hooks/useUnifiedResources.ts": "4a434a0c0d2f42c1ee9dbed9ca3353ee5978821df2d9d725db1f4904f7a77b9e", + "frontend-modern/src/types/resource.ts": "a4d45afd0a4797ffe7075e8c4b0890e59f573ea8503a31eee68e22d2843cedd8", + "frontend-modern/src/utils/format.ts": "e0db6c68c1219481a9a3562239b35027916a04b1af6ec8df6fce0534a54ef8db" }, "routes": [ - "/patrol" + "/docker", + "/patrol", + "/standalone/machines" ], "viewports": [ { @@ -29,12 +45,15 @@ } ], "states": [ - "Captured healthy and dependency Assistant responses replayed without provider calls. All 7 and 16 terminal tool records retain exact inputs, outputs and distinct rows. Success, failed NO_AGENT and unsupported config results inspected.", - "Controlled stream: concurrent same-name calls, repeated starts, progress, two separate approval cards, sibling completion, canceled call and failed completion. No real action was attempted.", - "Restored persisted healthy and dependency sessions retain original prompts and evidence. Artificial route warning reflects blocked non-GET readiness checks, not route qualification." + "Live Docker container Overview and retained History against rebuilt Pro backend 59f05f954ff8080bd3e8f3054b2b059281c49172ee771b2e454c255241158a4a. Public history excludes legacy capacity and ambiguous I/O, while new observed rates remain available. Binary hash checked before and after.", + "Controlled Docker history loading, empty/unavailable, observed read zero with absent write, and error states. Controlled host partial read/write in Docker Overview and Machines. Missing directions display a dash and observed zero displays 0 B/s.", + "Controlled Assistant history tool completion on /patrol preserves zero read samples and an empty write series. This qualifies rendering only. Provider calls and infrastructure mutations were blocked. The artificial route warning is not provider health evidence.", + "Machines Disk I/O column enabled from fresh preferences, tooltip open/closed and persisted after reload at desktop. Existing responsive policy hides the column at 900 and 390 pixels. The unused UnifiedResourceHostTableCard has type and selector coverage, not an active-route browser claim." ], "interactions": [ - "Hover, focus, Enter/Space expand and collapse, scroll expanded output to its end, Escape, reload and select the original persisted session. Tool details and wrapping inspected at desktop, intermediate and narrow widths.", - "Private evidence: /Volumes/Development/pulse/tmp/patrol-assistant-lab-readiness/identity-verified-healthy, identity-verified-fault and identity-states. Baseline reproductions retained in identity-before-fault and failed focused regressions." + "Container selection, Overview/History switching, keyboard focus, chart hover, scroll to deepest history, collapse and reopen, reload and history re-entry at 1440, 900 and 390 pixels. Inspected actual pixels for placement, wrapping and scrolling.", + "Machines View > Columns > Disk I/O, selected state, Escape from nested picker and parent, hover and keyboard focus on I/O tooltip, Escape and outside dismissal, reload preserving enabled column. Docker host Overview inspected at all three widths.", + "Assistant tool details expanded and collapsed with Enter and Space, output scrolled and inspected, and Escape dismissal at all widths. No action or model diagnosis was simulated as qualification.", + "Private final-source evidence: /Volumes/Development/pulse/tmp/patrol-docker-observed-metrics/browser, evidence-states and host-states. All three Playwright scripts passed after the final source edit. Runtime source hashes are recorded in runtime-binding.json." ] } diff --git a/frontend-modern/src/components/Infrastructure/UnifiedResourceHostTableCard.tsx b/frontend-modern/src/components/Infrastructure/UnifiedResourceHostTableCard.tsx index c6bbdd20c..e81d3612f 100644 --- a/frontend-modern/src/components/Infrastructure/UnifiedResourceHostTableCard.tsx +++ b/frontend-modern/src/components/Infrastructure/UnifiedResourceHostTableCard.tsx @@ -3,6 +3,7 @@ import type { Component } from 'solid-js'; import { formatBytes, formatSpeed, + formatObservedSpeed, formatUptime, getResourceDiskSummary, normalizeDiskArray, @@ -334,8 +335,11 @@ export const UnifiedResourceHostTableCard: Component getOutlierEmphasis(networkTotal(), table.ioScale().network), ); - const diskIOTotal = createMemo( - () => (resource.diskIO?.readRate ?? 0) + (resource.diskIO?.writeRate ?? 0), + const diskIOTotal = createMemo(() => + resource.diskIO?.readRate !== undefined && + resource.diskIO?.writeRate !== undefined + ? resource.diskIO.readRate + resource.diskIO.writeRate + : NaN, ); const diskIOEmphasis = createMemo(() => getOutlierEmphasis(diskIOTotal(), table.ioScale().diskIO), @@ -662,11 +666,11 @@ export const UnifiedResourceHostTableCard: Component - {formatSpeed(resource.diskIO!.readRate)} + {formatObservedSpeed(resource.diskIO!.readRate)} W @@ -675,11 +679,11 @@ export const UnifiedResourceHostTableCard: Component - {formatSpeed(resource.diskIO!.writeRate)} + {formatObservedSpeed(resource.diskIO!.writeRate)} diff --git a/frontend-modern/src/components/Infrastructure/__tests__/infrastructureSelectors.test.ts b/frontend-modern/src/components/Infrastructure/__tests__/infrastructureSelectors.test.ts index d71e08b27..23406beda 100644 --- a/frontend-modern/src/components/Infrastructure/__tests__/infrastructureSelectors.test.ts +++ b/frontend-modern/src/components/Infrastructure/__tests__/infrastructureSelectors.test.ts @@ -458,3 +458,16 @@ describe('infrastructureSelectors', () => { }); }); }); + +it('does not compare partial disk observations as complete throughput totals', () => { + const known = [ + makeResource(1, { diskIO: { readRate: 0, writeRate: 0 } }), + makeResource(2, { diskIO: { readRate: 100, writeRate: 200 } }), + ]; + const partial = [ + makeResource(3, { diskIO: { readRate: 10000 } }), + makeResource(4, { diskIO: { writeRate: 0 } }), + makeResource(5, { diskIO: {} }), + ]; + expect(computeIOScale([...known, ...partial]).diskIO).toEqual(computeIOScale(known).diskIO); +}); diff --git a/frontend-modern/src/components/Infrastructure/infrastructureSelectors.ts b/frontend-modern/src/components/Infrastructure/infrastructureSelectors.ts index 230feaee7..2110fe9a3 100644 --- a/frontend-modern/src/components/Infrastructure/infrastructureSelectors.ts +++ b/frontend-modern/src/components/Infrastructure/infrastructureSelectors.ts @@ -86,7 +86,9 @@ const getSortValue = (resource: Resource, key: string): number | string | null = case 'network': return resource.network ? resource.network.rxBytes + resource.network.txBytes : null; case 'diskio': - return resource.diskIO ? resource.diskIO.readRate + resource.diskIO.writeRate : null; + return resource.diskIO?.readRate !== undefined && resource.diskIO?.writeRate !== undefined + ? resource.diskIO.readRate + resource.diskIO.writeRate + : null; case 'source': return getInfrastructureSystemIdentitySortLabel(resource); case 'temp': @@ -349,9 +351,8 @@ export const computeIOScale = ( networkValues.push(networkTotal); } - const diskIOTotal = (resource.diskIO?.readRate ?? 0) + (resource.diskIO?.writeRate ?? 0); - if (resource.diskIO) { - diskIOValues.push(diskIOTotal); + if (resource.diskIO?.readRate !== undefined && resource.diskIO?.writeRate !== undefined) { + diskIOValues.push(resource.diskIO.readRate + resource.diskIO.writeRate); } } diff --git a/frontend-modern/src/features/docker/DockerHostDrawerOverview.tsx b/frontend-modern/src/features/docker/DockerHostDrawerOverview.tsx index 3d77cdedb..f3cce4702 100644 --- a/frontend-modern/src/features/docker/DockerHostDrawerOverview.tsx +++ b/frontend-modern/src/features/docker/DockerHostDrawerOverview.tsx @@ -20,7 +20,13 @@ import { hostOverrideIdCandidates } from '@/features/alerts/alertOverridesModel' import { areSystemSettingsLoaded, shouldHideDockerUpdateActions } from '@/stores/systemSettings'; import { useAlertsActivation } from '@/stores/alertsActivation'; import type { Resource } from '@/types/resource'; -import { formatBytes, formatRelativeTime, formatSpeed, normalizeDiskArray } from '@/utils/format'; +import { + formatBytes, + formatRelativeTime, + formatSpeed, + formatObservedSpeed, + normalizeDiskArray, +} from '@/utils/format'; import { formatTemperature, getTemperatureTextClass } from '@/utils/temperature'; interface DockerHostDrawerOverviewProps { @@ -279,7 +285,7 @@ export function DockerHostDrawerOverview(props: DockerHostDrawerOverviewProps) { ) { rows.push({ label: 'Disk I/O', - value: `${formatSpeed(props.host.diskIO?.readRate ?? 0)} / ${formatSpeed(props.host.diskIO?.writeRate ?? 0)}`, + value: `${formatObservedSpeed(props.host.diskIO?.readRate)} / ${formatObservedSpeed(props.host.diskIO?.writeRate)}`, }); } return rows; diff --git a/frontend-modern/src/features/standalone/AgentsMachinesTable.tsx b/frontend-modern/src/features/standalone/AgentsMachinesTable.tsx index 698aeb37c..9aad830f5 100644 --- a/frontend-modern/src/features/standalone/AgentsMachinesTable.tsx +++ b/frontend-modern/src/features/standalone/AgentsMachinesTable.tsx @@ -61,7 +61,7 @@ import type { Disk } from '@/types/api'; import type { Resource, ResourceAvailabilityMeta } from '@/types/resource'; import type { MetricDisplayThresholds } from '@/utils/metricThresholds'; import { getActionableAgentIdFromResource } from '@/utils/agentResources'; -import { formatBytes, formatSpeed, normalizeDiskArray } from '@/utils/format'; +import { formatBytes, formatSpeed, formatObservedSpeed, normalizeDiskArray } from '@/utils/format'; import { STORAGE_KEYS } from '@/utils/localStorage'; import { useAlertsActivation } from '@/stores/alertsActivation'; import { notificationStore } from '@/stores/notifications'; @@ -84,7 +84,6 @@ import { getAgentMachineGPUTitle, getAgentMachineGPUUtilizationPercent, getAgentMachineDiskIODetails, - getAgentMachineDiskIOTotal, getAgentMachineIpValues, matchesAgentMachineSearch, getAgentMachineNetworkInterfaceDetails, @@ -502,9 +501,9 @@ const AgentMachineDiskIOCell: Component<{ trigger={ <> R - {formatSpeed(props.diskIO?.readRate ?? 0)} + {formatObservedSpeed(props.diskIO?.readRate)} W - {formatSpeed(props.diskIO?.writeRate ?? 0)} + {formatObservedSpeed(props.diskIO?.writeRate)} } > @@ -513,11 +512,11 @@ const AgentMachineDiskIOCell: Component<{
Read - {formatSpeed(props.diskIO?.readRate ?? 0)} + {formatObservedSpeed(props.diskIO?.readRate)} Write - {formatSpeed(props.diskIO?.writeRate ?? 0)} + {formatObservedSpeed(props.diskIO?.writeRate)}
@@ -1038,7 +1037,7 @@ const networkTitleFor = (machine: Resource): string => { const diskIOTitleFor = (machine: Resource): string => { if (!machine.diskIO) return ''; - return `Read ${formatSpeed(machine.diskIO.readRate)}\nWrite ${formatSpeed(machine.diskIO.writeRate)}`; + return `Read ${formatObservedSpeed(machine.diskIO.readRate)}\nWrite ${formatObservedSpeed(machine.diskIO.writeRate)}`; }; const agentIdentityIdFor = (machine: Resource): string => @@ -1527,7 +1526,6 @@ export const AgentsMachinesTable: Component<{ aggregateDisk() !== undefined || (disks()?.length ?? 0) > 0; const networkTotal = () => getAgentMachineNetworkTotal(machine); const networkInterfaces = () => getAgentMachineNetworkInterfaceDetails(machine); - const diskIOTotal = () => getAgentMachineDiskIOTotal(machine); const diskIODetails = () => getAgentMachineDiskIODetails(machine); const primaryIp = () => getPreferredResourceIP(machine) ?? getAgentMachinePrimaryIp(machine); @@ -1738,7 +1736,11 @@ export const AgentsMachinesTable: Component<{ class={`${getPlatformTableCellClassForKind('numeric-value')} ${machineColumnWidthClass('diskio')} text-base-content`} > { ); }); }); + +it('requires both disk directions for a throughput total', () => { + expect(getAgentMachineDiskIOTotal(resource({ diskIO: { readRate: 0, writeRate: 0 } }))).toBe(0); + expect(getAgentMachineDiskIOTotal(resource({ diskIO: { readRate: 100, writeRate: 200 } }))).toBe( + 300, + ); + expect(getAgentMachineDiskIOTotal(resource({ diskIO: { readRate: 0 } }))).toBeUndefined(); + expect(getAgentMachineDiskIOTotal(resource({ diskIO: { writeRate: 100 } }))).toBeUndefined(); +}); diff --git a/frontend-modern/src/features/standalone/agentMachineTableModel.ts b/frontend-modern/src/features/standalone/agentMachineTableModel.ts index f8307964e..5fe81f91f 100644 --- a/frontend-modern/src/features/standalone/agentMachineTableModel.ts +++ b/frontend-modern/src/features/standalone/agentMachineTableModel.ts @@ -493,8 +493,8 @@ export const getAgentMachineNetworkInterfaceDetails = ( export const getAgentMachineDiskIOTotal = (machine: Resource): number | undefined => { const read = getPlatformTableFiniteMetric(machine.diskIO?.readRate); const write = getPlatformTableFiniteMetric(machine.diskIO?.writeRate); - if (read === undefined && write === undefined) return undefined; - return (read ?? 0) + (write ?? 0); + if (read === undefined || write === undefined) return undefined; + return read + write; }; export const getAgentMachineDiskIODetails = (machine: Resource): AgentMachineDiskIODetail[] => { diff --git a/frontend-modern/src/hooks/__tests__/useColumnVisibility.test.ts b/frontend-modern/src/hooks/__tests__/useColumnVisibility.test.ts index 18ae6ed4f..e6d4460e8 100644 --- a/frontend-modern/src/hooks/__tests__/useColumnVisibility.test.ts +++ b/frontend-modern/src/hooks/__tests__/useColumnVisibility.test.ts @@ -120,6 +120,29 @@ describe('useColumnVisibility', () => { }); }); + it('does not reapply a default-hidden migration after a fresh user shows the column', async () => { + const columns: ColumnDef[] = [ + { id: 'name', label: 'Name' }, + { id: 'diskio', label: 'Disk I/O', toggleable: true, defaultHidden: true }, + ]; + let dispose = () => {}; + let visibility: ReturnType; + createRoot((d) => { + dispose = d; + visibility = useColumnVisibility(storageKey, columns, [], undefined, {}, ['diskio']); + }); + await Promise.resolve(); + visibility!.show('diskio'); + await Promise.resolve(); + expect(window.localStorage.getItem(storageKey)).toBe('[]'); + dispose(); + createRoot((d) => { + const reloaded = useColumnVisibility(storageKey, columns, [], undefined, {}, ['diskio']); + expect(reloaded.isHiddenByUser('diskio')).toBe(false); + d(); + }); + }); + it('resets back to the canonical default-hidden set', () => { createRoot((dispose) => { const columns: ColumnDef[] = [ diff --git a/frontend-modern/src/hooks/__tests__/useUnifiedResources.test.ts b/frontend-modern/src/hooks/__tests__/useUnifiedResources.test.ts index 74aba914e..45ea92111 100644 --- a/frontend-modern/src/hooks/__tests__/useUnifiedResources.test.ts +++ b/frontend-modern/src/hooks/__tests__/useUnifiedResources.test.ts @@ -1451,6 +1451,25 @@ describe('useUnifiedResources', () => { dispose(); }); + it('preserves an observed zero without inventing its absent I/O direction', async () => { + setWsConnected(false); + setWsInitialDataReceived(false); + setWsState('resources', []); + apiFetchMock.mockResolvedValueOnce({ + ok: true, + json: async () => ({ data: [{ ...v2Resource, metrics: { diskRead: { value: 0 } } }] }), + }); + let dispose = () => {}; + let result: ReturnType | undefined; + createRoot((d) => { + dispose = d; + result = useUnifiedResources(); + }); + await result!.refetch(); + expect(result!.resources()[0]?.diskIO).toEqual({ readRate: 0, writeRate: undefined }); + dispose(); + }); + it('preserves richer REST resource details across thinner websocket updates', async () => { setWsConnected(false); setWsInitialDataReceived(false); diff --git a/frontend-modern/src/hooks/useColumnVisibility.ts b/frontend-modern/src/hooks/useColumnVisibility.ts index 8a4d5bc43..23953810b 100644 --- a/frontend-modern/src/hooks/useColumnVisibility.ts +++ b/frontend-modern/src/hooks/useColumnVisibility.ts @@ -124,21 +124,21 @@ export function useColumnVisibility( const appliedDefaultHiddenMigrations = hasUserPreference ? readAppliedDefaultHiddenMigrations(storageKey, persistedIdAliases) : []; - const pendingDefaultHiddenMigrations = hasUserPreference - ? Array.from( - new Set( - defaultHiddenMigrationIds - .map((id) => id.trim()) - .filter( - (id) => - id && - effectiveDefaultHidden.includes(id) && - toggleableIds.includes(id) && - !appliedDefaultHiddenMigrations.includes(id), - ), + // Fresh preferences already contain these defaults. Mark their migration as + // applied too, so the first reload cannot undo a user's subsequent choice. + const pendingDefaultHiddenMigrations = Array.from( + new Set( + defaultHiddenMigrationIds + .map((id) => id.trim()) + .filter( + (id) => + id && + effectiveDefaultHidden.includes(id) && + toggleableIds.includes(id) && + !appliedDefaultHiddenMigrations.includes(id), ), - ) - : []; + ), + ); let defaultHiddenMigrationsPersisted = false; // Persist hidden columns to localStorage @@ -169,7 +169,7 @@ export function useColumnVisibility( createEffect(() => { const hasUnpersistedDefaultHiddenMigrations = pendingDefaultHiddenMigrations.length > 0 && !defaultHiddenMigrationsPersisted; - if (!hasUserPreference || (!persistedIdsMigrated && !hasUnpersistedDefaultHiddenMigrations)) { + if (!persistedIdsMigrated && !hasUnpersistedDefaultHiddenMigrations) { return; } persistedIdsMigrated = false; diff --git a/frontend-modern/src/hooks/useUnifiedResources.ts b/frontend-modern/src/hooks/useUnifiedResources.ts index aace88c99..a9b46a114 100644 --- a/frontend-modern/src/hooks/useUnifiedResources.ts +++ b/frontend-modern/src/hooks/useUnifiedResources.ts @@ -915,8 +915,8 @@ const toResource = (v2: APIResource): Resource => { diskIO: v2.metrics?.diskRead || v2.metrics?.diskWrite ? { - readRate: v2.metrics?.diskRead?.value ?? 0, - writeRate: v2.metrics?.diskWrite?.value ?? 0, + readRate: v2.metrics?.diskRead?.value, + writeRate: v2.metrics?.diskWrite?.value, } : undefined, uptime: diff --git a/frontend-modern/src/types/resource.ts b/frontend-modern/src/types/resource.ts index d0d1263ce..8fe58ef6e 100644 --- a/frontend-modern/src/types/resource.ts +++ b/frontend-modern/src/types/resource.ts @@ -133,8 +133,8 @@ export interface ResourceNetwork { // Disk I/O metrics (rates in bytes/sec from backend) export interface ResourceDiskIO { - readRate: number; // Read rate (bytes/sec) - writeRate: number; // Write rate (bytes/sec) + readRate?: number; // Observed read rate (bytes/sec), including measured zero. + writeRate?: number; // Absent directions remain unavailable. } // Alert associated with a resource diff --git a/frontend-modern/src/utils/__tests__/format.test.ts b/frontend-modern/src/utils/__tests__/format.test.ts index b501850ba..bf337a3b8 100644 --- a/frontend-modern/src/utils/__tests__/format.test.ts +++ b/frontend-modern/src/utils/__tests__/format.test.ts @@ -5,6 +5,7 @@ import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'; import { formatBytes, formatSpeed, + formatObservedSpeed, formatPercent, formatNumber, formatUptime, @@ -362,3 +363,13 @@ describe('getBackupInfo', () => { }); }); }); + +describe('formatObservedSpeed', () => { + it('preserves zero and leaves missing or invalid observations unavailable', () => { + expect(formatObservedSpeed(0)).toBe('0 B/s'); + expect(formatObservedSpeed(1024)).toBe('1.00 KB/s'); + for (const value of [undefined, null, -1, NaN, Infinity]) { + expect(formatObservedSpeed(value)).toBe('-'); + } + }); +}); diff --git a/frontend-modern/src/utils/format.ts b/frontend-modern/src/utils/format.ts index 223157944..01ed35b2b 100644 --- a/frontend-modern/src/utils/format.ts +++ b/frontend-modern/src/utils/format.ts @@ -67,6 +67,14 @@ export function formatSpeed(bytesPerSecond: number, decimals: number | 'auto' = return `${formatBytes(bytesPerSecond, decimals)}/s`; } +export function formatObservedSpeed(bytesPerSecond: number | null | undefined): string { + return typeof bytesPerSecond === 'number' && + Number.isFinite(bytesPerSecond) && + bytesPerSecond >= 0 + ? formatSpeed(bytesPerSecond) + : '-'; +} + export function formatPercent(value: number): string { if (!Number.isFinite(value)) return '0%'; const abs = Math.abs(value); diff --git a/internal/api/router_state_test.go b/internal/api/router_state_test.go index 2ff24521f..db89c0727 100644 --- a/internal/api/router_state_test.go +++ b/internal/api/router_state_test.go @@ -175,7 +175,7 @@ func TestRouterHandleStatePreservesNumericIdleRatesAndOmitsUnknownRates(t *testi byName[resource.Name] = resource } idle := byName["idle-vm"] - if idle.DiskIO == nil || idle.DiskIO.ReadRate != 0 || idle.DiskIO.WriteRate != 0 { + if idle.DiskIO == nil || idle.DiskIO.ReadRate == nil || idle.DiskIO.WriteRate == nil || *idle.DiskIO.ReadRate != 0 || *idle.DiskIO.WriteRate != 0 { t.Fatalf("valid idle rates were not emitted as numeric zero: %+v", idle.DiskIO) } if unknown := byName["unknown-vm"]; unknown.DiskIO != nil { diff --git a/internal/dockeragent/blockio_presence_test.go b/internal/dockeragent/blockio_presence_test.go new file mode 100644 index 000000000..da6de610e --- /dev/null +++ b/internal/dockeragent/blockio_presence_test.go @@ -0,0 +1,48 @@ +package dockeragent + +import ( + "encoding/json" + "testing" + + containertypes "github.com/moby/moby/api/types/container" + agentsdocker "github.com/rcourtman/pulse-go-rewrite/pkg/agents/docker" +) + +func TestSummarizeBlockIOPreservesDirectionPresenceAndZero(t *testing.T) { + for _, tc := range []struct { + name string + entries []containertypes.BlkioStatEntry + read, write bool + }{ + {"absent", nil, false, false}, + {"unrelated", []containertypes.BlkioStatEntry{{Op: "Total", Value: 100}}, false, false}, + {"observed idle", []containertypes.BlkioStatEntry{{Op: "Read", Value: 0}, {Op: "Write", Value: 0}}, true, true}, + {"read only idle", []containertypes.BlkioStatEntry{{Op: "Read", Value: 0}}, true, false}, + {"write only", []containertypes.BlkioStatEntry{{Op: "Write", Value: 123}}, false, true}, + } { + t.Run(tc.name, func(t *testing.T) { + got := summarizeBlockIO(containertypes.StatsResponse{BlkioStats: containertypes.BlkioStats{IoServiceBytesRecursive: tc.entries}}) + if !tc.read && !tc.write { + if got != nil { + t.Fatalf("absent counters became observations: %+v", got) + } + return + } + if got == nil { + t.Fatal("explicit counters were dropped") + } + encoded, err := json.Marshal(got) + if err != nil { + t.Fatal(err) + } + var decoded agentsdocker.ContainerBlockIO + if err := json.Unmarshal(encoded, &decoded); err != nil { + t.Fatal(err) + } + read, write := decoded.CounterPresence() + if read != tc.read || write != tc.write { + t.Fatalf("presence lost through report JSON %s: %v/%v", encoded, read, write) + } + }) + } +} diff --git a/internal/dockeragent/collect.go b/internal/dockeragent/collect.go index ef318a2c9..ebb8d0dfa 100644 --- a/internal/dockeragent/collect.go +++ b/internal/dockeragent/collect.go @@ -1318,35 +1318,34 @@ func randomDuration(max time.Duration) time.Duration { } func summarizeBlockIO(stats containertypes.StatsResponse) *agentsdocker.ContainerBlockIO { - // BlkioStats structure varies by cgroup version - // Cgroup v1: IoServiceBytesRecursive []BlkioStatEntry - // Cgroup v2: IoServiceBytesRecursive is empty? No, Docker maps it? - // Docker API guarantees IoServiceBytesRecursive is populated? - // It seems to try to handle both. - if len(stats.BlkioStats.IoServiceBytesRecursive) == 0 { return nil } var readBytes, writeBytes uint64 + var readPresent, writePresent bool for _, entry := range stats.BlkioStats.IoServiceBytesRecursive { op := strings.ToLower(entry.Op) switch op { case "read": + readPresent = true readBytes += entry.Value case "write": + writePresent = true writeBytes += entry.Value } } - if readBytes == 0 && writeBytes == 0 { + if !readPresent && !writePresent { return nil } return &agentsdocker.ContainerBlockIO{ - ReadBytes: readBytes, - WriteBytes: writeBytes, + ReadBytes: readBytes, + WriteBytes: writeBytes, + ReadBytesPresent: &readPresent, + WriteBytesPresent: &writePresent, } } diff --git a/internal/models/converters.go b/internal/models/converters.go index da5c5b882..607863401 100644 --- a/internal/models/converters.go +++ b/internal/models/converters.go @@ -1163,9 +1163,8 @@ type ResourceConvertInput struct { NetworkRX int64 NetworkTX int64 HasNetwork bool - DiskReadRate int64 - DiskWriteRate int64 - HasDiskIO bool + DiskReadRate *int64 + DiskWriteRate *int64 Temperature *float64 Uptime *int64 Tags []string @@ -1343,7 +1342,7 @@ func ConvertResourceToFrontend(input ResourceConvertInput) ResourceFrontend { } } - if input.HasDiskIO { + if input.DiskReadRate != nil || input.DiskWriteRate != nil { rf.DiskIO = &ResourceDiskIOFrontend{ ReadRate: input.DiskReadRate, WriteRate: input.DiskWriteRate, diff --git a/internal/models/converters_test.go b/internal/models/converters_test.go index 2fab91577..2c2f24aac 100644 --- a/internal/models/converters_test.go +++ b/internal/models/converters_test.go @@ -289,6 +289,7 @@ func TestVMToFrontend_NegativeNetworkValues(t *testing.T) { } func TestConvertResourceToFrontendIncludesDiskIO(t *testing.T) { + read, write := int64(4096), int64(8192) frontend := ConvertResourceToFrontend(ResourceConvertInput{ ID: "agent-1", Type: "agent", @@ -299,15 +300,14 @@ func TestConvertResourceToFrontendIncludesDiskIO(t *testing.T) { SourceType: "agent", Status: "online", LastSeenUnix: time.Now().UnixMilli(), - HasDiskIO: true, - DiskReadRate: 4096, - DiskWriteRate: 8192, + DiskReadRate: &read, + DiskWriteRate: &write, }) if frontend.DiskIO == nil { t.Fatal("expected disk I/O rates to be present") } - if frontend.DiskIO.ReadRate != 4096 || frontend.DiskIO.WriteRate != 8192 { + if frontend.DiskIO.ReadRate == nil || frontend.DiskIO.WriteRate == nil || *frontend.DiskIO.ReadRate != 4096 || *frontend.DiskIO.WriteRate != 8192 { t.Fatalf("unexpected disk I/O rates: %+v", frontend.DiskIO) } } diff --git a/internal/models/models_frontend.go b/internal/models/models_frontend.go index 5eecd6ab7..36ed0a480 100644 --- a/internal/models/models_frontend.go +++ b/internal/models/models_frontend.go @@ -1152,8 +1152,8 @@ type ResourceNetworkFrontend struct { // ResourceDiskIOFrontend represents aggregate disk I/O rates for the frontend. type ResourceDiskIOFrontend struct { - ReadRate int64 `json:"readRate"` - WriteRate int64 `json:"writeRate"` + ReadRate *int64 `json:"readRate,omitempty"` + WriteRate *int64 `json:"writeRate,omitempty"` } // ResourceAlertFrontend represents an alert on a resource. diff --git a/internal/monitoring/canonical_guardrails_test.go b/internal/monitoring/canonical_guardrails_test.go index ac7f80490..bd58ae94d 100644 --- a/internal/monitoring/canonical_guardrails_test.go +++ b/internal/monitoring/canonical_guardrails_test.go @@ -211,15 +211,15 @@ func TestProxmoxActionObserverUsesDirectControlPlaneClient(t *testing.T) { } func TestBroadcastResourceDiskIOUsesUnifiedResourceMetrics(t *testing.T) { - hasDiskIO, readRate, writeRate := monitorDiskIOMetricInput(&unifiedresources.ResourceMetrics{ + readRate, writeRate := monitorDiskIOMetricInput(&unifiedresources.ResourceMetrics{ DiskRead: &unifiedresources.MetricValue{Value: 4096.4, Unit: "bytes/s", Source: unifiedresources.SourceAgent}, DiskWrite: &unifiedresources.MetricValue{Value: 8191.6, Unit: "bytes/s", Source: unifiedresources.SourceAgent}, }) - if !hasDiskIO { + if readRate == nil || writeRate == nil { t.Fatal("expected disk I/O metrics to be projected") } - if readRate != 4096 || writeRate != 8192 { - t.Fatalf("unexpected projected disk I/O rates: read=%d write=%d", readRate, writeRate) + if *readRate != 4096 || *writeRate != 8192 { + t.Fatalf("unexpected projected disk I/O rates: read=%d write=%d", *readRate, *writeRate) } } diff --git a/internal/monitoring/docker_metric_presence_test.go b/internal/monitoring/docker_metric_presence_test.go new file mode 100644 index 000000000..717d745e3 --- /dev/null +++ b/internal/monitoring/docker_metric_presence_test.go @@ -0,0 +1,164 @@ +package monitoring + +import ( + "encoding/json" + "github.com/rcourtman/pulse-go-rewrite/internal/models" + "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/mock" + agentsdocker "github.com/rcourtman/pulse-go-rewrite/pkg/agents/docker" + "github.com/rcourtman/pulse-go-rewrite/pkg/metrics" +) + +func TestApplyDockerReportPreservesDiskObservationPresence(t *testing.T) { + previous := mock.IsMockEnabled() + mustSetMockEnabled(t, false) + t.Cleanup(func() { mustSetMockEnabled(t, previous) }) + m := newTestMonitor(t) + store, err := metrics.NewStore(metrics.DefaultConfig(t.TempDir())) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { store.Close() }) + m.metricsStore = store + start := time.Now().Add(-time.Minute) + report := agentsdocker.Report{ + Agent: agentsdocker.AgentInfo{ID: "presence-agent", Version: "6.4.2", IntervalSeconds: 30}, + Host: agentsdocker.HostInfo{Hostname: "presence-host"}, + Containers: []agentsdocker.Container{{ID: "presence-container", Name: "api", WritableLayerBytes: 200, RootFilesystemBytes: 1000}}, + } + observed := &agentsdocker.ContainerBlockIO{ReadBytes: 5000, WriteBytes: 7000} + steps := []struct { + name string + io *agentsdocker.ContainerBlockIO + wantSamples int + }{ + {"absent", nil, 0}, + {"first observation", observed, 0}, + {"measured idle", observed, 1}, + {"missing after observation", nil, 1}, + {"idle after gap", observed, 2}, + } + for i, step := range steps { + t.Run(step.name, func(t *testing.T) { + report.Timestamp = start.Add(time.Duration(i) * time.Second) + report.Containers[0].BlockIO = step.io + host, err := m.ApplyDockerReport(report, nil) + if err != nil { + t.Fatal(err) + } + if len(host.Containers) != 1 { + t.Fatalf("containers: %+v", host.Containers) + } + ct := host.Containers[0] + if step.io == nil && ct.BlockIO != nil { + t.Fatalf("absent BlockIO retained: %+v", ct.BlockIO) + } + if i == 1 && (ct.BlockIO.ReadRateBytesPerSecond != nil || ct.BlockIO.WriteRateBytesPerSecond != nil) { + t.Fatalf("warmup became a rate: %+v", ct.BlockIO) + } + if i == 2 || i == 4 { + if ct.BlockIO.ReadRateBytesPerSecond == nil || *ct.BlockIO.ReadRateBytesPerSecond != 0 || ct.BlockIO.WriteRateBytesPerSecond == nil || *ct.BlockIO.WriteRateBytesPerSecond != 0 { + t.Fatalf("unchanged counters must remain measured idle across gaps: %+v", ct.BlockIO) + } + } + store.Flush() + for _, metric := range []string{"diskread", "diskwrite"} { + points := m.metricsHistory.GetGuestMetrics("docker:"+ct.ID, metric, time.Hour) + if len(points) != step.wantSamples { + t.Fatalf("%s history = %d, want %d", metric, len(points), step.wantSamples) + } + persisted, err := store.Query("dockerContainer", ct.ID, metric, start, time.Now().Add(time.Minute), 0) + if err != nil { + t.Fatal(err) + } + if step.wantSamples == 0 && len(persisted) != 0 { + t.Fatalf("fabricated persisted %s: %+v", metric, persisted) + } + if step.wantSamples > 0 && (len(persisted) == 0 || persisted[len(persisted)-1].Value != 0) { + t.Fatalf("lost persisted idle %s: %+v", metric, persisted) + } + } + if points := m.metricsHistory.GetGuestMetrics("docker:"+ct.ID, "disk", time.Hour); len(points) != 0 { + t.Fatalf("layer sizes became capacity history: %+v", points) + } + points, err := store.Query("dockerContainer", ct.ID, "disk", start, time.Now().Add(time.Minute), 0) + if err != nil || len(points) != 0 { + t.Fatalf("layer sizes became persisted capacity: %+v, %v", points, err) + } + }) + } +} + +func TestApplyDockerReportPreservesIndependentZeroCounters(t *testing.T) { + previous := mock.IsMockEnabled() + mustSetMockEnabled(t, false) + t.Cleanup(func() { mustSetMockEnabled(t, previous) }) + m := newTestMonitor(t) + present, absent := true, false + report := agentsdocker.Report{ + Agent: agentsdocker.AgentInfo{ID: "zero-agent", Version: "6.4.2", IntervalSeconds: 30}, + Host: agentsdocker.HostInfo{Hostname: "zero-host"}, + Containers: []agentsdocker.Container{{ID: "zero-container", Name: "idle", BlockIO: &agentsdocker.ContainerBlockIO{ReadBytesPresent: &present, WriteBytesPresent: &absent}}}, + } + for i := 0; i < 2; i++ { + report.Timestamp = time.Now().Add(time.Duration(i) * time.Second) + host, err := m.ApplyDockerReport(report, nil) + if err != nil { + t.Fatal(err) + } + io := host.Containers[0].BlockIO + if io == nil { + t.Fatal("explicit zero counter lost") + } + if io.WriteRateBytesPerSecond != nil { + t.Fatalf("missing write direction became measured rate: %+v", io) + } + if i == 0 && io.ReadRateBytesPerSecond != nil { + t.Fatal("first zero counter became a measured rate") + } + if i == 1 && (io.ReadRateBytesPerSecond == nil || *io.ReadRateBytesPerSecond != 0) { + t.Fatalf("measured zero lost: %+v", io) + } + } + if points := m.metricsHistory.GetGuestMetrics("docker:zero-container", "diskwrite", time.Hour); len(points) != 0 { + t.Fatalf("missing writes recorded: %+v", points) + } + if points := m.metricsHistory.GetGuestMetrics("docker:zero-container", "diskread", time.Hour); len(points) != 1 { + t.Fatalf("idle reading lost: %+v", points) + } +} + +func TestResourceDiskIOWirePreservesAbsentDirection(t *testing.T) { + zero := &unifiedresources.MetricValue{Value: 0, Unit: "bytes/s", Source: unifiedresources.SourceDocker} + for _, tc := range []struct { + name string + metrics *unifiedresources.ResourceMetrics + wire string + }{ + {"absent", nil, ``}, + {"read idle", &unifiedresources.ResourceMetrics{DiskRead: zero}, `{"readRate":0}`}, + {"write idle", &unifiedresources.ResourceMetrics{DiskWrite: zero}, `{"writeRate":0}`}, + {"both idle", &unifiedresources.ResourceMetrics{DiskRead: zero, DiskWrite: zero}, `{"readRate":0,"writeRate":0}`}, + } { + t.Run(tc.name, func(t *testing.T) { + read, write := monitorDiskIOMetricInput(tc.metrics) + frontend := models.ConvertResourceToFrontend(models.ResourceConvertInput{DiskReadRate: read, DiskWriteRate: write}) + if tc.wire == "" { + if frontend.DiskIO != nil { + t.Fatalf("absent IO became a payload: %+v", frontend.DiskIO) + } + return + } + wire, err := json.Marshal(frontend.DiskIO) + if err != nil { + t.Fatal(err) + } + if string(wire) != tc.wire { + t.Fatalf("wire = %s, want %s", wire, tc.wire) + } + }) + } +} diff --git a/internal/monitoring/issue1613_contract_test.go b/internal/monitoring/issue1613_contract_test.go index 01e69fed3..7211b51d0 100644 --- a/internal/monitoring/issue1613_contract_test.go +++ b/internal/monitoring/issue1613_contract_test.go @@ -138,7 +138,7 @@ func TestIssue1613NodeDoesNotGreyBetweenNinetySecondPolls(t *testing.T) { } } -func TestIssue1613WebsocketStateKeepsUnknownRatesNumeric(t *testing.T) { +func TestIssue1613WebsocketStateKeepsObservedZeroAndOmitsUnknownRates(t *testing.T) { monitor := &Monitor{ state: models.NewState(), resourceStore: &resourceOnlyStore{resources: []unifiedresources.Resource{ @@ -168,7 +168,7 @@ func TestIssue1613WebsocketStateKeepsUnknownRatesNumeric(t *testing.T) { t.Fatal(err) } wire := string(payload) - if !strings.Contains(wire, `"diskIO":{"readRate":0,"writeRate":0}`) { + if !strings.Contains(wire, `"diskIO":{"readRate":0}`) { t.Fatalf("websocket payload does not contain numeric valid zero disk rate: %s", wire) } if strings.Count(wire, `"diskIO"`) != 1 { diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index 11a370b76..f0c9b0328 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -6181,10 +6181,7 @@ func monitorResourceToConvertInput(resource unifiedresources.Resource) models.Re input.HasNetwork = hasNetwork input.NetworkRX = rx input.NetworkTX = tx - hasDiskIO, diskRead, diskWrite := monitorDiskIOMetricInput(resource.Metrics) - input.HasDiskIO = hasDiskIO - input.DiskReadRate = diskRead - input.DiskWriteRate = diskWrite + input.DiskReadRate, input.DiskWriteRate = monitorDiskIOMetricInput(resource.Metrics) return input } @@ -6615,20 +6612,20 @@ func monitorNetworkMetricInput(metrics *unifiedresources.ResourceMetrics) (bool, return true, rx, tx } -func monitorDiskIOMetricInput(metrics *unifiedresources.ResourceMetrics) (bool, int64, int64) { +func monitorDiskIOMetricInput(metrics *unifiedresources.ResourceMetrics) (*int64, *int64) { if metrics == nil || (metrics.DiskRead == nil && metrics.DiskWrite == nil) { - return false, 0, 0 + return nil, nil } - - var read int64 - var write int64 + var read, write *int64 if metrics.DiskRead != nil { - read = int64(math.Round(metrics.DiskRead.Value)) + value := int64(math.Round(metrics.DiskRead.Value)) + read = &value } if metrics.DiskWrite != nil { - write = int64(math.Round(metrics.DiskWrite.Value)) + value := int64(math.Round(metrics.DiskWrite.Value)) + write = &value } - return true, read, write + return read, write } func monitorTemperature(resource unifiedresources.Resource) *float64 { diff --git a/internal/monitoring/monitor_agents.go b/internal/monitoring/monitor_agents.go index a4bf3c3af..732916aed 100644 --- a/internal/monitoring/monitor_agents.go +++ b/internal/monitoring/monitor_agents.go @@ -2347,10 +2347,15 @@ func (m *Monitor) ApplyDockerReport(report agentsdocker.Report, tokenRecord *con containerIdentifier = payload.Name } if strings.TrimSpace(containerIdentifier) != "" { + readPresent, writePresent := payload.BlockIO.CounterPresence() metrics := models.IOMetrics{ NetworkIn: clampToInt64(payload.NetworkRXBytes), NetworkOut: clampToInt64(payload.NetworkTXBytes), Timestamp: receivedAt, + Presence: models.IOCounterPresence{ + Explicit: true, DiskRead: readPresent, DiskWrite: writePresent, + NetworkIn: true, NetworkOut: true, + }, } if payload.BlockIO != nil { metrics.DiskRead = clampToInt64(payload.BlockIO.ReadBytes) @@ -2633,16 +2638,9 @@ func (m *Monitor) ApplyDockerReport(report agentsdocker.Report, tokenRecord *con } metricKey := fmt.Sprintf("docker:%s", container.ID) - var diskPercent float64 - if container.RootFilesystemBytes > 0 && container.WritableLayerBytes > 0 { - diskPercent = float64(container.WritableLayerBytes) / float64(container.RootFilesystemBytes) * 100 - if diskPercent > 100 { - diskPercent = 100 - } - } - - var diskReadRate float64 - var diskWriteRate float64 + // Layer sizes describe container images, not filesystem capacity. + // Missing rate observations must not become measured idle samples. + diskReadRate, diskWriteRate := -1.0, -1.0 if container.BlockIO != nil { if container.BlockIO.ReadRateBytesPerSecond != nil { diskReadRate = *container.BlockIO.ReadRateBytesPerSecond @@ -2655,7 +2653,6 @@ func (m *Monitor) ApplyDockerReport(report agentsdocker.Report, tokenRecord *con if m.metricsHistory != nil { m.metricsHistory.AddGuestMetric(metricKey, "cpu", models.DockerContainerCPUCapacityPercent(container, host.CPUs), now) m.metricsHistory.AddGuestMetric(metricKey, "memory", container.MemoryPercent, now) - m.metricsHistory.AddGuestMetric(metricKey, "disk", diskPercent, now) if container.NetInRate >= 0 { m.metricsHistory.AddGuestMetric(metricKey, "netin", container.NetInRate, now) } @@ -2673,7 +2670,6 @@ func (m *Monitor) ApplyDockerReport(report agentsdocker.Report, tokenRecord *con if m.metricsStore != nil { m.metricsStore.Write("dockerContainer", container.ID, "cpu", models.DockerContainerCPUCapacityPercent(container, host.CPUs), now) m.metricsStore.Write("dockerContainer", container.ID, "memory", container.MemoryPercent, now) - m.metricsStore.Write("dockerContainer", container.ID, "disk", diskPercent, now) if container.NetInRate >= 0 { m.metricsStore.Write("dockerContainer", container.ID, "netin", container.NetInRate, now) } diff --git a/internal/monitoring/monitor_unified_state_test.go b/internal/monitoring/monitor_unified_state_test.go index 9011a8e87..e920fec1e 100644 --- a/internal/monitoring/monitor_unified_state_test.go +++ b/internal/monitoring/monitor_unified_state_test.go @@ -165,7 +165,7 @@ func TestConvertResourcesForBroadcastCoalescesSplitHostResources(t *testing.T) { if resource.DiskIO == nil { t.Fatal("expected aggregate disk I/O rates in broadcast resource") } - if resource.DiskIO.ReadRate != 4096 || resource.DiskIO.WriteRate != 8192 { + if resource.DiskIO.ReadRate == nil || resource.DiskIO.WriteRate == nil || *resource.DiskIO.ReadRate != 4096 || *resource.DiskIO.WriteRate != 8192 { t.Fatalf("unexpected aggregate disk I/O rates: %+v", resource.DiskIO) } } diff --git a/internal/unifiedresources/metrics.go b/internal/unifiedresources/metrics.go index 6c50f57b1..399060c00 100644 --- a/internal/unifiedresources/metrics.go +++ b/internal/unifiedresources/metrics.go @@ -270,17 +270,8 @@ func metricsFromDockerContainer(ct models.DockerContainer, hostCPUs ...int) *Res percent := percentFromReportedPercent(ct.MemoryPercent) metrics.Memory = &MetricValue{Used: &ct.MemoryUsage, Total: &ct.MemoryLimit, Percent: percent, Unit: "bytes", Source: SourceDocker} } - if ct.RootFilesystemBytes > 0 { - used := ct.WritableLayerBytes - if used < 0 { - used = 0 - } - if used > ct.RootFilesystemBytes { - used = ct.RootFilesystemBytes - } - percent := clampMetricValue((float64(used)/float64(ct.RootFilesystemBytes))*100, 0, 100) - metrics.Disk = &MetricValue{Used: &used, Total: &ct.RootFilesystemBytes, Percent: percent, Unit: "bytes", Source: SourceDocker} - } + // Writable and root layer sizes are image metadata, not used/total + // filesystem capacity. Docker does not supply a capacity observation here. if ct.NetInRate > 0 { metrics.NetIn = &MetricValue{Value: ct.NetInRate, Unit: "bytes/s", Source: SourceDocker} } @@ -288,10 +279,10 @@ func metricsFromDockerContainer(ct models.DockerContainer, hostCPUs ...int) *Res metrics.NetOut = &MetricValue{Value: ct.NetOutRate, Unit: "bytes/s", Source: SourceDocker} } if ct.BlockIO != nil { - if ct.BlockIO.ReadRateBytesPerSecond != nil && *ct.BlockIO.ReadRateBytesPerSecond > 0 { + if ct.BlockIO.ReadRateBytesPerSecond != nil && *ct.BlockIO.ReadRateBytesPerSecond >= 0 && !math.IsInf(*ct.BlockIO.ReadRateBytesPerSecond, 0) { metrics.DiskRead = &MetricValue{Value: *ct.BlockIO.ReadRateBytesPerSecond, Unit: "bytes/s", Source: SourceDocker} } - if ct.BlockIO.WriteRateBytesPerSecond != nil && *ct.BlockIO.WriteRateBytesPerSecond > 0 { + if ct.BlockIO.WriteRateBytesPerSecond != nil && *ct.BlockIO.WriteRateBytesPerSecond >= 0 && !math.IsInf(*ct.BlockIO.WriteRateBytesPerSecond, 0) { metrics.DiskWrite = &MetricValue{Value: *ct.BlockIO.WriteRateBytesPerSecond, Unit: "bytes/s", Source: SourceDocker} } } diff --git a/internal/unifiedresources/metrics_test.go b/internal/unifiedresources/metrics_test.go index d99e54af5..b1513c437 100644 --- a/internal/unifiedresources/metrics_test.go +++ b/internal/unifiedresources/metrics_test.go @@ -1,12 +1,34 @@ package unifiedresources import ( + "math" "testing" "time" "github.com/rcourtman/pulse-go-rewrite/internal/models" ) +func TestMetricsFromDockerContainerDistinguishesAbsentAndIdleIO(t *testing.T) { + for _, value := range []float64{0, 123, -1, math.NaN(), math.Inf(1)} { + ct := models.DockerContainer{BlockIO: &models.DockerContainerBlockIO{ReadRateBytesPerSecond: &value, WriteRateBytesPerSecond: &value}} + got := metricsFromDockerContainer(ct) + valid := value >= 0 && !math.IsInf(value, 0) + if valid { + if got.DiskRead == nil || got.DiskWrite == nil || got.DiskRead.Value != value || got.DiskWrite.Value != value { + t.Fatalf("lost measured rate %v: %+v", value, got) + } + } else if got.DiskRead != nil || got.DiskWrite != nil { + t.Fatalf("invalid rate %v became observation: %+v", value, got) + } + } + for _, io := range []*models.DockerContainerBlockIO{nil, {ReadBytes: 5000, WriteBytes: 7000}} { + got := metricsFromDockerContainer(models.DockerContainer{BlockIO: io}) + if got.DiskRead != nil || got.DiskWrite != nil { + t.Fatalf("absent rates became observations: %+v", got) + } + } +} + func TestMetricsFromDockerHostIncludesIORates(t *testing.T) { host := models.DockerHost{ CPUUsage: 12.5, @@ -447,8 +469,8 @@ func TestMetricsFromDockerContainerIncludesContainerIORates(t *testing.T) { if metrics.DiskWrite == nil || metrics.DiskWrite.Value != writeRate { t.Fatalf("expected diskWrite=%v, got %+v", writeRate, metrics.DiskWrite) } - if metrics.Disk == nil || metrics.Disk.Percent <= 0 { - t.Fatalf("expected non-zero disk usage metric, got %+v", metrics.Disk) + if metrics.Disk != nil { + t.Fatalf("container layer sizes are not filesystem capacity, got %+v", metrics.Disk) } } diff --git a/pkg/agents/docker/blockio_presence_test.go b/pkg/agents/docker/blockio_presence_test.go new file mode 100644 index 000000000..0e507b911 --- /dev/null +++ b/pkg/agents/docker/blockio_presence_test.go @@ -0,0 +1,32 @@ +package dockeragent + +import ( + "encoding/json" + "testing" +) + +func TestContainerBlockIOCounterPresenceWireCompatibility(t *testing.T) { + for _, tc := range []struct { + wire string + read, write bool + }{ + {`null`, false, false}, + {`{}`, false, false}, + {`{"readBytes":5000}`, true, false}, + {`{"writeBytes":7000}`, false, true}, + {`{"readBytes":5000,"writeBytes":7000}`, true, true}, + {`{"readBytesPresent":true,"writeBytesPresent":true}`, true, true}, + {`{"readBytes":5000,"readBytesPresent":false,"writeBytesPresent":true}`, false, true}, + } { + t.Run(tc.wire, func(t *testing.T) { + var io *ContainerBlockIO + if err := json.Unmarshal([]byte(tc.wire), &io); err != nil { + t.Fatal(err) + } + read, write := io.CounterPresence() + if read != tc.read || write != tc.write { + t.Fatalf("presence = %v/%v, want %v/%v", read, write, tc.read, tc.write) + } + }) + } +} diff --git a/pkg/agents/docker/report.go b/pkg/agents/docker/report.go index 04459ba3e..e23eb8697 100644 --- a/pkg/agents/docker/report.go +++ b/pkg/agents/docker/report.go @@ -124,8 +124,27 @@ type ContainerNetwork struct { // ContainerBlockIO summarises high-level block I/O metrics for a container. type ContainerBlockIO struct { - ReadBytes uint64 `json:"readBytes,omitempty"` - WriteBytes uint64 `json:"writeBytes,omitempty"` + ReadBytes uint64 `json:"readBytes,omitempty"` + WriteBytes uint64 `json:"writeBytes,omitempty"` + ReadBytesPresent *bool `json:"readBytesPresent,omitempty"` + WriteBytesPresent *bool `json:"writeBytesPresent,omitempty"` +} + +// CounterPresence distinguishes observed zero from an omitted direction. Older +// agents omitted zero values and presence, so only their positive counters are +// unambiguous. New agents carry explicit presence independently of counter value. +func (io *ContainerBlockIO) CounterPresence() (read, write bool) { + if io == nil { + return false, false + } + read, write = io.ReadBytes > 0, io.WriteBytes > 0 + if io.ReadBytesPresent != nil { + read = *io.ReadBytesPresent + } + if io.WriteBytesPresent != nil { + write = *io.WriteBytesPresent + } + return } // PodmanContainer carries metadata extracted from Podman-specific annotations. diff --git a/pkg/metrics/docker_observation_contract.go b/pkg/metrics/docker_observation_contract.go new file mode 100644 index 000000000..8b03199b5 --- /dev/null +++ b/pkg/metrics/docker_observation_contract.go @@ -0,0 +1,35 @@ +package metrics + +// Docker history written before explicit counter presence mixed unavailable +// readings with measured zero. Its disk percentage also measured image-layer +// composition rather than filesystem capacity. Keep those rows for retention +// and rollback, but never reinterpret them as observations under this contract. +// +// The app-container storage family is shared with other providers. Their valid +// new capacity readings remain supported. Public metric names are unchanged. +func hasDockerObservationContract(resourceType string) bool { + return resourceType == "dockercontainer" || resourceType == "docker" +} + +func storedObservationMetric(resourceType, metricType string) string { + if hasDockerObservationContract(resourceType) { + switch metricType { + case "disk", "diskread", "diskwrite": + return metricType + ".observed" + } + } + return metricType +} + +func projectDockerObservations(result map[string]map[string][]MetricPoint) { + for _, series := range result { + for _, metric := range []string{"disk", "diskread", "diskwrite"} { + delete(series, metric) + stored := metric + ".observed" + if points, ok := series[stored]; ok { + series[metric] = points + delete(series, stored) + } + } + } +} diff --git a/pkg/metrics/store.go b/pkg/metrics/store.go index 00281afe7..dfe48f58c 100644 --- a/pkg/metrics/store.go +++ b/pkg/metrics/store.go @@ -174,10 +174,11 @@ type SeriesKey struct { // NormalizedSeriesKey builds the SeriesKey the write path would store for the // given identifiers, so callers can match MaxTimestampsForTier results. func NormalizedSeriesKey(resourceType, resourceID, metricType string) SeriesKey { + resourceType = normalizeMetricResourceType(resourceType) return SeriesKey{ - ResourceType: normalizeMetricResourceType(resourceType), + ResourceType: resourceType, ResourceID: normalizeMetricIdentifier(resourceID), - MetricType: normalizeMetricType(metricType), + MetricType: storedObservationMetric(resourceType, normalizeMetricType(metricType)), } } @@ -723,7 +724,7 @@ func validateMetricWrite(resourceType, resourceID, metricType string, tier Tier) return "", "", "", false, fmt.Sprintf("unsupported metric tier %q", tier) } - return normalizedType, normalizedID, normalizedMetric, true, "" + return normalizedType, normalizedID, storedObservationMetric(normalizedType, normalizedMetric), true, "" } // Write adds a metric to the write buffer with the 'raw' tier by default @@ -1297,6 +1298,11 @@ func (s *Store) queryBatch( return map[string]map[string][]MetricPoint{}, nil } normalizedMetricTypes := normalizeMetricTypes(metricTypes) + if hasDockerObservationContract(resourceType) { + for i, metric := range normalizedMetricTypes { + normalizedMetricTypes[i] = storedObservationMetric(resourceType, metric) + } + } tiers := s.tierFallbacks(end.Sub(start)) if len(tiers) == 0 { @@ -1730,6 +1736,9 @@ func (s *Store) queryRetainedChunk(resourceType string, resourceIDs []string, me flushBucket() flushSeries() + if hasDockerObservationContract(resourceType) { + projectDockerObservations(result) + } return result, nil } diff --git a/pkg/metrics/store_docker_observation_contract_test.go b/pkg/metrics/store_docker_observation_contract_test.go new file mode 100644 index 000000000..cde253191 --- /dev/null +++ b/pkg/metrics/store_docker_observation_contract_test.go @@ -0,0 +1,148 @@ +package metrics + +import ( + "fmt" + "testing" + "time" +) + +func TestDockerObservationContractSeparatesLegacyAcrossRetainedReads(t *testing.T) { + for _, family := range []string{"dockerContainer", "docker"} { + t.Run(family, func(t *testing.T) { + store, err := NewStore(DefaultConfig(t.TempDir())) + if err != nil { + t.Fatal(err) + } + defer store.Close() + ts := time.Now().Add(-10 * time.Minute).Truncate(time.Minute) + // Seed the old physical schema directly. New public writes must not grant + // these ambiguous historical rows an observation provenance retroactively. + for _, id := range []string{"a", "b", "legacy-only"} { + for _, metric := range []string{"disk", "diskread", "diskwrite"} { + for _, tier := range []Tier{TierRaw, TierMinute, TierHourly} { + _, err := store.db.Exec(`INSERT INTO metrics(resource_type,resource_id,metric_type,value,timestamp,tier) VALUES(?,?,?,?,?,?)`, normalizeMetricResourceType(family), id, metric, 99, ts.Unix(), string(tier)) + if err != nil { + t.Fatal(err) + } + } + } + } + for _, id := range []string{"a", "b"} { + // Exercise the buffered, synchronous and bounded writer entry points. + store.Write(family, id, "diskread", 0, ts) + store.WriteBatchSync([]WriteMetric{{ResourceType: family, ResourceID: id, MetricType: "diskwrite", Value: 0, Timestamp: ts, Tier: TierRaw}}) + store.WriteBatchBounded([]WriteMetric{{ResourceType: family, ResourceID: id, MetricType: "disk", Value: 25, Timestamp: ts, Tier: TierRaw}}) + store.Write(family, id, "cpu", 0, ts) + } + store.Flush() + // Both generations roll up independently. Old minute/hourly data must + // neither replace the corrected zeros nor contaminate their averages. + if !store.rollupTierWindow(TierRaw, TierMinute, 60, ts.Unix()-60, ts.Unix()+120) { + t.Fatal("rollup failed") + } + coverage, err := store.MaxTimestampsForTier(TierRaw) + if err != nil { + t.Fatal(err) + } + if !coverage[NormalizedSeriesKey(family, "a", "diskread")].Equal(ts) { + t.Fatalf("physical coverage mismatch: %+v", coverage) + } + for _, step := range []int64{0, 60} { + t.Run(fmt.Sprintf("step-%d", step), func(t *testing.T) { + start, end := ts.Add(-2*time.Hour), ts.Add(time.Hour) + assert := func(series map[string][]MetricPoint, legacy bool) { + t.Helper() + for _, metric := range []string{"disk", "diskread", "diskwrite"} { + points := series[metric] + if legacy { + if len(points) > 0 { + t.Fatalf("legacy %s leaked: %+v", metric, points) + } + continue + } + want := 0.0 + if metric == "disk" { + want = 25 + } + if len(points) != 1 || points[0].Value != want || points[0].Min != want || points[0].Max != want { + t.Fatalf("%s observation contaminated: %+v", metric, points) + } + if _, exists := series[metric+".observed"]; exists { + t.Fatalf("physical metric leaked: %+v", series) + } + } + } + all, err := store.QueryAll(family, "a", start, end, step) + if err != nil { + t.Fatal(err) + } + assert(all, false) + if len(all["cpu"]) != 1 || all["cpu"][0].Value != 0 { + t.Fatalf("unrelated zero lost: %+v", all) + } + batch, err := store.QueryAllBatch(family, []string{"a", "b", "legacy-only"}, start, end, step) + if err != nil { + t.Fatal(err) + } + assert(batch["a"], false) + assert(batch["b"], false) + assert(batch["legacy-only"], true) + selected, err := store.QueryMetricTypesBatch(family, []string{"a", "b", "legacy-only"}, []string{"disk", "diskread", "diskwrite"}, start, end, step) + if err != nil { + t.Fatal(err) + } + assert(selected["a"], false) + assert(selected["b"], false) + assert(selected["legacy-only"], true) + for _, metric := range []string{"disk", "diskread", "diskwrite"} { + points, err := store.Query(family, "a", metric, start, end, step) + if err != nil { + t.Fatal(err) + } + if len(points) != 1 || points[0].Value != all[metric][0].Value { + t.Fatalf("selected %s differs: %+v", metric, points) + } + old, err := store.Query(family, "legacy-only", metric, start, end, step) + if err != nil || len(old) != 0 { + t.Fatalf("legacy selected %s leaked: %+v %v", metric, old, err) + } + } + }) + } + var legacyCount int + err = store.db.QueryRow(`SELECT COUNT(*) FROM metrics WHERE resource_type=? AND metric_type IN ('disk','diskread','diskwrite')`, normalizeMetricResourceType(family)).Scan(&legacyCount) + if err != nil || legacyCount != 27 { + t.Fatalf("legacy history was changed: count=%d err=%v", legacyCount, err) + } + }) + } +} + +func TestDockerObservationContractLeavesOtherFamiliesUnchanged(t *testing.T) { + store, err := NewStore(DefaultConfig(t.TempDir())) + if err != nil { + t.Fatal(err) + } + defer store.Close() + ts := time.Now().Truncate(time.Second) + for _, family := range []string{"vm", "ct", "agent", "dockerHost", "disk", "storage", "k8s"} { + for _, metric := range []string{"disk", "diskread", "diskwrite"} { + store.WriteWithTier(family, "one", metric, 0, ts, TierRaw) + } + } + store.Flush() + for _, family := range []string{"vm", "ct", "agent", "dockerHost", "disk", "storage", "k8s"} { + all, err := store.QueryAll(family, "one", ts.Add(-time.Second), ts.Add(time.Second), 0) + if err != nil { + t.Fatal(err) + } + for _, metric := range []string{"disk", "diskread", "diskwrite"} { + if len(all[metric]) != 1 || all[metric][0].Value != 0 { + t.Fatalf("%s/%s zero changed: %+v", family, metric, all) + } + if NormalizedSeriesKey(family, "one", metric).MetricType != metric { + t.Fatal("unrelated physical key changed") + } + } + } +} diff --git a/scripts/release_control/canonical_completion_guard_test.py b/scripts/release_control/canonical_completion_guard_test.py index 08bfcc31d..0697340f4 100644 --- a/scripts/release_control/canonical_completion_guard_test.py +++ b/scripts/release_control/canonical_completion_guard_test.py @@ -225,6 +225,7 @@ class CanonicalCompletionGuardTest(unittest.TestCase): "internal/config/host_continuity_test.go", "internal/models/metrics_types_test.go", "internal/monitoring/availability_probe_agent_test.go", + "internal/monitoring/docker_metric_presence_test.go", "internal/monitoring/monitor_host_agent_removal_lifecycle_test.go", "internal/monitoring/monitor_host_agents_test.go", "scripts/installtests/agent_state_dir_lifecycle_test.go", @@ -354,6 +355,7 @@ class CanonicalCompletionGuardTest(unittest.TestCase): "internal/dockeragent/agent_collect_test.go", "internal/dockeragent/agent_cpu_test.go", "internal/dockeragent/agent_internal_test.go", + "internal/dockeragent/blockio_presence_test.go", "internal/dockeragent/swarm_coverage_test.go", ], } @@ -451,6 +453,7 @@ class CanonicalCompletionGuardTest(unittest.TestCase): "internal/config/host_continuity_test.go", "internal/models/metrics_types_test.go", "internal/monitoring/availability_probe_agent_test.go", + "internal/monitoring/docker_metric_presence_test.go", "internal/monitoring/monitor_host_agent_removal_lifecycle_test.go", "internal/monitoring/monitor_host_agents_test.go", "scripts/installtests/agent_state_dir_lifecycle_test.go", @@ -481,6 +484,7 @@ class CanonicalCompletionGuardTest(unittest.TestCase): "test_prefixes": [], "exact_files": [ "internal/config/host_continuity_test.go", + "internal/monitoring/docker_metric_presence_test.go", "internal/monitoring/issue1485_unraid_lifecycle_test.go", "internal/monitoring/issue1595_collection_trust_test.go", "internal/monitoring/monitor_docker_test.go", diff --git a/scripts/release_control/subsystem_lookup_test.py b/scripts/release_control/subsystem_lookup_test.py index 5823d4d78..197272676 100644 --- a/scripts/release_control/subsystem_lookup_test.py +++ b/scripts/release_control/subsystem_lookup_test.py @@ -4327,6 +4327,7 @@ class SubsystemLookupTest(unittest.TestCase): "internal/config/host_continuity_test.go", "internal/models/metrics_types_test.go", "internal/monitoring/availability_probe_agent_test.go", + "internal/monitoring/docker_metric_presence_test.go", "internal/monitoring/monitor_host_agent_removal_lifecycle_test.go", "internal/monitoring/monitor_host_agents_test.go", "scripts/installtests/agent_state_dir_lifecycle_test.go", @@ -4349,6 +4350,7 @@ class SubsystemLookupTest(unittest.TestCase): monitoring_match["verification_requirement"]["exact_files"], [ "internal/config/host_continuity_test.go", + "internal/monitoring/docker_metric_presence_test.go", "internal/monitoring/issue1485_unraid_lifecycle_test.go", "internal/monitoring/issue1595_collection_trust_test.go", "internal/monitoring/monitor_docker_test.go",