Add 'list_namespaces_io_stats' grpc call#1769
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new Gateway gRPC API to fetch I/O stats across all namespaces (via SPDK bdev iostat) and exposes it through a new CLI subcommand.
Changes:
- Extend
GatewaygRPC service withlist_namespaces_io_statsand new response messages for aggregated bdev I/O stats. - Implement
list_namespaces_io_statshandler in the Python gRPC server usingspdk_rpc_client.bdev_get_iostat(). - Add CLI command
namespace ns_list_io_statsto display aggregated I/O stats in text/table or structured formats.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
control/proto/gateway.proto |
Adds new RPC + request/response messages for listing all namespaces’ I/O stats. |
control/grpc.py |
Implements the new RPC by calling SPDK bdev_get_iostat and mapping the result into the proto response. |
control/cli.py |
Adds a new namespace subcommand that calls the new RPC and formats the output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
gbregman
left a comment
There was a problem hiding this comment.
Do we need separate commands for one namespace and all the namespaces? We don't do that in other cases. We just rely on "--nsid" to execute the command for one namespace or all.
2784ed3 to
b63c752
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b63c752 to
4a00e8b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4a00e8b to
6ebdf77
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
control/cli.py:3009
- The JSON output structure for the CLI command 'namespace get_io_stats' has changed significantly. Previously, when querying a specific namespace with --subsystem and --nsid, the JSON output included top-level fields like 'subsystem_nqn', 'nsid', 'uuid', and 'bdev_name', along with the stats. Now, the stats are returned in a 'namespaces' array, and these identifying fields are no longer present in the JSON output. This is a breaking change for any scripts or tools that parse the JSON output of this CLI command. Consider documenting this breaking change in the PR description or release notes, and consider whether the old format should be preserved for backward compatibility.
elif args.format == "json" or args.format == "yaml":
ret_str = json_format.MessageToJson(ns_io_stats, indent=4,
including_default_value_fields=True,
preserving_proto_field_name=True)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
And make namespace_get_io_stats grpc method wrapper around list_namespaces_io_stats. Also change "namespace get_io_stats" cli cmd output from dictionary of bdev stats to list of bdev stats. Make --nsid & --subsystem optional args, and return single device stats if present. Otherwise, return all bdev stats in a list. Signed-off-by: Vallari Agrawal <val.agl002@gmail.com>
6ebdf77 to
b94454d
Compare
And make namespace_get_io_stats grpc method
wrapper around list_namespaces_io_stats.
Also change "namespace get_io_stats" cli cmd
output from dictionary of bdev stats to list
of bdev stats. Make --nsid & --subsystem optional
args, and return single device stats if present.
Otherwise, return all bdev stats in a list.