* [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.