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

cs collector wmi_cs_logical_processors metric reports false data #786

Closed
1sttec opened this issue May 19, 2021 · 10 comments
Closed

cs collector wmi_cs_logical_processors metric reports false data #786

1sttec opened this issue May 19, 2021 · 10 comments
Labels

Comments

@1sttec
Copy link

1sttec commented May 19, 2021

On two test desktops the cs collector wmi_cs_logical_processors metric returns 0. On a test server it returns 65535. Please advise.

version: 0.16.0

@1sttec
Copy link
Author

1sttec commented May 20, 2021

systeminfoapi is using GetSystemInfo (lpSysteminfo) but doc here https://docs.microsoft.com/en-us/windows/win32/api/sysinfoapi/ns-sysinfoapi-system_info says to use GetLogicalProcessorInformation for logical core count instead of GetSystemInfo.dwNumberOfProcessors

...
dwNumberOfProcessors
The number of logical processors in the current group. To retrieve this value, use the GetLogicalProcessorInformation function.
...

@carlpett
Copy link
Collaborator

@benridley @retryW any input?

@carlpett
Copy link
Collaborator

@1sttec Thanks for that extra bit of info! What versions of Windows are you testing on?

@1sttec
Copy link
Author

1sttec commented May 20, 2021

Server 2019 = 0
Server 2016 = 0
Server 2012 R2 = 65535
Win 7 & 10 = 0

@benridley
Copy link
Contributor

Thanks @carlpett and @1sttec!

Apologies for this, looks like one of the struct fields was incorrectly sized, so when accessing them Go would be reading the wrong memory.

I've created a PR here to fix the issue: #788

@benridley
Copy link
Contributor

@1sttec the fix has been merged to master now. I've tested this on a 2016 machine and it worked ok, but I'm curious how it'll go on your machines since they were getting different values. If you get a chance to test please do so and let us know. Cheers!

@1sttec
Copy link
Author

1sttec commented May 21, 2021

It appears windows_cs_logical_processor is now reporting sockets * cores but not the true logical processors # which would include those available via hyper-threading.

@benridley
Copy link
Contributor

benridley commented May 21, 2021

Thanks @1sttec, seems like this might be a consequence of the function we use. I found this which suggests similar problems with our underlying info.

I'll try converting the metric to use the function specified in that article which will hopefully report the correct values.

@benridley
Copy link
Contributor

Hi @1sttec, can you please try building my branch here and seeing if it reports the correct CPU count for you?

It's looking good on my windows machine, but so was the old way - So I just want to make sure. Thank you !

Copy link

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Nov 25, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants