Fix tests, refactor how we collect container and volume states

Author: hkaj

.travis.yml

@@ -35,6 +35,9 @@ env:
     - TRAVIS_FLAVOR=cassandra FLAVOR_VERSION=2.0.13
     - TRAVIS_FLAVOR=cassandra FLAVOR_VERSION=2.1.3
     - TRAVIS_FLAVOR=couchdb
+    # FIXME: cannot enable docker on Travis
+    # because it needs docker and we run tests in a container
+    # - TRAVIS_FLAVOR=docker_daemon
     - TRAVIS_FLAVOR=elasticsearch FLAVOR_VERSION=0.90.13
     - TRAVIS_FLAVOR=elasticsearch FLAVOR_VERSION=1.0.3
     - TRAVIS_FLAVOR=elasticsearch FLAVOR_VERSION=1.1.2

checks.d/docker_daemon.py

@@ -134,24 +134,6 @@ def compile_filter_rules(rules):
 
     return patterns, tag_names
 
-def get_filters(include, exclude):
-    # The reasoning is to check exclude first, so we can skip if there is no exclude
-    if not exclude:
-        return
-
-    filtered_tag_names = []
-    exclude_patterns = []
-    include_patterns = []
-
-    # Compile regex
-    exclude_patterns, tag_names = compile_filter_rules(exclude)
-    filtered_tag_names.extend(tag_names)
-
-    include_patterns, tag_names = compile_filter_rules(include)
-    filtered_tag_names.extend(tag_names)
-
-    return set(exclude_patterns), set(include_patterns), set(filtered_tag_names)
-
 
 class DockerDaemon(AgentCheck):
     """Collect metrics and events from Docker API and cgroups."""
@@ -219,10 +201,7 @@ def init(self):
             self.collect_image_stats = _is_affirmative(instance.get('collect_images_stats', False))
             self.collect_container_size = _is_affirmative(instance.get('collect_container_size', False))
             self.collect_container_count = _is_affirmative(instance.get('collect_container_count', False))
-            self.collect_dead_container_count = _is_affirmative(instance.get('collect_dead_container_count', False))
-            self.collect_exited_container_count = _is_affirmative(instance.get('collect_exited_container_count', False))
             self.collect_volume_count = _is_affirmative(instance.get('collect_volume_count', False))
-            self.collect_dangling_volume_count = _is_affirmative(instance.get('collect_dangling_volume_count', False))
             self.collect_events = _is_affirmative(instance.get('collect_events', True))
             self.collect_image_size = _is_affirmative(instance.get('collect_image_size', False))
             self.collect_disk_stats = _is_affirmative(instance.get('collect_disk_stats', False))
@@ -279,20 +258,11 @@ def check(self, instance):
             self._report_container_size(containers_by_id)
 
         if self.collect_container_count:
-            self._report_container_count_by_state(containers_by_id)
-
-        if self.collect_dead_container_count:
-            self._report_container_count_by_state(containers_by_id, state="Dead")
-
-        if self.collect_exited_container_count:
-            self._report_container_count_by_state(containers_by_id, state="Exited")
+            self._report_container_count(containers_by_id)
 
         if self.collect_volume_count:
             self._report_volume_count()
 
-        if self.collect_dangling_volume_count:
-            self._report_volume_count(filters={'dangling': True})
-
         # Collect disk stats from Docker info command
         if self.collect_disk_stats:
             self._report_disk_stats()
@@ -369,7 +339,7 @@ def _get_and_count_containers(self, custom_cgroups=False, healthchecks=False):
                 except Exception as e:
                     self.log.debug("Unable to inspect Docker container: %s", e)
 
-
+        # TODO: deprecate these 2, they should be replaced by _report_container_count
         for tags, count in running_containers_count.iteritems():
             self.gauge("docker.containers.running", count, tags=list(tags))
 
@@ -561,25 +531,32 @@ def _submit_healthcheck_sc(self, container):
         tags = self._get_tags(container, CONTAINER)
         self.service_check(HEALTHCHECK_SERVICE_CHECK_NAME, status, tags=tags)
 
-    def _report_container_count_by_state(self, containers_by_id, state="Any"):
-        tags = {}
-        filterlambda = lambda x: not self._is_container_excluded(x) and state is "Any" or container["State"] is state
-        filtered = list(filter(filterlambda, containers_by_id))
-        tags = self._get_tags(filtered[0], PERFORMANCE)
-
+    def _report_container_count(self, containers_by_id):
+        """Report container count per state"""
         m_func = FUNC_MAP[GAUGE][self.use_histogram]
-        # Report docker.container.count if state is "Any", otherwise
-        # report docker.container.state_STATE.count
-        suffix = ".state_{}".format(state.lower()) if state is not "Any" else ""
-        m_func(self, 'docker.container{}.count'.format(suffix), len(filtered), tags=tags)
 
-    def _report_volume_count(self, filters={}):
-        volumes = self.docker_client.volumes(filters=filters)
-        count = len(volumes['Volumes'])
+        per_state_count = defaultdict(int)
+
+        filterlambda = lambda ctr: not self._is_container_excluded(ctr)
+        containers = list(filter(filterlambda, containers_by_id.values()))
+
+        for ctr in containers:
+            per_state_count[ctr.get('State', '')] += 1
 
+        for state in per_state_count:
+            if state:
+                m_func(self, 'docker.container.count', per_state_count[state], tags=['container_state:%s' % state.lower()])
+
+    def _report_volume_count(self):
+        """Report volume count per state (dangling or not)"""
         m_func = FUNC_MAP[GAUGE][self.use_histogram]
-        suffix = '.' + '-'.join(sorted(filters.keys())) if len(filters) is not 0 else ''
-        m_func(self, 'docker.volumes{}.count'.format(suffix), count)
+
+        attached_volumes = self.docker_client.volumes(filters={'dangling': False})
+        dangling_volumes = self.docker_client.volumes(filters={'dangling': True})
+        attached_count = len(attached_volumes['Volumes'])
+        dangling_count = len(dangling_volumes['Volumes'])
+        m_func(self, 'docker.volume.count', attached_count, tags=['volume_state:attached'])
+        m_func(self, 'docker.volume.count', dangling_count, tags=['volume_state:dangling'])
 
     def _report_image_size(self, images):
         for image in images:
@@ -599,6 +576,7 @@ def _report_performance_metrics(self, containers_by_id):
                 continue
 
             tags = self._get_tags(container, PERFORMANCE)
+
             self._report_cgroup_metrics(container, tags)
             if "_proc_root" not in container:
                 containers_without_proc_root.append(DockerUtil.container_name_extractor(container)[0])

conf.d/docker_daemon.yaml.example

@@ -43,7 +43,7 @@ instances:
     # ensure that `docker ps -a -q` run fast before enabling it.
     # Defaults to false.
     #
-    # collect_container_size: false
+    # collect_container_size: true
 
     # Do you use custom cgroups for this particular instance?
     # Note: enabling this option modifies the way in which we inspect the containers and causes
@@ -58,48 +58,33 @@ instances:
     #       versions will result in an UNKNOWN state for the service check.
     #
     # You should whitelist the container images you wish to submit health service checks for.
-    # Example: ["tomcat", "nginx", "etcd"]
+    # Example: ["image_name:tomcat", "image_name:nginx", "image_name:etcd"]
     #
     # health_service_checks: false
     # health_service_check_whitelist: []
 
-    # Collect the total container count with the docker.containers.count metric.
+    # Collect the container count tagged by state (running, paused, exited, dead)
     # Defaults to false.
     #
-    # collect_container_count: false
+    # collect_container_count: true
 
-    # Collect the count of all containers in Dead state with the docker.container.state_dead.count metric.
+    # Collect the volume count for attached and dangling volumes.
     # Defaults to false.
     #
-    # collect_dead_container_count: false
-
-    # Collect the count of all containers in Exited state with the docker.container.state_exited.count metric.
-    # Defaults to false.
-    #
-    # collect_exited_container_count: false
-
-    # Collect the total volume count with the docker.volumes.count metric.
-    # Defaults to false.
-    #
-    # collect_volume_count: false
-
-    # Collect the count of all dangling volumes with the docker.volumes.dangling.count metric.
-    # Defaults to false.
-    #
-    # collect_dangling_volume_count: false
+    # collect_volume_count: true
 
     # Collect images stats
     # Number of available active images and intermediate images as gauges.
     # Defaults to false.
     #
-    # collect_images_stats: false
+    # collect_images_stats: true
 
     # Collect disk usage per image with docker.image.size and docker.image.virtual_size metrics.
     # The check gets this size with the `docker images` command.
     # Requires collect_images_stats to be enabled.
     # Defaults to false.
     #
-    # collect_image_size: false
+    # collect_image_size: true
 
     # Collect disk metrics (total, used, free) through the docker info command for data and metadata.
     # This is useful when these values can't be obtained by the disk check.

tests/checks/integration/test_docker_daemon.py

@@ -73,9 +73,9 @@ def mock_get_info_no_data(self):
     def mock_get_info_invalid_values(self):
         return {
             'DriverStatus': [
-                ['Metadata Space Available', '9 MB'],
-                ['Metadata Space Total', '10 MB'],
-                ['Metadata Space Used', '11 MB'],
+                ['Metadata Space Available', '0 MB'],
+                ['Metadata Space Total', '0 MB'],
+                ['Metadata Space Used', '0 MB'],
             ]
         }
 
@@ -132,9 +132,9 @@ def test_devicemapper_invalid_values(self, mock_info):
 
         self.run_check(MOCK_CONFIG, force_reload=True)
         metric_names = [metric[0] for metric in self.metrics]
-        self.assertMetric('docker.metadata.free', value=9e6)
-        self.assertMetric('docker.metadata.used', value=11e6)
-        self.assertMetric('docker.metadata.total', value=10e6)
+        self.assertMetric('docker.metadata.free', value=0)
+        self.assertMetric('docker.metadata.used', value=0)
+        self.assertMetric('docker.metadata.total', value=0)
         self.assertNotIn('docker.metadata.percent', metric_names)
 
     @mock.patch('docker.Client.info')
@@ -292,7 +292,8 @@ def test_exclude_filter(self):
             },
             ],
         }
-        DockerUtil().set_docker_settings(config['init_config'], config['instances'][0])
+        DockerUtil._drop()
+        DockerUtil(init_config=config['init_config'], instance=config['instances'][0])
 
         self.run_check_twice(config, force_reload=True)
 
@@ -347,7 +348,8 @@ def test_include_filter(self):
             },
             ],
         }
-        DockerUtil().set_docker_settings(config['init_config'], config['instances'][0])
+        DockerUtil._drop()
+        DockerUtil(init_config=config['init_config'], instance=config['instances'][0])
 
         self.run_check_twice(config, force_reload=True)
 
@@ -409,7 +411,8 @@ def test_tags_options(self):
             },
             ],
         }
-        DockerUtil().set_docker_settings(config['init_config'], config['instances'][0])
+        DockerUtil._drop()
+        DockerUtil(init_config=config['init_config'], instance=config['instances'][0])
 
         self.run_check_twice(config, force_reload=True)
         for mname, tags in expected_metrics:
@@ -474,7 +477,9 @@ def test_labels_collection(self):
             },
             ],
         }
-        DockerUtil().set_docker_settings(config['init_config'], config['instances'][0])
+        DockerUtil._drop()
+        DockerUtil(init_config=config['init_config'], instance=config['instances'][0])
+
         self.run_check(config, force_reload=True)
         for mname, tags in expected_metrics:
             self.assertMetric(mname, tags=tags, count=1, at_least=1)
@@ -515,7 +520,8 @@ def test_histogram(self):
             },
             ],
         }
-        DockerUtil().set_docker_settings(config['init_config'], config['instances'][0])
+        DockerUtil._drop()
+        DockerUtil(init_config=config['init_config'], instance=config['instances'][0])
 
         self.run_check(config, force_reload=True)
         for mname, tags in expected_metrics:
@@ -547,11 +553,13 @@ def test_healthcheck(self):
                 "url": "unix://var/run/docker.sock",
                 "collect_images_stats": True,
                 "health_service_checks": True,
+                "health_service_check_whitelist": ["image_name:.*"]
             },
             ],
         }
 
-        DockerUtil().set_docker_settings(config['init_config'], config['instances'][0])
+        DockerUtil._drop()
+        DockerUtil(init_config=config['init_config'], instance=config['instances'][0])
 
         self.run_check(config, force_reload=True)
         self.assertServiceCheck('docker.container_health', at_least=2)