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

@@ -133,24 +133,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."""
@@ -217,10 +199,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))
@@ -277,20 +256,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()
@@ -367,7 +337,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))
 
@@ -559,26 +529,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"):
-        count = 0
-        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:
@@ -598,6 +574,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
@@ -63,43 +63,28 @@ instances:
     #
     # 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

@@ -291,7 +291,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)
 
@@ -346,7 +347,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)
 
@@ -408,7 +410,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:
@@ -473,7 +476,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)
@@ -514,7 +519,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:
@@ -563,7 +569,8 @@ def test_healthcheck(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)
         self.assertServiceCheck('docker.container_health', count=0)