From: Francis Dupont <fdupont@isc.org>
Date: Thu, 19 Aug 2021 08:12:26 +0000 (+0200)
Subject: [#2040] Checkpoint: todo UTs, ...
X-Git-Tag: Kea-2.0.0~155
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=eccdffbf66cc9d6c6b5d432f0df2cc7301a6e3f7;p=thirdparty%2Fkea.git

[#2040] Checkpoint: todo UTs, ...
---

diff --git a/doc/sphinx/arm/ddns.rst b/doc/sphinx/arm/ddns.rst
index afefb6d29e..2c813f85ee 100644
--- a/doc/sphinx/arm/ddns.rst
+++ b/doc/sphinx/arm/ddns.rst
@@ -298,6 +298,14 @@ The D2 server supports the following operational commands:
 -  status-get
 -  version-get
 
+Starting with Kea version 2.0.0 the D2 server supports too the following
+operational commands for statistics:
+
+-  statistic-get
+-  statistic-get-all
+-  statistic-reset
+-  statistic-reset-all
+
 The ``shutdown`` command supports the extra ``type`` argument which controls the
 way the D2 server cleans up on exit.
 The supported shutdown types are:
@@ -908,6 +916,67 @@ These Reverse DDNS Domains are specified as follows:
        }
    }
 
+DHCP-DDNS Server Statistics
+===========================
+
+Kea version 2.0.0 introduced statistics support for the DHCP-DDNS.
+
+Statistics are divided in three groups: Name Change Request, DNS update
+and per TSIG key DNS updates. If the statistics of the first two groups
+are cummulative, i.e. not affected by configuration change or reload,
+per key statistics are reset to 0 when the underlying object is
+(re)created.
+
+Currently the statistics management is limited:
+
+-  only integer samples (i.e. a counter and a timestamp) are used
+-  the maximum sample count is 1
+-  there is no API to remove one or all statistics
+-  there is no API to set the maxium sample count or age
+
+.. note::
+
+    Hook libraries like the GSS-TSIG add new statistics.
+
+A reference about Kea statistics can be found at :ref:`stats`.
+
+NCR Statistics
+--------------
+
+The Name Change Request statistics are:
+
+-  ``ncr-received`` - received valid NCRs
+-  ``ncr-invalid`` - received invalid NCRs
+-  ``ncr-error`` - errors in NCR receptions other than I/) cancel on shutdown
+
+DNS Update Statistics
+---------------------
+
+The global DNS update statistics are:
+
+-  ``update-sent`` - sent DNS updates
+-  ``update-signed`` - sent DNS updates protected by TSIG
+-  ``update-unsigned`` - sent DNS updates not protected by TSIG
+-  ``update-success`` - DNS updates which completed with a success
+-  ``update-timeout`` - DNS updates which completed on timeout
+-  ``update-error`` - DNS updates which completed with an error other than
+   timeout
+
+Per TSIG key DNS Update Statistics
+----------------------------------
+
+The per TSIG key DNS update statistics are:
+
+-  ``update-sent`` - sent DNS updates
+-  ``update-success`` - DNS updates which completed with a success
+-  ``update-timeout`` - DNS updates which completed on timeout
+-  ``update-error`` - DNS updates which completed with an error other than
+   timeout
+
+The name of a per key statistics is ``key[<key-DNS-name>].<stat-name>``,
+for instance he name of the ``update-sent`` statistics for the
+``key.example.com.`` TSIG key is ``key[key.example.com.].update-sent``.
+
 DHCP-DDNS Server Limitations
 ============================
 
diff --git a/src/bin/d2/d2_process.cc b/src/bin/d2/d2_process.cc
index f26bc07f0a..e4db265d3f 100644
--- a/src/bin/d2/d2_process.cc
+++ b/src/bin/d2/d2_process.cc
@@ -12,6 +12,7 @@
 #include <d2/d2_process.h>
 #include <d2srv/d2_cfg_mgr.h>
 #include <d2srv/d2_log.h>
+#include <d2srv/d2_stats.h>
 #include <d2srv/d2_tsig_key.h>
 #include <hooks/hooks.h>
 #include <hooks/hooks_manager.h>
@@ -65,11 +66,15 @@ 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.
+    // Instantiate  stats manager.
+    // Initialize statistics.
     isc::stats::StatsMgr& stats_mgr = isc::stats::StatsMgr::instance();
     stats_mgr.setMaxSampleCountDefault(0);
-    for (const auto& name : D2TsigKey::globalStats) {
-        stats_mgr.setValue("global." + name, static_cast<int64_t>(0));
+    for (const auto& name : D2Stats::ncr) {
+        stats_mgr.setValue(name, static_cast<int64_t>(0));
+    }
+    for (const auto& name : D2Stats::update) {
+        stats_mgr.setValue(name, static_cast<int64_t>(0));
     }
 };
 
diff --git a/src/bin/d2/d2_update_mgr.h b/src/bin/d2/d2_update_mgr.h
index a06d787dcc..2ff8765bcd 100644
--- a/src/bin/d2/d2_update_mgr.h
+++ b/src/bin/d2/d2_update_mgr.h
@@ -69,18 +69,6 @@ public:
     /// implementation.
     static const size_t MAX_TRANSACTIONS_DEFAULT = 32;
 
-    // @todo This structure is not yet used. It is here in anticipation of
-    // enabled statistics capture.
-    struct Stats {
-        uint64_t start_time_;
-        uint64_t stop_time_;
-        uint64_t update_count_;
-        uint64_t min_update_time_;
-        uint64_t max_update_time_;
-        uint64_t server_rejects_;
-        uint64_t server_timeouts_;
-    };
-
     /// @brief Constructor
     ///
     /// @param queue_mgr reference to the queue manager receiving requests
diff --git a/src/bin/d2/dns_client.cc b/src/bin/d2/dns_client.cc
index a7f91182b8..5aaddd6474 100644
--- a/src/bin/d2/dns_client.cc
+++ b/src/bin/d2/dns_client.cc
@@ -144,12 +144,12 @@ DNSClientImpl::operator()(asiodns::IOFetch::Result result) {
         try {
             response_->fromWire(in_buf_->getData(), in_buf_->getLength(),
                                 tsig_context_.get());
-            incrStats("success");
+            incrStats("update-success");
         } catch (const isc::Exception& ex) {
             status = DNSClient::INVALID_RESPONSE;
             LOG_DEBUG(d2_to_dns_logger, isc::log::DBGLVL_TRACE_DETAIL,
                       DHCP_DDNS_INVALID_RESPONSE).arg(ex.what());
-            incrStats("error");
+            incrStats("update-error");
         }
 
         if (tsig_context_) {
@@ -157,9 +157,9 @@ DNSClientImpl::operator()(asiodns::IOFetch::Result result) {
             tsig_context_.reset();
         }
     } else if (status == DNSClient::TIMEOUT) {
-        incrStats("timeout");
+        incrStats("update-timeout");
     } else {
-        incrStats("error");
+        incrStats("update-error");
     }
 
     // Once we are done with internal business, let's call a callback supplied
@@ -244,18 +244,18 @@ DNSClientImpl::doUpdate(asiolink::IOService& io_service,
     io_service.post(io_fetch);
 
     // Update sent statistics.
-    incrStats("sent");
+    incrStats("update-sent");
     if (tsig_key) {
-        incrStats("signed", false);
+        incrStats("update-signed", false);
     } else {
-        incrStats("unsigned", false);
+        incrStats("update-unsigned", false);
     }
 }
 
 void
 DNSClientImpl::incrStats(const std::string& stat, bool update_key) {
     StatsMgr& mgr = StatsMgr::instance();
-    mgr.addValue("global." + stat, static_cast<int64_t>(1));
+    mgr.addValue(stat, static_cast<int64_t>(1));
     if (update_key && !tsig_key_name_.empty()) {
         mgr.addValue(StatsMgr::generateName("key", tsig_key_name_, stat),
                      static_cast<int64_t>(1));
diff --git a/src/lib/d2srv/Makefile.am b/src/lib/d2srv/Makefile.am
index af8a226565..fa6ad63e59 100644
--- a/src/lib/d2srv/Makefile.am
+++ b/src/lib/d2srv/Makefile.am
@@ -16,6 +16,7 @@ libkea_d2srv_la_SOURCES += d2_config.cc d2_config.h
 libkea_d2srv_la_SOURCES += d2_log.cc d2_log.h
 libkea_d2srv_la_SOURCES += d2_messages.cc d2_messages.h
 libkea_d2srv_la_SOURCES += d2_simple_parser.cc d2_simple_parser.h
+libkea_d2srv_la_SOURCES += d2_stats.cc d2_stats.h 
 libkea_d2srv_la_SOURCES += d2_tsig_key.cc d2_tsig_key.h
 EXTRA_DIST += d2_messages.mes
 
diff --git a/src/lib/d2srv/d2_stats.cc b/src/lib/d2srv/d2_stats.cc
new file mode 100644
index 0000000000..5c94f4304f
--- /dev/null
+++ b/src/lib/d2srv/d2_stats.cc
@@ -0,0 +1,44 @@
+// 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/.
+
+/// Defines the logger used by the top-level component of kea-dhcp-ddns.
+
+#include <config.h>
+
+#include <d2srv/d2_stats.h>
+
+using namespace std;
+
+namespace isc {
+namespace d2 {
+
+set<string>
+D2Stats::ncr = {
+    "ncr-received",
+    "ncr-invalid",
+    "ncr-error"
+};
+
+set<string>
+D2Stats::update = {
+    "update-sent",
+    "update-signed",
+    "update-unsigned",
+    "update-success",
+    "update-timeout",
+    "update-error"
+};
+
+set<string>
+D2Stats::key = {
+    "update-sent",
+    "update-success",
+    "update-timeout",
+    "update-error"
+};
+
+} // namespace d2
+} // namespace isc
diff --git a/src/lib/d2srv/d2_stats.h b/src/lib/d2srv/d2_stats.h
new file mode 100644
index 0000000000..be40415e50
--- /dev/null
+++ b/src/lib/d2srv/d2_stats.h
@@ -0,0 +1,48 @@
+// 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/.
+
+#ifndef D2_STATS_H
+#define D2_STATS_H
+
+#include <set>
+#include <string>
+
+namespace isc {
+namespace d2 {
+
+/// @brief Statistics Names.
+class D2Stats {
+public:
+    /// @brief Global NCR statistics names.
+    ///
+    /// - ncr-received
+    /// - ncr-invalid
+    /// - ncr-error
+    static std::set<std::string> ncr;
+
+    /// @brief Global DNS update statistics names.
+    ///
+    /// - update-sent
+    /// - update-signed
+    /// - update-unsigned
+    /// - update-success
+    /// - update-timeout
+    /// - update-error
+    static std::set<std::string> update;
+
+    /// @brief Key DNS update statistics names.
+    ///
+    /// - update-sent
+    /// - update-success
+    /// - update-timeout
+    /// - update-error
+    static std::set<std::string> key;
+};
+
+} // namespace d2
+} // namespace isc
+
+#endif // D2_STATS_H
diff --git a/src/lib/d2srv/d2_tsig_key.cc b/src/lib/d2srv/d2_tsig_key.cc
index bee8b08adf..2d9055a833 100644
--- a/src/lib/d2srv/d2_tsig_key.cc
+++ b/src/lib/d2srv/d2_tsig_key.cc
@@ -8,6 +8,7 @@
 
 #include <config.h>
 
+#include <d2srv/d2_stats.h>
 #include <d2srv/d2_tsig_key.h>
 #include <stats/stats_mgr.h>
 
@@ -18,24 +19,6 @@ using namespace std;
 namespace isc {
 namespace d2 {
 
-set<string>
-D2TsigKey::keyStats = {
-    "sent",
-    "success",
-    "timeout",
-    "error"
-};
-
-set<string>
-D2TsigKey::globalStats = {
-    "sent",
-    "signed",
-    "unsigned",
-    "success",
-    "timeout",
-    "error"
-};
-
 D2TsigKey::D2TsigKey(const std::string& key_spec) : TSIGKey(key_spec) {
     initStats();
 }
@@ -50,7 +33,7 @@ void
 D2TsigKey::initStats() {
     StatsMgr& stats_mgr = StatsMgr::instance();
     const string& kname = getKeyName().toText();
-    for (const auto& name : keyStats) {
+    for (const auto& name : D2Stats::key) {
         const string& sname = StatsMgr::generateName("key", kname, name);
         stats_mgr.setValue(sname, static_cast<int64_t>(0));
     }
@@ -59,7 +42,7 @@ D2TsigKey::initStats() {
 D2TsigKey::~D2TsigKey() {
     StatsMgr& stats_mgr = StatsMgr::instance();
     const string& kname = getKeyName().toText();
-    for (const auto& name : keyStats) {
+    for (const auto& name : D2Stats::key) {
         string sname = StatsMgr::generateName("key", kname, name);
         stats_mgr.del(sname);
     }
@@ -69,7 +52,7 @@ void
 D2TsigKey::resetStats() {
     StatsMgr& stats_mgr = StatsMgr::instance();
     const string& kname = getKeyName().toText();
-    for (const auto& name : keyStats) {
+    for (const auto& name : D2Stats::key) {
         string sname = StatsMgr::generateName("key", kname, name);
         stats_mgr.reset(sname);
     }
diff --git a/src/lib/d2srv/d2_tsig_key.h b/src/lib/d2srv/d2_tsig_key.h
index 3f37c4c078..2815cb0a79 100644
--- a/src/lib/d2srv/d2_tsig_key.h
+++ b/src/lib/d2srv/d2_tsig_key.h
@@ -11,8 +11,6 @@
 #include <dns/tsigkey.h>
 #include <boost/shared_ptr.hpp>
 
-#include <set>
-
 namespace isc {
 namespace d2 {
 
@@ -54,16 +52,6 @@ public:
     ///
     virtual void resetStats();
 
-    /// @brief Statistics names (key).
-    ///
-    /// The list of statistic names for keys.
-    static std::set<std::string> keyStats;
-
-    /// @brief Statistics names (global).
-    ///
-    /// The list of global statistic names.
-    static std::set<std::string> globalStats;
-
 private:
     /// @brief Initialize key statistics.
     void initStats();
diff --git a/src/lib/d2srv/tests/d2_tsig_key_unittest.cc b/src/lib/d2srv/tests/d2_tsig_key_unittest.cc
index edb6aeba5f..d54ba362a0 100644
--- a/src/lib/d2srv/tests/d2_tsig_key_unittest.cc
+++ b/src/lib/d2srv/tests/d2_tsig_key_unittest.cc
@@ -7,6 +7,7 @@
 #include <config.h>
 
 #include <cc/data.h>
+#include <d2srv/d2_stats.h>
 #include <d2srv/d2_tsig_key.h>
 #include <stats/stats_mgr.h>
 
@@ -22,6 +23,13 @@ using namespace std;
 
 namespace {
 
+/// @brief Check statistics names.
+TEST(D2StatsTest, names) {
+    ASSERT_EQ(3, D2Stats::ncr.size());
+    ASSERT_EQ(6, D2Stats::update.size());
+    ASSERT_EQ(4, D2Stats::key.size());
+}
+
 /// @brief Fixture class for TSIG key / DNS update statictics.
 class D2TsigKeyTest : public ::testing::Test {
 public:
@@ -41,10 +49,6 @@ public:
 
 /// @brief Check TSIG key life.
 TEST_F(D2TsigKeyTest, key) {
-    // Statistics names.
-    ASSERT_EQ(4, D2TsigKey::keyStats.size());
-    ASSERT_EQ(6, D2TsigKey::globalStats.size());
-
     // Get the statistics manager.
     StatsMgr& stat_mgr = StatsMgr::instance();
     ASSERT_EQ(0, stat_mgr.count());
@@ -55,7 +59,7 @@ TEST_F(D2TsigKeyTest, key) {
     EXPECT_EQ(4, stat_mgr.count());
 
     // Get the 'sent' statistics.
-    const string& stat_name = "key[foo.bar.].sent";
+    const string& stat_name = "key[foo.bar.].update-sent";
     EXPECT_EQ(1, stat_mgr.getSize(stat_name));
     ObservationPtr stat = stat_mgr.getObservation(stat_name);
     ASSERT_TRUE(stat);
diff --git a/src/lib/dhcp_ddns/Makefile.am b/src/lib/dhcp_ddns/Makefile.am
index 4769f616bb..e50c85c271 100644
--- a/src/lib/dhcp_ddns/Makefile.am
+++ b/src/lib/dhcp_ddns/Makefile.am
@@ -24,6 +24,7 @@ libkea_dhcp_ddns_la_LDFLAGS += $(CRYPTO_LDFLAGS)
 libkea_dhcp_ddns_la_LDFLAGS += -no-undefined -version-info 16:0:0
 
 libkea_dhcp_ddns_la_LIBADD  =
+libkea_dhcp_ddns_la_LIBADD += $(top_builddir)/src/lib/stats/libkea-stats.la
 libkea_dhcp_ddns_la_LIBADD += $(top_builddir)/src/lib/dhcp/libkea-dhcp++.la
 libkea_dhcp_ddns_la_LIBADD += $(top_builddir)/src/lib/asiolink/libkea-asiolink.la
 libkea_dhcp_ddns_la_LIBADD += $(top_builddir)/src/lib/cc/libkea-cc.la
diff --git a/src/lib/dhcp_ddns/ncr_udp.cc b/src/lib/dhcp_ddns/ncr_udp.cc
index 9ba44ca713..fc46f5b651 100644
--- a/src/lib/dhcp_ddns/ncr_udp.cc
+++ b/src/lib/dhcp_ddns/ncr_udp.cc
@@ -8,6 +8,7 @@
 
 #include <dhcp_ddns/dhcp_ddns_log.h>
 #include <dhcp_ddns/ncr_udp.h>
+#include <stats/stats_mgr.h>
 
 #include <functional>
 
@@ -161,9 +162,13 @@ NameChangeUDPListener::receiveCompletionHandler(const bool successful,
 
         try {
             ncr = NameChangeRequest::fromFormat(format_, input_buffer);
+            isc::stats::StatsMgr::instance().addValue("ncr-received",
+                                                      static_cast<int64_t>(0));
         } catch (const NcrMessageError& ex) {
             // log it and go back to listening
             LOG_ERROR(dhcp_ddns_logger, DHCP_DDNS_INVALID_NCR).arg(ex.what());
+            isc::stats::StatsMgr::instance().addValue("ncr-invalid",
+                                                      static_cast<int64_t>(0));
 
             // Queue up the next receive.
             // NOTE: We must call the base class, NEVER doReceive
@@ -181,6 +186,8 @@ NameChangeUDPListener::receiveCompletionHandler(const bool successful,
         } else {
             LOG_ERROR(dhcp_ddns_logger, DHCP_DDNS_NCR_UDP_RECV_ERROR)
                       .arg(error_code.message());
+            isc::stats::StatsMgr::instance().addValue("ncr-error",
+                                                      static_cast<int64_t>(0));
             result = ERROR;
         }
     }
@@ -376,5 +383,5 @@ NameChangeUDPSender::closeWatchSocket() {
     }
 }
 
-}; // end of isc::dhcp_ddns namespace
-}; // end of isc namespace
+} // end of isc::dhcp_ddns namespace
+} // end of isc namespace
diff --git a/src/lib/dhcp_ddns/tests/Makefile.am b/src/lib/dhcp_ddns/tests/Makefile.am
index 1bbeb8053a..e1f27d1fa2 100644
--- a/src/lib/dhcp_ddns/tests/Makefile.am
+++ b/src/lib/dhcp_ddns/tests/Makefile.am
@@ -32,6 +32,7 @@ libdhcp_ddns_unittests_CXXFLAGS = $(AM_CXXFLAGS)
 libdhcp_ddns_unittests_LDFLAGS  = $(AM_LDFLAGS) $(CRYPTO_LDFLAGS) $(GTEST_LDFLAGS)
 
 libdhcp_ddns_unittests_LDADD = $(top_builddir)/src/lib/dhcp_ddns/libkea-dhcp_ddns.la
+libdhcp_ddns_unittests_LDADD += $(top_builddir)/src/lib/stats/libkea-stats.la
 libdhcp_ddns_unittests_LDADD += $(top_builddir)/src/lib/dhcp/libkea-dhcp++.la
 libdhcp_ddns_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libkea-asiolink.la
 libdhcp_ddns_unittests_LDADD += $(top_builddir)/src/lib/cc/libkea-cc.la