Report container FS metrics into prometheus /metrics #1642
Merged
Conversation
This file contains hidden or 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
|
DirectXMan12
approved these changes
Apr 20, 2017
PerDiskStats reported from cgroups were not being surfaced into prometheus. In order to properly correlate the metrics, we need to assign a device label to each metric (which is the FS or device path). Since blkio cgroup tracks devices, we create a synthetic device `/dev/NAME` for the metric. Assign a Device label to each PerDiskStat for the handlers up front, and then surface the PerDiskStat values into the prometheus metrics. Report two new metrics - total bytes read and total bytes written.
smarterclayton
force-pushed
the
fix_fs_metrics
branch
from
f7c360e to
4e25a79
Compare
|
@dashpole as discussed last week - we don't have a prometheus e2e test, and I wasn't sure what expectations are around new tests on something like this. |
|
The code changes look good, but I am still trying to wrap my head around: |
|
All the filesystem data is in form /dev/sda1. What we get from the machine info is "sda". To make the two roughly consistent, I turn "sda" into "/dev/sda" |
|
And at least on the few systems I looked at (RHEL / Mac / ubuntu) there is always a /dev/NAME for what mount returns |
|
Ok, so the name comes from reading the /sys/block/ directory. Would it make more sense to name these |
|
Interestingly enough - we depend on /dev/sda existing because we do this: https://github.com/google/cadvisor/blob/master/utils/sysfs/sysfs.go#L82 So a client that wanted to lookup the device by name can rely on this path (to then get major/minor). |
|
yes, so that is /sys/block/sda/dev |
|
My review is mostly based on the resulting metric output, and that lgtm. I only had a chance to glance over the implementation, but generally looks good as well. (coming from sig-instrumentation) |
|
Any chance to get a release with this PR included soon? |
|
yep, ill cut a cAdvisor release next week sometime before the kubernetes 1.7 |
|
Hmm, I applied this patch on 0.25.0 and with The actual difference in reported metrics between defaults and my case: Looks like no disk metrics were dropped. Is this expected? |
|
these are diskIO stats |
|
@dashpole, thanks, makes sense now. |
This was referenced Jun 6, 2017
|
If I'm not mistake, this should be in 0.26.1, correct? But I still don't have the metrics: |
|
Hi, I try this in ubuntu and centos, the container_fs_writes_total show that ubuntu is corrected and centos are not? why ? |
|
@ZhiqinYang Maybe differences in how the cgroups are mounted? |
|
Thank you for your reply, I do it with this command on centos7: |
|
Thank you for help, I take a mistake, The cgroup is set to error! |
|
Hello @ZhiqinYang , I'm not sure to understand how you managed to solve this issue with the cgroup ? |
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.
Fixes #1403
PerDiskStats reported from cgroups were not being surfaced into
prometheus. In order to properly correlate the metrics, we need to
assign a device label to each metric (which is the FS or device path).
Since blkio cgroup tracks devices, we create a synthetic device
/dev/NAMEfor the metric.Assign a Device label to each PerDiskStat for the handlers up front, and
then surface the PerDiskStat values into the prometheus metrics. Report
two new metrics - total bytes read and total bytes written.
Open questions:
_bytes_totalmetrics?