[subprocess][go] run the subprocesses from go-land (#843)

* [python] use subprocess32 whenever possible

* [python] collect interpreter usage statistics.

[py] install pympler too

[py] fix multiple issues with python memory collector

[py] initialize entries

* [api] be REST friendly

* [subprocess] fork from go-land add C-binding

* [subprocess] run from go-land - WIP

* [py] wrap new call in legacy call for BC

* [subprocess][go] almost ready - pending exception import from go

* [subprocess][go] compiles

* [subprocess][go] return None if returning null

* [subprocess][go] adding GIL to GetSubprocessOutput go func

* [subprocess] adding test, removing obsolete py code + typo fix

* [subprocess] no need for subprocess32 now that we run from goland.

* [subprocess][go] fix race condition to collect cmd output/stderr

* [subprocess][c] make C90 compliant.

* [circleci] updating image to go1.9.1 and newer ruby.

* [circleci] updating builder to address openssl issue with image

* [subprocess][py] naturally make the exception available to python scripts - BC

* [subprocess][go] lock thread + indentation issues

* [subprocess][go] dont decrement references

Author: truthbk

.circleci/config.yml

@@ -13,7 +13,7 @@ experimental:
 templates:
   job_template: &job_template
     docker:
-      - image: jfullaondo/datadog-agent-builder-deb:0.0.4
+      - image: jfullaondo/datadog-agent-builder-deb:0.1.1
         environment:
           USE_SYSTEM_LIBS: "1"
     working_directory: /go/src/github.com/DataDog/datadog-agent

.circleci/images/builder/Dockerfile

@@ -1,10 +1,10 @@
-FROM golang:1.8.3
+FROM golang:1.9.1
 
 RUN sed -i 's/^#\s*\(deb.*universe\)$/\1/g' /etc/apt/sources.list \
     && sed -i 's/^#\s*\(deb.*multiverse\)$/\1/g' /etc/apt/sources.list \
     && sed -i 's/main/main contrib non-free/' /etc/apt/sources.list
 
-RUN apt-get update && apt-get install -y python2.7-dev autoconf autogen intltool
+RUN apt-get update && apt-get install -y python2.7-dev autoconf autogen intltool libssl1.0-dev
 RUN apt-get install -y libsnmp-base libsnmp-dev libpq-dev snmp-mibs-downloader
 
 # Ruby,,,
@@ -14,9 +14,9 @@ RUN mkdir -p /usr/local/etc \
 		echo 'update: --no-document'; \
 	} >> /usr/local/etc/gemrc
 
-ENV RUBY_MAJOR 2.3
-ENV RUBY_VERSION 2.3.4
-ENV RUBY_DOWNLOAD_SHA256 341cd9032e9fd17c452ed8562a8d43f7e45bfe05e411d0d7d627751dd82c578c
+ENV RUBY_MAJOR 2.4
+ENV RUBY_VERSION 2.4.2
+ENV RUBY_DOWNLOAD_SHA256 748a8980d30141bd1a4124e11745bb105b436fb1890826e0d2b9ea31af27f735
 ENV RUBYGEMS_VERSION 2.6.12
 
 # some of ruby's build scripts are written in ruby
@@ -57,14 +57,14 @@ RUN set -ex \
 		--build="$gnuArch" \
 		--disable-install-doc \
 		--enable-shared \
+        --with-openssl=/usr/lib/ssl \
 	&& make -j "$(nproc)" \
 	&& make install \
 	\
 	&& apt-get purge -y --auto-remove $buildDeps \
 	&& cd / \
-	&& rm -r /usr/src/ruby \
-	\
-	&& gem update --system "$RUBYGEMS_VERSION"
+	&& gem update --system "$RUBYGEMS_VERSION" \
+	&& rm -r /usr/src/ruby
 
 ENV BUNDLER_VERSION 1.15.3
 

cmd/agent/dist/utils/subprocess_output.py

@@ -6,73 +6,12 @@
 # (C) Datadog, Inc. 2010-2017
 # All rights reserved
 
-# stdlib
-from functools import wraps
-import logging
-import tempfile
-import os
-import sys
-
-if os.name == 'posix' and sys.version_info[0] < 3:
-    try:
-        import subprocess32 as subprocess
-    except ImportError:
-        import subprocess
-else:
-    import subprocess
-
-
-log = logging.getLogger(__name__)
-
-
-class SubprocessOutputEmptyError(Exception):
-    pass
-
+from util import get_subprocess_output as subprocess_output
+from util import SubprocessOutputEmptyError  # noqa
 
 def get_subprocess_output(command, log, raise_on_empty_output=True):
     """
     Run the given subprocess command and return its output. Raise an Exception
     if an error occurs.
     """
-
-    # Use tempfile, allowing a larger amount of memory. The subprocess.Popen
-    # docs warn that the data read is buffered in memory. They suggest not to
-    # use subprocess.PIPE if the data size is large or unlimited.
-    with tempfile.TemporaryFile() as stdout_f, tempfile.TemporaryFile() as stderr_f:
-        proc = subprocess.Popen(command, stdout=stdout_f, stderr=stderr_f)
-        pid = proc.pid
-        log.debug("running process: {0} with pid: {1}".format(" ".join(command), pid))
-
-        retcode = proc.wait()
-        if retcode != 0:
-            log.debug("Error while running {0} with pid: {1}. It returned with code {2}".format(" ".join(command), pid, retcode))
-
-        stderr_f.seek(0)
-        err = stderr_f.read()
-        if err:
-            log.debug("Error while running {0} : {1}".format(" ".join(command), err))
-
-        stdout_f.seek(0)
-        output = stdout_f.read()
-
-    if not output and raise_on_empty_output:
-        raise SubprocessOutputEmptyError("get_subprocess_output expected output but had none.")
-
-    return (output, err, proc.returncode)
-
-
-def log_subprocess(func):
-    """
-    Wrapper around subprocess to log.debug commands.
-    """
-    @wraps(func)
-    def wrapper(*params, **kwargs):
-        fc = "%s(%s)" % (func.__name__, ', '.join(
-            [a.__repr__() for a in params] +
-            ["%s = %s" % (a, b) for a, b in kwargs.items()]
-        ))
-        log.debug("%s called" % fc)
-        return func(*params, **kwargs)
-    return wrapper
-
-subprocess.Popen = log_subprocess(subprocess.Popen)
+    return subprocess_output(command, raise_on_empty_output)

omnibus/config/software/datadog-agent-integrations.rb

@@ -96,7 +96,6 @@
     # Manually add "core" dependencies that are not listed in the checks requirements
     all_reqs_file.puts "requests==2.11.1"
     all_reqs_file.puts "pympler==0.5"
-    all_reqs_file.puts "subprocess32==3.2.7" if os != 'windows'
 
     all_reqs_file.close
   end

pkg/collector/py/datadog_agent.c

@@ -1,10 +1,15 @@
 #include "datadog_agent.h"
 
+// Functions 
 PyObject* GetVersion(PyObject *self, PyObject *args);
 PyObject* Headers(PyObject *self, PyObject *args);
 PyObject* GetHostname(PyObject *self, PyObject *args);
 PyObject* LogMessage(char *message, int logLevel);
 PyObject* GetConfig(char *key);
+PyObject* GetSubprocessOutput(char **args, int argc, int raise);
+
+// Exceptions
+PyObject* SubprocessOutputEmptyError;
 
 static PyObject *get_config(PyObject *self, PyObject *args) {
     char *key;
@@ -38,6 +43,65 @@ static PyObject *log_message(PyObject *self, PyObject *args) {
     return LogMessage(message, log_level);
 }
 
+static PyObject *get_subprocess_output(PyObject *self, PyObject *args) {
+    PyObject *cmd_args, *cmd_raise_on_empty; 
+    int raise = 1, i=0;
+    int subprocess_args_sz;
+    char ** subprocess_args, * subprocess_arg;
+    PyObject * py_result = Py_None;
+
+    PyGILState_STATE gstate = PyGILState_Ensure();
+
+    cmd_raise_on_empty = NULL;
+    if (!PyArg_ParseTuple(args, "O|O:get_subprocess_output", &cmd_args, &cmd_raise_on_empty)) {
+        PyGILState_Release(gstate);
+        PyErr_SetString(PyExc_TypeError, "unable to parse arguments");
+        Py_RETURN_NONE;
+    }
+
+    if (!PyList_Check(cmd_args)) {
+        PyGILState_Release(gstate);
+        PyErr_SetString(PyExc_TypeError, "command args not a list");
+        Py_RETURN_NONE;
+    }
+
+    if (cmd_raise_on_empty != NULL && !PyBool_Check(cmd_raise_on_empty)) {
+        PyGILState_Release(gstate);
+        PyErr_SetString(PyExc_TypeError, "bad raise on empty argument - should be bool");
+        Py_RETURN_NONE;
+    }
+
+    if (cmd_raise_on_empty != NULL) {
+        raise = (int)(cmd_raise_on_empty == Py_True);
+    }
+
+    subprocess_args_sz = PyList_Size(cmd_args);
+    if(!(subprocess_args = (char **)malloc(sizeof(char *)*subprocess_args_sz))) {
+        PyGILState_Release(gstate);
+        PyErr_SetString(PyExc_MemoryError, "unable to allocate memory, bailing out");
+        Py_RETURN_NONE;
+    }
+
+    for (i = 0; i < subprocess_args_sz; i++) {
+        subprocess_arg = PyString_AsString(PyList_GetItem(cmd_args, i));
+        if (subprocess_arg == NULL) {
+            PyErr_SetString(PyExc_Exception, "unable to parse arguments to cgo/go-land");
+            free(subprocess_args);
+            Py_RETURN_NONE;
+        }
+        subprocess_args[i] = subprocess_arg;
+    }
+
+    PyGILState_Release(gstate);
+    py_result = GetSubprocessOutput(subprocess_args, subprocess_args_sz, raise);
+    free(subprocess_args);
+
+    if (py_result == NULL) {
+        Py_RETURN_NONE;
+    }
+    return py_result;
+}
+
 static PyMethodDef datadogAgentMethods[] = {
   {"get_version", GetVersion, METH_VARARGS, "Get the Agent version."},
   {"get_config", get_config, METH_VARARGS, "Get value from the agent configuration."},
@@ -53,6 +117,7 @@ static PyMethodDef datadogAgentMethods[] = {
  */
 static PyMethodDef utilMethods[] = {
   {"headers", (PyCFunction)Headers, METH_VARARGS, "Get basic HTTP headers with the right UserAgent."},
+  {"get_subprocess_output", (PyCFunction)get_subprocess_output, METH_VARARGS, "Run subprocess and return its output."},
   {NULL, NULL}
 };
 
@@ -64,5 +129,9 @@ void initdatadogagent()
   PyObject *da = Py_InitModule("datadog_agent", datadogAgentMethods);
   PyObject *util = Py_InitModule("util", utilMethods);
 
+  SubprocessOutputEmptyError = PyErr_NewException("util.SubprocessOutputEmptyError", NULL, NULL);
+  Py_INCREF(SubprocessOutputEmptyError);
+  PyModule_AddObject(util, "SubprocessOutputEmptyError", SubprocessOutputEmptyError);
+
   PyGILState_Release(gstate);
 }

pkg/collector/py/datadog_agent.go

@@ -6,6 +6,12 @@
 package py
 
 import (
+	"fmt"
+	"io/ioutil"
+	"os/exec"
+	"runtime"
+	"sync"
+	"syscall"
 	"unsafe"
 
 	log "github.com/cihub/seelog"
@@ -111,6 +117,115 @@ func LogMessage(message *C.char, logLevel C.int) *C.PyObject {
 	return C._none()
 }
 
+// GetSubprocessOutput runs the subprocess and returns the output
+//export GetSubprocessOutput
+func GetSubprocessOutput(argv **C.char, argc, raise int) *C.PyObject {
+
+	// https://github.com/golang/go/wiki/cgo#turning-c-arrays-into-go-slices
+	length := int(argc)
+	subprocessArgs := make([]string, length-1)
+	cmdSlice := (*[1 << 30]*C.char)(unsafe.Pointer(argv))[:length:length]
+	subprocessCmd := C.GoString(cmdSlice[0])
+	for i := 1; i < length; i++ {
+		subprocessArgs[i-1] = C.GoString(cmdSlice[i])
+	}
+	cmd := exec.Command(subprocessCmd, subprocessArgs...)
+
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
+	glock := C.PyGILState_Ensure()
+	defer C.PyGILState_Release(glock)
+
+	stdout, err := cmd.StdoutPipe()
+	if err != nil {
+		cErr := C.CString(fmt.Sprintf("internal error creating stdout pipe: %v", err))
+		C.PyErr_SetString(C.PyExc_Exception, cErr)
+		C.free(unsafe.Pointer(cErr))
+		return C._none()
+	}
+
+	var wg sync.WaitGroup
+	var output []byte
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		output, _ = ioutil.ReadAll(stdout)
+	}()
+
+	stderr, err := cmd.StderrPipe()
+	if err != nil {
+		cErr := C.CString(fmt.Sprintf("internal error creating stderr pipe: %v", err))
+		C.PyErr_SetString(C.PyExc_Exception, cErr)
+		C.free(unsafe.Pointer(cErr))
+		return C._none()
+	}
+
+	var outputErr []byte
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		outputErr, _ = ioutil.ReadAll(stderr)
+	}()
+
+	cmd.Start()
+
+	retCode := 0
+	err = cmd.Wait()
+	if exiterr, ok := err.(*exec.ExitError); ok {
+		if status, ok := exiterr.Sys().(syscall.WaitStatus); ok {
+			retCode = status.ExitStatus()
+		}
+	}
+	wg.Wait()
+
+	if raise > 0 {
+		// raise on error
+		if len(output) == 0 {
+			cModuleName := C.CString("util")
+			utilModule := C.PyImport_ImportModule(cModuleName)
+			C.free(unsafe.Pointer(cModuleName))
+			if utilModule == nil {
+				cErr := C.CString("unable to import subprocess empty output exception")
+				C.PyErr_SetString(C.PyExc_Exception, cErr)
+				C.free(unsafe.Pointer(cErr))
+				return C._none()
+			}
+			defer C.Py_DecRef(utilModule)
+
+			cExcName := C.CString("SubprocessOutputEmptyError")
+			excClass := C.PyObject_GetAttrString(utilModule, cExcName)
+			C.free(unsafe.Pointer(cExcName))
+			if excClass == nil {
+				cErr := C.CString("unable to import subprocess empty output exception")
+				C.PyErr_SetString(C.PyExc_Exception, cErr)
+				C.free(unsafe.Pointer(cErr))
+				return C._none()
+			}
+			defer C.Py_DecRef(excClass)
+
+			cErr := C.CString("get_subprocess_output expected output but had none.")
+			C.PyErr_SetString((*C.PyObject)(unsafe.Pointer(excClass)), cErr)
+			C.free(unsafe.Pointer(cErr))
+			return C._none()
+		}
+	}
+
+	cOutput := C.CString(string(output[:]))
+	pyOutput := C.PyString_FromString(cOutput)
+	C.free(unsafe.Pointer(cOutput))
+	cOutputErr := C.CString(string(outputErr[:]))
+	pyOutputErr := C.PyString_FromString(cOutputErr)
+	C.free(unsafe.Pointer(cOutputErr))
+	pyRetCode := C.PyInt_FromLong(C.long(retCode))
+
+	pyResult := C.PyTuple_New(3)
+	C.PyTuple_SetItem(pyResult, 0, pyOutput)
+	C.PyTuple_SetItem(pyResult, 1, pyOutputErr)
+	C.PyTuple_SetItem(pyResult, 2, pyRetCode)
+
+	return pyResult
+}
+
 func initDatadogAgent() {
 	C.initdatadogagent()
 }

pkg/collector/py/datadog_agent.h

@@ -3,6 +3,7 @@
 
 #include <Python.h>
 
+
 void initdatadogagent();
 
 #endif /* DATADOG_HEADER */