[disk] Add include_all_devices option and improve error logs (#7378)

Using `all` option from `psutil.disk_partitions`

Author: mx-psi

disk/assets/configuration/spec.yaml

@@ -42,6 +42,9 @@ files:
               start matching from the beginning and therefore to match anywhere you
               must prepend `.*`. For exact matches append `$`.
 
+              Devices from pseudo or memory-based file systems can be excluded by disabling the
+              `include_all_devices` option.
+
               When conflicts arise, this will override `file_system_whitelist`.
             value:
               example:
@@ -110,6 +113,15 @@ files:
               type: array
               items:
                 type: string
+          - name: include_all_devices
+            description: |
+              Instruct the check to collect from all devices, including non-physical devices.
+              Set this to false to exclude pseudo, memory, duplicate or inaccessible file systems.
+
+              For more fine-grained control, use the inclusion and exclusion options.
+            value:
+              example: true
+              type: boolean
           - name: service_check_rw
             description: |
               Instruct the check to notify based on partition state.

disk/datadog_checks/disk/data/conf.yaml.default

@@ -44,6 +44,9 @@ instances:
     ## start matching from the beginning and therefore to match anywhere you
     ## must prepend `.*`. For exact matches append `$`.
     ##
+    ## Devices from pseudo or memory-based file systems can be excluded by disabling the
+    ## `include_all_devices` option.
+    ##
     ## When conflicts arise, this will override `file_system_whitelist`.
     #
     # file_system_blacklist:
@@ -98,6 +101,14 @@ instances:
     #   - /dev/sde
     #   - '[FJ]:'
 
+    ## @param include_all_devices - boolean - optional - default: true
+    ## Instruct the check to collect from all devices, including non-physical devices.
+    ## Set this to false to exclude pseudo, memory, duplicate or inaccessible file systems.
+    ##
+    ## For more fine-grained control, use the inclusion and exclusion options.
+    #
+    # include_all_devices: true
+
     ## @param service_check_rw - boolean - optional - default: false
     ## Instruct the check to notify based on partition state.
     ##

disk/datadog_checks/disk/disk.py

@@ -47,6 +47,8 @@ def __init__(self, name, init_config, instances):
         self._all_partitions = is_affirmative(instance.get('all_partitions', False))
         self._file_system_whitelist = instance.get('file_system_whitelist', [])
         self._file_system_blacklist = instance.get('file_system_blacklist', [])
+        # FIXME (8.X): Exclude special file systems by default
+        self._include_all_devices = is_affirmative(instance.get('include_all_devices', True))
         self._device_whitelist = instance.get('device_whitelist', [])
         self._device_blacklist = instance.get('device_blacklist', [])
         self._mount_point_whitelist = instance.get('mount_point_whitelist', [])
@@ -71,7 +73,7 @@ def check(self, instance):
             self.devices_label = self._get_devices_label()
 
         self._valid_disks = {}
-        for part in psutil.disk_partitions(all=True):
+        for part in psutil.disk_partitions(all=self._include_all_devices):
             # we check all exclude conditions
             if self.exclude_disk(part):
                 continue
@@ -88,7 +90,12 @@ def check(self, instance):
                 )
                 continue
             except Exception as e:
-                self.log.warning('Unable to get disk metrics for %s: %s', part.mountpoint, e)
+                self.log.warning(
+                    u'Unable to get disk metrics for %s: %s. '
+                    u'You can exclude this mountpoint in the settings if it is invalid.',
+                    part.mountpoint,
+                    e,
+                )
                 continue
 
             # Exclude disks with size less than min_disk_size
@@ -147,7 +154,6 @@ def _exclude_disk(self, device, file_system, mount_point):
         """
         Return True for disks we don't want or that match regex in the config file
         """
-        self.log.debug('_exclude_disk: %s, %s, %s', device, file_system, mount_point)
 
         if not device or device == 'none':
             device = None
@@ -245,7 +251,12 @@ def _collect_inodes_metrics(self, mountpoint):
             )
             return metrics
         except Exception as e:
-            self.log.warning('Unable to get disk metrics for %s: %s', mountpoint, e)
+            self.log.warning(
+                u'Unable to get disk metrics for %s: %s. '
+                u'You can exclude this mountpoint in the settings if it is invalid.',
+                mountpoint,
+                e,
+            )
             return metrics
 
         if inodes.f_files != 0:

disk/tests/test_unit.py

@@ -271,3 +271,20 @@ def f(mountpoint):
         aggregator.assert_metric_has_tag(name, 'device_name:{}'.format(DEFAULT_DEVICE_BASE_NAME))
 
     aggregator.assert_all_metrics_covered()
+
+
+@pytest.mark.usefixtures('psutil_mocks')
+def test_include_all_devices(aggregator, gauge_metrics, rate_metrics):
+    c = Disk('disk', {}, [{}])
+
+    with mock.patch('psutil.disk_partitions', return_value=[]) as m:
+        c.check({})
+        # By default, we include all devices
+        m.assert_called_with(all=True)
+
+    instance = {'include_all_devices': False}
+    c = Disk('disk', {}, [instance])
+
+    with mock.patch('psutil.disk_partitions', return_value=[]) as m:
+        c.check({})
+        m.assert_called_with(all=False)