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

[sbates130272]

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]

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]

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]

@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]

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]

@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.

[igaw]

Thanks for the PR. I like where this is going. I suggest though, to really introduce a --format-version=1 for the legacy case and --format-version=2 for the new. Because legacy and new will eventually replaced by next generation, followed by the even better one :)

[igaw]

Hmm, I think it should be called --output-format-version to avoid collisions with other commands.

[sbates130272]

Thanks @igaw. I will update the PR with that and include the documentation update. Stay tuned!

[sbates130272]

@igaw can you take a look at my updated branch? I think I caught your requests and added a documentation update.