Improve log messages for failed commands

fixes #6970

diff --git a/lib/base/process.cpp b/lib/base/process.cpp
index bfae2506d0d606f3c680ce8c483f15d6dfe421ad..46de696019c834e87981991d9ca57a80a2f9db7d 100644 (file)
--- a/lib/base/process.cpp
+++ b/lib/base/process.cpp
@@ -280,12 +280,12 @@ void Process::IOThreadProc(int tid)
        }
 }
 
-String Process::PrettyPrintArguments(void) const
+String Process::PrettyPrintArguments(const Process::Arguments& arguments)
 {
 #ifdef _WIN32
-       return "'" + m_Arguments + "'";
+       return "'" + arguments + "'";
 #else /* _WIN32 */
-       return "'" + boost::algorithm::join(m_Arguments, "' '") + "'";
+       return "'" + boost::algorithm::join(arguments, "' '") + "'";
 #endif /* _WIN32 */
 }
 
@@ -436,7 +436,7 @@ void Process::Run(const boost::function<void(const ProcessResult&)>& callback)
        m_FD = outReadPipe;
        m_PID = pi.dwProcessId;
 
-       Log(LogNotice, "Process", "Running command " + PrettyPrintArguments() +
+       Log(LogNotice, "Process", "Running command " + PrettyPrintArguments(m_Arguments) +
            ": PID " + Convert::ToString(m_PID));
 
 #else /* _WIN32 */
@@ -515,7 +515,7 @@ void Process::Run(const boost::function<void(const ProcessResult&)>& callback)
                // child process
 
                if (dup2(fds[1], STDOUT_FILENO) < 0 || dup2(fds[1], STDERR_FILENO) < 0) {
-                       perror("dup2() failed.");
+                       perror("dup2() failed");
                        _exit(128);
                }
 
@@ -529,7 +529,7 @@ void Process::Run(const boost::function<void(const ProcessResult&)>& callback)
                        char errmsg[512];
                        strcpy(errmsg, "execvpe(");
                        strncat(errmsg, argv[0], sizeof(errmsg) - 1);
-                       strncat(errmsg, ") failed.", sizeof(errmsg) - 1);
+                       strncat(errmsg, ") failed", sizeof(errmsg) - 1);
                        errmsg[sizeof(errmsg) - 1] = '\0';
                        perror(errmsg);
                        _exit(128);
@@ -542,7 +542,8 @@ void Process::Run(const boost::function<void(const ProcessResult&)>& callback)
 
        m_PID = m_Process;
 
-       Log(LogNotice, "Process", "Running command " + PrettyPrintArguments() + ": PID " + Convert::ToString(m_PID));
+       Log(LogNotice, "Process", "Running command " + PrettyPrintArguments(m_Arguments) +
+           ": PID " + Convert::ToString(m_PID));
 
        // free arguments
        for (int i = 0; argv[i] != NULL; i++)
@@ -592,7 +593,8 @@ bool Process::DoEvents(void)
 
                if (timeout < Utility::GetTime()) {
                        Log(LogWarning, "Process", "Killing process " + Convert::ToString(m_PID) +
-                           " (" + PrettyPrintArguments() + ") after timeout of " + Convert::ToString(m_Timeout) + " seconds");
+                           " (" + PrettyPrintArguments(m_Arguments) + ") after timeout of " +
+                           Convert::ToString(m_Timeout) + " seconds");
 
                        m_OutputStream << "<Timeout exceeded.>";
 #ifdef _WIN32
@@ -640,8 +642,9 @@ bool Process::DoEvents(void)
        DWORD exitcode;
        GetExitCodeProcess(m_Process, &exitcode);
 
-       Log((exitcode == 0) ? LogNotice : LogWarning, "Process", "PID " + Convert::ToString(m_PID) +
-           " (" + PrettyPrintArguments() + ") terminated with exit code " + Convert::ToString(exitcode));
+       Log(LogNotice, "Process", "PID " + Convert::ToString(m_PID) +
+           " (" + PrettyPrintArguments(m_Arguments) + ") terminated with exit code " +
+           Convert::ToString(exitcode));
 #else /* _WIN32 */
        int status, exitcode;
        if (waitpid(m_Process, &status, 0) != m_Process) {
@@ -653,8 +656,9 @@ bool Process::DoEvents(void)
        if (WIFEXITED(status)) {
                exitcode = WEXITSTATUS(status);
 
-               Log((exitcode == 0) ? LogNotice : LogWarning, "Process", "PID " + Convert::ToString(m_PID) +
-                   " (" + PrettyPrintArguments() + ") terminated with exit code " + Convert::ToString(exitcode));
+               Log(LogNotice, "Process", "PID " + Convert::ToString(m_PID) +
+                   " (" + PrettyPrintArguments(m_Arguments) + ") terminated with exit code " +
+                   Convert::ToString(exitcode));
        } else if (WIFSIGNALED(status)) {
                Log(LogWarning, "Process", "PID " + Convert::ToString(m_PID) + " was terminated by signal " +
                    Convert::ToString(WTERMSIG(status)));
@@ -668,6 +672,7 @@ bool Process::DoEvents(void)
        }
 #endif /* _WIN32 */
 
+       m_Result.PID = m_PID;
        m_Result.ExecutionEnd = Utility::GetTime();
        m_Result.ExitStatus = exitcode;
        m_Result.Output = output;

diff --git a/lib/base/process.hpp b/lib/base/process.hpp
index 0e31a3ef86c490a0304528e160afdb5f7e348bcc..a23942ba2601ba7dd58c80a6a053cdd126e0f1bd 100644 (file)
--- a/lib/base/process.hpp
+++ b/lib/base/process.hpp
@@ -37,6 +37,7 @@ namespace icinga
  */
 struct ProcessResult
 {
+       pid_t PID;
        double ExecutionStart;
        double ExecutionEnd;
        long ExitStatus;
@@ -80,6 +81,8 @@ public:
        static void StaticInitialize(void);
        static void ThreadInitialize(void);
 
+       static String PrettyPrintArguments(const Arguments& arguments);
+
 private:
        Arguments m_Arguments;
        Dictionary::Ptr m_ExtraEnvironment;
@@ -97,8 +100,6 @@ private:
        static void IOThreadProc(int tid);
        bool DoEvents(void);
        int GetTID(void) const;
-
-       String PrettyPrintArguments(void) const;
 };
 
 }

diff --git a/lib/methods/pluginchecktask.cpp b/lib/methods/pluginchecktask.cpp
index 7ba0c64969687d840f3457cab26f71c0bf13433c..462ce890fbbab364a797134cba79e5531a35494b 100644 (file)
--- a/lib/methods/pluginchecktask.cpp
+++ b/lib/methods/pluginchecktask.cpp
@@ -27,6 +27,7 @@
 #include "base/scriptfunction.hpp"
 #include "base/utility.hpp"
 #include "base/process.hpp"
+#include "base/convert.hpp"
 #include <boost/algorithm/string/classification.hpp>
 #include <boost/algorithm/string/split.hpp>
 #include <boost/foreach.hpp>
@@ -55,6 +56,13 @@ void PluginCheckTask::ScriptFunc(const Checkable::Ptr& checkable, const CheckRes
 
 void PluginCheckTask::ProcessFinishedHandler(const Checkable::Ptr& checkable, const CheckResult::Ptr& cr, const Value& commandLine, const ProcessResult& pr)
 {
+       if (pr.ExitStatus > 3) {
+               Process::Arguments parguments = Process::PrepareCommand(commandLine);
+               Log(LogWarning, "PluginCheckTask", "Check command for object '" + checkable->GetName() + "' (PID: " + Convert::ToString(pr.PID) +
+                   ", arguments: " + Process::PrettyPrintArguments(parguments) + ") terminated with exit code " +
+                   Convert::ToString(pr.ExitStatus) + ", output: " + pr.Output);
+       }
+
        String output = pr.Output;
        output.Trim();
        std::pair<String, Value> co = PluginUtility::ParseCheckOutput(output);

diff --git a/lib/methods/pluginchecktask.hpp b/lib/methods/pluginchecktask.hpp
index f92e33a663bb48c4ecadf9b409b7546e20106a04..526c49251739297adfcabbdd81a917e3e22dfe49 100644 (file)
--- a/lib/methods/pluginchecktask.hpp
+++ b/lib/methods/pluginchecktask.hpp
@@ -40,8 +40,8 @@ public:
 private:
        PluginCheckTask(void);
 
-       static void ProcessFinishedHandler(const Checkable::Ptr& service, const CheckResult::Ptr& cr, const Value& commandLine, const ProcessResult& pr);
-
+       static void ProcessFinishedHandler(const Checkable::Ptr& service,
+           const CheckResult::Ptr& cr, const Value& commandLine, const ProcessResult& pr);
 };
 
 }

diff --git a/lib/methods/plugineventtask.cpp b/lib/methods/plugineventtask.cpp
index f3f274c133ea510f4d2512a689ea28e84ac8cf2e..91096d0c6a6a8e3b4c5d9419bcc92bff74f7c4cc 100644 (file)
--- a/lib/methods/plugineventtask.cpp
+++ b/lib/methods/plugineventtask.cpp
@@ -27,6 +27,7 @@
 #include "base/scriptfunction.hpp"
 #include "base/utility.hpp"
 #include "base/process.hpp"
+#include "base/convert.hpp"
 #include <boost/foreach.hpp>
 
 using namespace icinga;
@@ -51,13 +52,12 @@ void PluginEventTask::ScriptFunc(const Checkable::Ptr& checkable)
        PluginUtility::ExecuteCommand(commandObj, checkable, checkable->GetLastCheckResult(), resolvers, boost::bind(&PluginEventTask::ProcessFinishedHandler, checkable, _1, _2));
 }
 
-void PluginEventTask::ProcessFinishedHandler(const Checkable::Ptr& checkable, const Value& command, const ProcessResult& pr)
+void PluginEventTask::ProcessFinishedHandler(const Checkable::Ptr& checkable, const Value& commandLine, const ProcessResult& pr)
 {
        if (pr.ExitStatus != 0) {
-               std::ostringstream msgbuf;
-               msgbuf << "Event command '" << command << "' for object '"
-                      << checkable->GetName() << "' failed; exit status: "
-                      << pr.ExitStatus << ", output: " << pr.Output;
-               Log(LogWarning, "PluginEventTask", msgbuf.str());
+               Process::Arguments parguments = Process::PrepareCommand(commandLine);
+               Log(LogNotice, "PluginEventTask", "Event command for object '" + checkable->GetName() + "' (PID: " + Convert::ToString(pr.PID) +
+                   ", arguments: " + Process::PrettyPrintArguments(parguments) + ") terminated with exit code " +
+                   Convert::ToString(pr.ExitStatus) + ", output: " + pr.Output);
        }
 }

diff --git a/lib/methods/plugineventtask.hpp b/lib/methods/plugineventtask.hpp
index 35fc1a97a6959565bfedb0fc1dab5cf104fe64a9..755fc9d1c5d8cced73b3ef82e6cc36b24939d12e 100644 (file)
--- a/lib/methods/plugineventtask.hpp
+++ b/lib/methods/plugineventtask.hpp
@@ -40,7 +40,8 @@ public:
 private:
        PluginEventTask(void);
 
