SERVER-62215 Improve the error message for logRotate command

4 files changed, 60 insertions, 26 deletions

diff --git a/src/mongo/db/commands/generic_servers.cpp b/src/mongo/db/commands/generic_servers.cpp
index 17df971cd85..41448e90dde 100644
--- a/src/mongo/db/commands/generic_servers.cpp
+++ b/src/mongo/db/commands/generic_servers.cpp
@@ -214,17 +214,24 @@ OkReply LogRotateCmd::Invocation::typedRun(OperationContext* opCtx) {
         logType = stdx::get<std::string>(arg);
     }
 
-    int minorErrorCount = 0;
-    const bool ok = logv2::rotateLogs(
-        serverGlobalParams.logRenameOnRotate, logType, [&](Status) { ++minorErrorCount; });
-    if (ok) {
-        logProcessDetailsForLogRotate(opCtx->getServiceContext());
+    logv2::LogRotateErrorAppender minorErrors;
+    auto status = logv2::rotateLogs(serverGlobalParams.logRenameOnRotate,
+                                    logType,
+                                    [&minorErrors](Status err) { minorErrors.append(err); });
+
+    // Mask the detailed error message so file paths & host info are not
+    // revealed to the client, but keep the real status code as a hint.
+    constexpr auto rotateErrmsg = "Log rotation failed due to one or more errors"_sd;
+    uassert(status.code(), rotateErrmsg, status.isOK());
+
+    logProcessDetailsForLogRotate(opCtx->getServiceContext());
+
+    status = minorErrors.getCombinedStatus();
+    if (!status.isOK()) {
+        LOGV2_ERROR(6221501, "Log rotation failed", "error"_attr = status);
+        uasserted(status.code(), rotateErrmsg);
     }
 
-    uassert(ErrorCodes::OperationFailed,
-            "Log rotation failed due to one or more errors",
-            ok && (minorErrorCount == 0));
-
     return OkReply();
 }
 LogRotateCmd logRotateCmd;
diff --git a/src/mongo/logv2/log_util.cpp b/src/mongo/logv2/log_util.cpp
index ded9e390e89..c44ff5988af 100644
--- a/src/mongo/logv2/log_util.cpp
+++ b/src/mongo/logv2/log_util.cpp
@@ -49,9 +49,19 @@ void addLogRotator(StringData logType, LogRotateCallback cb) {
     logRotateCallbacks.emplace(logType, std::move(cb));
 }
 
-bool rotateLogs(bool renameFiles,
-                boost::optional<StringData> logType,
-                std::function<void(Status)> onMinorError) {
+void LogRotateErrorAppender::append(const Status& err) {
+    if (_combined.isOK()) {
+        _combined = err;
+    } else if (!err.isOK()) {
+        // if there are multiple, distinct error codes, use OperationFailed instead
+        auto newCode = (_combined.code() == err.code()) ? err.code() : ErrorCodes::OperationFailed;
+        _combined = Status(newCode, str::stream() << _combined.reason() << ", " << err.reason());
+    }
+}
+
+Status rotateLogs(bool renameFiles,
+                  boost::optional<StringData> logType,
+                  std::function<void(Status)> onMinorError) {
     std::string suffix = "." + terseCurrentTimeForFilename();
 
     LOGV2(23166, "Log rotation initiated", "suffix"_attr = suffix, "logType"_attr = logType);
@@ -59,19 +69,17 @@ bool rotateLogs(bool renameFiles,
     if (logType) {
         auto it = logRotateCallbacks.find(logType.get());
         if (it == logRotateCallbacks.end()) {
-            LOGV2_WARNING(
-                ErrorCodes::NoSuchKey, "Unknown log type for rotate", "logType"_attr = logType);
-            return false;
+            LOGV2_WARNING(6221500, "Unknown log type for rotate", "logType"_attr = logType);
+            return Status(ErrorCodes::NoSuchKey, "Unknown log type for rotate");
         }
         auto status = it->second(renameFiles, suffix, onMinorError);
         if (!status.isOK()) {
             LOGV2_WARNING(
                 1947001, "Log rotation failed", "reason"_attr = status, "logType"_attr = logType);
-            return false;
         }
-        return true;
+        return status;
     } else {
-        bool ret = true;
+        LogRotateErrorAppender errors;
         for (const auto& entry : logRotateCallbacks) {
             auto status = entry.second(renameFiles, suffix, onMinorError);
             if (!status.isOK()) {
@@ -79,10 +87,10 @@ bool rotateLogs(bool renameFiles,
                               "Log rotation failed",
                               "reason"_attr = status,
                               "logType"_attr = entry.first);
-                ret = false;
+                errors.append(status);
             }
         }
-        return ret;
+        return errors.getCombinedStatus();
     }
 }
 
diff --git a/src/mongo/logv2/log_util.h b/src/mongo/logv2/log_util.h
index a60e5f1dd67..e1b6c2c34e0 100644
--- a/src/mongo/logv2/log_util.h
+++ b/src/mongo/logv2/log_util.h
@@ -49,7 +49,25 @@ using LogRotateCallback = std::function<Status(bool, StringData, std::function<v
 void addLogRotator(StringData logType, LogRotateCallback cb);
 
 /**
- * Rotates the log files.  Returns true if all logs rotate successfully.
+ * Class that combines error Status objects into a single Status object.
+ */
+class LogRotateErrorAppender {
+public:
+    LogRotateErrorAppender() : _combined(Status::OK()) {}
+    LogRotateErrorAppender(const Status& init) : _combined(init) {}
+
+    const Status& getCombinedStatus() const {
+        return _combined;
+    }
+
+    void append(const Status& err);
+
+private:
+    Status _combined;
+};
+
+/**
+ * Rotates the log files.  Returns Status::OK() if all logs rotate successfully.
  *
  * renameFiles - true means we rename files, false means we expect the file to be renamed
  *               externally
@@ -59,9 +77,9 @@ void addLogRotator(StringData logType, LogRotateCallback cb);
  * We expect logrotate to rename the existing file before we rotate, and so the next open
  * we do should result in a file create.
  */
-bool rotateLogs(bool renameFiles,
-                boost::optional<StringData> logType,
-                std::function<void(Status)> onMinorError);
+Status rotateLogs(bool renameFiles,
+                  boost::optional<StringData> logType,
+                  std::function<void(Status)> onMinorError);
 
 /**
  * Returns true if system logs should be redacted.
diff --git a/src/mongo/util/signal_handlers.cpp b/src/mongo/util/signal_handlers.cpp
index b48b83798ee..95a4bee2ccc 100644
--- a/src/mongo/util/signal_handlers.cpp
+++ b/src/mongo/util/signal_handlers.cpp
@@ -238,8 +238,9 @@ void handleOneSignal(const SignalWaitResult& waited, LogRotationState* rotation)
             rotation->previous = now;
         }
 
-        if (!logv2::rotateLogs(serverGlobalParams.logRenameOnRotate, {}, {})) {
-            LOGV2_ERROR(4719800, "Log rotation failed");
+        if (auto status = logv2::rotateLogs(serverGlobalParams.logRenameOnRotate, {}, {});
+            !status.isOK()) {
+            LOGV2_ERROR(4719800, "Log rotation failed", "error"_attr = status);
         }
         if (rotation->logFileStatus == LogFileStatus::kNeedToRotateLogFile) {
             logProcessDetailsForLogRotate(getGlobalServiceContext());