From: Thomas Markwalder <tmark@isc.org>
Date: Tue, 2 Apr 2024 19:49:24 +0000 (-0400)
Subject: [#3297] Verify enable-monitoring and cleanup
X-Git-Tag: Kea-2.5.8~98
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=07f0b4e031cdfc659e7e88c799b37dbc34991970;p=thirdparty%2Fkea.git

[#3297] Verify enable-monitoring and cleanup

src/hooks/dhcp/perfmon/perfmon_callouts.cc
    pkt4_send(CalloutHandle& handle)
    pkt6_send(CalloutHandle& handle) - move query check, log
     and enable check into processPktEventStack()

src/hooks/dhcp/perfmon/perfmon_mgr.cc
    PerfMonMgr::processPktEventStack() - do log, query check,
    and enable check

src/hooks/dhcp/perfmon/tests/perfmon_mgr_unittests.cc
    testProcessPktEventStack() - updated to verify
    disabling monitoring inhibits duration data storage
---

diff --git a/src/hooks/dhcp/perfmon/perfmon_callouts.cc b/src/hooks/dhcp/perfmon/perfmon_callouts.cc
index 99bdbb77fe..fa029d0528 100644
--- a/src/hooks/dhcp/perfmon/perfmon_callouts.cc
+++ b/src/hooks/dhcp/perfmon/perfmon_callouts.cc
@@ -74,14 +74,6 @@ int pkt4_send(CalloutHandle& handle) {
     Subnet4Ptr subnet;
     handle.getArgument("subnet4", subnet);
 
-    if (!query) {
-        /// @todo this needs to be logged
-        return (0);
-    }
-
-    LOG_DEBUG(perfmon_logger, DBGLVL_TRACE_DETAIL, PERFMON_DHCP4_PKT_EVENTS)
-              .arg(query->getLabel())
-              .arg(query->dumpPktEvents());
     try {
         mgr->processPktEventStack(query, response, subnet->getID());
     } catch (const std::exception& ex) {
@@ -114,15 +106,6 @@ int pkt6_send(CalloutHandle& handle) {
     Subnet6Ptr subnet;
     handle.getArgument("subnet6", subnet);
 
-    if (!query) {
-        /// @todo this needs to be logged
-        return (0);
-    }
-
-    LOG_DEBUG(perfmon_logger, DBGLVL_TRACE_DETAIL, PERFMON_DHCP6_PKT_EVENTS)
-              .arg(query->getLabel())
-              .arg(query->dumpPktEvents());
-
     try {
         mgr->processPktEventStack(query, response, subnet->getID());
     } catch (const std::exception& ex) {
diff --git a/src/hooks/dhcp/perfmon/perfmon_mgr.cc b/src/hooks/dhcp/perfmon/perfmon_mgr.cc
index 79b7c1bea2..6d0739a33c 100644
--- a/src/hooks/dhcp/perfmon/perfmon_mgr.cc
+++ b/src/hooks/dhcp/perfmon/perfmon_mgr.cc
@@ -20,6 +20,7 @@ namespace perfmon {
 
 using namespace isc::data;
 using namespace isc::dhcp;
+using namespace isc::log;
 using namespace isc::stats;
 using namespace isc::util;
 using namespace boost::posix_time;
@@ -91,7 +92,15 @@ PerfMonMgr::processPktEventStack(isc::dhcp::PktPtr query,
                               << events.size());
     }
 
-    // no subnet id?
+    LOG_DEBUG(perfmon_logger, DBGLVL_TRACE_DETAIL,
+              (family_ == AF_INET ? PERFMON_DHCP4_PKT_EVENTS : PERFMON_DHCP6_PKT_EVENTS))
+              .arg(query->getLabel())
+              .arg(query->dumpPktEvents());
+
+    // If monitoring is disabled, then punt.
+    if (!enable_monitoring_) {
+        return;
+    }
 
     boost::posix_time::ptime start_time;
     boost::posix_time::ptime prev_time;
diff --git a/src/hooks/dhcp/perfmon/tests/perfmon_mgr_unittests.cc b/src/hooks/dhcp/perfmon/tests/perfmon_mgr_unittests.cc
index e51a4573d8..2ce563dfb9 100644
--- a/src/hooks/dhcp/perfmon/tests/perfmon_mgr_unittests.cc
+++ b/src/hooks/dhcp/perfmon/tests/perfmon_mgr_unittests.cc
@@ -507,7 +507,7 @@ public:
     void testProcessPktEventStack() {
         // Minimal valid configuration.
         std::string valid_config =
-            R"({ "enable-monitoring": true })";
+            R"({ "enable-monitoring": false })";
 
         ASSERT_NO_THROW_LOG(createMgr(valid_config));
 
@@ -529,11 +529,21 @@ public:
         query->addPktEvent("process_completed", event_time);
 
         // Now process the packets on two different subnets.
+        // With monitoring disabled no duration data should be stored.
         ASSERT_NO_THROW_LOG(mgr_->processPktEventStack(query, response, 22));
         ASSERT_NO_THROW_LOG(mgr_->processPktEventStack(query, response, 33));
 
         // Fetch all the durations in primary key order.
         MonitoredDurationCollectionPtr durations = mgr_->getDurationStore()->getAll();
+        ASSERT_EQ(durations->size(), 0);
+
+        // Enabled monitoring and process the queries again.
+        mgr_->setEnableMonitoring(true);
+        ASSERT_NO_THROW_LOG(mgr_->processPktEventStack(query, response, 22));
+        ASSERT_NO_THROW_LOG(mgr_->processPktEventStack(query, response, 33));
+
+        // Fetch all the durations in primary key order.
+        durations = mgr_->getDurationStore()->getAll();
         ASSERT_EQ(durations->size(), 12);
 
         // Contains the expected values for single duration.