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

feat(diskstats): add metric for bare disk capacity #3068

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
20 changes: 20 additions & 0 deletions collector/diskstats_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ type diskstatsCollector struct {
filesystemInfoDesc typedFactorDesc
deviceMapperInfoDesc typedFactorDesc
ataDescs map[string]typedFactorDesc
diskSizeDesc typedFactorDesc
logger log.Logger
getUdevDeviceProperties func(uint32, uint32) (udevInfo, error)
}
Expand Down Expand Up @@ -257,6 +258,12 @@ func NewDiskstatsCollector(logger log.Logger) (Collector, error) {
), valueType: prometheus.GaugeValue,
},
},
diskSizeDesc: typedFactorDesc{
desc: prometheus.NewDesc(prometheus.BuildFQName(namespace, diskSubsystem, "size_bytes"),
"Size of the disk in bytes.",
diskLabelNames, nil,
), valueType: prometheus.GaugeValue,
},
logger: logger,
}

Expand Down Expand Up @@ -366,6 +373,19 @@ func (c *diskstatsCollector) Update(ch chan<- prometheus.Metric) error {
}
}
}

queueStats, err := c.fs.SysBlockDeviceQueueStats(dev)
if err != nil {
level.Error(c.logger).Log("msg", "Failed to get device queue stats", "err", err)
continue
}
size, err := c.fs.SysBlockDeviceSize(dev)
if err != nil {
level.Error(c.logger).Log("msg", "Failed to get device size", "err", err)
continue
}
sizeBytes := size * queueStats.LogicalBlockSize
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I wrote in prometheus/procfs#650 (comment), this will be incorrect for Advanced Format (AF / 4K sector) drives, since /sys/block/<dev>/size, which your proposed SysBlockDeviceSize method returns, is always reported in units of 512-byte sectors.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated that PR, as well as this one to use the new method

d8e74d6

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

happy with the change? wondering if we can try get these PRs moving

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but obviously this is contingent on the procfs PR being accepted first.

ch <- c.diskSizeDesc.mustNewConstMetric(float64(sizeBytes), dev)
}
return nil
}
Expand Down
4 changes: 4 additions & 0 deletions collector/diskstats_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,10 @@ node_disk_reads_merged_total{device="sdb"} 841
node_disk_reads_merged_total{device="sdc"} 141
node_disk_reads_merged_total{device="sr0"} 0
node_disk_reads_merged_total{device="vda"} 15386
# HELP node_disk_size_bytes Size of the disk in bytes.
# TYPE node_disk_size_bytes gauge
node_disk_size_bytes{device="nvme0n1"} 1.073741824e+10
node_disk_size_bytes{device="nvme1n1"} 9e+11
# HELP node_disk_write_time_seconds_total This is the total number of seconds spent by all writes.
# TYPE node_disk_write_time_seconds_total counter
node_disk_write_time_seconds_total{device="dm-0"} 1.1585578e+06
Expand Down