-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
win_perf_counter plugin does not work on 386 #2468
Comments
It's possible that lxn/win#12 was caused by the same issue |
I can confirm,
|
@kmonsoor No one is working on this as far as I know, would you be able to take a look? |
@danielnelson I could but I have no idea how |
It would be great to fix this issue, but maybe in the meantime the default configuration shipped with the 32-bit Windows build could be changed so that all the
This would at least prevent the bad out-of-the-box experience with this 32-bit build. |
@ronnix Your work around worked perfectly! Thanks :) |
@PierreF That confirms the cause.
|
I also looked for existing solutions. Those do exist. Elastic might have working performance counters. His github repository says many things are broken. |
This issue is causing some major problems for me. I would like to deploy telegraf to several pieces of mining equipment and to collect data from the windows PC's collecting data from the mining equipment's onboard systems. Telegraf won't even start. I would be willing to help out with testing and development in any way I can for a quick turn around. |
I looked over the project at https://github.com/alexbrainman/pc and looked at how he was padding the structs to provide 64bit alignment and used a similar tactic in pdh.go. It looks like it's worked, and I'm collecting data on a 32bit platform without crashing. I'm also new to go, and would need some help productizing this but for the time being the software appears to be collecting data reliably. |
thanks everyone for the report. We will see where we can fit this in. in the meantime, @srclosson if you'd like to submit a PR with the changes you made to get it working, we (and the community) will review. |
@russorat: Sorry, I could look for the PR process, but I'm so busy. Could you point me in the right direction? |
@srclosson We don't have directions for this specifically, but here is a brief overview that I hope will help you get started. This is from memory so it might not be 100% accurate:
|
Okay, I've done an initial checkin. Some work to separate 64 bit and 32 bit builds is required. Also includes 2 enhancements:
Comments are welcome |
@srclosson thanks for sharing. could you open up an official pull request for your fix? https://github.com/influxdata/telegraf/compare/master...srclosson:386fix?expand=1 |
Yes, I can. I'm actually waitng for approval from my end. Sorry guys, this is taking time. I don't expect to have permission probably until Monday or Tuesday. |
I have created a pull request. @russorat, would you mind reviewing? |
connect #4076 |
This bug should be fixed by #4189, it would be great if everyone could try it out using the nightly builds. |
Bug report
Telegraf i386 crash on Windows:
Full telegraf.conf:
System info:
Windows 10 amd64 and Windows 8 i386
Telegraf 1.2.1 and nightly
Steps to reproduce:
Expected behavior:
No crash
Actual behavior:
Crash :)
Additional info:
On the same machine, amd64 version works well.
I've dig a bit on the probable root cause and I think that issue is a difference size in structure size between Go and Windows API.
PDH_FMT_COUNTERVALUE_ITEM_DOUBLE has a size (according to unsafe.Sizeof, so according to Go) of 24 bytes on amd64 and 16 bytes on i386.
Both seems logical if structure aligns its fields on machine word size (8 bytes on amd64; 4 bytes on i386).
The expanded structure is
But I think Windows and C++ do align on 8 bytes boundary for both i386 and amd64. I don't have C++ compiler on Windows to confirm this hypothesis, but by adding few fmt.Printf that leads my to this idea:
Just before this for loop I've added:
This will dump the number of items and the binary data in the buffer.
Result just before crash (on i386 version of telegraf):
But since Go assume alignment is done on machine word size, it will interpret value as:
Proposal:
We should verify that Windows C++ do align structure on 8 bytes boundary (anyone with a C++ compiler on Windows ?, just checking sizeof(PDH_FMT_COUNTERVALUE_ITEM) should be good).
If confirmed, we should find how to tell Go to align on 8 bytes boundary for i386 and amd64
The text was updated successfully, but these errors were encountered: