-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat(balloon): add logging for malformed balloon-stats tag #5572
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
base: main
Are you sure you want to change the base?
Conversation
031a64c to
903fd8b
Compare
Codecov Report❌ Patch coverage is Please upload reports for the commit 7c8a9a4 to get more accurate results.
Additional details and impacted files@@ Coverage Diff @@
## main #5572 +/- ##
==========================================
- Coverage 83.24% 83.23% -0.01%
==========================================
Files 277 277
Lines 29263 29265 +2
==========================================
Hits 24359 24359
- Misses 4904 4906 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thank you for your contribution looks good. Just a couple of minor comments |
68f2864 to
5cd0e68
Compare
|
Thank you @JackThomson2 for the review! I have addressed your comments. Please take another look when you have time! |
Added warn logging to the decode of BalloonStats if unknown tag is introduced. Additionally, updated integration test of BalloonStats to fail if this warn occurs. The Result type from the update method is removed and corresponding update metrics have been removed as it is no longer tracked. Signed-off-by: Aaron Lo <aaronlo0929@gmail.com>
5cd0e68 to
7c8a9a4
Compare
|
Thank you for the comments! I dropped the Result type from |
Changes
Replaced Error with warn logging to the decode of BalloonStats if unknown tag is introduced. Additionally, updated integration test of BalloonStats to fail if this warn occurs.
The Result type from the update method is removed and corresponding update metrics have been removed as it is no longer tracked.
Reason
Attempting to resolve Issue 5542
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
tools/devtool checkbuild --allto verify that the PR passesbuild checks on all supported architectures.
tools/devtool checkstyleto verify that the PR passes theautomated style checks.
how they are solving the problem in a clear and encompassing way.
in the PR.
CHANGELOG.md.Runbook for Firecracker API changes.
integration tests.
TODO.rust-vmm.