[master] Merge branch 'trac3798' (per subnet stats in DHCPv4)

Conflicts:
	src/bin/dhcp4/tests/dhcp4_test_utils.cc

doc/guide/dhcp4-srv.xml

@@ -1841,11 +1841,11 @@ temporarily override a list of interface names and listen on all interfaces.
       The rules for determining the FQDN option are as follows:
       <orderedlist>
       <listitem><para>
-      If configured to do, so ignore the REQUEST contents and generate a
+      If configured to do, so ignore the DHCPREQUEST contents and generate a
       FQDN using a configurable prefix and suffix.
       </para></listitem>
       <listitem><para>
-      If the REQUEST contains the client FQDN option, the candidate
+      If the DHCPREQUEST contains the client FQDN option, the candidate
       name is taken from there, otherwise it is taken from the Host Name option.
       The candidate name may then be modified:
       <orderedlist>
@@ -2825,6 +2825,41 @@ temporarily override a list of interface names and listen on all interfaces.
         </table>
     </section>
 
+<!-- proper section structure added in ticket 3794, will merge it
+appropiately -->
+
+
+<section id="dummy">
+  <title>MERGE ME</title>
+  <para>
+    <itemizedlist>
+      <listitem>
+        <simpara><emphasis>subnet[id].total-addresses</emphasis> (integer) -
+        this statistic shows the total number of addresses available for the
+        DHCPv4 management. In other words, this is the sum of all addresses in
+        all configured pools. This statistic changes only during configuration
+        changes. Note it does not take into account any addresses that may be
+        reserved due to host reservation. The <emphasis>id</emphasis> is the
+        subnet-id of a given subnet. This statistic is exposed for each subnet
+	separately. This statistic is reset during reconfiguration event.
+	</simpara>
+      </listitem>
+      <listitem>
+        <simpara><emphasis>subnet[id].assigned-addresses</emphasis> (integer) -
+        this statistic shows the number of assigned addresses in a given subnet.
+        This statistic increases every time a new lease is allocated (as a result
+        of receiving a DHCPREQUEST message) and is decreased every time a lease is
+        released (a DHCPRELEASE message is received). When lease expiration
+        is implemented (planned for Kea 1.0), it will also decrease when a lease
+        is expired. The <emphasis>id</emphasis> is the subnet-id of a given
+        subnet. This statistic is exposed for each subnet separately. This
+        statistic is reset during reconfiguration event.
+	</simpara>
+      </listitem>
+    </itemizedlist>
+    </para>
+</section>
+
     <section id="dhcp4-std">
       <title>Supported DHCP Standards</title>
       <para>The following standards are currently supported:</para>
@@ -2832,8 +2867,9 @@ temporarily override a list of interface names and listen on all interfaces.
           <listitem>
             <simpara><emphasis>Dynamic Host Configuration Protocol</emphasis>,
             <ulink url="http://tools.ietf.org/html/rfc2131">RFC 2131</ulink>:
-            Supported messages are DISCOVER (1), OFFER (2),
-            REQUEST (3), RELEASE (7), INFORM (8), ACK (5), and NAK(6).</simpara>
+            Supported messages are DHCPDISCOVER (1), DHCPOFFER (2),
+            DHCPREQUEST (3), DHCPRELEASE (7), DHCPINFORM (8), DHCPACK (5), and
+            DHCPNAK(6).</simpara>
           </listitem>
           <listitem>
             <simpara><emphasis>DHCP Options and BOOTP Vendor Extensions</emphasis>,

src/bin/dhcp4/Makefile.am

@@ -75,6 +75,7 @@ kea_dhcp4_LDADD += $(top_builddir)/src/lib/asiolink/libkea-asiolink.la
 kea_dhcp4_LDADD += $(top_builddir)/src/lib/log/libkea-log.la
 kea_dhcp4_LDADD += $(top_builddir)/src/lib/config/libkea-cfgclient.la
 kea_dhcp4_LDADD += $(top_builddir)/src/lib/cc/libkea-cc.la
+kea_dhcp4_LDADD += $(top_builddir)/src/lib/stats/libkea-stats.la
 kea_dhcp4_LDADD += $(top_builddir)/src/lib/hooks/libkea-hooks.la
 kea_dhcp4_LDADD += $(top_builddir)/src/lib/stats/libkea-stats.la
 kea_dhcp4_LDADD += $(top_builddir)/src/lib/cryptolink/libkea-cryptolink.la

src/bin/dhcp4/dhcp4_srv.cc

@@ -40,6 +40,7 @@
 #include <hooks/callout_handle.h>
 #include <hooks/hooks_log.h>
 #include <hooks/hooks_manager.h>
+#include <stats/stats_mgr.h>
 #include <util/strutil.h>
 #include <stats/stats_mgr.h>
 #include <log/logger.h>
@@ -60,6 +61,7 @@ using namespace isc::dhcp;
 using namespace isc::dhcp_ddns;
 using namespace isc::hooks;
 using namespace isc::log;
+using namespace isc::stats;
 using namespace std;
 
 /// Structure that holds registered hook indexes
@@ -1789,6 +1791,11 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
                     .arg(release->getLabel())
                     .arg(lease->addr_.toText());
 
+                // Need to decrease statistic for assigned addresses.
+                StatsMgr::instance().addValue(
+                    StatsMgr::generateName("subnet", lease->subnet_id_, "assigned-addresses"),
+                    static_cast<int64_t>(-1));
+
                 if (CfgMgr::instance().ddnsEnabled()) {
                     // Remove existing DNS entries for the lease, if any.
                     queueNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, lease);

src/bin/dhcp4/tests/dhcp4_test_utils.cc

@@ -42,6 +42,10 @@ namespace test {
 
 Dhcpv4SrvTest::Dhcpv4SrvTest()
 :rcode_(-1), srv_(0) {
+
+    // Wipe any existing statistics
+    isc::stats::StatsMgr::instance().removeAll();
+
     subnet_ = Subnet4Ptr(new Subnet4(IOAddress("192.0.2.0"), 24, 1000,
                                      2000, 3000));
     pool_ = Pool4Ptr(new Pool4(IOAddress("192.0.2.100"), IOAddress("192.0.2.110")));

src/bin/dhcp4/tests/release_unittest.cc

@@ -21,13 +21,16 @@
 #include <dhcpsrv/subnet_id.h>
 #include <dhcp4/tests/dhcp4_test_utils.h>
 #include <dhcp4/tests/dhcp4_client.h>
+#include <stats/stats_mgr.h>
 #include <boost/shared_ptr.hpp>
+#include <sstream>
 
 using namespace isc;
 using namespace isc::asiolink;
 using namespace isc::data;
 using namespace isc::dhcp;
 using namespace isc::dhcp::test;
+using namespace isc::stats;
 
 namespace {
 
@@ -144,6 +147,18 @@ ReleaseTest::acquireAndRelease(const std::string& hw_address_1,
     // Perform 4-way exchange to obtain a new lease.
     acquireLease(client);
 
+    std::stringstream name;
+
+    // Let's get the subnet-id and generate statistics name out of it
+    const Subnet4Collection* subnets =
+        CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->getAll();
+    ASSERT_EQ(1, subnets->size());
+    name << "subnet[" << subnets->at(0)->getID() << "].assigned-addresses";
+
+    ObservationPtr assigned_cnt = StatsMgr::instance().getObservation(name.str());
+    ASSERT_TRUE(assigned_cnt);
+    uint64_t before = assigned_cnt->getInteger().first;
+
     // Remember the acquired address.
     IOAddress leased_address = client.config_.lease_.addr_;
 
@@ -157,14 +172,25 @@ ReleaseTest::acquireAndRelease(const std::string& hw_address_1,
     ASSERT_NO_THROW(client.doRelease());
     Lease4Ptr lease = LeaseMgrFactory::instance().getLease4(leased_address);
 
+    assigned_cnt = StatsMgr::instance().getObservation(name.str());
+    ASSERT_TRUE(assigned_cnt);
+    uint64_t after = assigned_cnt->getInteger().first;
+
     // We check if the release process was successful by checking if the
     // lease is in the database. It is expected that it is not present,
     // i.e. has been deleted with the release.
     if (expected_result == SHOULD_PASS) {
         EXPECT_FALSE(lease);
 
+        // The removal succeded, so the assigned-addresses statistic should
+        // be decreased by one
+        EXPECT_EQ(before, after + 1);
     } else {
         EXPECT_TRUE(lease);
+
+        // The removal failed, so the assigned-address should be the same
+        // as before
+        EXPECT_EQ(before, after);
     }
 }
 

src/lib/dhcpsrv/Makefile.am

@@ -150,6 +150,7 @@ libkea_dhcpsrv_la_LIBADD  += $(top_builddir)/src/lib/hooks/libkea-hooks.la
 libkea_dhcpsrv_la_LIBADD  += $(top_builddir)/src/lib/log/libkea-log.la
 libkea_dhcpsrv_la_LIBADD  += $(top_builddir)/src/lib/util/libkea-util.la
 libkea_dhcpsrv_la_LIBADD  += $(top_builddir)/src/lib/cc/libkea-cc.la
+libkea_dhcpsrv_la_LIBADD  += $(top_builddir)/src/lib/stats/libkea-stats.la
 libkea_dhcpsrv_la_LIBADD  += $(top_builddir)/src/lib/hooks/libkea-hooks.la
 libkea_dhcpsrv_la_LIBADD  += $(top_builddir)/src/lib/exceptions/libkea-exceptions.la
 

src/lib/dhcpsrv/alloc_engine.cc

@@ -21,11 +21,12 @@
 #include <dhcpsrv/host.h>
 #include <dhcpsrv/lease_mgr_factory.h>
 #include <dhcp/dhcp6.h>
-
+#include <stats/stats_mgr.h>
 #include <hooks/server_hooks.h>
 #include <hooks/hooks_manager.h>
 
 #include <cstring>
+#include <sstream>
 #include <limits>
 #include <vector>
 #include <stdint.h>
@@ -34,6 +35,7 @@
 using namespace isc::asiolink;
 using namespace isc::dhcp;
 using namespace isc::hooks;
+using namespace isc::stats;
 
 namespace {
 
@@ -1693,6 +1695,11 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) {
             .arg(client_lease->addr_.toText());
 
         lease_mgr.deleteLease(client_lease->addr_);
+
+        // Need to decrease statistic for assigned addresses.
+        StatsMgr::instance().addValue(
+            StatsMgr::generateName("subnet", ctx.subnet_->getID(), "assigned-addresses"),
+            static_cast<int64_t>(-1));
     }
 
     // Return the allocated lease or NULL pointer if allocation was
@@ -1770,6 +1777,12 @@ AllocEngine::createLease4(const ClientContext4& ctx, const IOAddress& addr) {
         // That is a real (REQUEST) allocation
         bool status = LeaseMgrFactory::instance().addLease(lease);
         if (status) {
+
+            // The lease insertion succeeded, let's bump up the statistic.
+            isc::stats::StatsMgr::instance().addValue(
+                StatsMgr::generateName("subnet", ctx.subnet_->getID(), "assigned-addresses"),
+                static_cast<int64_t>(1));
+
             return (lease);
         } else {
             // One of many failures with LeaseMgr (e.g. lost connection to the

src/lib/dhcpsrv/cfg_subnets4.cc

@@ -17,6 +17,7 @@
 #include <dhcpsrv/cfg_subnets4.h>
 #include <dhcpsrv/dhcpsrv_log.h>
 #include <dhcpsrv/subnet_id.h>
+#include <stats/stats_mgr.h>
 
 using namespace isc::asiolink;
 
@@ -109,7 +110,7 @@ CfgSubnets4::selectSubnet(const SubnetSelector& selector) const {
     }
 
     // We have identified an address in the client's packet that can be
-    // used for subnet selection. Match this packet with the subnets. 
+    // used for subnet selection. Match this packet with the subnets.
     return (selectSubnet(address, selector.client_classes_));
 }
 
@@ -145,8 +146,40 @@ CfgSubnets4::isDuplicate(const Subnet4& subnet) const {
     return (false);
 }
 
+void
+CfgSubnets4::removeStatistics() {
+    using namespace isc::stats;
+
+    // For each v4 subnet currently configured, remove the statistic.
+    /// @todo: May move this to CfgSubnets4 class if there will be more
+    /// statistics here.
+    for (Subnet4Collection::const_iterator subnet4 = subnets_.begin();
+         subnet4 != subnets_.end(); ++subnet4) {
+
+        StatsMgr::instance().del(StatsMgr::generateName("subnet",
+                                                        (*subnet4)->getID(),
+                                                        "total-addresses"));
+
+        StatsMgr::instance().del(StatsMgr::generateName("subnet",
+                                                        (*subnet4)->getID(),
+                                                        "assigned-addresses"));
+    }
+}
 
+void
+CfgSubnets4::updateStatistics() {
+    using namespace isc::stats;
 
+    /// @todo: May move this to CfgSubnets4 class if there will be more
+    /// statistics here.
+    for (Subnet4Collection::const_iterator subnet = subnets_.begin();
+         subnet != subnets_.end(); ++subnet) {
+
+        StatsMgr::instance().setValue(
+            StatsMgr::generateName("subnet", (*subnet)->getID(), "total-addresses"),
+            static_cast<int64_t>((*subnet)->getPoolCapacity(Lease::TYPE_V4)));
+    }
+}
 
 } // end of namespace isc::dhcp
 } // end of namespace isc

src/lib/dhcpsrv/cfg_subnets4.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -125,6 +125,23 @@ public:
                             const ClientClasses& client_classes
                             = ClientClasses()) const;
 
+    /// @brief Updates statistics.
+    ///
+    /// This method updates statistics that are affected by the newly committed
+    /// configuration. In particular, it updates the number of available addresses
+    /// in each subnet. Other statistics may be added in the future. In general,
+    /// these are statistics that are dependant only on configuration, so they are
+    /// not expected to change until the next reconfiguration event.
+    void updateStatistics();
+
+    /// @brief Removes statistics.
+    ///
+    /// During commitment of a new configuration, we need to get rid of the old
+    /// statistics for the old configuration. In particular, we need to remove
+    /// anything related to subnets, as there may be fewer subnets in the new
+    /// configuration and also subnet-ids may change.
+    void removeStatistics();
+
 private:
 
     /// @brief Checks that the IPv4 subnet with the given id already exists.

src/lib/dhcpsrv/cfgmgr.cc

@@ -19,10 +19,13 @@
 #include <dhcpsrv/cfgmgr.h>
 #include <dhcpsrv/dhcpsrv_log.h>
 #include <dhcpsrv/subnet_id.h>
+#include <stats/stats_mgr.h>
+#include <sstream>
 #include <string>
 
 using namespace isc::asiolink;
 using namespace isc::util;
+using namespace isc::stats;
 
 namespace isc {
 namespace dhcp {
@@ -110,13 +113,24 @@ CfgMgr::ensureCurrentAllocated() {
 
 void
 CfgMgr::clear() {
+    if (configuration_) {
+        configuration_->removeStatistics();
+    }
     configs_.clear();
     ensureCurrentAllocated();
 }
 
 void
 CfgMgr::commit() {
+
+
     ensureCurrentAllocated();
+
+    // First we need to remove statistics. The new configuration can have fewer
+    // subnets. Also, it may change subnet-ids. So we need to remove them all
+    // and add it back.
+    configuration_->removeStatistics();
+
     if (!configs_.back()->sequenceEquals(*configuration_)) {
         configuration_ = configs_.back();
         // Keep track of the maximum size of the configs history. Before adding
@@ -127,6 +141,9 @@ CfgMgr::commit() {
             configs_.erase(configs_.begin(), it);
         }
     }
+
+    // Now we need to set the statistics back.
+    configuration_->updateStatistics();
 }
 
 void

src/lib/dhcpsrv/cfgmgr.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above

src/lib/dhcpsrv/srv_config.cc

@@ -145,5 +145,20 @@ SrvConfig::equals(const SrvConfig& other) const {
             (*cfg_option_ == *other.cfg_option_));
 }
 
+void
+SrvConfig::removeStatistics() {
+
+    // For now, this method only removes statistics for v4 subnets, but in the
+    // near future, we'll also get statistics for v6 subnets.
+    getCfgSubnets4()->removeStatistics();
+}
+
+void
+SrvConfig::updateStatistics() {
+    // For now, this method only updates statistics for v4 subnets, but in the
+    // near future, we'll also get statistics for v6 subnets.
+    getCfgSubnets4()->updateStatistics();
+}
+
 }
 }

src/lib/dhcpsrv/srv_config.h

@@ -349,6 +349,17 @@ public:
 
     //@}
 
+    /// @brief Updates statistics.
+    ///
+    /// This method calls appropriate methods in child objects that update
+    /// related statistics. See @ref CfgSubnets4::updateStatistics for details.
+    void updateStatistics();
+
+    /// @brief Removes statistics.
+    ///
+    /// This method calls appropriate methods in child objects that remove
+    /// related statistics. See @ref CfgSubnets4::removeStatistics for details.
+    void removeStatistics();
 
 private:
 

src/lib/dhcpsrv/tests/Makefile.am

@@ -139,6 +139,7 @@ libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/cc/libkea-cc.la
 libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libkea-asiolink.la
 libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/hooks/libkea-hooks.la
 libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/log/libkea-log.la
+libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/stats/libkea-stats.la
 libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libkea-exceptions.la
 libdhcpsrv_unittests_LDADD += $(GTEST_LDADD)
 endif

src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc

@@ -16,10 +16,12 @@
 #include <dhcp/pkt4.h>
 #include <dhcpsrv/tests/alloc_engine_utils.h>
 #include <dhcpsrv/tests/test_utils.h>
+#include <stats/stats_mgr.h>
 
 using namespace std;
 using namespace isc::hooks;
 using namespace isc::asiolink;
+using namespace isc::stats;
 
 namespace isc {
 namespace dhcp {
@@ -80,7 +82,7 @@ TEST_F(AllocEngine4Test, simpleAlloc4) {
     detailCompareLease(lease, from_mgr);
 }
 
-// This test checks if the fake allocation (for DISCOVER) can succeed
+// This test checks if the fake allocation (for DHCPDISCOVER) can succeed
 TEST_F(AllocEngine4Test, fakeAlloc4) {
     boost::scoped_ptr<AllocEngine> engine;
     ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
@@ -431,7 +433,8 @@ TEST_F(AllocEngine4Test, outOfAddresses4) {
     EXPECT_FALSE(ctx.old_lease_);
 }
 
-// This test checks if an expired lease can be reused in DISCOVER (fake allocation)
+// This test checks if an expired lease can be reused in DHCPDISCOVER (fake
+// allocation)
 TEST_F(AllocEngine4Test, discoverReuseExpiredLease4) {
     boost::scoped_ptr<AllocEngine> engine;
     ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
@@ -1547,6 +1550,114 @@ TEST_F(AllocEngine4Test, findReservation) {
     EXPECT_FALSE(ctx.host_);
 }
 
+// This test checks if the simple IPv4 allocation can succeed and that
+// statistic for allocated addresses is increased appropriately.
+TEST_F(AllocEngine4Test, simpleAlloc4Stats) {
+    boost::scoped_ptr<AllocEngine> engine;
+    ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
+                                                 100, false)));
+    ASSERT_TRUE(engine);
+
+    AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_, IOAddress("0.0.0.0"),
+                                    false, true, "somehost.example.com.", false);
+
+    // Let's pretend 100 addresses were allocated already
+    stringstream name;
+    name << "subnet[" << subnet_->getID() << "].assigned-addresses";
+    StatsMgr::instance().addValue(name.str(), static_cast<int64_t>(100));
+
+    Lease4Ptr lease = engine->allocateLease4(ctx);
+    // The new lease has been allocated, so the old lease should not exist.
+    EXPECT_FALSE(ctx.old_lease_);
+
+    // Check that we got a lease
+    ASSERT_TRUE(lease);
+
+    // The statistic should be there and it should be increased by 1 (to 101).
+    ObservationPtr stat = StatsMgr::instance().getObservation(name.str());
+    ASSERT_TRUE(stat);
+    EXPECT_EQ(101, stat->getInteger().first);
+}
+
+// This test checks if the fake allocation (for DHCPDISCOVER) can succeed
+// and that it doesn't increase allocated-addresses statistic.
+TEST_F(AllocEngine4Test, fakeAlloc4Stat) {
+    boost::scoped_ptr<AllocEngine> engine;
+    ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
+                                                 100, false)));
+    ASSERT_TRUE(engine);
+
+    AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_,
+                                    IOAddress("0.0.0.0"), false, true,
+                                    "host.example.com.", true);
+
+    // Let's pretend 100 addresses were allocated already
+    stringstream name;
+    name << "subnet[" << subnet_->getID() << "].assigned-addresses";
+    StatsMgr::instance().addValue(name.str(), static_cast<int64_t>(100));
+
+    Lease4Ptr lease = engine->allocateLease4(ctx);
+
+    // The new lease has been allocated, so the old lease should not exist.
+    EXPECT_FALSE(ctx.old_lease_);
+
+    // Check that we got a lease
+    ASSERT_TRUE(lease);
+
+    // The statistic should be there and it should not be increased
+    // (should be still equal to 100).
+    ObservationPtr stat = StatsMgr::instance().getObservation(name.str());
+    ASSERT_TRUE(stat);
+    EXPECT_EQ(100, stat->getInteger().first);
+}
+
+// This test checks that the allocated-addresses statistic is decreased when
+// the client has a lease and a reservation for a different address is
+// available.
+TEST_F(AllocEngine4Test, reservedAddressExistingLeaseStat) {
+    // Create the reservation for the client.
+    HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(),
+                          Host::IDENT_HWADDR, subnet_->getID(),
+                          SubnetID(0), IOAddress("192.0.2.123")));
+    CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
+    CfgMgr::instance().commit();
+
+    // Create a lease for the client with a different address than the reserved
+    // one.
+    Lease4Ptr lease(new Lease4(IOAddress("192.0.2.101"), hwaddr_,
+                               &clientid_->getClientId()[0],
+                               clientid_->getClientId().size(),
+                               100, 30, 60, time(NULL), subnet_->getID(),
+                               false, false, ""));
+    LeaseMgrFactory::instance().addLease(lease);
+
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+
+    // Let's pretend 100 addresses were allocated already
+    stringstream name;
+    name << "subnet[" << subnet_->getID() << "].assigned-addresses";
+    StatsMgr::instance().addValue(name.str(), static_cast<int64_t>(100));
+
+    // Request allocation of the reserved address.
+    AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_,
+                                    IOAddress("192.0.2.123"), false, false,
+                                    "", false);
+    AllocEngine::findReservation(ctx);
+    Lease4Ptr allocated_lease = engine.allocateLease4(ctx);
+
+    ASSERT_TRUE(allocated_lease);
+
+    // The statistic should be still at 100. Note that it was decreased
+    // (because old lease was removed), but also increased (because the
+    // new lease was immediately allocated).
+    ObservationPtr stat = StatsMgr::instance().getObservation(name.str());
+    ASSERT_TRUE(stat);
+    EXPECT_EQ(100, stat->getInteger().first);
+
+    // Lets' double check that the actual allocation took place.
+    EXPECT_FALSE(ctx.fake_allocation_);
+}
+
 }; // namespace test
 }; // namespace dhcp
 }; // namespace isc

