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

[Windows] Update pdh library to support generic pdh based checks #3581

Merged
merged 1 commit into from Nov 21, 2017

Conversation

derekwbrown
Copy link
Contributor

Updates the PDH library. Provide a generic PDHBaseCheck to build PDH based system checks.

Copy link
Contributor

@gmmeyer gmmeyer left a comment

Choose a reason for hiding this comment

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

I think the import stuff needs to be fixed. Otherwise it looks great!

try:
from checks.libs.win.winpdh import WinPDHCounter
except ImportError:
def WinPDHCounter(*args, **kwargs):
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be a class instead of a function? You'd probably want to have the check error out if it can't import it, but not on import. That will be ugly

@@ -8,24 +8,24 @@ class WinPDHCounter(object):
# store the dictionary of pdh counter names
pdh_counter_dict = {}

def __init__(self, class_name, instance_name, log):
def __init__(self, class_name, counter_name, log, instance_name = None):
Copy link
Contributor

Choose a reason for hiding this comment

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

Will making these changes affect the pdh check in integrations core?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No; the pdh check always will supply no instance name. The problem is confusion between what PDH calls countersets, counters, and instances, and what we generally refer to as instances. The generic PDH counter does all instances of the counters, so it's instance_name param is always none.

Copy link
Contributor

@gmmeyer gmmeyer left a comment

Choose a reason for hiding this comment

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

👍

@derekwbrown derekwbrown merged commit c7a2917 into master Nov 21, 2017
@derekwbrown derekwbrown deleted the db/pdh_update branch November 21, 2017 17:48
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.

None yet

3 participants