Commit 0e3de871 authored by Tomek Mrugalski's avatar Tomek Mrugalski 🛰

[3798] Changes after review:

 - packet names updated in User's Guide
 - StatsMgr now uses int64_t, not uint64_t
 - StatsMgr::generateName() added
 - DISCOVER => DHCPDISCOVER
parent f2fd5b69
......@@ -1809,11 +1809,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>
......@@ -2606,8 +2606,8 @@ appropiately -->
<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 REQUEST message) and is decreased every time a lease is
released (a RELEASE message is received). When proper lease expiration
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
......@@ -2625,8 +2625,9 @@ appropiately -->
<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>,
......
......@@ -56,6 +56,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
......@@ -1747,10 +1748,9 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
.arg(lease->addr_.toText());
// Need to decrease statistic for assigned addresses.
std::stringstream name;
name << "subnet[" << lease->subnet_id_ << "].assigned-addresses";
isc::stats::StatsMgr::instance().addValue(name.str(),
static_cast<uint64_t>(-1));
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.
......
......@@ -34,6 +34,7 @@
using namespace isc::asiolink;
using namespace isc::dhcp;
using namespace isc::hooks;
using namespace isc::stats;
namespace {
......@@ -1549,10 +1550,9 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) {
lease_mgr.deleteLease(client_lease->addr_);
// Need to decrease statistic for assigned addresses.
std::stringstream name;
name << "subnet[" << ctx.subnet_->getID() << "].assigned-addresses";
isc::stats::StatsMgr::instance().addValue(name.str(),
static_cast<uint64_t>(-1));
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
......@@ -1631,10 +1631,9 @@ AllocEngine::createLease4(const ClientContext4& ctx, const IOAddress& addr) {
if (status) {
// The lease insertion succeeded, let's bump up the statistic.
std::stringstream name;
name << "subnet[" << ctx.subnet_->getID() << "].assigned-addresses";
isc::stats::StatsMgr::instance().addValue(name.str(),
static_cast<uint64_t>(1));
isc::stats::StatsMgr::instance().addValue(
StatsMgr::generateName("subnet", ctx.subnet_->getID(), "assigned-addresses"),
static_cast<int64_t>(1));
return (lease);
} else {
......
......@@ -228,7 +228,7 @@ CfgMgr::updateStatistics() {
name << "subnet[" << (*subnet)->getID() << "].total-addresses";
StatsMgr::instance().setValue(name.str(),
(*subnet)->getPoolCapacity(Lease::TYPE_V4));
static_cast<int64_t>((*subnet)->getPoolCapacity(Lease::TYPE_V4)));
}
}
......
......@@ -80,7 +80,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,
......@@ -422,7 +422,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,
......@@ -1519,7 +1520,7 @@ TEST_F(AllocEngine4Test, simpleAlloc4Stats) {
// Let's pretend 100 addresses were allocated already
stringstream name;
name << "subnet[" << subnet_->getID() << "].assigned-addresses";
StatsMgr::instance().addValue(name.str(), static_cast<uint64_t>(100));
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.
......@@ -1534,7 +1535,7 @@ TEST_F(AllocEngine4Test, simpleAlloc4Stats) {
EXPECT_EQ(101, stat->getInteger().first);
}
// This test checks if the fake allocation (for DISCOVER) can succeed
// 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;
......@@ -1549,7 +1550,7 @@ TEST_F(AllocEngine4Test, fakeAlloc4Stat) {
// Let's pretend 100 addresses were allocated already
stringstream name;
name << "subnet[" << subnet_->getID() << "].assigned-addresses";
StatsMgr::instance().addValue(name.str(), static_cast<uint64_t>(100));
StatsMgr::instance().addValue(name.str(), static_cast<int64_t>(100));
Lease4Ptr lease = engine->allocateLease4(ctx);
......@@ -1591,7 +1592,7 @@ TEST_F(AllocEngine4Test, reservedAddressExistingLeaseStat) {
// Let's pretend 100 addresses were allocated already
stringstream name;
name << "subnet[" << subnet_->getID() << "].assigned-addresses";
StatsMgr::instance().addValue(name.str(), static_cast<uint64_t>(100));
StatsMgr::instance().addValue(name.str(), static_cast<int64_t>(100));
// Request allocation of the reserved address.
AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_,
......
......@@ -588,8 +588,8 @@ TEST_F(CfgMgrTest, commitStats) {
CfgSubnets4Ptr subnets = cfg_mgr.getStagingCfg()->getCfgSubnets4();
subnets->add(subnet1);
cfg_mgr.commit();
stats_mgr.addValue("subnet[123].total-addresses", static_cast<uint64_t>(256));
stats_mgr.setValue("subnet[123].assigned-addresses", static_cast<uint64_t>(150));
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.
......
......@@ -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: {
......
......@@ -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
///
......
......@@ -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);
}
......
......@@ -22,6 +22,7 @@
#include <map>
#include <string>
#include <vector>
#include <sstream>
namespace isc {
namespace stats {
......@@ -65,7 +66,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 +94,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 +198,25 @@ 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:
/// generateName("subnet", 123, "received-packets") will return
/// subnet[123].received-packets.
///
/// @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,7 +225,7 @@ 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.
......@@ -226,7 +246,7 @@ 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.
......
......@@ -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) + "\" ] ]";
......
......@@ -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"));
}
};
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment