Add monitoring instrumentation around dhcp-disable state.

ChangeLog

@@ -1,3 +1,13 @@
+2204.	[func]		dhankins
+	New metrics pkt4-dhcp-disabled, pkt4-raw-received, disabled-by-db,
+	disabled-by-ha-local, disabled-by-ha-remote, disabled-by-user, and
+	disabled-globally provide monitoring level visibility into the network_state
+	manager, and closes a monitoring gap where packets dropped while disabled
+	are not counted by any metric.
+	While disabled, a WARN log is emitted every DISABLE_WARN_PACKET{4|6}(100)
+	dropped packets, in addition to the existing DEBUG log.
+	(Gitlab #3228)
+
 Kea 2.5.5 (development) released on January 31, 2024
 
 2203.	[build]		razvan

src/bin/dhcp4/dhcp4_srv.cc

@@ -123,6 +123,8 @@ struct Dhcp4Hooks {
 /// List of statistics which is initialized to 0 during the DHCPv4
 /// server startup.
 std::set<std::string> dhcp4_statistics = {
+    "pkt4-dhcp-disabled",
+    "pkt4-raw-received",
     "pkt4-received",
     "pkt4-discover-received",
     "pkt4-offer-received",
@@ -1093,10 +1095,24 @@ Dhcpv4Srv::runOne() {
         return;
     }
 
+    isc::stats::StatsMgr& stats_mgr = isc::stats::StatsMgr::instance();
+
+    // Count raw packets received as soon as we know a packet has been read.
+    stats_mgr.addValue("pkt4-raw-received", static_cast<int64_t>(1));
+
     // If the DHCP service has been globally disabled, drop the packet.
     if (!network_state_->isServiceEnabled()) {
         LOG_DEBUG(bad_packet4_logger, DBGLVL_PKT_HANDLING, DHCP4_PACKET_DROP_0008)
             .arg(query->getLabel());
+
+        // Log a warning every DISABLE_WARN_PACKET4.
+        if (stats_mgr.getInteger("pkt4-dhcp-disabled") % DISABLE_WARN_PACKET4 == 0) {
+            LOG_WARN(dhcp4_logger, DHCP4_PACKET_DROP_0008)
+                .arg(query->getLabel());
+        }
+
+        // Count packets dropped due to (hopefully temporary) service disabled.
+        stats_mgr.addValue("pkt4-dhcp-disabled", static_cast<int64_t>(1));
         return;
     } else {
         if (MultiThreadingMgr::instance().getMode()) {

src/bin/dhcp4/dhcp4_srv.h

@@ -40,6 +40,8 @@
 namespace isc {
 namespace dhcp {
 
+static const int64_t DISABLE_WARN_PACKET4 = 100;
+
 /// @brief DHCPv4 message exchange.
 ///
 /// This class represents the DHCPv4 message exchange. The message exchange

src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc

@@ -634,11 +634,23 @@ TEST_F(CtrlChannelDhcpv4SrvTest, controlChannelStats) {
                     "  \"name\":\"bogus\" }}", response);
     EXPECT_EQ("{ \"arguments\": {  }, \"result\": 0 }", response);
 
+    // Set max sample count to 1 to match test expectation (disabled state
+    // gauges normally have 2 samples).
+    sendUnixCommand("{ \"command\" : \"statistic-sample-count-set-all\", "
+                    "  \"arguments\": { \"max-samples\": 1 }}", response);
+    EXPECT_EQ("{ \"result\": 0, \"text\": \"All statistics count limit are set.\" }", response);
     // Check statistic-get-all
     sendUnixCommand("{ \"command\" : \"statistic-get-all\", "
                     "  \"arguments\": {}}", response);
 
     std::set<std::string> initial_stats = {
+        "disabled-by-db",
+        "disabled-by-ha-local",
+        "disabled-by-ha-remote",
+        "disabled-by-user",
+        "disabled-globally",
+        "pkt4-dhcp-disabled",
+        "pkt4-raw-received",
         "pkt4-received",
         "pkt4-discover-received",
         "pkt4-offer-received",

src/bin/dhcp4/tests/dhcp4_test_utils.cc

@@ -994,14 +994,17 @@ Dhcpv4SrvTest::pretendReceivingPkt(NakedDhcpv4Srv& srv, const std::string& confi
 
     using namespace isc::stats;
     StatsMgr& mgr = StatsMgr::instance();
+    ObservationPtr pkt4_raw_received = mgr.getObservation("pkt4-raw-received");
     ObservationPtr pkt4_rcvd = mgr.getObservation("pkt4-received");
     ObservationPtr tested_stat = mgr.getObservation(stat_name);
 
     // All expected statistics must be present.
+    ASSERT_TRUE(pkt4_raw_received);
     ASSERT_TRUE(pkt4_rcvd);
     ASSERT_TRUE(tested_stat);
 
     // They also must have expected values.
+    EXPECT_EQ(1, pkt4_raw_received->getInteger().first);
     EXPECT_EQ(1, pkt4_rcvd->getInteger().first);
     EXPECT_EQ(1, tested_stat->getInteger().first);
 }

src/bin/dhcp4/tests/dora_unittest.cc

@@ -2504,6 +2504,8 @@ DORATest::statisticsDORA() {
     // Ok, let's check the statistics.
     using namespace isc::stats;
     StatsMgr& mgr = StatsMgr::instance();
+    ObservationPtr pkt4_dhcp_disabled = mgr.getObservation("pkt4-dhcp-disabled");
+    ObservationPtr pkt4_raw_received = mgr.getObservation("pkt4-raw-received");
     ObservationPtr pkt4_received = mgr.getObservation("pkt4-received");
     ObservationPtr pkt4_discover_received = mgr.getObservation("pkt4-discover-received");
     ObservationPtr pkt4_offer_sent = mgr.getObservation("pkt4-offer-sent");
@@ -2512,6 +2514,8 @@ DORATest::statisticsDORA() {
     ObservationPtr pkt4_sent = mgr.getObservation("pkt4-sent");
 
     // All expected statistics must be present.
+    ASSERT_TRUE(pkt4_dhcp_disabled);
+    ASSERT_TRUE(pkt4_raw_received);
     ASSERT_TRUE(pkt4_received);
     ASSERT_TRUE(pkt4_discover_received);
     ASSERT_TRUE(pkt4_offer_sent);
@@ -2520,6 +2524,8 @@ DORATest::statisticsDORA() {
     ASSERT_TRUE(pkt4_sent);
 
     // They also must have expected values.
+    EXPECT_EQ(0, pkt4_dhcp_disabled->getInteger().first);
+    EXPECT_EQ(2, pkt4_raw_received->getInteger().first);
     EXPECT_EQ(2, pkt4_received->getInteger().first);
     EXPECT_EQ(1, pkt4_discover_received->getInteger().first);
     EXPECT_EQ(1, pkt4_offer_sent->getInteger().first);
@@ -2535,6 +2541,8 @@ DORATest::statisticsDORA() {
     ASSERT_NO_THROW(client.doRequest());
 
     // Let's see if the stats are properly updated.
+    EXPECT_EQ(0, pkt4_dhcp_disabled->getInteger().first);
+    EXPECT_EQ(5, pkt4_raw_received->getInteger().first);
     EXPECT_EQ(5, pkt4_received->getInteger().first);
     EXPECT_EQ(1, pkt4_discover_received->getInteger().first);
     EXPECT_EQ(1, pkt4_offer_sent->getInteger().first);
@@ -2576,20 +2584,23 @@ DORATest::statisticsNAK() {
 
     using namespace isc::stats;
     StatsMgr& mgr = StatsMgr::instance();
+    ObservationPtr pkt4_raw_received = mgr.getObservation("pkt4-raw-received");
     ObservationPtr pkt4_received = mgr.getObservation("pkt4-received");
     ObservationPtr pkt4_request_received = mgr.getObservation("pkt4-request-received");
     ObservationPtr pkt4_ack_sent = mgr.getObservation("pkt4-ack-sent");
     ObservationPtr pkt4_nak_sent = mgr.getObservation("pkt4-nak-sent");
     ObservationPtr pkt4_sent = mgr.getObservation("pkt4-sent");
 
     // All expected statistics must be present.
+    ASSERT_TRUE(pkt4_raw_received);
     ASSERT_TRUE(pkt4_received);
     ASSERT_TRUE(pkt4_request_received);
     ASSERT_FALSE(pkt4_ack_sent); // No acks were sent, no such statistic expected.
     ASSERT_TRUE(pkt4_nak_sent);
     ASSERT_TRUE(pkt4_sent);
 
     // They also must have expected values.
+    EXPECT_EQ(1, pkt4_raw_received->getInteger().first);
     EXPECT_EQ(1, pkt4_received->getInteger().first);
     EXPECT_EQ(1, pkt4_request_received->getInteger().first);
     EXPECT_EQ(1, pkt4_nak_sent->getInteger().first);

src/bin/dhcp4/tests/inform_unittest.cc

@@ -623,18 +623,21 @@ TEST_F(InformTest, statisticsInform) {
     // Ok, let's check the statistics.
     using namespace isc::stats;
     StatsMgr& mgr = StatsMgr::instance();
+    ObservationPtr pkt4_raw_received = mgr.getObservation("pkt4-raw-received");
     ObservationPtr pkt4_received = mgr.getObservation("pkt4-received");
     ObservationPtr pkt4_inform_received = mgr.getObservation("pkt4-inform-received");
     ObservationPtr pkt4_ack_sent = mgr.getObservation("pkt4-ack-sent");
     ObservationPtr pkt4_sent = mgr.getObservation("pkt4-sent");
 
     // All expected statistics must be present.
+    ASSERT_TRUE(pkt4_raw_received);
     ASSERT_TRUE(pkt4_received);
     ASSERT_TRUE(pkt4_inform_received);
     ASSERT_TRUE(pkt4_ack_sent);
     ASSERT_TRUE(pkt4_sent);
 
     // And they must have expected values.
+    EXPECT_EQ(1, pkt4_raw_received->getInteger().first);
     EXPECT_EQ(1, pkt4_received->getInteger().first);
     EXPECT_EQ(1, pkt4_inform_received->getInteger().first);
     EXPECT_EQ(1, pkt4_ack_sent->getInteger().first);
@@ -648,6 +651,7 @@ TEST_F(InformTest, statisticsInform) {
     ASSERT_NO_THROW(client.doInform());
 
     // Let's see if the stats are properly updated.
+    EXPECT_EQ(5, pkt4_raw_received->getInteger().first);
     EXPECT_EQ(5, pkt4_received->getInteger().first);
     EXPECT_EQ(5, pkt4_inform_received->getInteger().first);
     EXPECT_EQ(5, pkt4_ack_sent->getInteger().first);

src/bin/dhcp6/dhcp6_srv.cc

@@ -178,6 +178,8 @@ createStatusCode(const Pkt6& pkt, const Option6IA& ia, const uint16_t status_cod
 /// List of statistics which is initialized to 0 during the DHCPv6
 /// server startup.
 std::set<std::string> dhcp6_statistics = {
+    "pkt6-dhcp-disabled",
+    "pkt6-raw-received",
     "pkt6-received",
     "pkt6-solicit-received",
     "pkt6-advertise-received",
@@ -682,10 +684,24 @@ Dhcpv6Srv::runOne() {
         return;
     }
 
+    isc::stats::StatsMgr& stats_mgr = isc::stats::StatsMgr::instance();
+
+    // Count raw packets received as soon as we know a packet has been read.
+    stats_mgr.addValue("pkt6-raw-received", static_cast<int64_t>(1));
+
     // If the DHCP service has been globally disabled, drop the packet.
     if (!network_state_->isServiceEnabled()) {
         LOG_DEBUG(bad_packet6_logger, DBGLVL_PKT_HANDLING, DHCP6_PACKET_DROP_DHCP_DISABLED)
             .arg(query->getLabel());
+
+        // Log a warning every DISABLE_WARN_PACKET6.
+        if (stats_mgr.getInteger("pkt6-dhcp-disabled") % DISABLE_WARN_PACKET6 == 0) {
+            LOG_WARN(dhcp6_logger, DHCP6_PACKET_DROP_DHCP_DISABLED)
+                .arg(query->getLabel());
+        }
+
+        // Count packets dropped due to (hopefully temporary) service disabled.
+        stats_mgr.addValue("pkt6-dhcp-disabled", static_cast<int64_t>(1));
         return;
     } else {
         if (MultiThreadingMgr::instance().getMode()) {

src/bin/dhcp6/dhcp6_srv.h

@@ -43,6 +43,8 @@
 namespace isc {
 namespace dhcp {
 
+static const int64_t DISABLE_WARN_PACKET6 = 100;
+
 /// @brief This exception is thrown when DHCP server hits the error which should
 /// result in discarding the message being processed.
 class DHCPv6DiscardMessageError : public Exception {

src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc

@@ -1392,11 +1392,23 @@ TEST_F(CtrlChannelDhcpv6SrvTest, controlChannelStats) {
                     "  \"name\":\"bogus\" }}", response);
     EXPECT_EQ("{ \"arguments\": {  }, \"result\": 0 }", response);
 
+    // Set max sample count to 1 to match test expectation (disabled state
+    // gauges normally have 2 samples).
+    sendUnixCommand("{ \"command\" : \"statistic-sample-count-set-all\", "
+                    "  \"arguments\": { \"max-samples\": 1 }}", response);
+    EXPECT_EQ("{ \"result\": 0, \"text\": \"All statistics count limit are set.\" }", response);
     // Check statistic-get-all
     sendUnixCommand("{ \"command\" : \"statistic-get-all\", "
                     "  \"arguments\": {}}", response);
 
     std::set<std::string> initial_stats = {
+        "disabled-by-db",
+        "disabled-by-ha-local",
+        "disabled-by-ha-remote",
+        "disabled-by-user",
+        "disabled-globally",
+        "pkt6-dhcp-disabled",
+        "pkt6-raw-received",
         "pkt6-received",
         "pkt6-solicit-received",
         "pkt6-advertise-received",

src/lib/dhcpsrv/network_state.cc

@@ -13,6 +13,7 @@
 #include <boost/enable_shared_from_this.hpp>
 #include <functional>
 #include <sstream>
+#include <stats/stats_mgr.h>
 #include <string>
 #include <unordered_set>
 
@@ -31,6 +32,7 @@ class NetworkStateImpl : public boost::enable_shared_from_this<NetworkStateImpl>
           disabled_subnets_(), disabled_networks_(),
           timer_mgr_(TimerMgr::instance()), disabled_by_origin_(),
           disabled_by_db_connection_(0) {
+        updateStats();
     }
 
     /// @brief Destructor.
@@ -76,6 +78,7 @@ class NetworkStateImpl : public boost::enable_shared_from_this<NetworkStateImpl>
                 globally_disabled_ = false;
             }
         }
+        updateStats();
     }
 
     /// @brief Reset internal counters for a database connection origin.
@@ -86,6 +89,7 @@ class NetworkStateImpl : public boost::enable_shared_from_this<NetworkStateImpl>
         if (disabled_by_origin_.empty()) {
             globally_disabled_ = false;
         }
+        updateStats();
     }
 
     /// @brief Enables DHCP service for an origin.
@@ -168,6 +172,46 @@ class NetworkStateImpl : public boost::enable_shared_from_this<NetworkStateImpl>
     /// @brief Flag which indicates the state has been disabled by a DB
     /// connection loss.
     uint32_t disabled_by_db_connection_;
+
+private:
+    /// @private
+
+    /// @brief Update monitoring metrics to expose disable states.
+    ///
+    /// A 0 value is used for false, 1 for true.
+    void updateStats() {
+        isc::stats::StatsMgr& stats_mgr = isc::stats::StatsMgr::instance();
+
+        stats_mgr.setValue("disabled-globally",
+                static_cast<int64_t>(globally_disabled_));
+
+        bool user = false, ha_local = false, ha_remote = false;
+        for (auto origin : disabled_by_origin_) {
+            switch (origin) {
+                case NetworkState::USER_COMMAND:
+                    user = true;
+                    break;
+                default:
+                    if ((NetworkState::HA_LOCAL_COMMAND <= origin) &&
+                        (origin < NetworkState::HA_REMOTE_COMMAND)) {
+                        ha_local = true;
+                    } else if ((NetworkState::HA_LOCAL_COMMAND <= origin) &&
+                        (origin < NetworkState::DB_CONNECTION)) {
+                        ha_remote = true;
+                    }
+                    break;
+            }
+        }
+        stats_mgr.setValue("disabled-by-user", static_cast<int64_t>(user));
+        stats_mgr.setValue("disabled-by-ha-local",
+                static_cast<int64_t>(ha_local));
+        stats_mgr.setValue("disabled-by-ha-remote",
+                static_cast<int64_t>(ha_remote));
+
+        // Expose the disabled-by-db-connection counter by direct value.
+        stats_mgr.setValue("disabled-by-db",
+                static_cast<int64_t>(disabled_by_db_connection_));
+    }
 };
 
 NetworkState::NetworkState(const NetworkState::ServerType& server_type)

src/lib/dhcpsrv/network_state.h

@@ -200,6 +200,9 @@ class NetworkState {
 
 private:
 
+    /// @brief Update monitoring metrics to expose disable states.
+    void updateStats();
+
     /// @brief Pointer to the @c NetworkState implementation.
     boost::shared_ptr<NetworkStateImpl> impl_;