Skip to content

ci: record benchmark host provenance and storage telemetry - #479

Merged
Pedro Henrique Penna (ppenna) merged 1 commit into
devfrom
agents/fix-issue-119-validate-implementation
Oct 10, 2026
Merged

Pedro Henrique Penna (ppenna) merged 1 commit into
devfrom
agents/fix-issue-119-validate-implementation

Conversation

@ppenna

@ppenna Pedro Henrique Penna (ppenna) commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #119.

Summary

Windows/WHP lifecycle acceptance has split snapshot generation into fast and slow clusters, isolated to capture.mapped_memory_flush (runs 35382585623 and 35393916034). #118 made such a series remeasure once, and #203 moved benchmark scratch to a data disk, but the next split would still be hard to explain. This change makes every split self-describing:

  • Attempt identification. performance validate-openvmm --attempt-log appends each bounded attempt's file, SHA-256 digest, outcome (valid, unstable, invalid), exit status, and error to acceptance-attempts.json. performance collect --lifecycle-attempt-log fails before writing a CSV unless acceptance.json is a byte-identical copy of exactly one valid attempt, prints the attempt that supplied the published metrics, and lists every attempt in the step summary. Both acceptance steps clear a stale log and stale attempt files first.
  • Host provenance. A new nvx_tools.host_telemetry module records host in lifecycle JSON results and benchmark-metadata.json: runner and machine names, OS, CPU model, logical processors, memory, and the volume, file system, size, disk number, and SCSI address behind the workspace, temporary, and scratch directories. On a possible Azure VM it queries instance metadata once, without a proxy, and keeps only the VM size and managed-disk SKU, size, caching, write-accelerator, ephemeral, LUN, and Ultra Disk limit fields. It never records names, resource or VM IDs, the subscription, tags, addresses, or credentials. Summaries add a benchmark host table.
  • Storage telemetry (Windows). With --snapshot-profile, each retained capture samples volume IOCTL_DISK_PERFORMANCE, GetSystemTimes, and SystemPerformanceInformation about every 50 ms, plus Defender (MsMpEng.exe, MpDefenderCoreService.exe) CPU and I/O from SystemProcessInformation about every 500 ms. These work for the Network Service account that runs CI jobs, which cannot read performance counters. Windows keeps disk counters on demand, so the sampler holds the disk-performance WMI data block (query access for every account) open while it samples. Each capture's capture.mapped_memory_flush and capture.snapshot_generation windows are mapped onto the observer clock and summarized: throughput, IOPS, latency, queue length, busy time, CPU busy time, lazy-writer and flush rates, dirty cache, and Defender activity. Each counter group spans its own samples nearest to the phase boundaries, so one failed read cannot blank a group; a group whose samples differ from the phase's reports its own interval. The job log prints each sample's flush with its host activity; the step summary tabulates every attempt, rejected ones included. Failed reads are recorded and never fail a benchmark. Linux captures flush no file-backed RAM image and record none.
  • Policy. doc/benchmarks.md explains how to tell disk throttling, scanning, writeback, and unrelated host load apart, keeps runner identity diagnostic-only, and requires any runner storage change to apply to every pooled runner together with a history reset of the affected metrics.

Validation

Local (Windows 11, non-elevated):

  • SystemProcessInformation offsets match GetProcessTimes and GetProcessIoCounters exactly; available pages match GlobalMemoryStatusEx; a child process's mapped-view flush moved the flush counters by the bytes the volume wrote.
  • ruff check, ruff format --check, compileall, and Pyright (Linux and Windows) report no findings. The validate-nvx unit tests (932), adversarial (70) and host-inventory (4) tests, help smoke checks, and git diff --check pass.

Bare metal, with each CI step run verbatim from this revision's run-benchmark action using CI inputs (--cpus 0-7, reserve 0, timeout 40, 1 warmup + 10 runs):

Host Result
Linux/MSHV (Azure Linux 3.0, Xeon Silver 4114) 554 unit tests pass, including the bash acceptance-step test. Acceptance recorded attempt 1 as valid; collect named it for all 29 metrics; the summary lists the attempt and host.
Linux/KVM (Ubuntu, Xeon Silver 4114) Same: 554 unit tests pass; one valid attempt; collect names attempt 1 of 1.
Windows Server 2025/WHP (Xeon Silver 4114) Acceptance rejected attempt 1 as unstable (a 2/8 split between 488 and 725 ms, 48.4% gap, on an otherwise idle host), recorded it, and remeasured; collect named attempt 2 of 2 and tabulated both attempts' telemetry.

The Windows run exposed that Windows Server keeps disk counters on demand: the volume IOCTL_DISK_PERFORMANCE failed (ERROR_FILE_NOT_FOUND) until something enabled collection. The sampler now holds the disk-performance WMI block open (its descriptor grants query access to Everyone and full access to Network Service), and the Windows counter test passes on that host.

A rerun overlapped another session's cargo build --release -p openvmm, and the telemetry captured it: during flushes, CPU busy time rose to 7–70% (1–2% idle), dirty cache to 205–450 MiB (0.1 MiB idle), lazy writes to 38 MiB/s, and write latency to 30–240 ms, as flushes grew from 477 to 1509 ms.

Overhead: 14 interleaved e2e runs (7 pairs, both orders) on the otherwise idle Windows host gave a snapshot-generation p50 median of 527 ms with telemetry and 522 ms without; cold start (476 ms) and restore (94 ms) matched.

CI Azure runners (Standard_D8s_v5, jobs run as Network Service): in runs 38067120152 (azure-windows-3) and 38068806476 (azure-windows-5), every counter group was recorded without errors, the IMDS provenance kept only the sanitized VM size and disk fields, and collect named attempt 1 of 1. In all 20 captures, the 512 GiB Premium_LRS (P20) scratch disk wrote 148–183 MiB/s during capture.mapped_memory_flush at 93–100% busy (average write latency 66–174 ms, queue 1.0–1.8), while CPU busy time stayed at 0–7%, the dirty cache stayed under 2 MiB, and Defender used at most 9.5% of one CPU. That rate is close to the P20 tier's documented 150 MB/s provisioned and 170 MB/s credit-based burst throughput, so the flush appears bound by the scratch disk rather than by host CPU, writeback, or scanning. A faster disk tier would be a storage change that falls under the history-reset policy above.

The storage migration in #203 has aged out of the current 10-point Windows windows (all ~0.95–1.11 s), so no history reset is needed for it now.

Copilot AI balanced review requested due to automatic review settings October 10, 2026 16:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

A transient counter-read failure can suppress otherwise available phase telemetry because endpoint samples are not selected per counter group.

1 open finding
What changed in this PR

Adds benchmark-attempt tracking, safe host provenance, and Windows storage telemetry to diagnose lifecycle performance instability.

Changes:

  • Preserves and identifies lifecycle acceptance attempts.
  • Records host/storage provenance and capture telemetry.
  • Expands CI validation, summaries, tests, and documentation.
