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

RFC: rename the zpool_influxdb command #11156

Closed
snajpa opened this issue Nov 5, 2020 · 2 comments
Closed

RFC: rename the zpool_influxdb command #11156

snajpa opened this issue Nov 5, 2020 · 2 comments
Labels
Type: Question Issue for discussion

Comments

@snajpa
Copy link
Contributor

snajpa commented Nov 5, 2020

Describe the problem you're observing

Firstly, I'm really sorry I'm coming with this now, that #10786 has been merged; but I've mised the (arguably tiny) window for reviews for that, so here we are...

The problem I'm having:

I've been using ZFS for years, some of the commands are so deeply embedded in my muscle memory and I would say I'm not alone in this... so it would be best if we could avoid changes upsetting this equilibrium :-D

For years, I've been typing this, when I wanted to get the status of zpool:

zpo<tab>status

... and it has always worked on all the platforms supported by OpenZFS.

But now, that the zpool_influxdb command has been merged and since it installs to globally accessible directory within common $PATH settings, it just irks me, but really badly.

So I would like to propose a change, which will fix this.

Looking at the PR, where the command got merged, I think the best course of action would be rewiring the logic directly under zpool command, my best bet would be a new zpool monitor subcommand, where I'd be for having a -i switch, which would produce the format needed for InfluxDB.

Then we could have -p for Prometheus, etc.

I was thinking whether to send this as a PR directly, but I think I would like to discuss this first, before I invest some real time into this 🙂

What do others think?

@richardelling as the author of the original PR, would you have any problem with me rewiring this under the zpool command directly? Or is there some caveat I'm overlooking, which prevented you to do that in the first place? Thx! ;)

@snajpa snajpa added Status: Triage Needed New issue which needs to be triaged Type: Defect Incorrect behavior (e.g. crash, hang) labels Nov 5, 2020
@behlendorf behlendorf added Type: Question Issue for discussion and removed Status: Triage Needed New issue which needs to be triaged Type: Defect Incorrect behavior (e.g. crash, hang) labels Nov 5, 2020
@behlendorf
Copy link
Contributor

That's a good point about breaking tab completion of the zpool command. There's years of muscle memory there and I can easily see that becoming a real annoyance. @snajpa if you're volunteering to do the work to convert it to a proper subcommand I personally wouldn't have any objection. Since this command isn't part of the 2.0 release now would be the time to do it.

@snajpa
Copy link
Contributor Author

snajpa commented Nov 5, 2020

So it ended up being easier to rewire it as monitor-influxdb subcommand, we can add monitor-prometheus and such later :)

snajpa added a commit to vpsfreecz/zfs that referenced this issue Nov 20, 2020
Move zpool_influxdb to libexec dir, as per following discussions:

openzfs#11156
openzfs#11160

Signed-off-by: Pavel Snajdr <snajpa@snajpa.net>
snajpa added a commit to vpsfreecz/zfs that referenced this issue Nov 20, 2020
Move zpool_influxdb to libexec dir, as per following discussions:

openzfs#11156
openzfs#11160

Signed-off-by: Pavel Snajdr <snajpa@snajpa.net>
snajpa added a commit to vpsfreecz/zfs that referenced this issue Nov 21, 2020
Move zpool_influxdb to libexec dir, as per following discussions:

openzfs#11156
openzfs#11160

Signed-off-by: Pavel Snajdr <snajpa@snajpa.net>
snajpa added a commit to vpsfreecz/zfs that referenced this issue Nov 27, 2020
Move zpool_influxdb to libexec dir, as per following discussions:

openzfs#11156
openzfs#11160

Signed-off-by: Pavel Snajdr <snajpa@snajpa.net>
snajpa added a commit to vpsfreecz/zfs that referenced this issue Nov 27, 2020
Move zpool_influxdb to libexec dir, as per following discussions:

openzfs#11156
openzfs#11160

Signed-off-by: Pavel Snajdr <snajpa@snajpa.net>
jsai20 pushed a commit to jsai20/zfs that referenced this issue Mar 30, 2021
Move the zpool_influxdb command to /usr/libexec/zfs,
and include the /usr/libexec/zfs path in the system search
directory when running the test suite.

Reviewed-by: Richard Elling <Richard.Elling@RichardElling.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Pavel Snajdr <snajpa@snajpa.net>
Closes openzfs#11156
Closes openzfs#11160
Closes openzfs#11224
sempervictus pushed a commit to sempervictus/zfs that referenced this issue May 31, 2021
Move the zpool_influxdb command to /usr/libexec/zfs,
and include the /usr/libexec/zfs path in the system search
directory when running the test suite.

Reviewed-by: Richard Elling <Richard.Elling@RichardElling.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Pavel Snajdr <snajpa@snajpa.net>
Closes openzfs#11156
Closes openzfs#11160
Closes openzfs#11224
tonyhutter pushed a commit to tonyhutter/zfs that referenced this issue Jun 11, 2021
Move the zpool_influxdb command to /usr/libexec/zfs,
and include the /usr/libexec/zfs path in the system search
directory when running the test suite.

Reviewed-by: Richard Elling <Richard.Elling@RichardElling.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Pavel Snajdr <snajpa@snajpa.net>
Closes openzfs#11156
Closes openzfs#11160
Closes openzfs#11224
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Question Issue for discussion
Projects
None yet
Development

No branches or pull requests

2 participants