Merge pull request #3060 from DataDog/haissam/sd-cache-kv-tpls

[Service Discovery] create a template cache to reduce calls to the KV store

Author: hkaj

agent.py

@@ -91,7 +91,7 @@ def __init__(self, pidfile, autorestart, start_event=True, in_developer_mode=Fal
         self._checksd = []
         self.collector_profile_interval = DEFAULT_COLLECTOR_PROFILE_INTERVAL
         self.check_frequency = None
-        # this flag can be set to True, False, or a list of checks (for partial reload)
+        # this flag can be set to True, False, or a set of checks (for partial reload)
         self.reload_configs_flag = False
         self.sd_backend = None
         self.supervisor_proxy = None
@@ -161,7 +161,7 @@ def reload_configs(self, checks_to_reload=set()):
             log.info("No checksd configs found")
 
     def refresh_specific_checks(self, hostname, checksd, checks):
-        """take a list of checks and for each of them:
+        """take a set of checks and for each of them:
             - remove it from the init_failed_checks if it was there
             - load a fresh config for it
             - replace its old config with the new one in initialized_checks if there was one
@@ -245,7 +245,7 @@ def run(self, config=None):
         if self._agentConfig.get('service_discovery'):
             self.sd_backend = get_sd_backend(self._agentConfig)
 
-        if _is_affirmative(self._agentConfig.get('sd_jmx_enable')):
+        if _is_affirmative(self._agentConfig.get('sd_jmx_enable', False)):
             pipe_path = get_jmx_pipe_path()
             if Platform.is_windows():
                 pipe_name = pipe_path.format(pipename=SD_PIPE_NAME)

checks.d/docker_daemon.py

@@ -851,7 +851,7 @@ def _parse_cgroup_file(self, stat_file):
         except IOError:
             # It is possible that the container got stopped between the API call and now.
             # Some files can also be missing (like cpu.stat) and that's fine.
-            self.log.info("Can't open %s. Some metrics for this container may be missing." % stat_file)
+            self.log.debug("Can't open %s. Its metrics will be missing." % stat_file)
 
     def _parse_blkio_metrics(self, stats):
         """Parse the blkio metrics."""

config.py

@@ -1123,7 +1123,7 @@ def load_check(agentConfig, hostname, checkname):
 
             # try to load the check and return the result
             load_success, load_failure = load_check_from_places(check_config, check_name, checks_places, agentConfig)
-            return load_success.values()[0] or load_failure
+            return load_success.values()[0] if load_success else load_failure
 
     return None
 

tests/core/test_service_discovery.py

@@ -2,6 +2,7 @@
 import copy
 import mock
 import unittest
+from collections import defaultdict
 
 # 3p
 from nose.plugins.attrib import attr
@@ -11,7 +12,8 @@
 from utils.service_discovery.config_stores import get_config_store
 from utils.service_discovery.consul_config_store import ConsulStore
 from utils.service_discovery.etcd_config_store import EtcdStore
-from utils.service_discovery.abstract_config_store import AbstractConfigStore, CONFIG_FROM_KUBE
+from utils.service_discovery.abstract_config_store import AbstractConfigStore, \
+    _TemplateCache, CONFIG_FROM_KUBE, CONFIG_FROM_TEMPLATE, CONFIG_FROM_AUTOCONF
 from utils.service_discovery.sd_backend import get_sd_backend
 from utils.service_discovery.sd_docker_backend import SDDockerBackend, _SDDockerBackendConfigFetchState
 
@@ -66,11 +68,11 @@ def client_read(path, **kwargs):
     if 'all' in kwargs:
         return {}
     else:
-        return TestServiceDiscovery.mock_tpls.get(image)[0][config_parts.index(config_part)]
+        return TestServiceDiscovery.mock_raw_templates.get(image)[0][config_parts.index(config_part)]
 
 
 def issue_read(identifier):
-    return TestServiceDiscovery.mock_tpls.get(identifier)
+    return TestServiceDiscovery.mock_raw_templates.get(identifier)
 
 @attr('unix')
 class TestServiceDiscovery(unittest.TestCase):
@@ -122,7 +124,7 @@ class TestServiceDiscovery(unittest.TestCase):
     }
 
     # raw templates coming straight from the config store
-    mock_tpls = {
+    mock_raw_templates = {
         # image_name: ('[check_name]', '[init_tpl]', '[instance_tpl]', expected_python_tpl_list)
         'image_0': (
             ('["check_0"]', '[{}]', '[{"host": "%%host%%"}]'),
@@ -509,12 +511,12 @@ def test_fill_tpl(self, *args):
     def test_get_auto_config(self):
         """Test _get_auto_config"""
         expected_tpl = {
-            'redis': ('redisdb', None, {"host": "%%host%%", "port": "%%port%%"}),
-            'consul': ('consul', None, {
-                "url": "http://%%host%%:%%port%%", "catalog_checks": True, "new_leader_checks": True
-            }),
-            'redis:v1': ('redisdb', None, {"host": "%%host%%", "port": "%%port%%"}),
-            'foobar': None
+            'redis': [('redisdb', None, {"host": "%%host%%", "port": "%%port%%"})],
+            'consul': [('consul', None, {
+                        "url": "http://%%host%%:%%port%%", "catalog_checks": True, "new_leader_checks": True
+                        })],
+            'redis:v1': [('redisdb', None, {"host": "%%host%%", "port": "%%port%%"})],
+            'foobar': []
         }
 
         config_store = get_config_store(self.auto_conf_agentConfig)
@@ -529,10 +531,10 @@ def test_get_check_tpls(self, mock_client_read):
         invalid_config = ['bad_image_0', 'bad_image_1']
         config_store = get_config_store(self.auto_conf_agentConfig)
         for image in valid_config:
-            tpl = self.mock_tpls.get(image)[1]
+            tpl = self.mock_raw_templates.get(image)[1]
             self.assertEquals(tpl, config_store.get_check_tpls(image))
         for image in invalid_config:
-            tpl = self.mock_tpls.get(image)[1]
+            tpl = self.mock_raw_templates.get(image)[1]
             self.assertEquals(tpl, config_store.get_check_tpls(image))
 
     @mock.patch.object(AbstractConfigStore, 'client_read', side_effect=client_read)
@@ -542,7 +544,7 @@ def test_get_check_tpls_kube(self, mock_client_read):
         invalid_config = ['bad_image_0']
         config_store = get_config_store(self.auto_conf_agentConfig)
         for image in valid_config + invalid_config:
-            tpl = self.mock_tpls.get(image)[1]
+            tpl = self.mock_raw_templates.get(image)[1]
             tpl = [(CONFIG_FROM_KUBE, t[1]) for t in tpl]
             if tpl:
                 self.assertNotEquals(
@@ -558,7 +560,7 @@ def test_get_check_tpls_kube(self, mock_client_read):
                         ['service-discovery.datadoghq.com/foo.check_names',
                          'service-discovery.datadoghq.com/foo.init_configs',
                          'service-discovery.datadoghq.com/foo.instances'],
-                        self.mock_tpls[image][0]))))
+                        self.mock_raw_templates[image][0]))))
 
     def test_get_config_id(self):
         """Test get_config_id"""
@@ -570,8 +572,8 @@ def test_get_config_id(self):
                     expected_ident)
                 clear_singletons(self.auto_conf_agentConfig)
 
