From 70230b7a106820807c8d74fcf3d21ebab3442fbb Mon Sep 17 00:00:00 2001 From: Adam Simpkins Date: Tue, 28 Nov 2017 17:44:55 -0800 Subject: [PATCH] logging: add a FileHandlerFactory class Summary: Add a new LogHandlerFactory interface for creating LogHandler objects from a LogHandlerConfig. Also add an initial FileHandlerFactory implementation capable of creating LogHandler objects that write to a file descriptor. Reviewed By: bolinfest Differential Revision: D6200567 fbshipit-source-id: 14b86fc14ad475223aa4b57d45c40638b48c7594 --- CMakeLists.txt | 1 + folly/Makefile.am | 2 + .../experimental/logging/AsyncFileWriter.cpp | 12 + folly/experimental/logging/AsyncFileWriter.h | 31 ++- .../logging/FileHandlerFactory.cpp | 125 +++++++++ .../experimental/logging/FileHandlerFactory.h | 38 +++ .../logging/ImmediateFileWriter.h | 7 + .../experimental/logging/LogHandlerFactory.h | 70 +++++ folly/experimental/logging/Makefile.am | 1 + .../experimental/logging/StandardLogHandler.h | 25 +- .../logging/test/FileHandlerFactoryTest.cpp | 245 ++++++++++++++++++ 11 files changed, 554 insertions(+), 3 deletions(-) create mode 100644 folly/experimental/logging/FileHandlerFactory.cpp create mode 100644 folly/experimental/logging/FileHandlerFactory.h create mode 100644 folly/experimental/logging/LogHandlerFactory.h create mode 100644 folly/experimental/logging/test/FileHandlerFactoryTest.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index ec5a985c..8669d265 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -386,6 +386,7 @@ if (BUILD_TESTS) DIRECTORY experimental/logging/test/ TEST async_file_writer_test SOURCES AsyncFileWriterTest.cpp TEST config_parser_test SOURCES ConfigParserTest.cpp + TEST file_handler_factory_test SOURCES FileHandlerFactoryTest.cpp TEST glog_formatter_test SOURCES GlogFormatterTest.cpp TEST immediate_file_writer_test SOURCES ImmediateFileWriterTest.cpp TEST log_category_test SOURCES LogCategoryTest.cpp diff --git a/folly/Makefile.am b/folly/Makefile.am index 5c60a728..a1c508bc 100644 --- a/folly/Makefile.am +++ b/folly/Makefile.am @@ -156,6 +156,7 @@ nobase_follyinclude_HEADERS = \ experimental/JSONSchema.h \ experimental/LockFreeRingBuffer.h \ experimental/logging/AsyncFileWriter.h \ + experimental/logging/FileHandlerFactory.h \ experimental/logging/GlogStyleFormatter.h \ experimental/logging/ImmediateFileWriter.h \ experimental/logging/Init.h \ @@ -167,6 +168,7 @@ nobase_follyinclude_HEADERS = \ experimental/logging/Logger.h \ experimental/logging/LoggerDB.h \ experimental/logging/LogHandler.h \ + experimental/logging/LogHandlerFactory.h \ experimental/logging/LogHandlerConfig.h \ experimental/logging/LogLevel.h \ experimental/logging/LogMessage.h \ diff --git a/folly/experimental/logging/AsyncFileWriter.cpp b/folly/experimental/logging/AsyncFileWriter.cpp index fbd06415..18895ffd 100644 --- a/folly/experimental/logging/AsyncFileWriter.cpp +++ b/folly/experimental/logging/AsyncFileWriter.cpp @@ -25,6 +25,8 @@ using folly::StringPiece; namespace folly { +constexpr size_t AsyncFileWriter::kDefaultMaxBufferSize; + AsyncFileWriter::AsyncFileWriter(StringPiece path) : AsyncFileWriter{File{path.str(), O_WRONLY | O_APPEND | O_CREAT}} {} @@ -80,6 +82,16 @@ void AsyncFileWriter::flush() { } } +void AsyncFileWriter::setMaxBufferSize(size_t size) { + auto data = data_.lock(); + data->maxBufferBytes = size; +} + +size_t AsyncFileWriter::getMaxBufferSize() const { + auto data = data_.lock(); + return data->maxBufferBytes; +} + void AsyncFileWriter::ioThread() { folly::setThreadName("log_writer"); diff --git a/folly/experimental/logging/AsyncFileWriter.h b/folly/experimental/logging/AsyncFileWriter.h index 4fdd24eb..824b3c53 100644 --- a/folly/experimental/logging/AsyncFileWriter.h +++ b/folly/experimental/logging/AsyncFileWriter.h @@ -43,6 +43,13 @@ namespace folly { */ class AsyncFileWriter : public LogWriter { public: + /** + * The default maximum buffer size. + * + * The comments for setMaxBufferSize() explain how this parameter is used. + */ + static constexpr size_t kDefaultMaxBufferSize = 1024 * 1024; + /** * Construct an AsyncFileWriter that appends to the file at the specified * path. @@ -65,6 +72,28 @@ class AsyncFileWriter : public LogWriter { */ void flush() override; + /** + * Set the maximum buffer size for this AsyncFileWriter, in bytes. + * + * This controls the upper bound on how much unwritten data will be buffered + * in memory. If messages are being logged faster than they can be written + * to output file, new messages will be discarded if they would cause the + * amount of buffered data to exceed this limit. + */ + void setMaxBufferSize(size_t size); + + /** + * Get the maximum buffer size for this AsyncFileWriter, in bytes. + */ + size_t getMaxBufferSize() const; + + /** + * Get the output file. + */ + const folly::File& getFile() const { + return file_; + } + private: /* * A simple implementation using two queues. @@ -79,7 +108,7 @@ class AsyncFileWriter : public LogWriter { bool stop{false}; bool ioThreadDone{false}; uint64_t ioThreadCounter{0}; - size_t maxBufferBytes{1024 * 1024}; + size_t maxBufferBytes{kDefaultMaxBufferSize}; size_t currentBufferSize{0}; size_t numDiscarded{0}; diff --git a/folly/experimental/logging/FileHandlerFactory.cpp b/folly/experimental/logging/FileHandlerFactory.cpp new file mode 100644 index 00000000..43d3a18e --- /dev/null +++ b/folly/experimental/logging/FileHandlerFactory.cpp @@ -0,0 +1,125 @@ +/* + * Copyright 2004-present Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#include + +#include + +#include +#include +#include +#include +#include +#include + +using std::make_shared; +using std::shared_ptr; +using std::string; + +namespace folly { + +std::shared_ptr FileHandlerFactory::createHandler( + const Options& options) { + // Raise an error if we receive unexpected options + auto knownOptions = + std::set{"path", "stream", "async", "max_buffer_size"}; + for (const auto& entry : options) { + if (knownOptions.find(entry.first) == knownOptions.end()) { + throw std::invalid_argument(to( + "unknown parameter \"", entry.first, "\" for FileHandlerFactory")); + } + } + + // Construct the formatter + // TODO: We should eventually support parameters to control the formatter + // behavior. + auto formatter = make_shared(); + + // Get the output file to use + File outputFile; + auto* path = get_ptr(options, "path"); + auto* stream = get_ptr(options, "stream"); + if (path && stream) { + throw std::invalid_argument( + "cannot specify both \"path\" and \"stream\" " + "parameters for FileHandlerFactory"); + } else if (path) { + outputFile = File{*path, O_WRONLY | O_APPEND | O_CREAT}; + } else if (stream) { + if (*stream == "stderr") { + outputFile = File{STDERR_FILENO, /* ownsFd */ false}; + } else if (*stream == "stdout") { + outputFile = File{STDOUT_FILENO, /* ownsFd */ false}; + } else { + throw std::invalid_argument(to( + "unknown stream for FileHandlerFactory: \"", + *stream, + "\" expected one of stdout or stderr")); + } + } else { + throw std::invalid_argument( + "must specify a \"path\" or \"stream\" " + "parameter for FileHandlerFactory"); + } + + // Determine whether we should use ImmediateFileWriter or AsyncFileWriter + shared_ptr writer; + bool async = true; + auto* asyncOption = get_ptr(options, "async"); + if (asyncOption) { + try { + async = to(*asyncOption); + } catch (const std::exception& ex) { + throw std::invalid_argument(to( + "expected a boolean value for FileHandlerFactory \"async\" " + "parameter: ", + *asyncOption)); + } + } + auto* maxBufferOption = get_ptr(options, "max_buffer_size"); + if (async) { + auto asyncWriter = make_shared(std::move(outputFile)); + if (maxBufferOption) { + size_t maxBufferSize; + try { + maxBufferSize = to(*maxBufferOption); + } catch (const std::exception& ex) { + throw std::invalid_argument(to( + "expected an integer value for FileHandlerFactory " + "\"max_buffer_size\": ", + *maxBufferOption)); + } + if (maxBufferSize == 0) { + throw std::invalid_argument(to( + "expected a positive value for FileHandlerFactory " + "\"max_buffer_size\": ", + *maxBufferOption)); + } + asyncWriter->setMaxBufferSize(maxBufferSize); + } + writer = std::move(asyncWriter); + } else { + if (maxBufferOption) { + throw std::invalid_argument(to( + "the \"max_buffer_size\" option is only valid for async file " + "handlers")); + } + writer = make_shared(std::move(outputFile)); + } + + return make_shared(formatter, writer); +} + +} // namespace folly diff --git a/folly/experimental/logging/FileHandlerFactory.h b/folly/experimental/logging/FileHandlerFactory.h new file mode 100644 index 00000000..20142be5 --- /dev/null +++ b/folly/experimental/logging/FileHandlerFactory.h @@ -0,0 +1,38 @@ +/* + * Copyright 2004-present Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#pragma once + +#include + +namespace folly { + +/** + * FileHandlerFactory is a LogHandlerFactory that constructs log handlers + * that write to a file. + * + * It can construct handlers that use either ImmediateFileWriter or + * AsyncFileWriter. + */ +class FileHandlerFactory : public LogHandlerFactory { + public: + StringPiece getType() const override { + return "file"; + } + + std::shared_ptr createHandler(const Options& options) override; +}; + +} // namespace folly diff --git a/folly/experimental/logging/ImmediateFileWriter.h b/folly/experimental/logging/ImmediateFileWriter.h index 42c341b3..8678fa30 100644 --- a/folly/experimental/logging/ImmediateFileWriter.h +++ b/folly/experimental/logging/ImmediateFileWriter.h @@ -50,6 +50,13 @@ class ImmediateFileWriter : public LogWriter { void writeMessage(folly::StringPiece buffer, uint32_t flags = 0) override; void flush() override; + /** + * Get the output file. + */ + const folly::File& getFile() const { + return file_; + } + private: ImmediateFileWriter(ImmediateFileWriter const&) = delete; ImmediateFileWriter& operator=(ImmediateFileWriter const&) = delete; diff --git a/folly/experimental/logging/LogHandlerFactory.h b/folly/experimental/logging/LogHandlerFactory.h new file mode 100644 index 00000000..f85c5fbc --- /dev/null +++ b/folly/experimental/logging/LogHandlerFactory.h @@ -0,0 +1,70 @@ +/* + * Copyright 2004-present Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#pragma once + +#include +#include +#include + +#include +#include + +namespace folly { + +class LogHandler; + +class LogHandlerFactory { + public: + using Options = std::unordered_map; + + virtual ~LogHandlerFactory() = default; + + /** + * Get the type name of this LogHandlerFactory. + * + * The type field in the LogHandlerConfig for all LogHandlers created by this + * factory should match the type of the LogHandlerFactory. + * + * The type of a LogHandlerFactory should never change. The returned + * StringPiece should be valid for the lifetime of the LogHandlerFactory. + */ + virtual StringPiece getType() const = 0; + + /** + * Create a new LogHandler. + */ + virtual std::shared_ptr createHandler(const Options& options) = 0; + + /** + * Update an existing LogHandler with a new configuration. + * + * This may create a new LogHandler object, or it may update the existing + * LogHandler in place. + * + * The returned pointer will point to the input handler if it was updated in + * place, or will point to a new LogHandler if a new one was created. + */ + virtual std::shared_ptr updateHandler( + FOLLY_MAYBE_UNUSED const std::shared_ptr& existingHandler, + const Options& options) { + // Subclasses may override this with functionality to update an existing + // handler in-place. However, provide a default implementation that simply + // calls createHandler() to always create a new handler object. + return createHandler(options); + } +}; + +} // namespace folly diff --git a/folly/experimental/logging/Makefile.am b/folly/experimental/logging/Makefile.am index 56121025..61e64088 100644 --- a/folly/experimental/logging/Makefile.am +++ b/folly/experimental/logging/Makefile.am @@ -4,6 +4,7 @@ lib_LTLIBRARIES = libfollylogging.la libfollylogging_la_SOURCES = \ AsyncFileWriter.cpp \ + FileHandlerFactory.cpp \ GlogStyleFormatter.cpp \ ImmediateFileWriter.cpp \ Init.cpp \ diff --git a/folly/experimental/logging/StandardLogHandler.h b/folly/experimental/logging/StandardLogHandler.h index 94b2699a..3a1e4e48 100644 --- a/folly/experimental/logging/StandardLogHandler.h +++ b/folly/experimental/logging/StandardLogHandler.h @@ -44,6 +44,20 @@ class StandardLogHandler : public LogHandler { std::shared_ptr writer); ~StandardLogHandler(); + /** + * Get the LogFormatter used by this handler. + */ + const std::shared_ptr& getFormatter() const { + return formatter_; + } + + /** + * Get the LogWriter used by this handler. + */ + const std::shared_ptr& getWriter() const { + return writer_; + } + /** * Get the handler's current LogLevel. * @@ -71,7 +85,14 @@ class StandardLogHandler : public LogHandler { private: std::atomic level_{LogLevel::NONE}; - std::shared_ptr formatter_; - std::shared_ptr writer_; + + // The formatter_ and writer_ member variables are const, and cannot be + // modified after the StandardLogHandler is constructed. This allows them to + // be accessed without locking when handling a message. To change these + // values, create a new StandardLogHandler object and replace the old handler + // with the new one in the LoggerDB. + + const std::shared_ptr formatter_; + const std::shared_ptr writer_; }; } // namespace folly diff --git a/folly/experimental/logging/test/FileHandlerFactoryTest.cpp b/folly/experimental/logging/test/FileHandlerFactoryTest.cpp new file mode 100644 index 00000000..83724f29 --- /dev/null +++ b/folly/experimental/logging/test/FileHandlerFactoryTest.cpp @@ -0,0 +1,245 @@ +/* + * Copyright 2004-present Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#include + +#include +#include +#include +#include +#include +#include +#include + +using namespace folly; +using folly::test::TemporaryFile; +using std::make_pair; + +void checkAsyncWriter( + const LogWriter* writer, + const char* expectedPath, + size_t expectedMaxBufferSize) { + auto asyncWriter = dynamic_cast(writer); + ASSERT_TRUE(asyncWriter) + << "FileHandlerFactory should have created an AsyncFileWriter"; + EXPECT_EQ(expectedMaxBufferSize, asyncWriter->getMaxBufferSize()); + + // Make sure this refers to the expected output file + struct stat expectedStatInfo; + checkUnixError(stat(expectedPath, &expectedStatInfo), "stat failed"); + struct stat actualStatInfo; + checkUnixError( + fstat(asyncWriter->getFile().fd(), &actualStatInfo), "fstat failed"); + EXPECT_EQ(expectedStatInfo.st_dev, actualStatInfo.st_dev); + EXPECT_EQ(expectedStatInfo.st_ino, actualStatInfo.st_ino); +} + +void checkAsyncWriter( + const LogWriter* writer, + int expectedFD, + size_t expectedMaxBufferSize) { + auto asyncWriter = dynamic_cast(writer); + ASSERT_TRUE(asyncWriter) + << "FileHandlerFactory should have created an AsyncFileWriter"; + EXPECT_EQ(expectedMaxBufferSize, asyncWriter->getMaxBufferSize()); + EXPECT_EQ(expectedFD, asyncWriter->getFile().fd()); +} + +TEST(FileHandlerFactory, pathOnly) { + FileHandlerFactory factory; + + TemporaryFile tmpFile{"logging_test"}; + auto options = FileHandlerFactory::Options{ + make_pair("path", tmpFile.path().string()), + }; + auto handler = factory.createHandler(options); + + auto stdHandler = std::dynamic_pointer_cast(handler); + ASSERT_TRUE(stdHandler); + + auto formatter = + std::dynamic_pointer_cast(stdHandler->getFormatter()); + EXPECT_TRUE(formatter) + << "FileHandlerFactory should have created a GlogStyleFormatter"; + + checkAsyncWriter( + stdHandler->getWriter().get(), + tmpFile.path().string().c_str(), + AsyncFileWriter::kDefaultMaxBufferSize); +} + +TEST(FileHandlerFactory, stderrStream) { + FileHandlerFactory factory; + + TemporaryFile tmpFile{"logging_test"}; + auto options = FileHandlerFactory::Options{ + make_pair("stream", "stderr"), + }; + auto handler = factory.createHandler(options); + + auto stdHandler = std::dynamic_pointer_cast(handler); + ASSERT_TRUE(stdHandler); + + auto formatter = + std::dynamic_pointer_cast(stdHandler->getFormatter()); + EXPECT_TRUE(formatter) + << "FileHandlerFactory should have created a GlogStyleFormatter"; + + checkAsyncWriter( + stdHandler->getWriter().get(), + STDERR_FILENO, + AsyncFileWriter::kDefaultMaxBufferSize); +} + +TEST(FileHandlerFactory, stdoutWithMaxBuffer) { + FileHandlerFactory factory; + + TemporaryFile tmpFile{"logging_test"}; + auto options = FileHandlerFactory::Options{ + make_pair("stream", "stdout"), + make_pair("max_buffer_size", "4096"), + }; + auto handler = factory.createHandler(options); + + auto stdHandler = std::dynamic_pointer_cast(handler); + ASSERT_TRUE(stdHandler); + + auto formatter = + std::dynamic_pointer_cast(stdHandler->getFormatter()); + EXPECT_TRUE(formatter) + << "FileHandlerFactory should have created a GlogStyleFormatter"; + + checkAsyncWriter(stdHandler->getWriter().get(), STDOUT_FILENO, 4096); +} + +TEST(FileHandlerFactory, pathWithMaxBufferSize) { + FileHandlerFactory factory; + + TemporaryFile tmpFile{"logging_test"}; + auto options = FileHandlerFactory::Options{ + make_pair("path", tmpFile.path().string()), + make_pair("max_buffer_size", "4096000"), + }; + auto handler = factory.createHandler(options); + + auto stdHandler = std::dynamic_pointer_cast(handler); + ASSERT_TRUE(stdHandler); + + auto formatter = + std::dynamic_pointer_cast(stdHandler->getFormatter()); + EXPECT_TRUE(formatter) + << "FileHandlerFactory should have created a GlogStyleFormatter"; + + checkAsyncWriter( + stdHandler->getWriter().get(), tmpFile.path().string().c_str(), 4096000); +} + +TEST(FileHandlerFactory, nonAsyncStderr) { + FileHandlerFactory factory; + + TemporaryFile tmpFile{"logging_test"}; + auto options = FileHandlerFactory::Options{ + make_pair("stream", "stderr"), + make_pair("async", "no"), + }; + auto handler = factory.createHandler(options); + + auto stdHandler = std::dynamic_pointer_cast(handler); + ASSERT_TRUE(stdHandler); + + auto formatter = + std::dynamic_pointer_cast(stdHandler->getFormatter()); + EXPECT_TRUE(formatter) + << "FileHandlerFactory should have created a GlogStyleFormatter"; + + auto writer = + std::dynamic_pointer_cast(stdHandler->getWriter()); + ASSERT_TRUE(writer); + EXPECT_EQ(STDERR_FILENO, writer->getFile().fd()); +} + +TEST(FileHandlerFactory, errors) { + FileHandlerFactory factory; + TemporaryFile tmpFile{"logging_test"}; + + { + auto options = FileHandlerFactory::Options{}; + EXPECT_THROW(factory.createHandler(options), std::invalid_argument) + << "one of path or stream required"; + } + + { + auto options = FileHandlerFactory::Options{ + make_pair("path", tmpFile.path().string()), + make_pair("stream", "stderr"), + }; + EXPECT_THROW(factory.createHandler(options), std::invalid_argument) + << "path and stream cannot both be specified"; + } + + { + auto options = FileHandlerFactory::Options{ + make_pair("stream", "nonstdout"), + }; + EXPECT_THROW(factory.createHandler(options), std::invalid_argument) + << "invalid stream"; + } + + { + auto options = FileHandlerFactory::Options{ + make_pair("stream", "stderr"), + make_pair("async", "foobar"), + }; + EXPECT_THROW(factory.createHandler(options), std::invalid_argument) + << "invalid async value"; + } + + { + auto options = FileHandlerFactory::Options{ + make_pair("stream", "stderr"), + make_pair("async", "false"), + make_pair("max_buffer_size", "1234"), + }; + EXPECT_THROW(factory.createHandler(options), std::invalid_argument) + << "max_buffer_size only valid for async writers"; + } + + { + auto options = FileHandlerFactory::Options{ + make_pair("stream", "stderr"), + make_pair("max_buffer_size", "hello"), + }; + EXPECT_THROW(factory.createHandler(options), std::invalid_argument) + << "max_buffer_size must be an integer"; + } + + { + auto options = FileHandlerFactory::Options{ + make_pair("stream", "stderr"), + make_pair("max_buffer_size", "0"), + }; + EXPECT_THROW(factory.createHandler(options), std::invalid_argument) + << "max_buffer_size must be a positive integer"; + } + + { + auto options = FileHandlerFactory::Options{ + make_pair("stream", "stderr"), + make_pair("foo", "bar"), + }; + EXPECT_THROW(factory.createHandler(options), std::invalid_argument) + << "unknown parameter foo"; + } +} -- 2.34.1