Skip to content
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

Improve Bmc metrics collection #24

Merged
merged 4 commits into from
Jun 5, 2024
Merged

Improve Bmc metrics collection #24

merged 4 commits into from
Jun 5, 2024

Conversation

ErwanAliasr1
Copy link
Collaborator

This PR is about

fixing the broken logic in the mocked BMC that generated incorrect data structures
simplify the addition of a new metric logic to get common code across various BMC drivers
The PR was made with two commits to get a history of what was wrong in the mocked BMC driver and then the cleanup with a helper.

This PR also fixes the missing packaging python deps and adds a very simple job used during the development of this PR.

Commit 938cbde was adding 'packaging'
dependency that wasn't added in the requirements.

This commit is also regenerating hashes done with "make regen_hashes".

Signed-off-by: Erwan Velu <e.velu@criteo.com>
With the current code, the mocked BMC was returning a new data structure
at each read_<component>() call.

Let's detail with the CPU temperature.

At each read_thermal() call, the thermals looks like the following :
	{'CPU': {'CPU1': Temperature(name='CPU1', unit='Celsius', values=[40], mean=[], min=[], max=[], stdev=[], samples=[])}}

As per the monitoring logic, each call to read_thermal() must add a new metric in "values".
After a monitoring sampling period, mean/min/max/stdev are computed.
As values remained a single value list, the mean/min/max/stdev were
never computed leading to empty lists.

This commit is ensuring the mocked BMC is using the same logic as real
drivers so the accumulation works.

Signed-off-by: Erwan Velu <e.velu@criteo.com>
The current code was a great set of code that got copy/paste over
various functions and vendors.

This generated lots of hard to read code which is error-prone.

This commit is merging all the metric addition logic in BMC.add_monitoring_value()
so it can be used across all vendors including the mocked one.

Signed-off-by: Erwan Velu <e.velu@criteo.com>
This job is very useful to make a quick test on a host.
It can be used to validate a fresh install or even during development
phases.

Signed-off-by: Erwan Velu <e.velu@criteo.com>
@anisse anisse merged commit cb123d0 into main Jun 5, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants