From 297c6f0fb626693511df64be59e5bcb22b958e2e Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Sat, 4 Sep 2021 22:56:11 +0200 Subject: [PATCH 1/3] [#2040] Began stats UTs --- src/bin/d2/tests/Makefile.am | 1 + src/bin/d2/tests/dns_client_unittests.cc | 45 ++++++++++++++-- src/bin/d2/tests/stats_test_utils.cc | 54 +++++++++++++++++++ src/bin/d2/tests/stats_test_utils.h | 66 ++++++++++++++++++++++++ 4 files changed, 163 insertions(+), 3 deletions(-) create mode 100644 src/bin/d2/tests/stats_test_utils.cc create mode 100644 src/bin/d2/tests/stats_test_utils.h diff --git a/src/bin/d2/tests/Makefile.am b/src/bin/d2/tests/Makefile.am index 2d0013561c..820c4900c4 100644 --- a/src/bin/d2/tests/Makefile.am +++ b/src/bin/d2/tests/Makefile.am @@ -60,6 +60,7 @@ d2_unittests_SOURCES += get_config_unittest.cc d2_unittests_SOURCES += d2_command_unittest.cc d2_unittests_SOURCES += simple_add_unittests.cc d2_unittests_SOURCES += simple_remove_unittests.cc +d2_unittests_SOURCES += stats_test_utils.cc stats_test_utils.h d2_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) d2_unittests_LDFLAGS = $(AM_LDFLAGS) $(CRYPTO_LDFLAGS) diff --git a/src/bin/d2/tests/dns_client_unittests.cc b/src/bin/d2/tests/dns_client_unittests.cc index 85927cbe01..a2915cb0f4 100644 --- a/src/bin/d2/tests/dns_client_unittests.cc +++ b/src/bin/d2/tests/dns_client_unittests.cc @@ -16,6 +16,7 @@ #include #include #include +#include #include using namespace std; @@ -23,9 +24,10 @@ using namespace isc; using namespace isc::asiolink; using namespace isc::asiodns; using namespace isc::d2; - -using namespace isc; +using namespace isc::d2::test; +using namespace isc::data; using namespace isc::dns; +using namespace isc::stats; using namespace isc::util; using namespace boost::asio; using namespace boost::asio::ip; @@ -51,7 +53,7 @@ const long TEST_TIMEOUT = 5 * 1000; // properly handled a task may be hanging for a long time. In order to prevent // it, the asiolink::IntervalTimer is used to break a running test if test // timeout is hit. This will result in test failure. -class DNSClientTest : public virtual ::testing::Test, DNSClient::Callback { +class DNSClientTest : public virtual D2StatTest, DNSClient::Callback { public: IOService service_; D2UpdateMessagePtr response_; @@ -484,19 +486,36 @@ TEST_F(DNSClientTest, invalidTimeout) { // Verifies that TSIG can be used to sign requests and verify responses. TEST_F(DNSClientTest, runTSIGTest) { + ConstElementPtr stats_all = StatsMgr::instance().getAll(); + ASSERT_TRUE(stats_all); + EXPECT_TRUE(stats_all->empty()); std::string secret ("key number one"); D2TsigKeyPtr key_one; ASSERT_NO_THROW(key_one.reset(new D2TsigKey(Name("one.com"), TSIGKey::HMACMD5_NAME(), secret.c_str(), secret.size()))); + StatMap stats_key = { + { "update-sent", 0}, + { "update-success", 0}, + { "update-timeout", 0}, + { "update-error", 0} + }; + checkStats("one.com.", stats_key); secret = "key number two"; D2TsigKeyPtr key_two; ASSERT_NO_THROW(key_two.reset(new D2TsigKey(Name("two.com"), TSIGKey::HMACMD5_NAME(), secret.c_str(), secret.size()))); + checkStats("two.com.", stats_key); D2TsigKeyPtr nokey; + StatsMgr::instance().setValue("update-sent", 0LL); + StatsMgr::instance().setValue("update-signed", 0LL); + StatsMgr::instance().setValue("update-unsigned", 0LL); + StatsMgr::instance().setValue("update-success", 0LL); + StatsMgr::instance().setValue("update-timeout", 0LL); + StatsMgr::instance().setValue("update-error", 0LL); // Should be able to send and receive with no keys. // Neither client nor server will attempt to sign or verify. @@ -514,6 +533,26 @@ TEST_F(DNSClientTest, runTSIGTest) { // Client neither signs nor verifies, server responds with a signed answer // Since we are "liberal" in what we accept this should be ok. runTSIGTest(nokey, key_two); + + // Check statistics. + stats_all = StatsMgr::instance().getAll(); + StatMap stats_one = { + { "update-sent", 3}, + { "update-success", 1}, + { "update-timeout", 0}, + { "update-error", 2} + }; + checkStats("one.com.", stats_one); + checkStats("two.com.", stats_key); + StatMap stats_upd = { + { "update-sent", 5}, + { "update-signed", 3}, + { "update-unsigned", 2}, + { "update-success", 3}, + { "update-timeout", 0}, + { "update-error", 2} + }; + checkStats(stats_upd); } // Verify that the DNSClient receives the response from DNS and the received diff --git a/src/bin/d2/tests/stats_test_utils.cc b/src/bin/d2/tests/stats_test_utils.cc new file mode 100644 index 0000000000..905a765a22 --- /dev/null +++ b/src/bin/d2/tests/stats_test_utils.cc @@ -0,0 +1,54 @@ +// Copyright (C) 2021 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. +#include + +#include + +using namespace isc::data; +using namespace isc::stats; +using namespace std; + +namespace isc { +namespace d2 { +namespace test { + +D2StatTest::D2StatTest() { + StatsMgr::instance().removeAll(); +} + +D2StatTest::~D2StatTest() { + StatsMgr::instance().removeAll(); +} + +void +D2StatTest::checkStat(const string& name, const int64_t expected_value) { + ObservationPtr obs = StatsMgr::instance().getObservation(name); + ASSERT_TRUE(obs) << " stat: " << name << " not found "; + ASSERT_EQ(expected_value, obs->getInteger().first) + << " stat: " << name << " value wrong"; +} + +void +D2StatTest::checkStats(const StatMap& expected_stats) { + for (const auto& it : expected_stats) { + checkStat(it.first, it.second); + } +} + +void +D2StatTest::checkStats(const string& key_name, const StatMap& expected_stats) { + StatMap key_stats; + for (const auto& it : expected_stats) { + const string& stat_name = + StatsMgr::generateName("key", key_name, it.first); + key_stats[stat_name] = it.second; + } + checkStats(key_stats); +} + +} +} +} diff --git a/src/bin/d2/tests/stats_test_utils.h b/src/bin/d2/tests/stats_test_utils.h new file mode 100644 index 0000000000..1cc17fe559 --- /dev/null +++ b/src/bin/d2/tests/stats_test_utils.h @@ -0,0 +1,66 @@ +// Copyright (C) 2020 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#ifndef STATS_TEST_UTILS_H +#define STATS_TEST_UTILS_H + +#include +#include +#include +#include + +#include + +namespace isc { +namespace d2 { +namespace test { + +/// @brief Type of name x value for statistics. +typedef std::map StatMap; + +/// @brief Test fixture class with utility functions to test statistics. +class D2StatTest : public ::testing::Test { +public: + /// @brief Constructor. + D2StatTest(); + + /// @brief Destructor. + virtual ~D2StatTest(); + + /// @brief Compares a statistic to an expected value. + /// + /// Attempt to fetch the named statistic from the StatsMgr and if + /// found, compare its observed value to the given value. + /// Fails if the stat is not found or if the values do not match. + /// + /// @param name StatsMgr name for the statistic to check. + /// @param expected_value expected value of the statistic. + void checkStat(const std::string& name, const int64_t expected_value); + + /// @brief Compares StatsMgr statistics against expected values. + /// + /// Iterates over a list of statistic names and expected values, attempting + /// to fetch each from the StatsMgr and if found, compare its observed + /// value to the expected value. Fails if any of the expected stats are not + /// found or if the values do not match. + /// + /// @param expected_stats Map of expected static names and values. + void checkStats(const StatMap& expected_stats); + + /// @brief Compares StatsMgr key statistics against expected values. + /// + /// Prepend key part of names before calling checkStats simpler variant. + /// + /// @param key_name Name of the key. + /// @param expected_stats Map of expected static names and values. + void checkStats(const std::string& key_name, const StatMap& expected_stats); +}; + +} +} +} + +#endif // STATS_TEST_UTILS_H -- GitLab From bf6eefb82f4cf02079c1f1afd15b047586b486a8 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Sun, 5 Sep 2021 09:04:01 +0200 Subject: [PATCH 2/3] [#2040] Added D2Stats::init --- src/bin/d2/d2_process.cc | 13 ++----------- src/bin/d2/tests/dns_client_unittests.cc | 10 ---------- src/bin/d2/tests/stats_test_utils.cc | 2 ++ src/lib/d2srv/d2_stats.cc | 14 ++++++++++++++ src/lib/d2srv/d2_stats.h | 5 +++++ 5 files changed, 23 insertions(+), 21 deletions(-) diff --git a/src/bin/d2/d2_process.cc b/src/bin/d2/d2_process.cc index ebd3865e79..a4a738be24 100644 --- a/src/bin/d2/d2_process.cc +++ b/src/bin/d2/d2_process.cc @@ -16,7 +16,6 @@ #include #include #include -#include using namespace isc::hooks; using namespace isc::process; @@ -66,16 +65,8 @@ D2Process::D2Process(const char* name, const asiolink::IOServicePtr& io_service) D2CfgMgrPtr tmp = getD2CfgMgr(); update_mgr_.reset(new D2UpdateMgr(queue_mgr_, tmp, getIoService())); - // Instantiate stats manager. - // Initialize statistics. - isc::stats::StatsMgr& stats_mgr = isc::stats::StatsMgr::instance(); - stats_mgr.setMaxSampleCountDefault(0); - for (const auto& name : D2Stats::ncr) { - stats_mgr.setValue(name, static_cast(0)); - } - for (const auto& name : D2Stats::update) { - stats_mgr.setValue(name, static_cast(0)); - } + // Initialize stats manager. + D2Stats::init(); }; void diff --git a/src/bin/d2/tests/dns_client_unittests.cc b/src/bin/d2/tests/dns_client_unittests.cc index a2915cb0f4..33369935ab 100644 --- a/src/bin/d2/tests/dns_client_unittests.cc +++ b/src/bin/d2/tests/dns_client_unittests.cc @@ -486,9 +486,6 @@ TEST_F(DNSClientTest, invalidTimeout) { // Verifies that TSIG can be used to sign requests and verify responses. TEST_F(DNSClientTest, runTSIGTest) { - ConstElementPtr stats_all = StatsMgr::instance().getAll(); - ASSERT_TRUE(stats_all); - EXPECT_TRUE(stats_all->empty()); std::string secret ("key number one"); D2TsigKeyPtr key_one; ASSERT_NO_THROW(key_one.reset(new @@ -510,12 +507,6 @@ TEST_F(DNSClientTest, runTSIGTest) { secret.c_str(), secret.size()))); checkStats("two.com.", stats_key); D2TsigKeyPtr nokey; - StatsMgr::instance().setValue("update-sent", 0LL); - StatsMgr::instance().setValue("update-signed", 0LL); - StatsMgr::instance().setValue("update-unsigned", 0LL); - StatsMgr::instance().setValue("update-success", 0LL); - StatsMgr::instance().setValue("update-timeout", 0LL); - StatsMgr::instance().setValue("update-error", 0LL); // Should be able to send and receive with no keys. // Neither client nor server will attempt to sign or verify. @@ -535,7 +526,6 @@ TEST_F(DNSClientTest, runTSIGTest) { runTSIGTest(nokey, key_two); // Check statistics. - stats_all = StatsMgr::instance().getAll(); StatMap stats_one = { { "update-sent", 3}, { "update-success", 1}, diff --git a/src/bin/d2/tests/stats_test_utils.cc b/src/bin/d2/tests/stats_test_utils.cc index 905a765a22..7c8f6df6e3 100644 --- a/src/bin/d2/tests/stats_test_utils.cc +++ b/src/bin/d2/tests/stats_test_utils.cc @@ -5,6 +5,7 @@ // file, You can obtain one at http://mozilla.org/MPL/2.0/. #include +#include #include using namespace isc::data; @@ -17,6 +18,7 @@ namespace test { D2StatTest::D2StatTest() { StatsMgr::instance().removeAll(); + D2Stats::init(); } D2StatTest::~D2StatTest() { diff --git a/src/lib/d2srv/d2_stats.cc b/src/lib/d2srv/d2_stats.cc index 5c94f4304f..3d3d3b6ff1 100644 --- a/src/lib/d2srv/d2_stats.cc +++ b/src/lib/d2srv/d2_stats.cc @@ -9,8 +9,10 @@ #include #include +#include using namespace std; +using namespace isc::stats; namespace isc { namespace d2 { @@ -40,5 +42,17 @@ D2Stats::key = { "update-error" }; +void +D2Stats::init() { + StatsMgr& stats_mgr = isc::stats::StatsMgr::instance(); + stats_mgr.setMaxSampleCountDefault(0); + for (const auto& name : D2Stats::ncr) { + stats_mgr.setValue(name, static_cast(0)); + } + for (const auto& name : D2Stats::update) { + stats_mgr.setValue(name, static_cast(0)); + } +}; + } // namespace d2 } // namespace isc diff --git a/src/lib/d2srv/d2_stats.h b/src/lib/d2srv/d2_stats.h index be40415e50..4bfd1da7ab 100644 --- a/src/lib/d2srv/d2_stats.h +++ b/src/lib/d2srv/d2_stats.h @@ -40,6 +40,11 @@ public: /// - update-timeout /// - update-error static std::set key; + + /// @brief Initialize D2 statistics. + /// + /// @note: Add default samples if needed. + static void init(); }; } // namespace d2 -- GitLab From 2d337d8d81b0e53eb62e10b23d01e04307b11018 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Tue, 14 Sep 2021 15:24:15 +0200 Subject: [PATCH 3/3] [#2040] Added a ChangeLog entry --- ChangeLog | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ChangeLog b/ChangeLog index a42db2d4a3..79672b6955 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +1942. [func] fdupont + Added basic statistics to the DHCP-DDNS server. + (Gitlab #2040) + 1941. [func] fdupont Per DNS server TSIG keys are now supported in the DHCP-DDNS (aka D2) server configuration. A new callout point 'select_key' -- GitLab