File Description
.github/​actions/​run-benchmark/​action.yml Retains attempts and publishes telemetry.
.github/​actions/​validate-nvx/​action.yml Runs telemetry tests.
.github/​skills/​nvx-benchmark/​SKILL.md Adds provenance verification.
doc/​benchmarks.md Documents telemetry and policy.
doc/​usage.md Documents new CLI options.
scripts/​nvx_tools/​benchmark.py Collects provenance and telemetry.
scripts/​nvx_tools/​host_telemetry.py Implements host and storage monitoring.
scripts/​nvx_tools/​performance.py Tracks attempts and produces summaries.
scripts/​setup/​README.md Documents storage-change policy.
scripts/​test_host_telemetry.py Tests telemetry implementation.
scripts/​test_nvx_tools.py Tests benchmark and CI integration.
scripts/​test_performance.py Tests attempt collection and reporting.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread scripts/nvx_tools/host_telemetry.py Outdated
Windows/WHP lifecycle acceptance split snapshot generation into fast and
slow clusters in runs 35382585623 and 35393916034, isolated to
capture.mapped_memory_flush. #118 made such a series remeasure once, and
b51a0b8 moved benchmark scratch off the burst-limited OS disk, but a
future split would still leave three gaps (#119):

- The published CSV could not be traced to the bounded attempt that
  supplied it; only an unlabeled copy in acceptance.json remained.
- Results did not identify the runner, its Azure VM size and disks, or
  the volumes behind the workspace, temporary, and scratch directories.
- Nothing recorded the host's disk, CPU, writeback, or Defender
  activity during the flush, so disk throttling, scanning, writeback,
  and unrelated host load could not be told apart.

Attempt identification:
- performance validate-openvmm --attempt-log appends each attempt's
  file, SHA-256 digest, outcome, exit status, and error to
  acceptance-attempts.json.
- performance collect --lifecycle-attempt-log fails before it writes a
  CSV unless acceptance.json is a byte-identical copy of exactly one
  valid attempt, prints that attempt, and lists every attempt in the
  step summary.
- Both acceptance steps remove a stale log and stale attempt files
  before measuring, and both publish steps pass the log.

Host provenance:
- The new nvx_tools.host_telemetry module records the runner and
  machine names, OS, CPU model, logical processor count, and memory,
  and the volume, file system, size, disk number, and SCSI address
  behind each benchmark directory.
- On a possible Azure VM it queries the instance metadata service once,
  bypassing proxies, and keeps only the VM size and each managed disk's
  SKU, size, caching, write-accelerator, ephemeral, LUN, and Ultra Disk
  limit fields, plus the temporary disk size. It never records names,
  resource or VM identifiers, the subscription, tags, network
  addresses, or credentials, and records a failed query as azure.error.
- Lifecycle JSON results and benchmark-metadata.json record it under
  host, acceptance and diagnostic suites print it, and summaries add a
  host table. The device-io resume identity ignores host.

Storage telemetry:
- With --snapshot-profile on Windows, a coordinator thread samples
  volume performance IOCTLs, GetSystemTimes, and
  SystemPerformanceInformation about every 50 ms, and Defender process
  CPU and I/O from SystemProcessInformation about every 500 ms, from
  just before the snapshot request until the source process exits.
  These interfaces need no privileges; the Network Service account of
  the CI jobs cannot read performance counters. Failed reads are
  recorded and never fail the benchmark.
- Each retained capture maps its capture.mapped_memory_flush and
  capture.snapshot_generation phases onto the observer clock through
  the tightest record offset and summarizes throughput, IOPS, latency,
  queue length, busy time, CPU, writeback, dirty cache, and Defender
  activity. Each counter group spans its own samples nearest to each
  phase boundary, so a failed read does not blank the group, and
  reports its sampled interval when that differs from the phase's.
- The job log prints each sample's flush with its host activity, and
  the step summary tabulates it for every attempt, rejected ones
  included. Linux captures flush no file-backed guest RAM image and
  record no storage telemetry.

The benchmark documentation explains how to read the telemetry, keeps
runner identity diagnostic only, and requires any storage change to
apply to every pooled runner together with a history reset of the
affected metrics.

Validation:
- On Windows 11 without elevation, the SystemProcessInformation offsets
  match GetProcessTimes and GetProcessIoCounters exactly, available
  pages match GlobalMemoryStatusEx, and a child process's mapped-view
  flush moved the flush counters by the bytes the volume wrote.
- ruff check and format, compileall, and Pyright for Linux and Windows
  report no findings. The validate-nvx unit tests (932 on Windows), the
  adversarial (70) and host inventory (4) tests, the help smoke checks,
  and git diff --check pass.
- On bare-metal Linux/MSHV (Azure Linux 3.0, Python 3.12) and Linux/KVM
  (Ubuntu, Python 3.14) hosts, test_host_telemetry, test_performance,
  and test_nvx_tools pass (554), including the bash acceptance-step
  test. The CI acceptance and performance-suite steps, run verbatim
  with CI inputs, recorded one valid attempt, and collect named it for
  all 29 metrics and listed the attempt and host in the summary.
- On bare-metal Windows Server 2025/WHP, the verbatim acceptance step
  rejected attempt 1 as unstable (a 2/8 split between 488 and 725 ms),
  recorded it, and remeasured; collect named attempt 2 of 2 and
  reported both attempts' telemetry. That run showed that Windows Server
  keeps disk counters on demand: the volume IOCTL failed until the
  sampler held the disk performance WMI block open, after which the
  Windows counter test passes there too.
- A rerun overlapped another session's cargo build of OpenVMM. During
  its flushes, CPU busy time rose to 7-70% (1-2% idle), the dirty cache
  to 205-450 MiB (0.1 MiB idle), lazy writes to 38 MiB/s, and write
  latency to 30-240 ms as flushes grew from 477 to 1509 ms.
- Fourteen interleaved e2e runs on the otherwise idle host gave a
  snapshot-generation p50 median of 527 ms with telemetry and 522 ms
  without; cold start (476 ms) and restore (94 ms) matched.
- In PR CI run 38067120152, the Azure Windows runner, whose jobs run
  as Network Service, recorded every counter group without errors, and
  collect named the published attempt. During each flush, the 512 GiB
  Premium_LRS (P20) scratch disk wrote 148-165 MiB/s at 94-100% busy,
  near that tier's 170 MB/s burst limit.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ppenna
Pedro Henrique Penna (ppenna) force-pushed the agents/fix-issue-119-validate-implementation branch from 3d90017 to e276144 Compare October 10, 2026 16:43
Copilot AI balanced review requested due to automatic review settings October 10, 2026 16:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Low-level Windows counter parsing, timing correlation, and CI publication behavior warrant final human review despite extensive coverage.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

@ppenna
Pedro Henrique Penna (ppenna) merged commit 82af84c into dev Oct 10, 2026
32 checks passed
@ppenna
Pedro Henrique Penna (ppenna) deleted the agents/fix-issue-119-validate-implementation branch October 10, 2026 19:15
Pedro Henrique Penna (ppenna) added a commit that referenced this pull request Oct 11, 2026
PR #479 added scripts/nvx_tools/host_telemetry.py, which records benchmark host provenance and Windows capture storage telemetry, but the source tree did not list it.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ci: improve Windows benchmark provenance and storage diagnostics

2 participants