[3799] Updates per review comments

Change the -NAs and -PDs strings to -nas and -pds to be in line with
the rest of the statistic names

Remove @todo from CfgSubnet6::updateStatistics and CfgSubnet6::removeStatistics

ChangeLog

@@ -1,3 +1,9 @@
+XXX.	[func]		sar
+	Per IPv6 subnet statistics (subnet[id].assigned-nas, 
+	subnet[id].total-nas, subnet[id].assigned-pds, and subnet[id].total-pds)
+	has been implemented.
+	(Trac #3799, git TBD)
+
 957.	[func]		tomek
 	Per IPv4 subnet statistics (subnet[id].assigned-addresses and
 	subnet[id].total-addresses) has been implemented.

doc/guide/dhcp6-srv.xml

@@ -2605,7 +2605,7 @@ should include options from the isc option space:
           <tbody>
 
             <row>
-            <entry>subnet[id].total-NAs</entry>
+            <entry>subnet[id].total-nas</entry>
             <entry>integer</entry>
             <entry>
             This statistic shows the total number of NA addresses available for
@@ -2620,7 +2620,7 @@ should include options from the isc option space:
             </row>
 
             <row>
-            <entry>subnet[id].assigned-NAs</entry>
+            <entry>subnet[id].assigned-nas</entry>
             <entry>integer</entry>
             <entry>
             This statistic shows the number of NA addresses in a given subnet that
@@ -2635,7 +2635,7 @@ should include options from the isc option space:
             </row>
 
             <row>
-            <entry>subnet[id].total-PDs</entry>
+            <entry>subnet[id].total-pds</entry>
             <entry>integer</entry>
             <entry>
             This statistic shows the total number of PD prefixes available for
@@ -2650,7 +2650,7 @@ should include options from the isc option space:
             </row>
 
             <row>
-            <entry>subnet[id].assigned-PDs</entry>
+            <entry>subnet[id].assigned-pds</entry>
             <entry>integer</entry>
             <entry>
             This statistic shows the number of PD prefixes in a given subnet that

src/bin/dhcp6/dhcp6_srv.cc

@@ -2214,7 +2214,7 @@ Dhcpv6Srv::releaseIA_NA(const DuidPtr& duid, const Pkt6Ptr& query,
 
         // Need to decrease statistic for assigned addresses.
         StatsMgr::instance().addValue(
-            StatsMgr::generateName("subnet", lease->subnet_id_, "assigned-NAs"),
+            StatsMgr::generateName("subnet", lease->subnet_id_, "assigned-nas"),
             static_cast<int64_t>(-1));
 
         // Check if a lease has flags indicating that the FQDN update has
@@ -2370,7 +2370,7 @@ Dhcpv6Srv::releaseIA_PD(const DuidPtr& duid, const Pkt6Ptr& query,
 
         // Need to decrease statistic for assigned prefixes.
         StatsMgr::instance().addValue(
-            StatsMgr::generateName("subnet", lease->subnet_id_, "assigned-PDs"),
+            StatsMgr::generateName("subnet", lease->subnet_id_, "assigned-pds"),
             static_cast<int64_t>(-1));
     }
 

src/bin/dhcp6/tests/dhcp6_srv_unittest.cc

@@ -966,7 +966,7 @@ TEST_F(Dhcpv6SrvTest, pdRenewReject) {
 // - returned REPLY message has server-id
 // - returned REPLY message has IA_NA that does not include an IAADDR
 // - lease is actually removed from LeaseMgr
-// - assigned-NAs stats counter is properly decremented
+// - assigned-nas stats counter is properly decremented
 TEST_F(Dhcpv6SrvTest, ReleaseBasic) {
     testReleaseBasic(Lease::TYPE_NA, IOAddress("2001:db8:1:1::cafe:babe"),
                      IOAddress("2001:db8:1:1::cafe:babe"));
@@ -981,7 +981,7 @@ TEST_F(Dhcpv6SrvTest, ReleaseBasic) {
 // - returned REPLY message has server-id
 // - returned REPLY message has IA_PD that does not include an IAPREFIX
 // - lease is actually removed from LeaseMgr
-// - assigned-PDs stats counter is properly decremented
+// - assigned-pds stats counter is properly decremented
 TEST_F(Dhcpv6SrvTest, pdReleaseBasic) {
     testReleaseBasic(Lease::TYPE_PD, IOAddress("2001:db8:1:2::"),
                      IOAddress("2001:db8:1:2::"));
@@ -1000,7 +1000,7 @@ TEST_F(Dhcpv6SrvTest, pdReleaseBasic) {
 // - returned REPLY message has server-id
 // - returned REPLY message has IA_NA that includes STATUS-CODE
 // - No lease in LeaseMgr
-// - assigned-NAs stats counter is properly not decremented
+// - assigned-nas stats counter is properly not decremented
 TEST_F(Dhcpv6SrvTest, ReleaseReject) {
     testReleaseReject(Lease::TYPE_NA, IOAddress("2001:db8:1:1::dead"));
 }
@@ -1018,7 +1018,7 @@ TEST_F(Dhcpv6SrvTest, ReleaseReject) {
 // - returned REPLY message has server-id
 // - returned REPLY message has IA_PD that includes STATUS-CODE
 // - No lease in LeaseMgr
-// - assigned-PDs stats counter is properly not decremented
+// - assigned-pds stats counter is properly not decremented
 TEST_F(Dhcpv6SrvTest, pdReleaseReject) {
     testReleaseReject(Lease::TYPE_PD, IOAddress("2001:db8:1:2::"));
 }

src/bin/dhcp6/tests/dhcp6_test_utils.cc

@@ -594,8 +594,8 @@ Dhcpv6SrvTest::testReleaseBasic(Lease::Type type, const IOAddress& existing,
 
     // And prepopulate the stats counter
     std::string name = StatsMgr::generateName("subnet", subnet_->getID(),
-                                              type == Lease::TYPE_NA ? "assigned-NAs" :
-                                              "assigned-PDs");
+                                              type == Lease::TYPE_NA ? "assigned-nas" :
+                                              "assigned-pds");
     StatsMgr::instance().setValue(name, static_cast<int64_t>(1));
 
     // Let's create a RELEASE
@@ -671,8 +671,8 @@ Dhcpv6SrvTest::testReleaseReject(Lease::Type type, const IOAddress& addr) {
 
     // Pretend we have allocated 1 lease
     std::string name = StatsMgr::generateName("subnet", subnet_->getID(),
-                                              type == Lease::TYPE_NA ? "assigned-NAs" :
-                                              "assigned-PDs");
+                                              type == Lease::TYPE_NA ? "assigned-nas" :
+                                              "assigned-pds");
     StatsMgr::instance().setValue(name, static_cast<int64_t>(1));
 
     // Check that the lease is NOT in the database

src/lib/dhcpsrv/alloc_engine.cc

@@ -815,8 +815,8 @@ AllocEngine::removeNonmatchingReservedLeases6(ClientContext6& ctx,
         // Need to decrease statistic for assigned addresses.
         StatsMgr::instance().addValue(
             StatsMgr::generateName("subnet", ctx.subnet_->getID(),
-                                   ctx.type_ == Lease::TYPE_NA ? "assigned-NAs" :
-                                                                 "assigned-PDs"),
+                                   ctx.type_ == Lease::TYPE_NA ? "assigned-nas" :
+                                                                 "assigned-pds"),
             static_cast<int64_t>(-1));
 
         // In principle, we could trigger a hook here, but we will do this
@@ -878,8 +878,8 @@ AllocEngine::removeNonreservedLeases6(ClientContext6& ctx,
             // Need to decrease statistic for assigned addresses.
             StatsMgr::instance().addValue(
                 StatsMgr::generateName("subnet", ctx.subnet_->getID(),
-                                       ctx.type_ == Lease::TYPE_NA ? "assigned-NAs" :
-                                                                     "assigned-PDs"),
+                                       ctx.type_ == Lease::TYPE_NA ? "assigned-nas" :
+                                                                     "assigned-pds"),
                 static_cast<int64_t>(-1));
 
             /// @todo: Probably trigger a hook here
@@ -1043,8 +1043,8 @@ Lease6Ptr AllocEngine::createLease6(ClientContext6& ctx,
             if (ctx.subnet_->inPool(ctx.type_, addr)) {
                 StatsMgr::instance().addValue(
                     StatsMgr::generateName("subnet", ctx.subnet_->getID(),
-                                           ctx.type_ == Lease::TYPE_NA ? "assigned-NAs" :
-                                                                         "assigned-PDs"),
+                                           ctx.type_ == Lease::TYPE_NA ? "assigned-nas" :
+                                                                         "assigned-pds"),
                     static_cast<int64_t>(1));
             }
 
@@ -1165,7 +1165,7 @@ AllocEngine::extendLease6(ClientContext6& ctx, Lease6Ptr lease) {
 
         // Need to decrease statistic for assigned addresses.
         StatsMgr::instance().addValue(
-            StatsMgr::generateName("subnet", ctx.subnet_->getID(), "assigned-NAs"),
+            StatsMgr::generateName("subnet", ctx.subnet_->getID(), "assigned-nas"),
             static_cast<int64_t>(-1));
 
         // Add it to the removed leases list.

src/lib/dhcpsrv/cfg_subnets6.cc

@@ -184,27 +184,25 @@ void
 CfgSubnets6::removeStatistics() {
     using namespace isc::stats;
 
-    // For each v6 subnet currently configured, remove the statistic.
-    /// @todo: May move this to CfgSubnets6 class if there will be more
-    /// statistics here.
+    // For each v6 subnet currently configured, remove the statistics.
     for (Subnet6Collection::const_iterator subnet6 = subnets_.begin();
          subnet6 != subnets_.end(); ++subnet6) {
 
         StatsMgr::instance().del(StatsMgr::generateName("subnet",
                                                         (*subnet6)->getID(),
-                                                        "total-NAs"));
+                                                        "total-nas"));
 
         StatsMgr::instance().del(StatsMgr::generateName("subnet",
                                                         (*subnet6)->getID(),
-                                                        "assigned-NAs"));
+                                                        "assigned-nas"));
 
         StatsMgr::instance().del(StatsMgr::generateName("subnet",
                                                         (*subnet6)->getID(),
-                                                        "total-PDs"));
+                                                        "total-pds"));
 
         StatsMgr::instance().del(StatsMgr::generateName("subnet",
                                                         (*subnet6)->getID(),
-                                                        "assigned-PDs"));
+                                                        "assigned-pds"));
     }
 }
 
@@ -212,17 +210,15 @@ void
 CfgSubnets6::updateStatistics() {
     using namespace isc::stats;
 
-    /// @todo: May move this to CfgSubnets6 class if there will be more
-    /// statistics here.
     for (Subnet6Collection::const_iterator subnet = subnets_.begin();
          subnet != subnets_.end(); ++subnet) {
 
         StatsMgr::instance().setValue(
-            StatsMgr::generateName("subnet", (*subnet)->getID(), "total-NAs"),
+            StatsMgr::generateName("subnet", (*subnet)->getID(), "total-nas"),
             static_cast<int64_t>((*subnet)->getPoolCapacity(Lease::TYPE_NA)));
 
         StatsMgr::instance().setValue(
-            StatsMgr::generateName("subnet", (*subnet)->getID(), "total-PDs"),
+            StatsMgr::generateName("subnet", (*subnet)->getID(), "total-pds"),
             static_cast<int64_t>((*subnet)->getPoolCapacity(Lease::TYPE_PD)));
     }
 }

src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc

@@ -59,7 +59,7 @@ TEST_F(AllocEngine6Test, simpleAlloc6) {
     simpleAlloc6Test(pool_, IOAddress("::"), false);
 
     // We should have bumped the address counter by 1
-    string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-NAs");
+    string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-nas");
     ObservationPtr stat = StatsMgr::instance().getObservation(name);
     ASSERT_TRUE(stat);
     EXPECT_EQ(101, stat->getInteger().first);
@@ -72,7 +72,7 @@ TEST_F(AllocEngine6Test, pdSimpleAlloc6) {
     simpleAlloc6Test(pd_pool_, IOAddress("::"), false);
 
     // We should have bumped the address counter by 1
-    string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-PDs");
+    string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-pds");
     ObservationPtr stat = StatsMgr::instance().getObservation(name);
     ASSERT_TRUE(stat);
     EXPECT_EQ(101, stat->getInteger().first);
@@ -85,7 +85,7 @@ TEST_F(AllocEngine6Test, fakeAlloc6) {
     simpleAlloc6Test(pool_, IOAddress("::"), true);
 
     // We should not have bumped the address counter
-    string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-NAs");
+    string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-nas");
     ObservationPtr stat = StatsMgr::instance().getObservation(name);
     ASSERT_TRUE(stat);
     EXPECT_EQ(100, stat->getInteger().first);
@@ -97,7 +97,7 @@ TEST_F(AllocEngine6Test, pdFakeAlloc6) {
     simpleAlloc6Test(pd_pool_, IOAddress("::"), true);
 
     // We should not have bumped the address counter
-    string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-PDs");
+    string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-pds");
     ObservationPtr stat = StatsMgr::instance().getObservation(name);
     ASSERT_TRUE(stat);
     EXPECT_EQ(100, stat->getInteger().first);
@@ -559,7 +559,7 @@ TEST_F(AllocEngine6Test, requestReuseExpiredLease6) {
     ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
 
     // By default we pretend our subnet has 100 addresses
-    string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-NAs");
+    string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-nas");
     StatsMgr::instance().setValue(name, static_cast<int64_t>(100));
 
     // A client comes along, asking specifically for this address
@@ -643,7 +643,7 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolRequestNoHint) {
     AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
 
     // By default we pretend our subnet has 100 addresses
-    string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-NAs");
+    string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-nas");
     StatsMgr::instance().setValue(name, static_cast<int64_t>(100));
 
     Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("::"), false);
@@ -797,7 +797,7 @@ TEST_F(AllocEngine6Test, reservedAddressOutOfPoolRequestNoHint) {
     AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
 
     // By default we pretend our subnet has 100 addresses
-    string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-NAs");
+    string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-nas");
     StatsMgr::instance().setValue(name, static_cast<int64_t>(100));
 
     Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("::"), false, false);
@@ -925,7 +925,7 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolReassignedThis) {
     ASSERT_TRUE(lease1);
 
     // We should have bumped the address counter
-    string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-NAs");
+    string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-nas");
     ObservationPtr stat = StatsMgr::instance().getObservation(name);
     ASSERT_TRUE(stat);
     EXPECT_EQ(101, stat->getInteger().first);
@@ -985,7 +985,7 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolReassignedOther) {
     ASSERT_TRUE(lease1);
 
     // We should have bumped the address counter
-    string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-NAs");
+    string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-nas");
     ObservationPtr stat = StatsMgr::instance().getObservation(name);
     ASSERT_TRUE(stat);
     EXPECT_EQ(101, stat->getInteger().first);

src/lib/dhcpsrv/tests/alloc_engine_utils.cc

@@ -87,10 +87,10 @@ AllocEngine6Test::initSubnet(const asiolink::IOAddress& subnet,
 
     // By default we pretend our subnet has 100 addresses and prefixes allocated.
     StatsMgr::instance().setValue(
-        StatsMgr::generateName("subnet", subnet_->getID(), "assigned-NAs"),
+        StatsMgr::generateName("subnet", subnet_->getID(), "assigned-nas"),
         static_cast<int64_t>(100));
     StatsMgr::instance().setValue(
-        StatsMgr::generateName("subnet", subnet_->getID(), "assigned-PDs"),
+        StatsMgr::generateName("subnet", subnet_->getID(), "assigned-pds"),
         static_cast<int64_t>(100));
 }
 

src/lib/dhcpsrv/tests/cfgmgr_unittest.cc

@@ -654,11 +654,11 @@ TEST_F(CfgMgrTest, commitStats6) {
     CfgSubnets6Ptr subnets = cfg_mgr.getStagingCfg()->getCfgSubnets6();
     subnets->add(subnet1);
     cfg_mgr.commit();
-    stats_mgr.addValue("subnet[123].total-NAs", static_cast<int64_t>(256));
-    stats_mgr.setValue("subnet[123].assigned-NAs", static_cast<int64_t>(150));
+    stats_mgr.addValue("subnet[123].total-nas", static_cast<int64_t>(256));
+    stats_mgr.setValue("subnet[123].assigned-nas", static_cast<int64_t>(150));
 
-    stats_mgr.addValue("subnet[123].total-PDs", static_cast<int64_t>(256));
-    stats_mgr.setValue("subnet[123].assigned-PDs", static_cast<int64_t>(150));
+    stats_mgr.addValue("subnet[123].total-pds", static_cast<int64_t>(256));
+    stats_mgr.setValue("subnet[123].assigned-pds", static_cast<int64_t>(150));
 
     // Now, let's change the configuration to something new.
 
@@ -677,18 +677,18 @@ TEST_F(CfgMgrTest, commitStats6) {
     // Let's commit it
     cfg_mgr.commit();
 
-    EXPECT_FALSE(stats_mgr.getObservation("subnet[123].total-NAs"));
-    EXPECT_FALSE(stats_mgr.getObservation("subnet[123].assigned-NAs"));
+    EXPECT_FALSE(stats_mgr.getObservation("subnet[123].total-nas"));
+    EXPECT_FALSE(stats_mgr.getObservation("subnet[123].assigned-nas"));
 
-    EXPECT_FALSE(stats_mgr.getObservation("subnet[123].total-PDs"));
-    EXPECT_FALSE(stats_mgr.getObservation("subnet[123].assigned-PDs"));
+    EXPECT_FALSE(stats_mgr.getObservation("subnet[123].total-pds"));
+    EXPECT_FALSE(stats_mgr.getObservation("subnet[123].assigned-pds"));
 
     ObservationPtr total_addrs;
-    EXPECT_NO_THROW(total_addrs = stats_mgr.getObservation("subnet[42].total-NAs"));
+    EXPECT_NO_THROW(total_addrs = stats_mgr.getObservation("subnet[42].total-nas"));
     ASSERT_TRUE(total_addrs);
     EXPECT_EQ(128, total_addrs->getInteger().first);
 
-    EXPECT_NO_THROW(total_addrs = stats_mgr.getObservation("subnet[42].total-PDs"));
+    EXPECT_NO_THROW(total_addrs = stats_mgr.getObservation("subnet[42].total-pds"));
     ASSERT_TRUE(total_addrs);
     EXPECT_EQ(65536, total_addrs->getInteger().first);
 }
@@ -705,28 +705,28 @@ TEST_F(CfgMgrTest, clearStats6) {
     CfgSubnets6Ptr subnets = cfg_mgr.getStagingCfg()->getCfgSubnets6();
     subnets->add(subnet1);
     cfg_mgr.commit();
-    stats_mgr.addValue("subnet[123].total-NAs", static_cast<int64_t>(256));
-    stats_mgr.setValue("subnet[123].assigned-NAs", static_cast<int64_t>(150));
+    stats_mgr.addValue("subnet[123].total-nas", static_cast<int64_t>(256));
+    stats_mgr.setValue("subnet[123].assigned-nas", static_cast<int64_t>(150));
 
-    stats_mgr.addValue("subnet[123].total-PDs", static_cast<int64_t>(256));
-    stats_mgr.setValue("subnet[123].assigned-PDs", static_cast<int64_t>(150));
+    stats_mgr.addValue("subnet[123].total-pds", static_cast<int64_t>(256));
+    stats_mgr.setValue("subnet[123].assigned-pds", static_cast<int64_t>(150));
 
     // The stats should be there.
-    EXPECT_TRUE(stats_mgr.getObservation("subnet[123].total-NAs"));
-    EXPECT_TRUE(stats_mgr.getObservation("subnet[123].assigned-NAs"));
+    EXPECT_TRUE(stats_mgr.getObservation("subnet[123].total-nas"));
+    EXPECT_TRUE(stats_mgr.getObservation("subnet[123].assigned-nas"));
 
-    EXPECT_TRUE(stats_mgr.getObservation("subnet[123].total-PDs"));
-    EXPECT_TRUE(stats_mgr.getObservation("subnet[123].assigned-PDs"));
+    EXPECT_TRUE(stats_mgr.getObservation("subnet[123].total-pds"));
+    EXPECT_TRUE(stats_mgr.getObservation("subnet[123].assigned-pds"));
 
     // Let's remove all configurations
     cfg_mgr.clear();
 
     // The stats should not be there anymore.
-    EXPECT_FALSE(stats_mgr.getObservation("subnet[123].total-NAs"));
-    EXPECT_FALSE(stats_mgr.getObservation("subnet[123].assigned-NAs"));
+    EXPECT_FALSE(stats_mgr.getObservation("subnet[123].total-nas"));
+    EXPECT_FALSE(stats_mgr.getObservation("subnet[123].assigned-nas"));
 
-    EXPECT_FALSE(stats_mgr.getObservation("subnet[123].total-PDs"));
-    EXPECT_FALSE(stats_mgr.getObservation("subnet[123].assigned-PDs"));
+    EXPECT_FALSE(stats_mgr.getObservation("subnet[123].total-pds"));
+    EXPECT_FALSE(stats_mgr.getObservation("subnet[123].assigned-pds"));
 }
 
 /// @todo Add unit-tests for testing: