From 10b912bc0273499358c34166ebcc03b479aad27b Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Wed, 16 Oct 2019 13:06:48 +0200 Subject: [PATCH 01/16] Add more tests for agent signature --- .../datadog_checks/base/checks/base.py | 8 +++ .../base/checks/openmetrics/base_check.py | 10 ++++ .../tests/test_openmetrics_base_check.py | 50 ++++++++++++++++--- 3 files changed, 61 insertions(+), 7 deletions(-) diff --git a/datadog_checks_base/datadog_checks/base/checks/base.py b/datadog_checks_base/datadog_checks/base/checks/base.py index 43d68d6594650..a573c0e105d5c 100644 --- a/datadog_checks_base/datadog_checks/base/checks/base.py +++ b/datadog_checks_base/datadog_checks/base/checks/base.py @@ -110,6 +110,14 @@ class except the :py:meth:`check` method but sometimes it might be useful for a the Agent might create several different Check instances and the method would be called as many times. + Agent 5 signature: + + AgentCheck(name, init_config, agentConfig, instances=None) + + Agent 6 signature: + + AgentCheck(name, init_config, instances) + :warning: when loading a Custom check, the Agent will inspect the module searching for a subclass of `AgentCheck`. If such a class exists but has been derived in turn, it'll be ignored - **you should never derive from an existing Check**. diff --git a/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py b/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py index d2dfd4db6b3fc..caa2740e7452b 100644 --- a/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py +++ b/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py @@ -19,6 +19,16 @@ class OpenMetricsBaseCheck(OpenMetricsScraperMixin, AgentCheck): metrics: - bar - foo + + + Agent 5 signature: + + OpenMetricsBaseCheck(name, init_config, agentConfig, instances=None, default_instances=None, default_namespace=None) + + Agent 6 signature: + + OpenMetricsBaseCheck(name, init_config, instances, default_instances=None, default_namespace=None) + """ DEFAULT_METRIC_LIMIT = 2000 diff --git a/datadog_checks_base/tests/test_openmetrics_base_check.py b/datadog_checks_base/tests/test_openmetrics_base_check.py index 0a2c26ed1db50..1cac8b3c7fda2 100644 --- a/datadog_checks_base/tests/test_openmetrics_base_check.py +++ b/datadog_checks_base/tests/test_openmetrics_base_check.py @@ -9,47 +9,83 @@ class TestSignature: + INIT_CONFIG = {'my_init_config': 'foo bar init config'} + AGENT_CONFIG = {'my_agent_config': 'foo bar agent config'} + def test_default_legacy_basic(self): - check = OpenMetricsBaseCheck('openmetrics_check', {}, {}) + check = OpenMetricsBaseCheck('openmetrics_check', self.INIT_CONFIG, self.AGENT_CONFIG) assert check.default_instances == {} assert check.default_namespace is None + assert check.agentConfig == {'my_agent_config': 'foo bar agent config'} + assert check.name == 'openmetrics_check' + assert check.init_config == {'my_init_config': 'foo bar init config'} def test_default_instances(self): instance = {'prometheus_url': 'endpoint'} - check = OpenMetricsBaseCheck('openmetrics_check', {}, [instance], default_namespace='openmetrics') + check = OpenMetricsBaseCheck('openmetrics_check', self.INIT_CONFIG, [instance], default_namespace='openmetrics') assert check.default_instances == {} assert check.default_namespace == 'openmetrics' assert 'endpoint' in check.config_map + assert check.instance == instance + assert check.agentConfig == {} + assert check.name == 'openmetrics_check' + assert check.init_config == {'my_init_config': 'foo bar init config'} def test_default_instances_legacy(self): instance = {'prometheus_url': 'endpoint'} - check = OpenMetricsBaseCheck('openmetrics_check', {}, {}, [instance], default_namespace='openmetrics') + check = OpenMetricsBaseCheck( + 'openmetrics_check', self.INIT_CONFIG, self.AGENT_CONFIG, [instance], default_namespace='openmetrics' + ) assert check.default_instances == {} assert check.default_namespace == 'openmetrics' assert 'endpoint' in check.config_map + assert check.instance == instance + assert check.agentConfig == {'my_agent_config': 'foo bar agent config'} + assert check.name == 'openmetrics_check' + assert check.init_config == {'my_init_config': 'foo bar init config'} def test_default_instances_legacy_kwarg(self): instance1 = {'prometheus_url': 'endpoint1'} instance2 = {'prometheus_url': 'endpoint2'} check = OpenMetricsBaseCheck( - 'openmetrics_check', {}, {}, instances=[instance1, instance2], default_namespace='openmetrics' + 'openmetrics_check', + self.INIT_CONFIG, + {}, + instances=[instance1, instance2], + default_instances={'my_inst': {'foo': 'bar'}}, + default_namespace='openmetrics', ) - assert check.default_instances == {} + assert check.default_instances == {'my_inst': {'foo': 'bar'}} assert check.default_namespace == 'openmetrics' assert 'endpoint1' in check.config_map assert 'endpoint2' in check.config_map + assert check.instance == instance1 + assert check.agentConfig == {} + assert check.name == 'openmetrics_check' + assert check.init_config == {'my_init_config': 'foo bar init config'} def test_args_legacy(self): instance = {'prometheus_url': 'endpoint'} - check = OpenMetricsBaseCheck('openmetrics_check', {}, {}, [instance], None, 'openmetrics') + check = OpenMetricsBaseCheck( + 'openmetrics_check', + self.INIT_CONFIG, + self.AGENT_CONFIG, + [instance], + {'my_inst': {'foo': 'bar'}}, + 'openmetrics', + ) - assert check.default_instances == {} + assert check.default_instances == {'my_inst': {'foo': 'bar'}} assert check.default_namespace == 'openmetrics' assert 'endpoint' in check.config_map + assert check.instance == instance + assert check.agentConfig == {'my_agent_config': 'foo bar agent config'} + assert check.name == 'openmetrics_check' + assert check.init_config == {'my_init_config': 'foo bar init config'} def test_rate_override(): From 16e2bbed635683c76a6cb137724491fd6fd1bca2 Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Wed, 16 Oct 2019 13:22:24 +0200 Subject: [PATCH 02/16] Fix signature --- datadog_checks_base/datadog_checks/base/checks/base.py | 8 ++++---- .../datadog_checks/base/checks/openmetrics/base_check.py | 7 ++++++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/datadog_checks_base/datadog_checks/base/checks/base.py b/datadog_checks_base/datadog_checks/base/checks/base.py index a573c0e105d5c..51e27e0b91f30 100644 --- a/datadog_checks_base/datadog_checks/base/checks/base.py +++ b/datadog_checks_base/datadog_checks/base/checks/base.py @@ -142,14 +142,14 @@ class except the :py:meth:`check` method but sometimes it might be useful for a if len(args) > 1: self.init_config = args[1] if len(args) > 2: - if len(args) > 3 or 'instances' in kwargs: + if isinstance(args[2], list): + # new-style init: the 3rd argument is `instances` + self.instances = args[2] + else: # old-style init: the 3rd argument is `agentConfig` self.agentConfig = args[2] if len(args) > 3: self.instances = args[3] - else: - # new-style init: the 3rd argument is `instances` - self.instances = args[2] # Agent 6+ will only have one instance self.instance = self.instances[0] if self.instances else None diff --git a/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py b/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py index caa2740e7452b..0dcdbcb457919 100644 --- a/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py +++ b/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py @@ -52,12 +52,17 @@ def __init__(self, *args, **kwargs): self.default_namespace = default_namespace # pre-generate the scraper configurations + if 'instances' in kwargs: instances = kwargs['instances'] elif len(args) == 4: + # instances from legacy signature instances = args[3] - else: + elif isinstance(args[2], list): + # instances from new signature instances = args[2] + else: + instances = None if instances is not None: for instance in instances: From 946f1358e96f8f9dadaa846bf79f53c811d10ec9 Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Wed, 16 Oct 2019 13:26:24 +0200 Subject: [PATCH 03/16] Improve test case --- datadog_checks_base/tests/test_openmetrics_base_check.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/datadog_checks_base/tests/test_openmetrics_base_check.py b/datadog_checks_base/tests/test_openmetrics_base_check.py index 1cac8b3c7fda2..51ca997104e90 100644 --- a/datadog_checks_base/tests/test_openmetrics_base_check.py +++ b/datadog_checks_base/tests/test_openmetrics_base_check.py @@ -53,7 +53,7 @@ def test_default_instances_legacy_kwarg(self): check = OpenMetricsBaseCheck( 'openmetrics_check', self.INIT_CONFIG, - {}, + self.AGENT_CONFIG, instances=[instance1, instance2], default_instances={'my_inst': {'foo': 'bar'}}, default_namespace='openmetrics', @@ -64,7 +64,7 @@ def test_default_instances_legacy_kwarg(self): assert 'endpoint1' in check.config_map assert 'endpoint2' in check.config_map assert check.instance == instance1 - assert check.agentConfig == {} + assert check.agentConfig == {'my_agent_config': 'foo bar agent config'} assert check.name == 'openmetrics_check' assert check.init_config == {'my_init_config': 'foo bar init config'} From 10d9c42ac90757c221ba5461f428b412292f49b2 Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Wed, 16 Oct 2019 13:44:42 +0200 Subject: [PATCH 04/16] add test for agent signature --- datadog_checks_base/tests/test_agent_check.py | 140 ++++++++++++++++++ 1 file changed, 140 insertions(+) diff --git a/datadog_checks_base/tests/test_agent_check.py b/datadog_checks_base/tests/test_agent_check.py index b5d1db62ede40..182071dfc3f2f 100644 --- a/datadog_checks_base/tests/test_agent_check.py +++ b/datadog_checks_base/tests/test_agent_check.py @@ -72,6 +72,146 @@ def test_warning_args_errors(): assert ["should not raise error: %s"] == check.warnings +@pytest.mark.parametrize( + 'case_name, check, expected_properties', + [ + ( + 'agent 5 signature: only args', + AgentCheck('check_name', {'init_conf1': 'init_value1'}, {'agent_conf1': 'agent_value1'}, [{'foo': 'bar'}]), + { + 'name': 'check_name', + 'init_config': {'init_conf1': 'init_value1'}, + 'agentConfig': {'agent_conf1': 'agent_value1'}, + 'instance': {'foo': 'bar'}, + }, + ), + ( + 'agent 5 signature: instances as kwarg', + AgentCheck('check_name', {'init_conf1': 'init_value1'}, {'agent_conf1': 'agent_value1'}, instances=[{'foo': 'bar'}]), + { + 'name': 'check_name', + 'init_config': {'init_conf1': 'init_value1'}, + 'agentConfig': {'agent_conf1': 'agent_value1'}, + 'instance': {'foo': 'bar'}, + }, + ), + ( + 'agent 5 signature: agentConfig and instances as kwarg', + AgentCheck('check_name', {'init_conf1': 'init_value1'}, agentConfig={'agent_conf1': 'agent_value1'}, instances=[{'foo': 'bar'}]), + { + 'name': 'check_name', + 'init_config': {'init_conf1': 'init_value1'}, + 'agentConfig': {'agent_conf1': 'agent_value1'}, + 'instance': {'foo': 'bar'}, + }, + ), + ( + 'agent 5 signature: init_config, agentConfig and instances as kwarg', + AgentCheck('check_name', init_config={'init_conf1': 'init_value1'}, agentConfig={'agent_conf1': 'agent_value1'}, instances=[{'foo': 'bar'}]), + { + 'name': 'check_name', + 'init_config': {'init_conf1': 'init_value1'}, + 'agentConfig': {'agent_conf1': 'agent_value1'}, + 'instance': {'foo': 'bar'}, + }, + ), + ( + 'agent 5 signature: name, init_config, agentConfig and instances as kwarg', + AgentCheck(name='check_name', init_config={'init_conf1': 'init_value1'}, agentConfig={'agent_conf1': 'agent_value1'}, instances=[{'foo': 'bar'}]), + { + 'name': 'check_name', + 'init_config': {'init_conf1': 'init_value1'}, + 'agentConfig': {'agent_conf1': 'agent_value1'}, + 'instance': {'foo': 'bar'}, + }, + ), + ( + 'agent 5 signature: no instances', + AgentCheck('check_name', {'init_conf1': 'init_value1'}, {'agent_conf1': 'agent_value1'}), + { + 'name': 'check_name', + 'init_config': {'init_conf1': 'init_value1'}, + 'agentConfig': {'agent_conf1': 'agent_value1'}, + 'instance': None, + }, + ), + ( + 'agent 5 signature: no instances and agentConfig as kwarg', + AgentCheck('check_name', {'init_conf1': 'init_value1'}, agentConfig={'agent_conf1': 'agent_value1'}), + { + 'name': 'check_name', + 'init_config': {'init_conf1': 'init_value1'}, + 'agentConfig': {'agent_conf1': 'agent_value1'}, + 'instance': None, + }, + ), + ( + 'agent 5 signature: no instances and init_config, agentConfig as kwarg', + AgentCheck('check_name', init_config={'init_conf1': 'init_value1'}, agentConfig={'agent_conf1': 'agent_value1'}), + { + 'name': 'check_name', + 'init_config': {'init_conf1': 'init_value1'}, + 'agentConfig': {'agent_conf1': 'agent_value1'}, + 'instance': None, + }, + ), + ( + 'agent 5 signature: no instances and name, init_config, agentConfig as kwarg', + AgentCheck(name='check_name', init_config={'init_conf1': 'init_value1'}, agentConfig={'agent_conf1': 'agent_value1'}), + { + 'name': 'check_name', + 'init_config': {'init_conf1': 'init_value1'}, + 'agentConfig': {'agent_conf1': 'agent_value1'}, + 'instance': None, + }, + ), + ( + 'agent 6 signature: only args', + AgentCheck('check_name', {'init_conf1': 'init_value1'}, [{'foo': 'bar'}]), + { + 'name': 'check_name', + 'init_config': {'init_conf1': 'init_value1'}, + 'agentConfig': {}, + 'instance': {'foo': 'bar'}, + }, + ), + ( + 'agent 6 signature: instances as kwarg', + AgentCheck('check_name', {'init_conf1': 'init_value1'}, instances=[{'foo': 'bar'}]), + { + 'name': 'check_name', + 'init_config': {'init_conf1': 'init_value1'}, + 'agentConfig': {}, + 'instance': {'foo': 'bar'}, + }, + ), + ( + 'agent 6 signature: init_config, instances as kwarg', + AgentCheck('check_name', init_config={'init_conf1': 'init_value1'}, instances=[{'foo': 'bar'}]), + { + 'name': 'check_name', + 'init_config': {'init_conf1': 'init_value1'}, + 'agentConfig': {}, + 'instance': {'foo': 'bar'}, + }, + ), + ( + 'agent 6 signature: name, init_config, instances as kwarg', + AgentCheck(name='check_name', init_config={'init_conf1': 'init_value1'}, instances=[{'foo': 'bar'}]), + { + 'name': 'check_name', + 'init_config': {'init_conf1': 'init_value1'}, + 'agentConfig': {}, + 'instance': {'foo': 'bar'}, + }, + ), + ], +) +def test_agent_signature(case_name, check, expected_properties): + for attr, value in expected_properties.items(): + assert getattr(check, attr) == value + + class TestMetricNormalization: def test_default(self): check = AgentCheck() From ad8866e0245664849b9536ab62d5a0170a50973f Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Wed, 16 Oct 2019 13:56:32 +0200 Subject: [PATCH 05/16] Fix style --- .../base/checks/openmetrics/base_check.py | 3 +- datadog_checks_base/tests/test_agent_check.py | 35 +++++++++++++++---- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py b/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py index 0dcdbcb457919..971d9ba97e42a 100644 --- a/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py +++ b/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py @@ -23,7 +23,8 @@ class OpenMetricsBaseCheck(OpenMetricsScraperMixin, AgentCheck): Agent 5 signature: - OpenMetricsBaseCheck(name, init_config, agentConfig, instances=None, default_instances=None, default_namespace=None) + OpenMetricsBaseCheck(name, init_config, agentConfig, instances=None, default_instances=None, + default_namespace=None) Agent 6 signature: diff --git a/datadog_checks_base/tests/test_agent_check.py b/datadog_checks_base/tests/test_agent_check.py index 182071dfc3f2f..2c3096c5cbb66 100644 --- a/datadog_checks_base/tests/test_agent_check.py +++ b/datadog_checks_base/tests/test_agent_check.py @@ -87,7 +87,9 @@ def test_warning_args_errors(): ), ( 'agent 5 signature: instances as kwarg', - AgentCheck('check_name', {'init_conf1': 'init_value1'}, {'agent_conf1': 'agent_value1'}, instances=[{'foo': 'bar'}]), + AgentCheck( + 'check_name', {'init_conf1': 'init_value1'}, {'agent_conf1': 'agent_value1'}, instances=[{'foo': 'bar'}] + ), { 'name': 'check_name', 'init_config': {'init_conf1': 'init_value1'}, @@ -97,7 +99,12 @@ def test_warning_args_errors(): ), ( 'agent 5 signature: agentConfig and instances as kwarg', - AgentCheck('check_name', {'init_conf1': 'init_value1'}, agentConfig={'agent_conf1': 'agent_value1'}, instances=[{'foo': 'bar'}]), + AgentCheck( + 'check_name', + {'init_conf1': 'init_value1'}, + agentConfig={'agent_conf1': 'agent_value1'}, + instances=[{'foo': 'bar'}], + ), { 'name': 'check_name', 'init_config': {'init_conf1': 'init_value1'}, @@ -107,7 +114,12 @@ def test_warning_args_errors(): ), ( 'agent 5 signature: init_config, agentConfig and instances as kwarg', - AgentCheck('check_name', init_config={'init_conf1': 'init_value1'}, agentConfig={'agent_conf1': 'agent_value1'}, instances=[{'foo': 'bar'}]), + AgentCheck( + 'check_name', + init_config={'init_conf1': 'init_value1'}, + agentConfig={'agent_conf1': 'agent_value1'}, + instances=[{'foo': 'bar'}], + ), { 'name': 'check_name', 'init_config': {'init_conf1': 'init_value1'}, @@ -117,7 +129,12 @@ def test_warning_args_errors(): ), ( 'agent 5 signature: name, init_config, agentConfig and instances as kwarg', - AgentCheck(name='check_name', init_config={'init_conf1': 'init_value1'}, agentConfig={'agent_conf1': 'agent_value1'}, instances=[{'foo': 'bar'}]), + AgentCheck( + name='check_name', + init_config={'init_conf1': 'init_value1'}, + agentConfig={'agent_conf1': 'agent_value1'}, + instances=[{'foo': 'bar'}], + ), { 'name': 'check_name', 'init_config': {'init_conf1': 'init_value1'}, @@ -147,7 +164,9 @@ def test_warning_args_errors(): ), ( 'agent 5 signature: no instances and init_config, agentConfig as kwarg', - AgentCheck('check_name', init_config={'init_conf1': 'init_value1'}, agentConfig={'agent_conf1': 'agent_value1'}), + AgentCheck( + 'check_name', init_config={'init_conf1': 'init_value1'}, agentConfig={'agent_conf1': 'agent_value1'} + ), { 'name': 'check_name', 'init_config': {'init_conf1': 'init_value1'}, @@ -157,7 +176,11 @@ def test_warning_args_errors(): ), ( 'agent 5 signature: no instances and name, init_config, agentConfig as kwarg', - AgentCheck(name='check_name', init_config={'init_conf1': 'init_value1'}, agentConfig={'agent_conf1': 'agent_value1'}), + AgentCheck( + name='check_name', + init_config={'init_conf1': 'init_value1'}, + agentConfig={'agent_conf1': 'agent_value1'}, + ), { 'name': 'check_name', 'init_config': {'init_conf1': 'init_value1'}, From ca2f967c93f6593168ae71b9d74360c1e4742a60 Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Wed, 16 Oct 2019 14:05:15 +0200 Subject: [PATCH 06/16] Improve test --- datadog_checks_base/tests/test_agent_check.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/datadog_checks_base/tests/test_agent_check.py b/datadog_checks_base/tests/test_agent_check.py index 2c3096c5cbb66..61035ab5344c1 100644 --- a/datadog_checks_base/tests/test_agent_check.py +++ b/datadog_checks_base/tests/test_agent_check.py @@ -73,7 +73,7 @@ def test_warning_args_errors(): @pytest.mark.parametrize( - 'case_name, check, expected_properties', + 'case_name, check, expected_attributes', [ ( 'agent 5 signature: only args', @@ -230,9 +230,9 @@ def test_warning_args_errors(): ), ], ) -def test_agent_signature(case_name, check, expected_properties): - for attr, value in expected_properties.items(): - assert getattr(check, attr) == value +def test_agent_signature(case_name, check, expected_attributes): + actual_attributes = {attr: getattr(check, attr) for attr in expected_attributes} + assert expected_attributes == actual_attributes class TestMetricNormalization: From 717862fd79395acfba5231060e6f781009a45da7 Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Wed, 16 Oct 2019 14:24:45 +0200 Subject: [PATCH 07/16] Empty commit to trigger CI From 42baaccb8a478b5d68df53c9097ed32987b5585c Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Wed, 16 Oct 2019 15:05:44 +0200 Subject: [PATCH 08/16] fix base.py --- datadog_checks_base/datadog_checks/base/checks/base.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/datadog_checks_base/datadog_checks/base/checks/base.py b/datadog_checks_base/datadog_checks/base/checks/base.py index 51e27e0b91f30..942b858817e1d 100644 --- a/datadog_checks_base/datadog_checks/base/checks/base.py +++ b/datadog_checks_base/datadog_checks/base/checks/base.py @@ -142,14 +142,14 @@ class except the :py:meth:`check` method but sometimes it might be useful for a if len(args) > 1: self.init_config = args[1] if len(args) > 2: - if isinstance(args[2], list): - # new-style init: the 3rd argument is `instances` - self.instances = args[2] - else: + if (len(args) > 3 or not isinstance(args[2], list)) or 'instances' in kwargs: # old-style init: the 3rd argument is `agentConfig` self.agentConfig = args[2] if len(args) > 3: self.instances = args[3] + else: + # new-style init: the 3rd argument is `instances` + self.instances = args[2] # Agent 6+ will only have one instance self.instance = self.instances[0] if self.instances else None From e514e448032a8a466b696e3e82207ed366e3e8af Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Wed, 16 Oct 2019 15:10:31 +0200 Subject: [PATCH 09/16] fix base.py --- datadog_checks_base/datadog_checks/base/checks/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datadog_checks_base/datadog_checks/base/checks/base.py b/datadog_checks_base/datadog_checks/base/checks/base.py index 942b858817e1d..091045ebaa20c 100644 --- a/datadog_checks_base/datadog_checks/base/checks/base.py +++ b/datadog_checks_base/datadog_checks/base/checks/base.py @@ -142,7 +142,7 @@ class except the :py:meth:`check` method but sometimes it might be useful for a if len(args) > 1: self.init_config = args[1] if len(args) > 2: - if (len(args) > 3 or not isinstance(args[2], list)) or 'instances' in kwargs: + if len(args) > 3 or not isinstance(args[2], list) or 'instances' in kwargs: # old-style init: the 3rd argument is `agentConfig` self.agentConfig = args[2] if len(args) > 3: From cee5ab1a339fd4f856c7426f9982b8c74ce9ef85 Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Wed, 16 Oct 2019 16:55:08 +0200 Subject: [PATCH 10/16] add tuple to signature logic --- datadog_checks_base/datadog_checks/base/checks/base.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/datadog_checks_base/datadog_checks/base/checks/base.py b/datadog_checks_base/datadog_checks/base/checks/base.py index 091045ebaa20c..3b005e8fec156 100644 --- a/datadog_checks_base/datadog_checks/base/checks/base.py +++ b/datadog_checks_base/datadog_checks/base/checks/base.py @@ -142,7 +142,8 @@ class except the :py:meth:`check` method but sometimes it might be useful for a if len(args) > 1: self.init_config = args[1] if len(args) > 2: - if len(args) > 3 or not isinstance(args[2], list) or 'instances' in kwargs: + # agent pass instances as tuple but in test we are usually using list, so we are testing for both + if len(args) > 3 or not isinstance(args[2], (list, tuple)) or 'instances' in kwargs: # old-style init: the 3rd argument is `agentConfig` self.agentConfig = args[2] if len(args) > 3: From 49b4c7e958e2dafc27e488d1700d8a29ccd603ba Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Wed, 16 Oct 2019 17:04:07 +0200 Subject: [PATCH 11/16] Improve signature doc --- datadog_checks_base/datadog_checks/base/checks/base.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/datadog_checks_base/datadog_checks/base/checks/base.py b/datadog_checks_base/datadog_checks/base/checks/base.py index 3b005e8fec156..6f4391919a43f 100644 --- a/datadog_checks_base/datadog_checks/base/checks/base.py +++ b/datadog_checks_base/datadog_checks/base/checks/base.py @@ -113,10 +113,17 @@ class except the :py:meth:`check` method but sometimes it might be useful for a Agent 5 signature: AgentCheck(name, init_config, agentConfig, instances=None) + AgentCheck.check(instance) - Agent 6 signature: + Agent 6,7 signature: AgentCheck(name, init_config, instances) + AgentCheck.check(instance) + + Agent 8 signature: + + AgentCheck(name, init_config, instance) # one instance + AgentCheck.check() # no more instance argument for check method :warning: when loading a Custom check, the Agent will inspect the module searching for a subclass of `AgentCheck`. If such a class exists but has been derived in From 1a7ab29d8f6d333d89acf6b1daf3dec06beb7290 Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Wed, 16 Oct 2019 17:22:40 +0200 Subject: [PATCH 12/16] Update doc --- datadog_checks_base/datadog_checks/base/checks/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datadog_checks_base/datadog_checks/base/checks/base.py b/datadog_checks_base/datadog_checks/base/checks/base.py index 6f4391919a43f..bacd552004d6c 100644 --- a/datadog_checks_base/datadog_checks/base/checks/base.py +++ b/datadog_checks_base/datadog_checks/base/checks/base.py @@ -117,7 +117,7 @@ class except the :py:meth:`check` method but sometimes it might be useful for a Agent 6,7 signature: - AgentCheck(name, init_config, instances) + AgentCheck(name, init_config, instances) # instances contain only 1 instance AgentCheck.check(instance) Agent 8 signature: From df7bff2af4a0fa9445bf77e8139c99e334a4d86e Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Wed, 16 Oct 2019 17:24:59 +0200 Subject: [PATCH 13/16] test for tuple and list in OpenMetricsBaseCheck --- .../datadog_checks/base/checks/openmetrics/base_check.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py b/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py index 971d9ba97e42a..6cedadbe22ae5 100644 --- a/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py +++ b/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py @@ -59,7 +59,7 @@ def __init__(self, *args, **kwargs): elif len(args) == 4: # instances from legacy signature instances = args[3] - elif isinstance(args[2], list): + elif isinstance(args[2], (tuple, list)): # instances from new signature instances = args[2] else: From d433980936a3107d0e6804d585f3d68a37efbc0c Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Wed, 16 Oct 2019 17:37:57 +0200 Subject: [PATCH 14/16] add tests for tuple --- datadog_checks_base/tests/test_agent_check.py | 12 +++++++++++- .../tests/test_openmetrics_base_check.py | 16 +++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/datadog_checks_base/tests/test_agent_check.py b/datadog_checks_base/tests/test_agent_check.py index 61035ab5344c1..ea11d825542a6 100644 --- a/datadog_checks_base/tests/test_agent_check.py +++ b/datadog_checks_base/tests/test_agent_check.py @@ -189,7 +189,7 @@ def test_warning_args_errors(): }, ), ( - 'agent 6 signature: only args', + 'agent 6 signature: only args (instances as list)', AgentCheck('check_name', {'init_conf1': 'init_value1'}, [{'foo': 'bar'}]), { 'name': 'check_name', @@ -198,6 +198,16 @@ def test_warning_args_errors(): 'instance': {'foo': 'bar'}, }, ), + ( + 'agent 6 signature: only args (instances as tuple)', + AgentCheck('check_name', {'init_conf1': 'init_value1'}, ({'foo': 'bar'},)), + { + 'name': 'check_name', + 'init_config': {'init_conf1': 'init_value1'}, + 'agentConfig': {}, + 'instance': {'foo': 'bar'}, + }, + ), ( 'agent 6 signature: instances as kwarg', AgentCheck('check_name', {'init_conf1': 'init_value1'}, instances=[{'foo': 'bar'}]), diff --git a/datadog_checks_base/tests/test_openmetrics_base_check.py b/datadog_checks_base/tests/test_openmetrics_base_check.py index 51ca997104e90..05f91cfddb69a 100644 --- a/datadog_checks_base/tests/test_openmetrics_base_check.py +++ b/datadog_checks_base/tests/test_openmetrics_base_check.py @@ -21,7 +21,7 @@ def test_default_legacy_basic(self): assert check.name == 'openmetrics_check' assert check.init_config == {'my_init_config': 'foo bar init config'} - def test_default_instances(self): + def test_default_instances_with_list_instances(self): instance = {'prometheus_url': 'endpoint'} check = OpenMetricsBaseCheck('openmetrics_check', self.INIT_CONFIG, [instance], default_namespace='openmetrics') @@ -33,6 +33,20 @@ def test_default_instances(self): assert check.name == 'openmetrics_check' assert check.init_config == {'my_init_config': 'foo bar init config'} + def test_default_instances_with_tuple_instances(self): + instance = {'prometheus_url': 'endpoint'} + check = OpenMetricsBaseCheck( + 'openmetrics_check', self.INIT_CONFIG, (instance,), default_namespace='openmetrics' + ) + + assert check.default_instances == {} + assert check.default_namespace == 'openmetrics' + assert 'endpoint' in check.config_map + assert check.instance == instance + assert check.agentConfig == {} + assert check.name == 'openmetrics_check' + assert check.init_config == {'my_init_config': 'foo bar init config'} + def test_default_instances_legacy(self): instance = {'prometheus_url': 'endpoint'} check = OpenMetricsBaseCheck( From fb28f840691ad9fcb4a24db015f8750bbd40290c Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Tue, 22 Oct 2019 13:39:53 +0200 Subject: [PATCH 15/16] Update datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py Co-Authored-By: Julia <611228+hithwen@users.noreply.github.com> --- .../datadog_checks/base/checks/openmetrics/base_check.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py b/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py index 6cedadbe22ae5..f717448540f7d 100644 --- a/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py +++ b/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py @@ -57,7 +57,7 @@ def __init__(self, *args, **kwargs): if 'instances' in kwargs: instances = kwargs['instances'] elif len(args) == 4: - # instances from legacy signature + # instances from agent 5 signature instances = args[3] elif isinstance(args[2], (tuple, list)): # instances from new signature From 1d66eb282dae96795e807dc528775f27f85f58c7 Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Tue, 22 Oct 2019 13:40:04 +0200 Subject: [PATCH 16/16] Update datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py Co-Authored-By: Julia <611228+hithwen@users.noreply.github.com> --- .../datadog_checks/base/checks/openmetrics/base_check.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py b/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py index f717448540f7d..aeb87248c2812 100644 --- a/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py +++ b/datadog_checks_base/datadog_checks/base/checks/openmetrics/base_check.py @@ -60,7 +60,7 @@ def __init__(self, *args, **kwargs): # instances from agent 5 signature instances = args[3] elif isinstance(args[2], (tuple, list)): - # instances from new signature + # instances from agent 6 signature instances = args[2] else: instances = None