* [PATCH] btrfs-progs: Support json format in device stats
@ 2020-09-17 7:29 Sidong Yang
2020-10-02 16:21 ` David Sterba
0 siblings, 1 reply; 3+ messages in thread
From: Sidong Yang @ 2020-09-17 7:29 UTC (permalink / raw)
To: linux-btrfs; +Cc: Sidong Yang
This patch supports the feature that printing device stats in json
format. So textcollector like prometheus can parse the output easier.
This patch implements option for json format and print in json when
the option enabled.
This patch implements enhancement for this issue.
https://github.com/kdave/btrfs-progs/issues/291
Signed-off-by: Sidong Yang <realwakka@gmail.com>
---
cmds/device.c | 46 ++++++++++++++++++++++++++++++++++------------
1 file changed, 34 insertions(+), 12 deletions(-)
diff --git a/cmds/device.c b/cmds/device.c
index d72881f8..3e4bf837 100644
--- a/cmds/device.c
+++ b/cmds/device.c
@@ -467,6 +467,7 @@ static const char * const cmd_device_stats_usage[] = {
"",
"-c|--check return non-zero if any stat counter is not zero",
"-z|--reset show current stats and reset values to zero",
+ "-j|--json show stats in json format",
NULL
};
@@ -482,6 +483,7 @@ static int cmd_device_stats(const struct cmd_struct *cmd, int argc, char **argv)
int check = 0;
__u64 flags = 0;
DIR *dirstream = NULL;
+ bool json = 0;
optind = 0;
while (1) {
@@ -489,6 +491,7 @@ static int cmd_device_stats(const struct cmd_struct *cmd, int argc, char **argv)
static const struct option long_options[] = {
{"check", no_argument, NULL, 'c'},
{"reset", no_argument, NULL, 'z'},
+ {"json", no_argument, NULL, 'j'},
{NULL, 0, NULL, 0}
};
@@ -503,6 +506,9 @@ static int cmd_device_stats(const struct cmd_struct *cmd, int argc, char **argv)
case 'z':
flags = BTRFS_DEV_STATS_RESET;
break;
+ case 'j':
+ json = 1;
+ break;
default:
usage_unknown_option(cmd, argv);
}
@@ -574,18 +580,34 @@ static int cmd_device_stats(const struct cmd_struct *cmd, int argc, char **argv)
snprintf(canonical_path, 32,
"devid:%llu", args.devid);
}
-
- for (j = 0; j < ARRAY_SIZE(dev_stats); j++) {
- /* We got fewer items than we know */
- if (args.nr_items < dev_stats[j].num + 1)
- continue;
- printf("[%s].%-16s %llu\n", canonical_path,
- dev_stats[j].name,
- (unsigned long long)
- args.values[dev_stats[j].num]);
- if ((check == 1)
- && (args.values[dev_stats[j].num] > 0))
- err |= 64;
+ if (json) {
+ printf("{\n\t\"device\" : \"%s\",\n", canonical_path);
+ for (j = 0; j < ARRAY_SIZE(dev_stats); j++) {
+ /* We got fewer items than we know */
+ if (args.nr_items < dev_stats[j].num + 1)
+ continue;
+ printf("\t\"%s\" : %llu,\n",
+ dev_stats[j].name,
+ (unsigned long long)
+ args.values[dev_stats[j].num]);
+ if ((check == 1)
+ && (args.values[dev_stats[j].num] > 0))
+ err |= 64;
+ }
+ printf("}\n");
+ } else {
+ for (j = 0; j < ARRAY_SIZE(dev_stats); j++) {
+ /* We got fewer items than we know */
+ if (args.nr_items < dev_stats[j].num + 1)
+ continue;
+ printf("[%s].%-16s %llu\n", canonical_path,
+ dev_stats[j].name,
+ (unsigned long long)
+ args.values[dev_stats[j].num]);
+ if ((check == 1)
+ && (args.values[dev_stats[j].num] > 0))
+ err |= 64;
+ }
}
free(canonical_path);
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] btrfs-progs: Support json format in device stats
2020-09-17 7:29 [PATCH] btrfs-progs: Support json format in device stats Sidong Yang
@ 2020-10-02 16:21 ` David Sterba
2020-10-02 17:53 ` Sidong Yang
0 siblings, 1 reply; 3+ messages in thread
From: David Sterba @ 2020-10-02 16:21 UTC (permalink / raw)
To: Sidong Yang; +Cc: linux-btrfs
On Thu, Sep 17, 2020 at 07:29:47AM +0000, Sidong Yang wrote:
> This patch supports the feature that printing device stats in json
> format. So textcollector like prometheus can parse the output easier.
> This patch implements option for json format and print in json when
> the option enabled.
Thanks, but this got a few things wrong.
> This patch implements enhancement for this issue.
> https://github.com/kdave/btrfs-progs/issues/291
>
> Signed-off-by: Sidong Yang <realwakka@gmail.com>
> ---
> cmds/device.c | 46 ++++++++++++++++++++++++++++++++++------------
> 1 file changed, 34 insertions(+), 12 deletions(-)
>
> diff --git a/cmds/device.c b/cmds/device.c
> index d72881f8..3e4bf837 100644
> --- a/cmds/device.c
> +++ b/cmds/device.c
> @@ -467,6 +467,7 @@ static const char * const cmd_device_stats_usage[] = {
> "",
> "-c|--check return non-zero if any stat counter is not zero",
> "-z|--reset show current stats and reset values to zero",
> + "-j|--json show stats in json format",
The output format is a global option like:
$ btrfs --format=json device stats ...
> NULL
> };
>
> @@ -482,6 +483,7 @@ static int cmd_device_stats(const struct cmd_struct *cmd, int argc, char **argv)
> int check = 0;
> __u64 flags = 0;
> DIR *dirstream = NULL;
> + bool json = 0;
>
> optind = 0;
> while (1) {
> @@ -489,6 +491,7 @@ static int cmd_device_stats(const struct cmd_struct *cmd, int argc, char **argv)
> static const struct option long_options[] = {
> {"check", no_argument, NULL, 'c'},
> {"reset", no_argument, NULL, 'z'},
> + {"json", no_argument, NULL, 'j'},
> {NULL, 0, NULL, 0}
> };
>
> @@ -503,6 +506,9 @@ static int cmd_device_stats(const struct cmd_struct *cmd, int argc, char **argv)
> case 'z':
> flags = BTRFS_DEV_STATS_RESET;
> break;
> + case 'j':
> + json = 1;
> + break;
> default:
> usage_unknown_option(cmd, argv);
> }
> @@ -574,18 +580,34 @@ static int cmd_device_stats(const struct cmd_struct *cmd, int argc, char **argv)
> snprintf(canonical_path, 32,
> "devid:%llu", args.devid);
> }
> -
> - for (j = 0; j < ARRAY_SIZE(dev_stats); j++) {
> - /* We got fewer items than we know */
> - if (args.nr_items < dev_stats[j].num + 1)
> - continue;
> - printf("[%s].%-16s %llu\n", canonical_path,
> - dev_stats[j].name,
> - (unsigned long long)
> - args.values[dev_stats[j].num]);
> - if ((check == 1)
> - && (args.values[dev_stats[j].num] > 0))
> - err |= 64;
> + if (json) {
Two separate implementations that dump the data, so both would have to
be kept in sync, which is error prone.
I've implemented data dumper that automatically picks the right format
based on the --format option from a specification.
Last time the json output was discussed, there was not a consensus how
exactly it should look like. There's a preview for subvolume command
(https://github.com/kdave/btrfs-progs/tree/preview-json), you can see
how it's supposed to be used.
Given the output is not finazlied, the json output is still a bit of
research how other tools eg. from util-linux do it and also to have some
sample use cases, eg. examples with 'jq' extracting the data. The hard
part here is to specify a reasonable format that won't have to be
changed in the future.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] btrfs-progs: Support json format in device stats
2020-10-02 16:21 ` David Sterba
@ 2020-10-02 17:53 ` Sidong Yang
0 siblings, 0 replies; 3+ messages in thread
From: Sidong Yang @ 2020-10-02 17:53 UTC (permalink / raw)
To: dsterba, linux-btrfs
On Fri, Oct 02, 2020 at 06:21:10PM +0200, David Sterba wrote:
> On Thu, Sep 17, 2020 at 07:29:47AM +0000, Sidong Yang wrote:
> > This patch supports the feature that printing device stats in json
> > format. So textcollector like prometheus can parse the output easier.
> > This patch implements option for json format and print in json when
> > the option enabled.
>
> Thanks, but this got a few things wrong.
>
> > This patch implements enhancement for this issue.
> > https://github.com/kdave/btrfs-progs/issues/291
> >
> > Signed-off-by: Sidong Yang <realwakka@gmail.com>
> > ---
> > cmds/device.c | 46 ++++++++++++++++++++++++++++++++++------------
> > 1 file changed, 34 insertions(+), 12 deletions(-)
> >
> > diff --git a/cmds/device.c b/cmds/device.c
> > index d72881f8..3e4bf837 100644
> > --- a/cmds/device.c
> > +++ b/cmds/device.c
> > @@ -467,6 +467,7 @@ static const char * const cmd_device_stats_usage[] = {
> > "",
> > "-c|--check return non-zero if any stat counter is not zero",
> > "-z|--reset show current stats and reset values to zero",
> > + "-j|--json show stats in json format",
>
> The output format is a global option like:
>
> $ btrfs --format=json device stats ...
>
> > NULL
> > };
> >
> > @@ -482,6 +483,7 @@ static int cmd_device_stats(const struct cmd_struct *cmd, int argc, char **argv)
> > int check = 0;
> > __u64 flags = 0;
> > DIR *dirstream = NULL;
> > + bool json = 0;
> >
> > optind = 0;
> > while (1) {
> > @@ -489,6 +491,7 @@ static int cmd_device_stats(const struct cmd_struct *cmd, int argc, char **argv)
> > static const struct option long_options[] = {
> > {"check", no_argument, NULL, 'c'},
> > {"reset", no_argument, NULL, 'z'},
> > + {"json", no_argument, NULL, 'j'},
> > {NULL, 0, NULL, 0}
> > };
> >
> > @@ -503,6 +506,9 @@ static int cmd_device_stats(const struct cmd_struct *cmd, int argc, char **argv)
> > case 'z':
> > flags = BTRFS_DEV_STATS_RESET;
> > break;
> > + case 'j':
> > + json = 1;
> > + break;
> > default:
> > usage_unknown_option(cmd, argv);
> > }
> > @@ -574,18 +580,34 @@ static int cmd_device_stats(const struct cmd_struct *cmd, int argc, char **argv)
> > snprintf(canonical_path, 32,
> > "devid:%llu", args.devid);
> > }
> > -
> > - for (j = 0; j < ARRAY_SIZE(dev_stats); j++) {
> > - /* We got fewer items than we know */
> > - if (args.nr_items < dev_stats[j].num + 1)
> > - continue;
> > - printf("[%s].%-16s %llu\n", canonical_path,
> > - dev_stats[j].name,
> > - (unsigned long long)
> > - args.values[dev_stats[j].num]);
> > - if ((check == 1)
> > - && (args.values[dev_stats[j].num] > 0))
> > - err |= 64;
> > + if (json) {
>
Hi! David,
Thanks for review!
> Two separate implementations that dump the data, so both would have to
> be kept in sync, which is error prone.
>
> I've implemented data dumper that automatically picks the right format
> based on the --format option from a specification.
>
> Last time the json output was discussed, there was not a consensus how
> exactly it should look like. There's a preview for subvolume command
> (https://github.com/kdave/btrfs-progs/tree/preview-json), you can see
> how it's supposed to be used.
Yeah, There is a great function I didn't know. I should reimplement a new patch
with this format infrastructure. It's okay to output json like this?
{
"__header": {
"version": "1"
},
"device-stats": [{
"name": "/dev/sdb",
"write_io_errs": "0",
"read_io_errs": "0",
"flush_io_errs": "0",
"corruption_io_errs": "0",
"generation_io_errs": "0"
}]
}
>
> Given the output is not finazlied, the json output is still a bit of
> research how other tools eg. from util-linux do it and also to have some
> sample use cases, eg. examples with 'jq' extracting the data. The hard
> part here is to specify a reasonable format that won't have to be
> changed in the future.
You mean that there is no consensus with this problem?
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-10-02 17:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-17 7:29 [PATCH] btrfs-progs: Support json format in device stats Sidong Yang
2020-10-02 16:21 ` David Sterba
2020-10-02 17:53 ` Sidong Yang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.