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

Added plugin to read Windows performance counters #575

Closed

Conversation

TheFlyingCorpse
Copy link
Contributor

Plugin that reads performance counters from Windows, a lot of examples included to get an idea how to use this with IIS and AD to start with.
The plugin is flexible so you can have a master configuration to push out to all, it will only submit from each host what is matching to your configuration.

"fmt"
"strings"
"syscall"
//"time"
Copy link
Contributor

Choose a reason for hiding this comment

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

remove commented import

@sparrc
Copy link
Contributor

sparrc commented Jan 25, 2016

Thanks a bunch @TheFlyingCorpse, this is great for windows support 👍!

A few things to address:

  1. Write a README please!
  2. Can you unit test some or all of this?
  3. Put a dummy file called win_perfcounters_notwindows.go with the // +build !windows flag in the directory, otherwise unix builds will complain that there are no buildable source files in the directory.
  4. Are all of those examples necessary for the config file? Could you maybe cut those down by at least half and then put the rest in a README file?
  5. Try to reduce line-length to 80 characters throughout
  6. If you happen to have any useful "recipes" of combinations of perf counters that you commonly use, that would be great to have in the README as well. Such as "cpu monitoring" and then give an example of the objects users might want to monitor the cpu.

@TheFlyingCorpse TheFlyingCorpse force-pushed the win_perfcountersv2 branch 4 times, most recently from cf3a7d1 to 36e74db Compare January 25, 2016 23:53
@TheFlyingCorpse
Copy link
Contributor Author

I'm still working on the unit tests, will try to have something tomorrow!

@sparrc
Copy link
Contributor

sparrc commented Jan 26, 2016

@TheFlyingCorpse Is this ready to merge?

@TheFlyingCorpse
Copy link
Contributor Author

@sparrc - Yes, I have formatted the code to the best of my ability and added unit tests for both the configuration parsing and doing test queries on Windows.

@sparrc
Copy link
Contributor

sparrc commented Jan 28, 2016

@TheFlyingCorpse can you please squash your commits and rebase off master?

@TheFlyingCorpse TheFlyingCorpse force-pushed the win_perfcountersv2 branch 3 times, most recently from cab3691 to 6bad082 Compare January 28, 2016 22:05
@TheFlyingCorpse
Copy link
Contributor Author

@sparrc, done!

@sparrc sparrc closed this in f088dd7 Jan 28, 2016
@sparrc
Copy link
Contributor

sparrc commented Jan 28, 2016

thanks @TheFlyingCorpse! this is in master now

@sparrc
Copy link
Contributor

sparrc commented Jan 28, 2016

note: I changed the name to win_perf_counters for consistency

@TheFlyingCorpse TheFlyingCorpse deleted the win_perfcountersv2 branch January 29, 2016 14:54
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