From: Shawn Routhier Date: Fri, 19 Jun 2015 18:08:22 +0000 (-0700) Subject: [3799] Updates per review comments X-Git-Tag: trac3910_base~1^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7f5186e35e787c7c68e28a18cbe93fff23054b0b;p=thirdparty%2Fkea.git [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 --- diff --git a/ChangeLog b/ChangeLog index 17fc38a757..a919c41441 100644 --- a/ChangeLog +++ b/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. diff --git a/doc/guide/dhcp6-srv.xml b/doc/guide/dhcp6-srv.xml index 459ee501b2..7322019817 100644 --- a/doc/guide/dhcp6-srv.xml +++ b/doc/guide/dhcp6-srv.xml @@ -2605,7 +2605,7 @@ should include options from the isc option space: - subnet[id].total-NAs + subnet[id].total-nas integer This statistic shows the total number of NA addresses available for @@ -2620,7 +2620,7 @@ should include options from the isc option space: - subnet[id].assigned-NAs + subnet[id].assigned-nas integer 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: - subnet[id].total-PDs + subnet[id].total-pds integer This statistic shows the total number of PD prefixes available for @@ -2650,7 +2650,7 @@ should include options from the isc option space: - subnet[id].assigned-PDs + subnet[id].assigned-pds integer This statistic shows the number of PD prefixes in a given subnet that diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 7a853b91cf..b7d20eae8d 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/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(-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(-1)); } diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc index bc19453db4..2098871a41 100644 --- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc +++ b/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::")); } diff --git a/src/bin/dhcp6/tests/dhcp6_test_utils.cc b/src/bin/dhcp6/tests/dhcp6_test_utils.cc index 5beb1951f8..03847b3eb0 100644 --- a/src/bin/dhcp6/tests/dhcp6_test_utils.cc +++ b/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(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(1)); // Check that the lease is NOT in the database diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index 9103bc78a5..f5659bde95 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/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(-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(-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(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(-1)); // Add it to the removed leases list. diff --git a/src/lib/dhcpsrv/cfg_subnets6.cc b/src/lib/dhcpsrv/cfg_subnets6.cc index cc9c439263..125da803b4 100644 --- a/src/lib/dhcpsrv/cfg_subnets6.cc +++ b/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((*subnet)->getPoolCapacity(Lease::TYPE_NA))); StatsMgr::instance().setValue( - StatsMgr::generateName("subnet", (*subnet)->getID(), "total-PDs"), + StatsMgr::generateName("subnet", (*subnet)->getID(), "total-pds"), static_cast((*subnet)->getPoolCapacity(Lease::TYPE_PD))); } } diff --git a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc index 42a875101f..e15ecf0afd 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc +++ b/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(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(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(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); diff --git a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc b/src/lib/dhcpsrv/tests/alloc_engine_utils.cc index 5321da12e2..7b65171b0e 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc +++ b/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(100)); StatsMgr::instance().setValue( - StatsMgr::generateName("subnet", subnet_->getID(), "assigned-PDs"), + StatsMgr::generateName("subnet", subnet_->getID(), "assigned-pds"), static_cast(100)); } diff --git a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc index cf4949a422..d1ff4dfd34 100644 --- a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc +++ b/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(256)); - stats_mgr.setValue("subnet[123].assigned-NAs", static_cast(150)); + stats_mgr.addValue("subnet[123].total-nas", static_cast(256)); + stats_mgr.setValue("subnet[123].assigned-nas", static_cast(150)); - stats_mgr.addValue("subnet[123].total-PDs", static_cast(256)); - stats_mgr.setValue("subnet[123].assigned-PDs", static_cast(150)); + stats_mgr.addValue("subnet[123].total-pds", static_cast(256)); + stats_mgr.setValue("subnet[123].assigned-pds", static_cast(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(256)); - stats_mgr.setValue("subnet[123].assigned-NAs", static_cast(150)); + stats_mgr.addValue("subnet[123].total-nas", static_cast(256)); + stats_mgr.setValue("subnet[123].assigned-nas", static_cast(150)); - stats_mgr.addValue("subnet[123].total-PDs", static_cast(256)); - stats_mgr.setValue("subnet[123].assigned-PDs", static_cast(150)); + stats_mgr.addValue("subnet[123].total-pds", static_cast(256)); + stats_mgr.setValue("subnet[123].assigned-pds", static_cast(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: