feat(metrics): add CSV extraction and CI workflow#734
Conversation
Add measure_to_csv.py to output per-container timing CSV. Make spawnContainer() and deleteContainer() configurable. Add metrics.yml to run weekly measurements and upload artifacts. Closes urunc-dev#104 Signed-off-by: Chennamma-Hotkar <channuhotkar@gmail.com>
✅ Deploy Preview for urunc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
mdryaan
left a comment
mdryaan
left a comment
There was a problem hiding this comment.
Hi @Chennamma-Hotkar, refactoring is clean and the overall structure of measure_to_csv.py looks solid. I left two inline comments: one about a possible off-by-one on the start_ns timestamp ID at line 29, and one about the action pinning convention in metrics.yml. Happy to be corrected if either is intentional.
| # Key phase definitions: (start_tsID, end_tsID, column_name) | ||
| PHASES = [ | ||
| ("TS00", "TS11", "create_ns"), # full create phase | ||
| ("TS12", "TS19", "start_ns"), # full start phase |
There was a problem hiding this comment.
I noticed "TS19" is used as the end timestamp for start_ns, but checking internal/metrics/schema.go the last defined timestamp is TS18 (RX.execve_hypervisor) TimestampCount (value 19) is a sentinel and is never written to the log file. Because get_phase_duration returns "N/A" when the end key isn't found in the map, start_ns would always come out as "N/A" in every CSV row. worth double-checking if TS18 is the intended end of the start phase, the fix would be: ("TS12", "TS18", "start_ns")
There was a problem hiding this comment.
Fixed! Changed TS19 to TS18 in commit 2a29d93. Thanks for catching that!
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
Not sure if intentional, but the other workflows in this repo all pin actions to a full commit SHA with a version comment ,no floating @v4 references anywhere else in .github/workflows/. Might be worth pinning to match the project pattern:
uses: actions/checkout@ # v4
uses: actions/upload-artifact@ # v4
There was a problem hiding this comment.
Fixed! Pinned both actions to full commit SHAs in commit 2a29d93 to match the project convention.
|
Hi @mdryaan, I’ll link both actions against the full commit SHAs in accordance with the project policy. Please let me know if you can refer me to the SHA used by other workflows. Should I refer to an existing workflow such as ci_on_push.yml instead? |
Fix off-by-one: TS19 is TimestampCount sentinel, use TS18 (RX.execve_hypervisor) as end of start phase. Pin actions/checkout and actions/upload-artifact to full commit SHAs to match project convention. Signed-off-by: Chennamma-Hotkar <channuhotkar@gmail.com>
2 participants
Description
The timestamp instrumentation already exists in the urunc codebase but there was no automated way to extract and store results.
This PR wires the existing measurement scripts into CI:
measure_to_csv.pyto produce per-container CSV timing outputspawnContainer()anddeleteContainer()configurable to support different unikernel imagesmetrics.ymlworkflow to run weekly measurements for solo5-hvt and unikraft-nginx and upload CSV as GitHub Actions artifactsNote: kill and delete timestamps are not yet instrumented in the Go codebase. This PR captures create, start, network and disk phases.
Related issues
How was this tested?
Script validated locally using mock timestamp data:
Linter (flake8):
CSV output verified:
LLM usage
N/A
Checklist
make lint).make test_ctr,make test_nerdctl,make test_docker,make test_crictl).