From: JINMEI Tatuya Date: Sat, 7 Jan 2012 06:50:30 +0000 (-0800) Subject: [1399] added experimental support for per opcode statistics counters. X-Git-Tag: trac2351_base~281^2~3 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=70004148a3e39ca32f01d1ddd0cf0344df97b728;p=thirdparty%2Fkea.git [1399] added experimental support for per opcode statistics counters. --- diff --git a/src/bin/auth/auth.spec.pre.in b/src/bin/auth/auth.spec.pre.in index 2ce044e440..32748b9bb4 100644 --- a/src/bin/auth/auth.spec.pre.in +++ b/src/bin/auth/auth.spec.pre.in @@ -139,6 +139,134 @@ "item_default": 0, "item_title": "Queries UDP", "item_description": "A number of total query counts which all auth servers receive over UDP since they started initially" + }, + { + "item_name": "opcode.query", + "item_type": "integer", + "item_optional": true, + "item_default": 0, + "item_title": "Received query requests", + "item_description": "The number of total request counts whose opcode is query" + }, + { + "item_name": "opcode.iquery", + "item_type": "integer", + "item_optional": true, + "item_default": 0, + "item_title": "Received inverse query requests", + "item_description": "The number of total request counts whose opcode is inverse query" + }, + { + "item_name": "opcode.status", + "item_type": "integer", + "item_optional": true, + "item_default": 0, + "item_title": "Received status requests", + "item_description": "The number of total request counts whose opcode is status" + }, + { + "item_name": "opcode.reserved3", + "item_type": "integer", + "item_optional": true, + "item_default": 0, + "item_title": "Received requests opcode 3", + "item_description": "The number of total request counts whose opcode is 3 (reserved)" + }, + { + "item_name": "opcode.notify", + "item_type": "integer", + "item_optional": true, + "item_default": 0, + "item_title": "Received notify requests", + "item_description": "The number of total request counts whose opcode is notify" + }, + { + "item_name": "opcode.update", + "item_type": "integer", + "item_optional": true, + "item_default": 0, + "item_title": "Received update requests", + "item_description": "The number of total request counts whose opcode is update" + }, + { + "item_name": "opcode.reserved6", + "item_type": "integer", + "item_optional": true, + "item_default": 0, + "item_title": "Received requests opcode 6", + "item_description": "The number of total request counts whose opcode is 6 (reserved)" + }, + { + "item_name": "opcode.reserved7", + "item_type": "integer", + "item_optional": true, + "item_default": 0, + "item_title": "Received requests opcode 7", + "item_description": "The number of total request counts whose opcode is 7 (reserved)" + }, + { + "item_name": "opcode.reserved8", + "item_type": "integer", + "item_optional": true, + "item_default": 0, + "item_title": "Received requests opcode8", + "item_description": "The number of total request counts whose opcode is8 (reserved)" + }, + { + "item_name": "opcode.reserved9", + "item_type": "integer", + "item_optional": true, + "item_default": 0, + "item_title": "Received requests opcode9", + "item_description": "The number of total request counts whose opcode is9 (reserved)" + }, + { + "item_name": "opcode.reserved10", + "item_type": "integer", + "item_optional": true, + "item_default": 0, + "item_title": "Received requests opcode10", + "item_description": "The number of total request counts whose opcode is10 (reserved)" + }, + { + "item_name": "opcode.reserved11", + "item_type": "integer", + "item_optional": true, + "item_default": 0, + "item_title": "Received requests opcode11", + "item_description": "The number of total request counts whose opcode is11 (reserved)" + }, + { + "item_name": "opcode.reserved12", + "item_type": "integer", + "item_optional": true, + "item_default": 0, + "item_title": "Received requests opcode12", + "item_description": "The number of total request counts whose opcode is12 (reserved)" + }, + { + "item_name": "opcode.reserved13", + "item_type": "integer", + "item_optional": true, + "item_default": 0, + "item_title": "Received requests opcode13", + "item_description": "The number of total request counts whose opcode is13 (reserved)" + }, + { + "item_name": "opcode.reserved14", + "item_type": "integer", + "item_optional": true, + "item_default": 0, + "item_title": "Received requests opcode14", + "item_description": "The number of total request counts whose opcode is14 (reserved)" + }, + { + "item_name": "opcode.reserved15", + "item_type": "integer", + "item_optional": true, + "item_default": 0, + "item_title": "Received requests opcode15", + "item_description": "The number of total request counts whose opcode is15 (reserved)" } ] } diff --git a/src/bin/auth/auth_srv.cc b/src/bin/auth/auth_srv.cc index 9c5e594caa..f13b88ca12 100644 --- a/src/bin/auth/auth_srv.cc +++ b/src/bin/auth/auth_srv.cc @@ -457,12 +457,20 @@ AuthSrv::processMessage(const IOMessage& io_message, MessagePtr message, io_message.getDataSize()); } - bool sendAnswer = true; if (tsig_error != TSIGError::NOERROR()) { makeErrorMessage(message, buffer, tsig_error.toRcode(), tsig_context); - } else if (message->getOpcode() == Opcode::NOTIFY()) { - sendAnswer = impl_->processNotify(io_message, message, buffer, - tsig_context); + server->resume(true); + return; + } + + // update per opcode statistics counter. This can only be reliable after + // TSIG check succeeds. + impl_->counters_.inc(message->getOpcode()); + + bool send_answer = true; + if (message->getOpcode() == Opcode::NOTIFY()) { + send_answer = impl_->processNotify(io_message, message, buffer, + tsig_context); } else if (message->getOpcode() != Opcode::QUERY()) { LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_UNSUPPORTED_OPCODE) .arg(message->getOpcode().toText()); @@ -473,18 +481,18 @@ AuthSrv::processMessage(const IOMessage& io_message, MessagePtr message, ConstQuestionPtr question = *message->beginQuestion(); const RRType &qtype = question->getType(); if (qtype == RRType::AXFR()) { - sendAnswer = impl_->processXfrQuery(io_message, message, buffer, - tsig_context); + send_answer = impl_->processXfrQuery(io_message, message, buffer, + tsig_context); } else if (qtype == RRType::IXFR()) { - sendAnswer = impl_->processXfrQuery(io_message, message, buffer, - tsig_context); + send_answer = impl_->processXfrQuery(io_message, message, buffer, + tsig_context); } else { - sendAnswer = impl_->processNormalQuery(io_message, message, buffer, - tsig_context); + send_answer = impl_->processNormalQuery(io_message, message, + buffer, tsig_context); } } - server->resume(sendAnswer); + server->resume(send_answer); } bool @@ -770,6 +778,11 @@ AuthSrv::getCounter(const AuthCounters::ServerCounterType type) const { return (impl_->counters_.getCounter(type)); } +uint64_t +AuthSrv::getCounter(const Opcode opcode) const { + return (impl_->counters_.getCounter(opcode)); +} + const AddressList& AuthSrv::getListenAddresses() const { return (impl_->listen_addresses_); diff --git a/src/bin/auth/auth_srv.h b/src/bin/auth/auth_srv.h index a50e42759e..ad61fd865a 100644 --- a/src/bin/auth/auth_srv.h +++ b/src/bin/auth/auth_srv.h @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -345,6 +346,20 @@ public: /// \return the value of the counter. uint64_t getCounter(const AuthCounters::ServerCounterType type) const; + /// \brief Get the value of per Opcode counter in the Auth Counters. + /// + /// This function calls AuthCounters::getCounter(isc::dns::Opcode) and + /// returns its return value. + /// + /// \note This is a tentative interface as an attempt of experimentally + /// supporting more statistics counters. This should eventually be more + /// generalized. In any case, this method is mainly for testing. + /// + /// \throw None + /// \param opcode The opcode of the counter to get the value of + /// \return the value of the counter. + uint64_t getCounter(const isc::dns::Opcode opcode) const; + /** * \brief Set and get the addresses we listen on. */ diff --git a/src/bin/auth/statistics.cc b/src/bin/auth/statistics.cc index 7397a50f09..8a81bd5c34 100644 --- a/src/bin/auth/statistics.cc +++ b/src/bin/auth/statistics.cc @@ -15,17 +15,24 @@ #include #include +#include + #include #include #include #include +#include +#include +#include +#include #include #include #include +using namespace isc::dns; using namespace isc::auth; using namespace isc::statistics; @@ -40,6 +47,9 @@ public: AuthCountersImpl(); ~AuthCountersImpl(); void inc(const AuthCounters::ServerCounterType type); + void inc(const Opcode opcode) { + opcode_counter_.inc(opcode.getCode()); + } void inc(const std::string& zone, const AuthCounters::PerZoneCounterType type); bool submitStatistics() const; @@ -48,8 +58,13 @@ public: (AuthCounters::validator_type validator); // Currently for testing purpose only uint64_t getCounter(const AuthCounters::ServerCounterType type) const; + uint64_t getCounter(const Opcode opcode) const { + return (opcode_counter_.get(opcode.getCode())); + } private: Counter server_counter_; + Counter opcode_counter_; + static const size_t NUM_OPCODES = 16; CounterDictionary per_zone_counter_; isc::cc::AbstractSession* statistics_session_; AuthCounters::validator_type validator_; @@ -60,6 +75,7 @@ AuthCountersImpl::AuthCountersImpl() : // size of server_counter_: AuthCounters::SERVER_COUNTER_TYPES // size of per_zone_counter_: AuthCounters::PER_ZONE_COUNTER_TYPES server_counter_(AuthCounters::SERVER_COUNTER_TYPES), + opcode_counter_(NUM_OPCODES), per_zone_counter_(AuthCounters::PER_ZONE_COUNTER_TYPES), statistics_session_(NULL) { @@ -94,8 +110,22 @@ AuthCountersImpl::submitStatistics() const { << "{ \"queries.udp\": " << server_counter_.get(AuthCounters::SERVER_UDP_QUERY) << ", \"queries.tcp\": " - << server_counter_.get(AuthCounters::SERVER_TCP_QUERY) - << " }" + << server_counter_.get( + AuthCounters::SERVER_TCP_QUERY); + // Insert non 0 Opcode counters. + for (int i = 0; i < NUM_OPCODES; ++i) { + const Counter::Type counter = opcode_counter_.get(i); + if (counter != 0) { + // The counter item name should be derived lower-cased textual + // representation of the code. + std::string opcode_txt = Opcode(i).toText(); + std::transform(opcode_txt.begin(), opcode_txt.end(), + opcode_txt.begin(), ::tolower); + statistics_string << ", \"opcode." << opcode_txt << "\": " + << counter; + } + } + statistics_string << " }" << "}" << "]}"; isc::data::ConstElementPtr statistics_element = @@ -159,6 +189,11 @@ AuthCounters::inc(const AuthCounters::ServerCounterType type) { impl_->inc(type); } +void +AuthCounters::inc(const Opcode opcode) { + impl_->inc(opcode); +} + bool AuthCounters::submitStatistics() const { return (impl_->submitStatistics()); @@ -176,6 +211,11 @@ AuthCounters::getCounter(const AuthCounters::ServerCounterType type) const { return (impl_->getCounter(type)); } +uint64_t +AuthCounters::getCounter(const Opcode opcode) const { + return (impl_->getCounter(opcode)); +} + void AuthCounters::registerStatisticsValidator (AuthCounters::validator_type validator) const diff --git a/src/bin/auth/statistics.h b/src/bin/auth/statistics.h index 280b4a5d8f..8ca09ce18a 100644 --- a/src/bin/auth/statistics.h +++ b/src/bin/auth/statistics.h @@ -15,6 +15,8 @@ #ifndef __STATISTICS_H #define __STATISTICS_H 1 +#include + #include #include #include @@ -87,6 +89,15 @@ public: /// void inc(const ServerCounterType type); + /// \brief Increment the counter of a per opcode counter. + /// + /// \note This is a tentative interface. See \c getCounter(). + /// + /// \param opcode The opcode of the counter to increment. + /// + /// \throw None + void inc(const isc::dns::Opcode opcode); + /// \brief Submit statistics counters to statistics module. /// /// This method is desinged to be called periodically @@ -125,7 +136,7 @@ public: /// void setStatisticsSession(isc::cc::AbstractSession* statistics_session); - /// \brief Get a value of a counter in the AuthCounters. + /// \brief Get the value of a counter in the AuthCounters. /// /// This function returns a value of the counter specified by \a type. /// This method never throws an exception. @@ -135,9 +146,22 @@ public: /// \param type Type of a counter to get the value of /// /// \return the value of the counter specified by \a type. - /// uint64_t getCounter(const AuthCounters::ServerCounterType type) const; + /// \brief Get the value of a per opcode counter. + /// + /// This method returns the value of the per opcode counter for the + /// specified \c opcode. + /// + /// \note This is a tentative interface as an attempt of experimentally + /// supporting more statistics counters. This should eventually be more + /// generalized. In any case, this method is mainly for testing. + /// + /// \throw None + /// \param opcode The opcode of the counter to get the value of + /// \return the value of the counter. + uint64_t getCounter(const isc::dns::Opcode opcode) const; + /// \brief A type of validation function for the specification in /// isc::config::ModuleSpec. /// diff --git a/src/bin/auth/tests/auth_srv_unittest.cc b/src/bin/auth/tests/auth_srv_unittest.cc index 329a2dc775..4d3c1033a8 100644 --- a/src/bin/auth/tests/auth_srv_unittest.cc +++ b/src/bin/auth/tests/auth_srv_unittest.cc @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -372,6 +373,9 @@ TEST_F(AuthSrvTest, TSIGCheckFirst) { EXPECT_EQ(TSIGError::BAD_SIG_CODE, tsig->getRdata().getError()); EXPECT_EQ(0, tsig->getRdata().getMACSize()) << "It should be unsigned with this error"; + // TSIG should have failed, and so the per opcode counter shouldn't be + // incremented. + EXPECT_EQ(0, server.getCounter(Opcode::RESERVED14())); } TEST_F(AuthSrvTest, AXFRConnectFail) { @@ -835,6 +839,30 @@ TEST_F(AuthSrvTest, queryCounterTCPIXFR) { EXPECT_EQ(1, server.getCounter(AuthCounters::SERVER_TCP_QUERY)); } +TEST_F(AuthSrvTest, queryCounterOpcodes) { + for (int i = 0; i < 16; ++i) { + // The counter should be initialized to 0. + EXPECT_EQ(0, server.getCounter(Opcode(i))); + + // For each possible opcode, create a request message and send it + UnitTestUtil::createRequestMessage(request_message, Opcode(i), + default_qid, Name("example.com"), + RRClass::IN(), RRType::NS()); + createRequestPacket(request_message, IPPROTO_UDP); + + // "send" the request N-th times where N is i + 1 for i-th code. + // we intentionally use different values for each code + for (int j = 0; j <= i; ++j) { + parse_message->clear(Message::PARSE); + server.processMessage(*io_message, parse_message, response_obuffer, + &dnsserv); + } + + // Confirm the counter. + EXPECT_EQ(i + 1, server.getCounter(Opcode(i))); + } +} + // class for queryCounterUnexpected test // getProtocol() returns IPPROTO_IP class DummyUnknownSocket : public IOSocket { diff --git a/src/bin/auth/tests/statistics_unittest.cc b/src/bin/auth/tests/statistics_unittest.cc index 3f19f911f6..7d777fd995 100644 --- a/src/bin/auth/tests/statistics_unittest.cc +++ b/src/bin/auth/tests/statistics_unittest.cc @@ -14,10 +14,14 @@ #include +#include + #include #include +#include + #include #include @@ -170,6 +174,16 @@ TEST_F(AuthCountersTest, incrementInvalidCounter) { isc::OutOfRange); } +TEST_F(AuthCountersTest, incrementOpcodeCounter) { + // The counter should be initialized to 0. If we increment it by 1 + // the counter should be 1. + for (int i = 0; i < 16; ++i) { + EXPECT_EQ(0, counters.getCounter(Opcode(i))); + counters.inc(Opcode(i)); + EXPECT_EQ(1, counters.getCounter(Opcode(i))); + } +} + TEST_F(AuthCountersTest, submitStatisticsWithoutSession) { // Set statistics_session to NULL and call submitStatistics(). // Expect to return false. @@ -189,6 +203,28 @@ TEST_F(AuthCountersTest, submitStatisticsWithException) { statistics_session_.setThrowSessionTimeout(false); } +void +opcodeDataCheck(ConstElementPtr data, const int expected[16]) { + const char* item_names[] = { + "query", "iquery", "status", "reserved3", "notify", "update", + "reserved6", "reserved7", "reserved8", "reserved9", "reserved10", + "reserved11", "reserved12", "reserved13", "reserved14", "reserved15", + NULL + }; + int i; + for (i = 0; i < 16; ++i) { + ASSERT_NE(static_cast(NULL), item_names[i]); + const string item_name = "opcode." + string(item_names[i]); + if (expected[i] == 0) { + EXPECT_FALSE(data->get(item_name)); + } else { + EXPECT_EQ(expected[i], data->get(item_name)->intValue()); + } + } + // We should have examined all names + ASSERT_EQ(static_cast(NULL), item_names[i]); +} + TEST_F(AuthCountersTest, submitStatisticsWithoutValidator) { // Submit statistics data. // Validate if it submits correct data. @@ -217,6 +253,44 @@ TEST_F(AuthCountersTest, submitStatisticsWithoutValidator) { // UDP query counter is 2 and TCP query counter is 1. EXPECT_EQ(2, statistics_data->get("queries.udp")->intValue()); EXPECT_EQ(1, statistics_data->get("queries.tcp")->intValue()); + + // By default opcode counters are all 0 and omitted + const int opcode_results[16] = { 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0 }; + opcodeDataCheck(statistics_data, opcode_results); +} + +void +updateOpcodeCounters(AuthCounters &counters, const int expected[16]) { + for (int i = 0; i < 16; ++i) { + for (int j = 0; j < expected[i]; ++j) { + counters.inc(Opcode(i)); + } + } +} + +TEST_F(AuthCountersTest, submitStatisticsWithOpcodeCounters) { + // Increment some of the opcode counters. Then they should appear in the + // submitted data; others shouldn't + const int opcode_results[16] = { 1, 2, 3, 0, 4, 5, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0 }; + updateOpcodeCounters(counters, opcode_results); + counters.submitStatistics(); + ConstElementPtr statistics_data = statistics_session_.sent_msg + ->get("command")->get(1)->get("data"); + opcodeDataCheck(statistics_data, opcode_results); +} + +TEST_F(AuthCountersTest, submitStatisticsWithAllOpcodeCounters) { + // Increment all opcode counters. Then they should appear in the + // submitted data. + const int opcode_results[16] = { 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1 }; + updateOpcodeCounters(counters, opcode_results); + counters.submitStatistics(); + ConstElementPtr statistics_data = statistics_session_.sent_msg + ->get("command")->get(1)->get("data"); + opcodeDataCheck(statistics_data, opcode_results); } TEST_F(AuthCountersTest, submitStatisticsWithValidator) {