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

nvme_metrics: support nvme-cli v2.11+ verbose JSON output #227

Merged
merged 2 commits into from
Nov 19, 2024

Conversation

dswarbrick
Copy link
Member

nvme-cli v2.11 introduced a breaking change by forcing JSON output to always be verbose, resulting in a significantly different output structure.

Also drop the obsolete nvme_metrics.sh shell script collector, which will not work with nvme-cli v2.11+ (unless it were also refactored).

Fixes: #226

nvme-cli v2.11 introduced a breaking change by forcing JSON output to
always be verbose, resulting in a significantly different output
structure.

Fixes: #226

Signed-off-by: Daniel Swarbrick <[email protected]>
Since the shell script collector does not support nvme-cli v2.11+, and
was already effectively superseded by nvme_metrics.py, this is no longer
needed.

Signed-off-by: Daniel Swarbrick <[email protected]>
@dswarbrick dswarbrick requested a review from SuperQ November 19, 2024 04:18
@dswarbrick
Copy link
Member Author

dswarbrick commented Nov 19, 2024

This PR aims solely to add support for nvme-cli v2.11+, which produces more verbose and deeply nested JSON output than previous versions of the tool.

Some decisions that were made in the past regarding the nvme_metrics collector (i.e., the original shell script version) do not translate so well to this deeply nested information, and take a rather naïve approach to NVMe namespaces (effectively equating them to individual devices). This will ideally require some breaking changes to the metrics produced in terms of metric names and labels.

At the same time, there is a growing likelihood that this collector may be run in environments which have non-local NVMe devices, e.g. NVMe over fabrics or NVMe over TCP, which should perhaps be distinguished with a transport label in their info metric.

Copy link
Contributor

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@dswarbrick dswarbrick merged commit a2b43e1 into master Nov 19, 2024
6 checks passed
@dswarbrick dswarbrick deleted the nvme-cli-verbose branch November 19, 2024 08:25
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.

nvme-cli v2.11 breaks compatibility with nvme textfile collector
2 participants