-       static void ProcessFinishedHandler(const Checkable::Ptr& checkable, const Value& command, const ProcessResult& pr);
+       static void ProcessFinishedHandler(const Checkable::Ptr& checkable,
+           const Value& commandLine, const ProcessResult& pr);
 };
 
 }

diff --git a/lib/methods/pluginnotificationtask.cpp b/lib/methods/pluginnotificationtask.cpp
index a1857bb18f7ed73dcd28228fc9e236630ad78073..c37113263acde118cb30f4f2b4c6ff08dfc5b22c 100644 (file)
--- a/lib/methods/pluginnotificationtask.cpp
+++ b/lib/methods/pluginnotificationtask.cpp
@@ -28,6 +28,7 @@
 #include "base/logger_fwd.hpp"
 #include "base/utility.hpp"
 #include "base/process.hpp"
+#include "base/convert.hpp"
 #include <boost/foreach.hpp>
 
 using namespace icinga;
@@ -65,13 +66,12 @@ void PluginNotificationTask::ScriptFunc(const Notification::Ptr& notification, c
        PluginUtility::ExecuteCommand(commandObj, checkable, cr, resolvers, boost::bind(&PluginNotificationTask::ProcessFinishedHandler, checkable, _1, _2));
 }
 
-void PluginNotificationTask::ProcessFinishedHandler(const Checkable::Ptr& checkable, const Value& command, const ProcessResult& pr)
+void PluginNotificationTask::ProcessFinishedHandler(const Checkable::Ptr& checkable, const Value& commandLine, const ProcessResult& pr)
 {
        if (pr.ExitStatus != 0) {
-               std::ostringstream msgbuf;
-               msgbuf << "Notification command '" << command << "' for object '"
-                      << checkable->GetName() << "' failed; exit status: "
-                      << pr.ExitStatus << ", output: " << pr.Output;
-               Log(LogWarning, "PluginNotificationTask", msgbuf.str());
+               Process::Arguments parguments = Process::PrepareCommand(commandLine);
+               Log(LogWarning, "PluginNotificationTask", "Notification command for object '" + checkable->GetName() + "' (PID: " + Convert::ToString(pr.PID) +
+                   ", arguments: " + Process::PrettyPrintArguments(parguments) + ") terminated with exit code " +
+                   Convert::ToString(pr.ExitStatus) + ", output: " + pr.Output);
        }
 }

diff --git a/lib/methods/pluginnotificationtask.hpp b/lib/methods/pluginnotificationtask.hpp
index a7e843d19ea636285fc34a422f8629f8813a6d28..f96dff84d2425ca3a316a1f65b22e392acd03cbe 100644 (file)
--- a/lib/methods/pluginnotificationtask.hpp
+++ b/lib/methods/pluginnotificationtask.hpp
@@ -43,7 +43,8 @@ public:
 private:
        PluginNotificationTask(void);
 
-       static void ProcessFinishedHandler(const Checkable::Ptr& checkable, const Value& command, const ProcessResult& pr);
+       static void ProcessFinishedHandler(const Checkable::Ptr& checkable,
+           const Value& commandLine, const ProcessResult& pr);
 };
 
 }