fix(inputs.redfish)!: Partial rewrite of redfish to use the gofish lib#18963
fix(inputs.redfish)!: Partial rewrite of redfish to use the gofish lib#18963inhinias wants to merge 0 commit into
Conversation
srebhan
left a comment
There was a problem hiding this comment.
Thank you very much for your contribution @inhinias! To make this easier to digest, can we please split the PR into the following
- PR to cleanup/reorganize existing tests, including file rename (if really necessary)
- PR to cleanup/refactor existing code
- PR to extend tests to work with the upcoming code changes, i.e. extend the mock server, use redfish types where applicable etc.
- PR to move the actual code to the gofish library without functional changes
- PR(s) to add features
where step 1 & 2 are interchangeable...
This would allow us to review the changes more efficiently and separate non-functional changes from functional ones or even breaking ones...
I will try working on separating it this week. I may require some time, as I am on vacation next week. |
|
@inhinias sure, take your time. I will probably poke you after your holidays to verify you are still there. A short heartbeat would be nice in those cases. :-) |
|
Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
Summary
The current implementation of the redfish input plugin uses hand-made types and sends out http requests based on them.
This PR partially rewrites the plugin to use the gofish library.
The metrics produced by this PR are the same as before. Except the datacenter tag.
Apparantly that one is not part of the redfish standard.
Using the lib is done mostly for convenience as it simplifies adding new data to be collected.
I require some consideration on #8093. This removes the odata-version header for ILO4.
Currently this is not implemented in the rewrite. Considering ILO4 is in end of service life since 2023, i think we should not invest much time into fixes for it. Especially since it only has partial redfish support.
I invested some time to take care of #14488 thou.
Note on #18250 :
Since the new and old API's produce somewhat similar data but not exactly the same, I decided to put all the metrics produced by the new subsystem api in a new collection, redfish_powersubsys_XXX respectivly redfish_thermalsubsys_XXX
If null values for some tags/metrics are ok, then i can provide further commits to merge the two.
Lastly, this adds support for collecting storage metrics provided by redfish.
There is an open issue to collect processor metrics too. (#17618) But since they are just static data, i am unsure if telegraf is the right tool to collect them?!
Feel free to provide feedback.
Checklist
Related issues
related to #14488
related to #8093
resolves #15191
resolves #18250
resolves #11485