src/lib/dhcpsrv/tests/cfgmgr_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -20,6 +20,7 @@
 #include <dhcpsrv/cfgmgr.h>
 #include <dhcpsrv/subnet_id.h>
 #include <dhcpsrv/parsers/dhcp_parsers.h>
+#include <stats/stats_mgr.h>
 
 #include <gtest/gtest.h>
 
@@ -34,6 +35,7 @@ using namespace isc::data;
 using namespace isc::dhcp;
 using namespace isc::dhcp::test;
 using namespace isc::util;
+using namespace isc::stats;
 using namespace isc;
 
 // don't import the entire boost namespace.  It will unexpectedly hide uint8_t
@@ -574,6 +576,72 @@ TEST_F(CfgMgrTest, verbosity) {
     EXPECT_FALSE(CfgMgr::instance().isVerbose());
 }
 
+// This test verifies that once the configuration is committed, statistics
+// are updated appropriately.
+TEST_F(CfgMgrTest, commitStats) {
+    CfgMgr& cfg_mgr = CfgMgr::instance();
+    StatsMgr& stats_mgr = StatsMgr::instance();
+
+    // Let's prepare the "old" configuration: a subnet with id 123
+    // and pretent there ware addresses assigned, so statistics are non-zero.
+    Subnet4Ptr subnet1(new Subnet4(IOAddress("192.1.2.0"), 24, 1, 2, 3, 123));
+    CfgSubnets4Ptr subnets = cfg_mgr.getStagingCfg()->getCfgSubnets4();
+    subnets->add(subnet1);
+    cfg_mgr.commit();
+    stats_mgr.addValue("subnet[123].total-addresses", static_cast<int64_t>(256));
+    stats_mgr.setValue("subnet[123].assigned-addresses", static_cast<int64_t>(150));
+
+    // Now, let's change the configuration to something new.
+
+    // There's a subnet 192.1.2.0/24 with ID=42
+    Subnet4Ptr subnet2(new Subnet4(IOAddress("192.1.2.0"), 24, 1, 2, 3, 42));
+
+    // Let's make a pool with 128 addresses available.
+    PoolPtr pool(new Pool4(IOAddress("192.1.2.0"), 25)); // 128 addrs
+    subnet2->addPool(pool);
+
+    subnets = cfg_mgr.getStagingCfg()->getCfgSubnets4();
+    subnets->add(subnet2);
+
+    // Let's commit it
+    cfg_mgr.commit();
+
+    EXPECT_FALSE(stats_mgr.getObservation("subnet[123].total-addresses"));
+    EXPECT_FALSE(stats_mgr.getObservation("subnet[123].assigned-addresses"));
+
+    ObservationPtr total_addrs;
+    EXPECT_NO_THROW(total_addrs = stats_mgr.getObservation("subnet[42].total-addresses"));
+    ASSERT_TRUE(total_addrs);
+    EXPECT_EQ(128, total_addrs->getInteger().first);
+}
+
+// This test verifies that once the configuration is cleared, the statistics
+// are removed.
+TEST_F(CfgMgrTest, clearStats) {
+    CfgMgr& cfg_mgr = CfgMgr::instance();
+    StatsMgr& stats_mgr = StatsMgr::instance();
+
+    // Let's prepare the "old" configuration: a subnet with id 123
+    // and pretent there ware addresses assigned, so statistics are non-zero.
+    Subnet4Ptr subnet1(new Subnet4(IOAddress("192.1.2.0"), 24, 1, 2, 3, 123));
+    CfgSubnets4Ptr subnets = cfg_mgr.getStagingCfg()->getCfgSubnets4();
+    subnets->add(subnet1);
+    cfg_mgr.commit();
+    stats_mgr.addValue("subnet[123].total-addresses", static_cast<int64_t>(256));
+    stats_mgr.setValue("subnet[123].assigned-addresses", static_cast<int64_t>(150));
+
+    // The stats should be there.
+    EXPECT_TRUE(stats_mgr.getObservation("subnet[123].total-addresses"));
+    EXPECT_TRUE(stats_mgr.getObservation("subnet[123].assigned-addresses"));
+
+    // Let's remove all configurations
+    cfg_mgr.clear();
+
+    // The stats should not be there anymore.
+    EXPECT_FALSE(stats_mgr.getObservation("subnet[123].total-addresses"));
+    EXPECT_FALSE(stats_mgr.getObservation("subnet[123].assigned-addresses"));
+}
+
 /// @todo Add unit-tests for testing:
 /// - addActiveIface() with invalid interface name
 /// - addActiveIface() with the same interface twice

