Skip to content

Save tests metrics and performance artifacts #5191

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
May 13, 2025

Conversation

ShadowCurse
Copy link
Contributor

@ShadowCurse ShadowCurse commented May 6, 2025

Changes

Save metrics and performance artifacts(logs/json files) we get from test into test_results for further processing.
Allow to change the results directory for CI.

Previously the content of test_results looked like this:

test_results
  - test-report.json 

Now it will be:

test_results
  - test-report.json
  - test_boottime
    - 'test_boottime[vmlinux-5.10.233-1-128]'
      - metrics.json
      - other test related files
  - ...

If the CI is run in BK the test_results directory will be archived into test_results/data.tar.gz

Reason

Currently the only output of tests we have are emitted metrics. This means we discard a lot of data we get during test runs (mainly perf tests). This is turn makes it more difficult to analyze performance of Firecracker to the point where we have to use custom scripts to do so. By simply saving all original data, we simplify data gathering and processing step.

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.

PR Checklist

  • I have read and understand CONTRIBUTING.md.
  • I have run tools/devtool checkstyle to verify that the PR passes the
    automated style checks.
  • I have described what is done in these changes, why they are needed, and
    how they are solving the problem in a clear and encompassing way.
  • I have updated any relevant documentation (both in code and in the docs)
    in the PR.
  • I have mentioned all user-facing changes in CHANGELOG.md.
  • If a specific issue led to this PR, this PR closes the issue.
  • When making API changes, I have followed the
    Runbook for Firecracker API changes.
  • I have tested all new and changed functionalities in unit tests and/or
    integration tests.
  • I have linked an issue to every new TODO.

  • This functionality cannot be added in rust-vmm.

@ShadowCurse ShadowCurse force-pushed the save_perf_data branch 2 times, most recently from 9c1a1d2 to d8f774d Compare May 6, 2025 14:46
Copy link

codecov bot commented May 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.93%. Comparing base (85a094f) to head (9303fd7).
Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5191      +/-   ##
==========================================
+ Coverage   82.88%   82.93%   +0.05%     
==========================================
  Files         250      250              
  Lines       26936    26936              
==========================================
+ Hits        22325    22339      +14     
+ Misses       4611     4597      -14     
Flag Coverage Δ
5.10-c5n.metal 83.36% <ø> (-0.01%) ⬇️
5.10-m5n.metal 83.36% <ø> (ø)
5.10-m6a.metal 82.59% <ø> (+<0.01%) ⬆️
5.10-m6g.metal 79.19% <ø> (ø)
5.10-m6i.metal 83.36% <ø> (ø)
5.10-m7a.metal-48xl 82.57% <ø> (?)
5.10-m7g.metal 79.19% <ø> (ø)
5.10-m7i.metal-24xl 83.32% <ø> (?)
5.10-m7i.metal-48xl 83.32% <ø> (?)
5.10-m8g.metal-24xl 79.19% <ø> (?)
5.10-m8g.metal-48xl 79.19% <ø> (?)
6.1-c5n.metal 83.42% <ø> (+<0.01%) ⬆️
6.1-m5n.metal 83.41% <ø> (ø)
6.1-m6a.metal 82.63% <ø> (ø)
6.1-m6g.metal 79.19% <ø> (-0.01%) ⬇️
6.1-m6i.metal 83.40% <ø> (ø)
6.1-m7a.metal-48xl 82.62% <ø> (?)
6.1-m7g.metal 79.19% <ø> (ø)
6.1-m7i.metal-24xl 83.43% <ø> (?)
6.1-m7i.metal-48xl 83.42% <ø> (?)
6.1-m8g.metal-24xl 79.19% <ø> (?)
6.1-m8g.metal-48xl 79.19% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ShadowCurse ShadowCurse force-pushed the save_perf_data branch 9 times, most recently from b427828 to 3719b26 Compare May 6, 2025 16:36
@ShadowCurse ShadowCurse self-assigned this May 6, 2025
@ShadowCurse ShadowCurse added the Type: Enhancement Indicates new feature requests label May 6, 2025
@ShadowCurse ShadowCurse marked this pull request as ready for review May 6, 2025 16:39
@ShadowCurse ShadowCurse added the Status: Awaiting review Indicates that a pull request is ready to be reviewed label May 6, 2025
@ShadowCurse ShadowCurse requested a review from pb8o May 6, 2025 16:39
pb8o
pb8o previously requested changes May 6, 2025
@ShadowCurse ShadowCurse force-pushed the save_perf_data branch 2 times, most recently from 717f67b to 91befbb Compare May 6, 2025 17:53
@pb8o pb8o dismissed their stale review May 7, 2025 07:21

fixed

@ShadowCurse ShadowCurse force-pushed the save_perf_data branch 3 times, most recently from 0dde2df to 4696ae8 Compare May 7, 2025 10:05
@ShadowCurse ShadowCurse force-pushed the save_perf_data branch 5 times, most recently from baa10a9 to addfd53 Compare May 7, 2025 16:37
roypat
roypat previously approved these changes May 8, 2025
Copy link
Contributor

@roypat roypat left a comment

Choose a reason for hiding this comment

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

validated everything still works by running a dummy A/B test

@ShadowCurse ShadowCurse force-pushed the save_perf_data branch 3 times, most recently from a7155a9 to 53310e7 Compare May 12, 2025 11:35
roypat
roypat previously approved these changes May 12, 2025
@ShadowCurse ShadowCurse requested a review from pb8o May 12, 2025 12:29
Run formatter on python.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
For each test store metrics it emits into a metrics.json
file. This makes it easier to look at the metrics.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
Save logs and json files for block perf tests.

For log parsing, specify the root_dir for `glob.glob`
otherwise it wll struggle with directory names like:
`test_block_performance[vmlinux-5.10.233-Sync-bs4096-randread-1vcpu]`

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
Save logs and json files for network perf tests.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
Save logs and json files for vsock perf tests.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
roypat
roypat previously approved these changes May 13, 2025
Now `test_results` will contain a lot of files and directories
with all test metrics and data files.
To speed up upload and download of test_results, compress it's
content if tests are run in CI.
Also add an `--no-archive` option to skip archiving if needed.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
Allow to change the output directory of the CI run
with --json-report-file option

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
Currently when A/B is run, only results for B test
are available in the `test_results` dir because this
dir is shared for both runs and the last one overwrites the data.
Now we store results into separate dirs.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
Cross snapshot pipeline run commands after devtool test,
so no archiving is needed.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
@roypat roypat enabled auto-merge (rebase) May 13, 2025 15:10
@roypat roypat merged commit 3c3233c into firecracker-microvm:main May 13, 2025
6 of 7 checks passed
@ShadowCurse ShadowCurse deleted the save_perf_data branch May 13, 2025 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Awaiting review Indicates that a pull request is ready to be reviewed Type: Enhancement Indicates new feature requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants