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

Field descriptors in OCP SMART Log Page are different to those in standard SMART Log Page #2577

Open
sbates130272 opened this issue Nov 15, 2024 · 5 comments

Comments

@sbates130272
Copy link
Contributor

sbates130272 commented Nov 15, 2024

There is a lack of consistency between the field descriptors in the standard SMART log page (which uses all lower case and underscores) and the OCP SMART extended log page (which uses mixed cases and white spaces). This is rather annoying and also poses a bit of a challenge when trying to scrape the OCP metrics via a Prometheus node-exporter text collector.

I think it would be lovely to alter the format of the OCP output (both stdout and JSON) but I am concerned that might cause an issue for people who have already written tools that parse the existing log page output. So I am creating this issue so we can discuss if such a change is welcome or not.

$ nvme smart-log /dev/nvme0n1 -o json
{
  "critical_warning":0,
  "temperature":323,
  <snip>
}

versus

$ nvme ocp smart-add-log /dev/nvme0n1 -o json
{
  <snip>
  "Bad user nand blocks - Raw":0,
  "Bad user nand blocks - Normalized":0,
  <snip>
}

Note this is somewhat related to #2189 but is different enough that I wanted to create a new issue. Depending on how this resolves it may allow us to close the other issue.

@igaw
Copy link
Collaborator

igaw commented Nov 21, 2024

Changing the existing output is likely to cause regression, which I really like to avoid. We could introduce a output format version flag so the user can decide which version should be used. Luckily we have a plugin interface for this already in place.

Obvious it means additional maintenance overhead. When we do the next major version update we just rip out the older version then.

@sbates130272
Copy link
Contributor Author

Note there is a conversation going on in prometheus-community/node-exporter-textfile-collector-scripts#228 that also deals with JSON output format changes and some issues around per controller and per namespace SMART log pages.

@sbates130272
Copy link
Contributor Author

@igaw I will try and put together a PR for this in the next few days. I've noted your comments above and will do something that aligns with those.

@igaw
Copy link
Collaborator

igaw commented Nov 21, 2024

Cool. You don't need to go overboard in the first version. I think we have to figure out how to make this maintainable somehow without duplicating too much.

sbates130272 added a commit to sbates130272/nvme-cli that referenced this issue Nov 22, 2024
The current OCP JSON format for the SMART extended log page is not
condusive to metric collection via tools like Prometheus. So we add a
new output mode that uses all lower case and underscores (instead of
spaces). This should help with metric collection.

Fixes linux-nvme#2577.

Signed-off-by: Stephen Bates <[email protected]>
sbates130272 added a commit to sbates130272/nvme-cli that referenced this issue Nov 22, 2024
The current OCP JSON format for the SMART extended log page is not
condusive to metric collection via tools like Prometheus. So we add a
new output mode that uses all lower case and underscores (instead of
spaces). This should help with metric collection. At the same time we
clean up some of the field names.

Note documentation has NOT yet been updated.

Fixes linux-nvme#2577.

Signed-off-by: Stephen Bates <[email protected]>
@sbates130272
Copy link
Contributor Author

@igaw I threw something together that adds a --non-legacy flag and then selects the JSON fields based on that. Take a look and see if you think it looks semi-sane. Then I can clean it up a bit and add documentation updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants