Add bcache collector #597
Merged
Add bcache collector #597
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This changeset needs an update because some of its fixture filenames break Windows the same way they did for procfs. |
grobie
approved these changes
Jun 18, 2017
There was a problem hiding this comment.
Sorry for the late review. This looks good.
How many metrics can be created with the collector in a real production system? Specifically, how many label values for the UUID label are realistic? Below 10 per node?
I don't really care how we solve the filename issue for windows. I'm not entirely happy with the rename approach, but all approaches have drawbacks. I think my favorite would still be something like a plaintext tarball / yaml file. Use whatever is easiest, but it must not break tests when someone changes the fixture files.
|
We were talking about dropping windows support for the node_exporter anyway. Now that the WMI exporter is a much better replacement. |
|
That doesn't matter, the git repository still needs to be fetchable on
Windows systems, otherwise it would break `go get -u`.
…On Sun, Jun 18, 2017 at 6:07 PM Ben Kochie ***@***.***> wrote:
We were talking about dropping windows support for the node_exporter
anyway. Now that the WMI exporter is a much better replacement.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#597 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAANaG0EC9XJTPKDhp5B4f2bz6FlnUhfks5sFUs9gaJpZM4N0DJp>
.
|
Several per node is realistic. I have never seen 10 or more. In my limited experience having a few caching SSDs and one bcache device (i.e., one UUID) per SSD is typical. YMMV. |
|
Needs rebasing. |
This collector gathers metrics related to the Linux block cache (bcache) from sysfs.
Rename: - metric -> bcacheMetric - periodStatsToMetrics -> bcachePeriodStatsToMetric
This changeset moves the collector/fixtures/sys directory into collector/fixtures/sys.tar.gz and tweaks the Makefile to unpack the tarball before tests are run. The reason for this change is that Windows does not allow colons in a path (colons are present in some of the bcache fixture files), nor can it (out of the box) deal with pathnames longer than 260 characters (which we would be increasingly likely to hit if we tried to replace colons with longer codes that are guaranteed not the turn up in regular file names).
This changeset adds ttar, a plain text replacement for tar, and uses it for the sysfs fixture archive. The syntax is loosely based on tar(1). Using a plain text archive makes it possible to review changes without downloading and extracting the archive. Also, when working on the repo, git diff and git log become useful again, allowing a committer to verify and track changes over time. The code is written in bash, because bash is available out of the box on all major flavors of Linux and on macOS. The feature set used is restricted to bash version 3.2 because that is what Apple is still shipping. The programm also works on Windows if bash is installed. Obviously, it does not solve the Windows limitations (path length limited to 260 characters, no symbolic links) that prompted the move to an archive format in the first place.
|
Great, thanks! |
|
Can someone publish a new release including bcache? Thanks |
oblitorum
pushed a commit
to shatteredsilicon/node_exporter
that referenced
this pull request
Apr 9, 2024
* Add bcache collector for Linux This collector gathers metrics related to the Linux block cache (bcache) from sysfs. * Removed commented out code * Use project comment style * Add _sectors to metric name to indicate unit * Really use project comment style * Rename bcache.go to bcache_linux.go * Keep collector namespace clean Rename: - metric -> bcacheMetric - periodStatsToMetrics -> bcachePeriodStatsToMetric * Shorten slice initialization * Change label names to backing_device, cache_device * Remove five minute metrics (keep only total) * Include units in additional metric names * Enable bcache collector by default * Provide metrics in seconds, not nanoseconds * remove metrics with label "all" * Add fixtures, update end-to-end for bcache collector * Move fixtures/sys into tar.gz This changeset moves the collector/fixtures/sys directory into collector/fixtures/sys.tar.gz and tweaks the Makefile to unpack the tarball before tests are run. The reason for this change is that Windows does not allow colons in a path (colons are present in some of the bcache fixture files), nor can it (out of the box) deal with pathnames longer than 260 characters (which we would be increasingly likely to hit if we tried to replace colons with longer codes that are guaranteed not the turn up in regular file names). * Add ttar: plain text archive, replacement for tar This changeset adds ttar, a plain text replacement for tar, and uses it for the sysfs fixture archive. The syntax is loosely based on tar(1). Using a plain text archive makes it possible to review changes without downloading and extracting the archive. Also, when working on the repo, git diff and git log become useful again, allowing a committer to verify and track changes over time. The code is written in bash, because bash is available out of the box on all major flavors of Linux and on macOS. The feature set used is restricted to bash version 3.2 because that is what Apple is still shipping. The programm also works on Windows if bash is installed. Obviously, it does not solve the Windows limitations (path length limited to 260 characters, no symbolic links) that prompted the move to an archive format in the first place.
tamcore
pushed a commit
to gitgrave/node_exporter
that referenced
this pull request
Oct 22, 2024
Signed-off-by: Thomas Barrett <[email protected]> Co-authored-by: Thomas Barrett <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rebased version of PR #594.