[#2419] Make suggested changes from Code Review

diff --git a/ChangeLog b/ChangeLog
index fc55646b84c78acbafa771a5e24b488681f73f38..48c43d2eda1c927125f9e9b39c6d2f2bbab18758 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,8 +1,8 @@
 2038.  [func]          djt
-       Add additional stats counters for host reservation
-       conflicts to global and subnet scope stats. The new global
-       stat is called v4-reservation-conflicts and the subnet
-       specific stat is called subnet[id].reservation-conflicts.
+       Added a new statistic to kea-dhcp4 that counts host
+       reservation conflicts. It is tracked at both the global
+       and subnet level as v4-reservation-conflicts and
+       subnet[id].v4-reservation-conflicts respectively.
        (Gitlab #2419)
 
 2037.  [bug]           razvan, marcin

diff --git a/doc/sphinx/arm/dhcp4-srv.rst b/doc/sphinx/arm/dhcp4-srv.rst
index 615465e3fe44367a8acbbf62768a1924ad15c202..534c18e4f7681b3308dfb7d84783b0a06a227e40 100644 (file)
--- a/doc/sphinx/arm/dhcp4-srv.rst
+++ b/doc/sphinx/arm/dhcp4-srv.rst
@@ -6574,17 +6574,17 @@ The DHCPv4 server supports the following statistics:
    |                                              |                | occurred across every subnet. When |
    |                                              |                | a client sends a DHCP Discover and |
    |                                              |                | is matched to a host reservation   |
-   |                                              |                | which is already leased out by     |
-   |                                              |                | another client, this counter is    |
-   |                                              |                | increased by 1.                    |
+   |                                              |                | which is already leased to another |
+   |                                              |                | client, this counter is increased  |
+   |                                              |                | by 1.                              |
    +----------------------------------------------+----------------+------------------------------------+
-   | subnet[id].reservation-conflicts             | integer        | Number of host reservation         |
+   | subnet[id].v4-reservation-conflicts          | integer        | Number of host reservation         |
    |                                              |                | allocation conflicts which have    |
    |                                              |                | occurred in a specific subnet.     |
    |                                              |                | When a client sends a DHCP         |
    |                                              |                | Discover and is matched to a host  |
    |                                              |                | reservation which is already       |
-   |                                              |                | leased out by another client, this |
+   |                                              |                | leased to another client, this     |
    |                                              |                | counter is increased by 1.         |
    +----------------------------------------------+----------------+------------------------------------+
 

diff --git a/src/bin/dhcp4/tests/dora_unittest.cc b/src/bin/dhcp4/tests/dora_unittest.cc
index 54a5a1b715aabda0167d200263f08f3c65f045f3..3af9acbc467e64db26b5f2fcb0fb3ce391dbb698 100644 (file)
--- a/src/bin/dhcp4/tests/dora_unittest.cc
+++ b/src/bin/dhcp4/tests/dora_unittest.cc
@@ -2026,7 +2026,7 @@ DORATest::reservationsWithConflicts() {
     ASSERT_NE(client_b_addr, in_pool_addr);
     // Ensure stats are being recorded for HR conflicts
     ObservationPtr subnet_conflicts = StatsMgr::instance().getObservation(
-        "subnet[1].reservation-conflicts");
+        "subnet[1].v4-reservation-conflicts");
     ASSERT_TRUE(subnet_conflicts);
     ASSERT_EQ(1, subnet_conflicts->getInteger().first);
     subnet_conflicts = StatsMgr::instance().getObservation("v4-reservation-conflicts");
@@ -2056,7 +2056,8 @@ DORATest::reservationsWithConflicts() {
     ASSERT_NE(clientB.config_.lease_.addr_, in_pool_addr);
     ASSERT_EQ(client_b_addr, clientB.config_.lease_.addr_);
     // Ensure stats are being recorded for HR conflicts
-    subnet_conflicts = StatsMgr::instance().getObservation("subnet[1].reservation-conflicts");
+    subnet_conflicts = StatsMgr::instance().getObservation(
+        "subnet[1].v4-reservation-conflicts");
     ASSERT_TRUE(subnet_conflicts);
     ASSERT_EQ(2, subnet_conflicts->getInteger().first);
     subnet_conflicts = StatsMgr::instance().getObservation("v4-reservation-conflicts");

diff --git a/src/bin/dhcp4/tests/host_unittest.cc b/src/bin/dhcp4/tests/host_unittest.cc
index acbd3d88bd85864b4a98aed85aa9d9727e06d93c..503198c0e2b6ecfe94378fe0e0b5671e4e684385 100644 (file)
--- a/src/bin/dhcp4/tests/host_unittest.cc
+++ b/src/bin/dhcp4/tests/host_unittest.cc
@@ -772,7 +772,7 @@ TEST_F(HostTest, firstClientGetsReservedAddress) {
     EXPECT_NE("10.0.0.123", resp->getYiaddr().toText());
     // Ensure stats are being recorded for HR conflicts
     ObservationPtr subnet_conflicts = StatsMgr::instance().getObservation(
-        "subnet[10].reservation-conflicts");
+        "subnet[10].v4-reservation-conflicts");
     ASSERT_TRUE(subnet_conflicts);
     ASSERT_EQ(1, subnet_conflicts->getInteger().first);
     subnet_conflicts = StatsMgr::instance().getObservation("v4-reservation-conflicts");

diff --git a/src/bin/dhcp4/tests/shared_network_unittest.cc b/src/bin/dhcp4/tests/shared_network_unittest.cc
index cc621713160bf1aab94979d61bb434512a6689aa..f9c137c85812bcf049065398a56792260aea870c 100644 (file)
--- a/src/bin/dhcp4/tests/shared_network_unittest.cc
+++ b/src/bin/dhcp4/tests/shared_network_unittest.cc
@@ -1707,7 +1707,7 @@ TEST_F(Dhcpv4SharedNetworkTest, reservationInSharedNetwork) {
     EXPECT_NE("192.0.2.28", resp1->getYiaddr().toText());
     // Ensure stats are being recorded for HR conflicts
     ObservationPtr subnet_conflicts = StatsMgr::instance().getObservation(
-        "subnet[10].reservation-conflicts");
+        "subnet[10].v4-reservation-conflicts");
     ASSERT_TRUE(subnet_conflicts);
     ASSERT_EQ(1, subnet_conflicts->getInteger().first);
     subnet_conflicts = StatsMgr::instance().getObservation("v4-reservation-conflicts");

diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index 444c95fca29c723d94090629e7777a5876eb3534..d45672b8761590a8a1de1d3e1fbae44a6fd1a8d0 100644 (file)
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -3704,7 +3704,7 @@ AllocEngine::discoverLease4(AllocEngine::ClientContext4& ctx) {
                 StatsMgr::instance().addValue(StatsMgr::generateName(
                                                   "subnet",
                                                   ctx.conflicting_lease_->subnet_id_,
-                                                  "reservation-conflicts"),
+                                                  "v4-reservation-conflicts"),
                                               static_cast<int64_t>(1));
                 StatsMgr::instance().addValue("v4-reservation-conflicts",
                                               static_cast<int64_t>(1));

diff --git a/src/lib/dhcpsrv/cfg_subnets4.cc b/src/lib/dhcpsrv/cfg_subnets4.cc
index f6fce4a79fc2af6f7b7f4f7be8a9c5389f803afd..3a32d6baa449f48165dfdca45acbba2315055501 100644 (file)
--- a/src/lib/dhcpsrv/cfg_subnets4.cc
+++ b/src/lib/dhcpsrv/cfg_subnets4.cc
@@ -532,7 +532,8 @@ CfgSubnets4::updateStatistics() {
         if (!stats_mgr.getObservation(name)) {
             stats_mgr.setValue(name, static_cast<int64_t>(0));
         }
-        name = StatsMgr::generateName("subnet", subnet_id, "reservation-conflicts");
+
+        name = StatsMgr::generateName("subnet", subnet_id, "v4-reservation-conflicts");
         if (!stats_mgr.getObservation(name)) {
             stats_mgr.setValue(name, static_cast<int64_t>(0));
         }

diff --git a/src/share/api/statistic-get-all.json b/src/share/api/statistic-get-all.json
index 3c8c814b840e2a03d626d30165856400eb9cce08..df1eae38b13564541960b7d247f6d7cb10b5dbdf 100644 (file)
--- a/src/share/api/statistic-get-all.json
+++ b/src/share/api/statistic-get-all.json
@@ -43,7 +43,7 @@
         "       \"subnet[1].reclaimed-declined-addresses\": [ [ 0, \"2022-02-11 17:54:17.487595\" ] ],",
         "       \"subnet[1].reclaimed-leases\": [ [ 0, \"2022-02-11 17:54:17.487604\" ] ],",
         "       \"subnet[1].total-addresses\": [ [ 200, \"2022-02-11 17:54:17.487512\" ] ],",
-        "       \"subnet[1].reservation-conflicts\": [ [ 0, \"2022-02-11 17:54:17.487520\" ] ]",
+        "       \"subnet[1].v4-reservation-conflicts\": [ [ 0, \"2022-02-11 17:54:17.487520\" ] ]",
         "       \"v4-allocation-fail\": [ [ 0, \"2022-02-11 17:54:17.455302\" ] ],",
         "       \"v4-allocation-fail-classes\": [ [ 0, \"2022-02-11 17:54:17.455306\" ] ],",
         "       \"v4-allocation-fail-no-pools\": [ [ 0, \"2022-02-11 17:54:17.455310\" ] ],",