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

Change Cpu.Get() on Windows to not use floating point artimetic #1128

Merged

Conversation

andrewkroh
Copy link
Member

Incorporates elastic/gosigar#19 to hopefully fix an issue with incorrect cpu.system_p values on Windows. See discuss

Best case: it fixes the issue
Worst case: instead of incorrect values cpu.system_p values, it reports a negative percentage due to the system time value being reported as negative by Windows.

@andrewkroh andrewkroh force-pushed the feature/topbeat-windows-cpu-get branch 2 times, most recently from 2226293 to 423349c Compare March 8, 2016 17:08
@andrewkroh andrewkroh added review and removed review labels Mar 8, 2016
@andrewkroh andrewkroh force-pushed the feature/topbeat-windows-cpu-get branch from 423349c to 71bd823 Compare March 8, 2016 18:13
@tsg
Copy link
Contributor

tsg commented Mar 8, 2016

Waiting for Travis on this one. It shouldn't be that long now..

@monicasarbu
Copy link
Contributor

Great! LGTM

monicasarbu added a commit that referenced this pull request Mar 8, 2016
Change Cpu.Get() on Windows to not use floating point artimetic
@monicasarbu monicasarbu merged commit 3ccb42d into elastic:master Mar 8, 2016
@andrewkroh andrewkroh deleted the feature/topbeat-windows-cpu-get branch March 8, 2016 23:23
@andrewkroh
Copy link
Member Author

It looks like this fixed the issue. We got some feedback from the user that was experiencing the problem on Windows.

andrewkroh added a commit to andrewkroh/beats that referenced this pull request Mar 30, 2016
Backport of  elastic#1128 to 1.2 - Change Cpu.Get() on Windows to not use floating point arithmetic
andrewkroh added a commit to andrewkroh/beats that referenced this pull request Apr 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants