[subprocess] if output is expected, and there is none, make sure an e…

…xception is raised.
DataDog · Jan 22, 2016 · 449e94a · 449e94a
1 parent 5a9a606
commit 449e94a
Show file tree

Hide file tree

Showing 8 changed files with 46 additions and 36 deletions.
diff --git a/checks.d/disk.py b/checks.d/disk.py
@@ -176,7 +176,7 @@ def collect_latency_metrics(self):
 
     # no psutil, let's use df
     def collect_metrics_manually(self):
-        df_out, _, _ = get_subprocess_output(self.DF_COMMAND + ['-k'], self.log)
+        df_out, _, _ = get_subprocess_output(self.DF_COMMAND + ['-k'], self.log, output_expected=True)
         self.log.debug(df_out)
         for device in self._list_devices(df_out):
             self.log.debug("Passed: {0}".format(device))

diff --git a/checks.d/network.py b/checks.d/network.py
@@ -143,7 +143,7 @@ def _check_linux(self, instance):
                 for ip_version in ['4', '6']:
                     # Call `ss` for each IP version because there's no built-in way of distinguishing
                     # between the IP versions in the output
-                    output, _, _ = get_subprocess_output(["ss", "-n", "-u", "-t", "-a", "-{0}".format(ip_version)], self.log)
+                    output, _, _ = get_subprocess_output(["ss", "-n", "-u", "-t", "-a", "-{0}".format(ip_version)], self.log, output_expected=True)
                     lines = output.splitlines()
                     # Netid  State      Recv-Q Send-Q     Local Address:Port       Peer Address:Port
                     # udp    UNCONN     0      0              127.0.0.1:8125                  *:*
@@ -161,7 +161,7 @@ def _check_linux(self, instance):
 
             except OSError:
                 self.log.info("`ss` not found: using `netstat` as a fallback")
-                output, _, _ = get_subprocess_output(["netstat", "-n", "-u", "-t", "-a"], self.log)
+                output, _, _ = get_subprocess_output(["netstat", "-n", "-u", "-t", "-a"], self.log, output_expected=True)
                 lines = output.splitlines()
                 # Active Internet connections (w/o servers)
                 # Proto Recv-Q Send-Q Local Address           Foreign Address         State
@@ -176,6 +176,8 @@ def _check_linux(self, instance):
                 metrics = self._parse_linux_cx_state(lines[2:], self.TCP_STATES['netstat'], 5)
                 for metric, value in metrics.iteritems():
                     self.gauge(metric, value)
+            except Exception:
+                self.log.exception("Error collecting connection stats.")
 
         proc = open('/proc/net/dev', 'r')
         try:
@@ -289,7 +291,7 @@ def _check_bsd(self, instance):
         if Platform.is_freebsd():
             netstat_flags.append('-W')
 
-        output, _, _ = get_subprocess_output(["netstat"] + netstat_flags, self.log)
+        output, _, _ = get_subprocess_output(["netstat"] + netstat_flags, self.log, output_expected=True)
         lines = output.splitlines()
         # Name  Mtu   Network       Address            Ipkts Ierrs     Ibytes    Opkts Oerrs     Obytes  Coll
         # lo0   16384 <Link#1>                        318258     0  428252203   318258     0  428252203     0
@@ -353,7 +355,7 @@ def _check_bsd(self, instance):
                 self._submit_devicemetrics(iface, metrics)
 
 
-        netstat, _, _ = get_subprocess_output(["netstat", "-s", "-p" "tcp"], self.log)
+        netstat, _, _ = get_subprocess_output(["netstat", "-s", "-p" "tcp"], self.log, output_expected=True)
         #3651535 packets sent
         #        972097 data packets (615753248 bytes)
         #        5009 data packets (2832232 bytes) retransmitted
@@ -378,12 +380,12 @@ def _check_bsd(self, instance):
     def _check_solaris(self, instance):
         # Can't get bytes sent and received via netstat
         # Default to kstat -p link:0:
-        netstat, _, _ = get_subprocess_output(["kstat", "-p", "link:0:"], self.log)
+        netstat, _, _ = get_subprocess_output(["kstat", "-p", "link:0:"], self.log, output_expected=True)
         metrics_by_interface = self._parse_solaris_netstat(netstat)
         for interface, metrics in metrics_by_interface.iteritems():
             self._submit_devicemetrics(interface, metrics)
 
