Skip to content

Remove/hook unused metrics #2901

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

Open
alindima opened this issue Feb 14, 2022 · 4 comments
Open

Remove/hook unused metrics #2901

alindima opened this issue Feb 14, 2022 · 4 comments
Assignees
Labels
Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled ` Status: Parked Indicates that an issues or pull request will be revisited later Type: Fix Indicates a fix to existing code

Comments

@alindima
Copy link
Contributor

We have some metrics that are never incremented, for example: sync_response_fails and sync_vmm_send_timeout_count (I assume there are more).
We should assess whether it's worth hooking them in the specific Firecracker code or just remove them.

@roypat
Copy link
Contributor

roypat commented Oct 9, 2023

There is some ongoing work to redesign our metrics subsystem to allow for per-device metrics by @sudanl0 at the moment, which will also tackle this issue.

@ShadowCurse ShadowCurse added Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled ` Status: Parked Indicates that an issues or pull request will be revisited later labels Sep 18, 2024
@LDagnachew
Copy link

Not sure if this is still needed, but I'm fine with taking a look at this issue if needed @roypat.

@roypat
Copy link
Contributor

roypat commented Apr 25, 2025

Not sure if this is still needed, but I'm fine with taking a look at this issue if needed @roypat.

So there are definitely a ton of metrics that are never incremented/set anywhere in the code, and cleaning that up is still very much wanted I think! It's just a bit tricky to find them, because an IDE won't be able to find them as dead code (generally, they are still formally "used" in constructors and aggregation functions, or random tests), so it's literally a matter of going through each of them and checking whether any of the uses are actually incrementing/setting the metric from production code, and if none exist, deleting it.

@mdhaduk
Copy link

mdhaduk commented May 5, 2025

Hi @roypat, I submitted a draft PR and would love some comments on my work. Feel free to take a look at your own convenience. Thanks!

Note: I'm aware I'm failing tools/devtool checkstyle; I will rebase and fix this before submitting my PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled ` Status: Parked Indicates that an issues or pull request will be revisited later Type: Fix Indicates a fix to existing code
Projects
None yet
Development

No branches or pull requests

8 participants