commit 8e220c6c5e66dbc58e5478c52ce42445f8ce9a4e
author Sergey Shevchenko <sergeygs@google.com> Thu Jul 13 17:28:25 2023 -0700
committer Copybara-Service <copybara-worker@google.com> Thu Jul 13 17:29:09 2023 -0700
tree 776db245b8e0a120d5d37a435e625f5840295e6d
parent c3f4ee7f1b0fc3f3e1a869ae3da5b57415b01bff

#Centipede Improve failure logging in `Command`

PiperOrigin-RevId: 547973812

centipede/BUILD

diff --git a/centipede/BUILD b/centipede/BUILD
index 0d82b62..a24a8d8 100644
--- a/centipede/BUILD
+++ b/centipede/BUILD
@@ -486,6 +486,7 @@
         "@com_google_absl//absl/status",
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/strings:str_format",
+        "@com_google_absl//absl/synchronization",
         "@com_google_absl//absl/time",
     ],
 )

centipede/command.cc

diff --git a/centipede/command.cc b/centipede/command.cc
index cba1748..6ecca82 100644
--- a/centipede/command.cc
+++ b/centipede/command.cc
@@ -36,6 +36,7 @@
 #include "absl/strings/str_join.h"
 #include "absl/strings/str_replace.h"
 #include "absl/strings/str_split.h"
+#include "absl/synchronization/mutex.h"
 #include "absl/time/clock.h"
 #include "./centipede/logging.h"
 #include "./centipede/util.h"
@@ -374,8 +375,17 @@
 }
 
 void Command::LogProblemInfo(std::string_view message) const {
-  PLOG(ERROR) << message;
-  LOG(ERROR).NoPrefix() << VV(command_line_);
+  // Prevent confusing interlaced logs when multiple threads experience failures
+  // at the same time.
+  // TODO(ussuri): Non-failure related logs from other threads may still
+  //  interlace with these. Improve further, if possible. Note the printiing
+  //  line-by-line is unavoidable to overcome the single log line length limit.
+  static absl::Mutex mu{absl::kConstInit};
+  absl::MutexLock lock(&mu);
+
+  LOG(ERROR) << message;
+  LOG(ERROR).NoPrefix() << "=== COMMAND ===";
+  LOG(ERROR).NoPrefix() << command_line_;
   LOG(ERROR).NoPrefix() << "=== STDOUT ===";
   for (const auto &line : absl::StrSplit(ReadRedirectedStdout(), '\n')) {
     LOG(ERROR).NoPrefix() << line;

centipede/command.h

diff --git a/centipede/command.h b/centipede/command.h
index 6e40fc7..48c8557 100644
--- a/centipede/command.h
+++ b/centipede/command.h
@@ -37,8 +37,8 @@
   // `path`: path to the binary.
   // `args`: arguments.
   // `env`: environment variables/values (in the form "KEY=VALUE").
-  // `out`: stdout redirect path (empty means none).
-  // `err`: stderr redirect path (empty means none).
+  // `out`: stdout redirect path (empty means use parent's STDOUT).
+  // `err`: stderr redirect path (empty means use parent's STDERR).
   // `timeout`: terminate a fork server execution attempt after this duration.
   // `temp_file_path`: "@@" in `path` will be replaced with `temp_file_path`.
   // If `out` == `err` and both are non-empty, stdout/stderr are combined.