-        netstat, _, _ = get_subprocess_output(["netstat", "-s", "-P" "tcp"], self.log)
+        netstat, _, _ = get_subprocess_output(["netstat", "-s", "-P" "tcp"], self.log, output_expected=True)
         # TCP: tcpRtoAlgorithm=     4 tcpRtoMin           =   200
         # tcpRtoMax           = 60000 tcpMaxConn          =    -1
         # tcpActiveOpens      =    57 tcpPassiveOpens     =    50

diff --git a/checks.d/postfix.py b/checks.d/postfix.py
@@ -56,7 +56,8 @@ def _get_queue_count(self, directory, queues, tags):
                 # can dd-agent user run sudo?
                 test_sudo = os.system('setsid sudo -l < /dev/null')
                 if test_sudo == 0:
-                    output, _, _ = get_subprocess_output(['sudo', 'find', queue_path, '-type', 'f'], self.log)
+                    output, _, _ = get_subprocess_output(['sudo', 'find', queue_path, '-type', 'f'],
+                                                         self.log, output_expected=True)
                     count = len(output.splitlines())
                 else:
                     raise Exception('The dd-agent user does not have sudo access')

diff --git a/checks/collector.py b/checks/collector.py
@@ -638,7 +638,7 @@ def _populate_payload_metadata(self, payload, check_statuses, start_event=True):
                     command = "gohai"
                 else:
                     command = "gohai\gohai.exe"
-                gohai_metadata, gohai_err, _ = get_subprocess_output([command], log)
+                gohai_metadata, gohai_err, _ = get_subprocess_output([command], log, output_expected=True)
                 payload['gohai'] = gohai_metadata
                 if gohai_err:
                     log.warning("GOHAI LOG | {0}".format(gohai_err))

diff --git a/checks/system/unix.py b/checks/system/unix.py
@@ -113,7 +113,7 @@ def check(self, agentConfig):
         io = {}
         try:
             if Platform.is_linux():
-                stdout, _, _ = get_subprocess_output(['iostat', '-d', '1', '2', '-x', '-k'], self.logger)
+                stdout, _, _ = get_subprocess_output(['iostat', '-d', '1', '2', '-x', '-k'], self.logger, output_expected=True)
 
                 #                 Linux 2.6.32-343-ec2 (ip-10-35-95-10)   12/11/2012      _x86_64_        (2 CPU)
                 #
@@ -133,7 +133,7 @@ def check(self, agentConfig):
                 io.update(self._parse_linux2(stdout))
 
             elif sys.platform == "sunos5":
-                output, _, _ = get_subprocess_output(["iostat", "-x", "-d", "1", "2"], self.logger)
+                output, _, _ = get_subprocess_output(["iostat", "-x", "-d", "1", "2"], self.logger, output_expected=True)
                 iostat = output.splitlines()
 
                 #                   extended device statistics <-- since boot
@@ -163,7 +163,7 @@ def check(self, agentConfig):
                         io[cols[0]][self.xlate(headers[i], "sunos")] = cols[i]
 
             elif sys.platform.startswith("freebsd"):
-                output, _, _ = get_subprocess_output(["iostat", "-x", "-d", "1", "2"], self.logger)
+                output, _, _ = get_subprocess_output(["iostat", "-x", "-d", "1", "2"], self.logger, output_expected=True)
                 iostat = output.splitlines()
 
                 # Be careful!
@@ -190,7 +190,7 @@ def check(self, agentConfig):
                     for i in range(1, len(cols)):
                         io[cols[0]][self.xlate(headers[i], "freebsd")] = cols[i]
             elif sys.platform == 'darwin':
-                iostat, _, _ = get_subprocess_output(['iostat', '-d', '-c', '2', '-w', '1'], self.logger)
+                iostat, _, _ = get_subprocess_output(['iostat', '-d', '-c', '2', '-w', '1'], self.logger, output_expected=True)
                 #          disk0           disk1          <-- number of disks
                 #    KB/t tps  MB/s     KB/t tps  MB/s
                 #   21.11  23  0.47    20.01   0  0.00
@@ -229,7 +229,7 @@ def check(self, agentConfig):
         elif sys.platform in ('darwin', 'sunos5') or sys.platform.startswith("freebsd"):
             # Get output from uptime
             try:
