[master] Merged trac3752 (logger immediate flush)

doc/guide/logging.xml

@@ -535,7 +535,6 @@ TODO; there's a ticket to determine these levels, see #1074
 
         </para>
 
-        <!-- configuration of flush is not supported yet.
         <section>
           <title>flush (true of false)</title>
 
@@ -543,10 +542,10 @@ TODO; there's a ticket to determine these levels, see #1074
             Flush buffers after each log message. Doing this will
             reduce performance but will ensure that if the program
             terminates abnormally, all messages up to the point of
-            termination are output.
+            termination are output. Default is true.
           </para>
 
-        </section> -->
+        </section>
 
         <section>
           <title>maxsize (integer)</title>
@@ -638,7 +637,7 @@ file be created.</para>
                     "output": "/var/log/kea-debug.log",
                     "maxver": 8,
                     "maxsize": 204800,
-                    "destination": "file"
+                    "flush": true
                 }
             ],
             "severity": "DEBUG",

src/lib/dhcpsrv/Makefile.am

@@ -177,6 +177,9 @@ EXTRA_DIST += hosts_messages.mes
 # Database schema creation script moved to src/bin/admin
 EXTRA_DIST += database_backends.dox libdhcpsrv.dox
 
+# Specification file
+EXTRA_DIST += logging.spec
+
 install-data-local:
 	$(mkinstalldirs) $(DESTDIR)$(dhcp_data_dir)
 

src/lib/dhcpsrv/logging.cc

@@ -147,6 +147,11 @@ void LogConfigParser::parseOutputOptions(std::vector<LoggingDestination>& destin
             dest.maxsize_ = boost::lexical_cast<uint64_t>(maxsize_ptr->str());
         }
 
+        isc::data::ConstElementPtr flush_ptr = output_option->get("flush");
+        if (flush_ptr) {
+            dest.flush_ = flush_ptr->boolValue();
+        }
+
         destination.push_back(dest);
     }
 }

src/lib/dhcpsrv/logging.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014, 2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -34,7 +34,8 @@ namespace dhcp {
 ///         {
 ///             "output": "/home/thomson/kea-inst/kea-warn.log",
 ///             "maxver": 8,
-///             "maxsize": 204800
+///             "maxsize": 204800,
+///             "flush": true
 ///         }
 ///     ],
 ///     "severity": "WARN"
@@ -74,15 +75,8 @@ private:
 
     /// @brief Parses output_options structure
     ///
-    /// An example data structure that holds output_options in JSON format
-    /// looks like this:
-    ///     "output_options": [
-    ///         {
-    ///             "output": "/var/log/kea-warn.log",
-    ///             "maxver": 8,
-    ///             "maxsize": 204800
-    ///         }
-    ///     ],
+    /// @ref @c LogConfigParser for an example in JSON format.
+    ///
     /// @param destination parsed parameters will be stored here
     /// @param output_options element to be parsed
     void parseOutputOptions(std::vector<LoggingDestination>& destination,

src/lib/dhcpsrv/logging.spec

@@ -0,0 +1,94 @@
+{
+  "module_spec": {
+    "module_name": "Logging",
+    "module_description": "Logging configuration",
+    "config_data": [
+      {
+        "item_name": "loggers",
+        "item_type": "list",
+        "item_optional": false,
+        "item_default": [],
+        "list_item_spec":
+        {
+          "item_name": "logger",
+          "item_type": "map",
+          "item_optional": false,
+          "item_default": {},
+          "map_item_spec": [
+          {
+            "item_name": "name",
+            "item_type": "string",
+            "item_optional": false,
+            "item_default": "kea",
+            "item_description": "Logging name"
+          },
+
+          {
+            "item_name": "output_options",
+            "item_type": "list",
+            "item_optional": true,
+            "item_default": [],
+            "list_item_spec":
+            {
+               "item_name": "output_option",
+               "item_type": "map",
+               "item_optional": false,
+               "item_default": {},
+               "item_description": "Options for a logging destination",
+               "map_item_spec": [
+               {
+                 "item_name": "output",
+                 "item_type": "string",
+                 "item_optional": false,
+                 "item_default": "stdout",
+                 "item_description": "Logging destination (stdout, stderr, syslog, syslog:name, file name)"
+               },
+
+               {
+                 "item_name": "maxver",
+                 "item_type": "integer",
+                 "item_optional": true,
+                 "item_default": 1,
+                 "item_description": "Maximum number of log files in rotation"
+               },
+
+               {
+                 "item_name": "maxsize",
+                 "item_type": "integer",
+                 "item_optional": true,
+                 "item_default": 204800,
+                 "item_description": "Maximum log file size"
+               },
+
+               {
+                 "item_name": "flush",
+                 "item_type": "boolean",
+                 "item_optional": true,
+                 "item_default": true,
+                 "item_description": "Immediate flush"
+               }
+               ]
+             }
+          },
+
+          {
+            "item_name": "severity",
+            "item_type": "string",
+            "item_optional": false,
+            "item_default": "INFO",
+            "item_description": "Logging severity"
+          },
+
+          {
+            "item_name": "debuglevel",
+            "item_type": "integer",
+            "item_optional": true,
+            "item_default": 0,
+            "item_description": "Debug level (for DEBUG severity, 0..99 range)"
+          }
+          ]
+        }
+      }
+    ]
+  }
+}

src/lib/dhcpsrv/logging_info.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2014, 2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -26,7 +26,8 @@ bool
 LoggingDestination::equals(const LoggingDestination& other) const {
     return (output_ == other.output_ &&
             maxver_ == other.maxver_ &&
-            maxsize_ == other.maxsize_);
+            maxsize_ == other.maxsize_ &&
+            flush_ == other.flush_);
 }
 
 LoggingInfo::LoggingInfo()
@@ -134,6 +135,9 @@ LoggingInfo::toSpec() const {
             option.filename = dest->output_;
         }
 
+        // Copy the immediate flush flag
+        option.flush = dest->flush_;
+
         // ... and set the destination
         spec.addOutputOption(option);
     }

src/lib/dhcpsrv/logging_info.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -40,6 +40,9 @@ struct LoggingDestination {
     /// @brief Maximum log file size
     uint64_t maxsize_;
 
+    /// @brief Immediate flush
+    bool flush_;
+
     /// @brief Compares two objects for equality.
     ///
     /// @param other Object to be compared with this object.
@@ -49,7 +52,7 @@ struct LoggingDestination {
 
     /// @brief Default constructor.
     LoggingDestination()
-        : output_("stdout"), maxver_(1), maxsize_(204800) {
+        : output_("stdout"), maxver_(1), maxsize_(204800), flush_(true) {
     }
 };
 
@@ -63,7 +66,8 @@ struct LoggingDestination {
 ///                {
 ///                    "output": "/path/to/the/logfile.log",
 ///                    "maxver": 8,
-///                    "maxsize": 204800
+///                    "maxsize": 204800,
+///                    "flush": true
 ///                }
 ///            ],
 ///            "severity": "WARN",

src/lib/dhcpsrv/tests/logging_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -15,6 +15,7 @@
 #include <config.h>
 #include <exceptions/exceptions.h>
 #include <cc/data.h>
+#include <config/module_spec.h>
 #include <dhcpsrv/logging.h>
 #include <gtest/gtest.h>
 #include <log/logger_support.h>
@@ -45,6 +46,11 @@ class LoggingTest : public ::testing::Test {
         }
 };
 
+// Tests that the spec file is valid.
+TEST_F(LoggingTest, basicSpec) {
+    std::string specfile = std::string(TEST_DATA_BUILDDIR) + "/../logging.spec";
+    ASSERT_NO_THROW(isc::config::moduleSpecFromFile(specfile));
+}
 
 // Checks that contructor is able to process specified storage properly
 TEST_F(LoggingTest, constructor) {
@@ -68,7 +74,8 @@ TEST_F(LoggingTest, parsingConsoleOutput) {
     "        \"name\": \"kea\","
     "        \"output_options\": ["
     "            {"
-    "                \"output\": \"stdout\""
+    "                \"output\": \"stdout\","
+    "                \"flush\": true"
     "            }"
     "        ],"
     "        \"debuglevel\": 99,"
@@ -96,6 +103,7 @@ TEST_F(LoggingTest, parsingConsoleOutput) {
 
     ASSERT_EQ(1, storage->getLoggingInfo()[0].destinations_.size());
     EXPECT_EQ("stdout" , storage->getLoggingInfo()[0].destinations_[0].output_);
+    EXPECT_TRUE(storage->getLoggingInfo()[0].destinations_[0].flush_);
 }
 
 // Checks if the LogConfigParser class is able to transform JSON structures
@@ -136,6 +144,8 @@ TEST_F(LoggingTest, parsingFile) {
 
     ASSERT_EQ(1, storage->getLoggingInfo()[0].destinations_.size());
     EXPECT_EQ("logfile.txt" , storage->getLoggingInfo()[0].destinations_[0].output_);
+    // Default for immediate flush is true
+    EXPECT_TRUE(storage->getLoggingInfo()[0].destinations_[0].flush_);
 }
 
 // Checks if the LogConfigParser class is able to transform data structures
@@ -149,7 +159,8 @@ TEST_F(LoggingTest, multipleLoggers) {
     "        \"name\": \"kea\","
     "        \"output_options\": ["
     "            {"
-    "                \"output\": \"logfile.txt\""
+    "                \"output\": \"logfile.txt\","
+    "                \"flush\": true"
     "            }"
     "        ],"
     "        \"severity\": \"INFO\""
@@ -158,7 +169,8 @@ TEST_F(LoggingTest, multipleLoggers) {
     "        \"name\": \"wombat\","
     "        \"output_options\": ["
     "            {"
-    "                \"output\": \"logfile2.txt\""
+    "                \"output\": \"logfile2.txt\","
+    "                \"flush\": false"
     "            }"
     "        ],"
     "        \"severity\": \"DEBUG\","
@@ -185,12 +197,14 @@ TEST_F(LoggingTest, multipleLoggers) {
     EXPECT_EQ(isc::log::INFO, storage->getLoggingInfo()[0].severity_);
     ASSERT_EQ(1, storage->getLoggingInfo()[0].destinations_.size());
     EXPECT_EQ("logfile.txt" , storage->getLoggingInfo()[0].destinations_[0].output_);
+    EXPECT_TRUE(storage->getLoggingInfo()[0].destinations_[0].flush_);
 
     EXPECT_EQ("wombat", storage->getLoggingInfo()[1].name_);
     EXPECT_EQ(99, storage->getLoggingInfo()[1].debuglevel_);
     EXPECT_EQ(isc::log::DEBUG, storage->getLoggingInfo()[1].severity_);
     ASSERT_EQ(1, storage->getLoggingInfo()[1].destinations_.size());
     EXPECT_EQ("logfile2.txt" , storage->getLoggingInfo()[1].destinations_[0].output_);
+    EXPECT_FALSE(storage->getLoggingInfo()[1].destinations_[0].flush_);
 }
 
 // Checks if the LogConfigParser class is able to transform data structures
@@ -233,9 +247,13 @@ TEST_F(LoggingTest, multipleLoggingDestinations) {
     EXPECT_EQ(isc::log::INFO, storage->getLoggingInfo()[0].severity_);
     ASSERT_EQ(2, storage->getLoggingInfo()[0].destinations_.size());
     EXPECT_EQ("logfile.txt" , storage->getLoggingInfo()[0].destinations_[0].output_);
+    EXPECT_TRUE(storage->getLoggingInfo()[0].destinations_[0].flush_);
     EXPECT_EQ("stdout" , storage->getLoggingInfo()[0].destinations_[1].output_);
+    EXPECT_TRUE(storage->getLoggingInfo()[0].destinations_[1].flush_);
 }
 
+/// @todo Add tests for malformed logging configuration
+
 /// @todo There is no easy way to test applyConfiguration() and defaultLogging().
 /// To test them, it would require instrumenting log4cplus to actually fake
 /// the logging set up. Alternatively, we could develop set of test suites