-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
cli: Add JSON and Pretty Print formatting for consul snapshot inspect
#9006
Conversation
edc0526
to
245fa28
Compare
consul snapshot inspect
consul snapshot inspect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks great. I just have the one question about the JSON formats structure.
// through the formatter | ||
type OutputFormat struct { | ||
Meta *MetadataInfo | ||
Stats map[structs.MessageType]typeStats |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder whether this field should be Stats []typeStats
. In the pretty output I see that you are creating a slice of these and then sorting. For the JSON output I wonder whether it would be better to see:
"Stats": [
{
"Name": "Register",
"Count": 20,
"Sum": 1537,
}
]
Having the MessageType (int
) as a map key in JSON doesn't seem that useful. Also having it in an array would more easily allow us to aggregate stats for namespaces without having the map keys conflict.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I didn't even notice that, good catch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previous functionality still available unchanged:
However the addition of JSON output with specified flag is available: