-
Notifications
You must be signed in to change notification settings - Fork 23
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
Added DiskIO Reporter #825
Conversation
Old Energy EstimationEco-CI Output:
🌳 CO2 Data: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My C is a little rusty. Not sure I found everything.
Eco-CI Output:
🌳 CO2 Data: |
@ribalba All done. Quick Re-review please as I changed this also in all other C source files we have |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
* main: Authentication in GMT (#872) Phase name check (#893) Carbondb unique (#877) Added DiskIO Reporter (#825) Bump pylint from 3.3.0 to 3.3.1 (#913) Bump aiohttp from 3.10.5 to 3.10.6 (#912) Bump plotext from 5.3.1 to 5.3.2 (#914) Bump pandas from 2.2.2 to 2.2.3 (#910) Bump plotext from 5.2.8 to 5.3.1 (#909) Bump pylint from 3.2.7 to 3.3.0 (#911) Bump psycopg-pool from 3.2.2 to 3.2.3 (#901) Bump pydantic from 2.9.1 to 2.9.2 (#904) Bump fastapi[standard] from 0.114.2 to 0.115.0 (#905) Importer for Measurements (#908)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR adds a Disk I/O reporter.
To make it container agnostic the data is pulled directly from the
sysfs
. This needs the IO delegator to be set as specified in the documentation.I am currently under the assumption that this controller is always active for docker in root mode, but will double check soon.
The approach here is similar to the other cgroup reading metric providers. However while crafting this the idea came up to actually split Input and Output and not supply the value as a cumulated number.
I see here two options:
Here I think @ribalba it makes sense to discuss how this was solved in the lm_sensors