From 773410cf26dd86a6db62327003361eefdc627955 Mon Sep 17 00:00:00 2001 From: Mia Henderson Date: Wed, 14 May 2025 15:15:00 -0400 Subject: [PATCH 1/7] 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 f66ead04b2efc..5a00170e9281e 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 585197fb62b48..3be63fab82934 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 a854346a77b3c..d4d10e1c16760 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 a3c6de5aa922e7af80d934301739d2d51190ab82 Mon Sep 17 00:00:00 2001 From: Mia Henderson Date: Wed, 14 May 2025 17:03:26 -0400 Subject: [PATCH 2/7] 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 9faa729dd4dd379fdba07143922097e3b3421775 Mon Sep 17 00:00:00 2001 From: Mia Date: Thu, 15 May 2025 15:11:17 -0400 Subject: [PATCH 3/7] 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 d4d10e1c16760..dcb5f4baa5cf1 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 ba00f61522220302dbb42256847da838eb89bf8b Mon Sep 17 00:00:00 2001 From: Mia Henderson Date: Thu, 15 May 2025 15:14:51 -0400 Subject: [PATCH 4/7] 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 21e1c078d0b048eae1f8501f3cd7aba5ff555345 Mon Sep 17 00:00:00 2001 From: Mia Henderson Date: Thu, 15 May 2025 15:24:48 -0400 Subject: [PATCH 5/7] 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 3be63fab82934..c326c11ecde28 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 6e14fd545b8d6fa9e852a62446d33e3dd2701739 Mon Sep 17 00:00:00 2001 From: Mia Henderson Date: Thu, 15 May 2025 15:37:11 -0400 Subject: [PATCH 6/7] 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) From 3d513a5a9a390d558a4480467558318b899ff813 Mon Sep 17 00:00:00 2001 From: Mia Henderson Date: Mon, 21 Jul 2025 12:39:31 -0400 Subject: [PATCH 7/7] rename configuration value to allowed_service_tags --- consul/assets/configuration/spec.yaml | 2 +- consul/changelog.d/20306.added | 2 +- consul/datadog_checks/consul/config_models/instance.py | 2 +- consul/datadog_checks/consul/consul.py | 8 ++++---- consul/datadog_checks/consul/data/conf.yaml.example | 4 ++-- consul/tests/test_unit.py | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/consul/assets/configuration/spec.yaml b/consul/assets/configuration/spec.yaml index da68e820076fc..afeabbee43589 100644 --- a/consul/assets/configuration/spec.yaml +++ b/consul/assets/configuration/spec.yaml @@ -109,7 +109,7 @@ files: - - - - name: services_tags_keys_include + - name: allowed_service_tags 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 diff --git a/consul/changelog.d/20306.added b/consul/changelog.d/20306.added index 8fdc03cf2211c..8928e4773bd36 100644 --- a/consul/changelog.d/20306.added +++ b/consul/changelog.d/20306.added @@ -1 +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. +Add a new feature to filter Consul service tags being sent to Datadog using an allow list. It can be configured using the `allowed_service_tags` option. diff --git a/consul/datadog_checks/consul/config_models/instance.py b/consul/datadog_checks/consul/config_models/instance.py index 5a00170e9281e..3020fd931fb6f 100644 --- a/consul/datadog_checks/consul/config_models/instance.py +++ b/consul/datadog_checks/consul/config_models/instance.py @@ -56,6 +56,7 @@ class InstanceConfig(BaseModel): ) acl_token: Optional[str] = None allow_redirects: Optional[bool] = None + allowed_service_tags: Optional[tuple[str, ...]] = None auth_token: Optional[AuthToken] = None auth_type: Optional[str] = None aws_host: Optional[str] = None @@ -91,7 +92,6 @@ 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 c326c11ecde28..e78ba2b45dd45 100644 --- a/consul/datadog_checks/consul/consul.py +++ b/consul/datadog_checks/consul/consul.py @@ -106,8 +106,8 @@ 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.allowed_service_tags = set( + self.instance.get("allowed_service_tags", self.init_config.get("allowed_service_tags", [])) ) 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)) @@ -316,13 +316,13 @@ def _cull_services_list(self, services): return services def _cull_services_tags_list(self, services): - if self.services_tags_keys_include: + if self.allowed_service_tags: # 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] + tags = [t for t in tags if t.split("=")[0].lower() in self.allowed_service_tags] services[service] = tags return services diff --git a/consul/datadog_checks/consul/data/conf.yaml.example b/consul/datadog_checks/consul/data/conf.yaml.example index dcb5f4baa5cf1..038dedcc361bd 100644 --- a/consul/datadog_checks/consul/data/conf.yaml.example +++ b/consul/datadog_checks/consul/data/conf.yaml.example @@ -126,13 +126,13 @@ instances: # - # - - ## @param services_tags_keys_include - list of strings - optional + ## @param allowed_service_tags - 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. Otherwise, the check will downcase tags from Consul before comparing. # - # services_tags_keys_include: + # allowed_service_tags: # - # - diff --git a/consul/tests/test_unit.py b/consul/tests/test_unit.py index 1f06466fd4604..9f5a70b754b5a 100644 --- a/consul/tests/test_unit.py +++ b/consul/tests/test_unit.py @@ -81,7 +81,7 @@ def test_cull_services_tags_keys(aggregator): "unwanted.tag.noequals", } - consul_check.services_tags_keys_include = include_tags + consul_check.allowed_service_tags = include_tags services = consul_mocks.mock_get_n_custom_tagged_services_in_cluster(6, all_tags) services = consul_check._cull_services_tags_list(services)