From 842b6f2f28786f2302ceb0ffec5b10c8b6875523 Mon Sep 17 00:00:00 2001 From: Mia Henderson Date: Wed, 14 May 2025 15:15:00 -0400 Subject: [PATCH 1/6] consul: add an allowlist for service tags - consul services frequently have a lot of tags that are not relevant to monitoring (routing rules, etc.) - add a configuration option `services_tags_keys_include` to the consul check to allow specification of a list of allowed tag keys to be sent to Datadog - if the allow list is empty/not set, all tags are sent (matching existing behavior) --- consul/assets/configuration/spec.yaml | 14 ++++++++++++++ .../consul/config_models/instance.py | 1 + consul/datadog_checks/consul/consul.py | 14 ++++++++++++++ .../datadog_checks/consul/data/conf.yaml.example | 10 ++++++++++ 4 files changed, 39 insertions(+) diff --git a/consul/assets/configuration/spec.yaml b/consul/assets/configuration/spec.yaml index 39448f373a7fe..469061ee276e4 100644 --- a/consul/assets/configuration/spec.yaml +++ b/consul/assets/configuration/spec.yaml @@ -109,6 +109,20 @@ files: - - + - name: services_tags_keys_include + description: | + If set, only tags with keys matching this list will be sent to Datadog. + This is helpful if you have a lot of tags on services that are not + relevant to Datadog (ingress routing tags, etc). Tags should be specified + here in lowercase, the check will downcase tags from Consul before comparing. + value: + type: array + items: + type: string + example: + - + - + - name: max_services description: | Increase the maximum number of queried services. diff --git a/consul/datadog_checks/consul/config_models/instance.py b/consul/datadog_checks/consul/config_models/instance.py index 11cb2e2afb164..c0ca477df4dbf 100644 --- a/consul/datadog_checks/consul/config_models/instance.py +++ b/consul/datadog_checks/consul/config_models/instance.py @@ -91,6 +91,7 @@ class InstanceConfig(BaseModel): service: Optional[str] = None services_exclude: Optional[tuple[str, ...]] = None services_include: Optional[tuple[str, ...]] = None + services_tags_keys_include: Optional[tuple[str, ...]] = None single_node_install: Optional[bool] = None skip_proxy: Optional[bool] = None tags: Optional[tuple[str, ...]] = None diff --git a/consul/datadog_checks/consul/consul.py b/consul/datadog_checks/consul/consul.py index 7ea41109f30df..1cab177acbc98 100644 --- a/consul/datadog_checks/consul/consul.py +++ b/consul/datadog_checks/consul/consul.py @@ -106,6 +106,7 @@ def __init__(self, name, init_config, instances): 'service_whitelist', self.instance.get('services_include', default_services_include) ) self.services_exclude = set(self.instance.get('services_exclude', self.init_config.get('services_exclude', []))) + self.services_tags_keys_include = set(self.instance.get("services_tags_keys_include", self.init_config.get("services_tags_keys_include", []))) self.max_services = self.instance.get('max_services', self.init_config.get('max_services', MAX_SERVICES)) self.threads_count = self.instance.get('threads_count', self.init_config.get('threads_count', THREADS_COUNT)) if self.threads_count > 1: @@ -312,6 +313,18 @@ def _cull_services_list(self, services): return services + def _cull_services_tags_list(self, services): + if self.services_tags_keys_include: + # services is a dict of {service_name: [tags]} where tags is a list + # of string having the form of "tagkey=tagvalue" + for service in services: + tags = services[service] + # get the tagkey (the part before the "=") and check it against the include list + tags = [t for t in tags if t.split("=")[0].lower() in self.services_tags_keys_include] + services[service] = tags + + return services + @staticmethod def _get_service_tags(service, tags): service_tags = ['consul_service_id:{}'.format(service)] @@ -397,6 +410,7 @@ def check(self, _): self.count_all_nodes(main_tags) services = self._cull_services_list(services) + tags = self._cull_services_tags_list(services) # {node_id: {"up: 0, "passing": 0, "warning": 0, "critical": 0} nodes_to_service_status = defaultdict(lambda: defaultdict(int)) diff --git a/consul/datadog_checks/consul/data/conf.yaml.example b/consul/datadog_checks/consul/data/conf.yaml.example index 92601618a8339..2a69fad22db7d 100644 --- a/consul/datadog_checks/consul/data/conf.yaml.example +++ b/consul/datadog_checks/consul/data/conf.yaml.example @@ -126,6 +126,16 @@ instances: # - # - + ## @param services_tags_keys_include - list of strings - optional + ## If set, only tags with keys matching this list will be sent to Datadog. + ## This is helpful if you have a lot of tags on services that are not + ## relevant to Datadog (ingress routing tags, etc). Tags should be specified + ## here in lowercase, the check will downcase tags from Consul before comparing. + # + # services_tags_keys_include: + # - + # - + ## @param max_services - number - optional - default: 50 ## Increase the maximum number of queried services. # From 3302f72976bda971ed123065bf1369bccae871c9 Mon Sep 17 00:00:00 2001 From: Mia Henderson Date: Wed, 14 May 2025 17:03:26 -0400 Subject: [PATCH 2/6] consul: add test for service tag allowlist --- consul/tests/consul_mocks.py | 6 ++++++ consul/tests/test_unit.py | 41 ++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/consul/tests/consul_mocks.py b/consul/tests/consul_mocks.py index be6cd803f221b..b840913e2d566 100644 --- a/consul/tests/consul_mocks.py +++ b/consul/tests/consul_mocks.py @@ -92,6 +92,12 @@ def mock_get_services_in_cluster(): "service-6": ["active", "standby"], } +def mock_get_n_custom_tagged_services_in_cluster(n, tags): + svcs = {} + for i in range(n): + k = "service-{}".format(i) + svcs[k] = tags + return svcs def mock_get_n_services_in_cluster(n): dct = {} diff --git a/consul/tests/test_unit.py b/consul/tests/test_unit.py index 6e93105a5734c..61b583d99c2f2 100644 --- a/consul/tests/test_unit.py +++ b/consul/tests/test_unit.py @@ -54,6 +54,47 @@ def test_get_nodes_with_service(aggregator): aggregator.assert_metric('consul.catalog.services_count', value=1, tags=expected_tags) +def test_cull_services_tags_keys(aggregator): + consul_check = ConsulCheck(common.CHECK_NAME, {}, [consul_mocks.MOCK_CONFIG]) + consul_mocks.mock_check(consul_check, consul_mocks._get_consul_mocks()) + + all_tags = set([ + "active", + "standby", + "unwanted.tag=unwantedvalue", + "unwanted.tag.but.actually.wanted=wantedvalue", + "wanted.tag", + "unwanted.tag.noequals", + ]) + + include_tags = set([ + 'active', + 'standby', + 'unwanted.tag.but.actually.wanted', + 'wanted.tag' + ]) + + expected_tags = set([ + "active", + "standby", + "unwanted.tag.but.actually.wanted=wantedvalue", + "wanted.tag", + ]) + + unwanted_tags = set([ + "unwanted.tag=unwantedvalue", + "unwanted.tag.noequals", + ]) + + consul_check.services_tags_keys_include = include_tags + services = consul_mocks.mock_get_n_custom_tagged_services_in_cluster(6, all_tags) + + services = consul_check._cull_services_tags_list(services) + for service in services: + assert unwanted_tags.isdisjoint(set(services[service])) + assert expected_tags == set(services[service]) + + def test_get_peers_in_cluster(aggregator): my_mocks = consul_mocks._get_consul_mocks() consul_check = ConsulCheck(common.CHECK_NAME, {}, [consul_mocks.MOCK_CONFIG]) From 62def917d34892d34a35a57d1b7726d244e3d801 Mon Sep 17 00:00:00 2001 From: Mia Date: Thu, 15 May 2025 15:11:17 -0400 Subject: [PATCH 3/6] Apply suggestions from code review Clean up description phrasing Co-authored-by: Janine Chan <64388808+janine-c@users.noreply.github.com> --- consul/assets/configuration/spec.yaml | 2 +- consul/datadog_checks/consul/data/conf.yaml.example | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/consul/assets/configuration/spec.yaml b/consul/assets/configuration/spec.yaml index 469061ee276e4..da68e820076fc 100644 --- a/consul/assets/configuration/spec.yaml +++ b/consul/assets/configuration/spec.yaml @@ -114,7 +114,7 @@ files: If set, only tags with keys matching this list will be sent to Datadog. This is helpful if you have a lot of tags on services that are not relevant to Datadog (ingress routing tags, etc). Tags should be specified - here in lowercase, the check will downcase tags from Consul before comparing. + here in lowercase. Otherwise, the check will downcase tags from Consul before comparing. value: type: array items: diff --git a/consul/datadog_checks/consul/data/conf.yaml.example b/consul/datadog_checks/consul/data/conf.yaml.example index 2a69fad22db7d..cb6a60bf30a18 100644 --- a/consul/datadog_checks/consul/data/conf.yaml.example +++ b/consul/datadog_checks/consul/data/conf.yaml.example @@ -130,7 +130,7 @@ instances: ## If set, only tags with keys matching this list will be sent to Datadog. ## This is helpful if you have a lot of tags on services that are not ## relevant to Datadog (ingress routing tags, etc). Tags should be specified - ## here in lowercase, the check will downcase tags from Consul before comparing. + ## here in lowercase. Otherwise, the check will downcase tags from Consul before comparing. # # services_tags_keys_include: # - From a416b90b27dcbc84975770d1f0c176c413e5dd25 Mon Sep 17 00:00:00 2001 From: Mia Henderson Date: Thu, 15 May 2025 15:14:51 -0400 Subject: [PATCH 4/6] Add changelog --- consul/changelog.d/20306.added | 1 + 1 file changed, 1 insertion(+) create mode 100644 consul/changelog.d/20306.added diff --git a/consul/changelog.d/20306.added b/consul/changelog.d/20306.added new file mode 100644 index 0000000000000..8fdc03cf2211c --- /dev/null +++ b/consul/changelog.d/20306.added @@ -0,0 +1 @@ +Add a new feature to filter Consul service tags being sent to Datadog using an allow list. It can be configured using the `services_tags_keys_include` option. From ade7cd23714d4d0f10d38d2d554f9fd5ff143b42 Mon Sep 17 00:00:00 2001 From: Mia Henderson Date: Thu, 15 May 2025 15:24:48 -0400 Subject: [PATCH 5/6] Clean up formatting --- consul/datadog_checks/consul/consul.py | 4 +- consul/tests/consul_mocks.py | 2 + consul/tests/test_unit.py | 55 +++++++++++++------------- 3 files changed, 33 insertions(+), 28 deletions(-) diff --git a/consul/datadog_checks/consul/consul.py b/consul/datadog_checks/consul/consul.py index 1cab177acbc98..e44df6ce9198b 100644 --- a/consul/datadog_checks/consul/consul.py +++ b/consul/datadog_checks/consul/consul.py @@ -106,7 +106,9 @@ def __init__(self, name, init_config, instances): 'service_whitelist', self.instance.get('services_include', default_services_include) ) self.services_exclude = set(self.instance.get('services_exclude', self.init_config.get('services_exclude', []))) - self.services_tags_keys_include = set(self.instance.get("services_tags_keys_include", self.init_config.get("services_tags_keys_include", []))) + self.services_tags_keys_include = set( + self.instance.get("services_tags_keys_include", self.init_config.get("services_tags_keys_include", [])) + ) self.max_services = self.instance.get('max_services', self.init_config.get('max_services', MAX_SERVICES)) self.threads_count = self.instance.get('threads_count', self.init_config.get('threads_count', THREADS_COUNT)) if self.threads_count > 1: diff --git a/consul/tests/consul_mocks.py b/consul/tests/consul_mocks.py index b840913e2d566..d97016561b5f0 100644 --- a/consul/tests/consul_mocks.py +++ b/consul/tests/consul_mocks.py @@ -92,6 +92,7 @@ def mock_get_services_in_cluster(): "service-6": ["active", "standby"], } + def mock_get_n_custom_tagged_services_in_cluster(n, tags): svcs = {} for i in range(n): @@ -99,6 +100,7 @@ def mock_get_n_custom_tagged_services_in_cluster(n, tags): svcs[k] = tags return svcs + def mock_get_n_services_in_cluster(n): dct = {} for i in range(n): diff --git a/consul/tests/test_unit.py b/consul/tests/test_unit.py index 61b583d99c2f2..bce57ff6d9e1d 100644 --- a/consul/tests/test_unit.py +++ b/consul/tests/test_unit.py @@ -58,33 +58,34 @@ def test_cull_services_tags_keys(aggregator): consul_check = ConsulCheck(common.CHECK_NAME, {}, [consul_mocks.MOCK_CONFIG]) consul_mocks.mock_check(consul_check, consul_mocks._get_consul_mocks()) - all_tags = set([ - "active", - "standby", - "unwanted.tag=unwantedvalue", - "unwanted.tag.but.actually.wanted=wantedvalue", - "wanted.tag", - "unwanted.tag.noequals", - ]) - - include_tags = set([ - 'active', - 'standby', - 'unwanted.tag.but.actually.wanted', - 'wanted.tag' - ]) - - expected_tags = set([ - "active", - "standby", - "unwanted.tag.but.actually.wanted=wantedvalue", - "wanted.tag", - ]) - - unwanted_tags = set([ - "unwanted.tag=unwantedvalue", - "unwanted.tag.noequals", - ]) + all_tags = set( + [ + "active", + "standby", + "unwanted.tag=unwantedvalue", + "unwanted.tag.but.actually.wanted=wantedvalue", + "wanted.tag", + "unwanted.tag.noequals", + ] + ) + + include_tags = set(['active', 'standby', 'unwanted.tag.but.actually.wanted', 'wanted.tag']) + + expected_tags = set( + [ + "active", + "standby", + "unwanted.tag.but.actually.wanted=wantedvalue", + "wanted.tag", + ] + ) + + unwanted_tags = set( + [ + "unwanted.tag=unwantedvalue", + "unwanted.tag.noequals", + ] + ) consul_check.services_tags_keys_include = include_tags services = consul_mocks.mock_get_n_custom_tagged_services_in_cluster(6, all_tags) From a8c9442218d291d6310e53f03972d0473f470204 Mon Sep 17 00:00:00 2001 From: Mia Henderson Date: Thu, 15 May 2025 15:37:11 -0400 Subject: [PATCH 6/6] Use set literals --- consul/tests/test_unit.py | 50 +++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/consul/tests/test_unit.py b/consul/tests/test_unit.py index bce57ff6d9e1d..1f06466fd4604 100644 --- a/consul/tests/test_unit.py +++ b/consul/tests/test_unit.py @@ -58,34 +58,28 @@ def test_cull_services_tags_keys(aggregator): consul_check = ConsulCheck(common.CHECK_NAME, {}, [consul_mocks.MOCK_CONFIG]) consul_mocks.mock_check(consul_check, consul_mocks._get_consul_mocks()) - all_tags = set( - [ - "active", - "standby", - "unwanted.tag=unwantedvalue", - "unwanted.tag.but.actually.wanted=wantedvalue", - "wanted.tag", - "unwanted.tag.noequals", - ] - ) - - include_tags = set(['active', 'standby', 'unwanted.tag.but.actually.wanted', 'wanted.tag']) - - expected_tags = set( - [ - "active", - "standby", - "unwanted.tag.but.actually.wanted=wantedvalue", - "wanted.tag", - ] - ) - - unwanted_tags = set( - [ - "unwanted.tag=unwantedvalue", - "unwanted.tag.noequals", - ] - ) + all_tags = { + "active", + "standby", + "unwanted.tag=unwantedvalue", + "unwanted.tag.but.actually.wanted=wantedvalue", + "wanted.tag", + "unwanted.tag.noequals", + } + + include_tags = {'active', 'standby', 'unwanted.tag.but.actually.wanted', 'wanted.tag'} + + expected_tags = { + "active", + "standby", + "unwanted.tag.but.actually.wanted=wantedvalue", + "wanted.tag", + } + + unwanted_tags = { + "unwanted.tag=unwantedvalue", + "unwanted.tag.noequals", + } consul_check.services_tags_keys_include = include_tags services = consul_mocks.mock_get_n_custom_tagged_services_in_cluster(6, all_tags)