src/lib/stats/observation.cc

@@ -26,7 +26,7 @@ using namespace boost::posix_time;
 namespace isc {
 namespace stats {
 
-Observation::Observation(const std::string& name, const uint64_t value)
+Observation::Observation(const std::string& name, const int64_t value)
     :name_(name), type_(STAT_INTEGER) {
     setValue(value);
 }
@@ -46,7 +46,7 @@ Observation::Observation(const std::string& name, const std::string& value)
     setValue(value);
 }
 
-void Observation::addValue(const uint64_t value) {
+void Observation::addValue(const int64_t value) {
     IntegerSample current = getInteger();
     setValue(current.first + value);
 }
@@ -66,7 +66,7 @@ void Observation::addValue(const std::string& value) {
     setValue(current.first + value);
 }
 
-void Observation::setValue(const uint64_t value) {
+void Observation::setValue(const int64_t value) {
     setValueInternal(value, integer_samples_, STAT_INTEGER);
 }
 
@@ -208,7 +208,7 @@ Observation::getJSON() const {
 void Observation::reset() {
     switch(type_) {
     case STAT_INTEGER: {
-        setValue(static_cast<uint64_t>(0));
+        setValue(static_cast<int64_t>(0));
         return;
     }
     case STAT_FLOAT: {

src/lib/stats/observation.h

@@ -46,8 +46,8 @@ typedef boost::posix_time::time_duration StatsDuration;
 ///
 /// @{
 
-/// @brief Integer (implemented as unsigned 64-bit integer)
-typedef std::pair<uint64_t, boost::posix_time::ptime> IntegerSample;
+/// @brief Integer (implemented as signed 64-bit integer)
+typedef std::pair<int64_t, boost::posix_time::ptime> IntegerSample;
 
 /// @brief Float (implemented as double precision)
 typedef std::pair<double, boost::posix_time::ptime> FloatSample;
@@ -62,7 +62,7 @@ typedef std::pair<std::string, boost::posix_time::ptime> StringSample;
 
 /// @brief Represents a single observable characteristic (a 'statistic')
 ///
-/// Currently it supports one of four types: integer (implemented as unsigned 64
+/// Currently it supports one of four types: integer (implemented as signed 64
 /// bit integer), float (implemented as double), time duration (implemented with
 /// millisecond precision) and string. Absolute (setValue) and
 /// incremental (addValue) modes are supported. Statistic type is determined
@@ -85,7 +85,7 @@ class Observation {
     /// an easy to understand names were chosen (integer instead of uint64).
     /// To avoid confusion, we will support only one type of integer and only
     /// one type of floating points. Initially, these are represented by
-    /// uint64_t and double. If convincing use cases appear to change them
+    /// int64_t and double. If convincing use cases appear to change them
     /// to something else, we may change the underlying type.
     enum Type {
         STAT_INTEGER, ///< this statistic is unsinged 64-bit integer value
@@ -98,7 +98,7 @@ class Observation {
     ///
     /// @param name observation name
     /// @param value integer value observed.
-    Observation(const std::string& name, const uint64_t value);
+    Observation(const std::string& name, const int64_t value);
 
     /// @brief Constructor for floating point observations
     ///
@@ -122,7 +122,7 @@ class Observation {
     ///
     /// @param value integer value observed
     /// @throw InvalidStatType if statistic is not integer
-    void setValue(const uint64_t value);
+    void setValue(const int64_t value);
 
     /// @brief Records absolute floating point observation
     ///
@@ -146,7 +146,7 @@ class Observation {
     ///
     /// @param value integer value observed
     /// @throw InvalidStatType if statistic is not integer
-    void addValue(const uint64_t value);
+    void addValue(const int64_t value);
 
     /// @brief Records incremental floating point observation
     ///

src/lib/stats/stats_mgr.cc

@@ -30,7 +30,7 @@ StatsMgr::StatsMgr()
 
 }
 
-void StatsMgr::setValue(const std::string& name, const uint64_t value) {
+void StatsMgr::setValue(const std::string& name, const int64_t value) {
     setValueInternal(name, value);
 }
 
@@ -45,7 +45,7 @@ void StatsMgr::setValue(const std::string& name, const std::string& value) {
     setValueInternal(name, value);
 }
 
-void StatsMgr::addValue(const std::string& name, const uint64_t value) {
+void StatsMgr::addValue(const std::string& name, const int64_t value) {
     addValueInternal(name, value);
 }
 

src/lib/stats/stats_mgr.h

@@ -22,6 +22,7 @@
 #include <map>
 #include <string>
 #include <vector>
+#include <sstream>
 
 namespace isc {
 namespace stats {
@@ -48,6 +49,23 @@ namespace stats {
 /// is today, except happening in a separate thread. One unsolved issue in
 /// this approach is how to extract data, but that will remain unsolvable
 /// until we get the control socket implementation.
+///
+/// Statistics Manager does not use logging by design. The reasons are:
+/// - performance impact (logging every observation would degrade performance
+///   significantly. While it's possible to log on sufficiently high debug
+///   level, such a log would be not that useful)
+/// - dependency (statistics are intended to be a lightweight library, adding
+///   dependency on libkea-log, which has its own dependecies, including
+///   external log4cplus, is against 'lightweight' design)
+/// - if logging of specific statistics is warranted, it is recommended to
+///   add log entries in the code that calls StatsMgr.
+/// - enabling logging in StatsMgr does not offer fine tuning. It would be
+///   either all or nothing. Adding logging entries only when necessary
+///   in the code that uses StatsMgr gives better granularity.
+///
+/// If this decision is revisited in the futere, the most universal places
+/// for adding logging have been marked in @ref addValueInternal and
+/// @ref setValueInternal.
 class StatsMgr : public boost::noncopyable {
  public:
 
@@ -65,7 +83,7 @@ class StatsMgr : public boost::noncopyable {
     /// @param name name of the observation
     /// @param value integer value observed
     /// @throw InvalidStatType if statistic is not integer
-    void setValue(const std::string& name, const uint64_t value);
+    void setValue(const std::string& name, const int64_t value);
 
     /// @brief Records absolute floating point observation.
     ///
@@ -93,7 +111,7 @@ class StatsMgr : public boost::noncopyable {
     /// @param name name of the observation
     /// @param value integer value observed
     /// @throw InvalidStatType if statistic is not integer
-    void addValue(const std::string& name, const uint64_t value);
+    void addValue(const std::string& name, const int64_t value);
 
     /// @brief Records incremental floating point observation.
     ///
@@ -197,6 +215,28 @@ class StatsMgr : public boost::noncopyable {
     /// @return Pointer to the Observation object
     ObservationPtr getObservation(const std::string& name) const;
 
+    /// @brief Generates statistic name in a given context
+    ///
+    /// Example:
+    /// @code
+    /// generateName("subnet", 123, "received-packets");
+    /// @endcode
+    /// will return subnet[123].received-packets. Any printable type
+    /// can be used as index.
+    ///
+    /// @tparam Type any type that can be used to index contexts
+    /// @param context name of the context (e.g. 'subnet')
+    /// @param index value used for indexing contexts (e.g. subnet_id)
+    /// @param stat_name name of the statistic
+    /// @return returns full statistic name in form context[index].stat_name
+    template<typename Type>
+    static std::string generateName(const std::string& context, Type index,
+                             const std::string& stat_name) {
+        std::stringstream name;
+        name << context << "[" << index << "]." << stat_name;
+        return (name.str());
+    }
+
  private:
 
     /// @brief Sets a given statistic to specified value (internal version).
@@ -205,12 +245,14 @@ class StatsMgr : public boost::noncopyable {
     /// specified by value. This internal method is used by public @ref setValue
     /// methods.
     ///
-    /// @tparam DataType one of uint64_t, double, StatsDuration or string
+    /// @tparam DataType one of int64_t, double, StatsDuration or string
     /// @param name name of the statistic
     /// @param value specified statistic will be set to this value
     /// @throw InvalidStatType is statistic exists and has a different type.
     template<typename DataType>
     void setValueInternal(const std::string& name, DataType value) {
+
+        // If we want to log each observation, here would be the best place for it.
         ObservationPtr stat = getObservation(name);
         if (stat) {
             stat->setValue(value);
@@ -226,12 +268,14 @@ class StatsMgr : public boost::noncopyable {
     /// by name to a value). This internal method is used by public @ref setValue
     /// methods.
     ///
-    /// @tparam DataType one of uint64_t, double, StatsDuration or string
+    /// @tparam DataType one of int64_t, double, StatsDuration or string
     /// @param name name of the statistic
     /// @param value specified statistic will be set to this value
     /// @throw InvalidStatType is statistic exists and has a different type.
     template<typename DataType>
     void addValueInternal(const std::string& name, DataType value) {
+
+        // If we want to log each observation, here would be the best place for it.
         ObservationPtr existing = getObservation(name);
         if (!existing) {
             // We tried to add to a non-existing statistic. We can recover from

src/lib/stats/tests/observation_unittest.cc

@@ -42,7 +42,7 @@ public:
     /// @brief Constructor
     /// Initializes four observations.
     ObservationTest()
-        :a("alpha", static_cast<uint64_t>(1234)), // integer
+        :a("alpha", static_cast<int64_t>(1234)), // integer
          b("beta", 12.34), // float
          c("gamma", millisec::time_duration(1,2,3,4)), // duration
          d("delta", "1234") { // string
@@ -92,7 +92,7 @@ TEST_F(ObservationTest, constructor) {
 // given types.
 TEST_F(ObservationTest, setValue) {
 
-    EXPECT_NO_THROW(a.setValue(static_cast<uint64_t>(5678)));
+    EXPECT_NO_THROW(a.setValue(static_cast<int64_t>(5678)));
     EXPECT_NO_THROW(b.setValue(56e+78));
     EXPECT_NO_THROW(c.setValue(millisec::time_duration(5,6,7,8)));
     EXPECT_NO_THROW(d.setValue("fiveSixSevenEight"));
@@ -110,15 +110,15 @@ TEST_F(ObservationTest, setValue) {
     EXPECT_THROW(a.setValue(millisec::time_duration(5,6,7,8)), InvalidStatType);
     EXPECT_THROW(a.setValue("fiveSixSevenEight"), InvalidStatType);
 
-    EXPECT_THROW(b.setValue(static_cast<uint64_t>(5678)), InvalidStatType);
+    EXPECT_THROW(b.setValue(static_cast<int64_t>(5678)), InvalidStatType);
     EXPECT_THROW(b.setValue(millisec::time_duration(5,6,7,8)), InvalidStatType);
     EXPECT_THROW(b.setValue("fiveSixSevenEight"), InvalidStatType);
 
-    EXPECT_THROW(c.setValue(static_cast<uint64_t>(5678)), InvalidStatType);
+    EXPECT_THROW(c.setValue(static_cast<int64_t>(5678)), InvalidStatType);
     EXPECT_THROW(c.setValue(56e+78), InvalidStatType);
     EXPECT_THROW(c.setValue("fiveSixSevenEight"), InvalidStatType);
 
-    EXPECT_THROW(d.setValue(static_cast<uint64_t>(5678)), InvalidStatType);
+    EXPECT_THROW(d.setValue(static_cast<int64_t>(5678)), InvalidStatType);
     EXPECT_THROW(d.setValue(56e+78), InvalidStatType);
     EXPECT_THROW(d.setValue(millisec::time_duration(5,6,7,8)), InvalidStatType);
 }
@@ -130,7 +130,7 @@ TEST_F(ObservationTest, addValue) {
     // Note: all Observations were set to 1234,12.34 or similar in
     // ObservationTest constructor.
 
-    EXPECT_NO_THROW(a.addValue(static_cast<uint64_t>(5678)));
+    EXPECT_NO_THROW(a.addValue(static_cast<int64_t>(5678)));
     EXPECT_NO_THROW(b.addValue(56.78));
     EXPECT_NO_THROW(c.addValue(millisec::time_duration(5,6,7,8)));
     EXPECT_NO_THROW(d.addValue("fiveSixSevenEight"));
@@ -167,7 +167,7 @@ TEST_F(ObservationTest, timers) {
 // See http://kea.isc.org/wiki/StatsDesign for details.
 TEST_F(ObservationTest, integerToJSON) {
 
-    a.setValue(static_cast<uint64_t>(1234));
+    a.setValue(static_cast<int64_t>(1234));
 
     std::string exp = "[ [ 1234, \""
         + isc::util::ptimeToText(a.getInteger().second) + "\" ] ]";

src/lib/stats/tests/stats_mgr_unittest.cc

@@ -65,7 +65,7 @@ TEST_F(StatsMgrTest, basic) {
 // an integer statistic.
 TEST_F(StatsMgrTest, integerStat) {
     EXPECT_NO_THROW(StatsMgr::instance().setValue("alpha",
-                                                  static_cast<uint64_t>(1234)));
+                                                  static_cast<int64_t>(1234)));
 
     ObservationPtr alpha;
     EXPECT_NO_THROW(alpha = StatsMgr::instance().getObservation("alpha"));
@@ -140,13 +140,13 @@ TEST_F(StatsMgrTest, setLimits) {
 TEST_F(StatsMgrTest, getGetAll) {
 
     // Set a couple of statistics
-    StatsMgr::instance().setValue("alpha", static_cast<uint64_t>(1234));
+    StatsMgr::instance().setValue("alpha", static_cast<int64_t>(1234));
     StatsMgr::instance().setValue("beta", 12.34);
     StatsMgr::instance().setValue("gamma", time_duration(1,2,3,4));
     StatsMgr::instance().setValue("delta", "Lorem");
 
     // Now add some values to them
-    StatsMgr::instance().addValue("alpha", static_cast<uint64_t>(5678));
+    StatsMgr::instance().addValue("alpha", static_cast<int64_t>(5678));
     StatsMgr::instance().addValue("beta", 56.78);
     StatsMgr::instance().addValue("gamma", time_duration(5,6,7,8));
     StatsMgr::instance().addValue("delta", " ipsum");
@@ -210,7 +210,7 @@ TEST_F(StatsMgrTest, getGetAll) {
 TEST_F(StatsMgrTest, reset) {
 
     // Set a couple of statistics
-    StatsMgr::instance().setValue("alpha", static_cast<uint64_t>(1234));
+    StatsMgr::instance().setValue("alpha", static_cast<int64_t>(1234));
     StatsMgr::instance().setValue("beta", 12.34);
     StatsMgr::instance().setValue("gamma", time_duration(1,2,3,4));
     StatsMgr::instance().setValue("delta", "Lorem ipsum");
@@ -246,7 +246,7 @@ TEST_F(StatsMgrTest, reset) {
 TEST_F(StatsMgrTest, resetAll) {
 
     // Set a couple of statistics
-    StatsMgr::instance().setValue("alpha", static_cast<uint64_t>(1234));
+    StatsMgr::instance().setValue("alpha", static_cast<int64_t>(1234));
     StatsMgr::instance().setValue("beta", 12.34);
     StatsMgr::instance().setValue("gamma", time_duration(1,2,3,4));
     StatsMgr::instance().setValue("delta", "Lorem ipsum");
@@ -269,7 +269,7 @@ TEST_F(StatsMgrTest, resetAll) {
 TEST_F(StatsMgrTest, removeAll) {
 
     // Set a couple of statistics
-    StatsMgr::instance().setValue("alpha", static_cast<uint64_t>(1234));
+    StatsMgr::instance().setValue("alpha", static_cast<int64_t>(1234));
     StatsMgr::instance().setValue("beta", 12.34);
     StatsMgr::instance().setValue("gamma", time_duration(1,2,3,4));
     StatsMgr::instance().setValue("delta", "Lorem ipsum");
@@ -352,12 +352,12 @@ TEST_F(StatsMgrTest, DISABLED_performanceMultipleAdd) {
     for (uint32_t i = 0; i < stats; ++i) {
         std::stringstream tmp;
         tmp << "statistic" << i;
-        StatsMgr::instance().setValue(tmp.str(), static_cast<uint64_t>(i));
+        StatsMgr::instance().setValue(tmp.str(), static_cast<int64_t>(i));
     }
 
     ptime before = microsec_clock::local_time();
     for (uint32_t i = 0; i < cycles; ++i) {
-        StatsMgr::instance().addValue("metric1", static_cast<uint64_t>(i));
+        StatsMgr::instance().addValue("metric1", static_cast<int64_t>(i));
     }
     ptime after = microsec_clock::local_time();
 
@@ -382,12 +382,12 @@ TEST_F(StatsMgrTest, DISABLED_performanceMultipleSet) {
     for (uint32_t i = 0; i < stats; ++i) {
         std::stringstream tmp;
         tmp << "statistic" << i;
-        StatsMgr::instance().setValue(tmp.str(), static_cast<uint64_t>(i));
+        StatsMgr::instance().setValue(tmp.str(), static_cast<int64_t>(i));
     }
 
     ptime before = microsec_clock::local_time();
     for (uint32_t i = 0; i < cycles; ++i) {
-        StatsMgr::instance().setValue("metric1", static_cast<uint64_t>(i));
+        StatsMgr::instance().setValue("metric1", static_cast<int64_t>(i));
     }
     ptime after = microsec_clock::local_time();
 
@@ -397,4 +397,20 @@ TEST_F(StatsMgrTest, DISABLED_performanceMultipleSet) {
               << " times took: " << isc::util::durationToText(dur) << std::endl;
 }
 
+// Test checks whether statistics name can be generated using various
+// indexes.
+TEST_F(StatsMgrTest, generateName) {
+    // generateName is a templated method, so in principle anything printable
+    // to stream can be used as index. However, in practice only integers
+    // and possibly strings will be used.
+
+    // Let's text integer as index.
+    EXPECT_EQ("subnet[123].pkt4-received",
+              StatsMgr::generateName("subnet", 123, "pkt4-received"));
+
+    // Lets' test string as index.
+    EXPECT_EQ("subnet[foo].pkt4-received",
+              StatsMgr::generateName("subnet", "foo", "pkt4-received"));
+}
+
 };