-                uptime, _, _ = get_subprocess_output(['uptime'], self.logger)
+                uptime, _, _ = get_subprocess_output(['uptime'], self.logger, output_expected=True)
             except Exception:
                 self.logger.exception('Cannot extract load')
                 return False
@@ -272,7 +272,7 @@ def __init__(self, logger):
         self.pagesize = 0
         if sys.platform == 'sunos5':
             try:
-                pgsz, _, _ = get_subprocess_output(['pagesize'], self.logger)
+                pgsz, _, _ = get_subprocess_output(['pagesize'], self.logger, output_expected=True)
                 self.pagesize = int(pgsz.strip())
             except Exception:
                 # No page size available
@@ -402,7 +402,7 @@ def check(self, agentConfig):
 
         elif sys.platform.startswith("freebsd"):
             try:
-                output, _, _ = get_subprocess_output(['sysctl', 'vm.stats.vm'], self.logger)
+                output, _, _ = get_subprocess_output(['sysctl', 'vm.stats.vm'], self.logger, output_expected=True)
                 sysctl = output.splitlines()
             except Exception:
                 self.logger.exception('getMemoryUsage')
@@ -457,7 +457,7 @@ def check(self, agentConfig):
 
             # Swap
             try:
-                output, _, _ = get_subprocess_output(['swapinfo', '-m'], self.logger)
+                output, _, _ = get_subprocess_output(['swapinfo', '-m'], self.logger, output_expected=True)
                 sysctl = output.splitlines()
             except Exception:
                 self.logger.exception('getMemoryUsage')
@@ -488,7 +488,7 @@ def check(self, agentConfig):
             try:
                 memData = {}
                 cmd = ["kstat", "-m", "memory_cap", "-c", "zone_memory_cap", "-p"]
-                output, _, _ = get_subprocess_output(cmd, self.logger)
+                output, _, _ = get_subprocess_output(cmd, self.logger, output_expected=True)
                 kmem = output.splitlines()
 
                 # memory_cap:360:53aa9b7e-48ba-4152-a52b-a6368c:anon_alloc_fail   0
@@ -542,7 +542,7 @@ def check(self, agentConfig):
             ps_arg = 'auxww'
         # Get output from ps
         try:
-            output, _, _ = get_subprocess_output(['ps', ps_arg], self.logger)
+            output, _, _ = get_subprocess_output(['ps', ps_arg], self.logger, output_expected=True)
             processLines = output.splitlines()  # Also removes a trailing empty line
         except StandardError:
             self.logger.exception('getProcesses')
@@ -586,7 +586,7 @@ def get_value(legend, data, name, filter_value=None):
                 return 0.0
         try:
             if Platform.is_linux():
-                output, _, _ = get_subprocess_output(['mpstat', '1', '3'], self.logger)
+                output, _, _ = get_subprocess_output(['mpstat', '1', '3'], self.logger, output_expected=True)
                 mpstat = output.splitlines()
                 # topdog@ip:~$ mpstat 1 3
                 # Linux 2.6.32-341-ec2 (ip)   01/19/2012  _x86_64_  (2 CPU)
@@ -647,7 +647,7 @@ def get_value(legend, data, name, filter_value=None):
             elif sys.platform == 'darwin':
                 # generate 3 seconds of data
                 # ['          disk0           disk1       cpu     load average', '    KB/t tps  MB/s     KB/t tps  MB/s  us sy id   1m   5m   15m', '   21.23  13  0.27    17.85   7  0.13  14  7 79  1.04 1.27 1.31', '    4.00   3  0.01     5.00   8  0.04  12 10 78  1.04 1.27 1.31', '']
-                iostats, _, _ = get_subprocess_output(['iostat', '-C', '-w', '3', '-c', '2'], self.logger)
+                iostats, _, _ = get_subprocess_output(['iostat', '-C', '-w', '3', '-c', '2'], self.logger, output_expected=True)
                 lines = [l for l in iostats.splitlines() if len(l) > 0]
                 legend = [l for l in lines if "us" in l]
                 if len(legend) == 1:
