From c8caba3c784ec2ba305316cbd1a83dc20bd34c01 Mon Sep 17 00:00:00 2001 From: Julia Simon <611228+hithwen@users.noreply.github.com> Date: Mon, 2 Mar 2020 13:27:55 +0100 Subject: [PATCH 01/10] Adapt PDHBaseCheck to new agent Signature --- .../base/checks/win/winpdh_base.py | 147 +++++++++--------- 1 file changed, 71 insertions(+), 76 deletions(-) diff --git a/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py b/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py index c6d7e6e12e60e..d166b2e55f838 100644 --- a/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py +++ b/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py @@ -27,82 +27,79 @@ class PDHBaseCheck(AgentCheck): Windows only. """ - - def __init__(self, name, init_config, agentConfig, instances, counter_list): - AgentCheck.__init__(self, name, init_config, agentConfig, instances) + def __init__(self, *args, **kwargs): # To support optional agentConfig + AgentCheck.__init__(self, *args, **kwargs) self._missing_counters = {} self._metrics = {} self._tags = {} - key = None + self.refresh_counters = is_affirmative(self.instance.get('refresh_counters', True)) + self.key = hash_mutable(self.instance) + self._metrics = [] try: - for instance in instances: - key = hash_mutable(instance) - - cfg_tags = instance.get('tags') - if cfg_tags is not None: - if not isinstance(cfg_tags, list): - self.log.error("Tags must be configured as a list") - raise ValueError("Tags must be type list, not %s" % str(type(cfg_tags))) - self._tags[key] = list(cfg_tags) - - remote_machine = None - host = instance.get('host') - self._metrics[key] = [] - if host is not None and host != ".": - try: - remote_machine = host - - username = instance.get('username') - password = instance.get('password') - nr = self._get_netresource(remote_machine) - win32wnet.WNetAddConnection2(nr, password, username, 0) - - except Exception as e: - self.log.error("Failed to make remote connection %s", str(e)) - return - - # counter_data_types allows the precision with which counters are queried - # to be configured on a per-metric basis. In the metric instance, precision - # should be specified as - # counter_data_types: - # - iis.httpd_request_method.get,int - # - iis.net.bytes_rcvd,float - # - # the above would query the counter associated with iis.httpd_request_method.get - # as an integer (LONG) and iis.net.bytes_rcvd as a double - datatypes = {} - precisions = instance.get('counter_data_types') - if precisions is not None: - if not isinstance(precisions, list): - self.log.warning("incorrect type for counter_data_type %s", str(precisions)) - else: - for p in precisions: - k, v = p.split(",") - v = v.lower().strip() - if v in int_types: - self.log.info("Setting datatype for %s to integer", k) - datatypes[k] = DATA_TYPE_INT - elif v in double_types: - self.log.info("Setting datatype for %s to double", k) - datatypes[k] = DATA_TYPE_DOUBLE - else: - self.log.warning("Unknown data type %s", str(v)) - - self._make_counters(key, (counter_list, (datatypes, remote_machine, False, 'entry'))) - - # get any additional metrics in the instance - addl_metrics = instance.get('additional_metrics') - if addl_metrics is not None: - self._make_counters( - key, (addl_metrics, (datatypes, remote_machine, True, 'additional metric entry')) - ) + cfg_tags = self.instance.get('tags') + if cfg_tags is not None: + if not isinstance(cfg_tags, list): + self.log.error("Tags must be configured as a list") + raise ValueError("Tags must be type list, not %s" % str(type(cfg_tags))) + self._tags = list(cfg_tags) + + remote_machine = None + host = self.instance.get('host') + if host is not None and host != ".": + try: + remote_machine = host + + username = self.instance.get('username') + password = self.instance.get('password') + nr = self._get_netresource(remote_machine) + win32wnet.WNetAddConnection2(nr, password, username, 0) + + except Exception as e: + self.log.error("Failed to make remote connection %s", str(e)) + return + + # counter_data_types allows the precision with which counters are queried + # to be configured on a per-metric basis. In the metric instance, precision + # should be specified as + # counter_data_types: + # - iis.httpd_request_method.get,int + # - iis.net.bytes_rcvd,float + # + # the above would query the counter associated with iis.httpd_request_method.get + # as an integer (LONG) and iis.net.bytes_rcvd as a double + datatypes = {} + precisions = self.instance.get('counter_data_types') + if precisions is not None: + if not isinstance(precisions, list): + self.log.warning("incorrect type for counter_data_type %s", str(precisions)) + else: + for p in precisions: + k, v = p.split(",") + v = v.lower().strip() + if v in int_types: + self.log.info("Setting datatype for %s to integer", k) + datatypes[k] = DATA_TYPE_INT + elif v in double_types: + self.log.info("Setting datatype for %s to double", k) + datatypes[k] = DATA_TYPE_DOUBLE + else: + self.log.warning("Unknown data type %s", str(v)) + + self._make_counters((counter_list, (datatypes, remote_machine, False, 'entry'))) + + # get any additional metrics in the instance + addl_metrics = self.instance.get('additional_metrics') + if addl_metrics is not None: + self._make_counters( + (addl_metrics, (datatypes, remote_machine, True, 'additional metric entry')) + ) except Exception as e: self.log.debug("Exception in PDH init: %s", str(e)) raise - if key is None or not self._metrics.get(key): + if self.key is None or not self._metrics: raise AttributeError('No valid counters to collect') def _get_netresource(self, remote_machine): @@ -143,22 +140,20 @@ def _get_netresource(self, remote_machine): def check(self, instance): self.log.debug("PDHBaseCheck: check()") - key = hash_mutable(instance) - refresh_counters = is_affirmative(instance.get('refresh_counters', True)) - if refresh_counters: + if self.refresh_counters: for counter, values in list(iteritems(self._missing_counters)): - self._make_counters(key, ([counter], values)) + self._make_counters(([counter], values)) - for inst_name, dd_name, metric_func, counter in self._metrics[key]: + for inst_name, dd_name, metric_func, counter in self._metrics: try: - if refresh_counters: + if self.refresh_counters: counter.collect_counters() vals = counter.get_all_values() for instance_name, val in iteritems(vals): tags = [] - if key in self._tags: - tags = list(self._tags[key]) + if self._tags: + tags = list(self._tags) if not counter.is_single_instance(): tag = "instance:%s" % instance_name @@ -168,7 +163,7 @@ def check(self, instance): # don't give up on all of the metrics because one failed self.log.error("Failed to get data for %s %s: %s", inst_name, dd_name, str(e)) - def _make_counters(self, key, counter_data): + def _make_counters(self, counter_data): counter_list, (datatypes, remote_machine, check_instance, message) = counter_data # list of the metrics. Each entry is itself an entry, @@ -205,7 +200,7 @@ def _make_counters(self, key, counter_data): entry = [inst_name, dd_name, m, obj] self.log.debug('%s: %s', message, entry) - self._metrics[key].append(entry) + self._metrics.append(entry) @classmethod def _no_instance(cls, inst_name): From 5a4361d7cca0acaf46c8b6e874e639660d527eab Mon Sep 17 00:00:00 2001 From: Julia Simon <611228+hithwen@users.noreply.github.com> Date: Mon, 2 Mar 2020 13:41:01 +0100 Subject: [PATCH 02/10] Extract counter_list param --- .../datadog_checks/base/checks/win/winpdh_base.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py b/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py index d166b2e55f838..6ab203900ba5e 100644 --- a/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py +++ b/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py @@ -28,6 +28,7 @@ class PDHBaseCheck(AgentCheck): Windows only. """ def __init__(self, *args, **kwargs): # To support optional agentConfig + # TODO: Change signature to (self, name, init_config, instances, counter_list) once subclasses have been edited AgentCheck.__init__(self, *args, **kwargs) self._missing_counters = {} self._metrics = {} @@ -35,6 +36,8 @@ def __init__(self, *args, **kwargs): # To support optional agentConfig self.refresh_counters = is_affirmative(self.instance.get('refresh_counters', True)) self.key = hash_mutable(self.instance) self._metrics = [] + # TODO Remove once signature is restored to (self, name, init_config, instances, counter_list) + counter_list = kwargs.get('counter_list', args[-1]) try: cfg_tags = self.instance.get('tags') From 286f6f423afd199ec28bf9c147b80cf8a4a9c3e7 Mon Sep 17 00:00:00 2001 From: Julia Simon <611228+hithwen@users.noreply.github.com> Date: Mon, 2 Mar 2020 13:42:32 +0100 Subject: [PATCH 03/10] Style fix --- .../datadog_checks/base/checks/win/winpdh_base.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py b/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py index 6ab203900ba5e..43b3a94f75fe2 100644 --- a/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py +++ b/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py @@ -27,6 +27,7 @@ class PDHBaseCheck(AgentCheck): Windows only. """ + def __init__(self, *args, **kwargs): # To support optional agentConfig # TODO: Change signature to (self, name, init_config, instances, counter_list) once subclasses have been edited AgentCheck.__init__(self, *args, **kwargs) @@ -94,9 +95,7 @@ def __init__(self, *args, **kwargs): # To support optional agentConfig # get any additional metrics in the instance addl_metrics = self.instance.get('additional_metrics') if addl_metrics is not None: - self._make_counters( - (addl_metrics, (datatypes, remote_machine, True, 'additional metric entry')) - ) + self._make_counters((addl_metrics, (datatypes, remote_machine, True, 'additional metric entry'))) except Exception as e: self.log.debug("Exception in PDH init: %s", str(e)) From a4cc5756001199ffd81cec4ecfe9f09b463682ee Mon Sep 17 00:00:00 2001 From: Julia Simon <611228+hithwen@users.noreply.github.com> Date: Mon, 2 Mar 2020 13:46:36 +0100 Subject: [PATCH 04/10] Do not store self.key --- .../datadog_checks/base/checks/win/winpdh_base.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py b/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py index 43b3a94f75fe2..ee1143f7a2a53 100644 --- a/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py +++ b/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py @@ -35,7 +35,6 @@ def __init__(self, *args, **kwargs): # To support optional agentConfig self._metrics = {} self._tags = {} self.refresh_counters = is_affirmative(self.instance.get('refresh_counters', True)) - self.key = hash_mutable(self.instance) self._metrics = [] # TODO Remove once signature is restored to (self, name, init_config, instances, counter_list) counter_list = kwargs.get('counter_list', args[-1]) @@ -101,7 +100,7 @@ def __init__(self, *args, **kwargs): # To support optional agentConfig self.log.debug("Exception in PDH init: %s", str(e)) raise - if self.key is None or not self._metrics: + if not self._metrics or not hash_mutable(self.instance): raise AttributeError('No valid counters to collect') def _get_netresource(self, remote_machine): From d13b089c197fe6acb1a3cc30338e58c9d7db6883 Mon Sep 17 00:00:00 2001 From: Julia Simon <611228+hithwen@users.noreply.github.com> Date: Mon, 2 Mar 2020 13:50:08 +0100 Subject: [PATCH 05/10] Change init types --- .../datadog_checks/base/checks/win/winpdh_base.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py b/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py index ee1143f7a2a53..1082782314a76 100644 --- a/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py +++ b/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py @@ -31,11 +31,11 @@ class PDHBaseCheck(AgentCheck): def __init__(self, *args, **kwargs): # To support optional agentConfig # TODO: Change signature to (self, name, init_config, instances, counter_list) once subclasses have been edited AgentCheck.__init__(self, *args, **kwargs) - self._missing_counters = {} - self._metrics = {} - self._tags = {} - self.refresh_counters = is_affirmative(self.instance.get('refresh_counters', True)) + self._missing_counters = [] + self._metrics = [] + self._tags = [] self._metrics = [] + self.refresh_counters = is_affirmative(self.instance.get('refresh_counters', True)) # TODO Remove once signature is restored to (self, name, init_config, instances, counter_list) counter_list = kwargs.get('counter_list', args[-1]) From ebca5f50a421d5249a6e665f96a0419fef5c02f6 Mon Sep 17 00:00:00 2001 From: Julia Simon <611228+hithwen@users.noreply.github.com> Date: Mon, 2 Mar 2020 14:02:33 +0100 Subject: [PATCH 06/10] Add types --- .../datadog_checks/base/checks/win/winpdh_base.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py b/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py index 1082782314a76..c4e793cf5da8f 100644 --- a/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py +++ b/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py @@ -1,6 +1,8 @@ # (C) Datadog, Inc. 2018-present # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) +from typing import List + import win32wnet from six import iteritems @@ -31,13 +33,12 @@ class PDHBaseCheck(AgentCheck): def __init__(self, *args, **kwargs): # To support optional agentConfig # TODO: Change signature to (self, name, init_config, instances, counter_list) once subclasses have been edited AgentCheck.__init__(self, *args, **kwargs) - self._missing_counters = [] - self._metrics = [] - self._tags = [] - self._metrics = [] - self.refresh_counters = is_affirmative(self.instance.get('refresh_counters', True)) + self._missing_counters = [] # type: List[tuple] + self._metrics = [] # type: List[List] + self._tags = [] # type: List[str] + self.refresh_counters = is_affirmative(self.instance.get('refresh_counters', True)) # type: bool # TODO Remove once signature is restored to (self, name, init_config, instances, counter_list) - counter_list = kwargs.get('counter_list', args[-1]) + counter_list = kwargs.get('counter_list', args[-1]) # type: List[List[str]] try: cfg_tags = self.instance.get('tags') From 60795c20724e7d31c34b1c73f888b80dd114df83 Mon Sep 17 00:00:00 2001 From: Julia Simon <611228+hithwen@users.noreply.github.com> Date: Mon, 2 Mar 2020 17:01:28 +0100 Subject: [PATCH 07/10] Missing counters to stay a dict --- .../datadog_checks/base/checks/win/winpdh_base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py b/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py index c4e793cf5da8f..e5da2d4bfcea9 100644 --- a/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py +++ b/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py @@ -1,7 +1,7 @@ # (C) Datadog, Inc. 2018-present # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) -from typing import List +from typing import List, Dict import win32wnet from six import iteritems @@ -33,7 +33,7 @@ class PDHBaseCheck(AgentCheck): def __init__(self, *args, **kwargs): # To support optional agentConfig # TODO: Change signature to (self, name, init_config, instances, counter_list) once subclasses have been edited AgentCheck.__init__(self, *args, **kwargs) - self._missing_counters = [] # type: List[tuple] + self._missing_counters = {} # type: Dict[str, tuple] self._metrics = [] # type: List[List] self._tags = [] # type: List[str] self.refresh_counters = is_affirmative(self.instance.get('refresh_counters', True)) # type: bool From 64c5574e44da3c4691cab0eba7da8b7b643a1cf6 Mon Sep 17 00:00:00 2001 From: Julia Simon <611228+hithwen@users.noreply.github.com> Date: Mon, 16 Mar 2020 13:34:03 +0100 Subject: [PATCH 08/10] Call super --- .../datadog_checks/base/checks/win/winpdh_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py b/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py index e5da2d4bfcea9..fe91e545892ff 100644 --- a/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py +++ b/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py @@ -32,7 +32,7 @@ class PDHBaseCheck(AgentCheck): def __init__(self, *args, **kwargs): # To support optional agentConfig # TODO: Change signature to (self, name, init_config, instances, counter_list) once subclasses have been edited - AgentCheck.__init__(self, *args, **kwargs) + super(PDHBaseCheck, self).__init__(*args, **kwargs) self._missing_counters = {} # type: Dict[str, tuple] self._metrics = [] # type: List[List] self._tags = [] # type: List[str] From 0e5af69dbd0505983671b30ca0b6ccbda6c69912 Mon Sep 17 00:00:00 2001 From: Julia Simon <611228+hithwen@users.noreply.github.com> Date: Wed, 18 Mar 2020 13:10:02 +0100 Subject: [PATCH 09/10] Style fix --- .../datadog_checks/base/checks/win/winpdh_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py b/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py index fe91e545892ff..c1c2c71daf637 100644 --- a/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py +++ b/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py @@ -1,7 +1,7 @@ # (C) Datadog, Inc. 2018-present # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) -from typing import List, Dict +from typing import Dict, List import win32wnet from six import iteritems From 69fd7492c65da77ee8a3c7a5af117c4bd968cb8f Mon Sep 17 00:00:00 2001 From: Julia Simon <611228+hithwen@users.noreply.github.com> Date: Thu, 19 Mar 2020 12:24:02 +0100 Subject: [PATCH 10/10] Leave dictionaries as dictionaries --- .../base/checks/win/winpdh_base.py | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py b/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py index c1c2c71daf637..80ad7795fcc40 100644 --- a/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py +++ b/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py @@ -1,6 +1,7 @@ # (C) Datadog, Inc. 2018-present # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) +from collections import defaultdict from typing import Dict, List import win32wnet @@ -34,19 +35,20 @@ def __init__(self, *args, **kwargs): # To support optional agentConfig # TODO: Change signature to (self, name, init_config, instances, counter_list) once subclasses have been edited super(PDHBaseCheck, self).__init__(*args, **kwargs) self._missing_counters = {} # type: Dict[str, tuple] - self._metrics = [] # type: List[List] - self._tags = [] # type: List[str] + self._metrics = defaultdict(list) # type: Dict[int, List[List]] # This dictionary only has one key + self._tags = defaultdict(list) # type: Dict[int, List[str]] # This dictionary only has one key self.refresh_counters = is_affirmative(self.instance.get('refresh_counters', True)) # type: bool # TODO Remove once signature is restored to (self, name, init_config, instances, counter_list) counter_list = kwargs.get('counter_list', args[-1]) # type: List[List[str]] try: - cfg_tags = self.instance.get('tags') + self.instance_hash = hash_mutable(self.instance) # type: int + cfg_tags = self.instance.get('tags') # type: List[str] if cfg_tags is not None: if not isinstance(cfg_tags, list): self.log.error("Tags must be configured as a list") raise ValueError("Tags must be type list, not %s" % str(type(cfg_tags))) - self._tags = list(cfg_tags) + self._tags[self.instance_hash] = list(cfg_tags) remote_machine = None host = self.instance.get('host') @@ -90,18 +92,20 @@ def __init__(self, *args, **kwargs): # To support optional agentConfig else: self.log.warning("Unknown data type %s", str(v)) - self._make_counters((counter_list, (datatypes, remote_machine, False, 'entry'))) + self._make_counters(counter_data=(counter_list, (datatypes, remote_machine, False, 'entry'))) # get any additional metrics in the instance addl_metrics = self.instance.get('additional_metrics') if addl_metrics is not None: - self._make_counters((addl_metrics, (datatypes, remote_machine, True, 'additional metric entry'))) + self._make_counters( + counter_data=(addl_metrics, (datatypes, remote_machine, True, 'additional metric entry')) + ) except Exception as e: self.log.debug("Exception in PDH init: %s", str(e)) raise - if not self._metrics or not hash_mutable(self.instance): + if not self.instance_hash or not self._metrics.get(self.instance_hash): raise AttributeError('No valid counters to collect') def _get_netresource(self, remote_machine): @@ -145,17 +149,15 @@ def check(self, instance): if self.refresh_counters: for counter, values in list(iteritems(self._missing_counters)): - self._make_counters(([counter], values)) + self._make_counters(counter_data=([counter], values)) - for inst_name, dd_name, metric_func, counter in self._metrics: + for inst_name, dd_name, metric_func, counter in self._metrics[self.instance_hash]: try: if self.refresh_counters: counter.collect_counters() vals = counter.get_all_values() for instance_name, val in iteritems(vals): - tags = [] - if self._tags: - tags = list(self._tags) + tags = list(self._tags.get(self.instance_hash, [])) # type: List[str] if not counter.is_single_instance(): tag = "instance:%s" % instance_name @@ -165,7 +167,8 @@ def check(self, instance): # don't give up on all of the metrics because one failed self.log.error("Failed to get data for %s %s: %s", inst_name, dd_name, str(e)) - def _make_counters(self, counter_data): + def _make_counters(self, key=None, counter_data=([], ())): # Key left in for retrocompatibility + # type: (int, tuple) -> None counter_list, (datatypes, remote_machine, check_instance, message) = counter_data # list of the metrics. Each entry is itself an entry, @@ -202,7 +205,7 @@ def _make_counters(self, counter_data): entry = [inst_name, dd_name, m, obj] self.log.debug('%s: %s', message, entry) - self._metrics.append(entry) + self._metrics[self.instance_hash].append(entry) @classmethod def _no_instance(cls, inst_name):