Repository navigation
Conversation
node_disk_read_sectors_total and node_disk_written_sectors_total were computed as NumRead/BlockSize and NumWrite/BlockSize, i.e. operation counts divided by a size in bytes, which is not a sector count in any unit. The collector already exports node_disk_read_bytes_total and node_disk_written_bytes_total from the byte counters, so remove the two families, as the Linux collector did in prometheus#787. Split the metric emission out of Update so it can be exercised with synthetic iostat.DriveStats, and add the first test for the Darwin diskstats collector. Fixes prometheus#3850 Signed-off-by: Lucas Barros <lucasbarros.kam@gmail.com>
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
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 #3850
On Darwin,
node_disk_read_sectors_totalandnode_disk_written_sectors_totalwere computed asNumRead / BlockSizeandNumWrite / BlockSize.NumRead/NumWriteare operation counts andBlockSizeis in bytes, so the result is not a sector count in any unit (3 reads on a 4096-byte block size gives 0.00073).The collector already exposes
node_disk_read_bytes_totalandnode_disk_written_bytes_totalfrom the IOKit byte counters, so this removes the two sector families, the same way the Linux collector moved to bytes in #787.I also split the metric emission out of
Update(updateFromStats) so it can be driven with syntheticiostat.DriveStats, and addeddiskstats_darwin_test.go, which is the first test for this collector. It compares the full output, so the sector families coming back would fail it.If you would rather keep the metric names, the alternative is exporting
BytesRead / 512andBytesWritten / 512under the existing names. Happy to switch to that.Suggested changelog entry:
[CHANGE] diskstats: Remove node_disk_read_sectors_total and node_disk_written_sectors_total on Darwin, use the bytes counters insteadTesting, on macOS 26 (arm64):
go test ./collector/ -count=1golangci-lint run ./collector/...: no findings in the changed files--collector.disable-defaults --collector.diskstats: the bytes, operations, time, error and retry series are unchanged and no*_sectors_totalseries is exportedI used an AI assistant while working on this; I reviewed the change and tested it on my machine.