commit 595979d029ba6ea610bfa584db24f7b10b546105
author Philipp Schrader <philipp.schrader@bluerivertech.com> Wed Sep 13 11:31:48 2023 -0700
committer James Kuszmaul <james.kuszmaul@bluerivertech.com> Mon Oct 02 13:21:29 2023 -0700
tree 1a13446d776a539f8b04e4905b9839d0d5a2be77
parent 00f117c72afd169cdc4877e34c5d1e1c8ca70d89

Add additional logging options to Subprocess class

Change-Id: I4c3865c5a2875dc66f6fed64b4ba0bb85e8e18b1
Signed-off-by: James Kuszmaul <james.kuszmaul@bluerivertech.com>

aos/starter/subprocess.cc

diff --git a/aos/starter/subprocess.cc b/aos/starter/subprocess.cc
index 7b5fce6..039f3d8 100644
--- a/aos/starter/subprocess.cc
+++ b/aos/starter/subprocess.cc
@@ -141,11 +141,13 @@
       restart_timer_(event_loop_->AddTimer([this] { DoStart(); })),
       stop_timer_(event_loop_->AddTimer([this] {
         if (kill(pid_, SIGKILL) == 0) {
-          LOG_IF(WARNING, quiet_flag_ == QuietLogging::kNo)
+          LOG_IF(WARNING, quiet_flag_ == QuietLogging::kNo ||
+                              quiet_flag_ == QuietLogging::kNotForDebugging)
               << "Failed to stop, sending SIGKILL to '" << name_
               << "' pid: " << pid_;
         } else {
-          PLOG_IF(WARNING, quiet_flag_ == QuietLogging::kNo)
+          PLOG_IF(WARNING, quiet_flag_ == QuietLogging::kNo ||
+                               quiet_flag_ == QuietLogging::kNotForDebugging)
               << "Failed to send SIGKILL to '" << name_ << "' pid: " << pid_;
           stop_timer_->Schedule(event_loop_->monotonic_now() +
                                 std::chrono::seconds(1));
@@ -213,7 +215,8 @@
 
   if (pid != 0) {
     if (pid == -1) {
-      PLOG_IF(WARNING, quiet_flag_ == QuietLogging::kNo)
+      PLOG_IF(WARNING, quiet_flag_ == QuietLogging::kNo ||
+                           quiet_flag_ == QuietLogging::kNotForDebugging)
           << "Failed to fork '" << name_ << "'";
       stop_reason_ = aos::starter::LastStopReason::FORK_ERR;
       status_ = aos::starter::State::STOPPED;
@@ -323,7 +326,8 @@
   // If we got here, something went wrong
   status_pipes_.write->Write(
       static_cast<uint32_t>(aos::starter::LastStopReason::EXECV_ERR));
-  PLOG_IF(WARNING, quiet_flag_ == QuietLogging::kNo)
+  PLOG_IF(WARNING, quiet_flag_ == QuietLogging::kNo ||
+                       quiet_flag_ == QuietLogging::kNotForDebugging)
       << "Could not execute " << name_ << " (" << path_ << ')';
 
   _exit(EXIT_FAILURE);
@@ -371,12 +375,18 @@
   switch (status_) {
     case aos::starter::State::STARTING:
     case aos::starter::State::RUNNING: {
-      LOG_IF(INFO, quiet_flag_ == QuietLogging::kNo)
+      LOG_IF(INFO, quiet_flag_ == QuietLogging::kNo ||
+                       quiet_flag_ == QuietLogging::kNotForDebugging)
           << "Stopping '" << name_ << "' pid: " << pid_ << " with signal "
           << SIGINT;
       status_ = aos::starter::State::STOPPING;
 
-      kill(pid_, SIGINT);
+      if (kill(pid_, SIGINT) != 0) {
+        PLOG_IF(INFO, quiet_flag_ == QuietLogging::kNo ||
+                          quiet_flag_ == QuietLogging::kNotForDebugging)
+            << "Failed to send signal " << SIGINT << " to '" << name_
+            << "' pid: " << pid_;
+      }
 
       // Watchdog timer to SIGKILL application if it is still running 1 second
       // after SIGINT
@@ -591,7 +601,8 @@
             << "Application '" << name_ << "' pid " << pid_
             << " exited with status " << exit_code_.value();
       } else {
-        LOG_IF(WARNING, quiet_flag_ == QuietLogging::kNo)
+        LOG_IF(WARNING, quiet_flag_ == QuietLogging::kNo ||
+                            quiet_flag_ == QuietLogging::kNotForDebugging)
             << "Failed to start '" << name_ << "' on pid " << pid_
             << " : Exited with status " << exit_code_.value();
       }
@@ -609,7 +620,8 @@
             << "Application '" << name_ << "' pid " << pid_
             << " exited with status " << exit_code_.value();
       } else {
-        if (quiet_flag_ == QuietLogging::kNo) {
+        if (quiet_flag_ == QuietLogging::kNo ||
+            quiet_flag_ == QuietLogging::kNotForDebugging) {
           std::string version_string =
               latest_timing_report_version_.has_value()
                   ? absl::StrCat("'", latest_timing_report_version_.value(),

aos/starter/subprocess.h

diff --git a/aos/starter/subprocess.h b/aos/starter/subprocess.h
index 784c544..9630e00 100644
--- a/aos/starter/subprocess.h
+++ b/aos/starter/subprocess.h
@@ -68,7 +68,14 @@
 // automatically.
 class Application {
  public:
-  enum class QuietLogging { kYes, kNo };
+  enum class QuietLogging {
+    kYes,
+    kNo,
+    // For debugging child processes not behaving as expected. When a child
+    // experiences an event such as exiting with an error code or dying to due a
+    // signal, this option will cause a log statement to be printed.
+    kNotForDebugging,
+  };
   Application(const aos::Application *application, aos::EventLoop *event_loop,
               std::function<void()> on_change,
               QuietLogging quiet_flag = QuietLogging::kNo);
@@ -127,6 +134,8 @@
   bool autorestart() const { return autorestart_; }
   void set_autorestart(bool autorestart) { autorestart_ = autorestart; }
 
+  LastStopReason stop_reason() const { return stop_reason_; }
+
   const std::string &GetStdout();
   const std::string &GetStderr();
   std::optional<int> exit_code() const { return exit_code_; }