Add rotating log file stream.

webrtc/base/stream.cc

Index: webrtc/base/stream.cc
diff --git a/webrtc/base/stream.cc b/webrtc/base/stream.cc
index 3e3afa040dae9e0bf75531bc39f28a18acabaffe..4d0d1788ac271113f024e388fedd1c40ed65d2d7 100644
--- a/webrtc/base/stream.cc
+++ b/webrtc/base/stream.cc
@@ -22,6 +22,7 @@
 #include "webrtc/base/common.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/base/messagequeue.h"
+#include "webrtc/base/pathutils.h"
 #include "webrtc/base/stream.h"
 #include "webrtc/base/stringencode.h"
 #include "webrtc/base/stringutils.h"
@@ -517,6 +518,331 @@ void FileStream::DoClose() {
   fclose(file_);
 }
 
+///////////////////////////////////////////////////////////////////////////////
+// FileRotatingStream
+///////////////////////////////////////////////////////////////////////////////
+
+FileRotatingStream::FileRotatingStream(const std::string& dir_path,
+                                       const std::string& file_prefix)
+    : FileRotatingStream(dir_path, file_prefix, 0, 0, kRead) {
+}
+
+FileRotatingStream::FileRotatingStream(const std::string& dir_path,
+                                       const std::string& file_prefix,
+                                       size_t max_file_size,
+                                       size_t num_files)
+    : FileRotatingStream(dir_path,
+                         file_prefix,
+                         max_file_size,
+                         num_files,
+                         kWrite) {
+  DCHECK_GT(max_file_size, 0u);
+  DCHECK_GT(num_files, 1u);
+}
+
+FileRotatingStream::FileRotatingStream(const std::string& dir_path,
+                                       const std::string& file_prefix,
+                                       size_t max_file_size,
+                                       size_t num_files,
+                                       Mode mode)
+    : dir_path_(dir_path),
+      file_prefix_(file_prefix),
+      mode_(mode),
+      file_stream_(nullptr),
+      max_file_size_(max_file_size),
+      current_file_index_(0),
+      last_file_index_(0),
+      current_bytes_written_(0),
+      disable_buffering_(false) {
+  DCHECK(Filesystem::IsFolder(dir_path));
+  switch (mode) {
+    case kWrite: {
+      file_names_.clear();
+      for (size_t i = 0; i < num_files; ++i) {
+        file_names_.push_back(GetFilePath(i, num_files));
+      }
+      last_file_index_ = num_files - 1;
+      break;
+    }
+    case kRead: {
+      // We want the files in reverse sorted order. The last log file contains
+      // the earliest entries.
+      file_names_ = GetFilesWithPrefix();
+      std::sort(file_names_.begin(), file_names_.end());
+      std::reverse(file_names_.begin(), file_names_.end());
+      break;
+    }
+  }
+}
+
+FileRotatingStream::~FileRotatingStream() {
+}
+
+StreamState FileRotatingStream::GetState() const {
+  if (mode_ == kRead && current_file_index_ < file_names_.size()) {
+    return SS_OPEN;
+  }
+  if (!file_stream_) {
+    return SS_CLOSED;
+  }
+  return file_stream_->GetState();
+}
+
+StreamResult FileRotatingStream::Read(void* buffer,
+                                      size_t buffer_len,
+                                      size_t* read,
+                                      int* error) {
+  DCHECK(buffer);
+  if (current_file_index_ >= file_names_.size()) {
+    return SR_EOS;
+  }
+  // We will have no file stream initially, and when we are finished with the
+  // previous file.
+  if (!file_stream_) {
+    if (!Open(false)) {
+      return SR_ERROR;

[jiayl2, 2015/07/20 23:27:06]

Add logging for error cases?

[tkchin_webrtc, 2015/07/21 20:39:12]

I didn't add logging because I thought there could be infinite loop in write
case. Eg
OnLogMessage -> Write -> <error> -> LOG(LS_ERROR) -> OnLogMessage

Maybe we can log directly to std::err instead? Wdyt?

[jiayl2, 2015/07/21 23:36:19]

std::err for write seems fine. For read, we can log normally.

+    }
+  }
+  StreamResult result = file_stream_->Read(buffer, buffer_len, read, error);
+  if (result == SR_EOS || result == SR_ERROR) {
+    // Reached the end of the file or error-ed, read next file.
+    Close();
+    ++current_file_index_;
+    if (read) {
+      *read = 0;
+    }
+    return SR_SUCCESS;

[jiayl2, 2015/07/20 23:27:05]

I think we should surface the error to the client.

[tkchin_webrtc, 2015/07/21 20:39:12]

Done.

+  } else if (result == SR_SUCCESS) {
+    // Succeeded, continue reading from this file.
+    return SR_SUCCESS;
+  } else {
+    RTC_NOTREACHED();
+  }
+  return result;
+}
+
+StreamResult FileRotatingStream::Write(const void* data,
+                                       size_t data_len,
+                                       size_t* written,
+                                       int* error) {
+  if (!file_stream_) {
+    return SR_ERROR;

[jiayl2, 2015/07/20 23:27:06]

ditto logging

[tkchin_webrtc, 2015/07/21 20:39:12]

Acknowledged.

+  }
+  // Write as much as will fit in to the current file.
+  DCHECK_LT(current_bytes_written_, max_file_size_);
+  size_t remaining_bytes = max_file_size_ - current_bytes_written_;
+  size_t write_length = std::min(data_len, remaining_bytes);
+  size_t local_written = 0;
+  if (!written) {
+    written = &local_written;
+  }
+  StreamResult result = file_stream_->Write(data, write_length, written, error);
+  current_bytes_written_ += *written;
+
+  // If we're done with this file, rotate it out.
+  if (current_bytes_written_ >= max_file_size_) {
+    DCHECK_EQ(current_bytes_written_, max_file_size_);
+    RotateFiles();
+  }
+  return result;
+}
+
+bool FileRotatingStream::Flush() {
+  if (!file_stream_) {
+    return false;
+  }
+  return file_stream_->Flush();
+}
+
+void FileRotatingStream::Close() {
+  if (!file_stream_) {

[jiayl2, 2015/07/20 23:27:05]

please move the impl into a private method like CloseCurrentFile, to avoid
calling Close() from RotateFiles/Open, since we don't mean to close the stream?

[tkchin_webrtc, 2015/07/21 20:39:12]

Done.

+    return;
+  }
+  current_bytes_written_ = 0;
+  file_stream_.reset();
+}
+
+bool FileRotatingStream::Open() {
+  switch (mode_) {
+    case kRead:
+      // Defer opening to when we first read since we want to return read error
+      // if we fail to open next file.
+      return true;
+    case kWrite:
+      // Delete existing files when opening for write.
+      return Open(true);
+  }
+  return false;
+}
+
+bool FileRotatingStream::DisableBuffering() {
+  disable_buffering_ = true;
+  if (!file_stream_) {
+    return false;
+  }
+  return file_stream_->DisableBuffering();
+}
+
+std::string FileRotatingStream::GetFilePath(size_t index) const {
+  DCHECK_LT(index, file_names_.size());
+  return file_names_[index];

[jiayl2, 2015/07/20 23:27:05]

The order is different in read mode and write mode, which seems confusing. Can
we always put the oldeest one first (which I think more intuitive), or the other
way?

[tkchin_webrtc, 2015/07/21 20:39:12]

Sure, putting them in the same order in both modes is fine (ie match index to
name). I think it should stay newest first to match logrotate command behavior.

+}
+
+bool FileRotatingStream::Open(bool delete_existing) {

[jiayl2, 2015/07/20 23:27:05]

nit: overloading adds confusion. How about moving the deleting existing files
part to the public Open, and rename this one to OpenCurrentFile?

[tkchin_webrtc, 2015/07/21 20:39:12]

Done.

+  Close();
+  if (mode_ == kWrite && delete_existing) {
+    std::vector<std::string> matching_files = GetFilesWithPrefix();
+    for (auto matching_file : matching_files) {
+      Filesystem::DeleteFile(matching_file);
+    }
+  }
+
+  // Opens the appropriate file in the appropriate mode.
+  DCHECK_LT(current_file_index_, file_names_.size());
+  std::string file_path = file_names_[current_file_index_];
+  file_stream_.reset(new FileStream());
+  const char* mode = nullptr;
+  switch (mode_) {
+    case kWrite:
+      mode = "w+";
+      // We should always we writing to the zero-th file.
+      DCHECK_EQ(current_file_index_, 0u);
+      break;
+    case kRead:
+      mode = "r";
+      break;
+  }
+  int error = 0;
+  if (!file_stream_->Open(file_path, mode, &error)) {
+    file_stream_.reset();

[jiayl2, 2015/07/20 23:27:05]

ditto logging.

[tkchin_webrtc, 2015/07/21 20:39:12]

Acknowledged.

+    return false;
+  }
+  if (disable_buffering_) {
+    file_stream_->DisableBuffering();
+  }
+  return true;
+}
+
+void FileRotatingStream::RotateFiles() {
+  DCHECK_EQ(mode_, kWrite);

[jiayl2, 2015/07/20 23:27:05]

nit: add logging for critial operations like rotating.

[tkchin_webrtc, 2015/07/21 20:39:13]

Acknowledged.

+  Close();
+  // Starting at |last_file_index_|, delete the log file and rename the

[jiayl2, 2015/07/20 23:27:05]

the name of last_file_index_ seems confusing, since it could mean "the last file
we write to".
Maybe max_file_index_?
But overall I think puting the oldest file at index 0 will be more intuitive.

[tkchin_webrtc, 2015/07/21 20:39:13]

Changed to rotation_index

+  // previous file to be the current file.

[jiayl2, 2015/07/20 23:27:05]

"the current file" may be confusing with |current_file_index_|. I think an
example will be more clear, e.g. suppose the oldest file is file_2, delete
file_2, rename file_1 -> file_2, file_0 -> file_1, open file_0 as the current
file. 

[tkchin_webrtc, 2015/07/21 20:39:12]

Done.

+  DCHECK_LE(last_file_index_, file_names_.size());
+  for (auto i = last_file_index_; i > 0; --i) {
+    // Delete the current file.
+    std::string current_file_name = file_names_[i];
+    if (Filesystem::IsFile(current_file_name)) {
+      Filesystem::DeleteFile(current_file_name);
+    }
+    // Rename the previous file to be current if it exists.
+    std::string prev_file_name = file_names_[i - 1];
+    if (Filesystem::IsFile(prev_file_name)) {
+      Filesystem::MoveFile(prev_file_name, current_file_name);
+    }
+  }
+  Open(false);
+  OnRotation();
+}
+
+std::vector<std::string> FileRotatingStream::GetFilesWithPrefix() const {
+  std::vector<std::string> files;
+  // Iterate over the files in the directory.
+  DirectoryIterator it;
+  if (!it.Iterate(dir_path_)) {
+    return files;
+  }
+  std::string current_name = it.Name();
+  while (current_name.size() > 0) {
+    if (!it.IsDirectory() &&
+        current_name.compare(0, file_prefix_.size(), file_prefix_) == 0) {
+      Pathname path(dir_path_);
+      path.SetFilename(current_name);
+      files.push_back(path.pathname());
+    }
+    if (!it.Next()) {
+      break;
+    }
+    current_name = it.Name();
+  }

[jiayl2, 2015/07/20 23:27:05]

Why not

do {
  current_name = it.Name();
  if (...) 
  
} while (it.Next());

[tkchin_webrtc, 2015/07/21 20:39:12]

thx.

+  return files;
+}
+
+std::string FileRotatingStream::GetFilePath(size_t index,
+                                            size_t num_files) const {
+  DCHECK_LT(index, num_files);
+  std::ostringstream file_name;
+  // The format will be "_%<num_digits>zu". We want to zero pad the index so
+  // that it will sort nicely.
+  size_t max_digits = ((num_files - 1) / 10) + 1;
+  size_t num_digits = (index / 10) + 1;
+  DCHECK_LE(num_digits, max_digits);
+  size_t padding = max_digits - num_digits;
+
+  file_name << file_prefix_ << "_";
+  for (size_t i = 0; i < padding; ++i) {
+    file_name << "0";
+  }
+  file_name << index;
+
+  Pathname file_path(dir_path_);
+  file_path.SetFilename(file_name.str());
+  return file_path.pathname();
+}
+
+///////////////////////////////////////////////////////////////////////////////
+// MobileDeviceLogStream
+///////////////////////////////////////////////////////////////////////////////
+
+MobileDeviceLogStream::MobileDeviceLogStream(const std::string& dir_path)
+    : FileRotatingStream(dir_path, kLogPrefix),
+      max_total_log_size_(0),
+      num_rotations_(0) {
+}
+
+MobileDeviceLogStream::MobileDeviceLogStream(const std::string& dir_path,
+                                             size_t max_total_log_size)
+    : FileRotatingStream(dir_path,
+                         kLogPrefix,
+                         max_total_log_size / 2,
+                         GetNumRotatingLogFiles(max_total_log_size) + 1),
+      max_total_log_size_(max_total_log_size),
+      num_rotations_(0) {
+  DCHECK_GE(max_total_log_size, 4u);

[jiayl2, 2015/07/20 23:27:06]

nit: document this in the .h file

[tkchin_webrtc, 2015/07/21 20:39:12]

Done.

+}
+
+const char* MobileDeviceLogStream::kLogPrefix = "webrtc_log";
+const size_t MobileDeviceLogStream::kRotatingLogFileDefaultSize = 1024 * 1024;
+
+void MobileDeviceLogStream::OnRotation() {
+  ++num_rotations_;
+  if (num_rotations_ == 1) {
+    // On the first rotation adjust the max file size so subsequent files after
+    // the first are smaller.
+    SetMaxFileSize(GetRotatingLogSize(max_total_log_size_));
+  } else if (num_rotations_ == (GetNumFiles() - 1)) {
+    // On the next rotation the very first file is going to be deleted. Change
+    // the last index so this doesn't happen.
+    SetLastFileIndex(GetLastFileIndex() - 1);
+  }
+}
+
+size_t MobileDeviceLogStream::GetRotatingLogSize(size_t max_total_log_size) {
+  size_t num_rotating_log_files = GetNumRotatingLogFiles(max_total_log_size);
+  size_t rotating_log_size = num_rotating_log_files > 2
+                                 ? kRotatingLogFileDefaultSize
+                                 : max_total_log_size / 4;
+  return rotating_log_size;
+}
+
+size_t MobileDeviceLogStream::GetNumRotatingLogFiles(
+    size_t max_total_log_size) {
+  // At minimum have two rotating files. Otherwise split the available log size
+  // evenly across 1MB files.
+  return std::max((size_t)2,
+                  (max_total_log_size / 2) / kRotatingLogFileDefaultSize);
+}
+
 CircularFileStream::CircularFileStream(size_t max_size)
     : max_write_size_(max_size),
       position_(0),