BIND 10 master, updated. dcdabc780fce8d02c9263f8e98f03b29bb4e5210 [master] update changelog

BIND 10 source code commits bind10-changes at lists.isc.org
Mon Jun 27 12:28:17 UTC 2011


The branch, master has been updated
       via  dcdabc780fce8d02c9263f8e98f03b29bb4e5210 (commit)
       via  0fad7d4a8557741f953eda9fed1d351a3d9dc5ef (commit)
       via  701c0d6d7c484c2f46951d23fba47c760363b7e4 (commit)
       via  d57f30ffe93b7f45aa6492ea1fba5d594adc01df (commit)
       via  690dafd743f765f04b21d3ce15ec0a63da6a53bd (commit)
       via  251a32a1fd1e7be23d59790e57a4b40fbcdceae3 (commit)
       via  935bd760ed4f39213f8db8eab730bf41dc217da9 (commit)
       via  78cffeb00933814658da0867ada0209403946b51 (commit)
       via  417893fc06dcd5339e2cd0278a6badbbe847d6c4 (commit)
       via  8e715d5202d79361622e89ef11a0d433558768f8 (commit)
      from  3e861eb6aec036b3c5a2f6a71c6ff3adbdc9a55a (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit dcdabc780fce8d02c9263f8e98f03b29bb4e5210
Author: Jelte Jansen <jelte at isc.org>
Date:   Mon Jun 27 14:26:33 2011 +0200

    [master] update changelog

commit 0fad7d4a8557741f953eda9fed1d351a3d9dc5ef
Merge: 3e861eb6aec036b3c5a2f6a71c6ff3adbdc9a55a 701c0d6d7c484c2f46951d23fba47c760363b7e4
Author: Jelte Jansen <jelte at isc.org>
Date:   Mon Jun 27 14:18:22 2011 +0200

    Merge branch 'trac1004'

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog                                    |    7 ++
 src/bin/cfgmgr/plugins/b10logging.py         |   19 +++-
 src/bin/cfgmgr/plugins/tests/Makefile.am     |    2 +-
 src/bin/cfgmgr/plugins/tests/logging_test.py |  135 ++++++++++++++++++++++++++
 src/lib/cc/data.h                            |    2 +-
 src/lib/config/ccsession.cc                  |   52 ++++++++++-
 src/lib/config/ccsession.h                   |   37 +++++++-
 src/lib/config/tests/ccsession_unittests.cc  |   62 ++++++++++++
 8 files changed, 307 insertions(+), 9 deletions(-)
 create mode 100644 src/bin/cfgmgr/plugins/tests/logging_test.py

-----------------------------------------------------------------------
diff --git a/ChangeLog b/ChangeLog
index 61cc893..c1ff9d0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+263.	[func]      jelte
+	Logging configuration can now also accept a * as a first-level
+	name (e.g. '*', or '*.cache'), indicating that every module
+	should use that configuration, unless overridden by an explicit
+	logging configuration for that module
+	(Trac 1004, git 0fad7d4a8557741f953eda9fed1d351a3d9dc5ef)
+
 262.	[func]      stephen
 	Add some initial documentation about the logging framework.
 	Provide BIND 10 Messages Manual in HTML and DocBook? XML formats.
diff --git a/src/bin/cfgmgr/plugins/b10logging.py b/src/bin/cfgmgr/plugins/b10logging.py
index 6af3f66..e288c6d 100644
--- a/src/bin/cfgmgr/plugins/b10logging.py
+++ b/src/bin/cfgmgr/plugins/b10logging.py
@@ -48,6 +48,19 @@ def check(config):
         for logger in config['loggers']:
             # name should always be present
             name = logger['name']
+            # report an error if name starts with * but not *.,
+            # or if * is not the first character.
+            # TODO: we might want to also warn or error if the
+            # logger name is not an existing module, but we can't
+            # really tell that from here at this point
+            star_pos = name.find('*')
+            if star_pos > 0 or\
+               name == '*.' or\
+               (star_pos == 0 and len(name) > 1 and name[1] != '.'):
+                errors.append("Bad logger name: '" + name + "': * can "
+                              "only be used instead of the full "
+                              "first-level name, e.g. '*' or "
+                              "'*.subsystem'")
 
             if 'severity' in logger and\
                logger['severity'].lower() not in ALLOWED_SEVERITIES:
@@ -71,11 +84,11 @@ def check(config):
                                'output' in output_option and\
                                output_option['output'] not in ALLOWED_STREAMS:
                                 errors.append("bad output for logger " + name +
-                                              ": " + output_option['stream'] +
+                                              ": " + output_option['output'] +
                                               ", must be stdout or stderr")
                             elif destination == "file" and\
-                                 'output' not in output_option or\
-                                 output_option['output'] == "":
+                                 ('output' not in output_option or\
+                                  output_option['output'] == ""):
                                     errors.append("destination set to file but "
                                                   "output not set to any "
                                                   "filename for logger "
diff --git a/src/bin/cfgmgr/plugins/tests/Makefile.am b/src/bin/cfgmgr/plugins/tests/Makefile.am
index 725d391..07b7a85 100644
--- a/src/bin/cfgmgr/plugins/tests/Makefile.am
+++ b/src/bin/cfgmgr/plugins/tests/Makefile.am
@@ -1,5 +1,5 @@
 PYCOVERAGE_RUN = @PYCOVERAGE_RUN@
-PYTESTS = tsig_keys_test.py
+PYTESTS = tsig_keys_test.py logging_test.py
 
 EXTRA_DIST = $(PYTESTS)
 
diff --git a/src/bin/cfgmgr/plugins/tests/logging_test.py b/src/bin/cfgmgr/plugins/tests/logging_test.py
new file mode 100644
index 0000000..818a596
--- /dev/null
+++ b/src/bin/cfgmgr/plugins/tests/logging_test.py
@@ -0,0 +1,135 @@
+# Copyright (C) 2011  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+# Make sure we can load the module, put it into path
+import sys
+import os
+sys.path.extend(os.environ["B10_TEST_PLUGIN_DIR"].split(':'))
+
+import b10logging
+import unittest
+
+class LoggingConfCheckTest(unittest.TestCase):
+    def test_load(self):
+        """
+        Checks the entry point returns the correct values.
+        """
+        (spec, check) = b10logging.load()
+        # It returns the checking function
+        self.assertEqual(check, b10logging.check)
+        # The plugin stores it's spec
+        self.assertEqual(spec, b10logging.spec)
+
+    def test_logger_conf(self):
+        self.assertEqual(None,
+                         b10logging.check({'loggers':
+                                          [{'name': '*',
+                                            'severity': 'DEBUG',
+                                            'debuglevel': 50,
+                                            'output_options':
+                                            [{'destination': 'file',
+                                              'output': '/some/file'
+                                            }]
+                                           },
+                                           {'name': 'b10-resolver',
+                                            'severity': 'WARN',
+                                            'additive': True,
+                                            'output_options':
+                                            [{'destination': 'console',
+                                              'output': 'stderr',
+                                              'flush': True
+                                            }]
+                                           },
+                                           {'name': 'b10-resolver.resolver',
+                                            'severity': 'ERROR',
+                                            'output_options': []
+                                           },
+                                           {'name': '*.cache',
+                                            'severity': 'INFO'
+                                           }
+                                          ]}))
+    def do_bad_name_test(self, name):
+        err_str = "Bad logger name: '" + name + "': * can only be "\
+                  "used instead of the full first-level name, e.g. "\
+                  "'*' or '*.subsystem'"
+        self.assertEqual(err_str,
+                         b10logging.check({'loggers':
+                                          [{'name': name,
+                                            'severity': 'DEBUG'},
+                                          ]}))
+        
+    def test_logger_bad_name(self):
+        self.do_bad_name_test("*.")
+        self.do_bad_name_test("*foo")
+        self.do_bad_name_test("*foo.lib")
+        self.do_bad_name_test("foo*")
+        self.do_bad_name_test("foo*.lib")
+
+    def test_logger_bad_severity(self):
+        self.assertEqual('bad severity value for logger *: BADVAL',
+                         b10logging.check({'loggers':
+                                          [{'name': '*',
+                                            'severity': 'BADVAL'}]}))
+
+    def test_logger_bad_destination(self):
+        self.assertEqual('bad destination for logger *: baddest',
+                         b10logging.check({'loggers':
+                                          [{'name': '*',
+                                            'severity': 'INFO',
+                                            'output_options': [
+                                            { 'destination': 'baddest' }
+                                            ]}]}))
+
+    def test_logger_bad_console_output(self):
+        self.assertEqual('bad output for logger *: bad_output, must be stdout or stderr',
+                         b10logging.check({'loggers':
+                                          [{'name': '*',
+                                            'severity': 'INFO',
+                                            'output_options': [
+                                            { 'destination': 'console',
+                                              'output': 'bad_output'
+                                            }
+                                            ]}]}))
+
+    def test_logger_bad_file_output(self):
+        self.assertEqual('destination set to file but output not set to any filename for logger *',
+                         b10logging.check({'loggers':
+                                          [{'name': '*',
+                                            'severity': 'INFO',
+                                            'output_options': [
+                                            { 'destination': 'file' }
+                                            ]}]}))
+
+    def test_logger_bad_syslog_output(self):
+        self.assertEqual('destination set to syslog but output not set to any facility for logger *',
+                         b10logging.check({'loggers':
+                                          [{'name': '*',
+                                            'severity': 'INFO',
+                                            'output_options': [
+                                            { 'destination': 'syslog' }
+                                            ]}]}))
+
+    def test_logger_bad_type(self):
+        self.assertEqual('123 should be a string',
+                         b10logging.check({'loggers':
+                                          [{'name': 123,
+                                            'severity': 'INFO'}]}))
+        self.assertEqual('123 should be a string',
+                         b10logging.check({'loggers':
+                                          [{'name': 'bind10',
+                                            'severity': 123}]}))
+
+if __name__ == '__main__':
+        unittest.main()
diff --git a/src/lib/cc/data.h b/src/lib/cc/data.h
index 0a363f4..5c731e6 100644
--- a/src/lib/cc/data.h
+++ b/src/lib/cc/data.h
@@ -479,7 +479,7 @@ public:
         return (true);
     }
     using Element::setValue;
-    bool setValue(std::map<std::string, ConstElementPtr>& v) {
+    bool setValue(const std::map<std::string, ConstElementPtr>& v) {
         m = v;
         return (true);
     }
diff --git a/src/lib/config/ccsession.cc b/src/lib/config/ccsession.cc
index 9285940..6b094ec 100644
--- a/src/lib/config/ccsession.cc
+++ b/src/lib/config/ccsession.cc
@@ -23,6 +23,7 @@
 #include <fstream>
 #include <sstream>
 #include <cerrno>
+#include <set>
 
 #include <boost/bind.hpp>
 #include <boost/foreach.hpp>
@@ -38,6 +39,7 @@
 #include <log/logger_support.h>
 #include <log/logger_specification.h>
 #include <log/logger_manager.h>
+#include <log/logger_name.h>
 
 using namespace std;
 
@@ -213,7 +215,8 @@ readLoggersConf(std::vector<isc::log::LoggerSpecification>& specs,
                 ConstElementPtr logger,
                 const ConfigData& config_data)
 {
-    const std::string lname = logger->get("name")->stringValue();
+    std::string lname = logger->get("name")->stringValue();
+
     ConstElementPtr severity_el = getValueOrDefault(logger,
                                       "severity", config_data,
                                       "loggers/severity");
@@ -246,6 +249,50 @@ readLoggersConf(std::vector<isc::log::LoggerSpecification>& specs,
 
 } // end anonymous namespace
 
+
+ConstElementPtr
+getRelatedLoggers(ConstElementPtr loggers) {
+    // Keep a list of names for easier lookup later
+    std::set<std::string> our_names;
+    const std::string& root_name = isc::log::getRootLoggerName();
+
+    ElementPtr result = isc::data::Element::createList();
+
+    BOOST_FOREACH(ConstElementPtr cur_logger, loggers->listValue()) {
+        const std::string cur_name = cur_logger->get("name")->stringValue();
+        if (cur_name == root_name || cur_name.find(root_name + ".") == 0) {
+            our_names.insert(cur_name);
+            result->add(cur_logger);
+        }
+    }
+
+    // now find the * names
+    BOOST_FOREACH(ConstElementPtr cur_logger, loggers->listValue()) {
+        std::string cur_name = cur_logger->get("name")->stringValue();
+        // if name is '*', or starts with '*.', replace * with root
+        // logger name
+        if (cur_name == "*" || cur_name.length() > 1 &&
+            cur_name[0] == '*' && cur_name[1] == '.') {
+
+            cur_name = root_name + cur_name.substr(1);
+            // now add it to the result list, but only if a logger with
+            // that name was not configured explicitely
+            if (our_names.find(cur_name) == our_names.end()) {
+                // we substitute the name here already, but as
+                // we are dealing with consts, we copy the data
+                ElementPtr new_logger(Element::createMap());
+                // since we'll only be updating one first-level element,
+                // and we return as const again, a shallow map copy is
+                // enough
+                new_logger->setValue(cur_logger->mapValue());
+                new_logger->set("name", Element::create(cur_name));
+                result->add(new_logger);
+            }
+        }
+    }
+    return result;
+}
+
 void
 default_logconfig_handler(const std::string& module_name,
                           ConstElementPtr new_config,
@@ -255,8 +302,9 @@ default_logconfig_handler(const std::string& module_name,
     std::vector<isc::log::LoggerSpecification> specs;
 
     if (new_config->contains("loggers")) {
+        ConstElementPtr loggers = getRelatedLoggers(new_config->get("loggers"));
         BOOST_FOREACH(ConstElementPtr logger,
-                      new_config->get("loggers")->listValue()) {
+                      loggers->listValue()) {
             readLoggersConf(specs, logger, config_data);
         }
     }
diff --git a/src/lib/config/ccsession.h b/src/lib/config/ccsession.h
index 0d4b7f3..7dc34ba 100644
--- a/src/lib/config/ccsession.h
+++ b/src/lib/config/ccsession.h
@@ -373,8 +373,41 @@ default_logconfig_handler(const std::string& module_name,
                           isc::data::ConstElementPtr new_config,
                           const ConfigData& config_data);
 
-}
-}
+
+/// \brief Returns the loggers related to this module
+///
+/// This function does two things;
+/// - it drops the configuration parts for loggers for other modules
+/// - it replaces the '*' in the name of the loggers by the name of
+///   this module, but *only* if the expanded name is not configured
+///   explicitely
+///
+/// Examples: if this is the module b10-resolver,
+/// For the config names ['*', 'b10-auth']
+/// The '*' is replaced with 'b10-resolver', and this logger is used.
+/// 'b10-auth' is ignored (of course, it will not be in the b10-auth
+/// module).
+///
+/// For ['*', 'b10-resolver']
+/// The '*' is ignored, and only 'b10-resolver' is used.
+///
+/// For ['*.reslib', 'b10-resolver']
+/// Or ['b10-resolver.reslib', '*']
+/// Both are used, where the * will be expanded to b10-resolver
+///
+/// \note This is a public function at this time, but mostly for
+/// the purposes of testing. Once we can directly test what loggers
+/// are running, this function may be moved to the unnamed namespace
+///
+/// \param loggers the original 'loggers' config list
+/// \return ListElement containing only loggers relevant for this
+///         module, where * is replaced by the root logger name
+isc::data::ConstElementPtr
+getRelatedLoggers(isc::data::ConstElementPtr loggers);
+
+} // namespace config
+
+} // namespace isc
 #endif // __CCSESSION_H
 
 // Local Variables:
diff --git a/src/lib/config/tests/ccsession_unittests.cc b/src/lib/config/tests/ccsession_unittests.cc
index e5fe049..e1a4f9d 100644
--- a/src/lib/config/tests/ccsession_unittests.cc
+++ b/src/lib/config/tests/ccsession_unittests.cc
@@ -24,6 +24,8 @@
 
 #include <config/tests/data_def_unittests_config.h>
 
+#include <log/logger_name.h>
+
 using namespace isc::data;
 using namespace isc::config;
 using namespace isc::cc;
@@ -632,4 +634,64 @@ TEST_F(CCSessionTest, doubleStartWithAddRemoteConfig) {
     EXPECT_THROW(mccs.addRemoteConfig(ccspecfile("spec2.spec")),
                  FakeSession::DoubleRead);
 }
+
+namespace {
+void doRelatedLoggersTest(const char* input, const char* expected) {
+    ConstElementPtr all_conf = isc::data::Element::fromJSON(input);
+    ConstElementPtr expected_conf = isc::data::Element::fromJSON(expected);
+    EXPECT_EQ(*expected_conf, *isc::config::getRelatedLoggers(all_conf));
+}
+} // end anonymous namespace
+
+TEST(LogConfigTest, relatedLoggersTest) {
+    // make sure logger configs for 'other' programs are ignored,
+    // and that * is substituted correctly
+    // The default root logger name is "bind10"
+    doRelatedLoggersTest("[{ \"name\": \"other_module\" }]",
+                         "[]");
+    doRelatedLoggersTest("[{ \"name\": \"other_module.somelib\" }]",
+                         "[]");
+    doRelatedLoggersTest("[{ \"name\": \"bind10_other\" }]",
+                         "[]");
+    doRelatedLoggersTest("[{ \"name\": \"bind10_other.somelib\" }]",
+                         "[]");
+    doRelatedLoggersTest("[ { \"name\": \"other_module\" },"
+                         "  { \"name\": \"bind10\" }]",
+                         "[ { \"name\": \"bind10\" } ]");
+    doRelatedLoggersTest("[ { \"name\": \"bind10\" }]",
+                         "[ { \"name\": \"bind10\" } ]");
+    doRelatedLoggersTest("[ { \"name\": \"bind10.somelib\" }]",
+                         "[ { \"name\": \"bind10.somelib\" } ]");
+    doRelatedLoggersTest("[ { \"name\": \"other_module.somelib\" },"
+                         "  { \"name\": \"bind10.somelib\" }]",
+                         "[ { \"name\": \"bind10.somelib\" } ]");
+    doRelatedLoggersTest("[ { \"name\": \"other_module.somelib\" },"
+                         "  { \"name\": \"bind10\" },"
+                         "  { \"name\": \"bind10.somelib\" }]",
+                         "[ { \"name\": \"bind10\" },"
+                         "  { \"name\": \"bind10.somelib\" } ]");
+    doRelatedLoggersTest("[ { \"name\": \"*\" }]",
+                         "[ { \"name\": \"bind10\" } ]");
+    doRelatedLoggersTest("[ { \"name\": \"*.somelib\" }]",
+                         "[ { \"name\": \"bind10.somelib\" } ]");
+    doRelatedLoggersTest("[ { \"name\": \"*\", \"severity\": \"DEBUG\" },"
+                         "  { \"name\": \"bind10\", \"severity\": \"WARN\"}]",
+                         "[ { \"name\": \"bind10\", \"severity\": \"WARN\"} ]");
+    doRelatedLoggersTest("[ { \"name\": \"*\", \"severity\": \"DEBUG\" },"
+                         "  { \"name\": \"some_module\", \"severity\": \"WARN\"}]",
+                         "[ { \"name\": \"bind10\", \"severity\": \"DEBUG\"} ]");
+
+    // make sure 'bad' things like '*foo.x' or '*lib' are ignored
+    // (cfgmgr should have already caught it in the logconfig plugin
+    // check, and is responsible for reporting the error)
+    doRelatedLoggersTest("[ { \"name\": \"*foo\" }]",
+                         "[ ]");
+    doRelatedLoggersTest("[ { \"name\": \"*foo.bar\" }]",
+                         "[ ]");
+    doRelatedLoggersTest("[ { \"name\": \"*foo\" },"
+                         "  { \"name\": \"*foo.lib\" },"
+                         "  { \"name\": \"bind10\" } ]",
+                         "[ { \"name\": \"bind10\" } ]");
+}
+
 }




More information about the bind10-changes mailing list