Live-Metrics Support - #252
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
There are correctness/security-impacting issues (notably invalid CORS configuration and power-series timestamp construction) that should be fixed before approval.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds a new live-metrics/ component that exposes the existing Docker collector outputs as a live, pollable HTTP API for near-real-time resource utilization monitoring.
Changes:
- Introduces a FastAPI service (
/health,/metrics,/platform-info,/memory,/device-config) plus a parser that turns collector logs into JSON time series. - Extends the Docker image + supervisord setup to run the API alongside existing GPU/NPU/platform collectors.
- Makes collectors write to a configurable
RESULTS_DIRand updates Docker Compose build context to include the newlive-metrics/directory.
File summaries
| File | Description |
|---|---|
| live-metrics/requirements.txt | Adds FastAPI/uvicorn dependencies for the live API. |
| live-metrics/README.md | Documents the purpose, usage, env vars, and endpoints for live-metrics. |
| live-metrics/metrics_parser.py | Implements parsing of CPU/GPU/NPU/memory/power logs into time-series JSON payloads. |
| live-metrics/metrics_api.py | Implements the FastAPI HTTP endpoints and CORS middleware. |
| docker/supervisord.conf | Starts metrics_api under supervisord alongside existing collectors. |
| docker/scripts/collect_platform.sh | Writes platform metrics to ${RESULTS_DIR} instead of hardcoding /tmp/results. |
| docker/scripts/collect_gpu.sh | Writes qmassa output under ${RESULTS_DIR} and adds optional bounded-cycle qmassa restarts. |
| docker/Dockerfile | Copies in live-metrics/, installs its Python deps, exposes port 9000, and adjusts copy paths for docker/ layout. |
| docker/docker-compose.yaml | Adjusts build context/dockerfile path and wires env vars for RESULTS_DIR and METRICS_HTTP_PORT. |
Review details
Suppressed comments (1)
live-metrics/metrics_parser.py:90
build_memory_series()parses the entirememory_usage.logeach request. Like the CPU log, this can become a growing O(file_size) cost per poll for 24/7 usage. Consider log truncation/rotation in the collector or a tail-based parser that only reads the lastwindowsamples.
try:
lines = [l.strip() for l in _mem_log().open() if l.lstrip().startswith("Mem:")]
except (FileNotFoundError, OSError):
- Files reviewed: 9/9 changed files
- Comments generated: 5
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
72f1f6c
There was a problem hiding this comment.
🟡 Changes recommended
The live-metrics service has a few confirmed startup/runtime hazards (env parsing crash, qmassa-file rotation race, and uvicorn double-import behavior) that should be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (4)
Previously missed (4) — in code that hasn't changed since the last review.
live-metrics/metrics_api.py:87
- When this file is executed as a script (as it is under supervisord), uvicorn.run("metrics_api:app", ...) re-imports metrics_api under its module name, creating a second FastAPI instance and re-running module top-level code. Pass the in-memory app object instead to avoid double import side effects.
live-metrics/metrics_parser.py:184 - build_gpu_series() can crash with FileNotFoundError if the qmassa JSON file is deleted/rotated between globbing and selecting the newest file (collect_gpu.sh can rm/touch in a loop). Guard the max(getmtime) selection with an OSError handler.
live-metrics/metrics_parser.py:381 - build_metrics_payload() only returns the last
windowsamples, but build_cpu_series()/build_memory_series()/build_power_series() currently read and parse the full log/csv file on every request. For long-running (24/7) deployments those files will grow indefinitely, making each poll O(file_size) and increasing CPU/memory usage. Consider tail-reading only the lastwindow(+ a small buffer for headers) lines/rows per series.
live-metrics/requirements.txt:2 - These dependencies are specified as broad version ranges and without hashes, but docker/Dockerfile installs docker/requirements.txt with --require-hashes for reproducibility/supply-chain integrity. Consider pinning exact versions here (and adding hashes or using a lock/compile step) so Docker builds remain deterministic over time.
- Files reviewed: 9/9 changed files
- Comments generated: 2
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
PR Checklist
What are you changing?
Updating the performance-tools by adding
/live-metricssupport./live-metricsgives the live logs of CPU, GPU, NPU, Power and memory for resource utilization monitoring.Issue this PR will close
close: ITEP-96330
Anything the reviewer should know when reviewing this PR?
Test Instructions if applicable
README file is added, the instructions will give the guidance
If the there are associated PRs in other repositories, please link them here (i.e. intel-retail/performance-tools )