Use all option from psutil.disk_partitions

This does the same that this PR intended but supporting all operating systems
and by checking (on Linux) if the file system is backed by a block device.

Author: mx-psi

disk/assets/configuration/spec.yaml

@@ -42,9 +42,9 @@ files:
               start matching from the beginning and therefore to match anywhere you
               must prepend `.*`. For exact matches append `$`.
 
-              Some common Linux special file systems can be excluded by enabling the
-              `exclude_special_file_systems` option.
-              
+              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:
@@ -54,31 +54,6 @@ files:
               type: array
               items:
                 type: string
-          - name: exclude_special_file_systems
-            description: |
-              Instruct the check to not collect from some common special file systems.
-              Works on Linux only.
-
-              The special file systems excluded with this option are:
-                - binfmt_misc
-                - configfs
-                - debugfs
-                - devtmpfs
-                - overlay
-                - proc
-                - rootfs
-                - securityfs
-                - sysfs
-                - tmpfs
-                - tracefs
-
-              These are added to the `file_system_blacklist`.
-
-              For more fine-grained control, use `file_system_blacklist` and `file_system_whitelist`.
-            value:
-              example: true
-              type: boolean
-              default: false
           - name: device_whitelist
             description: |
                 Instruct the check to only collect from matching devices.
@@ -138,6 +113,16 @@ 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: false
+              type: boolean
+              default: true
           - name: service_check_rw
             description: |
               Instruct the check to notify based on partition state.

disk/datadog_checks/disk/disk.py

@@ -20,42 +20,13 @@
     # See: https://github.com/DataDog/integrations-core/pull/1109#discussion_r167133580
     IGNORE_CASE = re.I
 
-    KNOWN_SPECIAL_FILE_SYSTEMS = set()
-
     def _base_device_name(device):
         return device.strip('\\').lower()
 
 
-elif platform.system() == "Linux":
-    IGNORE_CASE = 0
-
-    # Remember to update the configuration specification
-    # if modifying the list of special file systems.
-    KNOWN_SPECIAL_FILE_SYSTEMS = set(
-        [
-            'binfmt_misc',
-            'configfs',
-            'debugfs',
-            'devtmpfs',
-            'overlay',
-            'proc',
-            'rootfs',
-            'securityfs',
-            'sysfs',
-            'tmpfs',
-            'tracefs',
-        ]
-    )
-
-    def _base_device_name(device):
-        return os.path.basename(device)
-
-
 else:
     IGNORE_CASE = 0
 
-    KNOWN_SPECIAL_FILE_SYSTEMS = set()
-
     def _base_device_name(device):
         return os.path.basename(device)
 
@@ -77,7 +48,7 @@ def __init__(self, name, init_config, instances):
         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._exclude_special_file_systems = instance.get('exclude_special_file_systems', False)
+        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', [])
@@ -102,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
@@ -119,21 +90,12 @@ def check(self, instance):
                 )
                 continue
             except Exception as e:
-                if not self._exclude_special_file_systems and part.fstype in KNOWN_SPECIAL_FILE_SYSTEMS:
-                    self.log.warning(
-                        u'Unable to get disk metrics for %s with special file system %s: %s. '
-                        u'Enable `exclude_special_file_systems` to ignore common special file systems.',
-                        part.mountpoint,
-                        part.fstype,
-                        e,
-                    )
-                else:
-                    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,
-                    )
+                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
@@ -234,9 +196,7 @@ def _file_system_blacklisted(self, file_system):
         if self._file_system_blacklist is None:
             return False
 
-        return (self._exclude_special_file_systems and file_system in KNOWN_SPECIAL_FILE_SYSTEMS) or bool(
-            self._file_system_blacklist.match(file_system)
-        )
+        return not not self._file_system_blacklist.match(file_system)
 
     def _device_whitelisted(self, device):
         if not device or self._device_whitelist is None:

disk/tests/test_filter.py

@@ -7,7 +7,7 @@
 from datadog_checks.disk.disk import IGNORE_CASE, Disk
 
 from .mocks import MockPart
-from .utils import assert_regex_equal, requires_linux, requires_windows
+from .utils import assert_regex_equal, requires_windows
 
 
 def test_default_casing():
@@ -104,16 +104,6 @@ def test_file_system_whitelist_blacklist():
     assert c.exclude_disk(MockPart(fstype='NTFS')) is True
 
 
-@requires_linux
-def test_exclude_special_file_system():
-    instance = {'exclude_special_file_systems': True}
-    c = Disk('disk', {}, [instance])
-
-    assert c.exclude_disk(MockPart(fstype='debugfs')) is True
-    assert c.exclude_disk(MockPart(fstype='tmpfs')) is True
-    assert c.exclude_disk(MockPart(fstype='ext4')) is False
-
-
 def test_device_whitelist():
     instance = {'device_whitelist': ['/dev/sda[1-3]', 'c:']}
     c = Disk('disk', {}, [instance])

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)