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

Use Perflib for logical_disk exporter #400

Merged
merged 3 commits into from
Oct 7, 2019
Merged

Use Perflib for logical_disk exporter #400

merged 3 commits into from
Oct 7, 2019

Conversation

breed808
Copy link
Contributor

PR modifies the logical_disk exporter to use Perflib in place of WMI. The PR also fixes value format for several latency metrics (ticks -> seconds).

However, perflib is reporting % Free Space as the number of free megabytes (identical to Free Megabytes), breaking the TotalSpace metric.
I've opened an issue in the perflib_exporter repository regarding this: leoluk/perflib_exporter/issues/23

@breed808 breed808 marked this pull request as ready for review September 23, 2019 08:52
collector/logical_disk.go Show resolved Hide resolved
@@ -171,26 +170,25 @@ func (c *LogicalDiskCollector) Collect(ctx *ScrapeContext, ch chan<- prometheus.
// - https://msdn.microsoft.com/en-us/library/ms803973.aspx - LogicalDisk object reference
type Win32_PerfRawData_PerfDisk_LogicalDisk struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Bit of a nitpick, but now that we don't use WMI here, we don't need to have the type name match the WMI class name. We also don't need to export the type, so perhaps change to just logicalDisk?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, better to clean it up now then to leave others wondering at the type name in the future.
Are you happy to leave the documentation link for the type?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the doc link still looked relevant, but I didn't look super closely, is there something off with it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not at all. My preference would be to leave it in, as it is still largely relevant for the perflib metrics. Just checking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Type has been renamed to logicalDisk. I've rebased the commits and force pushed them to my branch.

@carlpett
Copy link
Collaborator

carlpett commented Oct 7, 2019

LGTM, thanks!

@carlpett carlpett merged commit 1c605ad into prometheus-community:master Oct 7, 2019
@breed808 breed808 deleted the perf branch October 7, 2019 22:53
anubhavg-icpl pushed a commit to anubhavg-icpl/windows_exporter that referenced this pull request Sep 22, 2024
Use Perflib for logical_disk exporter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants