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

Fix: Filtering out ramdisks in diskio metricset #12829

Merged
merged 5 commits into from Jul 11, 2019

Conversation

narph
Copy link
Contributor

@narph narph commented Jul 9, 2019

Should fix #12814

The code filters volumes by disk type at the moment but ramdisk will show up as a fixed disk and will not be filtered out. The performance counter function (DeviceIoControl) will fail in this case.

  • PR will check for ramdisk volume label
  • PR will still generate docs for allowed disk types and log errors for any issues

@narph narph requested a review from a team as a code owner July 9, 2019 12:17
@narph narph self-assigned this Jul 9, 2019
@narph narph added Team:Integrations Label for the Integrations team :Windows Metricbeat Metricbeat labels Jul 9, 2019
Copy link
Contributor

@sayden sayden left a comment

Choose a reason for hiding this comment

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

Tested locally on Windows 10 Home

@@ -224,5 +228,36 @@ func isValidLogicalDrive(path string) bool {
if ret == 1 || ret == 5 || err != errorSuccess {
Copy link
Member

Choose a reason for hiding this comment

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

The code filters volumes by disk type at the moment but ramdisk will show up as a fixed disk and will not be filtered out.

So, GetDriveTypeW returns 3 (DRIVE_FIXED) for ram disks? In theory it can also return 6 (DRIVE_RAMDISK) 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jsoriano, I do see there is a separate type for it, 6 (https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getdrivetypew) based on the api call but it will return type 3, even when retrieving the drive type using the cmd line (ex. wmic logicaldisk get caption,description,drivetype,providername,volumename).
I can add the type 6 in the condition but it will not help in this specific case.

Copy link
Member

Choose a reason for hiding this comment

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

No need to add it if checking the volume label is more reliable. Thanks for the explanation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added it either way, along with unknown type

@narph narph requested a review from jsoriano July 11, 2019 13:56
Copy link
Contributor

@kaiyan-sheng kaiyan-sheng left a comment

Choose a reason for hiding this comment

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

LGTM 😬 Are you planning to add some unit tests?

@narph narph merged commit 2a76f5e into elastic:master Jul 11, 2019
@narph narph deleted the diskio-ramdisk branch July 11, 2019 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Metricbeat Metricbeat Team:Integrations Label for the Integrations team :Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Metricbeat] Diskio metricset: error message "disk io counters: Incorrect function." when ramdisk setup
5 participants