Add disable generic tags option (#10249)

* commit

* fix tests

* fix e2e test

* fix e2e test

* remove the skip check env var

* validate model

* fix test to include disable generic tag to true

* fix metadata plus test

* remove models validation for hdfs_datanode

nginx/assets/configuration/spec.yaml

@@ -59,6 +59,9 @@ files:
         example: false
     - template: instances/http
     - template: instances/default
+      overrides:
+        disable_generic_tags.hidden: False
+        disable_generic_tags.enabled: True
   - template: logs
     example:
     - type: file

nginx/datadog_checks/nginx/data/conf.yaml.example

@@ -405,6 +405,12 @@ instances:
     #
     # empty_default_hostname: false
 
+    ## @param disable_generic_tags - boolean - optional - default: false
+    ## Generic tags such as `cluster` will be replaced by <integration_name>_cluster to avoid
+    ## getting mixed with other integraton tags.
+    #
+    disable_generic_tags: true
+
 ## Log Section
 ##
 ## type - required - Type of log input source (tcp / udp / file / windows_event)

nginx/datadog_checks/nginx/nginx.py

@@ -9,7 +9,7 @@
 from six import PY3, iteritems, text_type
 from six.moves.urllib.parse import urlparse
 
-from datadog_checks.base import AgentCheck, ConfigurationError
+from datadog_checks.base import AgentCheck, ConfigurationError, to_native_string
 
 from .metrics import METRICS_SEND_AS_COUNT, VTS_METRIC_MAP
 
@@ -350,3 +350,20 @@ def _flatten_json(cls, metric_base, val, tags):
                     output.append((metric_base, int((timestamp - EPOCH).total_seconds()), tags, 'gauge'))
 
         return output
+
+    # override
+    def _normalize_tags_type(self, tags, device_name=None, metric_name=None):
+        if self.disable_generic_tags:
+            return super(Nginx, self)._normalize_tags_type(tags, device_name, metric_name)
+        # If disable_generic_tags is not enabled, for each generic tag we emmit both the generic and the non generic
+        # version to ease transition.
+        normalized_tags = []
+        for tag in tags:
+            if tag is not None:
+                try:
+                    tag = to_native_string(tag)
+                except UnicodeError:
+                    self.log.warning('Encoding error with tag `%s` for metric `%s`, ignoring tag', tag, metric_name)
+                    continue
+                normalized_tags.extend(list({tag, self.degeneralise_tag(tag)}))
+        return normalized_tags

nginx/tests/common.py

@@ -5,13 +5,15 @@
 
 from datadog_checks.dev import get_docker_hostname
 
+CHECK_NAME = 'nginx'
+
 HERE = os.path.dirname(os.path.abspath(__file__))
 FIXTURES_PATH = os.path.join(HERE, 'fixtures')
 
 HOST = get_docker_hostname()
 PORT = '8080'
 PORT_SSL = '8081'
-TAGS = ['foo', 'bar']
+TAGS = ['foo:foo', 'bar:bar']
 USING_VTS = os.getenv('NGINX_IMAGE', '').endswith('nginx-vts')
 USING_LATEST = os.getenv('NGINX_IMAGE', '').endswith('latest')
 NGINX_VERSION = os.getenv('NGINX_VERSION', os.environ['NGINX_IMAGE'])

nginx/tests/conftest.py

@@ -34,14 +34,27 @@ def check():
 
 @pytest.fixture(scope='session')
 def instance():
-    return {'nginx_status_url': 'http://{}:{}/nginx_status'.format(HOST, PORT), 'tags': TAGS}
+    return {
+        'nginx_status_url': 'http://{}:{}/nginx_status'.format(HOST, PORT),
+        'tags': TAGS,
+        'disable_generic_tags': True,
+    }
 
 
 @pytest.fixture
 def instance_ssl():
-    return {'nginx_status_url': 'https://{}:{}/nginx_status'.format(HOST, PORT_SSL), 'tags': TAGS}
+    return {
+        'nginx_status_url': 'https://{}:{}/nginx_status'.format(HOST, PORT_SSL),
+        'tags': TAGS,
+        'disable_generic_tags': True,
+    }
 
 
 @pytest.fixture(scope='session')
 def instance_vts():
-    return {'nginx_status_url': 'http://{}:{}/vts_status'.format(HOST, PORT), 'tags': TAGS, 'use_vts': True}
+    return {
+        'nginx_status_url': 'http://{}:{}/vts_status'.format(HOST, PORT),
+        'tags': TAGS,
+        'use_vts': True,
+        'disable_generic_tags': True,
+    }

nginx/tests/test_e2e.py

@@ -24,7 +24,10 @@ def test_e2e(dd_agent_check, instance):
 
     aggregator.assert_all_metrics_covered()
 
-    tags = common.TAGS + ['port:{}'.format(common.PORT), 'host:{}'.format(common.HOST)]
+    tags = common.TAGS + [
+        'nginx_host:{}'.format(common.HOST),
+        'port:{}'.format(common.PORT),
+    ]
     aggregator.assert_service_check('nginx.can_connect', status=Nginx.OK, tags=tags)
 
 
@@ -69,5 +72,8 @@ def test_e2e_vts(dd_agent_check, instance_vts):
 
     aggregator.assert_all_metrics_covered()
 
-    tags = common.TAGS + ['port:{}'.format(common.PORT), 'host:{}'.format(common.HOST)]
+    tags = common.TAGS + [
+        'nginx_host:{}'.format(common.HOST),
+        'port:{}'.format(common.PORT),
+    ]
     aggregator.assert_service_check('nginx.can_connect', status=Nginx.OK, tags=tags)

nginx/tests/test_nginx.py

@@ -21,7 +21,7 @@ def test_connect(check, instance, aggregator):
     check = check(instance)
     check.check(instance)
     aggregator.assert_metric("nginx.net.connections", tags=TAGS, count=1)
-    extra_tags = ['host:{}'.format(HOST), 'port:{}'.format(PORT)]
+    extra_tags = ['nginx_host:{}'.format(HOST), 'port:{}'.format(PORT)]
     aggregator.assert_service_check('nginx.can_connect', tags=TAGS + extra_tags)
 
 
@@ -73,6 +73,7 @@ def test_metadata_plus(_, aggregator, check, datadog_agent):
     instance = {
         'nginx_status_url': 'dummy_url',
         'use_plus_api': True,
+        'disable_generic_tags': True,
     }
 
     nginx_check = check(instance)

nginx/tests/test_unit.py

@@ -7,7 +7,9 @@
 import mock
 import pytest
 
-from .common import FIXTURES_PATH
+from datadog_checks.nginx import Nginx
+
+from .common import CHECK_NAME, FIXTURES_PATH, TAGS
 from .utils import mocked_perform_request
 
 
@@ -141,3 +143,23 @@ def test_no_version(check, instance, caplog):
 
     errors = [record for record in caplog.records if record.levelname == "ERROR"]
     assert not errors
+
+
+def test_emit_generic_and_non_generic_tags_by_default(instance):
+    instance = deepcopy(instance)
+    instance['disable_generic_tags'] = False
+    check = Nginx(CHECK_NAME, {}, [instance])
+    extra_tags = ['host:localhost']
+    tags = TAGS + extra_tags
+    normalised_tags = TAGS + ['nginx_host:localhost', 'host:localhost']
+    assert set(normalised_tags) == set(check._normalize_tags_type(tags))
+
+
+def test_emit_non_generic_tags_when_disabled(instance):
+    instance = deepcopy(instance)
+    instance['disable_generic_tags'] = True
+    check = Nginx(CHECK_NAME, {}, [instance])
+    extra_tags = ['host:localhost']
+    tags = TAGS + extra_tags
+    normalised_tags = TAGS + ['nginx_host:localhost']
+    assert set(normalised_tags) == set(check._normalize_tags_type(tags))

nginx/tox.ini

@@ -24,7 +24,6 @@ commands =
     pip install -r requirements.in
     pytest -v {posargs}
 setenv =
-    DDEV_SKIP_GENERIC_TAGS_CHECK=true
     1.12: NGINX_IMAGE=nginx:1.12
     1.13: NGINX_IMAGE=nginx:1.13
     vts: NGINX_IMAGE=datadog/docker-library:nginx-vts