@@ -669,7 +669,7 @@ def get_value(legend, data, name, filter_value=None):
                 # tin  tout  KB/t tps  MB/s   KB/t tps  MB/s   KB/t tps  MB/s  us ni sy in id
                 # 0    69 26.71   0  0.01   0.00   0  0.00   0.00   0  0.00   2  0  0  1 97
                 # 0    78  0.00   0  0.00   0.00   0  0.00   0.00   0  0.00   0  0  0  0 100
-                iostats, _, _ = get_subprocess_output(['iostat', '-w', '3', '-c', '2'], self.logger)
+                iostats, _, _ = get_subprocess_output(['iostat', '-w', '3', '-c', '2'], self.logger, output_expected=True)
                 lines = [l for l in iostats.splitlines() if len(l) > 0]
                 legend = [l for l in lines if "us" in l]
                 if len(legend) == 1:
@@ -699,7 +699,7 @@ def get_value(legend, data, name, filter_value=None):
                 # http://docs.oracle.com/cd/E23824_01/html/821-1462/mpstat-1m.html
                 #
                 # Will aggregate over all processor sets
-                    output, _, _ = get_subprocess_output(['mpstat', '-aq', '1', '2'], self.logger)
+                    output, _, _ = get_subprocess_output(['mpstat', '-aq', '1', '2'], self.logger, output_expected=True)
                     mpstat = output.splitlines()
                     lines = [l for l in mpstat if len(l) > 0]
                     # discard the first len(lines)/2 lines

diff --git a/config.py b/config.py
@@ -570,17 +570,20 @@ def get_system_stats():
 
     platf = sys.platform
 
-    if Platform.is_linux(platf):
-        output, _, _ = get_subprocess_output(['grep', 'model name', '/proc/cpuinfo'], log)
-        systemStats['cpuCores'] = len(output.splitlines())
-
-    if Platform.is_darwin(platf):
-        output, _, _ = get_subprocess_output(['sysctl', 'hw.ncpu'], log)
-        systemStats['cpuCores'] = int(output.split(': ')[1])
-
-    if Platform.is_freebsd(platf):
-        output, _, _ = get_subprocess_output(['sysctl', 'hw.ncpu'], log)
-        systemStats['cpuCores'] = int(output.split(': ')[1])
+    try:
+        if Platform.is_linux(platf):
+            output, _, _ = get_subprocess_output(['grep', 'model name', '/proc/cpuinfo'], log, output_expected=True)
+            systemStats['cpuCores'] = len(output.splitlines())
+
+        if Platform.is_darwin(platf):
+            output, _, _ = get_subprocess_output(['sysctl', 'hw.ncpu'], log, output_expected=True)
+            systemStats['cpuCores'] = int(output.split(': ')[1])
+
+        if Platform.is_freebsd(platf):
+            output, _, _ = get_subprocess_output(['sysctl', 'hw.ncpu'], log, output_expected=True)
+            systemStats['cpuCores'] = int(output.split(': ')[1])
+    except Exception as e:
+        log.warning("unable to retrieve number of cpuCores. Failed with error %s" % str(e))
 
     if Platform.is_linux(platf):
         systemStats['nixV'] = platform.dist()

diff --git a/resources/processes.py b/resources/processes.py
@@ -36,7 +36,7 @@ def _get_proc_list(self):
                 ps_arg = 'aux'
             else:
                 ps_arg = 'auxww'
-            output, _, _ = get_subprocess_output(['ps', ps_arg], self.log)
+            output, _, _ = get_subprocess_output(['ps', ps_arg], self.log, output_expected=True)
             processLines = output.splitlines()  # Also removes a trailing empty line
         except Exception:
             self.log.exception('Cannot get process list')

diff --git a/utils/subprocess_output.py b/utils/subprocess_output.py
@@ -12,7 +12,7 @@
 
 
 # FIXME: python 2.7 has a far better way to do this
-def get_subprocess_output(command, log, shell=False, stdin=None):
+def get_subprocess_output(command, log, shell=False, stdin=None, output_expected=False):
     """
     Run the given subprocess command and return it's output. Raise an Exception
     if an error occurs.
@@ -37,6 +37,10 @@ def get_subprocess_output(command, log, shell=False, stdin=None):
 
         stdout_f.seek(0)
         output = stdout_f.read()
+
+    if output_expected and output == "":
+        raise Exception("get_subprocess_output expected output but had none.")
+
     return (output, err, proc.returncode)