-    @mock.patch.object(AbstractConfigStore, '_issue_read', side_effect=issue_read)
-    def test_read_config_from_store(self, issue_read):
+    @mock.patch.object(_TemplateCache, '_issue_read', side_effect=issue_read)
+    def test_read_config_from_store(self, args):
         """Test read_config_from_store"""
         valid_idents = [('nginx', 'nginx'), ('nginx:latest', 'nginx:latest'),
                         ('custom-nginx', 'custom-nginx'), ('custom-nginx:latest', 'custom-nginx'),
@@ -582,7 +584,13 @@ def test_read_config_from_store(self, issue_read):
         for ident, expected_key in valid_idents:
             tpl = config_store.read_config_from_store(ident)
             # source is added after reading from the store
-            self.assertEquals(tpl, ('template',) + self.mock_tpls.get(expected_key))
+            self.assertEquals(
+                tpl,
+                {
+                    CONFIG_FROM_AUTOCONF: None,
+                    CONFIG_FROM_TEMPLATE: self.mock_raw_templates.get(expected_key)
+                }
+            )
         for ident in invalid_idents:
             self.assertEquals(config_store.read_config_from_store(ident), [])
 
@@ -600,3 +608,72 @@ def test_read_jmx_config_from_store(self, *args):
         for check in self.jmx_sd_configs:
             key = '{}_0'.format(check)
             self.assertEquals(jmx_configs[key], valid_configs[key])
+
+    # Template cache
+    @mock.patch('utils.service_discovery.abstract_config_store.get_auto_conf_images')
+    def test_populate_auto_conf(self, mock_get_auto_conf_images):
+        """test _populate_auto_conf"""
+        auto_tpls = {
+            'foo': [['check0', 'check1'], [{}, {}], [{}, {}]],
+            'bar': [['check2', 'check3', 'check3'], [{}, {}, {}], [{}, {'foo': 'bar'}, {'bar': 'foo'}]],
+        }
+        cache = _TemplateCache(issue_read, '')
+        cache.auto_conf_templates = defaultdict(lambda: [[]] * 3)
+        mock_get_auto_conf_images.return_value = auto_tpls
+
+        cache._populate_auto_conf()
+        self.assertEquals(cache.auto_conf_templates['foo'], auto_tpls['foo'])
+        self.assertEquals(cache.auto_conf_templates['bar'],
+            [['check2', 'check3'], [{}, {}], [{}, {'foo': 'bar'}]])
+
+    @mock.patch.object(_TemplateCache, '_issue_read', return_value=None)
+    def test_get_templates(self, args):
+        """test get_templates"""
+        kv_tpls = {
+            'foo': [['check0', 'check1'], [{}, {}], [{}, {}]],
+            'bar': [['check2', 'check3'], [{}, {}], [{}, {}]],
+        }
+        auto_tpls = {
+            'foo': [['check3', 'check5'], [{}, {}], [{}, {}]],
+            'bar': [['check2', 'check6'], [{}, {}], [{}, {}]],
+            'foobar': [['check4'], [{}], [{}]],
+        }
+        cache = _TemplateCache(issue_read, '')
+        cache.kv_templates = kv_tpls
+        cache.auto_conf_templates = auto_tpls
+        self.assertEquals(cache.get_templates('foo'),
+            {CONFIG_FROM_TEMPLATE: [['check0', 'check1'], [{}, {}], [{}, {}]],
+                CONFIG_FROM_AUTOCONF: [['check3', 'check5'], [{}, {}], [{}, {}]]}
+        )
+
+        self.assertEquals(cache.get_templates('bar'),
+            # check3 must come from template not autoconf
+            {CONFIG_FROM_TEMPLATE: [['check2', 'check3'], [{}, {}], [{}, {}]],
+                CONFIG_FROM_AUTOCONF: [['check6'], [{}], [{}]]}
+        )
+
+        self.assertEquals(cache.get_templates('foobar'),
+            {CONFIG_FROM_TEMPLATE: None,
+                CONFIG_FROM_AUTOCONF: [['check4'], [{}], [{}]]}
+        )
+
+        self.assertEquals(cache.get_templates('baz'), None)
+
+    def test_get_check_names(self):
+        """Test get_check_names"""
+        kv_tpls = {
+            'foo': [['check0', 'check1'], [{}, {}], [{}, {}]],
+            'bar': [['check2', 'check3'], [{}, {}], [{}, {}]],
+        }
+        auto_tpls = {
+            'foo': [['check4', 'check5'], [{}, {}], [{}, {}]],
+            'bar': [['check2', 'check6'], [{}, {}], [{}, {}]],
+            'foobar': None,
+        }
+        cache = _TemplateCache(issue_read, '')
+        cache.kv_templates = kv_tpls
+        cache.auto_conf_templates = auto_tpls
+        self.assertEquals(cache.get_check_names('foo'), set(['check0', 'check1', 'check4', 'check5']))
+        self.assertEquals(cache.get_check_names('bar'), set(['check2', 'check3', 'check6']))
+        self.assertEquals(cache.get_check_names('foobar'), set())
+        self.assertEquals(cache.get_check_names('baz'), set())

utils/checkfiles.py

@@ -50,7 +50,7 @@ def get_check_class(agentConfig, check_name):
     return None
 
 
-def get_auto_conf(agentConfig, check_name):
+def get_auto_conf(check_name):
     """Return the yaml auto_config dict for a check name (None if it doesn't exist)."""
     from config import PathNotFound, get_auto_confd_path
 
@@ -75,11 +75,11 @@ def get_auto_conf(agentConfig, check_name):
     return auto_conf
 
 
-def get_auto_conf_images(agentConfig):
+def get_auto_conf_images(full_tpl=False):
     """Walk through the auto_config folder and build a dict of auto-configurable images."""
     from config import PathNotFound, get_auto_confd_path
     auto_conf_images = {
-        # image_name: check_name
+        # image_name: [check_names] or [[check_names], [init_tpls], [instance_tpls]]
     }
 
     try:
@@ -100,5 +100,18 @@ def get_auto_conf_images(agentConfig):
         # extract the supported image list
         images = auto_conf.get('docker_images', [])
         for image in images:
-            auto_conf_images[image] = check_name
+            if full_tpl:
+                init_tpl = auto_conf.get('init_config') or {}
+                instance_tpl = auto_conf.get('instances', [])
+                if image not in auto_conf_images:
+                    auto_conf_images[image] = [[check_name], [init_tpl], [instance_tpl]]
+                else:
+                    for idx, item in enumerate([check_name, init_tpl, instance_tpl]):
+                        auto_conf_images[image][idx].append(item)
+            else:
+                if image in auto_conf_images:
+                    auto_conf_images[image].append(check_name)
+                else:
+                    auto_conf_images[image] = [check_name]
+
     return auto_conf_images

utils/configcheck.py

@@ -94,10 +94,10 @@ def print_templates(agentConfig):
         except Exception as ex:
             print("Failed to extract configuration templates from the backend:\n%s" % str(ex))
 
-        for img, tpl in templates.iteritems():
+        for ident, tpl in templates.iteritems():
             print(
-                "- Image %s:\n\tcheck names: %s\n\tinit_configs: %s\n\tinstances: %s" % (
-                    img,
+                "- Identifier %s:\n\tcheck names: %s\n\tinit_configs: %s\n\tinstances: %s" % (
+                    ident,
                     tpl.get('check_names'),
                     tpl.get('init_configs'),
                     tpl.get('instances'),