-
Couldn't load subscription status.
- Fork 24
prometheus: add btrfs_error_exporter example #26
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
prometheus: add btrfs_error_exporter example #26
Conversation
This extends the btrfs stats by Prometheus' native node_exporter with device errors without forking 'btrfs stats' multiple times.
|
Btw I noticed that compared to |
|
Hi! I try to keep naming as close/consistent/whatever as possible to actual kernel code. I did not notice yet that the progs code has labels that are inconsistent with the naming of the enum in kernel code. The progs code is from 2012, and the commit has no explanation why the _io was inserted in the labels. The DevStats object is an "Object representation of struct btrfs_ioctl_get_dev_stats" which uses btrfs_dev_stat_values with the names that we also see in progs code. So, since it's e.g. BTRFS_DEV_STAT_WRITE_ERRS and not BTRFS_DEV_STAT_WRITE_IO_ERRS, it ended up without _io in here. |
Interesting! In the case of Prometheus this means a previous series would be mismatched and no longer continue with the new data. Essentially the old timeline would end and a new one would begin, since a single series is uniquely identified by all tuple identifiers (called labels). However other than this lack of continuity it's no a big deal; the labels are not standardized and mismatches frequently happen between different exporters. Since Prometheus metrics are mostly short-lived and expire quickly it really shouldn't cause any problems; after all you'd only alert on a series if the value changed to >0 anyway. |
|
Yeah. Well, brrr, the exporter could also include a hard coded map of the counters and have the field names printed in the same way as they are in the progs stats output. However, using the counters convenience thing (which was added for the nagios plugin) of course automatically picks up new counters if they're implemented... Or have some ugly fixup_flapsie lookup table which filters/replaces the known different names and leaves the rest unchanged. So, pros and cons. Since there's already a current script at the location you linked, don't you think that we should prepare and test a PR against prometheus-community/node-exporter-textfile-collector-scripts indeed? In that case it should probably behave as similar as possible. Or, adopt it and provide an alternative here, and then make more changes and improvements, without caring about breaking all the output format compatibility with the old one. I mean, things like not indexing statistics under a device name, but under fsid uuid and deduplicate them, and not have them jumping around to another device name if you do btrfs replace etc etc... (The get_btrfs_mount_points function in the old one does not return mount points, it returns device names O_o) I think it's a great idea to adopt it and provide a better alternative in here. Also, the old script has an allocation reporting part, but as you know reporting on usage is a lot more hairy than that. It totally depends on what the use case is of course, but think it is highly likely that the use case is to know if you run out of space or not. When having a better look into doing that for the nagios plugin, I ended up writing the whole fs_usage module to end up with more extensive reporting, also taking unallocatable ('soft' and 'hard') raw disk space into account. So, I'd rather use that and actually have the whole thing be very much like the nagios plugin just with different output format. How often are the prometheus exporter things called? Because, creating an FsUsage object means running a chunk allocation simulation to try fill up all disks, in order to find out about unallocatable space. This can be a pretty heavy operation on a multi-TiB filesystem. For nagios and munin, where it happens only once every 5 minutes or so, it's not a big deal, but it can't be done every 10 seconds. So much thinking out loud. Thanks for starting this discussion. |
Yeah..no? This PR was supposed to be a) an example for how elegant python-btrfs is and b) do the one thing that's missing from the existing native node_exporter and its btrfs collector, namely collecting the device errors. Ideally this script shouldn't even exist and be part of node_exporter, but there are problems with that (the ioctl tree search permissions vs. the fact that node_exporter mandates unprivileged permissions at runtime vs. my
No..ish? The old script was contributed as a stopgap measure when the native integration in node_exporter didn't exist yet. It's also been moved to the "community" farm, where it can play with the other outdated and broken scripts. Also the allocation stats are no longer needed (see above), which is why I rewrote the error part only.
What I did so far was just my usual curiosity to see if I (not knowing much Python) can rewrite the thing to not fork/grep/regexp and whatnot. (I learned how to yield..yay.) That worked beautifully - the new code is almost self-explanatory! - and the metrics so far are as close to the original version as possible. You're completely correct that we could change that (like device -> uuid), but IMHO that would create more confusion than it solves. Prometheus is used for alerting, so if a counter for sdX goes up, you get an alert, replace it (or something) and then no longer care. Whether we skip the mountpoint and/or include the fsid is something I'll have to think about. Excluding the device seems weird since you'd then have to find it out manually..?
But I don't want to expand it - the point was really only to collect errors and nothing else, since all the other statistics are provided by node_exporter already. Now whether those values are correct is a very different discussion (the Go code just scrapes sysfs, nothing fancy and it's certainly not aware of edge cases). But that's just .. ¯\(ツ)/¯ for now.
Yeah I don't think we should do any of that. :-) Soo..I can haz no new features plz? 😸 |
|
Just to give you an example for the output of the Go-based node_exporter here's a subset (without generic filesystem data) of what my Prometheus stores, with the output of the btrfs_error_exporter mixed in. You can see that it's a happy mix of uuids, devices and whatnot. Positively noteworthy (but cut for brevity here): python-btrfs/btrfs_error_exporter correctly finds only real filesystems, whereas the sysfs-scraping node_exporter cannot distinguish between real mounts and bind mounts, generating a lot more redundant output than necessary. |
Aha, I see. I did not know/realize. This totally makes sense.
Aha. I see. I did not know that it requires non-root. And yes, most of the stuff now needs root because of searches and things. I haven't used prometheus yet, it's on my wishlist, part of my team at work does. So, you mean that it's possible to run this thing as root, but the official exporter would not accept such a change, even if you would program it in go and it would still require root?
Aha. I did not know that.
Hooray. Yes, it is very easy to read, and the yield is a nice trick. The python-btrfs lib also uses generators and yield a lot, but mostly because it makes it possible to process an endless stream of data with minimal buffering. In this case, the yield simply saves you making a list, an extra level of indentation with a loop and then returning the list, which makes it a bit easier to read. The calling code can just iterate over it and remains the same. And yes, device names indeed, so the error stats link to the individual devices. And a device is always part of exactly 1 filesystem, so that's fine.
Yes, I did not know.
Depends on what your goal is. :-) Learn more C++ or just get it over with, haha.
Ok. And if it's limited to the error counters all of that does not matter. If we would add more functionality later, then it probably makes sense to have multiple little export scripts that each do one thing and are good at it, instead of cramming all of it in one.
Sure. This all makes sense. I'll add some code review. Nothing really spectacular. And now I have to setup some prometheus to test it also. |
Yup, correct. The (now closed) issue for native btrfs usage stats is here (feat. usual suspects ;). The problem was getting the list of devices (requiring root when using btrfs-progs but not via ioctl) and the obvious workaround would be what you once called the "yolo method" aka trying devids in increasing order until you get an error. I think loooking into
I'll add an author notice and some licensing header like in the other examples. |
|
I don't mean to be pushy, but if you wait much longer the Go-based exporter is going to steal your thunder, and we can't have that.. ;) |
|
The latest version of the btrfs collector in node_exporter no longer requires elevated privileges and has working error stats, so this is no longer necessary. |
This is an extension for Prometheus' native
node_exporterwith a "text collector" (to be called e.g. from cron) thatcollects btrfs device errors. Unlike its original version (https://git.io/JkTIi) this is written with python-btrfs and therefore
does not shell out to
btrfs device statsor rely on its output. The original script was written before the nativenode_exporterhad btrfs stats, which it has now - but device errors are still missing, so here we are.I might need to add some licensing and a copyright but wasn't sure if you care..let me know what you think. =)